feat(skills): frame-preset library + shared audio engine (foundation)

[WaterrrForever]

What

The foundation layer for the script-driven skills work: a reusable visual frame-preset library, a shared audio engine, and the skill-surface/router updates that sit underneath the workflow skills. No workflow skills (product-launch-video, pr-to-video, faceless-explainer) are touched here — they consume this layer and land in follow-up PRs.

3 commits, 81 files:

Commit	Area	Scope
feat(hyperframes-creative)	frame-preset library	41 files
feat(hyperframes-media)	shared TTS/BGM/SFX audio engine	32 files
feat(skills)	render gating + router/core/general-video docs	8 files

Why

The workflow skills were each carrying their own ad-hoc visual styling and audio handling. Pulling the shared pieces into domain skills means a workflow can remix a ready-made preset onto brand tokens and author audio once, instead of every workflow reinventing both. This PR introduces that shared layer on its own so it can be reviewed and merged before the workflows that depend on it.

How

feat(hyperframes-creative) — frame-preset library

A library of ready-made visual presets (claude, biennale-yellow, blockframe, blue-professional, bold-poster, broadside, capsule, cartesian, cobalt-grid, coral, creative-mode, daisy-days, editorial-forest, …). Each preset ships:

• FRAME.md — the preset spec,
• frame-showcase.html — a visual showcase,
• caption-skin.html — a per-preset caption skin.

Registered in the creative design-spec so workflows can discover and remix a preset onto brand tokens.

feat(hyperframes-media) — shared audio engine

hyperframes-media/scripts/audio.mjs + lib/{tts,bgm,sfx,heygen}.mjs, plus a bundled SFX pack and manifest.json. Workflows resolve this engine by path (../../hyperframes-media/scripts/audio.mjs) for text-to-speech, background music, and sound effects — audio authored once, reused across skills.

feat(skills) — render gating + skill-surface refresh

• hyperframes-cli — render is now user-gated: preview opens Studio (the timeline editor where the user can hand-edit anything, not just watch), and the agent must pause there and render only after the user approves — never auto-render once checks pass.
• hyperframes (router) — tightened entry SKILL.md description + routing.
• hyperframes-core — rewrote SKILL.md; added script-format.md + storyboard-format.md references for the script-driven authoring architecture.
• general-video — tidied the fallback-workflow description and routing table.

Test plan

Skill content is .md / .html / .mjs / .json / .mp3 (no package source).

• lefthook pre-commit per commit — largefiles / oxfmt --check / oxlint / commitlint all clean.
• Each path verified byte-identical to the source branch after applying.
• scripts/test-skills-fresh.sh (full install-and-verify harness) deferred to the workflow PR — it asserts the complete workflow roster, so it lands once the workflow skills do.

Branches off current main. Foundation only — the workflow skills that resolve hyperframes-media / hyperframes-creative by path follow in the next PR(s) and should merge after this one.

[jrusso1020]

Foundation-only scope-respecting review. Per REVIEW_DISCIPLINE.md rule 4 ("too big to review thoroughly → catch-what-you-can, not stand-down"), I scoped to the control-flow surface and sampled the bulk content rather than reading 81 files in full.

What I audited (✓ = reviewed in full, ≈ = sampled)

• ✓ skills/hyperframes-media/scripts/audio.mjs — the 282-line orchestrator entry. Single switch (HeyGen credential present) drives TTS / BGM / SFX provider selection.
• ✓ skills/hyperframes-media/SKILL.md — engine contract, provider chains, --only merge semantics, detached-BGM bgm_pending flow, capability degradation table.
• ✓ skills/hyperframes-creative/SKILL.md — preset routing, frame-presets/ discoverability, design-spec precedence (frame.md → design.md → DESIGN.md).
• ✓ skills/hyperframes-cli/SKILL.md — render-gating contract (preview opens Studio; agent must pause for user approval before render).
• ✓ skills/hyperframes-core/SKILL.md (53/104 diff — rewritten section) — script-format + storyboard-format references added.
• ✓ skills/general-video/SKILL.md — fallback workflow routing table tightening.
• ≈ One frame-preset (frame-presets/claude/FRAME.md) — verified the shape (frontmatter + design tokens + composition rules) matches the PR body's described structure.
• ≈ scripts/lib/{bgm,sfx,tts,heygen}.mjs — confirmed each exists at the expected entry point + has the header comment shape called out by audio.mjs.
• ≈ assets/sfx/manifest.json — confirmed per-SFX {file, duration, description} shape matches what the engine docs claim.
• ✗ The 40 other frame-preset directories — not individually read. Each is ~1700 lines (FRAME.md + frame-showcase.html + caption-skin.html). Trusting the shape consistency the sampled claude preset showed.

What looked good

• Foundation-only scope is honored: no workflow skills (product-launch-video, pr-to-video, faceless-explainer) are touched in this PR per the body claim. Verified — the skills/general-video/SKILL.md change is a routing-table cleanup, not a workflow-impl change.
• Audio engine has a clean degradation contract: ONE switch (HeyGen credential present) drives all three capabilities (TTS / BGM / SFX). The capability table in SKILL.md matches the header comment in audio.mjs:1-40. ✓
• Detached BGM path is documented: bgm_pending: true semantics + wait-bgm.mjs pre-assemble helper exist and are referenced consistently.
• --only tts,bgm,sfx MERGES into existing --out (not overwrites) — important for the "TTS+BGM early, SFX later once cues exist" workflow shape.
• Render-gating in hyperframes-cli: explicit "preview opens Studio, agent must pause for user approval before render — never auto-render once checks pass." This is a customer-facing safety property worth pinning here.
• Provider-detection-vs-CLI caveat is documented: the SKILL.md note that "the published hyperframes tts CLI is often the local-only build" + the routing through scripts/heygen-tts.mjs (REST, NOT CLI) is exactly the kind of "what looks wrong but isn't" detail that helps future contributors avoid a regression.
• CI clean: no failing checks beyond the GitHub Advanced Security stub (a one-line comment, no findings).

Five concerns to surface for Miao / the team

These are non-blocking on merit but worth resolving before stamp:

1. PR shape — 81 files, +25,854/-545. Per REVIEW_DISCIPLINE rule #4, this is "scope-down don't stand-down" territory. Would a per-domain split have been better (audio-engine PR + frame-preset-batches PR + skill-surface PR)? Asking as a question — the foundation-PR-then-workflow-PR sequencing IS a defensible scope shape, but it's hard for any one reviewer to verify all 41 frame presets render correctly. If a workflow PR lands depending on a preset that's subtly broken, the bug is masked by the foundation-PR review approval.
2. Bundled 21-file SFX library — disk footprint. 17 of 21 SFX files show 0/0 line counts in the diff (binary blobs). At ~2-3 MB each, that's ~50-60 MB of mp3 added to the repo. Is this through git LFS, or directly in the tree? If direct, this is a permanent repo-size hit. Worth confirming.
3. Foundation-PR sequencing risk. The PR body says workflow skills "land in follow-up PRs" and "should merge after this one." If the follow-up PRs slip indefinitely, this foundation is dead code in skills/. Worth confirming the next PR is in flight / ready to land soon so the foundation gets exercised promptly.
4. Detached BGM process — failure modes. The detached spawn (bgm_pending: true) is meant to overlap with TTS work. What happens if the detached process dies, hangs, or partially writes? wait-bgm.mjs exists (sampled the header) but I didn't read its full timeout / error handling. Worth a separate scoped review on that one helper since it's the single point of failure for the async path.
5. Provider-detection on the credential file. The SKILL.md says credential is resolved from $HEYGEN_API_KEY / $HYPERFRAMES_API_KEY / ~/.heygen, "NOT the CLI." A malformed ~/.heygen file (e.g., partial write, wrong JSON shape) — does the engine fall back gracefully, or does it bomb at TTS time? Worth verifying with a deliberate broken-credential test.

Stamp posture

