fix(review): preserve cached AI gate findings

migrations/0078_ai_review_cache_findings.sql

@@ -0,0 +1,3 @@
+-- Preserve the gate-relevant AI verdict when reusing a cached review: the public notes alone are not enough
+-- because block-mode consensus/split/inconclusive findings are advisory side effects that must be replayed.
+ALTER TABLE ai_review_cache ADD COLUMN findings_json TEXT NOT NULL DEFAULT '[]';

src/db/repositories.ts

@@ -59,6 +59,7 @@ import {
 } from "./schema";
 import type {
   Advisory,
+  AdvisoryFinding,
   AgentActionRecord,
   AgentActionStatus,
   AgentActionType,
@@ -3217,14 +3218,14 @@ export async function getCachedAiReview(
   pullNumber: number,
   headSha: string | null | undefined,
   mode: string,
-): Promise<{ notes: string; reviewerCount: number } | null> {
+): Promise<{ notes: string; reviewerCount: number; findings: AdvisoryFinding[] } | null> {
   if (!headSha) return null;
   const row = await env.DB
-    .prepare("SELECT notes, reviewer_count AS reviewerCount, ai_review_mode AS mode FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?")
+    .prepare("SELECT notes, reviewer_count AS reviewerCount, ai_review_mode AS mode, findings_json AS findingsJson FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?")
     .bind(repoFullName, pullNumber, headSha)
-    .first<{ notes: string; reviewerCount: number; mode: string }>();
+    .first<{ notes: string; reviewerCount: number; mode: string; findingsJson: string | null }>();
   if (!row || row.mode !== mode) return null;
-  return { notes: row.notes, reviewerCount: row.reviewerCount };
+  return { notes: row.notes, reviewerCount: row.reviewerCount, findings: parseJson<AdvisoryFinding[]>(row.findingsJson, []) };
 }
 
 /** Upsert the AI review for (repo, pull, head SHA). A nullish head SHA is a no-op. */
@@ -3234,17 +3235,17 @@ export async function putCachedAiReview(
   pullNumber: number,
   headSha: string | null | undefined,
   mode: string,
-  review: { notes: string; reviewerCount: number },
+  review: { notes: string; reviewerCount: number; findings?: AdvisoryFinding[] },
 ): Promise<void> {
   if (!headSha) return;
   await env.DB
     .prepare(
-      `INSERT INTO ai_review_cache (repo_full_name, pull_number, head_sha, ai_review_mode, notes, reviewer_count)
-       VALUES (?, ?, ?, ?, ?, ?)
+      `INSERT INTO ai_review_cache (repo_full_name, pull_number, head_sha, ai_review_mode, notes, reviewer_count, findings_json)
+       VALUES (?, ?, ?, ?, ?, ?, ?)
        ON CONFLICT(repo_full_name, pull_number, head_sha) DO UPDATE SET
-         ai_review_mode = excluded.ai_review_mode, notes = excluded.notes, reviewer_count = excluded.reviewer_count, created_at = CURRENT_TIMESTAMP`,
+         ai_review_mode = excluded.ai_review_mode, notes = excluded.notes, reviewer_count = excluded.reviewer_count, findings_json = excluded.findings_json, created_at = CURRENT_TIMESTAMP`,
     )
-    .bind(repoFullName, pullNumber, headSha, mode, review.notes, review.reviewerCount)
+    .bind(repoFullName, pullNumber, headSha, mode, review.notes, review.reviewerCount, jsonString(review.findings ?? []))
     .run();
 }
 

src/queue/processors.ts

@@ -3505,7 +3505,12 @@ export async function runAiReviewForAdvisory(
     reviewInlineComments?: boolean | undefined;
   },
 ): Promise<
-  | { notes: string; reviewerCount: number; inlineFindings: InlineFinding[] }
+  | {
+      notes: string;
+      reviewerCount: number;
+      inlineFindings: InlineFinding[];
+      findings: AdvisoryFinding[];
+    }
   | undefined
 > {
   const packAllowsAnyAuthorBlockingReview =
@@ -3674,21 +3679,21 @@ export async function runAiReviewForAdvisory(
       ),
     });
     if (result.status !== "ok") return undefined;
