fix(gate): defer the screenshot-table close on a capture-pipeline blip instead of treating it as missing evidence

migrations/0190_pull_request_visual_capture_retry_pending_sha.sql

@@ -0,0 +1,14 @@
+-- Screenshot-table gate false-positive close (#9030). Visual capture (`review.visual.enabled`) can fail to
+-- reach a conclusive result for reasons that have NOTHING to do with whether the PR actually needs
+-- before/after evidence: the capture pipeline can throw (browserless down, timeout, a GitHub API hiccup
+-- fetching a token) or the preview deploy can still be building (previewPending). Before this column existed,
+-- both looked identical to "capture concluded normally and found no visual evidence" -- the very next
+-- maintenance pass could then close the PR under the screenshotTableGate purely because an internal service
+-- blipped, with no distinction and no grace period.
+--
+-- visual_capture_retry_pending_sha is the head SHA a bounded recapture retry is currently scheduled/in-flight
+-- for (see MAX_PREVIEW_POLL_ATTEMPTS) -- set ONLY when that retry was actually scheduled (budget not yet
+-- exhausted), so the screenshotTableGate's CLOSE action defers for exactly as long as a retry chance remains,
+-- never indefinitely. Scoped to head SHA (mirrors visual_capture_satisfied_sha, 0125) so a new commit re-arms
+-- it; cleared by a subsequent successful capture for the same head.
+ALTER TABLE pull_requests ADD COLUMN visual_capture_retry_pending_sha TEXT;

src/db/repositories.ts

@@ -4417,7 +4417,25 @@ export async function markPullRequestVisualCaptureSatisfied(env: Env, fullName:
   const db = getDb(env.DB);
   await db
     .update(pullRequests)
-    .set({ visualCaptureSatisfiedSha: headSha, updatedAt: nowIso() })
+    // #9030: a proven-successful capture for this head supersedes any earlier "retry pending" marker recorded
+    // for the SAME head (an error or a still-building preview on an earlier attempt) -- clearing it here keeps
+    // the row's state minimal instead of leaving a now-moot marker sitting alongside a satisfied one.
+    .set({ visualCaptureSatisfiedSha: headSha, visualCaptureRetryPendingSha: null, updatedAt: nowIso() })
+    .where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number), eq(pullRequests.headSha, headSha)));
+}
+
+/** False-positive close guard (#9030): record that a bounded visual-capture recapture retry is currently
+ *  scheduled/in-flight for `headSha` -- called ONLY when the capture pipeline errored, or the preview is still
+ *  building, AND a retry budget attempt remains (see MAX_PREVIEW_POLL_ATTEMPTS at the call site). While this
+ *  equals the PR's current headSha, the screenshotTableGate's CLOSE action defers instead of treating the
+ *  transient blip as missing evidence. Scoped to headSha (mirrors markPullRequestVisualCaptureSatisfied) so a
+ *  later commit re-arms the requirement; superseded by a later successful capture for the same head (see that
+ *  function's own clearing write above). */
+export async function markPullRequestVisualCaptureRetryPending(env: Env, fullName: string, number: number, headSha: string): Promise<void> {
+  const db = getDb(env.DB);
+  await db
+    .update(pullRequests)
+    .set({ visualCaptureRetryPendingSha: headSha, updatedAt: nowIso() })
     .where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number), eq(pullRequests.headSha, headSha)));
 }
 
@@ -7007,6 +7025,7 @@ function toPullRequestRecordFromRow(row: typeof pullRequests.$inferSelect): Pull
     linkedIssueHardRuleViolationIssues: parseJson<number[]>(row.linkedIssueHardRuleViolationIssuesJson, []),
     linkedIssueHardRuleViolationReason: row.linkedIssueHardRuleViolationReason,
     visualCaptureSatisfiedSha: row.visualCaptureSatisfiedSha,
+    visualCaptureRetryPendingSha: row.visualCaptureRetryPendingSha,
     screenshotTablePresenceSatisfied: parseJson<{ headSha: string; evidenceFingerprint: string } | null>(row.screenshotTablePresenceSatisfiedJson, null),
   };
 }

src/db/schema.ts

