feat(sdk,studio): ws-4 — add history:false option; disable unused sdk undo in studio

[vanceingalls]

Summary

Adds a history?: false option to openComposition() so hosts that own their own undo stack can skip attaching the SDK's internal history module. Applies it in Studio, where the existing hotkey-driven undo was silently firing on Cmd-Z but calling the SDK undo (a no-op write) instead of the Studio undo.

• packages/sdk/src/session.ts: adds optional history?: false to the OpenCompositionOptions interface; when set, the history module is not attached to the session (standalone non-embedded mode still gets history by default)
• packages/studio/src/hooks/useSdkSession.ts: passes history: false to openComposition() — Studio owns the undo stack via its own edit history, SDK undo is dead weight here and was producing confusing no-op Cmd-Z presses

Test plan

• bun test packages/sdk — passes
• Manual: Cmd-Z in Studio correctly invokes Studio undo (not SDK undo stub)
• Manual: openComposition(html) without opts still attaches history normally (standalone mode unaffected)

🤖 Generated with Claude Code

[vanceingalls]

• feat(sdk,core): ws-3 — unrollDynamicAnimations acorn port + SDK op #1501
• feat(sdk): stage 6 — arc path ops (setArcPath, updateArcSegment, removeArcPath) #1500
• feat(sdk,core): ws-3 prerequisites — acorn keyframe-collapse foundation + removeAllKeyframes #1499
• feat(sdk,studio): ws-1.2 — percentage-based removeGsapKeyframe #1498
• feat(sdk,studio): ws-1.1 — add set method to GsapTweenSpec; route addGsapAnimation(set) through sdk #1497
• feat(sdk,studio): ws-4 — add history:false option; disable unused sdk undo in studio #1496 👈 (View in Graphite)
• feat(sdk): ws-a2 — applyDraft/commitPreview/cancelPreview → moveElement op #1490
• feat(sdk): ws-a1 — iframe preview adapter (hit-test + selection) #1489
• fix(studio): resolve cross-cutting SDK-cutover review findings #1471
• feat(studio): route GSAP keyframe add through SDK (§3.5 PR2) #1470
• feat(studio): route GSAP tween add/update/delete through SDK (§3.5 PR1) #1469
• chore(studio): document CSS-path position cut-over; GSAP-path deferred (§3.3) #1467
• feat(studio): route timeline trim/move through SDK setTiming (§3.2) #1466
• feat(studio): route element delete through SDK removeElement (§3.1) #1465
• fix(studio): wire onTrySdkPersist to sdkCutoverPersist (cutover was unwired) #1463
• feat(studio): s7.5 — delete shadow scaffolding; keep cutover flag (dark launch) #1462
• fix(studio): force-reload sdk session after undo/redo bypasses suppress window #1460
• feat(studio): stage 7 step 3c — sdk cutover for inline-style ops #1452
• feat(core): acorn GSAP read path with T6b differential corpus tests #1488
• main

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

[vanceingalls]

Hey vance — small one to start the SDK side of the stack.

Summary of intent. Adds a history: false opt-out to openComposition so Studio (whose editHistory is the authoritative undo stack) can stop the SDK from spinning up its own history module that nobody reads. A trim of dead weight, not a feature.

PR-template flag. Filled boilerplate only — for a two-line surface change that's fine, no real ask here.

Per-file findings.

• packages/sdk/src/session.ts:51-56 — option introduced as history?: false (literal-false-only), not history?: boolean. Elegant: callers cannot enable history twice, only suppress the default. The default-undefined branch falls through to the existing !isEmbedded attach.
• packages/sdk/src/session.ts:509 — guard becomes !isEmbedded && opts?.history !== false. Embedded sessions remain untouched (they never got history anyway), and the new opt-out only matters in the standalone branch. Surface unchanged for every other caller.
• packages/studio/src/hooks/useSdkSession.ts:100-102 — Studio passes { history: false }. The doc-line above is the right archeology — explicit that Studio's editHistory is canonical and SDK history is unused.

Dispatch chain. Single boolean gate with one new caller. No switches, no fan-out — nothing to misroute.

CI. Preflight (lint + format) and preview-regression are failing across this whole stack — unlikely to be #1496-specific. Worth confirming once the parent of the stack re-runs clean.

Verdict. Clear-with-no-nits, modulo CI green. The trim is so small that it's effectively a no-op for any non-Studio consumer.

Cleared SHA: f1e6e32d. Any push past that voids this review.

Review by Via

[miga-heygen]

Review: ws-4 — history:false option

Clean, well-scoped change. Two files, 10 additions, 2 deletions. Let me walk through what I found.

The good

Type design is sharp. history?: false as a literal type is exactly right — it communicates "this is an opt-out flag, not a boolean config knob." You can't accidentally pass true and think you're doing something. The undefined-or-false semantics are self-documenting.

The guard reads naturally. !isEmbedded && opts?.history !== false is clear at a glance — two independent reasons to skip history, and both are explicit. The optional chaining handles the undefined-opts case without a separate null check.

