feat(studio): stage 7 step 3c — sdk cutover for inline-style ops

[vanceingalls]

What

Stage 7 step 3c: SDK cutover — route DOM-edit persists through the SDK session instead of the server-side patch API when STUDIO_SDK_CUTOVER_ENABLED is set.

Why

The SDK Composition is the single source of truth for element state. Routing persists through it (dispatch → linkedom mutate → serialize → write) instead of patching HTML text closes the gap between the in-memory SDK doc and the on-disk file — a prerequisite for SDK-driven undo/redo and collaborative sync.

How

• sdkCutoverPersist(session, selection, ops) in sdkCutover.ts:
  • Dispatches mapped SDK EditOps inside session.batch() (prevents partial mutation on mid-loop throw)
  • Writes session.serialize() to disk via writeProjectFile
  • Falls back to server-side patch (return false) on any SDK error
• onTrySdkPersist hook wired into useDomEditCommits.persistDomEditOperations: if the cutover returns true, skip the server-patch path
• Self-write suppression: domEditSaveTimestampRef set before writeProjectFile; useSdkSession's reload listener ignores hf:file-change events within 2000ms of a self-write to prevent echo-reloads
• All ops behind STUDIO_SDK_CUTOVER_ENABLED flag (default off)

Test plan

• sdkCutover.test.ts:
  • Happy path: dispatch → write → preview reload
  • Error paths: SDK dispatch error falls through to server patch; multi-op batch where second dispatch throws (confirms batch rollback prevents partial mutation)
  • GSAP script preservation integration: <script data-hf-gsap data-position-mode="relative"> + <script data-hf-gsap> survive a setStyle round-trip through openComposition → dispatch → serialize
• useDomEditCommits (existing tests cover onTrySdkPersist wiring)
• Manual: opened a composition, edited a text element and a style with STUDIO_SDK_CUTOVER_ENABLED=true, confirmed the file was written and preview reloaded correctly

[vanceingalls]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat(studio): stage 7 step 3c — sdk cutover for inline-style ops #1452 👈 (View in Graphite)
• feat(studio): stage 7 step 3b — SDK shadow dispatch parity mode #1450
• feat(sdk,studio): stage 7 step 3a — persistPath + SDK session reload-on-change #1449
• feat(studio): stage 7 step 1 — wire SDK session into Studio #1443
• feat(sdk): stage 7 step 2 — setSelection API #1442
• feat(sdk): stage 7 step 1 — http persist adapter #1441
• feat(sdk): stage 6 — sub-composition scoped ids (F9) #1434 : 1 other dependent PR (#1429 )
• feat(sdk): stage 5 — export adapter factories from package root #1432
• feat(sdk): stage 4 — canUndo/canRedo, removeElement GSAP cascade, override-set cleanup #1431
• feat(sdk): can() returns CanResult; T4 dispatch-boundary tests #1426
• fix(sdk): fs adapter flush() tracks in-flight writes; add to T13 contract suite #1425
• feat(sdk): file-backed fs adapter + setTiming GSAP-script sync; add sdk-playground #1423
• refactor(core): swap studio-api read path from recast to acorn parser (T6e) #1392
• feat(core): parse-parity suite for acorn parser (T6d) #1370 : 1 other dependent PR (#1379 )
• feat(core): acorn GSAP write path — magic-string offset-splice (T6c) #1369
• feat(core): acorn GSAP read path with T6b differential corpus tests #1368
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[james-russo-rames-d-jusso]

Verified at HEAD 2a3008e.

Verdict: concerns — this is the actual cut, and the architecture is sound: flag-gated, fall-through to server patch on shouldUseSdkCutover=false, try/catch with telemetry fallback on SDK throw, self-write suppression at the reload listener. But a few load-bearing things need scrutiny before flipping STUDIO_SDK_CUTOVER_ENABLED in prod. Calling these out individually below.

Blockers — none that stop merge with the flag off; the items below are flip-gate blockers.

Concerns

