fix(studio): honor DOM edit failure outcomes

Author: miguel-heygen

packages/studio/src/components/editor/anchoredResizeCommitFeedsOffset.test.ts

@@ -1,6 +1,7 @@
 // @vitest-environment happy-dom
 
 import { afterEach, describe, expect, it, vi } from "vitest";
+import { DomEditSaveQueueOpenError } from "../../utils/domEditSaveQueue";
 import type { DomEditSelection } from "./domEditing";
 import type { GestureState, UseDomEditOverlayGesturesOptions } from "./domEditOverlayGestures";
 
@@ -75,7 +76,9 @@ interface CommitCall {
   offset: { x: number; y: number } | undefined;
 }
 
-function buildHarness() {
+function buildHarness(
+  onBoxSizeCommit?: UseDomEditOverlayGesturesOptions["onBoxSizeCommitRef"]["current"],
+) {
   const element = document.createElement("div");
   document.body.append(element);
 
@@ -136,9 +139,12 @@ function buildHarness() {
     onManualDragStartRef: ref(() => {}),
     onPathOffsetCommitRef: ref(() => {}),
     onGroupPathOffsetCommitRef: ref(() => {}),
-    onBoxSizeCommitRef: ref((_s, size, offset) => {
-      commits.push({ size, offset });
-    }),
+    onBoxSizeCommitRef: ref(
+      onBoxSizeCommit ??
+        ((_s, size, offset) => {
+          commits.push({ size, offset });
+        }),
+    ),
     onRotationCommitRef: ref(() => {}),
     onCanvasPointerMoveRef: ref(() => Promise.resolve(null)),
     onCanvasMouseDown: () => {},
@@ -172,6 +178,15 @@ function evt(clientX: number, clientY: number) {
   } as unknown as React.PointerEvent<HTMLDivElement>;
 }
 
+async function finishResize(handlers: ReturnType<typeof createDomEditOverlayGestureHandlers>) {
+  handlers.startGesture("resize", evt(ORIGIN_CENTER.x + 100, ORIGIN_CENTER.y), {
+    resizeHandle: "se",
+  });
+  handlers.onPointerMove(evt(ORIGIN_CENTER.x + 150, ORIGIN_CENTER.y));
+  handlers.onPointerUp(evt(ORIGIN_CENTER.x + 150, ORIGIN_CENTER.y));
+  await Promise.resolve();
+}
+
 afterEach(() => {
   document.body.innerHTML = "";
 });
@@ -211,4 +226,23 @@ describe("anchored corner resize — the release commit feeds the center-pin off
     expect(offset.x).toBeCloseTo(-(size.width - ORIGIN.width) / 2, 0);
     expect(offset.y).toBeCloseTo(-(size.height - ORIGIN.height) / 2, 0);
   });
+
+  it("does not log a paused save queue as an ordinary resize failure", async () => {
+    const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
+    const { handlers } = buildHarness(() => Promise.reject(new DomEditSaveQueueOpenError()));
+
+    await finishResize(handlers);
+
+    expect(consoleError).not.toHaveBeenCalled();
+  });
+
+  it("still logs an ordinary resize failure", async () => {
+    const failure = new Error("save failed");
+    const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
+    const { handlers } = buildHarness(() => Promise.reject(failure));
+
+    await finishResize(handlers);
+
+    expect(consoleError).toHaveBeenCalledWith("resize commit failed", failure);
+  });
 });

packages/studio/src/components/editor/useDomEditOverlayGestures.ts

