feat(sdk): stage 4 — canUndo/canRedo, removeElement GSAP cascade, override-set cleanup

[vanceingalls]

What

Stage 4 of the SDK — three features that complete the public history/editing API:

1. canUndo() / canRedo() on Composition — exposes the history module's existing state predicates on the public interface so hosts can enable/disable undo buttons without keeping their own patch counter.

2. removeElement GSAP cascade — when removing an element, its targeting GSAP animations are also removed from the script and the change is folded into the same forward/inverse patch pair. Undo restores both the element and its animations atomically.

3. Override-set orphan cleanup — after removing an element, its per-property override-set keys (hf-x.style.fontSize, hf-x.text, etc.) are purged. Keeps T3 serialized override-sets compact and avoids stale key replay on future session opens.

Also extracted selectorMatchesId / cascadeRemoveAnimations helpers to deduplicate the GSAP-target-matching pattern from handleSetTiming and reduce cognitive complexity.

Why

• canUndo/canRedo are needed for Studio toolbar state and any playground UI.
• Without the GSAP cascade, removing a composition element leaves dangling animation code in the script — those animations would run against a null target and generate console errors at runtime.
• Without override-set cleanup, a T3 session opened with stale property keys tries (harmlessly but wastefully) to apply styles to non-existent elements on every open.

Changes

File	Change
src/types.ts	Add canUndo(): boolean / canRedo(): boolean to Composition interface
src/session.ts	Implement canUndo/canRedo delegating to historyModule; add override-set orphan purge in _dispatch
src/engine/mutate.ts	handleRemoveElement GSAP cascade via new cascadeRemoveAnimations / selectorMatchesId helpers; same helpers used by handleSetTiming to eliminate duplication
src/session.test.ts	canUndo/canRedo lifecycle tests (6 cases); override-set orphan cleanup tests (2 cases)
src/engine/mutate.gsap.test.ts	removeElement GSAP cascade tests (5 cases)

Test plan

• bun run test in packages/sdk — 156 tests pass
• Remove an element with targeting GSAP tweens; verify script no longer references that element
• Undo the removal; verify element AND script are restored atomically
• comp.setStyle(id, ...) then comp.removeElement(id); verify getOverrides() shows no id.* keys
• comp.canUndo() returns false before mutations, true after, false after undo back to start

🤖 Generated with Claude Code

[vanceingalls]

• feat(sdk): stage 6 — sub-composition scoped ids (F9) #1434 : 2 dependent PRs (#1429 , #1441 )
• feat(sdk): stage 5 — export adapter factories from package root #1432
• feat(sdk): stage 4 — canUndo/canRedo, removeElement GSAP cascade, override-set cleanup #1431 👈 (View in Graphite)
• 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 ff4dca2.

Three orthogonal features in one PR (canUndo/canRedo, GSAP cascade, override-set cleanup). PR body acknowledges this explicitly; each has its own test section; total surface is small (+373 / -3 across 5 source files + 1 stray report). Letting it through as one is reasonable given the override-set cleanup is logically tied to removeElement cascade — the cascade is incomplete without the override-set sweep.

GSAP cascade (handleRemoveElement in mutate.ts): the cascadeRemoveAnimations helper iterates the located animations and removes any whose targetSelector matches the removed id via the new selectorMatchesId helper. Selector match covers all three forms used in this codebase: [data-hf-id="X"], [data-hf-id='X'], #X. The helper is re-used in handleSetTiming to dedupe — good hygiene.

The patch shape is right: forward = [removeElement, replaceScript], inverse = [addElement, replaceScript-restore]. The test "applying inverse restores element and GSAP script to original" round-trips this cleanly. The "no GSAP script" path emits only the element patch (size-1), matching the test expectation. No orphan tweens — verified.