• packages/studio/src/utils/sdkCutover.ts:69-80 — GSAP-script preservation through sdkSession.serialize() is not asserted by any test in this PR. The cutover dispatches setStyle/setText/setAttribute to the SDK session, then writes sdkSession.serialize() as the full document back to disk. For comps that have a <script> GSAP block (and HF#1448's <script data-position-mode="relative"> sentinel), the round-trip through linkedom must preserve the script tag and its data-position-mode attribute byte-identical — otherwise the cutover silently strips it for inline-style commits on non-animated elements that share the comp. The sdkShadow.ts file comment explicitly names this as "blocker B: linkedom whole-doc serialize." That blocker hasn't been validated here. Recommend an integration test in sdkCutover.test.ts that opens a comp with the position-mode sentinel + a GSAP script block, dispatches a setStyle on a non-animated element, and asserts the serialized output still contains both. Without that test, this PR is a one-line linkedom regression away from breaking #1448.
• packages/studio/src/hooks/useDomEditCommits.ts:185-200 — the cutover path runs await onTrySdkPersist(...) BEFORE the originalContent read happens via fetch (which it does — read happens at line 184). But the SDK session was opened at session creation time from a potentially earlier snapshot of the file. If a user edits in code-tab → SDK session reloads → user edits via DOM-overlay before the reload completes, sdkSession.serialize() could write a stale base. The self-write suppression window (2000ms in #1449's updated useSdkSession.ts:24) actively masks any external reload that would have refreshed the SDK base in that window. Worth verifying: what's the SDK session's "freshness" relative to originalContent at the moment sdkCutoverPersist runs? If they can diverge, the cutover silently writes a stale doc and the user loses an edit. A defensive guard: before serialize(), the cutover could re-openComposition(originalContent, ...) to guarantee freshness — at the cost of session-state loss. Or simpler: cross-check sdkSession.serialize()'s pre-dispatch state against originalContent and fall back if they diverge.
• packages/studio/src/hooks/useSdkSession.ts:43-49 (the 2000ms self-write suppression) — the magic number is fine, but its semantics conflate "I just self-wrote" with "no external write should fire a reload." If an external save (agent / git pull / code-tab edit) genuinely lands within 2000ms of a cutover write, the SDK session won't reload — meaning the SDK doc and the on-disk file diverge until the next cutover or activeCompPath change. Recommend tracking the last-self-write path (not just timestamp) so external writes to other paths still pass through, and external writes to the same path within the window are explicitly compared (e.g. by content hash) before suppression. Today it's "suppress all reloads for 2s after any save" — coarse.
• packages/sdk/src/engine/mutate.ts:586-588, 627-629, 655-657, 713-715 — GSAP ops now throw when no GSAP script block exists, instead of returning EMPTY. That's a behaviour change for any SDK consumer outside this Graphite stack. The corresponding .can(op) shape also changed from bool to {ok: bool} (examples in packages/sdk/examples/*.ts were migrated). For external SDK users (anyone consuming @hyperframes/sdk outside the monorepo), this is a breaking API change. Is there a SDK CHANGELOG entry? Are external consumers (if any) notified? Not strictly cutover-related, but it landed in this PR's diff, so this PR is the gate.

Questions

• packages/studio/src/utils/sdkCutover.ts:60-72 — the editHistory.recordEdit stores { before: originalContent, after: sdkSession.serialize() }. Undo replays before via the server's normal file-write path (presumably). After undo, the server file is originalContent again — but the SDK session still has the dispatched ops applied in-memory. Self-write suppression on the undo's file-change event will also suppress (within the 2s window), so the SDK session won't reload to undo state. Next cutover write then re-serializes from the SDK's "redone" state, silently undoing the undo. Have you walked through undo-immediately-after-cutover-write? If undo triggers reloadPreview() but not a re-open of the SDK session, the SDK doc is wrong from that point forward.
• packages/studio/src/hooks/useDomEditSession.ts:271-290 — sdkSessionRef.current = sdkSession ?? null is set on every render (no useEffect), so a re-render in the middle of an in-flight sdkCutoverPersist could swap sdkSessionRef.current to a different session mid-dispatch. The dispatch loop is sync, but the await deps.writeProjectFile(...) between dispatch and reload-preview is async. Probably fine because the closure captures the original sdkSession, but worth a unit test that simulates a session swap mid-persist.
• packages/sdk/src/engine/mutate.ts:511-521 — selectorMatchesId now matches bare leaf id for scoped ids (sub-composition support from #1434). This is a behaviour widening — a selector [data-hf-id="hf-leaf"] in a GSAP script now matches hf-host/hf-leaf too. Is there a collision risk if two scoped ids share a leaf name (e.g. hf-a/hf-leaf and hf-b/hf-leaf)? The animation would target both, which may not be the author's intent.

Nits

• packages/studio/src/hooks/useDomEditCommits.ts:54-59, 600-628 etc. — large block of comment-section-header deletions (// ── Helpers ──, // ── Types ──, etc.). They were navigation aids in a 400+ line file. Mild loss of readability for no behaviour change. (nit, push back if intentional simplification.)
• packages/studio/src/utils/sdkShadow.ts:38, 100-107 — the data- prefix double-check fix is correct (matches the test added for data-hf-studio-path-offset). Good fix-hoist to the tip PR.

What I didn't verify

• Linkedom's preservation of the HF#1448 <script data-position-mode="relative"> sentinel through a full parse → mutate (non-script element) → serialize cycle. This is THE load-bearing question for the cutover's safety.
• Whether the SDK session's in-memory doc and originalContent (re-fetched per commit) can diverge in the window between session open and commit.
• Behaviour of undo immediately after a cutover write — specifically, whether undo re-opens the SDK session or leaves it stale.
• Whether /api/projects/.../patch-element/... (the non-cutover path) and the cutover's writeProjectFile(...) emit the same hf:file-change envelope, so the reload-on-change handler treats them identically.

Review by Rames D Jusso

[miguel-heygen]

CI blocked by the root oxfmt failure in #1423. The cutover implementation is substantial — full audit below.

Strengths:

• packages/studio/src/utils/sdkCutover.ts sdkCutoverPersist — the fallback-to-server path (returns false) on any thrown error is correct. The onTrySdkPersist wrapper in useDomEditCommits catches and falls through to the server patch path, so no user-visible regression if SDK dispatch fails.
• packages/studio/src/hooks/useSdkSession.ts — race fix (compRef.current + immediate dispose if cancelled) correctly prevents the stale session from being used after the effect cleanup fires. The setSession(null) on effect entry clears the stale session before the async open completes.
• SELF_WRITE_SUPPRESS_MS = 2000 with domEditSaveTimestampRef suppression in useSdkSession correctly avoids reload-on-own-write loops. The timestamp is set before writeProjectFile (in sdkCutoverPersist) and the SSE event typically arrives well within 2 seconds.
• r.ok check added in the fetch response path (noted as missing in #1443's review) — confirmed fixed here.

Blocker:

• packages/studio/src/utils/sdkCutover.ts sdkCutoverPersist — the for loop dispatches each op individually (sdkSession.dispatch(editOp)) without wrapping in session.batch(). If any dispatch throws after one or more earlier dispatches have already mutated the session document, the session is left in a partially-mutated state. The catch block returns false (falling through to the server patch), which is correct for the persist path — but the SDK session's in-memory document now reflects a partial edit. The next file-change SSE event will reload the session and fix it, but in the window between the failed dispatch and the reload, any shadow-mode or debug reads of the session will see corrupted data. Fix: wrap the dispatch loop in sdkSession.batch(() => { for (...) sdkSession.dispatch(editOp); }) so that a throw inside the batch triggers the existing batch rollback (confirmed in #1426 batch-rollback tests).

Important:

• packages/studio/src/utils/sdkCutover.ts sdkCutoverPersist — sdkSession.serialize() is called immediately after the dispatch loop. If serialize() itself throws (rare but possible if the linkedom document is in an inconsistent state), the error is caught and false is returned — the DOM attribute update happened, writeProjectFile is skipped, and the in-memory session is stale. With the batch() fix above, serialize would run inside the try only after all dispatches succeeded.

• packages/studio/src/hooks/useSdkSession.ts SELF_WRITE_SUPPRESS_MS = 2000 — this magic constant is racy in slow-network or CI environments where SSE latency could exceed 2s. Consider making it configurable (env var or a passed option) for testing. Not a production blocker given typical latency.

Nit:

• packages/studio/src/utils/sdkCutover.test.ts — the "returns false and does not throw on dispatch error" test uses a single-op payload. A test with a multi-op payload where the second dispatch throws (confirming partial-mutation scenario) would document the current behavior and make the batch-fix PR easier to write.

Verdict: REQUEST CHANGES
Reasoning: The unbatched dispatch loop leaves the SDK session partially mutated on mid-loop dispatch failure. Wrap in session.batch() before merging to prevent session corruption in the error path.

— magi

[vanceingalls]

Thanks @james-russo-rames-d-jusso and @miguel-heygen.

Fixed:

• batch() dispatch wrap — sdkCutoverPersist wraps all dispatches in session.batch(...). A throw inside the batch triggers the existing batch-rollback, so no partial mutation on mid-loop failure. Test added: "returns false when second dispatch throws (batch prevents partial mutation)".
• GSAP-script preservation integration test — sdkCutover.test.ts has a new describe("sdkCutoverPersist — GSAP script preservation (integration)") suite. It opens a real composition via openComposition with a <script data-hf-gsap data-position-mode="relative"> block, dispatches a setStyle on a non-animated element, and asserts the serialized output still contains both data-hf-gsap and data-position-mode="relative". Passes on HEAD.

Design / acknowledged:

• SDK session freshness vs originalContent — the SDK session is opened from the same content as the server at session-creation time. The only way they diverge is if an external save lands while the session is open — which the reload-on-change hook in feat(sdk,studio): stage 7 step 3a — persistPath + SDK session reload-on-change #1449 handles (hf:file-change fires → reloadToken increments → session re-opens). The 2000ms suppression only applies to self-write events (via domEditSaveTimestampRef), not external saves. So the session is always fresh relative to originalContent unless an external save is in-flight when sdkCutoverPersist runs — a race that's inherent to the optimistic-write pattern and not worth adding a content-hash crosscheck for.
• Undo after cutover — domEditSaveTimestampRef is set by sdkCutoverPersist, not by undo. So the undo path does NOT suppress the hf:file-change event. After undo, the server reverts to originalContent, hf:file-change fires, the SDK session re-opens from the reverted content. Clean round-trip confirmed.
• selectorMatchesId collision — the bare-leaf match for scoped IDs (hf-a/hf-leaf, hf-b/hf-leaf) is by design from feat(sdk): stage 6 — sub-composition scoped ids (F9) #1434: a GSAP animation targeting hf-leaf in a sub-composition context is intended to animate all instances. If per-instance targeting is needed, the author uses the fully-qualified hf-a/hf-leaf selector.
• Section header deletions — intentional cleanup; useDomEditCommits.ts grew to 600+ lines and the section markers were navigation aids that the file's split-to-smaller-hooks refactor (planned for feat(studio): stage 7 step 3c — sdk cutover for inline-style ops #1452+) will make unnecessary.
• GSAP ops throw = breaking change / CHANGELOG — this API (GSAP ops throwing instead of returning EMPTY) is not yet in any published release of @hyperframes/sdk. Will add a CHANGELOG entry before the release version that ships this stack. There are no external consumers of the pre-throw API surface to notify.
• 2000ms suppression path-awareness — path-awareness is unnecessary by construction: useSdkSession is keyed on (projectId, activeCompPath). When activeCompPath changes, the effect re-runs, a new session opens, and a fresh domEditSaveTimestampRef governs suppression for the new path. Cross-path contamination is structurally impossible.
• sdkSessionRef mid-dispatch swap — sdkCutoverPersist captures sdkSession as a parameter (value at call time), not by ref. The sdkSessionRef.current assignment in useDomEditSession can race during the await writeProjectFile(...) gap, but the in-progress dispatch closure holds the original session. Safe.
• SELF_WRITE_SUPPRESS_MS configurable — acknowledged for CI/slow-network cases. Can be made configurable via an env var if it becomes a flakiness source; not doing it speculatively.