Skip to content

fix(media-use): use a host app's music tools; fail loudly without the heygen CLI - #5154

Queued
miguel-heygen wants to merge 14 commits into
mainfrom
fix/media-use-host-music
Queued

miguel-heygen wants to merge 14 commits into
mainfrom
fix/media-use-host-music

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

Agents running inside a host app that has its own music and sound-effect tools were picking media-use resolve --type bgm instead, because the skill called itself "the single skill for every media need". Without the heygen CLI that path fails, and its JSON answer read like "no music": {"ok":false,"error":"no provider could resolve bgm: ..."}. The real reason only went to stderr.

Two ends:

  1. The skill text.
    • skills/media-use/SKILL.md no longer calls itself the single skill for every media need. Its description and body now say: when the host app provides its own music or sound-effect tools, use those; resolve --type bgm|sfx needs the heygen CLI.
    • The bgm and sfx rows name the CLI they search through.
    • references/setup-providers.md and the catalog's compressed copies in CLAUDE.md, README.md and docs/prompting/overview.mdx carry the same clause.
    • The workflow skills that source music (product-launch-video, pr-to-video, faceless-explainer, music-to-video, general-video, and the router's capability menu) carry it too, one line each.
  2. resolve fails loudly. When a resolve misses after the heygen CLI was missing or too old, the error is one line naming what is missing and the alternative, and the JSON carries a code and the fix:
{"ok":false,"code":"heygen_cli_missing","fix":"Install the CLI from https://developers.heygen.com/cli, then run: heygen auth login --oauth","error":"bgm needs the heygen CLI, which is not installed: use your host app's own music tool if it has one, or install the CLI."}

An outdated CLI gives heygen_cli_outdated with heygen update. Only music and sound effects get this message, where a host app's own tool is the known alternative; every other type keeps today's generic miss. The remediation was already recorded (consumeHeygenRemediation), but only a successful resolve read it; the miss path now reads it too. sfx still answers from its bundled library first, so this only shows when nothing bundled matches.

  1. The narrated workflows keep a host app's audio. product-launch-video, pr-to-video and faceless-explainer rebuild the audio_meta.json file from storyboard cues on every audio pass, which replaced or dropped anything a host app made. Now:
    • A host entry is listed with "source": "host": music as bgm, a sound as an sfx entry with its frame and length. Both audio passes (generate and fetch-sfx) keep it as long as its file exists, and a frame with a host sound gets no looked-up cue. A pass reads the host's entries again at its final write, so host audio added while the engine runs is kept too, and a host sound wins its frame over a looked-up one. The rule lives in one module, skills/media-use/audio/scripts/lib/host-audio.mjs, copied into the three workflows by scripts/generate-skill-module-copies.mjs like bgm-volume.mjs. A frame written as "1" counts as frame 1; an audio_meta.json that does not parse stops the pass instead of losing the host's entries; a host entry whose file is gone, or that has no path, is dropped with a warning saying why.
    • Silent now means no narration, no music, no sfx: cues and no host audio. A film with no narration and music: none but with sound cues or host audio is no longer treated as silent: the generate pass writes the audio_meta.json file instead of deleting it, so fetch-sfx fills it, and running generate again there keeps the sounds fetch-sfx already found. The SKILL.md marker, its gate and each references/story-design.md say the same.
  2. Follow-ups from review: the media-use first-run setup asks for the heygen CLI only when HeyGen media is used; a CLI so old it rejects --headers without printing a version is now reported as outdated.

What I measured

On Linux:

  • With no heygen on PATH, as CI runs them, node --test over the five affected test files reports 67 pass, 0 fail, 3 runs in a row; inside it resolve.test.mjs passes 61 of 61. Three new resolve tests (music without the CLI, a sound effect nothing bundled matches, music with an outdated CLI) fail on main's code and pass with the change. The sound-effect test serves the local media index from a local server that answers 404, so it never reaches the network.
  • Mutation: letting every type through heygenMiss fails the new heygen-cli test.
  • Host audio: four new behaviour tests in each of product-launch-video's and faceless-explainer's audio.test.mjs (faceless-explainer's copy is pinned byte-identical to pr-to-video's) fail on main's audio.mjs and pass with the change: fetch-sfx keeps a host sound and looks up no cue for its frame; fetch-sfx keeps a host bed; no narration + music: none + a sound cue is not silent; generate keeps a host bed. host-audio.test.mjs pins the rule, and a text test pins the host contract and the meaning of silent in all three workflows (reverting one skill's gate line fails it).
  • The rejected---headers test fails without its new clause.
  • Two more tests per workflow test file write host music and a host sound into the audio_meta.json file while the stub engine runs; both fail on the previous head (the passes wrote a snapshot taken before the engine) and pass now.
  • A host entry with no path is reported instead of thrown; its unit test fails on the previous module.
  • oxlint . over the whole repo exits 0.
  • Five more host-audio test runs (a broken audio_meta.json stops fetch-sfx, and a generate re-run keeps the looked-up sounds, in each of the two workflow test files; a string frame in host-audio.test.mjs) fail on the previous head's code and pass now.
  • Run the way CI runs them, with no heygen on PATH: both files pass 3 runs in a row; with a real heygen present they pass too. The existing "--json returns error JSON on stub provider failure" test used whatever heygen the machine had, so it now gets a fake one that finds nothing.
  • skills-manifest.json is regenerated after the last skill edit.
  • A flaky test on the way: the telemetry capture in resolve.test.mjs ran the CLI with a blocking spawn, which froze the test's own local telemetry server until the child gave up on its 1.5 s telemetry timeout, so the event arrived or not by timing. It now spawns asynchronously and the polling loop is gone. It failed once in 13 runs before; after the change the file passed 13 runs in a row, and the capture now also fails if the resolve run exits non-zero.
  • lint-skills (33 skill files), the skill mirror check, check-skill-import-boundaries and lint-skills tests (46 pass), formatting and the comment checks pass.
  • By hand, with no heygen on PATH: --type bgm and --type sfx --intent "dog barking" print the line above and exit 1; --type sfx --intent "whoosh" still resolves from the bundled library.