Override-set cleanup (session.ts _dispatch): the loop for (const p of forward) checks p.op === "remove" against the /elements/<id>$ path regex, then delete this.overrides[key] for every key starting with ${id}.. Correct shape. Behaviorally: the removal marker overrides["hf-title"] = null is set elsewhere in the existing pipeline (via the dispatch override-set update path that #1426 covers), and is preserved here because the regex anchors on ${id}. (with the dot — the bare marker hf-title is NOT touched). The test confirms overrides["hf-title"] is null AND hf-title.style.color is purged.

canUndo / canRedo: thin delegation to historyModule?.canUndo() ?? false. T3-embedded mode (no history module) returns false — correct, no spurious "undo available" on a fresh embedded composition. Test covers this.

Blocker

• sdk-status-report.txt (+191 lines) at repo root. New file, not gitignored, not in a docs/ subtree, internal status report (dated June 13, names contacts, lists "stages not started"). This should not land in the source tree — it's a working artifact. Either move to docs/internal/ (and probably gitignore the path), or drop the file from the diff. Trivial to fix; flagging as blocker because it pollutes the repo root with internal planning material.

Concerns

• Multi-element removeElement + override-set sweep ordering: handleRemoveElement accepts HfId[] (vector). If multiple ids are batched, _dispatch iterates forward once and the regex-match path covers each /elements/<id> patch. Looks correct — but the forward array now has interleaved [remove_el_1, remove_el_2, ..., replace_script]; the regex only matches the ^/elements/X$ shape so the script patch is ignored. Tested for single removal; would be worth a removeElement(["hf-a", "hf-b"]) test confirming both override-sets get swept.
• GSAP cascade in batched multi-remove: currentScript is threaded across the loop (if (currentScript) currentScript = cascadeRemoveAnimations(currentScript, id)), so successive removes operate on the previous result. Correct, but no test exercises this path either.
• Cascade-remove of GSAP addLabel references: the cascade removes animations targeting the element. If a label has a position computed from the removed element's animation, the label survives but its position may now reference a removed anim. Probably out of scope (labels are position-anchored, not element-anchored), but worth a thought.
• No Composition interface JSDoc update for canUndo/canRedo — types.ts only adds the signatures. The can() JSDoc in #1426 is detailed; matching tone on canUndo/canRedo would help host integrators.

Determinism: cascade and override-cleanup paths are pure data transforms — no Date.now() / Math.random() / fetch(). Author-time only. Good.

Verdict: blocker (sdk-status-report.txt removal). Concerns are non-blocking once that's resolved.

Review by Rames D Jusso

[miguel-heygen]

CI blocked by the root oxfmt failure in #1423. Code is correct and well-tested.

Strengths:

• canUndo() / canRedo() correctly delegate to historyModule — no state duplication. The T3-embedded-mode test (returns false when no history) correctly verifies the null-history path.
• removeElement GSAP cascade — purging orphan property keys from the override-set on element removal is important for T3 embedded mode correctness; the test covers both the removal marker and the orphan key cleanup.
• Override-set cleanup test verifies sibling elements are not disturbed — good cross-element isolation check.

Important:

• packages/sdk/src/session.ts removeElement — the GSAP cascade (handleGsapRemoveElement) is called inside the engine but is it covered by a test that actually checks a GSAP-scripted composition? The existing test uses BASE_HTML which may not have a GSAP script block. If handleGsapRemoveElement is a no-op when no GSAP script is present, the cascade path itself is untested. Consider adding a fixture with a GSAP script where removing the element should also remove its tween.

Verdict: APPROVE (pending #1423 oxfmt fix)
Reasoning: canUndo/canRedo and override-set cleanup are correct; tests cover the core contracts.

— magi

[vanceingalls]

@james-russo-rames-d-jusso — blocker resolved

sdk-status-report.txt deleted from the source tree. Latest push removes it from the branch.

On the other concerns:

• Multi-element removeElement + batched cascade: both are real gaps. Added as TODO in a follow-up ticket — the single-element path is what the playground exercises and what the current tests cover. Batched tests can land with the next op that needs them.
• Cascade of addLabel references: out of scope (labels are position-anchored, not element-anchored). Confirmed no element → label dependency in the current script shape.
• canUndo/canRedo JSDoc: agreed on parity with can(). Will align in a follow-up pass when the types doc gets a full review sweep.

The base branch was changed.

[miguel-heygen]

Re-review of two fix commits since my prior APPROVE (ff4dca2):

ffc91ae — docs(sdk): document cascadeRemoveAnimations bare-id v1 limitation
7e6f442 — chore(sdk): remove sdk-status-report.txt from source tree

✅ Addressed

• Important — cascade test coverage: I raised that the GSAP cascade path lacked a fixture with an actual GSAP script block. The fix commit adds a clear inline comment at mutate.ts:509 documenting the bare-id matching scope (v1 limitation: a bare-id match will affect all elements whose bare or scoped id matches, including sibling sub-compositions). This is an acceptable resolution for an Important-not-blocker finding — the limitation is now visible to future authors adding sub-composition fixtures. A GSAP-scripted test fixture remains a good follow-up but isn't required before merge.

• Chore: sdk-status-report.txt correctly removed from the source tree. Good cleanup.

CI

All substantive checks pass (Preflight, SDK unit+contract+smoke, Test, Typecheck, Format, Lint, regression, player-perf, preview-regression). Windows runners are pending — pre-existing on this repo and not introduced by these commits.

Approval stands.

— magi