fix(studio): respect GSAP transform ownership

[miguel-heygen]

Stack 1/2 after the parser foundation. Studio edits directly writable GSAP source or blocks before writing competing CSS. Covers drag and resize failure semantics. 616 changed lines. Split from #2984.

[vanceingalls]

APPROVE — Studio drag/resize/rotate handlers correctly route edits to writable GSAP source or block with actionable WHY-toast before any competing CSS write. Block-reason symmetry with #2985's parser editability enum (literal→direct, runtime-dynamic→source, helper|loop→unroll) verified.

Three P2 + one nit (no blockers):

• packages/studio/src/hooks/useAnimatedPropertyCommit.ts (bare } catch { bumpGsapCache(); } tail) — swallows the new GsapEditBlockedError that materializeIfDynamic now throws (previously it returned void). The wrapper at useGsapAwareEditing.ts:352-368 becomes dead code for this path. Scenario: user with autoKeyframeEnabled=false opens the 3D inspector on a helper- or runtime-dynamic tween and edits rotationX — silent no-op (just a cache bump), while drag/resize/rotate on the same tween correctly show the "Choose Unroll…" / "Edit in Code tab" toast. Inconsistent WHY-messaging across paths — the same block reason surfaces differently depending on whether the edit came from a gesture or the inspector. Fix: rethrow GsapEditBlockedError from the raw commit (or narrow the swallow to non-blocked errors).
• packages/studio/src/hooks/gsapRuntimeBridge.ts:539-543 — preflightGsapDragIntercept unconditionally awaits fetchFallbackAnimations(); the main body of tryGsapDragIntercept (via resolveGroupTween / findGsapPositionAnimation on sourceAnimations) may fetch again. Every drag doubles fallback cost. Cache the resolved list on first call and reuse.
• packages/studio/src/hooks/gsapRuntimeBridge.ts:326-340, gsapResizeIntercept.ts:62-73 — API asymmetry: tryGsapDragIntercept returns Promise<GsapEditOutcome> (caller uses assertGsapEditPersisted), but tryGsapRotationIntercept/tryGsapResizeIntercept still throw GsapEditBlockedError directly. Mixed contract is easy to miss in follow-ups; consider unifying.
• Nit: packages/studio/src/hooks/useGsapAwareEditing.ts:157-176 — the group preflight loop passes animations: [] to each member's tryGsapDragIntercept(..., preflightOnly: true). Behavior looks intentional (heterogeneous members) — worth a code comment recording the intent.

Standards: CI green; no new Fallow-ignore lines beyond the pre-existing one on tryGsapDragIntercept.

— Review by Via

[miga-heygen]

R1 — GSAP transform ownership (head 5ac984f)

Verdict: Approve (cross-PR dependency flagged)

Good systematic replacement of silent no-ops with explicit GsapEditBlockedError throws. The gsapEditOutcome.ts module is well-designed: one error class, one COPY map, one assertGsapAnimationDirectlyEditable gate used at all intercept entry points.

SSOT check

• GsapEditBlockedError is the single error type for all block reasons. COPY map centralizes messages + suggested actions. Good SSOT.
• assertGsapAnimationDirectlyEditable is the single editability gate, used in drag preflight, resize, rotation, and animated property commit. No duplication.
• materializeIfDynamic is gutted to a simple throw — the old runtime-rewrite logic is removed, not relocated. Clean.

Cross-checked Via's findings

1. commitAnimatedProperties bare-catch swallows GsapEditBlockedError — CONFIRMED, and this is the merge-order dependency. The catch block at the bottom of commitAnimatedProperties in useAnimatedPropertyCommit.ts is catch { bumpGsapCache(); } — swallows everything. This PR adds assertGsapAnimationDirectlyEditable throws INSIDE that try block, but the bare catch eats them. #2987 fixes this by changing the catch to catch (error) { bumpGsapCache(); throw error; }. Without #2987, inspector property edits on blocked animations silently no-op instead of toasting the error.

2. API asymmetry: drag returns outcome, rotate/resize throw — confirmed. tryGsapDragIntercept returns GsapEditOutcome. tryGsapRotationIntercept/tryGsapResizeIntercept throw GsapEditBlockedError. Both work, but the contract is asymmetric. The preflightOnly option on drag justified the outcome type; rotation/resize don't need preflight, so throws are simpler. Acceptable P2.

