[kevin9327]

What

A rejected window.__hf.buildReady promise now fails render, snapshot and check with a message naming the key and the rejection reason. Today the rejection is swallowed, the hold counts as released, and the run captures a scene that was never built and exits 0.

Why

The three.js adapter docs recommend putting setup the runtime can't see (meshes, shaders, baked data) behind a buildReady hold. When that setup fails, the composition rejects the hold. That's the only signal it has, and the runtime currently drops it, so a pipeline that trusts the exit code ships a blank film (see #5603).

Related work

Fixes #5603

How

• isBuildReadinessSettled wraps each registry entry once (cached in a WeakMap, so polling doesn't build new promises) and tags a rejection as BuildReadyRejection(key, reason).
• The tracker's onError passes that to a new recordBuildReadyRejection in runtime/diagnostics.ts. That function sets window.__hfStartupError to:
  window.__hf.buildReady["scene"] rejected: Error: build failed: …. The composition reported that its own setup failed, so it cannot be rendered. Fix that setup, or resolve the promise only once the scene is drawable.
  Any other error still goes to the existing swallow path.
• The hold is still treated as settled, so render-ready is released and the engine's pollHfReady and the CLI's waitForRuntimeReady see the startup error. Both already fail fast on __hfStartupError, so no engine or CLI change was needed.
• Teardown clears the error only if it's still the one this runtime set.

Test plan

• Unit tests added/updated: a new a rejected window.__hf.buildReady promise block in init.test.ts (4 tests): the error names the key and reason; render-ready is held until the rejection and then released with the error set; no error when every hold resolves; teardown clears it. Three of them fail without the fix. All 250 init.test.ts tests pass. oxfmt, oxlint, tsc --noEmit, check-comment-citations and comment-ratchet are clean.
• Manual testing performed with the issue's repro and a CLI built from this branch:
  • hyperframes render --no-best-effort -o out.mp4 gives ✗ Render failed with the message above, exit 1 (it exited 0 before).
  • hyperframes check gives Check failed, exit 1.
  • The same file with valid JSON, so the hold resolves, still renders, exit 0.
• Documentation updated (if applicable)
• Comments follow CONTRIBUTING.md "Comments"

---

AI assistance: I wrote this change with help from an AI coding assistant. I reviewed the code, ran the tests and checked the repro by hand.

[jrusso1020]

The code is correct and well tested. I'd approve it as written. The one open question is policy, not code: a rejected buildReady hold now fails render even in the default best-effort mode, not only under --no-best-effort. That's what the issue argues for ("arguably always"), and I agree, but it changes the exit code for any existing composition that rejects a hold and still renders something usable. A maintainer should sign off on that default.

What I checked

• packages/core/src/runtime/init.ts:3582: each registry entry is wrapped once and cached in a WeakMap. That keeps samePromiseSet stable across polls, and if the registry set changes, the new Promise.all still includes the cached wrapper, so the rejection stays handled.
• createSettledTracker (init.ts:263) calls onError before onSettled, so __hfStartupError is set before render-ready is published (:3594). The engine's pollHfReady (engine/src/services/frameCapture.ts:1747) checks the startup error first, so it can't race past it.
• Teardown (init.ts:5388) clears the error only if it's still the one this runtime set. That's the right scope.
• I checked that the regenerated position-edits-render-inline.ts changes only renamed minified identifiers.

What I ran

• vitest run src/runtime/init.test.ts: 250/250 pass.
• Mutation check: with init.ts reverted to main, 3 of the 4 new tests fail, as the PR says.
• git merge-tree against main and the other open capture PRs (#5650, #5748, #3941): no conflicts.

Nit

• class BuildReadyRejection (init.ts:254) sits between the Promise.all comment (:250) and createSettledTracker, so that comment now reads as if it documents the class. Moving the class above the comment fixes it.

— Rames

[kevin9327]

Thanks for the careful review, Rames. Fixed the nit in 183c90f: BuildReadyRejection now sits above the Promise.all comment, so the comment is next to createSettledTracker again. The regenerated position-edits bundle didn't change, and init.test.ts still passes 250/250.

Agreed on the policy point. I'll leave the best-effort default for a maintainer to decide. If you'd rather keep best-effort renders passing, I can limit the failure to --no-best-effort.

[jrusso1020]

Approving at 183c90f0. My earlier review was at 5cdc6c70, and the only commit since is the nit fix: one commit, init.ts only, moving BuildReadyRejection above the Promise.all comment. No behaviour change, and the full CI suite is green at this head.

The bug is still on main. init.ts there still sends a rejected buildReady hold to swallow("runtime.init.buildReady", ...), so render releases the hold and captures a scene that never built, with exit 0 (#5603 is still open). Everything I checked last time still holds: the wrappers are cached so the tracker's promise set stays stable across polls, the startup error is set before render-ready is published, and teardown only clears the error this runtime set.

On the policy point from my last review: I'm comfortable with the failure applying in the default best-effort mode too. A composition rejecting its own setup is the composition saying it can't be drawn, and shipping a blank film with exit 0 is the worse outcome. Anyone who relied on the old behaviour can resolve the hold instead of rejecting it, which the new error message says.

Need: yes, a failed build ships a blank render with exit 0 · Size: ok · Reuse: uses the existing __hfStartupError path that pollHfReady and waitForRuntimeReady already fail on, so no engine or CLI change · Simpler: already minimal

— Rames

[jrusso1020]

Thanks, @kevin9327. This one is approved, and it'll go into the merge queue once these are sorted:

• Signed commits: 1 commit(s) on this branch aren't verified, and main requires signed commits. GitHub's guide: https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits

Once it's green and signed, we'll queue it at your new head. If you push code changes beyond a rebase, we'll take another look first.

— Rames

[miguel-heygen]

+1 to the earlier approval. I looked at two things it didn't cover.

First, which shipped blocks this can newly fail. The 36 WebGPU shader blocks reject their shaders hold when WebGPU is unavailable, but every one of them declares data-requires-webgpu, and the launch-time adapter check already fails those renders on a host without a GPU. So nothing in the catalog that renders today starts failing because of this. The one other block that rejects a hold (frost-sequence-camera-orbit, on a loader error) now fails instead of rendering its error text into the film, which is the point of the change.

Second, drift. The branch is well behind main, but it merges cleanly, and the buildReady tracker on main still sends rejections to swallow, so the bug is still live. Neither diagnostics.ts nor the generated position-edits bundle has changed on main since your base, so the bundle check should still pass after the merge.

Optional: the skill references that tell authors to use buildReady (skills/hyperframes/references/runtimes.md and skills/hyperframes/references/types/3d.md) could say in one sentence that a rejected hold now fails the render, so authors know to reject only when the scene can't be drawn.

As already noted, 183c90f needs to be signed before this can be queued.

Verdict: APPROVE
Reasoning: The fix is correct at this head and CI is green; no shipped block that renders today starts failing, the branch merges cleanly with main, and only the commit signature remains.

[kevin9327]

Thanks for the reviews. I squashed the branch into one signed commit (1cc70827, shows as Verified) on top of current main. The code is the same as the approved head, no changes beyond the rebase. I re-ran build:position-edits-render (bundle unchanged) and init.test.ts passes (255 tests).

[jrusso1020]

Thanks for re-signing. It conflicts with main again, so could you rebase and push once more? We'll re-approve at the new head.

[kevin9327]

Rebased onto current main (64c7bbf2) as one signed commit, 81be532b (Verified). Two conflicts, both from main's new SCENE_CALLBACK_ERROR_PREFIX: diagnostics.ts keeps that constant next to recordBuildReadyRejection, and the init.ts import now pulls in both. No other code changes. position-edits-render-inline.ts is regenerated with build:position-edits-render (it reproduces main's file byte for byte on a clean main), and init.test.ts passes (260 tests).

[kevin9327]

Rebased onto current main and pushed a signed commit. I regenerated the conflicting generated file (skills-manifest.json / position-edits-render-inline.ts) with the repo's generator. The PR's tests pass locally at the new head.

[jrusso1020]

Reviewed at 8d32ae4f0581284e13e4b132b9418b712f8caa97.

The rebase preserves the previously approved failure policy. I compared current source with approved 1cc7082716ff36c504cf47a226a69128a19db467: the Promise tracker, keyed readiness collector/onError block, rejection recorder and owned-error teardown blocks are byte-identical.

Four current-source extracted controls pass: rejection records the startup error before ready, repeated polls of the same promises settle correctly, teardown preserves another startup error, and an unreadable rejection reason uses the fallback. The full runtime suite could not collect with borrowed dependencies, so I am not carrying the old 250-test count onto this head. No render or browser run.

Need: yes · Size: ok · Reuse: existing readiness tracker and diagnostic ownership · Simpler: already minimal · Structure: fine
Risk: medium - startup failure ordering and cleanup across a rebase.

Verdict: APPROVE
Reasoning: The approved logic is unchanged and the focused ordering controls pass.

Reasoning: The current readiness rejection, ordering and owned-error cleanup logic matches the approved implementation across the rebase.