fix(studio): an undo after an outside write shows the file the server restored

Author: miguel-heygen

packages/studio/src/App.tsx

@@ -155,6 +155,7 @@ export function StudioApp({ readOnlyPreview = false, readOnlyPreviewReason }: St
     previewPersistence,
     pendingTimelineEditPathRef,
     reloadPreview,
+    onOutsideChange: editHistory.noteOutsideChange,
   });
   const invalidateGsapCacheRef = useRef<() => void>(() => {});
   const invalidateGsapCache = useCallback(() => invalidateGsapCacheRef.current(), []);

packages/studio/src/hooks/useEditHistoryActions.paint.test.tsx

@@ -14,10 +14,11 @@ import { useEditHistoryActions } from "./useEditHistoryActions";
 import { usePersistentEditHistory } from "./usePersistentEditHistory";
 import { usePreviewPersistence } from "./usePreviewPersistence";
 
-const page = (left: string) =>
-  `<!doctype html><html><head></head><body><div id="root" data-composition-id="main"><div id="box" style="position: absolute; left: ${left}"></div></div></body></html>`;
+const page = (left: string, top = "0px") =>
+  `<!doctype html><html><head></head><body><div id="root" data-composition-id="main"><div id="box" style="position: absolute; left: ${left}"></div><div id="other" style="position: absolute; top: ${top}"></div></div></body></html>`;
 const BEFORE = page("10px");
 const AFTER = page("50px");
+const OUTSIDE = page("50px", "99px");
 
 const cleanup: Array<() => unknown> = [];
 let scratch = "";
@@ -85,6 +86,7 @@ async function studio() {
   await act(async () => root.render(createElement(Harness)));
   cleanup.push(() => act(() => root.unmount()));
   const box = () => iframe.contentDocument!.getElementById("box")!.style.left;
+  const other = () => iframe.contentDocument!.getElementById("other")!.style.top;
   const show = (html: string) => {
     iframe.contentDocument!.documentElement.innerHTML = new DOMParser().parseFromString(
       html,
@@ -108,6 +110,9 @@ async function studio() {
     persistence: () => persistence,
     actions: () => actions,
     box,
+    other,
+    show,
+    path,
     edit,
     file: () => readFileSync(path, "utf8"),
     reloads,
@@ -147,3 +152,15 @@ it("an undo pressed while a save is still running waits for the server instead o
   expect(s.file()).toBe(BEFORE);
   expect(s.box()).toBe("10px");
 });
+
+it("an undo after an outside write ends with the preview showing the file the server restored", async () => {
+  const s = await studio();
+  await s.edit();
+  writeFileSync(s.path, OUTSIDE);
+  s.show(OUTSIDE);
+
+  await act(() => s.actions().undo());
+  expect(s.file()).toBe(AFTER);
+  expect(s.other()).toBe("0px");
+  expect(s.box()).toBe("50px");
+});

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

@@ -12,6 +12,7 @@ let root: Root | null = null;
 afterEach(() => act(() => root?.unmount()));
 
 type RestoreFiles = Record<string, { previous: string; restored: string }>;
+type Prediction = { id: string; files: RestoreFiles };
 
 function mount(
   result: {
@@ -20,23 +21,26 @@ function mount(
     message?: string;
     label?: string;
     paths?: string[];
+    undoes?: string;
     files?: RestoreFiles;
   },
-  predicted?: RestoreFiles,
+  predicted?: Prediction,
 ) {
   const editHistory = {
     undo: vi.fn<EditHistoryHandle["undo"]>(async () => result),
     redo: vi.fn<EditHistoryHandle["redo"]>(async () => result),
     predict: () => predicted ?? null,
   };
+  const putBack = vi.fn();
   const deps = {
     editHistory,
     readOptionalProjectFile: vi.fn(async () => ""),
     readProjectFile: vi.fn(async () => ""),
     writeProjectFile: vi.fn(async () => undefined),
     showToast: vi.fn(),
     syncHistoryPreviewAfterApply: vi.fn(async (_restore: unknown) => undefined),
-    showHistoryRestoreNow: vi.fn((_files: RestoreFiles) => true),
+    putBack,
+    showHistoryRestoreNow: vi.fn((_files: RestoreFiles) => putBack),
     waitForPendingDomEditSaves: vi.fn(async () => undefined),
     onAfterUndoRedo: vi.fn(),
     activeCompPath: "index.html",
@@ -52,35 +56,53 @@ function mount(
   return { deps, actions };
 }
 
-const PREDICTED = { "index.html": { previous: "B", restored: "A" } };
+const PREDICTED = { id: "e1", files: { "index.html": { previous: "B", restored: "A" } } };
+const SERVER_FILES = { "index.html": { previous: "B", restored: "A2" } };
 
 describe("useEditHistoryActions", () => {
   it("corrects a shown step from the server's restore, diffed from what the preview shows", async () => {
-    const files = { "index.html": { previous: "B", restored: "A2" } };
     const { deps, actions } = mount(
-      { ok: true, label: "Undid: Move", paths: ["index.html"], files },
+      { ok: true, label: "Undid: Move", paths: ["index.html"], undoes: "e1", files: SERVER_FILES },
       PREDICTED,
     );
     await act(() => actions.undo());
-    expect(deps.showHistoryRestoreNow).toHaveBeenCalledWith(PREDICTED);
+    expect(deps.showHistoryRestoreNow).toHaveBeenCalledWith(PREDICTED.files);
+    expect(deps.putBack).not.toHaveBeenCalled();
     expect(deps.syncHistoryPreviewAfterApply).toHaveBeenCalledWith({
       paths: ["index.html"],
       files: { "index.html": { previous: "A", restored: "A2" } },
     });
   });
 
-  it("takes a shown step back when the server refuses it", async () => {
+  it("puts a shown step back and applies the server's own restore when it stepped another entry", async () => {
     const { deps, actions } = mount(
-      { ok: false, reason: "failed", message: "disk full" },
+      {
+        ok: true,
+        label: "Undid: Outside",
+        paths: ["index.html"],
+        undoes: "e0",
+        files: SERVER_FILES,
+      },
       PREDICTED,
     );
     await act(() => actions.undo());
+    expect(deps.putBack).toHaveBeenCalledTimes(1);
     expect(deps.syncHistoryPreviewAfterApply).toHaveBeenCalledWith({
       paths: ["index.html"],
-      files: { "index.html": { previous: "A", restored: "B" } },
+      files: SERVER_FILES,
     });
   });
 
+  it("puts a shown step back when the server refuses it", async () => {
+    const { deps, actions } = mount(
+      { ok: false, reason: "failed", message: "disk full" },
+      PREDICTED,
+    );
+    await act(() => actions.undo());
+    expect(deps.putBack).toHaveBeenCalledTimes(1);
+    expect(deps.syncHistoryPreviewAfterApply).not.toHaveBeenCalled();
+  });
+
   it("undo resyncs the preview and toasts the step as the history names it", async () => {
     const { deps, actions } = mount({ ok: true, label: "Undid: Move clip", paths: ["index.html"] });
     await act(() => actions.undo());

packages/studio/src/hooks/useEditHistoryActions.ts

@@ -2,25 +2,26 @@
 import { useCallback, useMemo } from "react";
 import { STUDIO_MOTION_PATH } from "../components/editor/studioMotion";
 import { serializeStudioFileMutations } from "../utils/studioFileMutationCoordinator";
+import type { RestoreFiles } from "../utils/gsapUndoRestore";
 
 interface HistoryResult {
   ok: boolean;
   reason?: string;
   message?: string;
   label?: string;
   paths?: string[];
+  undoes?: string;
   /** Per-file restored/previous content, used to soft-apply the preview. */
-  files?: Record<string, { previous: string; restored: string }>;
+  files?: RestoreFiles;
 }
-type RestoreFiles = Record<string, { previous: string; restored: string }>;
 interface HistoryFileCallbacks {
   readFile: (path: string) => Promise<string>;
   serialize?: <T>(paths: readonly string[], task: () => Promise<T>) => Promise<T>;
 }
 export interface EditHistoryHandle {
   undo: (cb: HistoryFileCallbacks) => Promise<HistoryResult>;
   redo: (cb: HistoryFileCallbacks) => Promise<HistoryResult>;
-  predict?: (direction: "undo" | "redo") => RestoreFiles | null;
+  predict?: (direction: "undo" | "redo") => { id: string; files: RestoreFiles } | null;
   state: {
     undo: ReadonlyArray<{ createdAt: number }>;
     redo: ReadonlyArray<{ createdAt: number }>;
@@ -34,7 +35,7 @@ export interface UseEditHistoryActionsOptions {
   writeProjectFile: (path: string, content: string) => Promise<void>;
   showToast: (message: string, tone?: "error" | "info") => void;
   syncHistoryPreviewAfterApply: (restore: Pick<HistoryResult, "paths" | "files">) => Promise<void>;
-  showHistoryRestoreNow?: (files: RestoreFiles) => boolean;
+  showHistoryRestoreNow?: (files: RestoreFiles) => (() => void) | null;
   waitForPendingDomEditSaves: () => Promise<void>;
   onAfterUndoRedo?: (restore: Pick<HistoryResult, "paths" | "files">) => void;
   /** Active composition path — decides whether undo/redo must resync the SDK session. */
@@ -73,21 +74,19 @@ export function useEditHistoryActions({
       const noun = direction === "undo" ? "Undo" : "Redo";
       // Paint the step in the key's own task when this tab knows it; the server's answer then confirms or corrects.
       const predicted = editHistory.predict?.(direction) ?? null;
-      const shown = predicted && showHistoryRestoreNow?.(predicted) ? predicted : null;
+      const putBack = predicted ? (showHistoryRestoreNow?.(predicted.files) ?? null) : null;
       let result: HistoryResult = { ok: false, reason: "failed" };
+      let serverSteppedShown = false;
       try {
         await waitForPendingDomEditSaves();
         result = await editHistory[direction]({
           readFile: readHistoryFile,
           serialize: serializeHistoryFiles,
         });
+        serverSteppedShown =
+          Boolean(putBack && result.ok && result.label) && result.undoes === predicted?.id;
       } finally {
-        if (shown && !(result.ok && result.label)) {
-          void syncHistoryPreviewAfterApply({
-            paths: Object.keys(shown),
-            files: swapRestore(shown),
-          });
-        }
+        if (putBack && !serverSteppedShown) putBack();
       }
       if (!result.ok && result.reason === "content-mismatch") {
         showToast(
@@ -101,7 +100,8 @@ export function useEditHistoryActions({
         return;
       }
       if (result.ok && result.label) {
-        const restore = { paths: result.paths, files: fromShown(result.files, shown) };
+        const files = serverSteppedShown ? fromShown(result.files, predicted!.files) : result.files;
+        const restore = { paths: result.paths, files };
         onAfterUndoRedo?.(restore);
         if (activeCompPath && result.paths?.includes(activeCompPath)) {
           forceReloadSdkSession?.();
@@ -129,17 +129,8 @@ export function useEditHistoryActions({
   return useMemo(() => ({ undo, redo }), [undo, redo]);
 }
 
-function swapRestore(files: RestoreFiles): RestoreFiles {
-  return Object.fromEntries(
-    Object.entries(files).map(([path, f]) => [
-      path,
-      { previous: f.restored, restored: f.previous },
-    ]),
-  );
-}
-
-function fromShown(files: RestoreFiles | undefined, shown: RestoreFiles | null) {
-  if (!files || !shown) return files;
+function fromShown(files: RestoreFiles | undefined, shown: RestoreFiles) {
+  if (!files) return files;
   return Object.fromEntries(
     Object.entries(files).map(([path, f]) => [
       path,

packages/studio/src/hooks/usePersistentEditHistory.test.ts

@@ -76,6 +76,7 @@ it("an edit Studio saved is undone and redone by the project's history, with the
   expect(undone).toEqual({
     ok: true,
     label: "Undid: Moved Title",
+    undoes: expect.any(String),
     paths: ["index.html"],
     files: { "index.html": { previous: "B", restored: "A" } },
   });
@@ -251,15 +252,23 @@ it("predicts a step from what this tab wrote, and not while a claim or step may
   });
   expect(hook().predict("undo")).toBeNull();
   await act(() => claim);
-  await vi.waitFor(() =>
-    expect(hook().predict("undo")).toEqual({ "index.html": { previous: "B", restored: "A" } }),
-  );
+  const predicted = await vi.waitFor(() => {
+    const next = hook().predict("undo");
+    expect(next?.files).toEqual({ "index.html": { previous: "B", restored: "A" } });
+    return next!;
+  });
+
+  hook().noteOutsideChange();
+  expect(hook().predict("undo")).toBeNull();
+  await vi.waitFor(() => expect(hook().predict("undo")?.id).toBe(predicted.id));
 
   const undone = hook().undo({ readFile });
   expect(hook().predict("undo")).toBeNull();
-  await act(() => undone);
+  expect((await act(() => undone)).undoes).toBe(predicted.id);
   await vi.waitFor(() =>
-    expect(hook().predict("redo")).toEqual({ "index.html": { previous: "A", restored: "B" } }),
+    expect(hook().predict("redo")?.files).toEqual({
+      "index.html": { previous: "A", restored: "B" },
+    }),
   );
 });
 

packages/studio/src/hooks/usePersistentEditHistory.ts

@@ -1,6 +1,7 @@
 import { useCallback, useEffect, useMemo, useRef, useState } from "react";
 import type { HistoryListItem, HistoryResult } from "@hyperframes/studio-server";
 import { studioFileContentVersion, studioWriteHeaders } from "../utils/studioFileVersion";
+import type { RestoreFiles } from "../utils/gsapUndoRestore";
 
 interface RecordEditInput {
   label: string;
@@ -18,19 +19,15 @@ export interface UsePersistentEditHistoryOptions {
   projectId: string | null;
 }
 
-interface ApplyRestoredFile {
-  previous: string;
-  restored: string;
-}
-
 interface ApplyResult {
   ok: boolean;
   /** content-mismatch: `paths` changed after the step's entry. failed: `message` says why. */
   reason?: "empty" | "content-mismatch" | "failed";
   message?: string;
   label?: string;
   paths?: string[];
-  files?: Record<string, ApplyRestoredFile>;
+  undoes?: string;
+  files?: RestoreFiles;
 }
 
 interface NextStep {
@@ -87,7 +84,7 @@ function createOwnHistory() {
     },
     afterOf: (id: string | undefined): Record<string, string> =>
       Object.fromEntries(Object.entries((id && own.get(id)) || {}).map(([p, f]) => [p, f.after])),
-    predict: (direction: "undo" | "redo"): Record<string, ApplyRestoredFile> | null => {
+    predict: (direction: "undo" | "redo"): { id: string; files: RestoreFiles } | null => {
       const step = next?.[direction];
       const files = step ? own.get(step.id) : undefined;
       const paths = files ? Object.keys(files) : [];
@@ -97,12 +94,11 @@ function createOwnHistory() {
         !paths.every((p) => step!.paths.includes(p))
       )
         return null;
-      return Object.fromEntries(
-        paths.map((path) => [
-          path,
-          { previous: files[path]!.after, restored: files[path]!.before },
-        ]),
-      );
+      const restore = paths.map((path) => [
+        path,
+        { previous: files[path]!.after, restored: files[path]!.before },
+      ]);
+      return { id: step!.id, files: Object.fromEntries(restore) };
     },
     clear: () => {
       own.clear();
@@ -171,7 +167,7 @@ async function restoredFiles(
   paths: readonly string[],
   previous: Record<string, string> | null,
   readFile: (path: string) => Promise<string>,
-): Promise<Record<string, ApplyRestoredFile> | undefined> {
+): Promise<RestoreFiles | undefined> {
   if (!previous || paths.some((path) => !(path in previous))) return undefined;
   const restored = await readAll(paths, readFile);
   if (!restored) return undefined;
@@ -261,6 +257,7 @@ export function usePersistentEditHistory({ projectId }: UsePersistentEditHistory
         return {
           ok: true,
           label: reply.entry.label,
+          undoes: reply.entry.undoes,
           paths: changed,
           files: await restoredFiles(
             changed,
@@ -274,6 +271,11 @@ export function usePersistentEditHistory({ projectId }: UsePersistentEditHistory
     [projectId, view, refresh, own],
   );
 
+  const noteOutsideChange = useCallback(() => {
+    own.overtake();
+    void refresh();
+  }, [own, refresh]);
+
   const undo = useCallback((callbacks: ApplyCallbacks) => step("undo", callbacks), [step]);
   const redo = useCallback((callbacks: ApplyCallbacks) => step("redo", callbacks), [step]);
 
@@ -299,5 +301,6 @@ export function usePersistentEditHistory({ projectId }: UsePersistentEditHistory
     undo,
     redo,
     predict: own.predict,
+    noteOutsideChange,
   };
 }