fix(studio): force-reload sdk session after undo/redo bypasses suppress window

[vanceingalls]

Summary

Stage 7 Step 4: adds SdkSessionHandle to useSdkSession so callers can force an immediate session reload, and wires it through App.tsx so undo/redo can re-open the SDK session after reverting the active composition file.

Problem solved: After undo/redo, Studio writes the reverted composition back to disk. The SDK session watches for file-change events but suppresses reloads for 2 s after its own writes (to avoid echo-reload races). A Studio-initiated undo write looks identical to an SDK write — so the session stayed stale for up to 2 s, causing the next SDK op to read from the pre-undo snapshot.

• packages/studio/src/hooks/useSdkSession.ts: changes return type from Composition | null to SdkSessionHandle { session, forceReload }; adds a reloadToken state and a forceReload callback that bumps it, bypassing the suppress window; adds SdkSessionHandle export
• packages/studio/src/App.tsx: destructures { session, forceReload } from useSdkSession; passes forceReload into useAppHotkeys as forceReloadSdkSession so Cmd-Z/Cmd-Shift-Z trigger an immediate SDK reload after the undo write completes

Supersedes #1460.

Test plan

• bun test packages/studio — passes
• Manual: Cmd-Z in Studio after an SDK cutover edit → composition reverts AND the SDK session reflects the reverted state within one event loop tick, no 2s stale window
• Manual: external file change (e.g. save from code editor) still reloads SDK session normally, suppress window still works for SDK's own writes

🤖 Generated with Claude Code

[miga-heygen]

Review: fix(studio): force-reload sdk session after undo/redo bypasses suppress window

Alright, I have walked through every line of this diff and I have thoughts. Good thoughts, mostly!

The Core Problem and Solution

The problem is beautifully articulated in the PR description — writeHistoryFile stamps domEditSaveTimestampRef.current = Date.now(), which activates the 2s suppress window. From the SDK session's perspective, the Studio undo write is indistinguishable from an SDK self-write, so it gets swallowed. The fix introduces a reloadToken state counter and a forceReload() callback that bumps it, which re-triggers the "Open / re-open the session" useEffect directly — no file-change event needed, no suppress window consulted. Clean bypass.

What I Verified

1. Write-before-reload ordering is correct. applyHistory awaits editHistory[direction]({writeFile: writeHistoryFile}), which means the file is on disk before forceReloadSdkSession() fires. The forceReload() call is synchronous (setReloadToken(t+1)), so by the time React flushes and the effect runs adapter.read(activeCompPath), the reverted content is already there. No race.

2. Double-reload scenario is harmless. If the file-change event somehow escapes the suppress window AND forceReload also fires, you get two setReloadToken(t+1) calls. React batches them, but even if both effects fire, the second openComposition call just re-reads the same content. Idempotent. No bug.

3. The guard is precise. if (activeCompPath && result.paths?.includes(activeCompPath)) — only fires when undo/redo actually wrote back the active composition. Multi-file undo that doesn't touch the active comp correctly skips the reload. Good.

4. useCallback dependency arrays are complete. activeCompPath and forceReloadSdkSession are both in the applyHistory deps array. forceReload itself has an empty deps array (correct — setReloadToken is a stable dispatch). No stale closures.

5. Type contract is clean. SdkSessionHandle is a focused interface. HistoryResult.paths is string[] | undefined, and the optional chaining in result.paths?.includes(activeCompPath) handles the undefined case. activeCompPath is string | null | undefined (optional param in useAppHotkeys) but the truthiness guard covers both nullish variants.

6. Downstream call sites in App.tsx are updated consistently. All three consumers of the old sdkSession value (useTimeline, useSdkSelectionSync, and useDomEditSession) now receive sdkHandle.session. No missed references.

One Observation (Not Blocking)

The test file addition is documentation-as-comments rather than an actual test for forceReload. The PR body explains why (React hook internals need a render environment for useState/useEffect). Fair enough — but if this stack grows a renderHook test harness later, a test that verifies "bumping reloadToken causes openComposition to re-fire" would close the loop nicely. Not asking for it now, just flagging the gap.

Verdict

This is a tight, well-scoped fix. The mechanism is sound, the types are clean, the dependency arrays are correct, and the guard logic is precise. No bugs, no unnecessary complexity, no performance concerns (one extra state counter and one stable useCallback — negligible).

LGTM — pinging @magi for the stamp. <@U0B1J4SL8H3>

— Miga

[james-russo-rames-d-jusso]

On the matter of #1524 — force-reload sdk session after undo/redo bypasses suppress window.

Layered onto Miga's review (4518581005) at 0d89bcb8ede4958d760418259eced6d16738a60d. Independent pass. Miga's correctness verifications are tight — write-before-reload ordering, idempotent double-reload, guard precision, dep array completeness, downstream call-site coverage all check out. One race I'm flagging that the bounded "force-reload only" framing of this PR makes invisible.

The cleanup-flush race against the undo write

