fix(github): self-heal the reviews cache on the periodic sweep, not just webhooks

src/github/backfill.ts

@@ -1975,6 +1975,35 @@ async function backfillRepository(env: Env, repo: RepositoryRecord, limits: Back
   }
 }
 
+// Bounded-age backstop (#2537 second gate pass): the reviewsInvalidatedAt comparison below is EXACT when the
+// invalidation write actually happens, but a silently DROPPED markPullRequestReviewsInvalidated write leaves
+// reviewsInvalidatedAt null forever -- there is then no marker at all to compare against, so the exact
+// comparison alone would read "up to date" indefinitely no matter how long ago reviewsSyncedAt was. This is the
+// only backstop for a signal that was never recorded in the first place; deliberately long so a
+// normally-behaving PR (invalidation writes succeeding) never hits it in practice.
+const REVIEWS_CACHE_MAX_AGE_MS = 48 * 60 * 60 * 1000;
+
+// #2537 follow-up (gate-flagged): a small, pure predicate mirroring fetchAndStorePullRequestDetails's own
+// reviewsUpToDate check below, exported so the periodic re-gate sweep (queue/processors.ts) can independently
+// decide whether a stale reviews cache is, on its own, a reason to force a refresh -- otherwise this row's
+// invalidation state only gets EVALUATED when something ELSE already calls refreshPullRequestDetails, which a
+// "quiet" PR (no new pushes, slop evidence + manifest gate both off, no pre-merge check paths) may never do. A
+// SINGLE authoritative definition (this function) rather than two independently-maintained copies that could
+// drift -- both the exact invalidation-marker comparison AND the bounded-age fallback live here, so a caller
+// that only checks THIS predicate (e.g. the sweep, before deciding whether to even call refreshPullRequestDetails)
+// agrees with fetchAndStorePullRequestDetails's own internal check once that call actually happens.
+export function isReviewsCacheUpToDate(
+  syncState: Pick<PullRequestDetailSyncStateRecord, "reviewsSyncedAt" | "reviewsInvalidatedAt"> | null | undefined,
+): boolean {
+  const reviewsSyncedAt = syncState?.reviewsSyncedAt;
+  if (!reviewsSyncedAt) return false;
+  const invalidationCleared = !syncState?.reviewsInvalidatedAt || reviewsSyncedAt > syncState.reviewsInvalidatedAt;
+  if (!invalidationCleared) return false;
+  const reviewsSyncedAtMs = Date.parse(reviewsSyncedAt);
+  if (!Number.isFinite(reviewsSyncedAtMs)) return false;
+  return Date.now() - reviewsSyncedAtMs < REVIEWS_CACHE_MAX_AGE_MS;
+}
+
 async function fetchAndStorePullRequestDetails(
   env: Env,
   repoFullName: string,
@@ -2007,9 +2036,7 @@ async function fetchAndStorePullRequestDetails(
   // millisecond, and sub-millisecond ordering is unknowable from the stored strings — a tie must fail toward
   // "still needs a refetch," never toward silently trusting a possibly-stale cache.
   const reviewsSyncedAtBefore = existingState?.reviewsSyncedAt;
-  const reviewsUpToDate =
-    Boolean(reviewsSyncedAtBefore) &&
-    (!existingState?.reviewsInvalidatedAt || (reviewsSyncedAtBefore ?? "") > existingState.reviewsInvalidatedAt);
+  const reviewsUpToDate = isReviewsCacheUpToDate(existingState);
   // Gate review finding (TOCTOU race): `existingState` above is a snapshot read at the TOP of this call. If a
   // `pull_request_review` webhook races in AFTER that read but BEFORE this function returns, an unconditional
   // "stamp reviewsSyncedAt to now" on the CALLER's side (the old design) would advance the timestamp PAST that

src/queue/processors.ts

@@ -6,6 +6,7 @@ import {
   getLatestRepoGithubTotalsSnapshot,
   getFreshOfficialMinerDetection,
   getPullRequest,
+  getPullRequestDetailSyncState,
   getRepoAuthorPullRequestHistory,
   getRepository,
   getDecryptedRepositoryAiKey,
@@ -93,6 +94,7 @@ import {
   fetchOpenPullRequestNumbersForCommit,
   fetchRequiredStatusContexts,
   invalidatePrStateCache,
+  isReviewsCacheUpToDate,
   primeDurablePrStateCache,
   refreshContributorActivity,
   refreshInstallationHealth,
@@ -2320,11 +2322,19 @@ async function reReviewStoredPullRequest(
     linkedIssueAuthorLogins,
   });
   await persistAdvisory(env, advisory);
-  if (
+  // #2537 follow-up (gate-flagged): the durable review cache's only invalidation path is markPullRequestReviewsInvalidated
+  // on a webhook (processors.ts). A "quiet" PR (no new pushes, slop evidence + manifest gate both off, no
+  // pre-merge check paths) never hits any of the three reasons below, so a DROPPED invalidation write could sit
+  // stale indefinitely even though this per-PR sweep unit visits every open PR on a bounded cadence.
+  // Short-circuit the extra read when another reason already forces the refresh.
+  const otherRefreshReasons =
     shouldCollectSlopEvidence(settings) ||
     settings.manifestPolicyGateMode !== "off" ||
-    (await shouldRefreshFilesForPreMergeChecks(env, repoFullName))
-  ) {
+    (await shouldRefreshFilesForPreMergeChecks(env, repoFullName));
+  const reviewsCacheStale =
+    !otherRefreshReasons &&
+    !isReviewsCacheUpToDate(await getPullRequestDetailSyncState(env, repoFullName, prNumber).catch(() => null));
+  if (otherRefreshReasons || reviewsCacheStale) {
     await refreshPullRequestDetails(env, repoFullName, prNumber).catch(
       () => undefined,
     );

test/unit/backfill-reviews-cache-scoping.test.ts

@@ -25,6 +25,7 @@ describe("GitHub PR reviews cache scoping (#2537)", () => {
     clearGitHubResponseCacheForTest();
     resetMetrics();
     vi.unstubAllGlobals();
+    vi.useRealTimers();
   });
 
   async function seedRegisteredRepo(env: Env) {
@@ -76,6 +77,10 @@ describe("GitHub PR reviews cache scoping (#2537)", () => {
 
   it("does not re-fetch reviews when reviewsSyncedAt is set and no invalidation has been recorded (cache hit), and leaves stored rows untouched", async () => {
     const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+    // Pin "now" shortly after the seeded reviewsSyncedAt -- within the new bounded-age backstop's
+    // 48h window, so this test's cache-hit assertion isn't defeated by real wall-clock time.
+    vi.useFakeTimers({ toFake: ["Date"] });
+    vi.setSystemTime(new Date("2026-05-20T01:00:00.000Z"));
     await seedRegisteredRepo(env);
     await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
       number: 61,
@@ -115,6 +120,10 @@ describe("GitHub PR reviews cache scoping (#2537)", () => {
 
   it("does not re-fetch reviews when reviewsInvalidatedAt predates reviewsSyncedAt (stale invalidation, still a cache hit)", async () => {
     const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+    // Pin "now" shortly after the seeded reviewsSyncedAt -- within the new bounded-age backstop's
+    // 48h window, so this test's cache-hit assertion isn't defeated by real wall-clock time.
+    vi.useFakeTimers({ toFake: ["Date"] });
+    vi.setSystemTime(new Date("2026-05-20T01:00:00.000Z"));
     await seedRegisteredRepo(env);
     await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
       number: 62,
@@ -141,6 +150,37 @@ describe("GitHub PR reviews cache scoping (#2537)", () => {
     expect(urls.some((url) => url.includes("/pulls/62/reviews"))).toBe(false);
   });
 
+  it("REGRESSION (bounded-age backstop): an unparseable reviewsSyncedAt is treated as stale (NaN branch, miss) rather than throwing", async () => {
+    const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+    await seedRegisteredRepo(env);
+    await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
+      number: 68,
+      title: "Open PR, unparseable review marker",
+      state: "open",
+      user: { login: "oktofeesh1" },
+      head: { sha: "head-68" },
+      labels: [],
+      body: "",
+    });
+    await upsertPullRequestDetailSyncState(env, {
+      repoFullName: "JSONbored/gittensory",
+      pullNumber: 68,
+      status: "complete",
+      headSha: "head-68",
+      reviewsSyncedAt: "not-a-date",
+    });
+    const urls = stubFetchTracking((url) =>
+      url.includes("/pulls/68/reviews")
+        ? Response.json([{ id: 9, user: { login: "reviewer9" }, state: "APPROVED", submitted_at: "2026-05-19T00:00:00.000Z" }])
+        : Response.json([]),
+    );
+
+    const result = await refreshPullRequestDetails(env, "JSONbored/gittensory", 68);
+
+    expect(result).toMatchObject({ status: "complete" });
+    expect(urls.some((url) => url.includes("/pulls/68/reviews"))).toBe(true);
+  });
+
   it("re-fetches reviews on the next sync after markPullRequestReviewsInvalidated bumps reviewsInvalidatedAt past reviewsSyncedAt (cache invalidation)", async () => {
     const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
     await seedRegisteredRepo(env);
@@ -283,6 +323,10 @@ describe("GitHub PR reviews cache scoping (#2537)", () => {
 
   it("does not treat a FILES-only failure as a reason to re-fetch reviews (only a review-specific failure forces a retry)", async () => {
     const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+    // Pin "now" shortly after the seeded reviewsSyncedAt -- within the new bounded-age backstop's
+    // 48h window, so this test's cache-hit assertion isn't defeated by real wall-clock time.
+    vi.useFakeTimers({ toFake: ["Date"] });
+    vi.setSystemTime(new Date("2026-05-20T01:00:00.000Z"));
     await seedRegisteredRepo(env);
     await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
       number: 66,
@@ -323,6 +367,10 @@ describe("GitHub PR reviews cache scoping (#2537)", () => {
 
   it("REGRESSION: a head SHA change alone does not invalidate cached reviews (reviews are independent of the head, unlike files)", async () => {
     const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+    // Pin "now" shortly after the seeded reviewsSyncedAt -- within the new bounded-age backstop's
+    // 48h window, so this test's cache-hit assertion isn't defeated by real wall-clock time.
+    vi.useFakeTimers({ toFake: ["Date"] });
+    vi.setSystemTime(new Date("2026-05-20T01:00:00.000Z"));
     await seedRegisteredRepo(env);
     await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
       number: 64,
@@ -373,6 +421,10 @@ describe("GitHub PR reviews cache scoping (#2537)", () => {
 
   it("REGRESSION (gate finding): a manual force-files refresh does not also force an unrelated reviews refetch", async () => {
     const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+    // Pin "now" shortly after the seeded reviewsSyncedAt -- within the new bounded-age backstop's
+    // 48h window, so this test's cache-hit assertion isn't defeated by real wall-clock time.
+    vi.useFakeTimers({ toFake: ["Date"] });
+    vi.setSystemTime(new Date("2026-05-20T01:00:00.000Z"));
     await seedRegisteredRepo(env);
     await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
       number: 67,