What I did NOT exercise

  • A real agent run inside a host app, to see it now pick the host's music tool, or a narrated workflow assembled end to end with host audio (the audio passes are tested; assembly reads the same entries). That is behaviour of the agent reading the new text, not something a unit test can show.
  • The unauthenticated case (heygen installed but not signed in): it is not recorded as a remediation today, so it still reports the generic miss.
  • macOS and Windows.
  • With no network at all, a sound-effect miss still fails earlier, on the local media index (local SFX index unavailable), before this message. That predates this change.

CI status

The Studio timeline viewport gate is red. Its overscan regression is owned by #5151, which is open; this PR does not change the viewport implementation. Other checks are still completing.

@mintlify

mintlify Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
hyperframes 🟢 Ready View Preview Oct 7, 2026, 4:34 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 7, 2026 04:50

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 1355c7c6dd09433d3aa3199efeba13abcb349b28 — requesting changes on the host-audio workflow contract. The new CLI diagnostics pass their focused tests; the concerns below are with the instructions that consume host audio.

  1. Host-created SFX are replaced or dropped by the prescribed fetch-sfx pass. The new host-first direction in skills/product-launch-video/SKILL.md:10 (also pr-to-video and faceless-explainer) has no sound-effect handoff corresponding to the new BGM handoff at :131. Step 5 still runs audio.mjs fetch-sfx (:175), which derives names from storyboard cues, resolves them against HeyGen/bundled sounds, then rewrites the entire audio_meta.json from its neutral sidecar (scripts/audio.mjs:200-223). I reproduced a host-provided assets/sfx/custom-whoosh.mp3 entry already in audio_meta.json being replaced with the bundled assets/sfx/whoosh.mp3 by that pass; a custom cue without a library match is skipped. Assembly reads only audio_meta.json.sfx, so the host-generated audio never reaches the film. Please give host SFX an explicit, stable path into the final metadata after the rewriting pass (or make the pass preserve/adopt them), and cover that flow.

  2. The no-script music: none path still tells the agent to skip SFX. The new Step 3.1 text at skills/product-launch-video/SKILL.md:131 says that when a host supplies BGM, use music: none, and even without SCRIPT.md run fetch-sfx for any frame cues. Four lines later (:135), the same combination is declared “fully silent” with “no SFX”; Step 5 (:171) says to skip both duration sync and fetch-sfx when silent. In a non-narrated host-music video whose storyboard has sfx: whoosh, following that Step 5 gate skips the only pass that writes SFX entries; the later manual BGM entry does not restore those sounds. The copied workflow text in pr-to-video and faceless-explainer, plus their references/story-design.md silent instructions, need the same reconciliation. The audio.mjs generate shortcut for this marker (:144-159) makes the contradictory interpretation particularly likely.

Nonblocking follow-ups: skills/media-use/SKILL.md:12 and references/setup-providers.md:20-24 still require HeyGen CLI installation/sign-in on first run even if the only requested audio comes from a host app; condition setup on using HeyGen. An older CLI emitting unknown flag: --headers without a version is still classified as other and returns a generic miss (base and head behave identically), so the new outdated-CLI remedy covers only errors that include a parseable version. Please add concept-level positive-pin tests for the changed agent-facing host audio and no-script/SFX instructions; the new tests only pin CLI error messages, not this workflow.

Focused local tests: resolve.test.mjs 61/61 and heygen-cli.test.mjs 23/23. Required CI checks passed on this head. No approval or merge performed.

