feat(github): cache bare PR reads and reviews, mirroring the head-SHA sync-state pattern

Closes #2537.

GET /pulls/{n} (PR state/mergeable_state) and GET /pulls/{n}/reviews were the
largest and third-largest GitHub REST contributors observed in a live
rate-limit trough, and neither was covered by the earlier PR-files cache
(#audit-rate-headroom / #2527). The bare-PR-state read was implemented as
four near-identical helpers with no caching or coalescing, called from
readiness checks, the maintenance planner, duplicate-sibling reconciliation,
and a gate-override command; reviews were explicitly left uncached ("more
volatile than files" -- true at a fixed head, but reviews only actually
change on a pull_request_review webhook).

Extends the existing pull_request_detail_sync_state table (migration 0093)
with prMergeableState/prState/prStateFetchedAt, and reuses the already-present
reviewsSyncedAt column as the review cache's freshness marker. PR-state is
event-invalidated on pull_request synchronize/closed/reopened (with a 5-minute
safety-net TTL so a missed webhook self-heals within one sweep tick); reviews
are invalidated on pull_request_review submitted/edited/dismissed rather than
on head-SHA change, since reviews are independent of the head.

The pre-merge verdict thread and its unified-comment mirror (the #4220
act-boundary) are deliberately left routed through the raw, uncached fetch --
they must always force-refetch live, since the stored/cached mergeable_state
lagging GitHub's async recompute is exactly what caused #4220. This is
verified by two dedicated regression tests plus direct mutation testing
(temporarily rerouting the act-boundary through the cache and confirming both
tests -- and no others in the suite -- catch it).

Author: JSONbored

migrations/0093_pull_request_detail_sync_pr_state.sql

@@ -0,0 +1,8 @@
+ALTER TABLE pull_request_detail_sync_state
+  ADD COLUMN pr_mergeable_state TEXT;
+
+ALTER TABLE pull_request_detail_sync_state
+  ADD COLUMN pr_state TEXT;
+
+ALTER TABLE pull_request_detail_sync_state
+  ADD COLUMN pr_state_fetched_at TEXT;

src/db/repositories.ts

@@ -1138,8 +1138,10 @@ export async function getRepoQueueTrendSnapshot(env: Env, repoFullName: string):
 // drizzle's `onConflictDoUpdate` strips `undefined` entries from the generated SQL `SET` clause rather than
 // writing NULL. Every "running" pre-fetch stamp (backfill.ts) relies on this to touch only `status` without
 // clearing the PREVIOUS `headSha`/`*SyncedAt` row — including the repo+PR+headSha file cache
-// (#audit-rate-headroom), which would silently stop hitting if a future edit here coalesced an omitted field to
-// `null` (e.g. `headSha: state.headSha ?? null`). Pass `null` explicitly to actually clear a column.
+// (#audit-rate-headroom) and the durable PR-state / review caches (#2537), which would silently stop hitting if a
+// future edit here coalesced an omitted field to `null` (e.g. `headSha: state.headSha ?? null`). Pass `null`
+// explicitly to actually clear a column (this is exactly how webhook invalidation clears prMergeableState/
+// prState/reviewsSyncedAt below).
 export async function upsertPullRequestDetailSyncState(env: Env, state: PullRequestDetailSyncStateRecord): Promise<void> {
   const db = getDb(env.DB);
   await db
@@ -1155,6 +1157,9 @@ export async function upsertPullRequestDetailSyncState(env: Env, state: PullRequ
       checksSyncedAt: state.checksSyncedAt,
       lastSyncedAt: state.lastSyncedAt,
       errorSummary: state.errorSummary,
+      prMergeableState: state.prMergeableState,
+      prState: state.prState,
+      prStateFetchedAt: state.prStateFetchedAt,
       updatedAt: nowIso(),
     })
     .onConflictDoUpdate({
@@ -1167,6 +1172,9 @@ export async function upsertPullRequestDetailSyncState(env: Env, state: PullRequ
         checksSyncedAt: state.checksSyncedAt,
         lastSyncedAt: state.lastSyncedAt,
         errorSummary: state.errorSummary,
+        prMergeableState: state.prMergeableState,
+        prState: state.prState,
+        prStateFetchedAt: state.prStateFetchedAt,
         updatedAt: nowIso(),
       },
     });
@@ -4229,6 +4237,9 @@ function toPullRequestDetailSyncStateRecord(row: typeof pullRequestDetailSyncSta
     checksSyncedAt: row.checksSyncedAt,
     lastSyncedAt: row.lastSyncedAt,
     errorSummary: row.errorSummary,
+    prMergeableState: row.prMergeableState,
+    prState: row.prState,
+    prStateFetchedAt: row.prStateFetchedAt,
     updatedAt: row.updatedAt,
   };
 }

