Skip to content

fix(media-use): a slow download no longer leaves an empty asset that undo brings back - #4989

Merged
miguel-heygen merged 4 commits into
mainfrom
fix/media-use-reserve-temp
Oct 4, 2026
Merged

miguel-heygen merged 4 commits into
mainfrom
fix/media-use-reserve-temp

Conversation

@miguel-heygen

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

Copy link
Copy Markdown
Collaborator

What

media-use no longer puts an empty file at an asset's final name while it downloads. The id is still reserved on disk, but now by an empty marker under an atomic temp name (<asset>.hfXXXXXX.tmp), which project history skips. The asset appears under its real name only when it is written, and the marker is removed when the reservation commits or rolls back.

Also: skills/media-use/SKILL.md now tells the agent to tell the person, before generating a voiceover or an avatar video, that signing in to the heygen CLI with OAuth (heygen auth login --oauth) gives a free allowance for TTS voiceover and avatar videos, while an API key bills API credits. This comes from references/setup-providers.md; no allowance amounts are stated.

Why

allocateId reserved an id by creating an empty file at the final path (.media/<type>/<id><ext>, flag wx) before the download. Studio's project history does not skip .media, and it commits outside changes after quietMs (2 s by default). When the download took longer, history committed the 0-byte file as an outside change; undoing past it later restored an empty asset.

Related work

Same shape as #4982 (freeze-frame stills), which moved the still's reservation to a temp name that history skips.

How

  • allocateId (media-use/lib/manifest.mjs): under the existing lock, creates the marker <final path>.hf<6 hex>.tmp with wx, the same suffix as core's atomicTempPath. Studio's projectSignature and the CLI file watcher skip it via isAtomicTempPath. nextFreeId still counts it (its name starts with the id), so a concurrent resolve cannot take the same id during the download.
  • withReservedFile / withReservedFileSync: a committed reservation removes only the marker; a failed or empty one removes the marker and any partial asset at the final path.
  • The writers are unchanged: freezeUrl buffers the whole body and writes once, cache imports copy a complete file, and the LUT paths already rename a validated file into place.
  • media-use's scripts also ship standalone in skills/media-use and cannot import @hyperframes/core, so the suffix is written out here and pinned by a test against core's isAtomicTempPath. skills/media-use/scripts/lib/manifest.mjs is the byte-identical copy the parity check requires.
  • manifest.d.mts declares the three exports the new TypeScript test uses, like config-lock.d.mts.

