From 23b7af63b360fe9243188a1cde44c33903aefd93 Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Mon, 20 Jul 2026 16:58:17 -0700
Subject: [PATCH] fix(review): retry an empty inline files fetch before
 trusting it

fetchAndStorePullRequestFilesForReview could accept a single empty
files response as final, even though GitHub can legitimately return a
clean, successful empty list for a PR whose diff it hasn't finished
computing yet. That empty result trips isGuardrailHit's fail-safe
(empty changed-files -> treat as a guardrail hit), which permanently
locks the PR into a manual-review hold that nothing ever auto-removes.
Retrying once after a short delay converts most of these transient
empty reads into a real diff before the guardrail check ever runs.
---
 src/github/backfill.ts       | 22 +++++++++++++++++++++-
 test/unit/backfill-2.test.ts | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/src/github/backfill.ts b/src/github/backfill.ts
index a6dfb1d4a..02ccec420 100644
--- a/src/github/backfill.ts
+++ b/src/github/backfill.ts
@@ -2416,6 +2416,17 @@ function toPullRequestFileRecordFromGitHub(repoFullName: string, pullNumber: num
   };
 }
 
+// A brand-new or just-pushed PR can have GitHub return a clean, successful EMPTY files list because its diff
+// isn't computed yet — not a rate limit (the shared client in ./client.ts already retries those), so this
+// single short retry is the only thing standing between that timing gap and a permanently-empty diff. An empty
+// changedPaths list is what makes isGuardrailHit fail-safe to "hit" (#1062), and that manual-review hold, once
+// applied, is never auto-removed (agent-actions.ts's #stale-disposition-label-cleanup deliberately leaves
+// manualReview alone — it might be a human's own hold, not just a stale bot one) — so a guardrail-configured
+// repo's PR could otherwise sit "held for manual review" indefinitely over nothing but a fetch timing gap.
+const REVIEW_FILES_EMPTY_RETRY_DELAY_MS = 500;
+
+const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));
+
 /**
  * Inline, best-effort file fetch for the REVIEW path (convergence). The PR-opened webhook can fire the review
  * BEFORE the async detail-sync has populated `pull_request_files`, leaving the AI review + grounding + unified
@@ -2424,6 +2435,10 @@ function toPullRequestFileRecordFromGitHub(repoFullName: string, pullNumber: num
  * detail-sync uses), persist them (so the rest of the same review run + any later read reuse them), and return
  * them mapped to the stored record shape.
  *
+ * An empty first attempt is retried once, after a short delay (see {@link REVIEW_FILES_EMPTY_RETRY_DELAY_MS}),
+ * before being accepted — GitHub's own diff computation lagging a just-created/just-pushed PR is common enough
+ * that a single retry converts most of these into a real diff.
+ *
  * Fail-safe by construction: a fetch failure returns `[]` (never throws), so the review degrades to the same
  * empty-diff state it has today rather than breaking. The persist is best-effort and only runs when the fetch
  * actually returned files (a failed REST+GraphQL fetch must not wipe a row another sync just wrote).
@@ -2436,7 +2451,12 @@ export async function fetchAndStorePullRequestFilesForReview(
   admissionKey?: GitHubRateLimitAdmissionKey,
 ): Promise<PullRequestFileRecord[]> {
   const warnings: string[] = [];
-  const files = await fetchPullRequestFiles(env, repoFullName, pullNumber, token, warnings, admissionKey, "live_review").catch(() => [] as GitHubFilePayload[]);
+  const fetchOnce = () => fetchPullRequestFiles(env, repoFullName, pullNumber, token, warnings, admissionKey, "live_review").catch(() => [] as GitHubFilePayload[]);
+  let files = await fetchOnce();
+  if (files.length === 0) {
+    await sleep(REVIEW_FILES_EMPTY_RETRY_DELAY_MS);
+    files = await fetchOnce();
+  }
   if (files.length === 0) return [];
   const records = files.map((file) => toPullRequestFileRecordFromGitHub(repoFullName, pullNumber, file));
   // Persist so the AI review, grounding, gate, check-run, and unified-comment reads in THIS run (and any later
diff --git a/test/unit/backfill-2.test.ts b/test/unit/backfill-2.test.ts
index 1915c3b2a..70a9f8f00 100644
--- a/test/unit/backfill-2.test.ts
+++ b/test/unit/backfill-2.test.ts
@@ -188,6 +188,42 @@ describe("GitHub backfill", () => {
       vi.stubGlobal("fetch", async () => new Response("boom", { status: 500 }));
       await expect(fetchAndStorePullRequestFilesForReview(env, "JSONbored/gittensory", 99, "public-token")).resolves.toEqual([]);
     });
+
+    it("REGRESSION (#stale-disposition-label-cleanup / guardrail false-positive): retries once and recovers when GitHub's first response is empty because the diff isn't computed yet", async () => {
+      const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+      let filesCalls = 0;
+      vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
+        const url = input.toString();
+        if (url.includes("/pulls/55/files")) {
+          filesCalls += 1;
+          if (filesCalls === 1) return Response.json([]);
+          return Response.json([{ filename: "src/bar.ts", status: "modified", additions: 3, deletions: 1, changes: 4, patch: "@@ -1 +1 @@\n-old\n+new" }]);
+        }
+        return new Response("not found", { status: 404 });
+      });
+
+      const records = await fetchAndStorePullRequestFilesForReview(env, "JSONbored/gittensory", 55, "public-token");
+      expect(filesCalls).toBe(2);
+      expect(records.map((r) => r.path)).toEqual(["src/bar.ts"]);
+      // Persisted from the SECOND (successful) attempt — a subsequent stored read reuses it.
+      expect((await listPullRequestFiles(env, "JSONbored/gittensory", 55)).map((r) => r.path)).toEqual(["src/bar.ts"]);
+    });
+
+    it("retries exactly once, not repeatedly, when both attempts come back empty", async () => {
+      const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+      let filesCalls = 0;
+      vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
+        const url = input.toString();
+        if (url.includes("/pulls/56/files")) {
+          filesCalls += 1;
+          return Response.json([]);
+        }
+        return new Response("not found", { status: 404 });
+      });
+
+      await expect(fetchAndStorePullRequestFilesForReview(env, "JSONbored/gittensory", 56, "public-token")).resolves.toEqual([]);
+      expect(filesCalls).toBe(2);
+    });
   });
 
   describe("fetchLiveCiAggregate", () => {