src/db/schema.ts

@@ -234,6 +234,13 @@ export const pullRequestDetailSyncState = sqliteTable(
     checksSyncedAt: text("checks_synced_at"),
     lastSyncedAt: text("last_synced_at"),
     errorSummary: text("error_summary"),
+    // Durable bare-PR-state cache (#2537): mirrors GET /pulls/{n}'s mutable state/mergeable_state, refreshed on
+    // synchronize/closed/reopened webhooks and read by the freshness-guard/readiness/dup-winner/gate-override
+    // call sites that don't need the disposition's own live-recompute guarantee. NEVER read by the act-boundary
+    // merge/close decision (planAgentMaintenanceActions / the unified-comment mirror), which always force-refetches.
+    prMergeableState: text("pr_mergeable_state"),
+    prState: text("pr_state"),
+    prStateFetchedAt: text("pr_state_fetched_at"),
     updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()),
   },
   (table) => ({

src/github/backfill.ts

@@ -75,6 +75,9 @@ import {
   backfillOpenPullRequestDetails,
   backfillRegisteredRepositories,
   backfillRepositorySegment,
+  cachedFetchLivePullRequestHeadSha,
+  cachedFetchLivePullRequestMergeState,
+  cachedFetchLivePullRequestState,
   enqueueRepositoryOpenDataBackfill,
   fetchAndStorePullRequestFilesForReview,
   fetchLinkedIssueFacts,
@@ -90,6 +93,9 @@ import {
   fetchLivePullRequestState,
   fetchOpenPullRequestNumbersForCommit,
   fetchRequiredStatusContexts,
+  invalidatePrReviewsCache,
+  invalidatePrStateCache,
+  primeDurablePrStateCache,
   refreshContributorActivity,
   refreshInstallationHealth,
   refreshPullRequestDetails,
@@ -614,15 +620,24 @@ function cachedLiveMergeState(
   const key = liveFactKey(repoFullName, prNumber, liveFactTokenPart(token));
   const cached = facts.mergeStates.get(key);
   if (cached) return cached;
+  // #2537: on a request-local miss, check the DURABLE cross-webhook cache before hitting GitHub — this is the
+  // readiness/freshness-guard path, not the act-boundary disposition (that's refreshLiveMergeState below, which
+  // NEVER routes through the durable cache). A durable hit is itself memoized request-locally for the rest of
+  // this pass via facts.mergeStates, same as a live fetch would be.
   const next = evictLiveFactOnReject(
     facts.mergeStates,
     key,
-    fetchLivePullRequestMergeState(env, repoFullName, prNumber, token, admissionKey),
+    cachedFetchLivePullRequestMergeState(env, repoFullName, prNumber, token, admissionKey),
   );
   facts.mergeStates.set(key, next);
   return next;
 }
 
+// #4220 contradiction: the stored pr.mergeableState lags GitHub's async recompute, so a base-conflicting PR could
+// read clean here (safe to merge) while the disposition reads the live dirty and auto-CLOSES it. This ALWAYS
+// force-refetches live from GitHub and MUST NEVER be routed through the durable pull_request_detail_sync_state
+// cache added by #2537 — both act-boundary-adjacent callers (runAgentMaintenancePlanAndExecute's disposition
+// input, and the unified-comment mirror) depend on this staying live and uncached.
 function refreshLiveMergeState(
   env: Env,
   repoFullName: string,
@@ -2188,6 +2203,10 @@ async function reReviewStoredPullRequest(
     resyncAdmissionKey,
   );
   primeLiveMergeState(liveFacts, repoFullName, prNumber, resyncToken, live?.mergeable_state);
+  // #2537: this resync ALREADY paid for a bare GET /pulls/{n} — persist it to the durable cross-webhook cache so
+  // the readiness/dup-winner/gate-override readers below (and future webhook deliveries) don't re-fetch it.
+  // Best-effort, never blocks the sweep on a write hiccup.
+  await primeDurablePrStateCache(env, repoFullName, prNumber, live).catch(() => undefined);
   // Terminal early-exit (#1942): the PR is CLOSED/merged on GitHub even though the stored row still reads open — a
   // dropped `closed` webhook (relay down). Reconcile the stored row from the live payload and RETURN before the
   // expensive resync (files) + readiness + re-review reads. A stale sweep must never spend GitHub budget — or post
@@ -4225,6 +4244,14 @@ async function processGitHubWebhook(
         repoFullName,
         payload.pull_request,
       );
+      // #2537: the durable PR-state cache (mergeable_state/state) goes stale exactly when GitHub recomputes them —
+      // synchronize (new head → new mergeable_state recompute), closed (state flips), reopened (state flips back).
+      // Clear explicitly (null, not omitted — PARTIAL-UPDATE CONTRACT) so the next cached read is a forced live
+      // miss; other pull_request actions (labeled, edited, etc.) don't change these fields and are left untouched
+      // to avoid spurious cache churn / extra writes on high-frequency low-signal actions.
+      if (eventName === "pull_request" && (payload.action === "synchronize" || payload.action === "closed" || payload.action === "reopened")) {
+        await invalidatePrStateCache(env, repoFullName, pr.number).catch(() => undefined);
+      }
       // Reopen-prevention (#one-shot-reopen): a CONTRIBUTOR may not reopen a PR that gittensory or a maintainer
       // closed — closes are one-shot (resubmit, don't reopen). If a non-maintainer reopened a PR whose last close
       // was by the bot / repo owner / admin, re-close it and skip the re-review. Self-closes (the contributor
@@ -4322,6 +4349,14 @@ async function processGitHubWebhook(
         installationId &&
         shouldProcessPullRequestPublicSurface(eventName, payload.action)
       ) {
+        // #2537: reviews only actually change on a pull_request_review webhook (submitted/edited/dismissed) — not
+        // on every sweep tick / unrelated pull_request action — so invalidate the durable review cache HERE,
+        // before the refreshPullRequestDetails call below reads it, rather than on head-SHA change (reviews are
+        // independent of head). shouldProcessPullRequestPublicSurface already gates pull_request_review to
+        // exactly these 3 actions.
+        if (eventName === "pull_request_review") {
+          await invalidatePrReviewsCache(env, repoFullName, pr.number).catch(() => undefined);
+        }
         if (
           shouldCollectSlopEvidence(settings) ||
           settings.manifestPolicyGateMode !== "off" ||
@@ -5615,7 +5650,9 @@ export async function reconcileLiveDuplicateSiblings(
   const staleClosed = new Set<number>();
   await Promise.all(
     lowerOverlapping.map(async (sibling) => {
-      const liveState = await fetchLivePullRequestState(
+      // #2537: durable-cached — this is a non-authoritative reconcile read (not the act-boundary decision), so
+      // repeat calls across webhook deliveries for an unchanged sibling can reuse the cross-webhook cache.
+      const liveState = await cachedFetchLivePullRequestState(
         env,
         repoFullName,
         sibling.number,
@@ -7390,6 +7427,8 @@ async function recordGithubProductUsage(
  * THAT commit (the neutral check-run is per-commit by design). FAIL-OPEN: an unreadable live fetch returns the
  * cached head, so a transient GitHub hiccup never strands the override — it just targets the stored SHA as before.
  * Mirrors the rebase path's live re-fetch (prReadyForReview) and the dup-winner live reconcile.
+ * #2537: durable-cached — a separate `issue_comment` webhook handler with no request-local `liveFacts`, so the
+ * cross-webhook cache is a pure win here (not the act-boundary merge/close decision).
  */
 export async function resolveOverrideHeadSha(
   env: Env,
@@ -7402,7 +7441,7 @@ export async function resolveOverrideHeadSha(
       () => undefined,
     )) ?? env.GITHUB_PUBLIC_TOKEN;
   const admissionKey = githubAdmissionKeyForToken(env, installationId, token);
-  const liveHeadSha = await fetchLivePullRequestHeadSha(
+  const liveHeadSha = await cachedFetchLivePullRequestHeadSha(
     env,
     repoFullName,
     pr.number,

src/queue/processors.ts

@@ -874,6 +874,10 @@ export type PullRequestDetailSyncStateRecord = {
   lastSyncedAt?: string | null | undefined;
   errorSummary?: string | null | undefined;
   updatedAt?: string | null | undefined;
+  // #2537: durable bare-PR-state cache fields (mergeable_state/state from GET /pulls/{n}).
+  prMergeableState?: string | null | undefined;
+  prState?: string | null | undefined;
+  prStateFetchedAt?: string | null | undefined;
 };
 
 export type GitHubRateLimitObservationRecord = {