Existing undo/redo methods are already null-safe. historyModule?.undo() and canUndo() ?? false mean a session opened with history: false won't throw — undo is a silent no-op and canUndo() returns false. No downstream callers need changes.

The Studio call site is correct. useSdkSession already doesn't pass persist, so the only module being suppressed is the history module — exactly what's intended. The comment update explaining why SDK undo is dead weight in Studio (forceReloadSdkSession discards it) is a nice touch for future readers.

One thing to flag (non-blocking)

history: false also suppresses the persist queue because the persist-queue creation lives inside the same if (!isEmbedded && opts?.history !== false) block. Today this is fine — Studio doesn't pass persist, and no other caller passes both persist and history: false. But the coupling is a silent foot-gun: a future caller writing openComposition(html, { persist: adapter, history: false }) would expect auto-save without undo, and would get neither. Two options if you want to address this later:

1. Split the guard: move persist-queue creation to its own if (!isEmbedded && opts?.persist) block.
2. Document the coupling in the JSDoc for history — "Note: also suppresses the persist queue."

Not blocking since no current call site hits this, and it's easy to unravel when the need arises. Just wanted to flag it before the interface calcifies.

Verdict

No bugs, no edge-case surprises, CI is green. The change does exactly what the PR says and nothing more.

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

— Miga

[james-russo-rames-d-jusso]

Independent pass, layered on top of Via and Miga.

Verdict. Approve-with-no-changes. Two files, ten lines, exactly the right shape. history?: false as a literal-false rather than boolean makes the API opt-out-only (the default-attach case is anchored to the source instead of split between two truthy values), and the !isEmbedded && opts?.history !== false guard reads correctly at a glance.

Cross-PR-coherence finding (the fresh angle for this stack). Group D adds four op-level primitives (set, removeGsapKeyframe by %, removeAllKeyframes, arc-path ops) on top of this history:false opt-out. None of those ops accept a history flag of their own — Studio's session-wide opt-out at useSdkSession.ts:100-102 is the only switch, and it's binary at composition-open time. That's the right design if Studio is the only host that needs per-session opt-out. If a future host wants per-op opt-out ("undo this batch via my own stack, undo this single op via SDK"), the door is still open — the SDK's historyModule?.undo() calls remain unaffected by the new ops. Just noting the shape so the design intent is on record before the API surface calcifies through Group E+.

On Miga's persist-queue coupling note (#1496 review). Concur — the persist queue creation is gated by the same !isEmbedded && opts?.history !== false block. Worth a one-line JSDoc clarification ("also suppresses the persist queue") even though no current caller hits the both-set-together case. Cheap to do in the next push.

CI. All green on 884b244.

— Rames D Jusso

(Reviewed at 884b2442dd4010fb04fd338b2a432d90d64216d2. Part of 18-PR HF SDK cutover stack, Group D review pass.)

[vanceingalls]

Second pass at HEAD 884b2442, after rebase from prior cleared SHA f1e6e32d. Co-reviewers (Miga, Rames) have already landed independent passes at this same SHA; layering rather than re-litigating.

Verdict. Clean — ready for stamp.

Re-verification. Two files, ten lines, exactly what the title says — history?: false literal-false opt-out on OpenCompositionOptions at packages/sdk/src/session.ts:51-56, guard reshaped to !isEmbedded && opts?.history !== false at session.ts:509, and useSdkSession.ts:100-102 passes { history: false } with the right archeology comment above it. The literal-false rather than boolean is the elegant part — you cannot accidentally pass true and think you've enabled anything; the only legal call shape is the opt-out.

Dispatch chain (re-checked at current head). Single boolean gate, single new caller (Studio). No switches, no fan-out. historyModule?.undo() and canUndo() ?? false are already null-safe, so a session opened with history: false is silent-no-op-safe for downstream undo callers — no surprise throws.

Concur with Miga's persist-queue coupling note. The persist queue and the history module share the same if (!isEmbedded && opts?.history !== false) block, so a hypothetical future caller writing openComposition(html, { persist: adapter, history: false }) would silently get neither persist nor history when they expected one. No live caller hits this — Studio doesn't pass persist — but a one-line JSDoc clarifier (or splitting the guard) closes the door before the API surface calcifies. Non-blocking; worth a follow-up commit.

Concur with Rames's history flag-shape forward note. The session-wide opt-out at composition-open time is the right shape for Studio's case (host owns the undo stack, full stop). If a future host wants per-op opt-out (some batches host-driven, some SDK-driven undo), the SDK's historyModule?.undo() calls are still unaffected, so the door's still open — just flagging that the API surface is session-wide-binary for now, which is sufficient until it isn't.

CI. Green on 884b2442 across regression, player-perf, preview-regression, preview-parity. No infra noise on this run.

Cleared at 884b2442. Re-verify if HEAD moves before stamp.

Review by Via

[miguel-heygen]

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

[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.

The base branch was changed.