fix(skills): hold frame content through transitions

[miguel-heygen]

What

• extend outgoing frame roots and tail-covering visual clips through injected transition overlap
• keep audio windows unchanged
• apply the fix consistently to faceless-explainer, PR-to-video, and product-launch-video
• add integration coverage for normal and pre-inflated worker roots

Why

Transition injection extended only the assembled host wrapper. Worker-authored ground/content clips still ended at the original storyboard boundary, so non-final scenes could turn transparent during their own crossfade or push-slide exit.

How

When an outgoing wrapper gains overlap, the injector now rewrites its source composition to the same target duration and extends non-audio timed elements that reached the original synced boundary. Elements ending earlier remain unchanged; already-extended elements are not extended twice.

Test plan

• bun run --cwd packages/core test --run src/storyboard/transitionsContract.test.ts (3/3)
• pre-commit lint, format, skills-manifest, fallow, and typecheck hooks

[terencecho]

LGTM on the code; CI is red on Format and needs a prettier run before merge.

Cross-checks

• Sibling parity (this is a 3-injector change, so parity is load-bearing). Post-PR, all three transitions.mjs files converge to the same git blob (843c57b208). Faceless-explainer and pr-to-video were already byte-identical pre-PR; product-launch-video differed only in a header-comment (for product-launch. → for the video workflow.) and this PR normalizes that. No cross-helper divergence — the [[cross-helper-semantic-divergence]] footgun I check for on look-alike sibling PRs does not apply here.
• describe.each runs the same fixture against all 3 skills. Validates parity at runtime, not just at the diff level — good.
• extendFrameTail traced against the fixture. Root branch hits via composition-id short-circuit (foundRoot guard prevents double-extension); ground clip (end=2.0=base) extends to 2.5; content clip (start=0.5, end=2.0) extends to 2.0; audio skipped by name-check; r3 rounding matches the expected literal strings.
• "Not extended twice" property. end >= targetDuration - EPS correctly no-ops already-extended clips (the worker pre-inflation case). Idempotency holds.

Findings

• [blocker before merge] CI Format check red (47s). Downstream Preflight (lint+format ×4), player-perf, preview-regression, and regression all cascade-fail at 2–3s because format-gated preflight blocks them. Test, Typecheck, Test: skills, Skills-manifest, CLI smoke, and Build all pass — so this is purely a prettier --write run away.
• [nit] Doc-vs-test mismatch — PR body claims coverage of "normal AND pre-inflated worker roots" but the fixture only exercises the pre-inflated case (root=2.5, tail=2.0). Normal case (root=2.0) exercises the same code path so it's a wording gap, not a bug. Add a second fixture or reword.
• [nit] transitions.mjs:96-146 regex-parses HTML attributes. Fine given the controlled skill markup; fragile if a worker ever emits attributes with escaped quotes or nested composition-id references. Fast-follow to reuse the existing attribute parser used elsewhere.

Diff-scope: +259/-7, 5 files. Base 2aadf450 is a real master merge-base.

— Review by tai (pr-review)

[vanceingalls]

Verdict: 🟡 LGTM-conditional at 9686b98

Concurring with @terencecho — code is right-layer, and his sibling-parity check (all three transitions.mjs files converge to the same git blob post-PR) is the load-bearing invariant for a 3-injector change. Endorsing that observation.

Fix mechanism verified: runInject already extended the host wrapper via outgoing.duration = r3(...); extendFrameTail now propagates that same baseDuration → targetDuration window into the mounted frame's root + any non-audio timed element ending exactly at baseDuration. The end < baseDuration - EPS and end >= targetDuration - EPS guards correctly skip already-extended and short-ending clips. Audio elements are explicitly bypassed at line 142/224/313 — correct scope.

Test is a genuine characterization test — execFileSync invokes the real skill script against a fixture, asserting on-disk output. describe.each(skillNames) covers all three copies. No aspirational-test shape (per feedback_aspirational_vs_real_characterization_tests.md).

Ship-blocker to flag (also flagged by Terence): CI red on Format — needs a bun run format before merge.

Post-merge follow-up worth filing: the 60-line extendFrameTail helper is now duplicated verbatim across three skill scripts. Per-skill script convention accepts vendored copies, but a future bug fix in one now demands the fix be synced to the others. Factoring into a shared skills-utility module would eliminate silent-divergence risk. Not blocking; skills-convention question.

R1 by Via

[miguel-heygen]

@via @tai Addressed the actionable review items at 1727770:

• Ran oxfmt on the regression test (Format issue resolved).
• Expanded the real execFileSync characterization coverage with both pre-inflated and normal worker-root fixtures across all three transition injectors, matching the PR description.
• Regex parser remains intentionally scoped to controlled skill markup; no behavior change needed.

Targeted validation: bunx vitest run packages/core/src/storyboard/transitionsContract.test.ts (6/6 passed). Please re-review current head.

[miguel-heygen]

Follow-up at ac527e7: formatted all three vendored transition injectors (oxfmt) after inspecting the fresh preflight logs; this was the source of the cascading player/preview/regression failures. Fresh CI rerun is in progress.

[miguel-heygen]

Final follow-up at 8d76705: regenerated skills-manifest.json for the formatted transition scripts; this keeps the manifest check deterministic on the final head.