+    const findings: AdvisoryFinding[] = [];
     if (result.consensusDefect) {
-      const defect: AdvisoryFinding = {
+      findings.push({
         code: "ai_consensus_defect",
         severity: "critical",
         title: `AI reviewers agree on a likely critical defect: ${result.consensusDefect.title}`,
         detail: result.consensusDefect.detail,
         action:
           "Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.",
-      };
-      args.advisory.findings.push(defect);
+      });
     } else if (result.split) {
       // The reviewers DISAGREED — exactly one flagged a blocking defect. reviewbot's quorum: ANY reviewer
       // rejection closes the PR, so a split is a HARD BLOCKER (advisory.ts gates `ai_review_split` like a
       // consensus defect → gate failure → close); the contributor resubmits a fresh PR. (#ai-review-split)
-      args.advisory.findings.push({
+      findings.push({
         code: "ai_review_split",
         severity: "critical",
         title: "An AI reviewer flagged a likely blocking defect",
@@ -3700,7 +3705,7 @@ export async function runAiReviewForAdvisory(
     } else if (result.inconclusive) {
       // Fail-CLOSED (#ai-fail-closed): block-mode AI could not return a usable verdict. Hold the PR for a human
       // (an evaluation-blocker code → neutral gate) rather than letting it pass to auto-merge uncertified.
-      args.advisory.findings.push({
+      findings.push({
         code: "ai_review_inconclusive",
         severity: "warning",
         title: "AI review could not be completed",
@@ -3710,11 +3715,13 @@ export async function runAiReviewForAdvisory(
           "The gate is held for a human reviewer rather than passed automatically; it re-evaluates on the next update.",
       });
     }
+    args.advisory.findings.push(...findings);
     return result.advisoryNotes
       ? {
           notes: result.advisoryNotes,
           reviewerCount: result.reviewerCount,
           inlineFindings: result.inlineFindings,
+          findings,
         }
       : undefined;
   } catch (error) {
@@ -4179,9 +4186,15 @@ async function maybePublishPrPublicSurface(
   let preflight!: ReturnType<typeof buildPreflightResult>;
   let gateEvaluation: ReturnType<typeof evaluateGateCheck> | undefined;
   // inlineFindings is present ONLY on a FRESH review (cache miss) with inline comments enabled; the AI cache
-  // round-trips just notes + reviewerCount, so a cache hit carries no findings and never re-posts (#inline-comments).
+  // round-trips notes + reviewerCount + the gate findings (so a cache hit replays consensus/split/inconclusive
+  // blockers — see below), but NOT inlineFindings, so a cache hit never re-posts inline comments (#inline-comments).
   let aiReview:
-    | { notes: string; reviewerCount: number; inlineFindings?: InlineFinding[] }
+    | {
+        notes: string;
+        reviewerCount: number;
+        inlineFindings?: InlineFinding[];
+        findings?: AdvisoryFinding[];
+      }
     | undefined;
   let inlineCommentsEnabledForReview = false;
   let gateFinalized = false;
@@ -4441,6 +4454,7 @@ async function maybePublishPrPublicSurface(
         settings.aiReviewMode,
       ).catch(() => null);
       if (cachedReview) {
+        advisory.findings.push(...cachedReview.findings);
         aiReview = cachedReview;
       } else {
         // `.gittensory.yml` review.profile + review.path_instructions + review.exclude_paths (#review-profile /

test/unit/ai-review-advisory.test.ts

@@ -208,6 +208,25 @@ describe("runAiReviewForAdvisory", () => {
     expect(result?.notes).toBeDefined(); // the single parseable opinion still produces advisory notes
   });
 
+  it("appends an ai_review_split finding (lone-blocker HOLD) when the two block-mode reviewers disagree", async () => {
+    const adv = advisory();
+    // Both opinions parse, but only the FIRST reviewer names a blocker → consensus needs BOTH → no defect → split
+    // (reviewbot's quorum: a lone rejection holds the PR). The split finding must be both applied to the advisory
+    // AND round-tripped on the returned cache payload so a cache hit can replay this blocker (#ai-review-split).
+    const run = (async (model: string) => ({ response: model === BEST_REVIEW_MODELS[0] ? defectJson() : notesOnlyJson() })) as unknown as () => Promise<unknown>;
+    const result = await runAiReviewForAdvisory(aiEnv(run), {
+      settings: { aiReviewMode: "block" } as RepositorySettings,
+      advisory: adv,
+      repoFullName: "acme/widgets",
+      pr,
+      author: "alice",
+      confirmedContributor: true,
+    });
+    expect(adv.findings.map((f) => f.code)).toEqual(["ai_review_split"]); // applied to the advisory (gate blocker)
+    expect(result?.findings.map((f) => f.code)).toEqual(["ai_review_split"]); // returned for the AI cache to persist
+    expect(result?.notes).toBeDefined();
+  });
+
   it("uses the caller's pre-resolved files (FIX B) instead of the stored read, so the model sees the real diff", async () => {
     // FIX B: the processor passes `files` (its resolvePullRequestFilesForReview output). With no rows ever
     // written to the test DB, a stored read would yield an EMPTY diff; passing files proves the model gets the

test/unit/ai-review-cache.test.ts

@@ -14,7 +14,7 @@ describe("AI review cache (#1)", () => {
   it("reuses a stored review ONLY on the same (repo, pull, head SHA, mode)", async () => {
     const env = createTestEnv();
     await putCachedAiReview(env, "o/r", 7, "sha1", "block", { notes: "the review", reviewerCount: 2 });
-    expect(await getCachedAiReview(env, "o/r", 7, "sha1", "block")).toEqual({ notes: "the review", reviewerCount: 2 });
+    expect(await getCachedAiReview(env, "o/r", 7, "sha1", "block")).toEqual({ notes: "the review", reviewerCount: 2, findings: [] });
     expect(await getCachedAiReview(env, "o/r", 7, "sha1", "advisory")).toBeNull(); // mode changed → miss
     expect(await getCachedAiReview(env, "o/r", 7, "sha2", "block")).toBeNull(); // new head SHA → miss
     expect(await getCachedAiReview(env, "o/r", 8, "sha1", "block")).toBeNull(); // different PR → miss
@@ -23,7 +23,15 @@ describe("AI review cache (#1)", () => {
   it("upserts — a re-run at the same key replaces the stored review (+ mode)", async () => {
     const env = createTestEnv();
     await putCachedAiReview(env, "o/r", 7, "sha1", "advisory", { notes: "first", reviewerCount: 1 });
-    await putCachedAiReview(env, "o/r", 7, "sha1", "block", { notes: "second", reviewerCount: 2 });
-    expect(await getCachedAiReview(env, "o/r", 7, "sha1", "block")).toEqual({ notes: "second", reviewerCount: 2 });
+    await putCachedAiReview(env, "o/r", 7, "sha1", "block", {
+      notes: "second",
+      reviewerCount: 2,
+      findings: [{ code: "ai_review_split", severity: "critical", title: "Split", detail: "One reviewer blocked." }],
+    });
+    expect(await getCachedAiReview(env, "o/r", 7, "sha1", "block")).toEqual({
+      notes: "second",
+      reviewerCount: 2,
+      findings: [{ code: "ai_review_split", severity: "critical", title: "Split", detail: "One reviewer blocked." }],
+    });
   });
 });

test/unit/queue.test.ts

@@ -735,11 +735,13 @@ describe("queue processors", () => {
     expect(mergeAudit?.n).toBe(0);
   });
 
-  it("#1: the block-mode re-gate sweep reuses a cached AI review for the same head SHA — no AI call re-spent", async () => {
+  it("#1: the block-mode re-gate sweep replays cached AI findings before gate evaluation", async () => {
     let aiCalls = 0;
     const env = createTestEnv({
       GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(),
       AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Critical defect found.", blockers: ["x"], nits: [], suggestions: [] }) }; } } as unknown as Ai,
+      AI_SUMMARIES_ENABLED: "true",
+      AI_PUBLIC_COMMENTS_ENABLED: "true",
       AI_DAILY_NEURON_BUDGET: "100000",
     });
     await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } });
@@ -748,7 +750,11 @@ describe("queue processors", () => {
     await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Stale PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" });
     await upsertPullRequestFile(env, { repoFullName: "owner/agent-repo", pullNumber: 7, path: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, payload: { patch: "@@\n+export const ok = value.length;" } });
     // Pre-seed the AI review for this exact head SHA + mode → the sweep's block-mode review must reuse it, not re-run.
-    await putCachedAiReview(env, "owner/agent-repo", 7, "a7", "block", { notes: "cached review", reviewerCount: 2 });
+    await putCachedAiReview(env, "owner/agent-repo", 7, "a7", "block", {
+      notes: "cached review",
+      reviewerCount: 2,
+      findings: [{ code: "ai_consensus_defect", severity: "critical", title: "Cached defect", detail: "Cached critical defect." }],
+    });
     vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
       const url = input.toString();
       if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
@@ -768,6 +774,10 @@ describe("queue processors", () => {
     await sweepAndDrainPerPr(env, "owner/agent-repo");
 
     expect(aiCalls).toBe(0); // the cached AI review was reused — the LLM was never called for this head SHA
+    const blocker = await env.DB.prepare("select blocker_codes_json from gate_outcomes where repo_full_name = ? and pull_number = ? order by rowid desc limit 1").bind("owner/agent-repo", 7).first<{ blocker_codes_json: string }>();
+    expect(blocker?.blocker_codes_json).toContain("ai_consensus_defect");
+    const mergeAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and detail like ?").bind("agent.action.merge", "%merged%").first<{ n: number }>();
+    expect(mergeAudit?.n).toBe(0);
   });
 
   it("posts the 🟪 reviewing placeholder before the AI review runs, then overwrites it with the verdict (#reviewing-placeholder)", async () => {