fix(review): stop mis-closing registry edits and AI-guessed CI diagnoses

src/github/backfill.ts

@@ -3279,6 +3279,36 @@ export async function fetchLiveBaseBranchAdvancedAt(
   return result?.data.commit?.committer?.date ?? undefined;
 }
 
+/**
+ * How many commits the repo's CURRENT default branch has landed since this PR's own base commit, via REST
+ * `GET /compare/{baseSha}...{defaultBranchRef}` (`ahead_by` from `baseSha`'s perspective — #review-grounding
+ * stale-base fact). `mergeable_state: "behind"` (the signal `prReadyForReview`'s auto-rebase-before-review path
+ * already uses) only ever fires when the repo's branch protection has "require branches to be up to date before
+ * merging" enabled — a repo without that setting can have a branch genuinely dozens of commits behind and GitHub
+ * will still never report it as "behind". This compare-API read is unconditional: it works regardless of branch
+ * protection config, so it can ground the AI reviewer in the TRUE fact even on a repo where the mergeable_state
+ * signal never fires. Best-effort: any fetch/shape error returns undefined so the caller degrades to "unknown"
+ * (no stale-base fact rendered) rather than throwing or asserting a wrong number.
+ */
+export async function fetchBaseAheadBy(
+  env: Env,
+  repoFullName: string,
+  baseSha: string,
+  defaultBranchRef: string,
+  token: string | undefined,
+  admissionKey?: GitHubRateLimitAdmissionKey,
+): Promise<number | undefined> {
+  const result = await githubJsonWithHeaders<{ ahead_by?: number | null }>(
+    env,
+    repoFullName,
+    `/compare/${encodeURIComponent(baseSha)}...${encodeURIComponent(defaultBranchRef)}`,
+    token,
+    githubRateLimitOptions(admissionKey),
+  ).catch(() => undefined);
+  const aheadBy = result?.data.ahead_by;
+  return typeof aheadBy === "number" && Number.isFinite(aheadBy) ? aheadBy : undefined;
+}
+
 /** The PR's LIVE state ("open" / "closed") via REST `GET /pulls/{n}`. The stored open-PR cache lags GitHub, so a
  *  sibling closed/merged on GitHub can still read `open` locally; the duplicate-winner election (#dup-winner /
  *  audit #15) confirms a lower sibling's live state before treating this PR as a cluster loser. Best-effort:

src/queue/ai-review-orchestration.ts

@@ -535,19 +535,25 @@ export async function runAiReviewForAdvisory(
     // byte-identical to today. Fully fail-safe.
     const grounding =
       groundingActive
-        ? await buildReviewGroundingText(env, {
-            repoFullName: args.repoFullName,
-            headSha: args.advisory.headSha,
-            files,
-            checks: await listCheckSummaries(
-              env,
-              args.repoFullName,
-              args.pr.number,
-            ),
-            installationId:
-              (await getRepository(env, args.repoFullName))?.installationId ??
-              null,
-          })
+        ? await (async () => {
+            const repo = await getRepository(env, args.repoFullName);
+            return buildReviewGroundingText(env, {
+              repoFullName: args.repoFullName,
+              headSha: args.advisory.headSha,
+              files,
+              checks: await listCheckSummaries(
+                env,
+                args.repoFullName,
+                args.pr.number,
+              ),
+              installationId: repo?.installationId ?? null,
+              // #review-grounding stale-base fact (metagraphed #7305-class incident): both are additive — either
+              // absent (no baseSha on a rare malformed webhook record, or an unregistered repo with no stored
+              // defaultBranch) just skips the BASE BRANCH STATUS fact, same as before it existed.
+              baseSha: args.pr.baseSha,
+              defaultBranchRef: repo?.defaultBranch,
+            });
+          })()
         : undefined;
     // RAG retrieval (convergence, flag-gated by LOOPOVER_REVIEW_RAG). Query the codebase vector index for code/docs
     // semantically related to the changed files and append them as additive reference context — exactly like

src/review/content-lane/orchestrator.ts

@@ -55,9 +55,11 @@ function surfacesOf(doc: unknown, field: string): unknown[] | null {
 /**
  * ALL surfaces[] entries present at head but absent at base — a pure head-vs-base structural diff. Returns null
  * when head is unreadable / has no surfaces[] array; returns an empty array when nothing was added (a
- * reorder/reformat/edit of existing entries reads as zero "added"). A missing base file (a brand-new entry file)
- * means every head entry is new. Makes no count judgement itself — the caller (runSurfaceReview) enforces the
- * spec's maxAppendedEntries cap and the ≥1-entry requirement.
+ * byte-identical reorder reads as zero "added" — an actual FIELD EDIT to an existing entry does NOT: the edited
+ * entry's new content differs from every base entry, so it reads as one "added" entry, same as a brand-new
+ * append — see survivingExistingEntries below for how the caller tells the two apart before duplicate-checking).
+ * A missing base file (a brand-new entry file) means every head entry is new. Makes no count judgement itself —
+ * the caller (runSurfaceReview) enforces the spec's maxAppendedEntries cap and the ≥1-entry requirement.
  */
 export function diffAppendedSurfaceEntries(headRaw: string | null, baseRaw: string | null, field: string): unknown[] | null {
   const headEntries = surfacesOf(safeParseJson(headRaw), field);
@@ -67,6 +69,34 @@ export function diffAppendedSurfaceEntries(headRaw: string | null, baseRaw: stri
   return headEntries.filter((entry) => !baseKeys.has(JSON.stringify(entry)));
 }
 
+/**
+ * Base surfaces[] entries that are STILL PRESENT, byte-identical, in the head document — i.e. entries this PR
+ * left completely untouched. Feeds findDuplicateAppendedEntry's `existingEntries` argument so a submission's
+ * duplicate check only ever collides against an entry that genuinely still occupies that identity in the
+ * registry, not one this very PR just edited away.
+ *
+ * Why this matters: an in-place edit (e.g. tightening `probe.expect` on an already-registered surface, keeping
+ * its url) makes diffAppendedSurfaceEntries read the edited entry as "added" (its new content differs from every
+ * base entry — see that function's own doc comment), and the edited entry's identity key (typically its url)
+ * still matches its OWN prior self in base. Passing the raw base surfaces[] array as `existingEntries` (the
+ * pre-fix behavior) makes an entry collide with its own now-superseded version and reads as a resubmitted
+ * duplicate, closing every legitimate "fix an existing surface" PR outright regardless of content correctness.
+ * Filtering to only the base entries that SURVIVE into head fixes this: an edited entry's old self is gone from
+ * head (replaced in place), so it is excluded here and can no longer collide with the edit. A genuine duplicate
+ * resubmission is unaffected — the untouched original entry remains in head, stays in this filtered set, and
+ * still collides with the newly appended entry sharing its identity, so that case still closes as before.
+ *
+ * Returns [] when head is unreadable (mirrors diffAppendedSurfaceEntries' own null-safety); the orchestrator only
+ * reaches this after diffAppendedSurfaceEntries has already confirmed head parses with a surfaces[] array.
+ */
+export function survivingExistingEntries(headRaw: string | null, baseRaw: string | null, field: string): unknown[] {
+  const headEntries = surfacesOf(safeParseJson(headRaw), field);
+  if (headEntries === null) return [];
+  const baseEntries = surfacesOf(safeParseJson(baseRaw), field) ?? [];
+  const headKeys = new Set(headEntries.map((entry) => JSON.stringify(entry)));
+  return baseEntries.filter((entry) => headKeys.has(JSON.stringify(entry)));
+}
+
 function fromProvider(assessment: ProviderAssessment): SurfaceReviewResult {
   // Decisive: a valid provider merges; an invalid one CLOSES (resubmit clean) — never a manual punt.
   return assessment.ok
@@ -237,7 +267,9 @@ export async function runSurfaceReview(spec: RegistryLaneSpec, input: SurfaceRev
   if (appendedEntries === null || appendedEntries.length === 0 || appendedEntries.length > maxAppendedEntries) {
     return { verdict: "close", summary: appendCountCloseSummary(maxAppendedEntries) };
   }
-  const existingEntries = surfacesOf(safeParseJson(baseRaw), spec.collectionField) ?? [];
+  // survivingExistingEntries, not the raw base array — an entry this PR edited in place must not collide with
+  // its own now-superseded prior self (see that function's doc comment for the full false-positive it fixes).
+  const existingEntries = survivingExistingEntries(headRaw, baseRaw, spec.collectionField);
   const duplicate = findDuplicateAppendedEntry(spec, appendedEntries, existingEntries);
   if (duplicate !== null) {
     return { verdict: "close", summary: duplicateEntryCloseSummary() };

src/review/grounding-wire.ts

@@ -12,6 +12,7 @@
 // fail-safe: any missing CI data / fetch error degrades to "no grounding" and the review proceeds on the diff.
 
 import { createInstallationToken } from "../github/app";
+import { fetchBaseAheadBy } from "../github/backfill";
 import { githubRateLimitAdmissionKeyForToken, PRODUCT_USER_AGENT, timeoutFetch, type GitHubRateLimitAdmissionKey } from "../github/client";
 import { getCachedGroundingFileContent, putCachedGroundingFileContent, recordAuditEvent } from "../db/repositories";
 import type { CheckSummaryRecord, PullRequestFileRecord } from "../types";
@@ -121,22 +122,35 @@ function toGroundingFiles(files: PullRequestFileRecord[]): PullRequestFile[] {
   });
 }
 
+/**
+ * Resolve (best-effort) the token to authenticate a grounding-wire GitHub read with: installation token > public
+ * token > none. `admissionKey` is derived from the FINAL token (#regression-safe-propagation), after the
+ * public-token fallback is applied -- computing it before that fallback (against the pre-fallback
+ * `installationId`-only branch) left every fallback call with `admissionKey: undefined` even though the actual
+ * token used (`GITHUB_PUBLIC_TOKEN`) has a perfectly nameable scope, silently dropping every such call into
+ * `key_scope="unknown"` on any rate-limited response. Shared by makeGithubFileFetcher's Contents-API reads and
+ * the base-branch staleness read (#review-grounding stale-base fact) so both authenticate identically without
+ * duplicating the fallback logic.
+ */
+async function resolveGroundingToken(
+  env: Env,
+  installationId: number | null | undefined,
+): Promise<{ token: string | undefined; admissionKey: GitHubRateLimitAdmissionKey | undefined }> {
+  let token: string | undefined;
+  if (installationId) token = await createInstallationToken(env, installationId).catch(() => undefined);
+  token = token ?? env.GITHUB_PUBLIC_TOKEN;
+  const admissionKey: GitHubRateLimitAdmissionKey | undefined = githubRateLimitAdmissionKeyForToken(env, token, installationId);
+  return { token, admissionKey };
+}
+
 /**
  * A {@link FileFetcher} backed by the GitHub Contents API. Authenticates with an installation token (so it
  * reads private repos), falling back to the public token, then to unauthenticated. Returns the raw file text,
  * or null on any non-OK / binary / oversized / error response. NEVER throws — the grounding engine already
  * treats null as "skip this file" and degrades to no-grounding when nothing is readable.
  */
 export async function makeGithubFileFetcher(env: Env, repoFullName: string, installationId: number | null | undefined): Promise<FileFetcher> {
-  // Resolve the token once (best-effort): installation token > public token > none. `admissionKey` is derived
-  // from the FINAL token (#regression-safe-propagation), after the public-token fallback is applied -- computing
-  // it before that fallback (against the pre-fallback `installationId`-only branch) left every fallback call
-  // with `admissionKey: undefined` even though the actual token used (`GITHUB_PUBLIC_TOKEN`) has a perfectly
-  // nameable scope, silently dropping every such call into `key_scope="unknown"` on any rate-limited response.
-  let token: string | undefined;
-  if (installationId) token = await createInstallationToken(env, installationId).catch(() => undefined);
-  token = token ?? env.GITHUB_PUBLIC_TOKEN;
-  const admissionKey: GitHubRateLimitAdmissionKey | undefined = githubRateLimitAdmissionKeyForToken(env, token, installationId);
+  const { token, admissionKey } = await resolveGroundingToken(env, installationId);
   const { owner, name } = repoParts(repoFullName);
   return {
     async getFileContent(path: string, ref: string, maxChars = 24_001): Promise<string | null> {
@@ -259,6 +273,12 @@ export async function buildReviewGroundingText(
     files: PullRequestFileRecord[];
     checks: CheckSummaryRecord[];
     installationId: number | null | undefined;
+    // #review-grounding stale-base fact (metagraphed #7305-class incident): when both are readable, an
+    // additional BASE BRANCH STATUS fact is folded into the SAME ciGrounding-gated section so an undetailed CI
+    // failure has a true, deterministic explanation available instead of an unverified guess. Either absent ⇒
+    // this fact is simply skipped (byte-identical to before it existed) — it is additive, never required.
+    baseSha?: string | null | undefined;
+    defaultBranchRef?: string | null | undefined;
   },
 ): Promise<ReviewGroundingText> {
   const flags = groundingFlags(env);
@@ -267,7 +287,16 @@ export async function buildReviewGroundingText(
     const aggregate = buildCheckAggregate(args.checks);
     const fetcher = await makeGithubFileFetcher(env, args.repoFullName, args.installationId);
     const fileContents = await fetchFullFileContents(flags, args.headSha ?? undefined, toGroundingFiles(args.files), fetcher);
-    const grounding = buildGrounding(flags, aggregate, fileContents);
+    const baseSha = args.baseSha;
+    const defaultBranchRef = args.defaultBranchRef;
+    const baseAheadBy =
+      flags.ciGrounding && baseSha && defaultBranchRef
+        ? await (async () => {
+            const { token, admissionKey } = await resolveGroundingToken(env, args.installationId);
+            return fetchBaseAheadBy(env, args.repoFullName, baseSha, defaultBranchRef, token, admissionKey);
+          })()
+        : undefined;
+    const grounding = buildGrounding(flags, aggregate, fileContents, baseAheadBy);
     const promptSection = formatGroundingSections(grounding);
     // Only attach the grounding-discipline system suffix when we actually produced grounding to verify
     // against; otherwise the prompt stays unchanged (no point telling the model to "check the file" with