fix(render): preserve interruption recovery

[miguel-heygen]

What

Long renders can now recover once when an ordinary single-worker stream loses its loopback connection — to the file server or to Chrome's DevTools port — before the first frame is captured. Interrupted audio-group submixes also retain their canonical retryable classification through the producer boundary.

Why

The streaming recovery gate previously covered verification failures and router-pinned paths only. Loopback connect ETIMEDOUT and net::ERR_TIMED_OUT were mislabeled as fatal authoring failures, so a render whose file server or DevTools port stopped answering before the first frame failed immediately despite complete session cleanup.

A bare loopback timeout does not identify whether the failed endpoint belongs to the file server or Chrome DevTools. The classifier now retains the reported host and port through the streaming-stage error wrapper, so the orchestrator can log the endpoint owner, probe the active file server with a bounded identity-checked request, and recreate the server only when that probe fails — all before the one allowed retry.

The loopback gate is deliberately narrow: a Chrome killed by a host shutdown looks exactly like Target closed, so a failure that names no loopback endpoint never enters this path, and a loss after the first frame is not eligible either, so progress never walks backwards onto the slower screenshot path. Remote ETIMEDOUT remains fatal. Note that main has since merged #3892, whose isTransientCaptureError retries every transient_browser failure routing-independently (for the CDP Page.captureScreenshot refusal under --low-memory-mode); after rebasing onto it, whether a bare Target closed retries at all is decided by that gate, not by this one. This PR keeps #3892 intact and adds only the loopback-specific probe-and-restart on top.

Separately, the audio-group wrapper flattened FFmpeg termination metadata into a permanent ffmpeg_failed, reported a user cancel as a system fault, and could rerun a managed deadline (or discard an interruption that landed on the automation rerun) before surfacing the first failure.

How