Test plan

  • New src/media-use/manifestReservation.test.ts (vitest, outside lib/ so the build does not ship it, imports core's real isAtomicTempPath): while a reservation is being filled, the type folder holds nothing history would commit, and a second allocateId gets the next id; afterwards only the asset remains. Fails on main (expected [ 'bgm_001.wav' ] to deeply equal []). It also fails when the marker uses a name core does not treat as temp (bgm_001.wav.reserved). A second test writes a partial file and then fails the download: the folder is left empty. It fails if the rollback stops removing the partial file.
  • manifest.test.mjs: the MU-23 test now checks that the marker holds the id and that nothing sits at the final name; the LUT allocation test checks the marker.
  • All media-use and skills node:test files plus the copy-parity check (the CI skills job's set): 772 pass, 0 fail, 3 runs in a row. The vitest media-use tests pass 3 times. Linux, Node 22.
  • CLI tsc --noEmit, oxlint, oxfmt and the pre-commit hooks (fallow, skills manifest) are clean.

Not exercised: a Studio walk with a real slow download. The test uses the same isAtomicTempPath check that history's projectSignature uses.

Size

One reservation change in manifest.mjs (plus its skills copy), one declaration file, two new tests, two updated assertions, one SKILL.md line.

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 1556 (base branch 1556), smooth 1447 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)

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 4, 2026 06:37

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at e8c2e6c4.

The fix holds. The empty reservation file used to sit at the asset's final name. Now it is an empty marker named <asset>.hfXXXXXX.tmp. That matches core's TEMP_SUFFIX (/\.hf[0-9a-f]{6}\.tmp$/), which both projectSignature (affectsProjectSignature and isSkippedEntry) and the CLI fileWatcher skip, so history never commits the marker. nextFreeId matches ^<type>_(\d+) against every file in the type folder. The marker starts with the id, so it still holds that id while the download runs, and the new test confirms a second allocateId returns bgm_002.

Every caller. Every reservation goes through withReservedFile or withReservedFileSync. Those are the three cache-import sites in resolve.mjs, the two freezeUrl/freezeLocalFile sites, the parametric LUT, and both LUT paths in lut-preset-provider.mjs. So every path releases the marker. None of the writers relied on the placeholder already existing at the final path:

  • writeFrozen uses writeFileSync;
  • the cache import uses copyFileSync with no EXCL flag;
  • the LUT paths rename into place.

In reuse --sha, the process.exit runs only after withReservedFileSync has returned, so the marker is already gone by then. skills/media-use/scripts/lib/manifest.mjs is byte-identical to the CLI copy.

SKILL.md line. It matches references/setup-providers.md:9-12: heygen auth login --oauth gives the free allowance for TTS and avatar video, and an API key bills API credits. It states no amounts.

Verified locally:

  • The CI skills job's exact file set (skills + packages/cli/src/media-use *.test.mjs + the two scripts, with test-home.mjs and HYPERFRAMES_MEDIA_HOME_REQUIRED=1): 774 tests, 772 pass, 0 fail. The import-boundary check passes (2/2), and so does the new vitest file (2/2).
  • Mutations, each reverted after:
    • naming the marker .reserved fails the first new test;
    • dropping the partial-file cleanup on failure fails the second;
    • putting the empty file back at the final name fails both the first new test and the MU-23 test in manifest.test.mjs.

Nonblocking. If the process dies mid-download, the .tmp marker is left behind and that id gets skipped from then on. That is still better than before: history ignores the marker, while the old leftover was a 0-byte asset history would commit.

— Rames

@miguel-heygen
miguel-heygen force-pushed the fix/media-use-reserve-temp branch from e8c2e6c to 4bd8497 Compare October 4, 2026 07:42

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-approving at 4bd8497c, which rebases the e8c2e6c4 head I approved onto main after #4988.

  • Range-diff against e8c2e6c4: all four commits are unchanged except for the skills-manifest.json media-use hash, which each commit regenerates. manifest.mjs is byte-identical to the approved version, and the skills copy still matches it.
  • Hash: bun packages/cli/scripts/gen-skills-manifest.ts --check reports it in sync (21 skills).
  • Interaction with #4988: the only other media-use change between the two heads is #4988's heygen.mjs .env fix, which came in from main. It doesn't touch manifest.mjs or anything that uses the reservation.
  • Verified locally at this head: I ran the CI skills job's exact file set (775 tests, 773 pass, 0 fail, 2 skipped; one more test than before, from #4988). The new vitest file passes 2/2.

— Rames

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 4, 2026
@miguel-heygen
miguel-heygen force-pushed the fix/media-use-reserve-temp branch from 4bd8497 to 0eb7ee9 Compare October 4, 2026 08:16

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-approving at 0eb7ee9b, which rebases 4bd8497c (approved) onto main after #4995.

  • Range-diff against 4bd8497c: all four commits are unchanged except the regenerated skills-manifest.json media-use hash. Apart from that hash, the two heads differ only in #4995's heygen.mjs credentials-folder fix, which came in from main. manifest.mjs is still byte-identical to the approved version and to its skills copy.
  • Hash: gen-skills-manifest.ts --check reports it in sync (21 skills).
  • Verified locally at this head: I ran the CI skills job's exact file set: 777 tests, 775 pass, 0 fail, 2 skipped. The two extra tests come from #4995. The new vitest file passes 2/2.

— Rames

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit 18a922f Oct 4, 2026
81 checks passed
@miguel-heygen
miguel-heygen deleted the fix/media-use-reserve-temp branch October 4, 2026 08:58
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