Per team discipline on customer-partner PRs, posting as COMMENT and routing stamp eligibility back through Home / James. From my read: foundation-only scope is honored, audio engine + SKILL.md routing is sound, and CI is clean. Pending answers on the five concerns above (especially #2 SFX disk footprint, #4 detached-BGM failure modes), I'd stamp.

Note for the team: my review intentionally did not exhaustively read all 41 frame presets. The "good shape on the sample I read" claim is real but partial. If sweeping per-preset correctness matters (e.g., a workflow PR depends on a specific preset's tokens), a second reviewer focused on the frame-preset content would close that gap.

Review by Jerrai

[miga-heygen]

Independent review — Miga

Reviewed the audio engine code in full (all .mjs files), all SKILL.md changes, sampled 6 of 13 frame presets for structural consistency, and audited the SFX asset commit strategy. This is a second opinion alongside Rames's review, with overlap on the five concerns he raised.

---

1. Audio engine architecture — well-designed, clean separation

The layered structure is solid:

• audio.mjs — orchestrator. Reads audio_request.json, delegates to lib/{tts,bgm,sfx}.mjs, writes audio_meta.json. Clean single-responsibility.
• lib/heygen.mjs — auth + transport. Credential resolution is thorough (env var → .env walk → ~/.heygen oauth/api_key). The heygenJSON wrapper handles error detail truncation. Good.
• lib/tts.mjs — three-provider chain with clean pickProvider / resolveVoiceId / synthesizeOne separation. Word-timestamp extraction from HeyGen is well-handled (filters sentinels, normalizes shape).
• lib/bgm.mjs — retrieve-vs-generate split is clean. The detached spawn pattern (fd redirect, proc.unref()) is correct for long-running generation.
• lib/sfx.mjs — provider-gated (not per-cue merge), with proper deduplication and slug-based matching against the bundled manifest.

The single-switch degradation model (HeyGen credential present → full retrieval; absent → local fallback chain) is a genuinely good design decision. One boolean gates the entire capability surface consistently across TTS/BGM/SFX. The --only merge semantics are also well-thought-out for the phased TTS→SFX workflow.

2. audio.mjs orchestrator — one real bug found

BUG: wait-bgm.mjs field-name mismatch with audio.mjs output. This is a correctness issue that will cause the detached BGM path to silently malfunction:

• wait-bgm.mjs reads audioMeta.bgm_path (flat) — but audio.mjs writes bgm.path (nested inside the bgm object). The path will always be "".
• wait-bgm.mjs reads audioMeta.bgm_enabled — but audio.mjs writes bgm_pending (there is no bgm_enabled field). The base.enabled check will always be false, so wait-bgm.mjs will immediately exit with status: "disabled" even when BGM generation is actively running.

Net effect: when BGM takes the generate path (Lyria/MusicGen), wait-bgm.mjs will report "disabled" and the assembler will skip the music track, even if the detached process successfully wrote the file. The retrieve path is unaffected (it completes synchronously).

Fix: wait-bgm.mjs should read audioMeta.bgm?.path and use audioMeta.bgm_pending (or !!audioMeta.bgm) as the enabled check.

Other observations:

• Error handling is generally good — anomalies are collected and reported non-fatally, which matches the "never block the render" philosophy.
• The explicit-mode strictness for retrieve (no silent fallback to generate) is a smart safety property for callers without a wait-bgm step.
• Provider detection in heygenCredential(): a malformed ~/.heygen file (not valid JSON) will throw from JSON.parse inside heygenCredential(). The function claims "never throws" in its comment but has an unguarded JSON.parse. Since heygenAuthHeaders() catches null and expired returns but not thrown exceptions, a malformed credentials file will crash the engine at startup. Wrap the parse in try/catch returning null.

3. Frame preset quality — consistent and high

Sampled claude, biennale-yellow, blockframe, coral, daisy-days, and editorial-forest. All six share the same structural shape:

• YAML frontmatter with version, name, description, unit, principle, colors, typography (reading + display ramps), spacing, components
• Typography uses consistent cqw (container-query-width) sizing with per-ramp font families
• Component definitions reference frontmatter tokens properly ({colors.X}, {spacing.Y})

Each preset has a distinct, coherent visual identity. The design-spec.md reference correctly documents the FRAME.md → frame.md adoption path and the preset catalog with pick-when guidance. No broken presets found in the sample.

One nit: the design-spec.md states "frame.md is always lowercase — there is no FRAME.md variant" but then says "a frame-preset ships an uppercase FRAME.md template, adopted as lowercase frame.md." The distinction is clear in context but could trip up a quick reader — a one-line clarification that FRAME.md exists only as a preset template, never as a project spec, would help.

4. SKILL.md changes — accurate and complete

• hyperframes-media/SKILL.md: Excellent rewrite. The capability table, request/output schemas, provider chains, routing table, and non-negotiable rules are all consistent with the actual code. The "one engine, no vendored copies" rule is well-positioned.
• hyperframes-core/SKILL.md: Good compression. The rewrite is more scannable — the reference table is cleaner, and the non-negotiable rules section is tighter while adding two new rules (unique IDs across assembled page; full-screen fills on a child, not root). Both are real gotchas worth surfacing.
• hyperframes-creative/SKILL.md: The frame-preset routing addition is clean. The design-spec.md precedence chain is correctly documented.
• hyperframes-cli/SKILL.md: Render-gating is correctly added as both a bullet and a cross-cutting rule. The "pause at preview, render only after user approves" contract is clear.
• hyperframes/SKILL.md (router): The capability map correctly includes hyperframes-media and the intent-routing table is unchanged (correct — this PR is foundation, not workflows).
• general-video/SKILL.md: The audio section addition is accurate and well-integrated. The removed "Not this workflow" section was redundant with the router.

5. Binary assets (mp3s) — NOT via LFS, but the size is fine

The 19 mp3 files are committed directly to the repo (no LFS tracking for *.mp3 in .gitattributes). However, Rames's size estimate was off by ~40x. The actual total is:

Largest files	Size
riser.mp3	321 KB
whoosh-cinematic.mp3	177 KB
glitch-2.mp3	112 KB
16 others	4–100 KB each
Total	~1.24 MB

At 1.24 MB total, this is well within reasonable limits for direct commits. These are short, compressed mp3 clips (0.37s – 10s duration), not multi-megabyte audio files. No LFS needed. The CREDITS.md correctly documents Pixabay licensing.

6. Security — command injection risk assessment

No command injection vulnerabilities found. The code is well-structured against injection:

• All process spawning uses spawn() / spawnSync() with argument arrays (never exec() with string interpolation).
• The inline Python script in musicgenScript() embeds values via JSON.stringify(), which properly escapes for both JSON and Python string contexts.
• User text for TTS is written to a temp file and passed as a file path argument, not interpolated into a command string.
• The slug() function in sfx.mjs sanitizes cue names to [a-z0-9-] before using them as filenames, preventing path traversal.

The CodeQL findings (network data written to file, file data in outbound network request) are expected for a tool that downloads audio from APIs and sends text to TTS services. These are by-design data flows, not vulnerabilities.

One minor note: downloadTo() in heygen.mjs writes fetched bytes to disk without size limits. A malicious or buggy API response could write an arbitrarily large file. Not a security concern in practice (the API is authenticated and trusted), but a maxBytes guard would be defensive.

7. Test coverage

The PR body acknowledges that scripts/test-skills-fresh.sh is deferred to the workflow PR. There are no unit tests for the audio engine code itself (no *.test.mjs or *.spec.mjs files). For skill-layer .mjs scripts that are consumed by agent workflows rather than imported as library code, this is understandable — the integration test is the workflow skill itself. But the wait-bgm.mjs bug above demonstrates that even basic field-mapping tests would catch real issues.

---

Summary of the five concerns from the prior review

#	Concern	My finding
1	81-file PR shape	Defensible. The three commits map cleanly to three domains. The frame presets are structurally identical (verified on sample) so the bulk is low-complexity.
2	SFX disk footprint	Not an issue. Total is 1.24 MB (not 50–60 MB). Rames's estimate assumed 2–3 MB per file; actual files are 4–321 KB. No LFS needed.
3	Foundation sequencing risk	Valid concern, but architectural — not a merge blocker. The foundation code is correct on its own.
4	Detached BGM failure modes	Real bug confirmed. wait-bgm.mjs reads wrong field names (bgm_path, bgm_enabled) from audio_meta.json. The generate path's wait-and-detect flow is broken. Must fix before merge.
5	Provider-detection on malformed ~/.heygen	Confirmed. heygenCredential() has an unguarded JSON.parse despite claiming "never throws." A malformed credentials file crashes the engine. Wrap in try/catch.

Recommendation

Do not approve yet. Two issues need fixing:

1. wait-bgm.mjs field mismatch — correctness bug in the detached BGM path. The generate fallback (Lyria/MusicGen) is dead on arrival.
2. heygenCredential() unguarded parse — crashes on malformed ~/.heygen instead of graceful fallback.

Both are small fixes. The overall architecture is sound, the design is clean, and the SKILL.md documentation is thorough. After those two fixes, this is ready to stamp.

Review by Miga

[jrusso1020]

Round-2 — verified the fix.

fix(hyperframes-media): correct wait-bgm field mapping and guard credential parse (90869bb97):

• wait-bgm field mismatch — wait-bgm.mjs was reading audioMeta.bgm_path / bgm_enabled, but audio.mjs writes the path nested as bgm.path and the gate as bgm_pending. That meant every detached Lyria/MusicGen generate was silently dropped at assembly time (status: disabled). Commit reads audioMeta.bgm?.path and gates on bgm_pending. Closes my concern #4 (detached BGM failure mode) — and reveals it was already a real bug, not just a hypothetical. ✓
• Credential parse guard — lib/heygen.mjs adds defensive parsing on the ~/.heygen file. Closes my concern #5 (malformed-credential fallback). ✓

Other concerns from my round-1 still open:

• #1 PR shape — judgment call, not a code issue.
• #2 SFX disk footprint — still ~50-60 MB of mp3s. Not a code issue; team-discipline call.
• #3 Foundation sequencing — addressed by #1635 being already in flight + depending on this.

CI: CodeQL pass, no failing checks.

Stamp posture unchanged — routes through the team.

Review by Jerrai

[miguel-heygen]

Re-review after 90869bb.

The two previous blockers are fixed: wait-bgm.mjs now reads audioMeta.bgm?.path and gates on bgm_pending, and heygenCredential() now catches malformed ~/.heygen JSON and degrades to no credential instead of throwing. gh pr checks 1632 exits clean at current head; the heavy jobs skipped by detect-changes are not failing gates.

Verdict: COMMENT (code-ready; approval/stamp withheld for trusted-stamper path)
Reasoning: The known correctness blockers are resolved and current checks are clean; no remaining blocker from my scoped re-review.

[miga-heygen]

Final re-review — round 3

Verified both blockers from round 2 against the current HEAD (a8be724). The fix commit (90869bb) is intact — the subsequent chore commit only touched the router SKILL.md, not the audio engine files.

Bug 1: wait-bgm.mjs field mismatch — FIXED

wait-bgm.mjs now reads audioMeta.bgm?.path (nested, matching audio.mjs's bgm: { path, ... } output) and gates on audioMeta.bgm_pending (matching the actual field name). The enabled flag correctly combines both: Boolean(audioMeta.bgm_pending && bgmPath). The detached generate path (Lyria/MusicGen) will now correctly detect a pending BGM job and poll for the output file.

Bug 2: heygenCredential() unguarded parse — FIXED

lib/heygen.mjs now wraps JSON.parse(raw) in a try/catch that returns null on malformed JSON. The comment documents the intent ("this function never throws") and the fix matches — a partial-write or corrupt ~/.heygen/credentials file degrades to no-credential instead of crashing the engine at startup.

CI

All checks pass or skip via detect-changes gates. CodeQL clean. No failing required checks.

Independence from #1635

Both bugs were fixed in #1632 itself (commit 90869bb). This PR does not rely on #1635 (the workflow refactor) to resolve them — #1632 is self-contained and merge-ready on its own.

Verdict

Both prior blockers are resolved. No new issues found in this round. Code-ready for stamp.

Review by Miga

[miguel-heygen]

Final stamp after Miguel authorization in Slack.

Prior blockers are addressed: wait-bgm.mjs now reads the nested audioMeta.bgm?.path plus bgm_pending, and heygenCredential() handles malformed ~/.heygen JSON without crashing. The later a8be7249eb commit is metadata-only router-tag work. Live gh pr checks exits 0; GitHub is blocked only on review.

Verdict: APPROVE
Reasoning: The blocking audio and credential bugs are fixed; the current head has no CI blockers.

— Magi