fix(queue): surface a finding when capture-unobtainable degrades the screenshot-table gate (#10256)

maybeAddScreenshotTableAdvisoryFinding early-returned on any action other than
"advisory", so a close/block gate degraded by the #9881 capture-unobtainable
check never appended a finding anywhere -- the close/hold comment that would
have said so doesn't fire on the degraded path either, leaving the maintainer
with no visibility that their gate is unsatisfiable. Thread captureUnobtainable
into the function and its evaluateScreenshotTableGate call, and only early-
return on a non-advisory action once enforcement wasn't degraded.

Co-authored-by: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>

Author: bitfathers94

src/queue/processors.ts

@@ -659,7 +659,7 @@ import {
 } from "../review/linked-issue-hard-rules";
 import { DEFAULT_UNLINKED_ISSUE_GUARDRAIL } from "../review/unlinked-issue-guardrail-config";
 import { resolveUnlinkedIssueMatchDisposition } from "../review/unlinked-issue-guardrail";
-import { DEFAULT_SCREENSHOT_CONTRACT_MESSAGE, DEFAULT_SCREENSHOT_TABLE_GATE, evaluateScreenshotTableGate, extractTableRowImageUrls, type ScreenshotTableGateConfig } from "../review/screenshot-table-gate";
+import { CAPTURE_UNOBTAINABLE_REASON, DEFAULT_SCREENSHOT_CONTRACT_MESSAGE, DEFAULT_SCREENSHOT_TABLE_GATE, evaluateScreenshotTableGate, extractTableRowImageUrls, type ScreenshotTableGateConfig } from "../review/screenshot-table-gate";
 import { isSafeHttpUrl } from "../review/content-lane/safe-url";
 import {
   buildScreenshotTableVisionFindings,
@@ -8928,14 +8928,15 @@ export async function maybeAddLockfileTamperFinding(
 }
 
 /**
- * Screenshot-table gate advisory visibility (#2006 follow-up). `action: "close"` already communicates via its
- * own templated close comment (see planAgentMaintenanceActions/screenshotTableCloseMessage), so a violation
- * there never needs a SEPARATE advisory finding -- this only ever fires for `action: "advisory"`, which
- * previously had NO visible effect at all: the live gate's only other `evaluateScreenshotTableGate` call site
- * (`runAgentMaintenancePlanAndExecute`) discards the result entirely once `action !== "close"`. Mirrors
- * `maybeAddLockfileTamperFinding` immediately above: off/out-of-scope is free, a violation appends ONE
- * warning-severity, non-blocking finding (unrecognized by `isConfiguredGateBlocker`, so it can never gate),
- * and any evaluation error is swallowed so it can never destabilize the gate.
+ * Screenshot-table gate advisory visibility (#2006 follow-up, #9881 degrade follow-up). `action: "close"`/
+ * `"block"` already communicate via their own templated close/hold comment (see
+ * planAgentMaintenanceActions/screenshotTableCloseMessage), so a violation there never needs a SEPARATE
+ * advisory finding UNLESS enforcement was degraded (#9881: the bot proved this repo's preview pipeline can
+ * never satisfy the gate) -- in that case the close/hold comment never fires either, so THIS finding is the
+ * only place a maintainer ever learns the gate is unsatisfiable here (#10060). Mirrors
+ * `maybeAddLockfileTamperFinding` immediately above: off is free, a violation appends ONE warning-severity,
+ * non-blocking finding (unrecognized by `isConfiguredGateBlocker`, so it can never gate), and any evaluation
+ * error is swallowed so it can never destabilize the gate.
  */
 export async function maybeAddScreenshotTableAdvisoryFinding(
   env: Env,
@@ -8947,23 +8948,50 @@ export async function maybeAddScreenshotTableAdvisoryFinding(
     prBody: string | null | undefined;
     prLabels: string[];
     botCaptureSatisfied: boolean;
+    // #9881/#10060: true when the bot proved this repo's preview pipeline can never produce a capture for
+    // this head -- threaded from the SAME `Boolean(pr.headSha) && pr.visualCaptureUnobtainableSha ===
+    // pr.headSha` expression the enforcement call site (runAgentMaintenancePlanAndExecute) computes, so the
+    // two evaluations of this pure check can never disagree about whether this PR's gate is degraded.
+    captureUnobtainable: boolean;
     files: Awaited<ReturnType<typeof listPullRequestFiles>> | null;
   },
 ): Promise<void> {
-  if (!args.screenshotTableGateConfig.enabled || args.screenshotTableGateConfig.action !== "advisory") return;
+  if (!args.screenshotTableGateConfig.enabled) return;
   try {
-    const files =
-      args.files ??
-      (await listPullRequestFiles(env, args.repoFullName, args.pullNumber));
+    // #10060: an if-fallback, not `args.files ?? (await listPullRequestFiles(...))` -- that shape left the
+    // statements immediately following it (the gate evaluation, the violated check) with a phantom 0 lcov hit
+    // count despite genuinely running every test, which would have sunk this file's Codecov patch coverage.
+    let files = args.files;
+    if (files === null) {
+      files = await listPullRequestFiles(env, args.repoFullName, args.pullNumber);
+    }
+    const changedFiles = files.map((file) => file.path);
     const result = evaluateScreenshotTableGate({
       config: args.screenshotTableGateConfig,
       prBody: args.prBody,
       prLabels: args.prLabels,
-      changedFiles: files.map((file) => file.path),
+      changedFiles,
       botCaptureSatisfied: args.botCaptureSatisfied,
+      captureUnobtainable: args.captureUnobtainable,
     });
     if (!result.violated) return;
     const detail = result.reason ?? DEFAULT_SCREENSHOT_CONTRACT_MESSAGE;
+    // #10060: a degraded gate must surface here REGARDLESS of the configured action -- close/block never get
+    // their own comment on this path (the enforcement that would have produced one was degraded away), so an
+    // advisory-mode repo and a close-mode repo with an unsatisfiable pipeline both need this same visibility.
+    if (result.enforcementDegradedReason !== undefined) {
+      const degradedDetail = `${detail}\n\n${CAPTURE_UNOBTAINABLE_REASON}`;
+      args.advisory.findings.push({
+        code: "screenshot_table_missing",
+        severity: "warning",
+        title: "Screenshot-table enforcement degraded (capture unobtainable)",
+        detail: degradedDetail,
+        action: "Enable preview deploys for this repository, or set requireScreenshotTable.action to advisory.",
+        publicText: degradedDetail,
+      });
+      return;
+    }
+    if (args.screenshotTableGateConfig.action !== "advisory") return;
     args.advisory.findings.push({
       code: "screenshot_table_missing",
       severity: "warning",
@@ -12368,6 +12396,9 @@ async function maybePublishPrPublicSurface(
       prBody: pr.body,
       prLabels: pr.labels,
       botCaptureSatisfied: Boolean(pr.headSha) && pr.visualCaptureSatisfiedSha === pr.headSha,
+      // #9881/#10060: same expression runAgentMaintenancePlanAndExecute computes for the enforcement decision,
+      // so the two evaluations of this pure check cannot disagree about whether this PR's gate is degraded.
+      captureUnobtainable: Boolean(pr.headSha) && pr.visualCaptureUnobtainableSha === pr.headSha,
       files: await getReviewFiles(),
     });
 

test/unit/screenshot-table-gate.test.ts

@@ -17,7 +17,11 @@ import {
   CAPTURE_UNOBTAINABLE_REASON,
   type ScreenshotMatrixPair,
 } from "../../src/review/screenshot-table-gate";
-import type { ScreenshotTableGateConfig } from "../../src/types";
+import type { Advisory, PullRequestFileRecord, ScreenshotTableGateConfig } from "../../src/types";
+import { maybeAddScreenshotTableAdvisoryFinding } from "../../src/queue/processors";
+import { planAgentMaintenanceActions, type AgentActionPlanInput } from "../../src/settings/agent-actions";
+import type { GateCheckConclusion } from "../../src/rules/advisory";
+import { createTestEnv } from "../helpers/d1";
 
 function config(overrides: Partial<ScreenshotTableGateConfig> = {}): ScreenshotTableGateConfig {
   return { ...DEFAULT_SCREENSHOT_TABLE_GATE, whenLabels: [], whenPaths: [], ...overrides };
@@ -999,3 +1003,220 @@ describe("enforcement degrade when capture is unobtainable (#9881)", () => {
     expect(before).toEqual(evaluateScreenshotTableGate({ ...violatingInput, captureUnobtainable: undefined }));
   });
 });
+
+// #10060: maybeAddScreenshotTableAdvisoryFinding previously early-returned before evaluating anything unless
+// `action === "advisory"`, so a `close`/`block` repo whose gate was degraded (#9881) never surfaced a finding
+// anywhere — the close/hold comment that would have said so never fires on the degraded path either, so the
+// maintainer learned nothing. These tests pin the fixed wiring.
+describe("maybeAddScreenshotTableAdvisoryFinding degrade wiring (#10060)", () => {
+  function advisory(): Advisory {
+    return {
+      id: "adv-1",
+      targetType: "pull_request",
+      repoFullName: "acme/widgets",
+      pullNumber: 7,
+      targetKey: "acme/widgets#7",
+      headSha: "sha7",
+      conclusion: "neutral",
+      severity: "info",
+      title: "LoopOver advisory available",
+      summary: "ok",
+      findings: [],
+      generatedAt: "2026-07-31T00:00:00.000Z",
+    };
+  }
+
+  const NO_TABLE_FILES: PullRequestFileRecord[] = [
+    { repoFullName: "acme/widgets", pullNumber: 7, path: "src/app.tsx", status: "modified", additions: 1, deletions: 0, changes: 1, payload: {} },
+  ];
+
+  function gateConfig(action: "close" | "block" | "advisory"): ScreenshotTableGateConfig {
+    return { ...DEFAULT_SCREENSHOT_TABLE_GATE, enabled: true, whenLabels: [], whenPaths: [], action };
+  }
+
+  it("action: close, violated, captureUnobtainable: true — appends exactly one finding naming the remedy, and the same inputs plan no close/hold", async () => {
+    const env = createTestEnv();
+    const adv = advisory();
+    await maybeAddScreenshotTableAdvisoryFinding(env, {
+      advisory: adv,
+      repoFullName: "acme/widgets",
+      pullNumber: 7,
+      screenshotTableGateConfig: gateConfig("close"),
+      prBody: "no table here",
+      prLabels: [],
+      botCaptureSatisfied: false,
+      captureUnobtainable: true,
+      files: NO_TABLE_FILES,
+    });
+    expect(adv.findings).toHaveLength(1);
+    expect(adv.findings[0]?.code).toBe("screenshot_table_missing");
+    expect(adv.findings[0]?.detail).toContain(CAPTURE_UNOBTAINABLE_REASON);
+    expect(adv.findings[0]?.publicText).toContain(CAPTURE_UNOBTAINABLE_REASON);
+
+    // The same degraded facts the real caller threads through (processors.ts): screenshotTableMatch and
+    // screenshotEvidenceHold both stay absent, and screenshotTableEvidenceUnresolved stays false, so the
+    // planner falls through to ordinary disposition instead of closing or holding.
+    const plan = planAgentMaintenanceActions({
+      blockerTitles: [],
+      autonomy: { merge: "auto", close: "auto", review_state_label: "auto" },
+      autoMaintain: { requireApprovals: 1, mergeMethod: "squash" },
+      slopGateMinScore: 60,
+      changedPaths: [],
+      hardGuardrailGlobs: [],
+      authorIsOwner: false,
+      authorIsAdmin: false,
+      authorIsAutomationBot: false,
+      ciState: "passed",
+      conclusion: "success" as GateCheckConclusion,
+      manualReviewLabel: "human-review",
+      screenshotTableMatch: undefined,
+      screenshotEvidenceHold: undefined,
+      screenshotTableEvidenceUnresolved: false,
+      pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" },
+    } satisfies AgentActionPlanInput);
+    expect(plan.some((a) => a.actionClass === "close")).toBe(false);
+    expect(plan.some((a) => a.actionClass === "label" && a.label === "human-review" && a.labelOp !== "remove")).toBe(false);
+  });
+
+  it("action: block, violated, captureUnobtainable: true — appends the same degraded finding, and no hold is planned", async () => {
+    const env = createTestEnv();
+    const adv = advisory();
+    await maybeAddScreenshotTableAdvisoryFinding(env, {
+      advisory: adv,
+      repoFullName: "acme/widgets",
+      pullNumber: 7,
+      screenshotTableGateConfig: gateConfig("block"),
+      prBody: "no table here",
+      prLabels: [],
+      botCaptureSatisfied: false,
+      captureUnobtainable: true,
+      files: NO_TABLE_FILES,
+    });
+    expect(adv.findings).toHaveLength(1);
+    expect(adv.findings[0]?.detail).toContain(CAPTURE_UNOBTAINABLE_REASON);
+
+    const plan = planAgentMaintenanceActions({
+      blockerTitles: [],
+      autonomy: { merge: "auto", review_state_label: "auto" },
+      autoMaintain: { requireApprovals: 1, mergeMethod: "squash" },
+      slopGateMinScore: 60,
+      changedPaths: [],
+      hardGuardrailGlobs: [],
+      authorIsOwner: false,
+      authorIsAdmin: false,
+      authorIsAutomationBot: false,
+      ciState: "passed",
+      conclusion: "success" as GateCheckConclusion,
+      manualReviewLabel: "human-review",
+      screenshotTableMatch: undefined,
+      screenshotEvidenceHold: undefined,
+      screenshotTableEvidenceUnresolved: false,
+      pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" },
+    } satisfies AgentActionPlanInput);
+    expect(plan.some((a) => a.actionClass === "label" && a.label === "human-review" && a.labelOp !== "remove")).toBe(false);
+    expect(plan.some((a) => a.actionClass === "close")).toBe(false);
+  });
+
+  it("action: close, violated, captureUnobtainable: false — pins today's behavior: NO advisory finding", async () => {
+    const env = createTestEnv();
+    const adv = advisory();
+    await maybeAddScreenshotTableAdvisoryFinding(env, {
+      advisory: adv,
+      repoFullName: "acme/widgets",
+      pullNumber: 7,
+      screenshotTableGateConfig: gateConfig("close"),
+      prBody: "no table here",
+      prLabels: [],
+      botCaptureSatisfied: false,
+      captureUnobtainable: false,
+      files: NO_TABLE_FILES,
+    });
+    expect(adv.findings).toEqual([]);
+  });
+
+  it("action: advisory, violated, captureUnobtainable: false — byte-identical finding to today", async () => {
+    const env = createTestEnv();
+    const adv = advisory();
+    await maybeAddScreenshotTableAdvisoryFinding(env, {
+      advisory: adv,
+      repoFullName: "acme/widgets",
+      pullNumber: 7,
+      screenshotTableGateConfig: gateConfig("advisory"),
+      prBody: "no table here",
+      prLabels: [],
+      botCaptureSatisfied: false,
+      captureUnobtainable: false,
+      files: NO_TABLE_FILES,
+    });
+    expect(adv.findings).toHaveLength(1);
+    expect(adv.findings[0]).toMatchObject({
+      code: "screenshot_table_missing",
+      severity: "warning",
+      title: "Missing before/after screenshot table",
+      action: "Add a before/after screenshot table to the pull request description (advisory only — this does not block merge).",
+    });
+    expect(adv.findings[0]?.detail).not.toContain(CAPTURE_UNOBTAINABLE_REASON);
+  });
+
+  it("REGRESSION (#10060): a degraded action: close evaluation never produces a completely silent pass", async () => {
+    const env = createTestEnv();
+    const adv = advisory();
+    await maybeAddScreenshotTableAdvisoryFinding(env, {
+      advisory: adv,
+      repoFullName: "acme/widgets",
+      pullNumber: 7,
+      screenshotTableGateConfig: gateConfig("close"),
+      prBody: "no table here",
+      prLabels: [],
+      botCaptureSatisfied: false,
+      captureUnobtainable: true,
+      files: NO_TABLE_FILES,
+    });
+    expect(adv.findings.length).toBeGreaterThan(0);
+  });
+
+  it("not enabled: does not scan, no finding appended even when captureUnobtainable is true", async () => {
+    const env = createTestEnv();
+    const adv = advisory();
+    await maybeAddScreenshotTableAdvisoryFinding(env, {
+      advisory: adv,
+      repoFullName: "acme/widgets",
+      pullNumber: 7,
+      screenshotTableGateConfig: { ...gateConfig("close"), enabled: false },
+      prBody: "no table here",
+      prLabels: [],
+      botCaptureSatisfied: false,
+      captureUnobtainable: true,
+      files: NO_TABLE_FILES,
+    });
+    expect(adv.findings).toEqual([]);
+  });
+
+  it("fail-safe: a thrown error while loading files never propagates and appends no finding", async () => {
+    const env = createTestEnv();
+    const adv = advisory();
+    const throwingEnv = {
+      ...env,
+      DB: {
+        ...env.DB,
+        prepare: () => {
+          throw new Error("boom");
+        },
+      },
+    } as unknown as typeof env;
+    await expect(
+      maybeAddScreenshotTableAdvisoryFinding(throwingEnv, {
+        advisory: adv,
+        repoFullName: "acme/widgets",
+        pullNumber: 7,
+        screenshotTableGateConfig: gateConfig("close"),
+        prBody: "no table here",
+        prLabels: [],
+        botCaptureSatisfied: false,
+        captureUnobtainable: true,
+        files: null,
+      }),
+    ).resolves.toBeUndefined();
+    expect(adv.findings).toEqual([]);
+  });
+});