3. preflightGsapDragIntercept double-fetches fallback — trusting Via's finding here; the preflight calls await fetchFallbackAnimations(), and the main body likely still calls it separately. P2.

Merge-order constraint

#2986 and #2987 MUST land together or in immediate succession. With #2986 alone:

• Drag/resize/rotate → toast (correct)
• Inspector property commit → silently swallowed (regression: same outcome as before, but now inconsistent with the other paths)

With both PRs: all paths toast + snap-back. The stack is correct in its final state but has a mid-stack window.

---

Review by Miga

[james-russo-rames-d-jusso]

Reviewed at 5ac984ff.

Load-bearing PR of the stack, so I went deep. Peer R1 (Via) has APPROVE with three P2s + a cross-PR pattern; I diverge on severity for two items and add several correctness findings peer missed. Two of my findings are flagged blocker-level relative to the customer-visible HITL regression this stack is supposed to close — I want them adjudicated before merge, but I'm comfortable being talked down if the sequencing story explains them.

BLOCKER A — inspector edit-path swallows the new GsapEditBlockedError. The PR adds an aspirational wrapper in useGsapAwareEditing.ts:342-366 that expects commitAnimatedPropertyRaw to reject with GsapEditBlockedError, then reports telemetry + rethrows. But the raw impl (useAnimatedPropertyCommit.ts:395-534) wraps the whole routing block in try { … } catch { bumpGsapCache(); } — the outer catch is unreachable. Net: inspector-driven 3D-rotation / opacity / any-property edit on a helper/runtime-owned tween silently no-ops (bump cache, return, done — no toast, no rejection), while drag/resize/rotate on the same tween surface the block correctly. This is the exact HITL regression signature ("edit disappears, no feedback") the parent PR #2984 committed to closing. Via classifies this as P2 (approves the merge); I'm reading the HITL frame as making this a stack-purpose blocker. Adjudicate before merge — either land the unswallow inside this PR, or explicitly de-scope inspector in the body and back out the aspirational wrapper + tautological test.

