fix(studio): restore timeline move/resize fallback parity (review #1466) (#1539)

* fix(studio): restore timeline move/resize fallback parity (review #1466)

The §3.2 sdkTimingPersist rewrite regressed the non-SDK fallback path vs the
pre-cutover behavior. Restored, on both fallback entry points (no-session and
sdkTimingPersist-returned-unhandled):

- Resize live DOM patch dropped the conditional data-playback-start/media-start
  attr — restored so a start-trim updates the preview's in-point immediately.
- Move/resize fallback dropped the GSAP-position sync (shift/scaleGsapPositions)
  + reloadPreview — restored so server-path edits keep GSAP tweens in sync and
  refresh the preview (the SDK path folds both into setTiming).
- Undo-coalesce drift: fallback enqueueEdit carried no coalesceKey while the SDK
  branch did — plumbed coalesceKey through persistTimelineEdit so undo
  granularity is identical on either path.
- Documented the hasPbsAdjustment second clause + sdkTimingPersist before-capture
  transition limitation.

Flag-off (dark launch) so this lands as one fix PR at the stack tip rather than
restacking the mid-stack §3.2 commit. #1500 review items: parity-harness gap
already closed at the tip (arc/unroll recast-vs-acorn parity added); blockRemoveRange
flagged 'potential' but verified correct (no comma residue on any block position).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(sdk): retire duplicate removeGsapKeyframe keyframeIndex variant (review #1498)

EditOp had two removeGsapKeyframe members with the same discriminant but
different shapes (keyframeIndex vs percentage) — TS can't discriminate them and
a handler could get the wrong shape. Per both reviewers (option 2): retire the
keyframeIndex variant. It had no production caller (Studio dispatches percentage
only); removed the dead by-index handleRemoveGsapKeyframe + simplified the
dispatcher. resolveKeyframe stays (setGsapKeyframe still uses keyframeIndex).
Converted the one by-index test to the percentage API.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(studio): gate ALL cutover persist paths on the flag — true dark launch (review #1469 finding #6)

Only sdkCutoverPersist (style/text/attr) checked STUDIO_SDK_CUTOVER_ENABLED.
sdkTimingPersist, dispatchGsapOpAndPersist (every GSAP op) and sdkDeletePersist
guarded only on `!sdkSession` — and useSdkSession opens a session by default
for shadow/selection, so timing/GSAP/keyframe/delete cutover was ALWAYS live
regardless of the flag. Flipping the flag OFF could not disable it, so the
data-loss bugs in those paths (single-prop wipe, wrong-keyframe match, tween
collapse, arc strip) ship LIVE on merge instead of being dark-launched.

Added the flag guard at all three chokepoints → flag OFF returns false → callers
fall back to the legacy server path. Makes the stack genuinely dark-launchable:
merge is now a no-op in prod, and the remaining cutover correctness bugs become
flip-prerequisites rather than merge-blockers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(core,sdk): correct 8 GSAP write-path review findings (#1539)

Eight correctness bugs from the SDK-cutover review. Several were cases where
BOTH writers were identically wrong, so the recast-vs-acorn parity suite stayed
green; the new tests assert the real-world-correct result, not agreement.

- #2 findKfPropByPct: match the CLOSEST keyframe within tolerance, not the first
  within 2% — removing/updating 50% on 0/49/50/100 no longer hits 49%.
- #3 handleSetTiming: shift each tween by the start DELTA and scale duration by
  the clip-duration RATIO per-tween, instead of writing absolute newStart/
  newDuration onto every tween (which collapsed staggers and blew durations).
- #4 enableArcPath: insert motionPath via appendRight at the object start so the
  insertion can't collide with the x/y remove-range end (which made MagicString
  discard the append and emit '{}').
- #5 splitAnimationsInScript: compute the inherited baseline in a forward pre-pass
  so the split-spanning midpoint sees earlier tweens (the reverse write loop is
  kept for stable count-suffixed ids).
- #9 unrollDynamicAnimations: preserve non-target loop-body statements (e.g.
  tl.set initial-state) per iteration instead of overwriting the whole loop.
- #10 buildMotionPathObjectCode (both writers): emit the cubic form when segment
  curviness varies so per-segment curviness survives, not just segments[0].
- #11 readLastWaypointXY: handle UnaryExpression so negative destination coords
  are recovered when disabling an arc path.
- #15 no-bang: removed every `!` non-null assertion in the touched files,
  replaced with guards/fallbacks.

Tests: gsapWriter.reviewFixes.test.ts (#2/#4/#5/#9/#10/#11) and
mutate.gsap.test.ts setTiming GSAP-sync block (#3). All fail on the base and
pass after the fix; tsc + full core/sdk suites + parity stay green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(studio): SDK cutover review fixes — merge tween props, stabilize debounce, serialize gsap writes, on-disk undo baseline, self-write identity

Addresses 5 SDK-cutover review findings (studio-only):

- #1 useGsapPropertyDebounce: editing one GSAP tween property no longer drops
  the tween's other animated props. setGsapTween REPLACES the property set, so
  merge the single edit into the tween's CURRENT properties (read from the SDK
  doc) before dispatching, mirroring the legacy server merge.
- #7 useGsapPropertyDebounce: stabilize the flush callback by reading sdk deps
  from a ref instead of an unmemoized literal, so a parent re-render mid-edit
  no longer tears down + flushes the debounce (one commit/undo entry per render).
- #8 sdkCutover/useGsapScriptCommits: route SDK gsap-write persists through the
  same per-file keyed serializer the legacy commitMutation uses, so concurrent
  same-file read-modify-writes can't interleave and lose an edit.
- #12 sdkCutover/useTimelineEditing: capture the exact on-disk bytes as the undo
  'before' for timing/GSAP persists (matching the style/delete paths) instead of
  a normalized SDK serialize() re-emit that reformatted the whole file on undo.
- #14 useSdkSession/sdkSelfWriteRegistry: discriminate a cutover echo from an
  undo write by CONTENT identity (registered self-write hash), not just the 2 s
  timestamp window — an undo write always reloads the SDK session.

Tests: useGsapPropertyDebounce(.test), useGsapPropertyDebounceFlush.test,
sdkSelfWriteRegistry.test, and new sdkCutover.test cases; each reproduces the
review scenario and asserts the corrected behavior (verified red before fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(core): extract split/collapse helpers to satisfy no-fallow-ignore rule

The #5 (split) and #15 (no-bang guards) fixes pushed splitAnimationsInScript and
removeAllKeyframesFromScript over fallow's complexity threshold, and a fallow-ignore
had been added to splitAnimationsInScript. Per the hard rule (never ignore — fix),
extracted buildSpanningSplit + applyTweenSplit (split) and buildCollapsedFlatVars
(collapse), and removed the ignore. Both functions now under threshold; fallow new-only
gate reports 0 new findings. Behavior unchanged — core 1811 green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(studio): pin dark-launch flag-gate contract (review #1539, Rames/Via)

flag OFF ⇒ sdkTimingPersist / sdkGsapTweenPersist (GSAP-op chokepoint) /
sdkDeletePersist all return false even with a valid session → legacy fallback.
The prod flag-flip rests on this contract; sdkCutover.test.ts only mocks the flag
TRUE, so a future gate refactor could silently re-enable cutover on flag-off
without failing CI. This sibling file mocks it FALSE and locks the three guards.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(studio): leading flag-gate on sdkGsapTweenPersist (review #1539 nit, Via)

The add-op getElement existence check ran before the inner gate, so flag-off did
an SDK touch before falling back. Lead with the flag guard to match the other
three chokepoints — flag-off is now a clean no-op at every entry point.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(core): unroll-preservation regressions — non-for loops + AST index substitution (review R2)

The #9 unroll-preservation fix had two confirmed regressions:
- Non-for loops (forEach/for-of/for-in/while): loopIndexVarName returns null, so
  substitution no-op'd and preserved siblings kept a now-undefined loop variable
  (e.g. `item`) → ReferenceError at render. Now returns null for those forms →
  caller falls back to the blanket loop overwrite (drops siblings, valid code).
  The #9 fixture only used `for(let i…)` so it never caught this.
- substituteLoopIndex did a \bvar\b regex over raw source including string
  literals, corrupting selectors like ".row-i" → ".row-0". Now AST-based:
  substitutes only real Identifier uses, skipping string literals and non-computed
  member/key positions (extracted isIndexBindingPosition helper to stay under the
  fallow complexity threshold — no ignore added).

Two regression tests added (forEach no-dangling-var; for-loop string-literal intact).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(sdk,core): unrollDynamicAnimations rejects empty element list (R1 #1501b)

An empty `elements` array has no unrolled form — the writer would overwrite
the loop/statement with zero tween calls, silently deleting the animation.

- gsapWriterAcorn: unrollDynamicAnimations returns the script verbatim on an
  empty list (no-op instead of a destructive overwrite).
- validateOp: reject unrollDynamicAnimations with empty elements as
  E_INVALID_ARGS so callers get a clean error rather than silent corruption.
- Tests: writer no-op on []; validateOp E_INVALID_ARGS on [].

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(sdk): cache draft element in applyDraft, drop HTMLElement casts (R1 #1490a)

applyDraft runs at 60fps during a drag but re-ran doc.querySelector on every
call — the _draftEl/_draftId fields were only consumed by commit/cancel, never
to skip the query. Reuse the tracked element when the id matches and the node
is still connected; re-query only on id change or detach (iframe reload).

Retypes _draftEl to HTMLElement | null (only ever set from
querySelector<HTMLElement>), which removes the `as HTMLElement` casts in
commitPreview / _clearDraft. Test asserts a repeated same-id drag queries once.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(sdk,core): round-3 correctness — unroll AST safety, single-dispatch undo, empty-arg guards, persist decouple

Addresses the highest-severity round-3 review findings:

- gsapWriterAcorn unroll (R3 #1/#2/#9): the round-2 AST-substitution fix emitted
  invalid GSAP for object shorthand `{ i }` (→ `{ 0 }`) and shadowed inner
  bindings (→ `for(let i=0;0<3;0++)`), and silently dropped sibling statements on
  non-`for` loops (forEach/for-of). The unroll now REFUSES (no-ops, leaving the
  dynamic loop intact) whenever siblings can't be safely reproduced — a non-`for`
  loop, an unmodeled statement, or an unsafe index use — instead of dropping or
  corrupting. Plain `for` loops with safe siblings still unroll.

- session single-dispatch undo (R3 #5/#11): _dispatch now reverses the inverse
  patch list (parity with batch()). A single op emitting order-dependent inverse
  patches — a nested parent+child removeElement, an aliased multi-target — undid
  forward and dropped the child subtree / landed on an intermediate value.

- materializeKeyframes empty-array (R3 #10): the unguarded twin of the just-fixed
  unrollDynamicAnimations. Writer no-ops on an empty keyframe list; validateOp
  rejects it as E_INVALID_ARGS (shared gsapScriptMissing helper).

- history:false persist decouple (R3 #4): persist (auto-save) no longer lives
  inside the history-enable block, so opting out of SDK undo no longer silently
  disables all disk writes (data-loss trap for #1496's flag consumers).

Tests: unroll refuse cases (shorthand/shadow/forEach) + safe-for-loop regression;
nested removeElement undo; materializeKeyframes writer no-op + validateOp reject;
history:false-still-persists.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(core): stripGsapForId re-parses per removal so all tweens for a deleted element are stripped (R3 #3)

Animation ids are count-based (positional), so removing one tween renumbers the
survivors. stripGsapForId captured every matching id from a single up-front parse
then removed against the mutating script — after the first removal the later ids
were stale and silently no-op'd, leaving an orphaned tl.to() referencing the
just-deleted element. Now re-parse after each removal and strip the first
still-matching animation until none remain.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(core): gsap writer — keyframe ease routing, convert preserves delay, addLabel dedup (R3 #7/#8/#12)

- #7: updateAnimationInScript routes an ease update on a keyframe tween to
  keyframes.easeEach (per-keyframe), not a top-level ease that GSAP ignores —
  the user's keyframe-easing edit was silently a no-op.
- #8: convertToKeyframesFromScript now preserves every non-editable vars key
  (delay/callbacks/stagger/yoyo/…) verbatim via preservedVarsEntries instead of
  rebuilding from the GsapAnimation object, which had no `delay` field and
  dropped it — shifting the tween's start time.
- #12: addLabelToScript moves an existing same-named label (overwrites its
  position) instead of appending a duplicate; duplicates made removeLabel
  over-remove (it deletes every match, including a pre-existing label).

Tests: easeEach routing, delay preservation, addLabel move-not-duplicate +
hand-authored-dup removal. Updated the old "no dedup contract" corpus test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(sdk): handleSetTiming #domId + data-duration sync; validateOp resolves ids + arc/selector (R3 #6/#13, CF2 #15/#16)

CF2 #15: handleSetTiming re-synced GSAP tweens only when the selector matched the
element's hf-id. The common #domId-targeted tween (authored by the Studio panel)
never matched, so moving/resizing a clip via the SDK timing path left its
animations unsynced. Now match the tween selector against the DOM id too.

CF2 #16: handleSetTiming read/wrote only data-end. Clips authored with
data-duration (what the runtime prefers) got a fresh data-end beside a stale
data-duration (no playback change) and oldDuration=null collapsed the GSAP
duration-scale ratio to 1. Now read duration preferring data-duration, and write
back to whichever attribute the clip uses (timingPath gains a "duration" field).

R3 #13b: deleteAllForSelector compared selectors with strict === and missed the
alternate quote style ([data-hf-id='x'] vs "x"); now quote-insensitive.

R3 #6/#13a: validateOp now resolves the animationId for id-bearing GSAP ops
(E_TARGET_NOT_FOUND instead of a misleading ok that no-ops at apply), and
updateArcSegment validates the arc is enabled + the segment index is in range.

Tests: #domId move sync, data-duration resize + scale, quote-insensitive delete,
unresolved-id rejection, arc-segment preconditions. Updated the loose-can() test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(core,sdk): name the acorn-node type alias; keyToPath round-trips timing.duration (R3 #14)

- gsapWriterAcorn: replace the bare `: any` AST-node annotations with the named
  `type Node = any` alias, matching the established convention in
  gsapParserAcorn.ts / gsapInline.ts ("acorn ESTree nodes are structurally
  untyped"). Documents intent and is greppable; type-identical (zero runtime
  change). A full ESTree typing is a deliberate architecture decision the
  codebase has not taken and is out of scope here.
- patches: keyToPath/timingPath now include the "duration" timing field added
  for the data-duration resize fix, so a timing.duration override round-trips on
  T3 replay instead of being dropped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(sdk): cascadeRemoveAnimations re-parses per removal (R4 — SDK twin of #3)

cascadeRemoveAnimations captured every matching animation id from a single
up-front parse, then removed against the mutating script — the SDK-side twin of
the stripGsapForId bug (R3 #3). Animation ids are positional, so removing the
first tween for an element renumbered the survivors and the stale later ids
no-op'd, orphaning those tweens on the just-removed element. Now re-parse after
each removal and strip the first still-matching animation until none remain.

Also adds the reviewer's defense-in-depth test: an aliased multi-target setStyle
(same id twice) undoes to the original, not the intermediate (exercises the
single-dispatch inverse reversal from R3 #5/#11).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-06-17 16:55:34 -07:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 09cefc1bb7
commit 8c981a451a
29 changed files with 2319 additions and 343 deletions
@@ -0,0 +1,67 @@
import { describe, expect, it, beforeEach } from "vitest";
import {
hashContent,
markSelfWrite,
isSelfWriteEcho,
resetSelfWriteRegistry,
} from "./sdkSelfWriteRegistry";
import { shouldReloadOnFileChange } from "./useSdkSession";
describe("sdkSelfWriteRegistry (finding #14)", () => {
beforeEach(() => resetSelfWriteRegistry());
it("recognizes the echo of bytes we just wrote", () => {
markSelfWrite("/comp.html", "<html>A</html>");
expect(isSelfWriteEcho("/comp.html", "<html>A</html>")).toBe(true);
});
it("does NOT match different content on the same path (an undo's reverted bytes)", () => {
markSelfWrite("/comp.html", "<html>A</html>");
expect(isSelfWriteEcho("/comp.html", "<html>REVERTED</html>")).toBe(false);
});
it("is keyed per file — a self-write to one file can't mask a change to another", () => {
markSelfWrite("/a.html", "<html>A</html>");
expect(isSelfWriteEcho("/b.html", "<html>A</html>")).toBe(false);
});
it("consumes a matched entry so a later genuine external write isn't suppressed", () => {
markSelfWrite("/comp.html", "<html>A</html>");
expect(isSelfWriteEcho("/comp.html", "<html>A</html>")).toBe(true);
// A second arrival of identical bytes is NOT our echo — must reload.
expect(isSelfWriteEcho("/comp.html", "<html>A</html>")).toBe(false);
});
it("expires entries past the TTL so a stale self-write can't suppress forever", () => {
const t0 = 1_000_000;
markSelfWrite("/comp.html", "<html>A</html>", t0);
// 3 s later (> 2 s TTL) the entry is gone.
expect(isSelfWriteEcho("/comp.html", "<html>A</html>", t0 + 3000)).toBe(false);
});
it("hashes are stable and distinguish different content", () => {
expect(hashContent("x")).toBe(hashContent("x"));
expect(hashContent("x")).not.toBe(hashContent("y"));
});
});
describe("shouldReloadOnFileChange (finding #14)", () => {
beforeEach(() => resetSelfWriteRegistry());
it("suppresses the reload when content matches a registered self-write (cutover echo)", () => {
markSelfWrite("/comp.html", "<html>SELF</html>");
expect(shouldReloadOnFileChange("/comp.html", "<html>SELF</html>", true)).toBe(false);
});
it("reloads on an undo write even inside the suppress window (content differs)", () => {
// The cutover registered SELF; the undo writes REVERTED bytes within the
// same 2 s window. Time-only suppression dropped this; identity reloads it.
markSelfWrite("/comp.html", "<html>SELF</html>");
expect(shouldReloadOnFileChange("/comp.html", "<html>REVERTED</html>", true)).toBe(true);
});
it("falls back to the time window only when content is unavailable", () => {
expect(shouldReloadOnFileChange("/comp.html", null, true)).toBe(false);
expect(shouldReloadOnFileChange("/comp.html", null, false)).toBe(true);
});
});
@@ -0,0 +1,77 @@
/**
* Self-write identity registry — discriminates an SDK cutover ECHO from a genuine
* external write (notably undo/redo) in the file-change reload-suppression path.
*
* The old suppression was purely time-based: any file-change within 2 s of the
* shared `domEditSaveTimestampRef` was swallowed. But BOTH an SDK cutover
* self-write AND an undo write set that same timestamp, so the window could not
* tell "the echo of the bytes I just wrote" (suppress) from "the reverted bytes
* an undo just wrote" (must reload). An undo that landed inside the window was
* silently dropped, leaving the in-memory SDK doc on stale pre-undo content.
*
* Fix: tag each cutover self-write with the CONTENT it wrote (by hash). A
* file-change reload is suppressed only when the new on-disk content matches a
* recently-registered self-write hash — i.e. it is provably our own echo. Undo
* writes are never registered (they don't flow through persistSdkSerialize), so
* their content won't match and the reload always fires. Identity, not a clock.
*/
const SELF_WRITE_TTL_MS = 2000;
interface SelfWriteEntry {
hash: string;
at: number;
}
// Module-scoped: the studio process has a single SDK session lifecycle at a time
// and persists are funnelled through one persistSdkSerialize. Keyed by file path
// so a self-write to one file can't mask a real external change to another.
const registry = new Map<string, SelfWriteEntry[]>();
/**
* Stable 32-bit FNV-1a hash of content. Collisions only risk SUPPRESSING a real
* reload, and only within the 2 s TTL for the exact same file — negligible, and
* strictly safer than the prior time-only window it replaces.
*/
export function hashContent(content: string): string {
let h = 0x811c9dc5;
for (let i = 0; i < content.length; i++) {
h ^= content.charCodeAt(i);
h = Math.imul(h, 0x01000193);
}
return (h >>> 0).toString(16);
}
function prune(entries: SelfWriteEntry[], now: number): SelfWriteEntry[] {
return entries.filter((e) => now - e.at < SELF_WRITE_TTL_MS);
}
/** Record that WE wrote `content` to `path` (an SDK cutover self-write). */
export function markSelfWrite(path: string, content: string, now: number = Date.now()): void {
const next = prune(registry.get(path) ?? [], now);
next.push({ hash: hashContent(content), at: now });
registry.set(path, next);
}
/**
* True when `content` matches a self-write registered for `path` within the TTL.
* Consumes the matched entry so a later genuinely-external write of identical
* bytes isn't suppressed forever.
*/
export function isSelfWriteEcho(path: string, content: string, now: number = Date.now()): boolean {
const entries = prune(registry.get(path) ?? [], now);
const hash = hashContent(content);
const idx = entries.findIndex((e) => e.hash === hash);
if (idx === -1) {
registry.set(path, entries);
return false;
}
entries.splice(idx, 1);
registry.set(path, entries);
return true;
}
/** Test-only: drop all registered self-writes. */
export function resetSelfWriteRegistry(): void {
registry.clear();
}
@@ -97,6 +97,7 @@ export interface PersistTimelineEditInput {
recordEdit: (input: RecordEditInput) => Promise<void>;
domEditSaveTimestampRef: React.MutableRefObject<number>;
pendingTimelineEditPathRef: React.MutableRefObject<Set<string>>;
coalesceKey?: string;
}
export async function persistTimelineEdit(input: PersistTimelineEditInput): Promise<void> {
@@ -119,6 +120,7 @@ export async function persistTimelineEdit(input: PersistTimelineEditInput): Prom
projectId: input.projectId,
label: input.label,
kind: "timeline",
coalesceKey: input.coalesceKey,
files: { [targetPath]: patchedContent },
readFile: async () => originalContent,
writeFile: input.writeProjectFile,
@@ -0,0 +1,70 @@
// @vitest-environment happy-dom
import { describe, it, expect } from "vitest";
import { openComposition } from "@hyperframes/sdk";
import { createMemoryAdapter } from "@hyperframes/sdk/adapters/memory";
import { parseGsapScriptAcorn } from "@hyperframes/core/gsap-parser-acorn";
import { mergeTweenProperties } from "./useGsapPropertyDebounce";
import { extractGsapScriptText } from "../utils/gsapSoftReload";
const HTML = `<!DOCTYPE html><html><head></head><body>
<div id="box" data-hf-id="hf-box" style="opacity:1"></div>
<script data-hf-gsap>
const tl = gsap.timeline({ paused: true });
window.__timelines = { main: tl };
tl.to('#box', { duration: 1, x: 100, y: 50, opacity: 1 });
</script>
</body></html>`;
const FROMTO_HTML = `<!DOCTYPE html><html><head></head><body>
<div id="box" data-hf-id="hf-box" style="opacity:1"></div>
<script data-hf-gsap>
const tl = gsap.timeline({ paused: true });
window.__timelines = { main: tl };
tl.fromTo('#box', { x: 0, y: 0 }, { duration: 1, x: 100, y: 50 });
</script>
</body></html>`;
function tweenProps(comp: { serialize(): string }) {
const parsed = parseGsapScriptAcorn(extractGsapScriptText(comp.serialize()) ?? "");
const anim = parsed.animations[0];
return { id: anim?.id, properties: anim?.properties, fromProperties: anim?.fromProperties };
}
describe("setGsapTween replace semantics (finding #1)", () => {
it("REGRESSION: a single-key set drops the tween's other animated props", async () => {
// This documents the bug the merge fixes: setGsapTween REPLACES the property
// set, so sending only the edited key loses the siblings.
const comp = await openComposition(HTML, { persist: createMemoryAdapter() });
const id = tweenProps(comp).id ?? "";
comp.setGsapTween(id, { properties: { x: 200 } });
const after = tweenProps(comp);
expect(after.properties).toEqual({ x: 200 });
expect(after.properties).not.toHaveProperty("y");
expect(after.properties).not.toHaveProperty("opacity");
});
});
describe("mergeTweenProperties (finding #1)", () => {
it("editing x preserves y and opacity through a real SDK write", async () => {
const comp = await openComposition(HTML, { persist: createMemoryAdapter() });
const id = tweenProps(comp).id ?? "";
// Mirror the send site: merge the single edited prop into the existing set.
const merged = mergeTweenProperties(comp, id, { x: 200 }, "to");
expect(merged).toEqual({ x: 200, y: 50, opacity: 1 });
comp.setGsapTween(id, { properties: merged });
const after = tweenProps(comp);
expect(after.properties).toMatchObject({ x: 200, y: 50, opacity: 1 });
});
it("editing a from-property preserves the other from-properties", async () => {
const comp = await openComposition(FROMTO_HTML, { persist: createMemoryAdapter() });
const id = tweenProps(comp).id ?? "";
const merged = mergeTweenProperties(comp, id, { x: 25 }, "from");
expect(merged).toEqual({ x: 25, y: 0 });
});
it("returns the single edit unchanged when the tween id is unknown", async () => {
const comp = await openComposition(HTML, { persist: createMemoryAdapter() });
expect(mergeTweenProperties(comp, "no-such-id", { x: 5 }, "to")).toEqual({ x: 5 });
});
});
@@ -1,16 +1,46 @@
import { useCallback, useEffect, useRef } from "react";
import type { Composition } from "@hyperframes/sdk";
import { parseGsapScriptAcorn } from "@hyperframes/core/gsap-parser-acorn";
import type { DomEditSelection } from "../components/editor/domEditingTypes";
import {
sdkGsapTweenPersist,
sdkGsapRemovePropertyPersist,
type CutoverDeps,
} from "../utils/sdkCutover";
import { extractGsapScriptText } from "../utils/gsapSoftReload";
import { PROPERTY_DEFAULTS } from "./gsapScriptCommitHelpers";
import type { SafeGsapCommitMutation } from "./gsapScriptCommitTypes";
const DEBOUNCE_MS = 150;
/**
* The SDK `setGsapTween` 'set' path REPLACES a tween's editable property set
* (engine `handleSetGsapTween` → `updateAnimationInScript`), so sending only the
* single edited key would silently drop the tween's other animated props. Mirror
* the legacy server path (`{ ...anim.properties, [property]: val }`): read the
* tween's CURRENT properties from the in-memory SDK doc and merge the one edit in,
* so REPLACE semantics preserve siblings. Returns the single-key map unchanged
* when the tween/script can't be found (best-effort; before===after then falls
* back to the server path).
*/
export function mergeTweenProperties(
sdkSession: Composition,
animationId: string,
edited: Record<string, number | string>,
kind: "to" | "from",
): Record<string, number | string> {
try {
const script = extractGsapScriptText(sdkSession.serialize());
if (!script) return { ...edited };
const anim = parseGsapScriptAcorn(script).animations.find((a) => a.id === animationId);
if (!anim) return { ...edited };
const existing = kind === "from" ? (anim.fromProperties ?? {}) : anim.properties;
return { ...existing, ...edited };
} catch {
return { ...edited };
}
}
interface SdkPropertyDeps {
sdkSession?: Composition | null;
sdkDeps?: CutoverDeps | null;
@@ -29,17 +59,32 @@ export function useGsapPropertyDebounce(
} | null>(null);
const debounceTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
// The caller passes `sdk` as a fresh object literal every render. Keying any
// callback's deps on it (esp. flushPendingPropertyEdit, whose identity drives
// the unmount-flush cleanup effect) re-fires the cleanup on EVERY parent
// re-render — so a playhead tick mid-slider-drag would flush + record an undo
// entry per render. Hold the latest value in a ref instead so every callback
// reads current deps without re-subscribing on identity churn.
const sdkRef = useRef(sdk);
sdkRef.current = sdk;
const flushPendingPropertyEdit = useCallback(async () => {
const pending = pendingPropertyEditRef.current;
if (!pending) return;
pendingPropertyEditRef.current = null;
const { selection, animationId, property, value } = pending;
const { sdkSession, sdkDeps, activeCompPath } = sdk ?? {};
const { sdkSession, sdkDeps, activeCompPath } = sdkRef.current ?? {};
if (sdkSession && sdkDeps) {
const targetPath = selection.sourceFile || activeCompPath || "index.html";
const handled = await sdkGsapTweenPersist(
targetPath,
{ kind: "set", animationId, properties: { properties: { [property]: value } } },
{
kind: "set",
animationId,
properties: {
properties: mergeTweenProperties(sdkSession, animationId, { [property]: value }, "to"),
},
},
sdkSession,
sdkDeps,
{ label: `Edit GSAP ${property}`, coalesceKey: `gsap:${animationId}:${property}` },
@@ -55,7 +100,7 @@ export function useGsapPropertyDebounce(
softReload: true,
},
);
}, [commitMutationSafely, sdk]);
}, [commitMutationSafely]);
const updateGsapProperty = useCallback(
(
@@ -93,12 +138,23 @@ export function useGsapPropertyDebounce(
const cs = el.ownerDocument.defaultView?.getComputedStyle(el);
defaultValue = cs ? Number.parseFloat(cs.opacity) || 1 : 1;
}
const { sdkSession, sdkDeps, activeCompPath } = sdk ?? {};
const { sdkSession, sdkDeps, activeCompPath } = sdkRef.current ?? {};
if (sdkSession && sdkDeps) {
const targetPath = selection.sourceFile || activeCompPath || "index.html";
const handled = await sdkGsapTweenPersist(
targetPath,
{ kind: "set", animationId, properties: { properties: { [property]: defaultValue } } },
{
kind: "set",
animationId,
properties: {
properties: mergeTweenProperties(
sdkSession,
animationId,
{ [property]: defaultValue },
"to",
),
},
},
sdkSession,
sdkDeps,
{ label: `Add GSAP ${property}` },
@@ -111,12 +167,12 @@ export function useGsapPropertyDebounce(
{ label: `Add GSAP ${property}` },
);
},
[commitMutationSafely, sdk],
[commitMutationSafely],
);
const removeProperty = useCallback(
async (selection: DomEditSelection, animationId: string, property: string, from: boolean) => {
const { sdkSession, sdkDeps, activeCompPath } = sdk ?? {};
const { sdkSession, sdkDeps, activeCompPath } = sdkRef.current ?? {};
if (sdkSession && sdkDeps) {
const targetPath = selection.sourceFile || activeCompPath || "index.html";
const handled = await sdkGsapRemovePropertyPersist(
@@ -148,7 +204,7 @@ export function useGsapPropertyDebounce(
);
}
},
[commitMutationSafely, sdk],
[commitMutationSafely],
);
const removeGsapProperty = useCallback(
@@ -164,12 +220,23 @@ export function useGsapPropertyDebounce(
property: string,
value: number | string,
) => {
const { sdkSession, sdkDeps, activeCompPath } = sdk ?? {};
const { sdkSession, sdkDeps, activeCompPath } = sdkRef.current ?? {};
if (sdkSession && sdkDeps) {
const targetPath = selection.sourceFile || activeCompPath || "index.html";
const handled = await sdkGsapTweenPersist(
targetPath,
{ kind: "set", animationId, properties: { fromProperties: { [property]: value } } },
{
kind: "set",
animationId,
properties: {
fromProperties: mergeTweenProperties(
sdkSession,
animationId,
{ [property]: value },
"from",
),
},
},
sdkSession,
sdkDeps,
{
@@ -188,13 +255,13 @@ export function useGsapPropertyDebounce(
},
);
},
[commitMutationSafely, sdk],
[commitMutationSafely],
);
const addGsapFromProperty = useCallback(
async (selection: DomEditSelection, animationId: string, property: string) => {
const defaultValue = PROPERTY_DEFAULTS[property] ?? 0;
const { sdkSession, sdkDeps, activeCompPath } = sdk ?? {};
const { sdkSession, sdkDeps, activeCompPath } = sdkRef.current ?? {};
if (sdkSession && sdkDeps) {
const targetPath = selection.sourceFile || activeCompPath || "index.html";
const handled = await sdkGsapTweenPersist(
@@ -202,7 +269,14 @@ export function useGsapPropertyDebounce(
{
kind: "set",
animationId,
properties: { fromProperties: { [property]: defaultValue } },
properties: {
fromProperties: mergeTweenProperties(
sdkSession,
animationId,
{ [property]: defaultValue },
"from",
),
},
},
sdkSession,
sdkDeps,
@@ -216,7 +290,7 @@ export function useGsapPropertyDebounce(
{ label: `Add GSAP from-${property}` },
);
},
[commitMutationSafely, sdk],
[commitMutationSafely],
);
const removeGsapFromProperty = useCallback(
@@ -0,0 +1,85 @@
// @vitest-environment happy-dom
import React, { act, useState } from "react";
import { createRoot } from "react-dom/client";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { useGsapPropertyDebounce } from "./useGsapPropertyDebounce";
import type { DomEditSelection } from "../components/editor/domEditingTypes";
globalThis.IS_REACT_ACT_ENVIRONMENT = true;
// The SDK path is gated on STUDIO_SDK_CUTOVER_ENABLED; keep it OFF so the flush
// routes through commitMutationSafely (the spy we count), keeping the test about
// flush TIMING, not the SDK write path.
vi.mock("../components/editor/manualEditingAvailability", () => ({
STUDIO_SDK_CUTOVER_ENABLED: false,
}));
vi.mock("../utils/studioTelemetry", () => ({ trackStudioEvent: vi.fn() }));
const selection = { sourceFile: "index.html" } as unknown as DomEditSelection;
describe("useGsapPropertyDebounce flush stability (finding #7)", () => {
let container: HTMLDivElement;
beforeEach(() => {
vi.useFakeTimers();
container = document.createElement("div");
document.body.appendChild(container);
});
afterEach(() => {
vi.useRealTimers();
container.remove();
});
it("re-rendering the parent while an edit is pending does NOT flush early or duplicate commits", () => {
const commitMutationSafely = vi.fn();
let queueEdit: (() => void) | null = null;
let forceRerender: (() => void) | null = null;
function Harness() {
const [tick, setTick] = useState(0);
forceRerender = () => setTick((t) => t + 1);
// A FRESH sdk wrapper literal every render — the exact churn that, before
// the ref-stabilization fix, re-fired the unmount-flush cleanup effect.
const ops = useGsapPropertyDebounce(commitMutationSafely, {
sdkSession: null,
sdkDeps: null,
activeCompPath: "index.html",
});
queueEdit = () => ops.updateGsapProperty(selection, "tw-1", "x", tick + 1);
return React.createElement("div", null, String(tick));
}
const root = createRoot(container);
act(() => {
root.render(React.createElement(Harness));
});
// Queue one pending edit.
act(() => {
queueEdit?.();
});
expect(commitMutationSafely).not.toHaveBeenCalled();
// Re-render the parent several times BEFORE the debounce elapses. The bug
// flushed (and recorded a commit) on every re-render via the cleanup effect.
act(() => {
forceRerender?.();
});
act(() => {
forceRerender?.();
});
act(() => {
forceRerender?.();
});
expect(commitMutationSafely).not.toHaveBeenCalled();
// The debounce fires exactly once.
act(() => {
vi.advanceTimersByTime(200);
});
expect(commitMutationSafely).toHaveBeenCalledTimes(1);
act(() => {
root.unmount();
});
});
});
@@ -115,6 +115,28 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra
},
[previewIframeRef, reloadPreview, onCacheInvalidate],
);
// Reuse the SAME per-file serializer the legacy commitMutation path uses, so
// SDK gsap-write flushes serialize against legacy commits AND each other —
// overlapping same-file read-modify-writes can't interleave and lose an edit.
const serializeByFile = useCallback(
<T>(key: string, task: () => Promise<T>): Promise<T> => serializerRef.current(key, task),
[],
);
// Read the on-disk bytes of targetPath so the SDK GSAP persist captures the
// exact prior content as its undo `before` (matching the style/delete paths),
// instead of a normalized full-DOM re-emit that would reformat the whole file.
const readProjectFileContent = useCallback(
async (path: string): Promise<string> => {
const pid = projectIdRef.current;
if (!pid) throw new Error("No active project");
const res = await fetch(`/api/projects/${pid}/files/${encodeURIComponent(path)}`);
if (!res.ok) throw new Error(`Failed to read ${path}`);
const data = (await res.json()) as { content?: string };
if (typeof data.content !== "string") throw new Error(`Missing file contents for ${path}`);
return data.content;
},
[projectIdRef],
);
const sdkDeps = useMemo<CutoverDeps | null>(
() =>
writeProjectFile
@@ -125,6 +147,8 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra
domEditSaveTimestampRef,
refresh: sdkRefresh,
compositionPath: activeCompPath,
serialize: serializeByFile,
readProjectFile: readProjectFileContent,
}
: null,
[
@@ -134,6 +158,8 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra
domEditSaveTimestampRef,
sdkRefresh,
activeCompPath,
serializeByFile,
readProjectFileContent,
],
);
+56 -14
View File
@@ -4,6 +4,7 @@ import { openComposition } from "@hyperframes/sdk";
import { createHttpAdapter } from "@hyperframes/sdk/adapters/http";
import type { Composition } from "@hyperframes/sdk";
import { readStudioFileChangePath } from "../components/editor/manualEdits";
import { isSelfWriteEcho } from "./sdkSelfWriteRegistry";
/**
* True when an external file-change payload targets the active composition and
@@ -24,14 +25,40 @@ export function shouldReloadSdkSession(payload: unknown, activeCompPath: string
* stale. The session has NO persist queue — Studio is the sole file writer; see
* the open effect below.
*/
// Time-window heuristic: suppress file-change reloads for 2 s after our own
// SDK cutover write, to avoid an echo-reload on the write we just committed.
// Footgun: if 2 s is too short (slow FS / network) the reload fires anyway;
// if too long it masks a legitimate external edit. The long-term shape is a
// sequence number or content hash threaded through the persist event so the
// comparison is exact rather than time-based.
// Reload-suppression baseline: a file-change within this window of our own SDK
// cutover write is a CANDIDATE echo, but the decision is content-identity based
// (isSelfWriteEcho) not time-only — so an undo write that lands inside the window
// still reloads (its reverted bytes were never registered as a self-write). The
// window only bounds how long a registered self-write stays suppressible.
const SELF_WRITE_SUPPRESS_MS = 2000;
/** Best-effort read of the changed file's content from a file-change payload. */
function readFileChangeContent(payload: unknown): string | null {
if (!payload || typeof payload !== "object") return null;
const record = payload as Record<string, unknown>;
if (typeof record.content === "string") return record.content;
if ("data" in record) return readFileChangeContent(record.data);
return null;
}
/**
* Decide whether a file-change for the active composition should reload the SDK
* session. `content` is the new on-disk bytes (from the payload or a re-read);
* pass null when unavailable. Content-identity wins: a change whose bytes match a
* registered self-write is our own echo (suppress). Without content we can't prove
* identity, so we fall back to the time window ONLY to suppress an echo — an undo
* write outside the window (or any non-self-write) still reloads. Exported for test.
*/
export function shouldReloadOnFileChange(
activeCompPath: string,
content: string | null,
withinSuppressWindow: boolean,
): boolean {
if (content != null) return !isSelfWriteEcho(activeCompPath, content);
// No content to compare — preserve the old time-window echo suppression.
return !withinSuppressWindow;
}
export interface SdkSessionHandle {
session: Composition | null;
/**
@@ -53,15 +80,30 @@ export function useSdkSession(
// ── Re-open on external change to the active composition ──
useEffect(() => {
if (!activeCompPath) return;
const compPath = activeCompPath;
const readAdapter =
projectId != null
? createHttpAdapter({ projectFilesUrl: `/api/projects/${projectId}` })
: null;
const handler = (payload?: unknown) => {
if (!shouldReloadSdkSession(payload, activeCompPath)) return;
// Suppress reload triggered by our own SDK cutover write.
if (
domEditSaveTimestampRef &&
Date.now() - domEditSaveTimestampRef.current < SELF_WRITE_SUPPRESS_MS
)
if (!shouldReloadSdkSession(payload, compPath)) return;
const withinWindow =
!!domEditSaveTimestampRef &&
Date.now() - domEditSaveTimestampRef.current < SELF_WRITE_SUPPRESS_MS;
const decide = (content: string | null) => {
if (shouldReloadOnFileChange(compPath, content, withinWindow)) setReloadToken((t) => t + 1);
};
const payloadContent = readFileChangeContent(payload);
// Prefer payload content; otherwise re-read so the decision is by IDENTITY
// (an undo's reverted bytes won't match a registered self-write → reload).
if (payloadContent != null || !readAdapter) {
decide(payloadContent);
return;
setReloadToken((t) => t + 1);
}
readAdapter
.read(compPath)
.then((c) => decide(typeof c === "string" ? c : null))
.catch(() => decide(null));
};
if (import.meta.hot) {
import.meta.hot.on("hf:file-change", handler);
@@ -72,7 +114,7 @@ export function useSdkSession(
es.addEventListener("file-change", handler);
return () => es.close();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [activeCompPath]);
}, [activeCompPath, projectId]);
// ── Open / re-open the session ──
useEffect(() => {
@@ -29,6 +29,8 @@ import {
readFileContent,
applyPatchByTarget,
formatTimelineAttributeNumber,
shiftGsapPositions,
scaleGsapPositions,
} from "./timelineEditingHelpers";
import type { PersistTimelineEditInput } from "./timelineEditingHelpers";
import { sdkTimingPersist } from "../utils/sdkCutover";
@@ -91,6 +93,7 @@ export function useTimelineEditing({
element: TimelineElement,
label: string,
buildPatches: PersistTimelineEditInput["buildPatches"],
coalesceKey?: string,
): Promise<void> => {
if (isRecordingRef?.current) {
showToast("Cannot edit timeline while recording", "error");
@@ -110,6 +113,7 @@ export function useTimelineEditing({
recordEdit,
domEditSaveTimestampRef,
pendingTimelineEditPathRef,
coalesceKey,
}),
)
.then(() => {
@@ -154,6 +158,24 @@ export function useTimelineEditing({
value: String(updates.track),
});
};
// Server-path fallback (no SDK session): persist the attr patch, then
// shift GSAP tween positions on the server and reload the preview — the
// SDK path folds both into setTiming, but the fallback must do them
// explicitly or the clip moves while its GSAP tweens stay put + the
// preview never refreshes. coalesceKey mirrors the SDK branch so undo
// granularity is identical on either path.
const coalesceKey = `timeline-move:${element.hfId ?? element.id}`;
const moveFallback = () =>
enqueueEdit(element, "Move timeline clip", buildMovePatches, coalesceKey).then(() => {
const pid = projectIdRef.current;
const delta = updates.start - element.start;
if (delta !== 0 && element.domId && pid) {
return shiftGsapPositions(pid, targetPath, element.domId, delta)
.then(() => reloadPreview())
.catch((err) => console.error("[Timeline] Failed to shift GSAP positions", err));
}
return reloadPreview();
});
if (sdkSession && element.hfId) {
return sdkTimingPersist(
element.hfId,
@@ -166,13 +188,16 @@ export function useTimelineEditing({
reloadPreview,
domEditSaveTimestampRef,
compositionPath: activeCompPath,
// Capture on-disk bytes as the undo `before` so undoing a timing move
// restores the file verbatim, not a normalized full-DOM re-emit.
readProjectFile: (path) => readFileContent(projectIdRef.current ?? "", path),
},
{ label: "Move timeline clip", coalesceKey: `timeline-move:${element.hfId}` },
{ label: "Move timeline clip", coalesceKey },
).then((handled) => {
if (!handled) return enqueueEdit(element, "Move timeline clip", buildMovePatches);
if (!handled) return moveFallback();
});
}
return enqueueEdit(element, "Move timeline clip", buildMovePatches);
return moveFallback();
},
[
previewIframeRef,
@@ -193,10 +218,21 @@ export function useTimelineEditing({
element: TimelineElement,
updates: Pick<TimelineElement, "start" | "duration" | "playbackStart">,
) => {
patchIframeDomTiming(previewIframeRef.current, element, [
const liveAttrs: Array<[string, string]> = [
["data-start", formatTimelineAttributeNumber(updates.start)],
["data-duration", formatTimelineAttributeNumber(updates.duration)],
]);
];
// Patch the live playback-start/media-start attr too, or a resize that
// trims the playback start leaves the preview showing the old in-point
// until the next reload (the persisted patch handles it via pbs below).
if (updates.playbackStart != null) {
const liveAttr =
element.playbackStartAttr === "playback-start"
? "data-playback-start"
: "data-media-start";
liveAttrs.push([liveAttr, formatTimelineAttributeNumber(updates.playbackStart)]);
}
patchIframeDomTiming(previewIframeRef.current, element, liveAttrs);
const targetPath = element.sourceFile || activeCompPath || "index.html";
const buildResizePatches: PersistTimelineEditInput["buildPatches"] = (original, target) => {
const pbs = resolveResizePlaybackStart(original, target, element, updates);
@@ -220,10 +256,38 @@ export function useTimelineEditing({
return patched;
};
// SDK path: skip when a playback-start adjustment is needed (setTiming has no pbs field).
// Condition: no explicit pbs override AND (no start change OR element has no pbs attribute).
// The second clause fires because trimming the start of a clip that has a
// playback-start attribute implicitly shifts that in-point — which the SDK
// setTiming op can't express — so those resizes must take the server path.
const hasPbsAdjustment =
updates.playbackStart != null ||
(updates.start !== element.start && element.playbackStart != null);
// Server-path fallback: after persisting the attr patch, scale GSAP tween
// positions/durations on the server and reload the preview. The SDK path
// folds both into setTiming; the fallback must do them explicitly or the
// clip resizes while its GSAP tweens keep their old timing + the preview
// never refreshes. coalesceKey mirrors the SDK branch for undo parity.
const coalesceKey = `timeline-resize:${element.hfId ?? element.id}`;
const timingChanged =
updates.start !== element.start || updates.duration !== element.duration;
const resizeFallback = () =>
enqueueEdit(element, "Resize timeline clip", buildResizePatches, coalesceKey).then(() => {
const pid = projectIdRef.current;
if (timingChanged && element.domId && pid) {
return scaleGsapPositions(
pid,
targetPath,
element.domId,
element.start,
element.duration,
updates.start,
updates.duration,
)
.then(() => reloadPreview())
.catch((err) => console.error("[Timeline] Failed to scale GSAP positions", err));
}
return reloadPreview();
});
if (sdkSession && element.hfId && !hasPbsAdjustment) {
return sdkTimingPersist(
element.hfId,
@@ -236,13 +300,16 @@ export function useTimelineEditing({
reloadPreview,
domEditSaveTimestampRef,
compositionPath: activeCompPath,
// Capture on-disk bytes as the undo `before` so undoing a timing
// resize restores the file verbatim, not a normalized full-DOM re-emit.
readProjectFile: (path) => readFileContent(projectIdRef.current ?? "", path),
},
{ label: "Resize timeline clip", coalesceKey: `timeline-resize:${element.hfId}` },
{ label: "Resize timeline clip", coalesceKey },
).then((handled) => {
if (!handled) return enqueueEdit(element, "Resize timeline clip", buildResizePatches);
if (!handled) return resizeFallback();
});
}
return enqueueEdit(element, "Resize timeline clip", buildResizePatches);
return resizeFallback();
},
[
previewIframeRef,
@@ -0,0 +1,60 @@
import { describe, expect, it, vi } from "vitest";
// Dark-launch contract: with STUDIO_SDK_CUTOVER_ENABLED=false, EVERY cutover
// persist chokepoint must return false so the caller takes the legacy server
// path — even when a valid SDK session exists (one always does, for
// shadow/selection). This is the contract the prod flag-flip rests on; a future
// refactor of the gate guards that silently re-enables cutover on flag-off
// turns these red. (sdkCutover.test.ts mocks the flag TRUE; this is its sibling.)
vi.mock("../components/editor/manualEditingAvailability", () => ({
STUDIO_SDK_CUTOVER_ENABLED: false,
}));
vi.mock("./studioTelemetry", () => ({ trackStudioEvent: vi.fn() }));
import { sdkTimingPersist, sdkGsapTweenPersist, sdkDeletePersist } from "./sdkCutover";
const makeSession = () =>
({
getElement: () => ({ inlineStyles: {} }),
serialize: () => "<html></html>",
batch: (fn: () => void) => fn(),
setTiming: vi.fn(),
dispatch: vi.fn(),
}) as never;
const makeDeps = () =>
({
editHistory: { recordEdit: vi.fn().mockResolvedValue(undefined) },
writeProjectFile: vi.fn().mockResolvedValue(undefined),
reloadPreview: vi.fn(),
domEditSaveTimestampRef: { current: 0 },
}) as never;
describe("dark-launch gate — STUDIO_SDK_CUTOVER_ENABLED=false ⇒ persist returns false", () => {
it("sdkTimingPersist falls back without writing", async () => {
const deps = makeDeps();
expect(await sdkTimingPersist("hf-a", "/c.html", { start: 1 }, makeSession(), deps)).toBe(
false,
);
expect(
(deps as unknown as { writeProjectFile: ReturnType<typeof vi.fn> }).writeProjectFile,
).not.toHaveBeenCalled();
});
it("sdkGsapTweenPersist (shared GSAP-op chokepoint) falls back", async () => {
expect(
await sdkGsapTweenPersist(
"/c.html",
{ kind: "remove", animationId: "a" },
makeSession(),
makeDeps(),
),
).toBe(false);
});
it("sdkDeletePersist falls back", async () => {
expect(
await sdkDeletePersist("hf-a", "<html></html>", "/c.html", makeSession(), makeDeps()),
).toBe(false);
});
});
@@ -455,6 +455,153 @@ describe("sdkTimingPersist", () => {
expect(result).toBe(false);
expect(deps.writeProjectFile).not.toHaveBeenCalled();
});
// Finding #12: undo baseline must be the EXACT on-disk bytes (matching the
// style/delete paths), not a normalized SDK serialize() re-emit — otherwise
// undoing a timing edit reformats the whole file.
it("records the on-disk content (not serialize()) as the undo before when a reader is provided", async () => {
const deps = {
...makeDeps(),
readProjectFile: vi.fn().mockResolvedValue("<html>EXACT ON-DISK BYTES</html>"),
};
const session = makeSession(true);
await sdkTimingPersist("hf-clip", "/comp.html", { start: 3 }, session, deps);
expect(deps.readProjectFile).toHaveBeenCalledWith("/comp.html");
expect(deps.editHistory.recordEdit).toHaveBeenCalledWith(
expect.objectContaining({
files: {
"/comp.html": { before: "<html>EXACT ON-DISK BYTES</html>", after: "<html>after</html>" },
},
}),
);
});
it("falls back to serialize() before when the reader throws", async () => {
const deps = {
...makeDeps(),
readProjectFile: vi.fn().mockRejectedValue(new Error("read failed")),
};
const session = makeSession(true);
await sdkTimingPersist("hf-clip", "/comp.html", { start: 3 }, session, deps);
expect(deps.editHistory.recordEdit).toHaveBeenCalledWith(
expect.objectContaining({
files: { "/comp.html": { before: "<html>before</html>", after: "<html>after</html>" } },
}),
);
});
});
describe("sdkGsapTweenPersist — undo baseline (finding #12)", () => {
const makeRef = <T>(val: T): MutableRefObject<T> => ({ current: val });
const makeSession = () =>
({
getElement: vi.fn().mockReturnValue({ id: "hf-box" }),
setGsapTween: vi.fn(),
serialize: vi
.fn()
.mockReturnValueOnce("<html>serialized-before</html>")
.mockReturnValue("<html>after</html>"),
batch: vi.fn((fn: () => void) => fn()),
}) as unknown as Parameters<typeof sdkGsapTweenPersist>[2];
it("records the on-disk content as the undo before, not serialize()", async () => {
const deps = {
editHistory: { recordEdit: vi.fn().mockResolvedValue(undefined) },
writeProjectFile: vi.fn().mockResolvedValue(undefined),
reloadPreview: vi.fn(),
domEditSaveTimestampRef: makeRef(0),
readProjectFile: vi.fn().mockResolvedValue("<html>on-disk gsap bytes</html>"),
};
const session = makeSession();
await sdkGsapTweenPersist(
"/comp.html",
{ kind: "set", animationId: "tw-1", properties: { ease: "power3.in" } },
session,
deps,
);
expect(deps.editHistory.recordEdit).toHaveBeenCalledWith(
expect.objectContaining({
files: {
"/comp.html": { before: "<html>on-disk gsap bytes</html>", after: "<html>after</html>" },
},
}),
);
});
});
describe("sdkGsapTweenPersist — per-file serialization (finding #8)", () => {
const makeRef = <T>(val: T): MutableRefObject<T> => ({ current: val });
it("routes the read-modify-write through the keyed serializer so same-file flushes can't interleave", async () => {
const order: string[] = [];
let writeResolve: (() => void) | null = null;
const deps = {
editHistory: { recordEdit: vi.fn().mockResolvedValue(undefined) },
// First write blocks until we release it, so without serialization the
// second op's serialize()/dispatch would interleave ahead of it.
writeProjectFile: vi.fn().mockImplementation((_p: string, content: string) => {
order.push(`write-start:${content}`);
if (content === "<html>after-1</html>") {
return new Promise<void>((res) => {
writeResolve = () => {
order.push(`write-done:${content}`);
res();
};
});
}
order.push(`write-done:${content}`);
return Promise.resolve();
}),
reloadPreview: vi.fn(),
domEditSaveTimestampRef: makeRef(0),
// A real per-key serializer: tasks under the same key run strictly in order.
serialize: (() => {
const inFlight = new Map<string, Promise<unknown>>();
return <T>(key: string, task: () => Promise<T>): Promise<T> => {
const prior = inFlight.get(key) ?? Promise.resolve();
const next = prior.then(task, task);
inFlight.set(key, next);
return next as Promise<T>;
};
})(),
};
let serializeCall = 0;
const session = {
getElement: vi.fn().mockReturnValue({ id: "hf-box" }),
setGsapTween: vi.fn(() => order.push("dispatch")),
serialize: vi.fn(() => {
serializeCall++;
// before-1, after-1, before-2, after-2
return `<html>${serializeCall % 2 === 1 ? "before" : "after"}-${Math.ceil(serializeCall / 2)}</html>`;
}),
batch: vi.fn((fn: () => void) => fn()),
} as unknown as Parameters<typeof sdkGsapTweenPersist>[2];
const p1 = sdkGsapTweenPersist(
"/comp.html",
{ kind: "set", animationId: "tw-1", properties: { ease: "a" } },
session,
deps,
);
const p2 = sdkGsapTweenPersist(
"/comp.html",
{ kind: "set", animationId: "tw-1", properties: { ease: "b" } },
session,
deps,
);
// Let the first op reach its (blocked) write before releasing it.
await Promise.resolve();
await Promise.resolve();
writeResolve?.();
await Promise.all([p1, p2]);
// The second op's write must NOT start before the first op's write completes.
const firstWriteDone = order.indexOf("write-done:<html>after-1</html>");
const secondWriteStart = order.indexOf("write-start:<html>after-2</html>");
expect(firstWriteDone).toBeGreaterThanOrEqual(0);
expect(secondWriteStart).toBeGreaterThan(firstWriteDone);
});
});
describe("sdkGsapTweenPersist", () => {
+85 -15
View File
@@ -5,6 +5,7 @@ import type { EditHistoryKind } from "./editHistory";
import type { PatchOperation } from "./sourcePatcher";
import { STUDIO_SDK_CUTOVER_ENABLED } from "../components/editor/manualEditingAvailability";
import { trackStudioEvent } from "./studioTelemetry";
import { markSelfWrite } from "../hooks/sdkSelfWriteRegistry";
const CUTOVER_OP_TYPES = new Set<PatchOperation["type"]>([
"inline-style",
@@ -90,6 +91,42 @@ export interface CutoverDeps {
* otherwise we'd write the full active-comp serialization into that file.
*/
compositionPath?: string | null;
/**
* Optional per-key task serializer (the same `gsap-file:${file}` serializer the
* legacy `commitMutation` uses). When provided, every GSAP-op persist routes its
* read-serialize dispatch serialize write through it so two concurrent
* same-file flushes can't interleave their read-modify-write and lose an edit.
* Absent (e.g. in unit tests) ops run unserialized as before.
*/
serialize?: <T>(key: string, task: () => Promise<T>) => Promise<T>;
/**
* Optional reader for the on-disk content of targetPath. Timing/GSAP persists
* use it to capture the EXACT prior bytes as the undo-history `before`, so undo
* restores the file verbatim instead of a normalized SDK re-emit (which would
* reformat the whole file). The style/delete paths already thread originalContent
* in explicitly; this gives timing/GSAP parity without touching every call site.
* Absent falls back to the SDK's pre-edit serialize() (the prior behavior).
*/
readProjectFile?: (path: string) => Promise<string>;
}
/**
* Capture the undo-history `before` baseline for timing/GSAP persists: the exact
* on-disk bytes when a reader is available (so undo restores them verbatim),
* falling back to the SDK's pre-edit serialization when it isn't. Never throws
* a failed read degrades to the serialized fallback rather than aborting the edit.
*/
async function captureOnDiskBefore(
deps: CutoverDeps,
targetPath: string,
serializedFallback: string,
): Promise<string> {
if (!deps.readProjectFile) return serializedFallback;
try {
return await deps.readProjectFile(targetPath);
} catch {
return serializedFallback;
}
}
/** True when targetPath isn't the composition the SDK session models. */
@@ -115,6 +152,11 @@ async function persistSdkSerialize(
options?: CutoverOptions,
): Promise<void> {
deps.domEditSaveTimestampRef.current = Date.now();
// Tag this write with the exact content (by hash) so the file-change
// reload-suppression can recognize its own echo by IDENTITY, not just a 2 s
// clock — an undo write (different bytes, not registered here) then always
// reloads instead of being swallowed by the time window.
markSelfWrite(targetPath, after);
await deps.writeProjectFile(targetPath, after);
await deps.editHistory.recordEdit({
label: options?.label ?? "Edit layer",
@@ -171,14 +213,22 @@ export async function sdkTimingPersist(
deps: CutoverDeps,
options?: CutoverOptions,
): Promise<boolean> {
// Dark-launch gate: without this, timing cutover runs whenever an SDK session
// exists (it always does, for shadow/selection) — flipping the flag OFF would
// NOT disable it. Gate here so flag-off routes back to the legacy server path.
if (!STUDIO_SDK_CUTOVER_ENABLED) return false;
if (!sdkSession || !sdkSession.getElement(hfId)) return false;
if (wrongCompositionFile(deps, targetPath)) return false;
try {
const before = sdkSession.serialize();
const serializedBefore = sdkSession.serialize();
sdkSession.batch(() => sdkSession.setTiming(hfId, timingUpdate));
const after = sdkSession.serialize();
if (after === before) return false;
await persistSdkSerialize(after, targetPath, before, deps, options);
if (after === serializedBefore) return false;
// Undo baseline = exact on-disk bytes (matching the style/delete paths), so
// undoing a timing edit restores the file verbatim instead of a normalized
// full-DOM re-emit. Falls back to serializedBefore when no reader is wired.
const undoBefore = await captureOnDiskBefore(deps, targetPath, serializedBefore);
await persistSdkSerialize(after, targetPath, undoBefore, deps, options);
trackStudioEvent("sdk_cutover_success", { hfId, opCount: 1 });
return true;
} catch (err) {
@@ -199,6 +249,9 @@ export function sdkGsapTweenPersist(
deps: CutoverDeps,
options?: CutoverOptions,
): Promise<boolean> {
// Leading dark-launch gate so flag-off does no SDK touch (getElement) at all —
// matches the other three chokepoints' discipline.
if (!STUDIO_SDK_CUTOVER_ENABLED) return Promise.resolve(false);
if (op.kind === "add" && sdkSession && !sdkSession.getElement(op.target))
return Promise.resolve(false);
// dispatchGsapOpAndPersist returns false on before===after — that catches stale
@@ -224,20 +277,35 @@ async function dispatchGsapOpAndPersist(
options: CutoverOptions | undefined,
dispatch: (s: Composition) => void,
): Promise<boolean> {
// Dark-launch gate (shared chokepoint for every GSAP-op cutover persist):
// flag OFF → return false → caller falls back to the legacy server path.
if (!STUDIO_SDK_CUTOVER_ENABLED) return false;
if (!sdkSession) return false;
if (wrongCompositionFile(deps, targetPath)) return false;
try {
const before = sdkSession.serialize();
dispatch(sdkSession);
const after = sdkSession.serialize();
if (after === before) return false;
await persistSdkSerialize(after, targetPath, before, deps, options);
trackStudioEvent("sdk_cutover_success", { opCount: 1 });
return true;
} catch (err) {
trackStudioEvent("sdk_cutover_fallback", { error: String(err) });
return false;
}
const session = sdkSession;
// Route the whole read-serialize → dispatch → serialize → write through the
// per-file serializer (when provided) so overlapping same-file flushes can't
// interleave their read-modify-write and drop an edit, matching the legacy
// commitMutation path's `gsap-file:${file}` serialization.
const run = async (): Promise<boolean> => {
try {
const serializedBefore = session.serialize();
dispatch(session);
const after = session.serialize();
if (after === serializedBefore) return false;
// Undo baseline = exact on-disk bytes (matching the style/delete paths), so
// undoing a GSAP edit restores the file verbatim instead of a normalized
// full-DOM re-emit. Falls back to serializedBefore when no reader is wired.
const undoBefore = await captureOnDiskBefore(deps, targetPath, serializedBefore);
await persistSdkSerialize(after, targetPath, undoBefore, deps, options);
trackStudioEvent("sdk_cutover_success", { opCount: 1 });
return true;
} catch (err) {
trackStudioEvent("sdk_cutover_fallback", { error: String(err) });
return false;
}
};
return deps.serialize ? deps.serialize(`gsap-file:${targetPath}`, run) : run();
}
export function sdkGsapKeyframePersist(
@@ -325,6 +393,8 @@ export async function sdkDeletePersist(
sdkSession: Composition | null | undefined,
deps: CutoverDeps,
): Promise<boolean> {
// Dark-launch gate: flag OFF → legacy server delete path.
if (!STUDIO_SDK_CUTOVER_ENABLED) return false;
if (!sdkSession || !sdkSession.getElement(hfId)) return false;
if (wrongCompositionFile(deps, targetPath)) return false;
try {