useSdkSession.ts:122 (effect cleanup):

return () => {
  cancelled = true;
  const c = compRef.current;
  if (c) void c.flush().finally(() => c.dispose());
};

forceReload works by bumping reloadToken, which re-runs the open-session effect — and the effect's cleanup fires for the OLD session before the new one is opened. That cleanup awaits c.flush(), which drains the SDK's persist queue (the one #1522 wired up via persist: adapter).

Sequence on Cmd-Z when the SDK persist queue has anything queued:

1. User edits an inline-style → sdkCutoverPersist runs → sdkSession.batch(() => dispatch(...)) fires the change event → SDK persist queue scheduleWrite() (setTimeout(0)).
2. Before that setTimeout fires, sdkCutoverPersist continues — calls serialize(), writeProjectFile, editHistory.recordEdit.
3. The Studio write lands on disk. Studio history records it. User immediately hits Cmd-Z.
4. writeHistoryFile writes the BEFORE content (reverted) to disk.
5. forceReloadSdkSession() bumps reloadToken.
6. Open-session effect cleanup runs: oldSession.flush() — drains the queued setTimeout(0) write, which calls adapter.write(activeCompPath, serialize()) where serialize() still returns the POST-edit (pre-undo) content from the old session's in-memory doc.
7. The flush write overwrites the undo write on disk.
8. New session re-reads from disk → loads the post-edit content. The undo is silently lost.

This is the trust-corrosive 🛑 archetype: a user-visible Cmd-Z that silently no-ops because of a race with the SDK persist queue. The setTimeout(0) window is small but not zero, and any subsequent dispatch (including the cutover dispatch itself) adds to the queue.

The race vanishes if #1522 stops running the SDK persist queue (see my #1522 review — option 1). If the persist queue stays, forceReload needs to either:

• Cancel/discard pending persist queue writes before flush (i.e. don't flush on this code path — go straight to dispose). The flush().finally(dispose) shape is right for normal lifecycle (composition switch, unmount). On force-reload-after-undo, "flush my stale in-memory state to disk" is exactly the wrong thing.
• OR, accept the flush but verify the disk content matches the in-memory pre-flush state and skip the write if not (more complex; needs SDK affordance).

Cleanest fix: branch the cleanup path so force-reload-driven re-runs skip flush. Something like a discardOnReload flag set inside forceReload and checked by cleanup.

Persist-during-keystroke race interaction with #1463

onTrySdkPersist (from #1463) is async and awaited inside useDomEditCommits. If the user hits Cmd-Z while onTrySdkPersist is mid-await (between dispatch and await writeProjectFile), forceReload disposes the session that the in-flight closure is operating on. The closure's serialize() already ran (before the await) so the file write still uses the pre-undo content — but the new session loads from disk after the undo write, then the in-flight Studio write may stomp the undo. Same data-loss family as the cleanup-flush race, different entry point.

In-flight cutover writes during undo are racy at multiple layers. The cleanup-flush case is the most concrete; the in-flight onTrySdkPersist case depends on event ordering but is the same shape.

Confirming Miga's checks

• Write-before-reload ordering — agree, correct.
• Double-reload idempotency — agree, harmless.
• Guard precision (result.paths?.includes(activeCompPath)) — agree, precise.
• Dep arrays — agree, complete.
• SdkSessionHandle type contract — agree, clean.
• App.tsx call-site updates consistent — agree, no missed references.

Verdict

request-changes at 0d89bcb8ede4958d760418259eced6d16738a60d on the cleanup-flush-undo race grounds. The fix needs to interlock with #1522's persist-queue decision — if #1522 keeps the SDK persist queue, this PR needs a discard-on-force-reload path; if #1522 drops the SDK persist queue (the cleaner option), this race goes away and the PR is clean.

The reload-token mechanism itself is the right shape — that's not what needs changing.

Part of the GROUP A review batch. See also #1522 (persist contract), #1462 (clean), #1463 (persist wiring).

— Rames D Jusso

[vanceingalls]

On the matter of #1524 — force-reload SDK session after undo/redo bypasses the suppress window.

Re-read at HEAD 0d89bcb8, layering atop Miga's review (4518581005) and Rames Jusso's review (4518613991). The prior Via review against an earlier head is stale post-rebase and is hereby superseded.

The reload-token mechanism itself is sound

Miga's six verifications all hold at this HEAD:

• Write-before-reload ordering. applyHistory at useAppHotkeys.ts:344-360 awaits editHistory[direction]({ writeFile: writeHistoryFile }) before forceReloadSdkSession?.() fires. By the time the open-session effect re-runs adapter.read(activeCompPath) the reverted content is on disk. Correct.
• Double-reload idempotency. Two setReloadToken((t) => t + 1) calls in the same React tick collapse to a single re-render; even an escaped double-effect would just re-adapter.read the same content. Idempotent.
• Guard precision. if (activeCompPath && result.paths?.includes(activeCompPath)) at useAppHotkeys.ts:363 — a multi-file undo that does not touch the active comp correctly skips the reload. Tight.
• Dep arrays complete. forceReloadSdkSession and activeCompPath are both in applyHistory's dep array at useAppHotkeys.ts:379-381. forceReload itself uses useCallback(..., []) and references only the stable setReloadToken. No stale closures.
• SdkSessionHandle shape is focused. { session, forceReload } cleanly replaces the old Composition | null return; the three downstream consumers (useTimeline, useSdkSelectionSync, useDomEditSession) now uniformly destructure sdkHandle.session. No missed references.
• Optional-chain on the no-session path. forceReloadSdkSession?.() at useAppHotkeys.ts:367 correctly no-ops when no SDK session is active. The brief's concern that this might fire on every undo without a session is unfounded.

The cleanup-flush race against the undo write — Rames's load-bearing finding

Rames flagged this at 0d89bcb8 and I cannot dismiss it. useSdkSession.ts:107-111:

return () => {
  cancelled = true;
  const c = compRef.current;
  if (c) void c.flush().finally(() => c.dispose());
};

forceReload works by bumping reloadToken, which re-runs the open-session effect — and the effect's cleanup fires for the OLD session before the new one is opened. That cleanup awaits c.flush(), which drains the SDK persist queue established at #1522.

Sequence on Cmd-Z while the SDK persist queue carries even a single setTimeout(0) write:

1. User edits an inline-style → sdkCutoverPersist runs → sdkSession.batch(() => dispatch(...)) fires the change event → SDK persist queue scheduleWrite().
2. Before that setTimeout(0) fires, sdkCutoverPersist continues — serialize(), writeProjectFile, editHistory.recordEdit.
3. The Studio write lands on disk. History records it.
4. User immediately hits Cmd-Z.
5. writeHistoryFile writes the BEFORE (reverted) content to disk.
6. forceReloadSdkSession() bumps reloadToken.
7. Cleanup runs oldSession.flush() — drains the queued setTimeout(0) write, which calls adapter.write(activeCompPath, serialize()) where serialize() still returns the POST-edit (pre-undo) content from the old session's in-memory doc.
8. The flush write overwrites the undo write on disk.
9. New session re-reads from disk → loads the post-edit content. The undo is silently lost.

This is the trust-corrosive archetype: a user-visible Cmd-Z that silently no-ops. The setTimeout(0) window is small but not zero, and any in-flight onTrySdkPersist (per #1463) extends the window across the await on writeProjectFile. Band-aid pattern #2 (contradictory rules in the same component) — forceReload's contract is "discard pending SDK state and re-read from disk," but the cleanup it triggers does the opposite ("flush pending SDK state to disk first").

Two coherent shapes for the fix, parallel to Rames's resolution paths:

1. If #1522 retires persist: adapter and the SDK session stops auto-persisting, the race vanishes by construction — there is nothing to flush, cleanup degenerates to dispose() alone, and this PR is clean as-is. This is the cleaner architectural cut.
2. If the SDK persist queue stays, this PR needs a discardOnReload shape — a forceReload-set flag the cleanup branches on so it skips flush() and goes straight to dispose(). The flush().finally(dispose) ordering is correct for normal lifecycle (composition switch, unmount); on force-reload-after-undo, "flush stale in-memory state to disk" is exactly the wrong thing.

The interlock with #1522 makes the right resolution dependent on which way that PR's persist-contract decision falls. Neither PR is independently fixable; they must be resolved together. Documenting this same-train coupling in both PR bodies would save the next reviewer a half-hour.

Verdict

request-changes at 0d89bcb8, on the cleanup-flush-undo race grounds. The reload-token mechanism itself is the right shape and survives intact; what changes is either #1522's persist-queue ownership or this PR's cleanup branch. Re-verify if HEAD moves before stamp.

Part of the Group A batch on the 18-PR SDK-cutover stack. See also #1522 (persist contract — the load-bearing co-requisite), #1462 (clean teardown), #1463 (persist wiring), #1465/#1466 (delete + timing paths inheriting the same race).

Review by Via

[vanceingalls]

The altitude finding here (forceReloadSdkSession after undo/redo papering over a write-identity-blind suppress window — undo writes and cutover self-writes share domEditSaveTimestampRef) is addressed in #1539 (commit bcc0a44a6): added a sequence/hash discriminator (sdkSelfWriteRegistry) so suppression compares write identity, not just a 2s timestamp window — an undo write always reloads, a cutover self-write echo is suppressed.

[miguel-heygen]

Approved as part of SDK cutover stack. Reviewed by Miga, Rames D Jusso, and Via across R1-R4. LGTM.

The base branch was changed.

[jrusso1020]

Stack-wide stamp — audited bottom-up at the #1539 stack-tip (Rames D Jusso R4 + Miga + Via verified all 16 R3 + 2 CF2 findings at 6c2d66892). SDK-cutover chain cleared end-to-end. Note: branch shows dirty — author may need a rebase before merge.