— tai

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Thanks, both points were real. Fixed at 5a45911:

  1. Host sound effects survive fetch-sfx. The rule lives in one module, skills/media-use/audio/scripts/lib/host-audio.mjs, copied into the three workflows by scripts/generate-skill-module-copies.mjs.
    • An audio_meta.json entry marked "source": "host" whose file exists is kept by both audio passes. That covers a bed in bgm and a sound in sfx.
    • A frame with a host sound gets no looked-up cue. So your assets/sfx/custom-whoosh.mp3 stays, and the bundled whoosh is not added.
    • A broken audio_meta.json stops the pass instead of losing the host entries.
    • Tests in both workflow audio.test.mjs files fail on the old code and pass now.
  2. Silent now means no narration, no music, no sfx: cues and no host audio. That holds in audio.mjs and in each SKILL.md marker, its gate, and references/story-design.md.
    • Without narration but with cues or host audio, generate writes audio_meta.json for fetch-sfx to fill, instead of deleting it.
    • A text test pins this wording and the host contract in all three workflows. Reverting one gate line fails it.

Non-blocking items, also done:

  • The media-use first-run setup asks for the heygen CLI only when HeyGen media is used.
  • A CLI that rejects --headers without printing a version is reported as outdated, with a test.

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 5a45911163080e1774e54b2d00932872433109ca — requesting changes. The two issues from my previous review are fixed: host BGM/SFX already present before a pass survive generation and fetch-sfx, and a no-script project with SFX/host audio no longer takes the fully-silent path. Focused tests and real assembly exercises passed.

Preserve host audio added while a background audio pass is running. In skills/product-launch-video/scripts/audio.mjs:153,198-201 (and the corresponding narrated adapters), runGenerate reads host entries before running the synchronous audio engine, then writes the rebuilt metadata using that old snapshot. runFetchSfx does the same at :251,259-262. The SKILL.md launches generation in the background and says every pass keeps host entries. I reproduced both paths at this head: start the engine, add valid host BGM and SFX entries/files to audio_meta.json while it runs, then let it finish. Both commands exit 0 yet leave { "bgm": null, "sfx": [] } without warning. Re-read/merge host entries at the final write (with an appropriate conflict rule), or otherwise establish and enforce sequencing that prevents a host write during the pass. A pre-pass-only snapshot is not sufficient for the documented guarantee.

Regenerate the skills manifest after the final skill edits. The Skills: manifest in sync check fails at this head: faceless-explainer is committed as 36f434caf058f16e but computes 65d08c8732942516; product-launch-video is committed as 92a85e48bb7d95c2 but computes c767ff7f62a78a54. The base passes the check, and each discrepancy is explained by a one-line test edit made after the manifest was generated. Please rerun bun run --cwd packages/cli gen:skills-manifest and commit the result. Required checks pass; this separate check is still red.

The host-audio race is the substantive remaining finding. No separate objection to the CLI compatibility behavior or mixed-source SFX cue policy was verified.

— tai

@miguel-heygen

miguel-heygen commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

Thanks, both real. Fixed at b2f1098:

  1. Host audio added mid-pass is kept. Each pass reads the host entries again at its final write (generate's no-narration path, generate after the engine, and fetch-sfx after the engine). The earlier read stays only to skip cue lookups for frames the host already scored. If a host sound and a looked-up cue land on the same frame, the host sound wins. In both workflow test files, two new tests have the stub engine write host music and a host sound into audio_meta.json while it runs. Both fail on 5a45911 and pass now.
  2. The manifest is regenerated after the last skill edit, and that is now the last step before each push.

Also in this head: a flaky telemetry test in resolve.test.mjs. It ran the CLI with a blocking spawn, so the test's own local telemetry server couldn't answer until the child had given up. It now spawns asynchronously.

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at b2f10984ed5bbbb80f33c2d4ff78b02930781c9b — approved.

The two previous host-audio findings are resolved. Both narrated generate and fetch-sfx re-read host metadata after the engine pass, so entries added during an in-flight pass survive; the earlier deterministic race reproduction now retains both host BGM and SFX. The no-script/SFX and sequential assembly paths also work, and the skills manifest now passes its generator check. Focused audio/helper tests passed 70/70 and the media resolver suite 61/61 (one optional core-conformance assertion self-skipped in a disposable checkout lacking tsx). The skill copies remain in sync.

Nonblocking follow-up: in the fully silent branch (music: none, no script or cues), if audio_meta.json names a host file that has disappeared, the initial host scan suppresses its warning; scored is false and the branch deletes metadata while printing only "project marked silent." The previous head warned that the host file was missing. Valid host files are preserved; please restore the missing-file warning before that early cleanup so the operator does not mistake a broken host path for an intentional silent film.

— tai

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 2059 (base branch 2059), smooth 1460 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

  • crop-none-px-r30-root-z100: tracking 0.04, pressJump 0, drop 40.08, reload 40.08, render 34.74, renderKey -, undo true, teleport true / tracking 0.04, pressJump 0, drop 0.08, reload 0.08, render 0.26, renderKey -, undo true, teleport true / tracking 0.04, pressJump 0, drop 0.08, reload 0.08, render 0.26, renderKey -, undo true, teleport true

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 7, 2026
Any commits made after this event will not be merged.

This branch was successfully deployed

1 active (outdated) deployment
staging - docs — b1102286 Deployed Oct 7, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants