fix(studio): undo keeps a move undone when its history claim landed before the key

Author: miguel-heygen

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

@@ -10,7 +10,6 @@ import {
   patchIframeDomTiming,
   persistElementAttribute,
   persistTimelineBatchEdit,
-  syncCompositionDurationToContent,
   type PersistTimelineBatchChange,
 } from "./timelineEditingHelpers";
 import type { TimelineElement } from "../player/store/playerStore";
@@ -318,19 +317,17 @@ describe("persistTimelineBatchEdit", () => {
     vi.unstubAllGlobals();
   });
 
-  it("finishes each file once, not once per member, and lands where per-member sync would", async () => {
+  it("syncs each file's root duration once, landing where per-member sync would", async () => {
     const source = `<div id="root" data-composition-id="main" data-duration="4"><video id="a" class="clip" data-start="1" data-duration="1"></video><video id="b" class="clip" data-start="2" data-duration="1"></video><video id="c" class="clip" data-start="3" data-duration="1"></video></div>`;
     const members = ["a", "b", "c"].map((id, i) => ({
       element: el({ id, tag: "video", domId: id, start: i + 1, duration: 1 }),
       buildPatches: (original: string, target: Parameters<typeof applyTimelineMoveAttributes>[1]) =>
         applyTimelineMoveAttributes(original, target, i + 6, 1),
     }));
-    const finishFile = vi.fn(syncCompositionDurationToContent);
     stubReadFileContent(source);
     const writes: Array<[string, string]> = [];
-    await persistTimelineBatchEdit({ ...batchInput(members, writes), finishFile });
+    await persistTimelineBatchEdit(batchInput(members, writes));
 
-    expect(finishFile).toHaveBeenCalledTimes(1);
     const perMember = members.reduce(
       (current, { element }, i) =>
         buildTimelineMoveTimingPatch(current, { id: element.id }, i + 6, 1),

packages/studio/src/hooks/timelineEditingHelpers.ts

@@ -406,8 +406,6 @@ export interface PersistTimelineBatchEditInput {
   coalesceKey?: string;
   /** Per-entry undo coalesce window override (ms) — see EditHistoryEntry.coalesceMs. */
   coalesceMs?: number;
-  /** Applied once to each patched file, after every change to it. */
-  finishFile?: (patched: string) => string;
 }
 
 export async function persistTimelineBatchEdit(
@@ -424,7 +422,7 @@ export async function persistTimelineBatchEdit(
       targetPath,
       changesByPath.get(targetPath)!,
     );
-    const next = input.finishFile ? input.finishFile(patched) : patched;
+    const next = syncCompositionDurationToContent(patched);
     if (next !== original) input.pendingTimelineEditPathRef.current.add(targetPath);
     return next;
   };

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

@@ -5,12 +5,19 @@ import { createRoot, type Root } from "react-dom/client";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { STUDIO_MOTION_PATH } from "../components/editor/studioMotion";
 import { useEditHistoryActions, type EditHistoryHandle } from "./useEditHistoryActions";
-import { beginStudioPendingEdit, trackStudioPendingEdit } from "../utils/studioPendingEdits";
+import {
+  beginStudioPendingEdit,
+  setStudioPendingEditClaimClock,
+  trackStudioPendingEdit,
+} from "../utils/studioPendingEdits";
 
 (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
 
 let root: Root | null = null;
-afterEach(() => act(() => root?.unmount()));
+afterEach(() => {
+  act(() => root?.unmount());
+  setStudioPendingEditClaimClock(null);
+});
 
 type RestoreFiles = Record<string, { previous: string; restored: string }>;
 type Prediction = { id: string; files: RestoreFiles };
@@ -126,23 +133,28 @@ describe("useEditHistoryActions", () => {
     expect(reapply).not.toHaveBeenCalled();
   });
 
+  // Claim counts: when the edit began, at the key, and once its save landed.
   it.each([
-    ["its claim counted, the server undo is the shown revert", 8, 0],
-    ["its claim never counted, the move is shown again", 7, 1],
-  ])("an edit that lands while undo waits: %s", async (_, claimsAfter, reapplied) => {
+    ["its claim counted while undo waited, the server undo is the shown revert", 7, 7, 8, 0],
+    ["its claim counted before the key, the server undo is the shown revert", 7, 8, 8, 0],
+    ["its claim never counted, the move is shown again", 7, 7, 7, 1],
+  ])("an edit that lands while undo waits: %s", async (_, atBegin, atKey, atLand, reapplied) => {
+    let claimCount = atBegin;
+    setStudioPendingEditClaimClock(() => claimCount);
     const reapply = vi.fn();
     const saving = beginStudioPendingEdit(() => reapply);
-    let claimCount = 7;
+    claimCount = atKey;
     const { deps, actions } = mount(
-      { ok: true, label: "Undid: Move", paths: ["index.html"], undoes: "e2" },
+      { ok: true, label: "Undid: Move", paths: ["index.html"], undoes: "e8" },
       PREDICTED,
       () => claimCount,
     );
     const undone = actions.undo();
     saving.settle(saving.adopt(() => Promise.resolve()));
-    claimCount = claimsAfter;
+    claimCount = atLand;
     await act(() => undone);
     expect(deps.editHistory.undo).toHaveBeenCalledTimes(1);
+    expect(deps.editHistory.undo.mock.calls[0]![0].claimedAfter).toBe(atBegin);
     expect(deps.showHistoryRestoreNow).not.toHaveBeenCalled();
     expect(reapply).toHaveBeenCalledTimes(reapplied);
   });

packages/studio/src/hooks/useEditHistoryActions.ts

@@ -84,7 +84,8 @@ export function useEditHistoryActions({
       const predictedShown = predicted ? (showHistoryRestoreNow?.(predicted.files) ?? null) : null;
       const putBack = predictedShown ?? pendingEditShown?.showAgain;
       const claimedAfter =
-        direction === "undo" && hasStudioPendingEdits() ? editHistory.claims?.() : undefined;
+        pendingEditShown?.claimsAtBegin ??
+        (direction === "undo" && hasStudioPendingEdits() ? editHistory.claims?.() : undefined);
       let result: HistoryResult = { ok: false, reason: "failed" };
       let serverSteppedShown = false;
       let revertIsTheUndo = false;
@@ -98,11 +99,12 @@ export function useEditHistoryActions({
           claimedAfter,
         });
         const stepped = Boolean(result.ok && result.label);
-        const claimedSinceKey =
-          claimedAfter !== undefined && (editHistory.claims?.() ?? claimedAfter) > claimedAfter;
+        const editClaimed =
+          pendingEditShown !== null &&
+          (editHistory.claims?.() ?? 0) > pendingEditShown.claimsAtBegin;
         serverSteppedShown = predictedShown
           ? stepped && result.undoes === predicted?.id
-          : stepped && Boolean(pendingEditShown) && claimedSinceKey;
+          : stepped && editClaimed;
       } finally {
         if (putBack && !serverSteppedShown && !revertIsTheUndo) putBack();
       }

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

@@ -14,6 +14,10 @@ import {
   type StudioApiAdapter,
 } from "@hyperframes/studio-server";
 import { consumeStudioWriteToken } from "../utils/studioFileVersion";
+import {
+  beginStudioPendingEdit,
+  paintBackNewestStudioPendingEdit,
+} from "../utils/studioPendingEdits";
 import { usePersistentEditHistory } from "./usePersistentEditHistory";
 
 const cleanup: Array<() => unknown> = [];
@@ -88,6 +92,22 @@ it("an edit Studio saved is undone and redone by the project's history, with the
   expect(file()).toBe("B");
 });
 
+it("gives an edit that begins the history's claim count, so undo can tell its claims from older ones", async () => {
+  const { hook, save } = await studio();
+  save("B");
+  await act(() =>
+    hook().recordEdit({
+      label: "Moved Title",
+      files: { "index.html": { before: "A", after: "B" } },
+    }),
+  );
+  const saving = beginStudioPendingEdit(() => () => {});
+
+  expect(paintBackNewestStudioPendingEdit()?.claimsAtBegin).toBe(hook().claims());
+  expect(hook().claims()).toBe(1);
+  saving.settle();
+});
+
 it("undoes the edit claimed since the key, not a later edit the server took in first", async () => {
   const { dir, hook, file, save, readFile } = await studio();
   const atKey = hook().claims();

packages/studio/src/hooks/usePersistentEditHistory.ts

@@ -3,6 +3,7 @@ import type { HistoryListItem, HistoryResult } from "@hyperframes/studio-server"
 import { studioFileContentVersion, studioWriteHeaders } from "../utils/studioFileVersion";
 import type { RestoreFiles } from "../utils/gsapUndoRestore";
 import { studioApiFetch } from "../utils/studioApiFetch";
+import { setStudioPendingEditClaimClock } from "../utils/studioPendingEdits";
 import type { RecordEditInput } from "../utils/studioFileHistory";
 
 interface ApplyCallbacks {
@@ -228,6 +229,11 @@ export function usePersistentEditHistory({ projectId }: UsePersistentEditHistory
     void refresh().finally(() => setLoaded(true));
   }, [refresh, own]);
 
+  useEffect(() => {
+    setStudioPendingEditClaimClock(own.claimCount);
+    return () => setStudioPendingEditClaimClock(null);
+  }, [own]);
+
   const recordEdit = useCallback(
     async ({ label, coalesceKey, coalesceMs, files, created = [] }: RecordEditInput) => {
       if (!projectId) return;

packages/studio/src/hooks/useTimelineGroupEditing.ts

@@ -12,7 +12,6 @@ import {
 import {
   applyTimelineMoveAttributes,
   applyTimelineResizeAttributes,
-  syncCompositionDurationToContent,
   extendRootDurationIfNeeded,
   formatTimelineAttributeNumber,
   formatTimelineMediaOffset,
@@ -176,7 +175,6 @@ export function useTimelineGroupEditing({
         pendingTimelineEditPathRef,
         coalesceKey,
         coalesceMs,
-        finishFile: syncCompositionDurationToContent,
       });
       forceReloadSdkSession?.();
     },

packages/studio/src/player/components/TimelineLanes.test.tsx

@@ -10,7 +10,11 @@ import { defaultTimelineTheme } from "./timelineTheme";
 import { TRACK_H, getTimelineRowGeometry } from "./timelineLayout";
 import { createTimelineClipIndex } from "../lib/timelineClipIndex";
 import { buildTimelineLogicalRows } from "./timelineKeyboardNavigation";
-import { usePlayerStore, type TimelineElement } from "../store/playerStore";
+import {
+  usePlayerStore,
+  type KeyframeCacheEntry,
+  type TimelineElement,
+} from "../store/playerStore";
 import type { MultiDragPreviewInput } from "./timelineMultiDragPreview";
 import type { TimelineEditCallbacks } from "./timelineCallbacks";
 import type { DraggedClipState, BlockedClipState } from "./useTimelineClipDrag";
@@ -73,6 +77,7 @@ function positionTween(id: string): GsapAnimation {
 
 interface RenderLanesOptions {
   elements?: TimelineElement[];
+  keyframeCache?: Map<string, KeyframeCacheEntry>;
   animations?: Map<string, GsapAnimation[]>;
   expandedClipIds?: string[];
   selectedElementIds?: Set<string>;
@@ -172,6 +177,7 @@ function renderLanes(options: RenderLanesOptions = {}): {
           getPreviewElement={(el) => el}
           getTrackStyle={getTrackStyle}
           gsapAnimations={gsapAnimations}
+          keyframeCache={next.keyframeCache}
           selectedKeyframes={new Set()}
           currentTime={0}
           onContextMenuLane={next.onContextMenuLane}
@@ -399,18 +405,37 @@ describe("TimelineLanes disclosure target", () => {
       draggedPreviewStart: 0.5,
       selectedKeys: selectedElementIds,
     };
-    const view = renderLanes({ elements, selectedElementIds });
+    const keyframeCache = new Map<string, KeyframeCacheEntry>([
+      [
+        "clip-a",
+        {
+          format: "percentage",
+          keyframes: [
+            { percentage: 0, properties: { x: 0 } },
+            { percentage: 100, properties: { x: 50 } },
+          ],
+        },
+      ],
+    ]);
+    const view = renderLanes({ elements, selectedElementIds, keyframeCache });
     const clipA = view.host.querySelector<HTMLElement>('[data-el-id="clip-a"]');
+    const diamonds = () =>
+      view.host
+        .querySelector('[data-el-id="clip-a"] ~ div button[aria-label*="keyframe at"]')
+        ?.closest<HTMLElement>("div.pointer-events-none");
     expect(clipA).not.toBeNull();
     expect(clipA?.parentElement).toBe(
       view.host.querySelector('[data-el-id="clip-b"]')?.parentElement,
     );
 
-    view.rerender({ elements, selectedElementIds, multiDragPreview: dragging });
+    expect(diamonds()).not.toBeNull();
+    view.rerender({ elements, selectedElementIds, keyframeCache, multiDragPreview: dragging });
     expect(view.host.querySelector('[data-el-id="clip-a"]')).toBe(clipA);
     expect(clipA?.style.transform).toMatch(/^translateX\([1-9]/);
+    // Its keyframe diamonds ride with it.
+    expect(diamonds()?.style.transform).toBe(clipA?.style.transform);
 
-    view.rerender({ elements, selectedElementIds });
+    view.rerender({ elements, selectedElementIds, keyframeCache });
     expect(view.host.querySelector('[data-el-id="clip-a"]')).toBe(clipA);
     expect(clipA?.style.transform).toBe("");
     act(() => view.root.unmount());
@@ -437,9 +462,11 @@ describe("TimelineLanes disclosure target", () => {
     });
 
     const before = ariaControlsTarget(view.host);
-    const beforeLane = before?.querySelector("[data-timeline-property-lane]");
+    const beforeLane = before?.querySelector<HTMLElement>("[data-timeline-property-lane]");
     expect(before).not.toBeNull();
     expect(beforeLane).not.toBeNull();
+    const startOffset = beforeLane?.style.transform;
+    expect(startOffset).toMatch(/^translateX\([1-9]/);
 
     view.rerender({
       elements,
@@ -452,6 +479,9 @@ describe("TimelineLanes disclosure target", () => {
     // Node identity, not just presence: a remount replaces these nodes.
     expect(ariaControlsTarget(view.host)).toBe(before);
     expect(before?.querySelector("[data-timeline-property-lane]")).toBe(beforeLane);
+    // The lanes ride with the formation, not just the clip.
+    expect(beforeLane?.style.transform).toMatch(/^translateX\([1-9]/);
+    expect(beforeLane?.style.transform).not.toBe(startOffset);
     act(() => view.root.unmount());
   });
 });

packages/studio/src/utils/studioPendingEdits.ts

@@ -16,6 +16,7 @@ interface PendingEdit {
   landed: () => Promise<boolean>;
   redraws: Array<() => void>;
   showAgain: (() => void) | null;
+  claimsAtBegin: number;
 }
 
 export interface StudioEditInFlight {
@@ -27,6 +28,11 @@ export interface StudioEditInFlight {
 }
 
 const pendingEdits = new Map<Promise<unknown>, PendingEdit>();
+let historyClaims: () => number = () => 0;
+
+export function setStudioPendingEditClaimClock(read: (() => number) | null): void {
+  historyClaims = read ?? (() => 0);
+}
 const NOT_SAVED = () => Promise.resolve(false);
 let adopting: StudioEditInFlight | null = null;
 
@@ -87,7 +93,13 @@ export function trackStudioPendingEdit(
   if (!result) return undefined;
   const promise = Promise.resolve(result);
   if (adopting) return promise;
-  pendingEdits.set(promise, { revert: null, landed: NOT_SAVED, redraws: [], showAgain: null });
+  pendingEdits.set(promise, {
+    revert: null,
+    landed: NOT_SAVED,
+    redraws: [],
+    showAgain: null,
+    claimsAtBegin: historyClaims(),
+  });
   promise.then(
     () => pendingEdits.delete(promise),
     () => pendingEdits.delete(promise),
@@ -162,6 +174,7 @@ export function beginStudioPendingEdit(revert: StudioEditRevert | null) {
 export function paintBackNewestStudioPendingEdit(): {
   showAgain: () => void;
   landed: () => Promise<boolean>;
+  claimsAtBegin: number;
 } | null {
   const newest = [...pendingEdits.values()].at(-1);
   const revert = newest?.revert;
@@ -174,6 +187,7 @@ export function paintBackNewestStudioPendingEdit(): {
       for (const redraw of newest.redraws.splice(0)) redraw();
     },
     landed: newest.landed,
+    claimsAtBegin: newest.claimsAtBegin,
   };
 }