BLOCKER B — group-drag preflight bypasses the ownership check on transient fetch failure. useGsapAwareEditing.ts:164-196 passes [] for the in-memory animations cache to both preflight and commit loops. preflightGsapDragIntercept uses [...animations, ...fetchedAnimations] — when fetchFallbackAnimations returns [] (definitive empty OR fetch failure — the comment at gsapRuntimeBridge.ts:143 explicitly warns), the editability loop iterates zero items and every group member's helper/runtime-source tween passes the check vacuously. Under a transient network failure during a multi-select drag, some members commit competing CSS on helper-owned layers, GSAP overwrites on tick, partial-commit is visible to the user with no error state. Fix: either pass the union of each selection's own selectedGsapAnimations, or distinguish "definitive empty" from "fetch failure" (make makeFetchFallback throw on failure so the preflight's catch propagates as blocked).

Five more concerns, all inline. Highlights:

• Resize / rotation preflight lacks the live-runtime fallback the drag path already has — helper-generated width/rotationX tweens the parser can't see slip past.
• Rotation intercept filter uses propertyGroup === "rotation" which per gsapConstants.ts:59 bundles skewX/skewY and misses rotationX/Y/Z. Two failure modes in one filter.
• Sibling intercept API is asymmetric — drag returns typed GsapEditOutcome, resize/rotate still throw. Callers must know which shape to catch.
• GsapEditAction type + add-selector/unroll/open-code copy defined, zero UX consumers — dead scaffolding at merge time.
• Group-drag "user-atomic" comment overpromises — commit refetch race between preflight and commit loop can partial-commit under concurrent source edits.

Endorsing Via's convergent findings — the P2 on preflight double-fetch (fold into a memoized fetch across preflight + commit) and the API-asymmetry point (all three intercepts should return GsapEditOutcome) are correct and worth landing.

Endorsing Via's cross-PR pattern — the "explain WHY the edit was blocked" surface is inconsistent (toast on drag/resize/rotate, silent no-op on inspector, silent snap-back on transform-commit per #2987). Unifying at the useGsapInteractionFailureTelemetry boundary before shipping would close the same-block-three-surfaces gap that BLOCKER A opens.

Downstream to #2987 reviewers: BLOCKER A's swallowing catch lives at useAnimatedPropertyCommit.ts:395-534 — #2987 DOES modify that file (see #2987 diff), so the fix belongs there whether or not it's unswallowed here.

— Review by Rames D Jusso

[miguel-heygen]

Fresh stacked browser evidence (production workspace build + real Chromium):

• Direct rotationX: 10 persisted and reloaded beside a separate 2D rotation tween, confirming the exact-axis filter does not overblock unrelated 2D rotation.
• A helper-owned tween declared through addTween(target, vars = {...}) rejected an Inspector X edit with the Unroll guidance, issued no mutation request, restored the field, and left source unchanged.
• A forced GSAP-mutation 500 restored the Inspector and preview DOM and surfaced the shared error telemetry/toast path.

Full browser transcript is summarized on #2987.

[miga-heygen]

R2 — Consistent GSAP edit ownership (head ca5befb)

Verdict: Approve

All R1 findings resolved plus the cross-PR dependency concern:

1. API asymmetry fixed — All three intercepts (drag, resize, rotation) now return GsapEditOutcome consistently. Tests use .resolves.toEqual({status: "blocked"}) instead of .rejects.toMatchObject(). The GsapEditAction type is removed — simplified COPY map.

2. Shared ownership check — directEditOutcomeForProperties(animations, properties) is the single gate for ownership verification, used by drag, resize, rotation, and inspector commits. animationWritesAnyProperty is the shared property matcher. assertGsapAnimationDirectlyEditable is now private (not exported). Clean SSOT.

3. Double-fetch fixed — Resize calls fetchFallbackAnimations once, result reused in allKnownAnimations. postSplitFetch only passed to resolveGroupTween when a legacy mixed tween might require re-read after split. Test added: "reuses the ownership parse instead of fetching a resolved size group twice" with expect(fetchAnimations).toHaveBeenCalledTimes(1).

4. Merge-order dependency resolved — With rotation/resize returning outcome (not throwing), the bare-catch issue is no longer the primary concern. directEditOutcomeForProperties catches GsapEditBlockedError internally and returns outcome. The stack is safe to merge in any order now.

New test coverage for runtime size motion blocking and rotation 3D property isolation is strong.

No new findings.

---

Review by Miga

[james-russo-rames-d-jusso]

Reviewed at ca5befb5 — delta since 5ac984ff.

All seven R1 findings — including both BLOCKERs — addressed with real (non-tautological) test coverage.

BLOCKER A (inspector edit-path swallow) — resolved with defense-in-depth across the stack. The commitAnimatedPropertyRaw layer in #2987 now (a) narrows its outer catch to re-throw and (b) hoists assertGsapEditPersisted(directEditOutcomeForProperties(...)) to the top of commitAnimatedProperties before the try/routing. New test at useAnimatedPropertyCommit.test.tsx:63-101 uses the real hook with a helper-provenance anim + rotationX prop and asserts (a) commit rejects with GsapEditBlockedError/reason:'unroll-required', (b) mutations.length === 0. No swallow, no silent no-op.

BLOCKER B (group-drag preflight vacuous on transient fetch failure) — two-part fix:

1. New failOnFetchError option in useGsapAnimationFetchFallback.ts:82 throws "GSAP animation ownership could not be verified" when the parse endpoint fails — eliminating the vacuous [] case.
2. Group-drag now runs an explicit preflight loop proving editability of every member before the first mutation, caching parsed animations in a preflightAnimations Map. The write loop reads from that cache instead of passing bare [].

Tests at useGsapAwareEditing.test.tsx:236-282 ("preflights every group member before the first mutation") and :284-302 ("fails a group preflight closed when ownership cannot be fetched") prove commit is never called on block / fetch-fail.

Five concerns also landed:

• Resize / rotation live-runtime fallback — resize and rotation intercepts now run directEditOutcomeForProperties over allKnownAnimations for their respective property groups AND fall back to hasNonHoldTweenForElement to block when a live runtime tween exists but no editable source. Dedicated tests for each intercept prove both branches.
• Rotation filter (rotationX/Y/Z + skew leak) — new ROTATION_CHANNELS = ['rotation', 'rotationX', 'rotationY', 'rotationZ'] set; filter uses animationWritesAnyProperty(anim, ROTATION_CHANNEL_SET). Both R1 sub-findings covered: 3D-rotation blocks correctly (tests at gsapRuntimeBridge.test.ts:246-262), and an unrelated helper-authored skew tween no longer blocks 2D rotation (:264-284).
• API asymmetry — all three intercepts now return Promise<GsapEditOutcome>. Every call site consumes via assertGsapEditPersisted(outcome); ~25 test-site updates from .toBe(true) → .toEqual({ status: 'persisted' }).
• Dead scaffolding — GsapEditAction gone. The COPY registry has a real UX consumer: useGsapInteractionFailureTelemetry.ts:22-24 threads error.message into showToast, so each blocked reason surfaces a reason-specific toast. Test at useGsapInteractionFailureTelemetry.test.tsx:17-42 proves the unroll-required message reaches showToast.
• Tautological inspector-rejection test — the mocked-hook test is replaced by the real integration-style test at useAnimatedPropertyCommit.test.tsx:63-101 (see BLOCKER A). useGsapAwareEditing.test.tsx still mocks its dependency, but that layer no longer claims to test inspector-rejection semantics.

Additional hardening spotted during the walk-through: commitStaticGsap{Position,Rotation,Size} and materializeIfDynamic now throw GsapEditBlockedError instead of silently returning; every catch in useGsapAwareEditing.ts tracks-and-rethrows.

46 focused tests + typecheck/lint green. Load-bearing PR of the stack, delivered clean. Looks good from my side, leaving as COMMENTED.

— Review by Rames D Jusso

[jrusso1020]

Approving to satisfy require_last_push_approval at ca5befb5.

What I verified at this head, rather than re-reviewing the diff: all eight required contexts are terminal-green (Semantic PR title, Test: runtime contract, Typecheck, Build, regression, Test, Render on windows-latest, Tests on windows-latest), there is no outstanding change request, and the follow-up passes from both requested reviewers land at this exact SHA with both earlier blockers recorded as closed.

Two notes for whoever runs the merge:

1. The earlier approval was never dismissed. dismiss_stale_reviews_on_push is false here, so it still reads APPROVED; it just predates the latest push, which is what require_last_push_approval actually gates on.
2. Expect this approval to disappear when #2985 lands. This PR is based on fix/gsap-helper-defaults, so merging #2985 retargets it onto main, and a base-branch change is the one thing that dismisses approvals on this branch (dismiss_stale_reviews_on_push: false, but base changes still dismiss). That will need a fresh approval at the retargeted head before this can merge. It is expected behavior, not a failed gate.

[vanceingalls]

R2 APPROVE at ca5befb5. All 4 findings from R1 addressed at the correct boundary.

• F1 (commitAnimatedProperties swallowing GsapEditBlockedError) — FIXED. useAnimatedPropertyCommit.ts:543-546 now catch (error) { bumpGsapCache(); throw error; }; useGsapAwareEditing.ts:356-360 wraps with trackGsapInteractionFailure + rethrow. Upstream fail-fast via assertGsapEditPersisted(directEditOutcomeForProperties(...)).
• F2 (double fetchFallbackAnimations per drag) — FIXED. preflightPassed flag lets group-drag caller reuse the preflight fetch. Resize/rotate use postSplitFetch = workingAnimations.some((a) => !a.propertyGroup) ? fetchFallbackAnimations : undefined with an explicit "would perform the same network read twice" comment.
• F3 (API asymmetry: drag returns outcome, rotate/resize throw) — FIXED. tryGsapResizeIntercept and tryGsapRotationIntercept now both return Promise<GsapEditOutcome>; all three callers converge on assertGsapEditPersisted(outcome) in useGsapAwareEditing.ts.
• F4 (group preflight animations: [] sentinel) — FIXED. Group preflight loop now fetches real animations via makeFetchFallback(selection, { failOnFetchError: true }), caches per-selection, and passes them to both preflight and commit. WHY comment at useGsapAwareEditing.ts:165-168 documents "Editability is user-atomic."

No new findings. Standards: CI green (Fallow SUCCESS, Typecheck SUCCESS, Lint SUCCESS, Format SUCCESS). No Golden Rule violations touched.

— Review by Via