Skip to content

fix(audio): preserve subprocess causes through CLI diagnostics - #5620

Merged
jrusso1020 merged 5 commits into
mainfrom
codex/audio-failure-diagnostics
Oct 14, 2026
Merged

jrusso1020 merged 5 commits into
mainfrom
codex/audio-failure-diagnostics

Conversation

@jrusso1020

@jrusso1020 jrusso1020 commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

What

Preserve audio subprocess causes from the engine through producer warnings to a versioned, bounded @hf-diagnostic CLI 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

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

  • 292 focused checks across CLI handler/wire (115), mixer/grouping/cloning (87), audio stage/event publisher/assembly (44), distributed plan/policy (46).
  • All 687 Bun producer unit tests across 39 files pass, including encodeStage's exact 20 previously failing cases. All four assembly interruption assertions pass with the original mock and assertions retained.
  • Real subprocess stand-ins induce ENOENT, exit 7, deadline/SIGTERM and group exit 9; exact fields survive mixer → producer policy → final CLI writer. POSIX executable stand-ins skip Windows; field/privacy tests run on every OS.
  • A real interrupted child following 16 missing sources reproduces the ordering bug without the fix. The final writer retains the triggering failure and exactly 16 records with the fix. Real local orchestration also reaches the final writer; CLI handler tests cover ordinary/interruption failures and quiet mode.
  • CLI diagnostic tests use isolated mkdtemp projects with cleanup. CodeQL SARIF traced three new temporary-file flows to the added shared /tmp/project test call; that predictable path is removed. Hosted CodeQL must re-run; no alert is suppressed.
  • Mutation: removing only subprocess preservation causes five final-consumer regressions to fail; restored implementation passes.
  • Fresh full workspace build, affected rebuilds, package typechecks, changed-file lint/format, test reachability, normal hooks and diff audit pass. Audit has three inherited unused interruption-error members and no new findings.
  • The broader local Vitest producer attempt has one real-media AAC loudness failure (4.4 LU vs 3 LU) in unchanged audio-pad code; its threshold is retained. The full local lane is not claimed green. Hosted CI reruns on every new head.
  • Probe errors keep existing stage/reason without new ffprobe child evidence. Browser/FX exceptions remain internal.
  • No packaged Desktop, signed-in Datadog delivery, real customer media, production deployment or release exercised. These are local/component and CLI-boundary checks, not live acceptance.

@jerrai-bot-heygen jerrai-bot-heygen 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.

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

Comment thread packages/producer/src/services/render/encoderInterruption.ts Outdated

@jerrai-bot-heygen jerrai-bot-heygen 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.

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

@jrusso1020
jrusso1020 enabled auto-merge October 14, 2026 20:19
@github-actions

Copy link
Copy Markdown
Contributor

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

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 14, 2026
Merged via the queue into main with commit 9c11715 Oct 14, 2026
80 of 81 checks passed
@jrusso1020
jrusso1020 deleted the codex/audio-failure-diagnostics branch October 14, 2026 20:46
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