@@ -450,6 +450,13 @@ export const pullRequests = sqliteTable(
     // new head. loopover-computed (publish-written), omitted from the GitHub-sync SET clause so a later sync
     // cannot clobber it.
     visualCaptureSatisfiedSha: text("visual_capture_satisfied_sha"),
+    // False-positive close guard (#9030): the head SHA a bounded visual-capture recapture retry is currently
+    // scheduled/in-flight for -- set ONLY when the capture pipeline errored or the preview is still building
+    // AND a retry budget attempt remains (MAX_PREVIEW_POLL_ATTEMPTS). While set for the PR's current head, the
+    // screenshotTableGate's CLOSE action defers instead of treating the transient blip as "no visual evidence
+    // provided". Cleared by a subsequent successful capture for the same head. loopover-computed
+    // (publish-written), omitted from the GitHub-sync SET clause so a later sync cannot clobber it.
+    visualCaptureRetryPendingSha: text("visual_capture_retry_pending_sha"),
     // Screenshot-table PRESENCE-mode staleness correlation (#stale-screenshot-table-fix, follow-up to #2006).
     // JSON `{headSha, evidenceFingerprint}` -- the head SHA and before/after-image-URL fingerprint that last
     // satisfied screenshotTableGate's presence-mode check (see evaluateScreenshotTableGate's staleness comment).

src/queue/processors.ts

@@ -56,6 +56,7 @@ import {
   markPullRequestReviewsInvalidated,
   markPullRequestSurfacePublished,
   markPullRequestVisualCaptureSatisfied,
+  markPullRequestVisualCaptureRetryPending,
   markPullRequestScreenshotTablePresenceSatisfied,
   getLatestRegatedAt,
   getLatestBacklogConvergenceRegatedAt,
@@ -3239,10 +3240,29 @@ async function runAgentMaintenancePlanAndExecute(
     headSha: pr.headSha,
     presenceModeSatisfied: pr.screenshotTablePresenceSatisfied,
   });
+  // #9030: a visual-capture pipeline ERROR (browserless down, timeout, GitHub hiccup) or a still-building
+  // preview looked IDENTICAL to "capture concluded normally, no visual evidence found" -- both left
+  // visualCaptureSatisfiedSha unset, and the very next maintenance pass could close a legitimate visual PR
+  // purely because an internal service blipped. visualCaptureRetryPendingSha (set only while a bounded
+  // recapture retry is genuinely still scheduled for this exact head -- see the capture block in
+  // maybePublishPrPublicSurface) defers the CLOSE for exactly as long as that retry chance remains; once the
+  // budget is exhausted, the marker is never set again and the gate falls through to its normal, accurate
+  // evaluation on the final attempt -- this can never hold a PR forever.
+  const botCaptureRetryPending = Boolean(pr.headSha) && pr.visualCaptureRetryPendingSha === pr.headSha;
   const screenshotTableMatch =
-    screenshotTableGateResult.violated && screenshotTableGateConfig.action === "close"
+    screenshotTableGateResult.violated && screenshotTableGateConfig.action === "close" && !botCaptureRetryPending
       ? { matched: true, reason: screenshotTableGateResult.reason }
       : undefined;
+  if (screenshotTableGateResult.violated && screenshotTableGateConfig.action === "close" && botCaptureRetryPending) {
+    await recordAuditEvent(env, {
+      eventType: "github_app.screenshot_table_close_deferred_capture_retry",
+      actor: null,
+      targetKey: `${repoFullName}#${pr.number}`,
+      outcome: "queued",
+      detail: "Screenshot-table gate would have closed this PR, but the bot's own visual-capture pipeline has a bounded retry still pending for this head -- deferring the close instead of treating the blip as missing evidence",
+      metadata: { deliveryId, repoFullName, headSha: pr.headSha ?? null },
+    }).catch(() => undefined);
+  }
   // #stale-screenshot-table-fix / #8866: presence or matrix mode just independently re-confirmed the gate for
   // THIS head SHA -- persist the (headSha, evidenceFingerprint) checkpoint so a LATER push that carries the
   // SAME UNCHANGED evidence correctly re-violates instead of silently staying green forever (see
@@ -9174,6 +9194,59 @@ async function logTypeLabelSkip(env: Env, repoFullName: string, pullNumber: numb
   }).catch(() => undefined);
 }
 
