fix(review): stop AI-review verdict-shopping — content-fingerprint stickiness + flip-count escalation

migrations/0183_ai_review_verdict_flips.sql

@@ -0,0 +1,15 @@
+-- #9016 (security): bound how many times an AI-review verdict can flip-flop for one PR before a human is
+-- required. The AI reviewer is non-deterministic even at temperature 0; without a bound, a contributor can
+-- force fresh re-rolls (a no-op recommit that invalidates the head-SHA cache key, or a same-head retry once
+-- the non-cacheable cooldown lapses) until a lucky CLEAN roll auto-merges a PR other rolls flagged as
+-- blocked. One row per (repo_full_name, pull_number) tracks the last FRESH (non-cache-hit) verdict's
+-- defect/clean state and how many times it has flipped since; the escalation itself is computed in
+-- src/review/verdict-flip-guard.ts (pure) and applied by the caller.
+CREATE TABLE IF NOT EXISTS ai_review_verdict_flips (
+  repo_full_name  TEXT NOT NULL,
+  pull_number     INTEGER NOT NULL,
+  last_had_defect INTEGER NOT NULL,   -- 0 | 1
+  flip_count      INTEGER NOT NULL DEFAULT 0,
+  updated_at      TEXT NOT NULL,
+  PRIMARY KEY (repo_full_name, pull_number)
+);

scripts/check-schema-drift.ts