@@ -58,6 +58,12 @@ import {
 import { logResize, logResizeMove, logResizeSettle } from "../../utils/resizeDebug";
 import { logDrag, logDragSettle, readDragPositions } from "../../utils/dragDebug";
 import { createGroupDragMover } from "./groupDragMove";
+import { DomEditSaveQueueOpenError } from "../../utils/domEditSaveQueue";
+
+function logGestureCommitFailure(message: string, error: unknown): void {
+  if (error instanceof DomEditSaveQueueOpenError) return;
+  console.error(message, error);
+}
 
 export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGesturesOptions) {
   const setDraftOverlayRect = (next: OverlayRect) => {
@@ -409,7 +415,7 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu
       }
       void Promise.resolve(opts.onRotationCommitRef.current(sel, finalRotation))
         .catch((error) => {
-          console.error("rotate commit failed", error);
+          logGestureCommitFailure("rotate commit failed", error);
           if (
             g.manualEditDragToken &&
             isStudioManualEditGestureCurrent(sel.element, g.manualEditDragToken)
@@ -493,7 +499,7 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu
         opts.onBoxSizeCommitRef.current(sel, finalSize, finalOffset ?? undefined, restore),
       )
         .catch((error) => {
-          console.error("resize commit failed", error);
+          logGestureCommitFailure("resize commit failed", error);
         })
         .finally(() => {
           if (member) endManualOffsetDragMembers([member]);

packages/studio/src/hooks/domEditCommitRunner.ts

@@ -61,9 +61,10 @@ export async function runDomEditCommit(config: DomEditCommitRunnerConfig): Promi
  *
  * `runDomEditCommit` resolves on persist failure by design (see its contract
  * above), so a caller cannot learn whether the write landed by awaiting it — a
- * failed commit and a successful one are indistinguishable. The human path does
- * not need to ask, because `onError` already put a toast on screen. A
- * programmatic caller has no screen, so it has to be told.
+ * failed persist and a successful one are indistinguishable. Capture and apply
+ * bugs still reject. The human path does not need to ask about handled persist
+ * failures, because `onError` already put a toast on screen. A programmatic
+ * caller has no screen, so it has to be told.
  */
 export type DomEditCommitDeclineReason =
   | "no-project"

packages/studio/src/hooks/useDomEditWiring.ts

@@ -107,7 +107,7 @@ export interface UseDomEditWiringParams {
     resolvedFromValues?: Record<string, number | string>,
   ) => Promise<void>;
   removeAllKeyframes: (sel: DomEditSelection, animId: string) => Promise<void>;
-  handleDomManualEditsReset: (sel: DomEditSelection) => void;
+  handleDomManualEditsReset: (sel: DomEditSelection) => Promise<void>;
 }
 
 // fallow-ignore-next-line complexity

packages/studio/src/hooks/useDomGeometryCommits.test.tsx

@@ -52,6 +52,7 @@ describe("useDomGeometryCommits rollback", () => {
       commits!.handleDomBoxSizeCommit(selection, { width: 200, height: 160 }, { x: 30, y: 40 }),
     ).rejects.toBe(failure);
     await expect(commits!.handleDomRotationCommit(selection, { angle: 45 })).rejects.toBe(failure);
+    await expect(commits!.handleDomManualEditsReset(selection)).rejects.toBe(failure);
 
     expect(readStudioPathOffset(element)).toEqual({ x: 10, y: 20 });
     expect(readStudioBoxSize(element)).toEqual({ width: 100, height: 80 });

packages/studio/src/hooks/useDomGeometryCommits.ts

@@ -130,6 +130,9 @@ export function useDomGeometryCommits({
   const handleDomManualEditsReset = useCallback(
     (selection: DomEditSelection) => {
       const element = selection.element;
+      const beforeOffset = captureStudioPathOffset(element);
+      const beforeSize = captureStudioBoxSize(element);
+      const beforeRotation = captureStudioRotation(element);
       const clearPatches = [
         ...buildClearPathOffsetPatches(element),
         ...buildClearBoxSizePatches(element),
@@ -139,11 +142,16 @@ export function useDomGeometryCommits({
       clearStudioBoxSize(element);
       clearStudioRotation(element);
       // skipRefresh:false triggers reloadPreview() which re-syncs selection on load
-      void commitPositionPatchToHtml(selection, clearPatches, {
+      return commitPositionPatchToHtml(selection, clearPatches, {
         label: "Reset layer edits",
         coalesceKey: `manual-reset:${getDomEditTargetKey(selection)}`,
         skipRefresh: false,
-      }).catch(() => undefined);
+      }).catch((error) => {
+        restoreStudioPathOffset(element, beforeOffset);
+        restoreStudioBoxSize(element, beforeSize);
+        restoreStudioRotation(element, beforeRotation);
+        throw error;
+      });
     },
     [commitPositionPatchToHtml],
   );

packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx

@@ -6,35 +6,60 @@ import { useElementLifecycleOps } from "./useElementLifecycleOps";
 import { makeLifecycleOpsParams } from "./elementLifecycleOpsTestUtils";
 import { mountReactHarness, makeSelection } from "./domSelectionTestHarness";
 
+Reflect.set(globalThis, "IS_REACT_ACT_ENVIRONMENT", true);
+
 function selectionFor(id: string) {
   const el = document.createElement("div");
   el.id = id;
   document.body.append(el);
   return { ...makeSelection(id, el), sourceFile: "index.html" };
 }
 
+function mountDeleteOps(overrides: Partial<Parameters<typeof useElementLifecycleOps>[0]> = {}) {
+  const captured: { ops: ReturnType<typeof useElementLifecycleOps> | null } = { ops: null };
+  function Probe() {
+    captured.ops = useElementLifecycleOps(
+      makeLifecycleOpsParams({
+        commitDomEditPatchBatches: vi.fn(async () => ({ ok: true }) as never),
+        ...overrides,
+      }),
+    );
+    return null;
+  }
+  mountReactHarness(<Probe />);
+  if (!captured.ops) throw new Error("hook did not initialize");
+  return captured.ops;
+}
+
 describe("useElementLifecycleOps — deleting a canvas multi-selection", () => {
   const removed: string[] = [];
   const requests: string[] = [];
   let changes = true;
+  let removeOk = true;
 
   beforeEach(() => {
     removed.length = 0;
     requests.length = 0;
     changes = true;
+    removeOk = true;
     vi.stubGlobal(
       "fetch",
       vi.fn(async (url: string, init?: RequestInit) => {
-        requests.push(String(url));
+        const requestUrl = String(url);
+        requests.push(requestUrl);
         const body = JSON.parse(String(init?.body ?? "{}")) as {
           targets?: { id?: string; selector?: string }[];
         };
-        for (const target of body.targets ?? []) {
-          const key = target.id ?? target.selector;
-          if (key) removed.push(key);
-        }
+        const keys = (body.targets ?? [])
+          .map((target) => target.id ?? target.selector)
+          .filter((key): key is string => key !== undefined);
+        removed.push(...keys);
+        const isRemove = requestUrl.includes("/file-mutations/remove-elements/");
+        const status = isRemove && !removeOk ? 500 : 200;
         return {
-          ok: true,
+          ok: status === 200,
+          status,
+          text: async () => (status === 200 ? "" : "server said no"),
           json: async () => ({ changed: changes, content: "<html></html>" }),
         } as unknown as Response;
       }),
@@ -48,28 +73,62 @@ describe("useElementLifecycleOps — deleting a canvas multi-selection", () => {
   it("removes every selected element, not just the first", async () => {
     // The reported bug: select several elements on the canvas, press Delete, and
     // one disappears while the rest stay — still drawn as selected.
-    let ops: ReturnType<typeof useElementLifecycleOps> | null = null;
-    function Probe() {
-      ops = useElementLifecycleOps(
-        makeLifecycleOpsParams({
-          commitDomEditPatchBatches: vi.fn(async () => ({ ok: true }) as never),
-          projectIdRef: { current: "p1" },
-        }),
-      );
-      return null;
-    }
-    mountReactHarness(<Probe />);
+    const ops = mountDeleteOps({ projectIdRef: { current: "p1" } });
 
     const selections = ["a", "b", "c"].map(selectionFor);
+    let outcome: unknown;
     await act(async () => {
-      await ops!.handleDomEditElementsDelete(selections);
+      outcome = await ops.handleDomEditElementsDelete(selections);
     });
 
     // The defect: only the first was ever removed.
     expect(removed).toEqual(["a", "b", "c"]);
     // And one request for the selection, not one per member: a canvas selection
     // runs to hundreds, and a round trip each made Delete look like a no-op.
     expect(requests.filter((url) => url.includes("remove-elements"))).toHaveLength(1);
+    expect(outcome).toEqual({ ok: true });
+  });
+
+  it("reports a successful SDK delete as landed", async () => {
+    const ops = mountDeleteOps({
+      projectIdRef: { current: "p1" },
+      onTrySdkDelete: vi.fn(async () => ({ status: "committed", version: "v1" }) as const),
+    });
+
+    const target = { ...selectionFor("a"), hfId: "hf-a" };
+    let outcome: unknown;
+    await act(async () => {
+      outcome = await ops.handleDomEditElementsDelete([target]);
+    });
+
+    expect(outcome).toEqual({ ok: true });
+    expect(requests.some((url) => url.includes("remove-elements"))).toBe(false);
+  });
+
+  it("reports missing project and selection without starting a request", async () => {
+    const projectIdRef = { current: null as string | null };
+    const ops = mountDeleteOps({ projectIdRef });
+
+    await expect(ops.handleDomEditElementsDelete([selectionFor("a")])).resolves.toEqual({
+      ok: false,
+      reason: "no-project",
+    });
+    projectIdRef.current = "p1";
+    await expect(ops.handleDomEditElementsDelete([])).resolves.toEqual({
+      ok: false,
+      reason: "no-selection",
+    });
+    expect(requests).toEqual([]);
+  });
+
+  it("reports an HTTP write failure instead of only toasting", async () => {
+    removeOk = false;
+    const ops = mountDeleteOps({ projectIdRef: { current: "p1" } });
+
+    await expect(ops.handleDomEditElementsDelete([selectionFor("a")])).resolves.toEqual({
+      ok: false,
+      reason: "persist-failed",
+    });
   });
 
   it("says so when the preview is stale instead of claiming a delete", async () => {
@@ -78,23 +137,14 @@ describe("useElementLifecycleOps — deleting a canvas multi-selection", () => {
     // nothing at all, with nothing on screen to explain it.
     changes = false;
     const showToast = vi.fn();
-    let ops: ReturnType<typeof useElementLifecycleOps> | null = null;
-    function Probe() {
-      ops = useElementLifecycleOps(
-        makeLifecycleOpsParams({
-          commitDomEditPatchBatches: vi.fn(async () => ({ ok: true }) as never),
-          projectIdRef: { current: "p1" },
-          showToast,
-        }),
-      );
-      return null;
-    }
-    mountReactHarness(<Probe />);
+    const ops = mountDeleteOps({ projectIdRef: { current: "p1" }, showToast });
 
+    let outcome: unknown;
     await act(async () => {
-      await ops!.handleDomEditElementsDelete([selectionFor("a")]);
+      outcome = await ops.handleDomEditElementsDelete([selectionFor("a")]);
     });
 
     expect(showToast.mock.calls.flat().join(" ")).toContain("out of date");
+    expect(outcome).toEqual({ ok: false, reason: "persist-failed" });
   });
 });

packages/studio/src/hooks/useElementLifecycleOps.ts

@@ -142,7 +142,7 @@ export function useElementLifecycleOps({
               `Deleted ${label}. Use Undo to restore ${sameFile.length === 1 ? "it" : "them"}.`,
               "info",
             );
-            return;
+            return { ok: true } as const;
           }
         }
 

packages/studio/src/hooks/useGsapSelectionHandlers.ts

@@ -111,7 +111,7 @@ export function useGsapSelectionHandlers({
   ) => Promise<void>;
   removeAllKeyframes: (sel: DomEditSelection, animId: string) => Promise<void>;
 
-  handleDomManualEditsReset: (sel: DomEditSelection) => void;
+  handleDomManualEditsReset: (sel: DomEditSelection) => Promise<void>;
   selectedGsapAnimations: GsapAnimation[];
   showToast: (message: string, tone?: "error" | "info") => void;
 }) {
@@ -230,7 +230,9 @@ export function useGsapSelectionHandlers({
         },
       );
       if (domEditSelection.element.hasAttribute("data-hf-studio-path-offset")) {
-        handleDomManualEditsReset(domEditSelection);
+        // The reset owns rollback and the position commit already owns user and
+        // telemetry reporting. This is only the fire-and-forget UI boundary.
+        void handleDomManualEditsReset(domEditSelection).catch(() => undefined);
       }
     },
     [domEditSelection, addGsapAnimation, handleDomManualEditsReset, trackGsapHandlerFailure],