Repository navigation
fix(audio): preserve subprocess causes through CLI diagnostics - #5620
Conversation
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
Reviewed exact ae5c571833f97e28324f51e727f1f9303eb62742. The allowlisted, bounded final CLI writer appears to keep raw messages, stderr, paths and element IDs out of the new record in the inspected path, but the interruption that actually aborts a render can be dropped behind 16 earlier audio-element failures (inline). Please preserve the triggering failure within the cap and add a mixed-order regression.
The current-head Producer: unit tests job failed: four existing assemble-stage interruption cases throw Vitest’s missing audioFailureDiagnostics export error instead of EncoderInterruptedError. assembleStage.test.ts replaces @hyperframes/engine with a mock exporting only the older functions, while encoderInterruption.ts now imports the new helper. Repair the mock and rerun that hosted job. Build, lint, typecheck, CLI smoke and other listed jobs have executed green; other jobs were still running at review time and are not counted as passes. No packaged Desktop/Datadog delivery was exercised by me. No merge or release action. — Jerrai
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
The exact repaired head prioritizes the actual interrupted child before the 16-record projection, and its final-consumer regression exercises sixteen missing sources followed by a real SIGTERM, asserting the interruption survives without paths or identifiers. Moving projection to audioStage leaves the shared encoder-error constructor with a type-only engine import, so both the Vitest assembly and Bun encode-stage mocks no longer require a new runtime export. The hosted producer unit job on ffa5716e ran 933 Vitest passes and the Bun encode-stage cases green; producer integration, build, typecheck, lint and CLI smoke also passed there. This final commit only replaces a predictable test project path with mkdtempSync plus cleanup; its hosted producer units and lint have passed, other core checks and JavaScript CodeQL analysis are still running. The CodeQL wrapper's neutral configuration warning is not a clean scan result. No packaged Desktop/Datadog acceptance was exercised. — Jerrai
Edit accuracy: accurate 271 (base branch 271), smooth 160 of thoseThe gate passes. Quarantined, measured but not gated (0) |
What
Preserve audio subprocess causes from the engine through producer warnings to a versioned, bounded
@hf-diagnosticCLI line. Missing executables, ordinary exits, deadlines and grouped-mix failures retain distinguishable stage/reason/exit/signal/OS-code fields.Why
Desktop export reports can currently say only that audio preparation failed and the CLI exited 1. The engine knows more but discards child-process evidence; group submixes also collapse typed causes. This is the independent source/CLI contract slice. Desktop ingestion follows once the contract is released.
Related work
mainand does not depend on it.How
Keep content-free subprocess evidence on audio failures, preserve group failures, and project at most 16 allowlisted records into local and distributed warning paths. Deep-clone nested evidence in warning snapshots. For an external interruption, put the triggering failure first and sanitize at the audio-stage boundary before passing records to the shared encoder error. The error class has only a type import of the audio contract, keeping encoding-only paths independent of its runtime helper.
Re-project at the final CLI writer: no detail, element IDs, messages, stderr, paths, URLs, arguments, tokens, prompts or project contents enter this new record. No telemetry upload or consent behavior is added.
Test plan
internal.