@@ -45,6 +45,7 @@ export const RAW_SQL_ONLY_TABLES: Set<string> = new Set([
   "decision_ledger",
   "decision_records",
   "decision_replay_inputs",
+  "ai_review_verdict_flips",
   "global_agent_controls",
   "global_contributor_blacklist",
   "global_moderation_config",

src/db/repositories.ts

@@ -5100,6 +5100,58 @@ export async function getCachedAiReview(
   };
 }
 
+/** Bounds the fingerprint scan below to a handful of rows — a PR rarely accumulates more than a few AI
+ *  review cache rows across its lifetime, and this is a fallback path, not the hot lookup. */
+const AI_REVIEW_FINGERPRINT_SCAN_LIMIT = 20;
+
+/**
+ * #9016 (security): a content-fingerprint-sticky FALLBACK for {@link getCachedAiReview}, used only when the
+ * exact head-SHA lookup misses. The head-SHA-keyed cache treats every new commit as an independent roll of
+ * the (non-deterministic) AI reviewer — so a contributor can force fresh re-rolls with a no-op recommit
+ * (an empty commit, a metadata-only push) until a lucky CLEAN roll auto-merges a PR another roll flagged as
+ * blocked. `expectedInputFingerprint` already hashes every prompt-shaping input including the actual per-
+ * file PATCH content (see ai-review-cache-input.ts's `reviewFiles`), so an IDENTICAL fingerprint under a
+ * DIFFERENT head SHA means the reviewed content is genuinely unchanged — this reuses that prior verdict
+ * instead of spending a fresh, independently-random roll on content the reviewer has already judged.
+ * Same recency/cacheable rules as getCachedAiReview: a durable (cacheable) row reuses unboundedly; a
+ * dynamic-context (non-cacheable) row only within `maxAgeMs` of being written, and never once published
+ * (a stale non-durable published verdict is not trustworthy indefinitely across heads either).
+ */
+export async function getCachedAiReviewAcrossHeads(
+  env: Env,
+  repoFullName: string,
+  pullNumber: number,
+  mode: string,
+  inputFingerprint: string,
+  options?: { maxAgeMs?: number } | undefined,
+): Promise<{ notes: string; reviewerCount: number; findings: AdvisoryFinding[]; metadata?: Record<string, unknown> | undefined } | null> {
+  const { results } = await env.DB
+    .prepare(
+      "SELECT notes, reviewer_count AS reviewerCount, findings_json AS findingsJson, metadata_json AS metadataJson, cacheable, published_at AS publishedAt, created_at AS createdAt FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND ai_review_mode = ? ORDER BY created_at DESC LIMIT ?",
+    )
+    .bind(repoFullName, pullNumber, mode, AI_REVIEW_FINGERPRINT_SCAN_LIMIT)
+    .all<{ notes: string; reviewerCount: number; findingsJson: string | null; metadataJson: string | null; cacheable: number; publishedAt: string | null; createdAt: string }>();
+  for (const row of results ?? []) {
+    const metadata = parseJson<Record<string, unknown>>(row.metadataJson, {});
+    if (metadata.inputFingerprint !== inputFingerprint) continue;
+    if (row.cacheable !== 1) {
+      if (row.publishedAt != null) continue;
+      const ageMs = Date.now() - Date.parse(row.createdAt);
+      if (!Number.isFinite(ageMs) || ageMs < 0 || ageMs > (options?.maxAgeMs ?? 0)) continue;
+    }
+    // Unlike getCachedAiReview above, `metadata` is NEVER empty here by construction: reaching this line
+    // already required `metadata.inputFingerprint === inputFingerprint` to pass, above — so no conditional
+    // spread is needed (there is no empty-metadata arm to guard).
+    return {
+      notes: row.notes,
+      reviewerCount: row.reviewerCount,
+      findings: parseJson<AdvisoryFinding[]>(row.findingsJson, []),
+      metadata,
+    };
+  }
+  return null;
+}
+
 /** #regate-churn (maintainer-gated freeze): the most recently PUBLISHED AI review for this PR, regardless of
  *  which head SHA it was computed against. Used ONLY when the PR is currently held for manual review — a repeat
  *  contributor push must not buy a fresh, real AI call (or a chance to flip the published verdict via plain LLM

src/queue/processors.ts

@@ -41,6 +41,7 @@ import {
   markRepositoriesRemovedFromInstallation,
   persistAdvisory,
   getCachedAiReview,
+  getCachedAiReviewAcrossHeads,
   getLatestPublishedAiReview,
   countPublishedAiReviewHeads,
   putCachedAiReview,
@@ -648,6 +649,7 @@ import {
 import { AI_JUDGMENT_BLOCKER_CODES } from "../rules/advisory";
 import { computeSalvageabilityForTarget } from "../review/salvageability-wire";
 import { deriveDecisionReasonCode, persistDecisionReplayInputForGate } from "../review/decision-replay";
+import { recordVerdictFlip } from "../review/verdict-flip-store";
 import { REVIEW_PROMPT_VERSION, REVIEW_SYSTEM_PROMPT } from "../services/ai-review";
 import { resolveAutomaticCloseConfidence } from "../review/risk-control-wire";
 import { maybeApplyCloseAuditHoldout } from "../review/close-audit-holdout";
@@ -10381,15 +10383,28 @@ async function maybePublishPrPublicSurface(
           // (unbounded reuse, exactly as before this fix).
           const cachedReview = webhook.forceAiReview === true
             ? null
-            : await getCachedAiReview(
+            : (await getCachedAiReview(
                 env,
                 repoFullName,
                 pr.number,
                 advisory.headSha,
                 settings.aiReviewMode,
                 inputFingerprint,
                 { allowNonCacheable: true, maxAgeMs: AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS },
-              ).catch(() => null);
+              ).catch(() => null)) ??
+              // #9016 (security): the exact-head lookup just missed. Before spending a fresh, independently
+              // random LLM roll, check whether this PR already has a verdict computed against IDENTICAL
+              // review content (the fingerprint already hashes the actual patch content, not just the head
+              // SHA) under a DIFFERENT head — closes the no-op-recommit reroll exploit without weakening the
+              // cache's normal same-head behavior at all.
+              (await getCachedAiReviewAcrossHeads(
+                env,
+                repoFullName,
+                pr.number,
+                settings.aiReviewMode,
+                inputFingerprint,
+                { maxAgeMs: AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS },
+              ).catch(() => null));
           if (cachedReview && hasPublicReviewAssessment(cachedReview.notes)) {
             advisory.findings.push(...cachedReview.findings);
             aiReview = cachedReview;
@@ -10475,6 +10490,34 @@ async function maybePublishPrPublicSurface(
               deliveryId: webhook.deliveryId,
               preComputedReputationSkip,
             });
+            // #9016 (security): a FRESH verdict (this branch only runs on a genuine cache miss — never for a
+            // reused cache hit, which is not a new independent roll) is checked against the PR's flip history.
+            // The AI reviewer is non-deterministic, so a contributor can otherwise force re-rolls (a no-op
+            // recommit, or a same-head retry after the non-cacheable cooldown lapses) until a lucky CLEAN roll
+            // auto-merges a PR another roll flagged as blocked. Scoped to block mode only — advisory mode never
+            // gates on the AI verdict, so there is nothing to shop for there. Best-effort/fail-open by
+            // construction (recordVerdictFlip never throws); a persistable placeholder never counts as a roll.
+            if (aiReview && aiReview.persistable !== false && settings.aiReviewMode === "block") {
+              const verdictFlip = await recordVerdictFlip(env, repoFullName, pr.number, aiReview.findings ?? []);
+              if (verdictFlip.escalate) {
+                advisory.findings.push({
+                  code: "ai_review_inconclusive",
+                  severity: "warning",
+                  title: "AI review verdict has flip-flopped too many times",
+                  detail: `This PR's AI review result has changed direction ${verdictFlip.flipCount} times across recent re-reviews of the same or similar content. Repeated re-rolls of a non-deterministic reviewer are held for a human instead of trusting the newest roll.`,
+                  action: "A maintainer should review this PR directly, or push a substantive fix so the next review reflects real content change.",
+                });
+                incr("loopover_ai_review_verdict_flip_escalated_total");
+                await recordAuditEvent(env, {
+                  eventType: "github_app.ai_review_verdict_flip_escalated",
+                  actor: author,
+                  targetKey: `${repoFullName}#${pr.number}`,
+                  outcome: "completed",
+                  detail: `verdict flipped ${verdictFlip.flipCount} times; held for human review`,
+                  metadata: { deliveryId: webhook.deliveryId, repoFullName, headSha: advisory.headSha ?? null, flipCount: verdictFlip.flipCount },
+                }).catch(() => undefined);
+              }
+            }
             // `persistable === false` (only the lock-contention placeholder — see runAiReviewForAdvisory's return
             // type doc comment) is excluded from EVERY write, not just the durable one: it describes a transient
             // scheduling race, not a real AI opinion, and the concurrent pass it deferred to persists the real

src/review/verdict-flip-guard.ts

@@ -0,0 +1,37 @@
+// AI-review verdict-flip escalation (#9016, security) — PURE state machine.
+//
+// The AI reviewer is non-deterministic even at temperature 0. Without a bound, a contributor can force
+// fresh re-rolls (a no-op recommit that invalidates the head-SHA cache key, or a same-head retry once the
+// non-cacheable-result cooldown lapses) until a lucky CLEAN roll auto-merges a PR other rolls flagged as
+// blocked — verdict-shopping against a randomized judge. This tracks the last FRESH (non-cache-hit)
+// verdict's defect/clean state per PR and counts how many times it has FLIPPED; once flips clear a
+// threshold, the caller holds the gate for a human instead of trusting the newest roll.
+import { AI_JUDGMENT_BLOCKER_CODES } from "../rules/advisory";
+
+/** Flips this large signal a genuine, sustained oscillation rather than reviewer noise on the first retry
+ *  — the exploit needs several rolls to land a lucky one, so the bound should not fire on ordinary review
+ *  churn (one legitimate re-review after a real fix is not abuse). */
+export const VERDICT_FLIP_ESCALATION_THRESHOLD = 3;
+
+export type VerdictFlipState = { lastHadDefect: boolean; flipCount: number };
+export type VerdictFlipResult = VerdictFlipState & { escalate: boolean };
+
+/**
+ * Advance the flip state with one FRESH verdict (never call this for a cache-hit reuse of a prior verdict —
+ * reusing a result is not a new independent roll). PURE. A first observation for a PR never escalates
+ * (there is nothing to flip against yet). A flip is a CHANGE from the immediately prior fresh verdict;
+ * repeating the same verdict does not add to the count — a PR that is consistently blocked, or
+ * consistently clean, across many honest re-reviews is not the abuse pattern this guards against, only
+ * genuine oscillation is.
+ */
+export function nextVerdictFlipState(prior: VerdictFlipState | null, hadDefect: boolean): VerdictFlipResult {
+  if (prior === null) return { lastHadDefect: hadDefect, flipCount: 0, escalate: false };
+  const flipCount = prior.lastHadDefect === hadDefect ? prior.flipCount : prior.flipCount + 1;
+  return { lastHadDefect: hadDefect, flipCount, escalate: flipCount >= VERDICT_FLIP_ESCALATION_THRESHOLD };
+}
+
+/** Whether a fresh review's findings carry a blocking AI-judgment defect (ai_consensus_defect /
+ *  ai_review_split) — the boolean this guard tracks flips of. PURE. */
+export function findingsHadAiDefect(findings: ReadonlyArray<{ code: string }>): boolean {
+  return findings.some((finding) => AI_JUDGMENT_BLOCKER_CODES.has(finding.code));
+}

src/review/verdict-flip-store.ts

@@ -0,0 +1,51 @@
+// Verdict-flip persistence (#9016, security) — the IO around src/review/verdict-flip-guard.ts's pure state
+// machine. One row per PR (migration 0183); fail-open throughout — an infra blip must degrade to "never
+// escalate," never block or unblock the gate on its own.
+import { findingsHadAiDefect, nextVerdictFlipState, type VerdictFlipResult, type VerdictFlipState } from "./verdict-flip-guard";
+import { errorMessage, nowIso } from "../utils/json";
+
+export async function readVerdictFlipState(env: Env, repoFullName: string, pullNumber: number): Promise<VerdictFlipState | null> {
+  try {
+    const row = await env.DB.prepare(
+      "SELECT last_had_defect AS lastHadDefect, flip_count AS flipCount FROM ai_review_verdict_flips WHERE repo_full_name = ? AND pull_number = ?",
+    )
+      .bind(repoFullName, pullNumber)
+      .first<{ lastHadDefect: number; flipCount: number }>();
+    if (!row) return null;
+    return { lastHadDefect: row.lastHadDefect === 1, flipCount: row.flipCount };
+  } catch (error) {
+    console.warn(JSON.stringify({ event: "verdict_flip_read_error", repoFullName, pullNumber, message: errorMessage(error).slice(0, 120) }));
+    return null;
+  }
+}
+
+async function writeVerdictFlipState(env: Env, repoFullName: string, pullNumber: number, next: VerdictFlipState): Promise<void> {
+  try {
+    await env.DB.prepare(
+      `INSERT INTO ai_review_verdict_flips (repo_full_name, pull_number, last_had_defect, flip_count, updated_at) VALUES (?, ?, ?, ?, ?)
+       ON CONFLICT(repo_full_name, pull_number) DO UPDATE SET last_had_defect = excluded.last_had_defect, flip_count = excluded.flip_count, updated_at = excluded.updated_at`,
+    )
+      .bind(repoFullName, pullNumber, next.lastHadDefect ? 1 : 0, next.flipCount, nowIso())
+      .run();
+  } catch (error) {
+    console.warn(JSON.stringify({ event: "verdict_flip_write_error", repoFullName, pullNumber, message: errorMessage(error).slice(0, 120) }));
+  }
+}
+
+/**
+ * Record one FRESH AI-review verdict (never call for a cache-hit reuse — see verdict-flip-guard's doc
+ * comment) and return the advanced flip state. Fail-open: any read/write error degrades to treating this
+ * as a first observation (flipCount 0, never escalates) — an infra blip must never itself force a hold.
+ */
+export async function recordVerdictFlip(
+  env: Env,
+  repoFullName: string,
+  pullNumber: number,
+  findings: ReadonlyArray<{ code: string }>,
+): Promise<VerdictFlipResult> {
+  const hadDefect = findingsHadAiDefect(findings);
+  const prior = await readVerdictFlipState(env, repoFullName, pullNumber);
+  const next = nextVerdictFlipState(prior, hadDefect);
+  await writeVerdictFlipState(env, repoFullName, pullNumber, next);
+  return next;
+}

src/selfhost/metrics.ts

@@ -153,6 +153,7 @@ export const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [
   ["loopover_ai_review_one_shot_reuse_total", { help: "AI review passes that reused a one-shot prior verdict instead of re-running.", type: "counter" }],
   ["loopover_ai_review_paused_reuse_total", { help: "AI review passes that reused a prior verdict because the repo is paused.", type: "counter" }],
   ["loopover_ai_review_tiebreak_order_unstable_total", { help: "Dual-reviewer tiebreak passes where reviewer order was not stable, by combine mode.", type: "counter" }],
+  ["loopover_ai_review_verdict_flip_escalated_total", { help: "AI review verdict-flip escalations (#9016): a PR's fresh AI-judgment verdict oscillated past the flip threshold and was held for a human instead of trusting the newest roll.", type: "counter" }],
   ["loopover_grounding_cache_hit_total", { help: "Review grounding-context cache hits.", type: "counter" }],
   ["loopover_grounding_cache_miss_total", { help: "Review grounding-context cache misses.", type: "counter" }],
   ["loopover_impact_map_cache_hit_total", { help: "Impact-map cache hits.", type: "counter" }],

src/selfhost/pg-dialect.ts

@@ -15,6 +15,9 @@ const REPLACE_CONFLICT_KEYS: Record<string, string[]> = {
   // #8893: orb_reuse_counters (migrations/0177) is written with INSERT OR REPLACE by src/orb/ingest.ts; its
   // PRIMARY KEY (instance_id, day) is the conflict target the hourly ORB export needs on self-host Postgres.
   orb_reuse_counters: ["instance_id", "day"],
+  // #9016: ai_review_verdict_flips (migrations/0183) is written with ON CONFLICT by recordVerdictFlip;
+  // its PRIMARY KEY (repo_full_name, pull_number) is the conflict target on self-host Postgres.
+  ai_review_verdict_flips: ["repo_full_name", "pull_number"],
   // ams_signals (#8382): TWO columns here, deliberately — this must name the table's REAL unique constraint
   // (`UNIQUE (instance_id, pr_hash)`, migrations/0148_ams_signals.sql), or Postgres rejects the generated
   // `ON CONFLICT` with "no unique or exclusion constraint matching". The 3-column shape orb_signals needed