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

migrations/0094_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;

migrations/0095_repair_reviews_synced_at.sql

@@ -0,0 +1,26 @@
+-- One-off repair for pull_request_detail_sync_state.reviews_synced_at (#2537 review fix).
+--
+-- ROOT CAUSE (already fixed in code, this same PR): every pre-existing writer of reviews_synced_at
+-- (backfillOpenPullRequestDetails / refreshPullRequestDetails / backfillRepository, all in
+-- src/github/backfill.ts) stamped it UNCONDITIONALLY on every sync pass, success or failure -- even a pass that
+-- ended with `status: 'partial'` due to a review-fetch error still wrote a fresh reviews_synced_at timestamp, as
+-- if the reviews had actually been captured. The column now gates a durable, head-independent read-through
+-- cache (fetchAndStorePullRequestDetails' reviewsUpToDate check): ANY non-null reviews_synced_at is trusted as
+-- "reviews are fully synced, skip refetching" until a `pull_request_review` webhook invalidates it.
+--
+-- Impact of leaving existing rows as-is: a PR whose review sync previously failed (rate limit, transient
+-- network error, etc.) already has a reviews_synced_at timestamp from that failed attempt. The new cache would
+-- treat that PR's reviews as permanently up to date -- silently serving incomplete/stale review data forever,
+-- unless that specific PR happens to receive a NEW review action after this deploys (the only thing that clears
+-- the marker going forward).
+--
+-- Repair strategy: there is no reliable way to tell, from the stored row alone, whether a PARTICULAR existing
+-- reviews_synced_at value came from a pass where reviews specifically succeeded (status reflects the aggregate
+-- outcome across files/reviews/checks together, not reviews alone). Clear it for every row instead of trying to
+-- guess -- the one-time cost is a single redundant review refetch for PRs that were already correctly synced,
+-- which is cheap and bounded; the alternative (leaving any ambiguous row as-is) risks silently trusting bad data
+-- indefinitely. The next sync pass for every tracked PR re-populates it correctly under the new,
+-- only-stamp-on-success semantics.
+UPDATE pull_request_detail_sync_state
+   SET reviews_synced_at = NULL
+ WHERE reviews_synced_at IS NOT NULL;

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) => ({