• Classify loopback connection losses as transient and retain their endpoint provenance. The classifier reads the whole cause chain (Node's fetch reports every connect failure as fetch failed with the errno underneath; a dual-stack localhost connect surfaces as an AggregateError with an empty message) and recognises the errno text Node and Bun's net emit — connect ETIMEDOUT|ECONNREFUSED|ECONNRESET against 127.0.0.1, localhost, ::1 or [::1] — and Chrome's navigation errors net::ERR_TIMED_OUT|ERR_CONNECTION_TIMED_OUT|ERR_CONNECTION_REFUSED|ERR_CONNECTION_RESET|ERR_CONNECTION_CLOSED|ERR_EMPTY_RESPONSE at http://<loopback>:<port>. In the hosted Bun producer the shape that reaches the classifier is Chrome's net::ERR_* from page.goto; Bun's own fetch/WebSocket errors carry no host or port and are not attributed. Because these Chrome spellings previously classified as fatal authoring, the multi-worker disk path and the probe stage now grant them their existing single bounded transient retry as well; only the one-worker stream probes and restarts the file server.
• Forward the classified endpoint through CaptureStageError, the only shape the orchestrator's catch sees; export isLoopbackConnectionLoss and CaptureEndpointDiagnostic from the engine.
• Extract the recovery decision into render/preFrameRecovery.ts: resolvePreFrameLoopbackLoss (loopback loss, exactly one worker, zero frames captured) is the single owner of the retry gate; recoverPreFrameFileServer logs endpoint health and restarts the file server only on an unhealthy probe. The orchestrator wires these in place of the previous transient_browser && workerCount === 1 check.
• Construct every render-path file server through one createRenderFileServer factory (probe discovery, frame capture, pre-frame retry) so the three sites cannot drift.
• Harden the health probe: a rejected body.cancel() can no longer mark a healthy server unhealthy, and the probe error carries the errno (ECONNREFUSED on Node, ConnectionRefused on Bun).
• Route group-submix failures through the canonical FFmpeg classifier; report a cancelled group submix with the same cancelled/user shape as every other cancelled audio stage.
• Share one canRetryMixLocally guard between grouped and ungrouped mixes so a retryable failure (external interruption, managed deadline/inactivity, missing FFmpeg) never triggers a compatibility or automation-degradation rerun, and adopt an interruption that lands on the automation rerun instead of discarding it.
• Keep deterministic unsupported-filter and automation-expression failures eligible for their existing local fallbacks.

Test plan

• Loopback classification: 38 tests — bare ::1, bracketed [::1], 127.0.0.1, localhost, Chrome ERR_CONNECTION_*, undici fetch failed with the errno in .cause, dual-stack AggregateError, cause-chain depth bound, remote-host and bare Target closed negatives, protocol-timeout precedence.
• Endpoint provenance through the production wrapper: CaptureStageError retains endpoint and classifyCaptureFailure returns it unchanged on the orchestrator's re-classification path.
• Recovery seam: 15 tests — qualifies only loopback loss with one worker and zero frames; rejects Target closed, browser-launch failure, page crash, runtime-not-ready, navigation timeout, remote timeout, multi-worker, post-first-frame, and cancelled; restarts on an unhealthy probe, keeps a healthy server, propagates a failed restart.
• Retry gate: a SIGTERM-shaped Target closed never qualifies for the loopback recovery path; cancellation and encoder interruption still veto a qualifying loopback loss; fix(producer): retry capture on a CDP Page.captureScreenshot refusal #3892's Page.captureScreenshot refusal tests still pass after the rebase.
• Mutation checks (each turned a test red): dropping endpoint forwarding, deleting the restart block, workerCount === 1 → >= 1, dropping the zero-frame gate, forcing the identity-header check true, removing the bare ::1 pattern, removing the cause walk, reverting the ungrouped audio guard, discarding the rerun interruption.
• File-server health: 59 tests, including a foreign listener answering 200 without the identity header and the errno on a refused probe.
• Audio mixer: 66 tests, including a cancelled group submix reporting cancelled/user, an interruption on the group automation rerun surfacing as external_interruption, and an ungrouped managed deadline running the mix once.
• Orchestrator + capture plan: 240 tests; engine and producer typechecks, oxlint and oxfmt clean.
• Documentation updated (not applicable).

[miga-heygen]

Preserves external_interruption and managed_deadline failure classification from group sub-mixes. canRetrySubmix guard correctly skips degradation retries for retryable failures. — Miga

[miga-heygen]

Second-pass quality review at head b5a91909 (isolated worktree; rebase checked file-for-file against the PR file list, 410/18 over 9 files, no hunk dropped or duplicated; captureFailure 20, audioMixer 63, fileServer 57, capturePlan 11, renderOrchestrator 221 all pass; producer tsc --noEmit clean). Three findings I consider blocking.

Blocking

• B1 — endpoint provenance never reaches the log on the production path. packages/producer/src/services/render/captureStageError.ts:10-15 builds classified = classifyCaptureFailure(cause) but does not forward classified.endpoint to super(); captureFailure.ts:152-154 and :162 then early-return on, or prefer, the carried endpoint that is now always absent, so renderOrchestrator.ts:3849-3862 logs endpointOwner: "unknown" every time. Proved with a temporary test (WRAPPED: transient_browser undefined). The description's "recovery now retains the reported host and port" does not hold; forward endpoint in the super() call.
• B2 — the loopback matcher misses the form Node actually emits. Node and Bun report connect ETIMEDOUT ::1:49152 unbracketed (verified locally), which does not match the \[::1\] pattern and so classifies as authoring → fatal, no retry — while fileServer.ts:964 advertises http://localhost:PORT, so IPv6 loopback is live. messageOf also never walks error.cause, and undici flattens connect failures to "fetch failed" (proved by running the new probe), so the classifier sees neither host nor code in that case.
• B3 — nothing pins the recovery. Three mutations survive: deleting the entire 26-line health-probe/restart block → 221/221 orchestrator tests green; workerCount === 1 → >= 1 → 221/221 green; replacing the x-hyperframes-file-server: healthy identity check with true → 57/57 fileServer tests green. The recovery path needs at least one test that fails when it is removed.

Important: dead detail property at audioMixer.ts:1454; the cancellation path still hand-rolls ffmpeg_failed / owner: system for a user cancel (:1064); an interrupt during the automation rerun discards the interruption (:1056-1062); the file server is constructed at two sites that can drift.

Refuted and recorded as such: the restarted server is used by the retry; job.framesRendered === 0 is a real pre-frame signal; the retry cannot loop; the restart passes the same server options. Mutations that did go red: loopback classification (3 failed), audio canRetrySubmix (2 failed).

Verdict: COMMENT — needs fixes before merge.
Reasoning: the provenance the description promises is dropped one constructor short of the log, the common IPv6 loopback failure is classified as fatal, and the recovery block can be deleted without a test noticing.

Review by Miga

[jrusso1020]

Reviewed the recovery path end-to-end at b5a919093341ef88b31ca6028b42937f31db5def, plus an independent adversarial pass over the same code. Additive to @miga-heygen's second-pass review at this head: we converge on the endpoint drop, the IPv6 form and the missing coverage, so below I give my own evidence and my own severity calibration where it differs, and add three findings that pass did not reach.

Strengths

• renderOrchestrator.ts:2753 changing const activeFileServer to let is the load-bearing line of the PR and easy to miss. invokeStreaming (:3692-3704) reads activeFileServer when called, so the rebind inside restartCaptureFileServer (:3450-3463) is what the retry actually picks up. Correct and subtle — and I confirmed no stale URL survives: buildCaptureOptions (:2814) and createRenderVideoFrameInjector (:2852) capture neither URL nor port.
• captureFailure.ts:176 places the new timeoutEndpoint || test after the protocol-timeout branch, so Page.captureScreenshot timed out still classifies as protocol_timeout rather than being swallowed by the new loopback path. Deliberate and right.
• audioMixer.ts:1041-1043 — gating the local fallbacks on canRetrySubmix() (!retryable) stops the mixer burning a compatibility rerun on a failure the caller will retry anyway. Right shape for the stated goal.
• The probe's identity check genuinely works: a coincidental foreign 200 on the same port is rejected, and the 1s bound is real (measured durationMs: 1008 against a bind-but-never-respond server).

Blockers

• 1. The classifier misses the dominant real-world spellings of the failure it targets. Running classifyCaptureFailure against the worktree sources:

  transient_browser  fatal=false  <- net::ERR_TIMED_OUT at http://localhost:49152/...
  authoring          fatal=TRUE   <- net::ERR_CONNECTION_TIMED_OUT at http://localhost:49152/...
  authoring          fatal=TRUE   <- net::ERR_CONNECTION_REFUSED  at http://localhost:49152/...
  transient_browser  fatal=false  <- connect ETIMEDOUT 127.0.0.1:49152
  authoring          fatal=TRUE   <- connect ETIMEDOUT ::1:49152
  transient_browser  fatal=false  <- connect ETIMEDOUT [::1]:49152
  

  Three separate gaps, all fatal-by-default via :181 + isFatalCaptureFailure (:202-204):

  • Bare IPv6. :115 requires \[::1\]. Node emits it unbracketed — a real connect on this box gives connect ECONNREFUSED ::1:1, and dns.lookup("localhost") returns ::1 first here, so that is the address a loopback connect lands on. The bracketed form the code matches comes from Chrome URLs, not from Node.
  • Chrome's connect-phase codes. ERR_CONNECTION_TIMED_OUT is the likelier code for this scenario than ERR_TIMED_OUT, and ERR_CONNECTION_REFUSED is exactly "the file server is gone" — the case restartCaptureFileServer exists to cure. /ECONNREFUSED/i in the transient list does not match ERR_CONNECTION_REFUSED (not a substring).
  • Wrapped errors. undici flattens every connect failure to a top-level "fetch failed" with the real error in .cause; localhost yields an AggregateError whose own message is empty. messageOf (:109-111) reads only .message, so even 127.0.0.1 — the form the regex does support — is invisible through fetch. frameCapture.ts:293-300 already implements a depth-5 cause walk for precisely this reason; this matcher doesn't use it.

• 2. The retry is widened well past what the PR describes, and it breaches this file's own documented invariant. The gate is kind === "transient_browser" && workerCount === 1 (:3757-3759), not "loopback timeout". At base, shouldRetryViaPinnedFallback ended in return args.deWorkerInversion === "inverted" || args.deParallelRouter === "routed" — so on an ordinary one-worker stream every one of these returned false; now they all return true:

  transient_browser <- Failed to launch the browser process! spawn ENOENT
  transient_browser <- Page crashed!
  transient_browser <- Target closed
  transient_browser <- Composition has zero duration ... Runtime ready: false
  transient_browser <- Navigation timeout of 30000 ms exceeded
  

  Two consequences. Deterministic failures (Chrome that cannot launch — missing sandbox, no /dev/shm, cgroup limit — or a comp whose runtime never becomes ready) now fail twice, doubling time-to-failure after a second full browser-launch timeout. More seriously, the docstring immediately above the function (:1886-1888) states: "Encoder interruptions remain excluded so a host shutdown cannot be hidden behind same-host retry work." That guard is err instanceof EncoderInterruptedError. On a preempted or SIGTERM'd host where Chrome dies before the encoder notices, the error is a plain Target closed / browser has disconnected → transient_browser → retry, spending the pod's grace period on a host that is going away. The new branch sits below the encoder guard, but the encoder guard does not cover this shape. If the intent really is loopback recovery, gate on the endpoint being present rather than on kind.

• 3. Nothing pins any of it, and it is not merely a missing test. executeRenderPipeline is not exported (:2187), so no unit test can reach the recovery block at all; renderOrchestrator.test.ts imports ~30 exported helpers and has zero references to it. The 33 added lines exercise shouldRetryViaPinnedFallback with isTransientSingleWorkerFailure handed in as a literal, and capturePlan.test.ts exercises replanAfterFailure directly. Nothing anywhere references probeFileServerHealth except fileServer.test.ts (standalone) and the orchestrator itself. So the probe call and its framesRendered === 0 gate (:3832-3835), the workerCount === 1 gate, restartCaptureFileServer, the endpointOwner classification, and the const→let rebind are all unpinned. The fix is a seam, not a test: extract the pre-frame decision the way those two predicates already were, then pin it. Note captureFailure.test.ts:68-76 asserts endpoint retention on a bare new Error(...) — the one input shape that takes the non-early-return path, so it cannot reproduce the production drop in finding 4.

Important

• endpoint is dropped one constructor short of the log. render/captureStageError.ts:10-15 forwards kind, message, cause, workerDiagnostics — not endpoint. Since the result is itself a CaptureFailure, classifyCaptureFailure early-returns it unchanged (:157-159) or prefers the carried endpoint over the recomputed one (:162), so endpointOwner (:3849-3853) is always "unknown" and reportedEndpoint always undefined. captureStreamingStage.ts:918 throws wrapCaptureStageError(...), so this is the only shape the retry block sees. I calibrate this important rather than blocking — I traced the consumer and the restart keys on health.healthy, not on endpoint, so recovery still functions; what breaks is the diagnostic and the PR body's "retains the reported host and port" claim. Worth noting it also behaves differently between monorepo and published-package consumers, since a duplicated engine instance fails the instanceof and would compute the endpoint.
• The retry has no frame-progress gate; only the probe does. preFrameHealthPromise is gated on job.framesRendered === 0, but isTransientSingleWorkerFailure is not. A transient Chrome death at frame 9500/10000 therefore replans to forceScreenshot and restarts from frame 0 on the slower path, and resetCaptureAttemptProgress (:965-967) zeroes framesRendered, so reported progress walks backwards. The PR body says "before the first frame"; the code says any time. Conversely, if the file server is the problem after frame 0, the restart that would fix it is skipped and the one allowed retry is spent against the same bad server.
• The audio fix landed on the group path only. mixGroupMembers now uses canRetrySubmix(), but the canonical ungrouped mixAudioTracks (:868-873) still guards on result.failureReason !== "external_interruption" alone, so a managed deadline / inactivity failure still fires a second full runMix(true) with its own ffmpegProcessTimeout. The claim "preserve … managed deadline/inactivity … without automation-degradation reruns" is half-implemented.
• A user cancel is reported as a system FFmpeg fault. audioMixer.ts:1063 returns { success: false, error: "Group sub-mix cancelled" } with no failure, so :1449-1454 stamps reason: "ffmpeg_failed", owner: "system", retryable: false. The ungrouped path emits { stage: "cancelled", reason: "cancelled", owner: "user" }. This feeds failureOwner at :2698-2706.

Nits

• packages/engine/src/index.ts:133-137 re-exports CaptureFailureKind and CaptureWorkerDiagnostic but not CaptureEndpointDiagnostic, so the new public endpoint field's type is unnameable outside the engine package.
• captureFailure.ts:120 stores host as the literal "[::1]" when the bracketed form matches, inconsistent with the bare "127.0.0.1" from the other branch — and :3850 compares only port, so the asymmetry is currently invisible rather than harmless.
• fileServer.ts:719-720 — await response.body?.cancel() sits inside the same try as the health computation, so a rejecting cancel() turns a healthy server into healthy: false and triggers a needless restart. Compute and return, or nest the cancel.
• audioMixer.ts:1454 — the detail in the fallback object is dead; :1458 overwrites it.
• fileServer.ts:730 — healthProbeError carries no errno and cannot distinguish refused from timed out ("fetch failed" on Node, "Unable to connect…" on Bun). In a change whose purpose is diagnostics, this is the field that gets read.

Checked and cleared (so nobody re-runs them)

• Can the probe even reach the server? It binds 127.0.0.1 (:962) but advertises http://localhost:PORT (:964), and localhost resolves to ::1 first here — I expected the probe to always report unhealthy. Refuted: autoSelectFamily defaults true on Node 22 and Bun's Node 24, so the ::1 attempt fails in ~1ms and falls back to IPv4.
• Can the retry loop? No — :3739-3741 is a plain try/catch, the second invokeStreaming() is outside it, and executeRenderPipeline has one caller with no loop.
• Handle leak or double-close on a failed restart? No — both the execution.defer closure (:2275-2279) and the inline close (:4135) read the let fileServer, which the restart updates; if createFileServer throws, the old handle is already closed and fileServer is null.
• Unawaited closeFileServerSafely? Not a defect — synchronous void (:757-769), destroys tracked sockets before server.close().
• Does the restart reproduce the server options? Yes, including the conditional HF_PAGE_SIDE_COMPOSITING_STUB; the sole runtime addPreHeadScript site (:3424) is the one it replicates.
• capturePlan.test.ts is a test-only edit to an out-of-diff module — it adds a new characterization test; no existing assertion was weakened.

Scope — Audited end-to-end: captureFailure.ts, captureStageError.ts, fileServer.ts, the renderOrchestrator.ts streaming catch and recovery block, the audioMixer.ts group-submix path, and all five test files. Trusting: the rest of renderOrchestrator.ts outside the streaming catch.

Verdict: REQUEST CHANGES
Reasoning: the targeted failure still classifies as fatal in the three spellings it actually arrives as (bare IPv6, Chrome's ERR_CONNECTION_*, anything wrapped by fetch), while the retry gate is simultaneously too broad — it now retries deterministic browser-launch failures and breaches the documented rule that a host shutdown must not hide behind same-host retry work. Neither direction is observable from the suite, because the recovery block sits in a non-exported function no test can reach.

— Rames Jusso

[miga-heygen]

Addressed in 6fcf230 (rebased onto main including #3892; review 5189509448 B1/B2/B3 + important; review 5191112187 blockers 1-3 + important/nits, except the audio observability field and Bun fetch/WebSocket attribution, which the PR body states as out of scope).

[miga-heygen]

Reviewed head 6fcf2308b68e1900c03da44a1ddb03b1e213f177 in an isolated worktree. All three blockers from my review at b5a91909 are closed and each is now pinned by a mutation that previously survived: captureStageError.ts:15 forwards the classified endpoint so the orchestrator logs the real owner; captureFailure.ts:119-160 walks .cause to depth five and AggregateError, and matches Node errno and Chrome net:: spellings against every loopback form including bare ::1 (fourteen real error shapes probed, five non-loopback decoys correctly rejected); the pre-frame decision now lives in render/preFrameRecovery.ts under fifteen tests, and the workerCount, framesRendered, identity-return and health-header mutations all go red. The overlap with the routing-independent transient retry from the base branch is benign: the narrow predicate implies the broad one, so there is a single bounded retry, and the narrow path still solely owns the file-server restart, as the docstring at renderOrchestrator.ts:1866-1872 states. Non-blocking follow-ups: the if (preFrameRecovery) wiring at :3866-3873 is unpinned (executeRenderPipeline is unexported); a loopback loss wrapped deeper than five cause levels classifies as fatal; endpoint is also populated on non-loopback kinds; a failed restart replaces the render's terminal error (root cause preserved in the warn and checkpoint); the four new gate tests omit the broad flag and so assert a branch with no production effect. 434 tests green across seven suites, producer tsc --noEmit clean, rebase integrity verified against the current main tip. — Miga