+/** False-positive close guard (#9030): schedule the SAME bounded self-heal `recapture-preview` retry for both
+ *  "the preview deploy is still building" (capture.previewPending) and "the capture pipeline itself errored"
+ *  (browserless down, timeout, a GitHub hiccup) -- neither means "this PR genuinely has no visual evidence",
+ *  so neither should let the screenshotTableGate treat it that way. Persists visualCaptureRetryPendingSha for
+ *  the current head ONLY when a retry was actually scheduled (the budget is not yet exhausted) -- once
+ *  MAX_PREVIEW_POLL_ATTEMPTS is reached, the marker is deliberately left unset so the gate falls through to its
+ *  normal (accurate) evaluation on this final attempt rather than holding the PR open forever. Best-effort:
+ *  either write failing only means this ONE recovery chance is silently missed, never a crash. */
+async function scheduleVisualCaptureRetry(
+  env: Env,
+  args: {
+    webhook: { deliveryId: string };
+    repoFullName: string;
+    pr: { number: number; headSha?: string | null | undefined };
+    installationId: number;
+    previewPollAttempt: number;
+  },
+): Promise<void> {
+  if (args.previewPollAttempt >= MAX_PREVIEW_POLL_ATTEMPTS) return;
+  if (args.pr.headSha) {
+    await markPullRequestVisualCaptureRetryPending(env, args.repoFullName, args.pr.number, args.pr.headSha).catch((error) => {
+      console.log(
+        JSON.stringify({
+          event: "visual_capture_retry_pending_mark_failed",
+          repoFullName: args.repoFullName,
+          pull: args.pr.number,
+          message: errorMessage(error).slice(0, 200),
+        }),
+      );
+    });
+  }
+  await env.JOBS.send(
+    {
+      type: "recapture-preview",
+      deliveryId: args.webhook.deliveryId,
+      repoFullName: args.repoFullName,
+      prNumber: args.pr.number,
+      installationId: args.installationId,
+      attempt: args.previewPollAttempt + 1,
+    },
+    { delaySeconds: PREVIEW_POLL_SECONDS },
+  ).catch((error) =>
+    console.log(
+      JSON.stringify({
+        event: "recapture_enqueue_failed",
+        repoFullName: args.repoFullName,
+        pull: args.pr.number,
+        message: errorMessage(error).slice(0, 120),
+      }),
+    ),
+  );
+}
+
 async function maybePublishPrPublicSurface(
   env: Env,
   installationId: number,
@@ -12106,30 +12179,14 @@ async function maybePublishPrPublicSurface(
           // the now-ready shot — bounded by `attempt` so a never-resolving preview can't loop (the deployment_status
           // webhook also refills it; this is the backstop when that event is missed/late).
           const previewPollAttempt = webhook.previewPollAttempt ?? 0;
-          if (
-            capture.previewPending &&
-            previewPollAttempt < MAX_PREVIEW_POLL_ATTEMPTS
-          ) {
-            await env.JOBS.send(
-              {
-                type: "recapture-preview",
-                deliveryId: webhook.deliveryId,
-                repoFullName,
-                prNumber: pr.number,
-                installationId,
-                attempt: previewPollAttempt + 1,
-              },
-              { delaySeconds: PREVIEW_POLL_SECONDS },
-            ).catch((error) =>
-              console.log(
-                JSON.stringify({
-                  event: "recapture_enqueue_failed",
-                  repoFullName,
-                  pull: pr.number,
-                  message: errorMessage(error).slice(0, 120),
-                }),
-              ),
-            );
+          if (capture.previewPending) {
+            await scheduleVisualCaptureRetry(env, {
+              webhook,
+              repoFullName,
+              pr,
+              installationId,
+              previewPollAttempt,
+            });
           }
         } catch (error) {
           console.log(
@@ -12140,6 +12197,17 @@ async function maybePublishPrPublicSurface(
               message: errorMessage(error).slice(0, 200),
             }),
           );
+          // #9030: a capture-pipeline ERROR (browserless down, timeout, a GitHub hiccup fetching a token) must
+          // not be silently indistinguishable from a legitimate "no visual routes found" result -- the
+          // screenshotTableGate's CLOSE action would otherwise fire on a false positive purely because an
+          // internal service blipped. Schedule the SAME bounded self-heal retry previewPending already uses.
+          await scheduleVisualCaptureRetry(env, {
+            webhook,
+            repoFullName,
+            pr,
+            installationId,
+            previewPollAttempt: webhook.previewPollAttempt ?? 0,
+          });
         }
       }
       // AI-vision analysis of a confirmed visual regression (#4111 wiring) — see runVisualVisionForAdvisory's

src/types.ts

@@ -741,6 +741,12 @@ export type PullRequestRecord = {
    *  screenshotTableGate treats visualCaptureSatisfiedSha === headSha as evidence equivalent to a hand-authored
    *  before/after table. Publish-written; read straight from the row. */
   visualCaptureSatisfiedSha?: string | null | undefined;
+  /** False-positive close guard (#9030): the head SHA a bounded visual-capture recapture retry is currently
+   *  scheduled/in-flight for -- set only when the capture pipeline errored, or the preview is still building,
+   *  AND a retry budget attempt remains. While this equals the PR's current headSha, the screenshotTableGate's
+   *  CLOSE action defers instead of treating the transient blip as missing evidence. Publish-written; read
+   *  straight from the row. */
+  visualCaptureRetryPendingSha?: string | null | undefined;
   /** Screenshot-table PRESENCE-mode staleness correlation (#stale-screenshot-table-fix): the (headSha,
    *  evidenceFingerprint) checkpoint the screenshotTableGate's presence-mode check last satisfied for this PR
    *  (see evaluateScreenshotTableGate's staleness comment). `null`/absent = presence mode has never satisfied