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

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,
@@ -8902,14 +8902,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,
@@ -8921,23 +8922,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",
@@ -12304,6 +12332,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([]);
+  });
+});