From 168bb4f1ad2a010c24c433913a4d0d604dcc7bcd Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 16 Jun 2026 11:01:46 -0700 Subject: [PATCH] fix(studio): serialize GSAP script commits per file (shadow request race) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rapid GSAP edits (ease/duration/keyframe/property) fired overlapping read-modify-write POSTs to one script file — coalesceKey only dedupes edit history, not requests. The gsap_fidelity shadow then diffed an op against whichever POST's scriptText resolved, which could predate that op → false "expected null, actual power2.out" mismatches. Server persists correctly; a pure client request-pairing race. Adds createKeyedSerializer (per-key promise chain, rejection-safe, self- cleaning). commitMutation now serializes every GSAP-script commit per target file by default (key `gsap-file:`) — covering all op types and all animations, not just one meta family — so same-file POSTs can't interleave. Distinct files run concurrently; an explicit serializeKey still overrides. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../studio/src/hooks/gsapScriptCommitTypes.ts | 8 ++ .../studio/src/hooks/serializeByKey.test.ts | 83 +++++++++++++++++++ packages/studio/src/hooks/serializeByKey.ts | 33 ++++++++ .../studio/src/hooks/useGsapAnimationOps.ts | 6 +- .../studio/src/hooks/useGsapScriptCommits.ts | 24 +++++- 5 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 packages/studio/src/hooks/serializeByKey.test.ts create mode 100644 packages/studio/src/hooks/serializeByKey.ts diff --git a/packages/studio/src/hooks/gsapScriptCommitTypes.ts b/packages/studio/src/hooks/gsapScriptCommitTypes.ts index 202a83add..afc949256 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -20,6 +20,14 @@ export interface CommitMutationOptions { softReload?: boolean; skipReload?: boolean; beforeReload?: () => void; + /** + * Serialize this commit against others sharing the same key. Used to chain + * per-animationId GSAP meta updates so overlapping read-modify-write POSTs to + * one file can't interleave — which would pair the shadow fidelity diff with a + * stale server result and report false ease mismatches. Commits without a key + * (and under distinct keys) run concurrently as before. + */ + serializeKey?: string; /** Stage 7 Step 3b: typed SDK equivalent of this mutation for value-fidelity shadow. */ shadowGsapOp?: ShadowGsapOp; /** Typed SDK equivalent of a keyframe mutation for keyframe value-fidelity shadow (gsap_keyframe). */ diff --git a/packages/studio/src/hooks/serializeByKey.test.ts b/packages/studio/src/hooks/serializeByKey.test.ts new file mode 100644 index 000000000..2b023118a --- /dev/null +++ b/packages/studio/src/hooks/serializeByKey.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from "vitest"; +import { createKeyedSerializer } from "./serializeByKey"; + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +describe("createKeyedSerializer", () => { + it("runs same-key tasks strictly in order (second awaits the first)", async () => { + const run = createKeyedSerializer(); + const order: string[] = []; + const first = deferred(); + + const p1 = run("k", async () => { + order.push("1-start"); + await first.promise; + order.push("1-end"); + }); + const p2 = run("k", async () => { + order.push("2-start"); + }); + + // Second task must not start until the first finishes. + await Promise.resolve(); + expect(order).toEqual(["1-start"]); + + first.resolve(); + await Promise.all([p1, p2]); + expect(order).toEqual(["1-start", "1-end", "2-start"]); + }); + + it("does not block tasks under different keys", async () => { + const run = createKeyedSerializer(); + const order: string[] = []; + const blockA = deferred(); + + const pa = run("a", async () => { + order.push("a-start"); + await blockA.promise; + order.push("a-end"); + }); + const pb = run("b", async () => { + order.push("b-start"); + order.push("b-end"); + }); + + // Different key runs to completion while "a" is still blocked. + await pb; + expect(order).toEqual(["a-start", "b-start", "b-end"]); + + blockA.resolve(); + await pa; + expect(order).toEqual(["a-start", "b-start", "b-end", "a-end"]); + }); + + it("does not wedge a key when a prior task rejects", async () => { + const run = createKeyedSerializer(); + const order: string[] = []; + + const p1 = run("k", async () => { + order.push("1"); + throw new Error("boom"); + }); + await expect(p1).rejects.toThrow("boom"); + + const p2 = run("k", async () => { + order.push("2"); + }); + await p2; + expect(order).toEqual(["1", "2"]); + }); + + it("propagates the task's resolved value to its caller", async () => { + const run = createKeyedSerializer(); + await expect(run("k", async () => 42)).resolves.toBe(42); + }); +}); diff --git a/packages/studio/src/hooks/serializeByKey.ts b/packages/studio/src/hooks/serializeByKey.ts new file mode 100644 index 000000000..878a2c4c6 --- /dev/null +++ b/packages/studio/src/hooks/serializeByKey.ts @@ -0,0 +1,33 @@ +/** + * Per-key task serializer. Tasks sharing a key run strictly in order: a new + * task for a key awaits the prior task for that key before starting, so their + * effects (e.g. overlapping read-modify-write POSTs to one file) can't + * interleave. Tasks under different keys are independent and never block each + * other. + * + * Used to serialize GSAP meta-update commits per animationId so the shadow + * fidelity diff always pairs an op with the server result that includes it — + * without globally serializing unrelated commits. + */ +export function createKeyedSerializer() { + const inFlight = new Map>(); + + return function run(key: string, task: () => Promise): Promise { + const prior = inFlight.get(key) ?? Promise.resolve(); + // Chain onto the prior task regardless of how it settled; a rejected prior + // commit must not wedge the key forever. + const next = prior.then(task, task); + inFlight.set(key, next); + // Once this task settles, drop it from the map if nothing newer replaced it, + // so completed keys don't leak. + void next.then( + () => { + if (inFlight.get(key) === next) inFlight.delete(key); + }, + () => { + if (inFlight.get(key) === next) inFlight.delete(key); + }, + ); + return next; + }; +} diff --git a/packages/studio/src/hooks/useGsapAnimationOps.ts b/packages/studio/src/hooks/useGsapAnimationOps.ts index a184807ac..aa953fb2d 100644 --- a/packages/studio/src/hooks/useGsapAnimationOps.ts +++ b/packages/studio/src/hooks/useGsapAnimationOps.ts @@ -40,10 +40,14 @@ export function useGsapAnimationOps({ animationId, properties: { duration: updates.duration, ease: updates.ease, position: updates.position }, }; + // coalesceKey groups rapid meta edits into one history entry. Request + // serialization is now handled per-file at the commitMutation chokepoint + // (useGsapScriptCommits), so no per-op serializeKey is needed here. + const metaKey = `gsap:${animationId}:meta`; commitMutationSafely( selection, { type: "update-meta", animationId, updates }, - { label: "Edit GSAP animation", coalesceKey: `gsap:${animationId}:meta`, shadowGsapOp }, + { label: "Edit GSAP animation", coalesceKey: metaKey, shadowGsapOp }, ); if (sdkSession) runShadowGsapTween(sdkSession, shadowGsapOp); }, diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index 49e57991c..1e4b4c7d9 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -1,10 +1,11 @@ -import { useCallback } from "react"; +import { useCallback, useRef } from "react"; import { findUnsafeMutationValues } from "@hyperframes/core/studio-api/finite-mutation"; import type { DomEditSelection } from "../components/editor/domEditingTypes"; import { applySoftReload } from "../utils/gsapSoftReload"; import { resolveGsapFidelityArgs, runShadowGsapFidelity } from "../utils/sdkShadowGsapFidelity"; import { runShadowGsapKeyframeFidelity } from "../utils/sdkShadowGsapKeyframe"; import { updateKeyframeCacheFromParsed } from "./gsapKeyframeCacheHelpers"; +import { createKeyedSerializer } from "./serializeByKey"; import { GsapMutationHttpError, formatGsapMutationRejectionToast, @@ -46,10 +47,16 @@ async function mutateGsapScript( // oxfmt-ignore // fallow-ignore-next-line complexity export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIframeRef, editHistory, domEditSaveTimestampRef, reloadPreview, onCacheInvalidate, onFileContentChanged, showToast, sdkSession }: GsapScriptCommitsParams) { + // Serializer for per-key commits (options.serializeKey). Keyed by + // `gsap:${animationId}:meta`, it chains a meta commit onto the prior one for + // the same animationId so their POSTs can't interleave — which is what made + // the shadow fidelity diff pair an op with a stale server result and report + // false ease mismatches. Held in a ref so the chain survives re-renders. + const serializerRef = useRef(createKeyedSerializer()); // Pre-existing complexity (server mutate + history + reload branches); this PR // adds only a guarded shadow-fidelity dispatch. // fallow-ignore-next-line complexity - const commitMutation = useCallback(async (selection: DomEditSelection, mutation: Record, options: CommitMutationOptions) => { + const runCommit = useCallback(async (selection: DomEditSelection, mutation: Record, options: CommitMutationOptions) => { const pid = projectIdRef.current; if (!pid) return; const unsafeFields = findUnsafeMutationValues(mutation); @@ -105,6 +112,19 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra } onCacheInvalidate(); }, [projectIdRef, activeCompPath, previewIframeRef, editHistory, domEditSaveTimestampRef, reloadPreview, onCacheInvalidate, onFileContentChanged, showToast, sdkSession]); + // Every GSAP-script commit is a read-modify-write of one file. Overlapping + // commits to the SAME file (any op type, any animation) interleave server-side + // and make the shadow fidelity diff pair an op with a stale server result — + // the false ease/value mismatches this serializer exists to prevent. So + // serialize per target file by default; an explicit serializeKey overrides. + const commitMutation = useCallback( + (selection: DomEditSelection, mutation: Record, options: CommitMutationOptions) => { + const file = selection.sourceFile || activeCompPath || "index.html"; + const key = options.serializeKey ?? `gsap-file:${file}`; + return serializerRef.current(key, () => runCommit(selection, mutation, options)); + }, + [runCommit, activeCompPath], + ); const trackGsapSaveFailure = useGsapSaveFailureTelemetry(activeCompPath); const commitMutationSafely = useSafeGsapCommitMutation(commitMutation, trackGsapSaveFailure, showToast); const propertyOps = useGsapPropertyDebounce(commitMutationSafely);