gate(visual): browserless render failure is swallowed, not deferred → screenshot gate false-closes legitimate PRs (#9030/#9207 unpatched sibling)

[JSONbored]

Parent: #9461

Summary

#9030 and #9207 taught the screenshot-table gate to defer its close when the bot's own capture pipeline blips — but only for failure classes that either report previewPending or throw. A browserless render failure does neither: it is swallowed per-shot and buildCapture returns normally. The gate then reads "no visual evidence, no retry pending" and closes a legitimate visual PR one-shot — precisely the false-close class #9030 exists to prevent, arriving through an unpatched door.

Mechanism (verified at HEAD, 776c414)

src/review/visual/capture.ts:408:

const { png, authWalled } = await captureShot(env, page, viewport, ...).catch(() => ({ png: null, authWalled: false }));

The failure is swallowed per route/viewport and substituted with a non-persisted on-demand URL. Consequently:

• hasSuccessfulBotCapture requires persisted pairs (src/review/visual/capture.ts:112-128) ⇒ false.
• buildCapture returns normally with previewPending: false and throws nothing.
• The orb(gate): screenshot-table gate closes a legitimate visual PR when the capture service blips #9030 retry is scheduled only on previewPending or a thrown error (src/queue/processors.ts:12422-12480) ⇒ never fires, so visualCaptureRetryPendingSha is never set.
• The next maintenance pass reads "not satisfied, no retry pending" (processors.ts:3449-3453) ⇒ screenshotTableMatch matched ⇒ close.

Trigger: the preview URL resolves fine (the deploy succeeded), but browserless is down, saturated, or times out per shot. The author relied on bot capture rather than pasting a hand-built table — which is the intended workflow.

Why the existing guards miss it

#9207 correctly classified "preview still building" and "capture threw" as blips. This third class — preview fine, renderer produced nothing, no exception — was not modelled, because the .catch() at capture.ts:408 converts it into an apparently-successful capture with empty output.

Compounding factor: nothing would alert on it

There is no metric on capture failure. src/review/visual/shot.ts:313-373 closes the browser correctly in a finally, but the failure path is console.log only (render_screenshot_error) with no counter. Readiness probes (src/selfhost/health.ts) cover Redis, Qdrant, the GitHub App and codex — nothing covers BROWSER_WS_ENDPOINT. A browserless outage is therefore invisible in Prometheus while it silently closes contributor PRs. (Live context: the browserless container is at 1.45 GiB / 4 GiB, up 6 days.)

Blast radius

The gate is configured action: close for all three repos in the private server-side config, and was re-enabled two commits before this audit (73d4e94, #9455). The last time this gate mis-fired it auto-closed 5 contributor PRs one-shot (#9414, #9411, #9410, #9409, #9405) — the recovery cost is a fresh PR each, because closes are one-shot by policy.

Requirements

1. "Renderer produced zero real pairs while the preview base resolved" must be treated as blip-equivalent, not as absence of evidence.
2. A browserless outage must be observable before it closes anything.
3. The fix must not suppress genuine "author supplied no visual evidence" closes.

Deliverables

• Distinguish "no evidence because nothing rendered" from "no evidence because none exists": have buildCapture surface a renderFailed (or equivalent) marker when previewBase resolved but zero real pairs were produced for any route.
• On that marker, schedule the same bounded recapture-preview retry as previewPending/thrown errors, so visualCaptureRetryPendingSha is set and the close is deferred. (Coordinate with gate(visual): capture-retry marker never cleared + pending marker skips instead of holds → screenshot gate silently disabled (false merge) #9462, which fixes the marker's lifecycle — do not reintroduce a marker that cannot be cleared.)
• Add loopover_visual_capture_total{result} (or similar) at the render_screenshot_error and success sites — one line, alertable.
• Add an optional readiness/health probe for BROWSER_WS_ENDPOINT when configured, alongside the existing dependency probes in src/selfhost/health.ts.
• Consider a small in-process semaphore over concurrent shots: capture.ts:808-826 fires Promise.all over 4+ simultaneous shots per capture, multiplied by queue concurrency, against one browserless container.

Tests (must fail against current main)

• captureShot rejects for every viewport/route while previewBase resolves ⇒ a retry is scheduled and the close is deferred (assert no close in the plan).
• Retry budget then exhausts with the renderer still failing ⇒ behaviour matches the agreed policy from gate(visual): capture-retry marker never cleared + pending marker skips instead of holds → screenshot gate silently disabled (false merge) #9462 (hold, not close, not merge).
• Genuine no-evidence case (renderer healthy, author supplied nothing, gate violated) ⇒ still closes. Regression guard so this fix does not neuter the gate.
• Metric increments on the failure path.

Expected outcome

A browserless outage degrades the review to "we could not capture evidence — holding", visibly, in metrics. It can no longer convert into one-shot closes of legitimate contributor PRs.

[linear-code]

JSO-6527