fix(producer): retry capture on a CDP Page.captureScreenshot refusal

[miga-heygen]

Summary

Chromium's CDP layer can refuse a screenshot capture outright — Protocol error (Page.captureScreenshot): Unable to capture screenshot — with no timeout wording at all. classifyCaptureFailure() didn't recognize this message in any of its pattern lists, so it fell through to the fatal authoring bucket. Both of this codebase's capture-retry mechanisms gate on that classification, so neither ever retried:

• The disk-capture retry loop (executeDiskCaptureWithAdaptiveRetry) only bounces a transient failure once via isRecoverableParallelCaptureError, which checks classifyCaptureFailure(...).kind.
• The streaming-capture pinned-fallback retry (shouldRetryViaPinnedFallback) only retries drawElement-specific failures or a failure on a worker count already pinned by inversion/the parallel router — --low-memory-mode's single-worker plain-screenshot capture engages neither, so that mode had no whole-render fallback for this error at all.

Changes

• packages/engine/src/services/captureFailure.ts: added the literal error text to TRANSIENT_BROWSER_ERROR_PATTERNS, anchored to the specific reason string (not just the CDP method name) so an unrelated, genuinely deterministic Page.captureScreenshot error isn't swept in too. A timed-out variant of the same call still classifies protocol_timeout — that pattern list is checked first, unchanged.
• packages/producer/src/services/renderOrchestrator.ts: shouldRetryViaPinnedFallback() gained a new isTransientCaptureError?: boolean argument (computed at its one call site from the engine's own isTransientBrowserError() helper), treated as routing-independent — the same way an existing drawElement-renderer-stall or sequential-capture-stall is retryable regardless of whether a worker count was pinned.
• No other code change was needed for the disk path: it already retries on classifyCaptureFailure(...).kind === "transient_browser", so the pattern-list addition alone fixes it.

Test plan

• bunx vitest run packages/engine/src/services/captureFailure.test.ts packages/producer/src/services/renderOrchestrator.test.ts — 241/241 pass
• Verified the new tests actually catch the bug: stashed the two source-file changes (kept the tests), reran — 5 assertions failed exactly as expected against the old fatal-on-first-occurrence behavior, then restored the fix
• New integration test drives the real executeDiskCaptureWithAdaptiveRetry with a mocked capture throwing this exact error and confirms it retries once and recovers
• bunx tsc --noEmit clean on packages/engine and packages/producer
• bunx oxlint / bunx oxfmt --write clean
• Confirmed via parallelCoordinator.ts that this reclassification also stops a single worker's screenshot refusal from aborting sibling workers as a "fatal" failure — consistent with how other transient browser errors (Target closed, Session closed, etc.) already behave there
• Confirmed the retry is bounded on both paths: the disk path's transient-retry branch is capped at MAX_TRANSIENT_CAPTURE_RETRIES = 1 before falling back to worker-halving; the streaming path's retry is a single, non-looping attempt

[terencecho]

Verdict — APPROVE

Head verified: 51c75f08e32f946e01289c2b367f334587269fbe. Small focused fix (4 files, +71/-5) that widens the transient-classification net to one specific CDP screenshot-refusal wording that was silently falling into the fatal authoring bucket.

The three axes I was asked to verify

1. Retry is scoped to the transient screenshot-refusal case. The classification widening is one regex added to TRANSIENT_BROWSER_ERROR_PATTERNS:

/Protocol error \(Page\.captureScreenshot\): Unable to capture screenshot/i

Anchored to BOTH the CDP method name AND the specific reason string. Other CDP errors on the same method (different reason strings) do not match; the fatal-authoring pattern Composition has zero duration. Runtime ready: true is a different pattern list and stays fatal. The parametrized test proves the boundary:

• Protocol error (Page.captureScreenshot): Unable to capture screenshot → transient_browser ✓
• Protocol error (Page.captureScreenshot): waiting for debugger timed out → protocol_timeout (matched first by PROTOCOL_TIMEOUT_PATTERNS, unchanged) ✓
• [Parallel] Capture failed: Worker 0: Protocol error (Page.captureScreenshot): Unable to capture screenshot → transient_browser (unwrapped-Parallel prefix still matches the substring) ✓

Genuine composition-broken errors are unaffected — different pattern list, different reason wording.

2. Bounded retries — no infinite loop.

• Disk path (executeDiskCaptureWithAdaptiveRetry): existing MAX_TRANSIENT_CAPTURE_RETRIES = 1 cap gates the transient-retry branch, then falls back to worker-halving on the next failure. The parametrized regression test "retries ONCE at the same worker count on … with zero progress" iterates both the tab-died and CDP-refusal shapes and confirms both hit the same one-shot behavior via the pre-existing cap.
• Streaming path (shouldRetryViaPinnedFallback): returns boolean — a single decision, non-looping by construction. The new isTransientCaptureError === true branch adds one more short-circuit but doesn't create a loop.
• Cancellation always wins: if (args.isCancellation || args.isEncoderInterrupted) return false; is checked FIRST, before the new transient branch. Test "never retries a transient capture-call refusal after cancellation" pins this.

3. Distinguishes Chromium-refusal from genuinely-broken composition.

• classifyCaptureFailure truth-table stays intact: authoring bucket (fatal) still catches Composition has zero duration. Runtime ready: true; new refusal wording is transient_browser; timed-out variant is protocol_timeout (checked first). Three separate pattern lists, each anchored to distinct wording.
• The wiring on the streaming path uses isTransientBrowserError(err) (the engine's own helper that checks classifyCaptureFailure(...).kind) — same classifier as the disk path, so both paths agree on what "transient" means. No divergent discrimination.

Extra correctness notes

• shouldRetryViaPinnedFallback truth-table exhaustive by branch:

  • Cancellation OR encoder-interrupted → false (short-circuit, unchanged)
  • Verify error OR de-capture-error → true (unchanged)
  • Renderer stall OR sequential stall → true (unchanged)
  • Transient capture error → true (NEW; routing-independent like the stalls above)
  • Otherwise → deWorkerInversion === "inverted" || deParallelRouter === "routed" (unchanged fallback)

  New branch order (after stalls, before routing) matches the sibling stall-retries — consistent placement for a routing-independent transient. Test "retries a transient capture-call refusal even with no pinned routing" proves the --low-memory-mode case (no pinned routing, no drawElement) that previously had zero fallback is now covered.

• Single new call site: isTransientCaptureError: isTransientBrowserError(err) in executeRenderPipeline. No other callers, so no legacy path silently getting the pre-fix behavior.

• parallelCoordinator.ts (not in diff) benefits from the reclassification transparently — a single worker's refusal now emits a transient_browser-classified error instead of authoring, so sibling workers don't abort as if the composition were broken. This is a side effect of the classification change, not a code change in the coordinator itself.

• Mutation-tested per PR body: stashing the two source-file changes (keeping tests) produced 5 assertion failures on the old fatal-on-first-occurrence behavior — the new tests actually catch the bug.

CI

10 pass, 0 fail, 0 pending on this head. mergeStateStatus=blocked waiting on this approval (last-push-approval requirement). No prior reviews on this SHA.

Stamp.

— tai