fix(review): close two gaps in the per-repo dual-AI combine override

src/queue/processors.ts

@@ -6734,6 +6734,9 @@ async function maybePublishPrPublicSurface(
             model: settings.aiReviewModel,
             aiReviewAllAuthors: settings.aiReviewAllAuthors,
             aiReviewCloseConfidence: settings.aiReviewCloseConfidence,
+            aiReviewCombine: settings.aiReviewCombine,
+            aiReviewOnMerge: settings.aiReviewOnMerge,
+            aiReviewReviewers: settings.aiReviewReviewers,
             gatePack: settings.gatePack,
             reviewerPlan: env.AI_REVIEW_PLAN,
             selfHostProviderConfig: env.AI_REVIEW_PLAN

src/review/ai-review-cache-input.ts

@@ -22,6 +22,14 @@ export type AiReviewCacheInput = {
   // eligibility/interpretation rules.
   aiReviewAllAuthors: boolean;
   aiReviewCloseConfidence: number | null | undefined;
+  // Per-repo dual-AI combine overrides (#2567): these directly shape the EFFECTIVE combine/onMerge/reviewers
+  // resolveEffectiveAiReviewPlan produces (which drives whether/how a consensus defect is computed), separate
+  // from `reviewerPlan` below (the operator's own boot-config plan). A repo flipping any of these warrants a
+  // fresh review under the new effective plan, not a replay of a decision made under the old one -- the same
+  // reasoning as aiReviewCloseConfidence above.
+  aiReviewCombine: string | null | undefined;
+  aiReviewOnMerge: string | null | undefined;
+  aiReviewReviewers: readonly { model: string; fallback?: string | null | undefined }[] | null | undefined;
   gatePack: string | null | undefined;
   reviewerPlan:
     | {
@@ -97,6 +105,16 @@ export async function aiReviewCacheInputFingerprint(input: AiReviewCacheInput):
     model: input.model ?? null,
     aiReviewAllAuthors: input.aiReviewAllAuthors,
     aiReviewCloseConfidence: input.aiReviewCloseConfidence ?? null,
+    aiReviewCombine: input.aiReviewCombine ?? null,
+    aiReviewOnMerge: input.aiReviewOnMerge ?? null,
+    // Nullish (no repo override) and an explicit [] are DIFFERENT effective plans (src/services/ai-review.ts's
+    // resolveEffectiveAiReviewPlan falls through to the built-in default reviewers for nullish but treats an
+    // explicit [] as a real, empty override) -- collapsing both to the same fingerprint would let a same-SHA
+    // cache hit replay a verdict produced under a different effective reviewer plan.
+    aiReviewReviewers:
+      input.aiReviewReviewers == null
+        ? null
+        : input.aiReviewReviewers.map((reviewer) => ({ model: reviewer.model, fallback: reviewer.fallback ?? null })),
     gatePack: input.gatePack ?? null,
     reviewerPlan: input.reviewerPlan
       ? {

src/services/ai-review.ts

@@ -111,6 +111,57 @@ export function resolveEffectiveAiReviewOnMerge(
   return { onMerge: repoOverride, clamped: false };
 }
 
+type AiReviewPlanShape = {
+  combine?: CombineStrategy | null | undefined;
+  onMerge?: OnMerge | null | undefined;
+  reviewers?: ReadonlyArray<{ model: string; fallback?: string | null | undefined }> | null | undefined;
+};
+
+/**
+ * Resolve the FULL effective dual-AI plan (combine + onMerge + reviewers together), extending
+ * resolveEffectiveAiReviewOnMerge to close a gap it left open (gate finding on #2567): clamping `onMerge`
+ * alone does not protect the operator's `either` floor if a repo can ALSO shrink the reviewer count or switch
+ * to `combine: "single"` -- either change reduces the number of independent opinions that can trigger a
+ * blocker, achieving the same effective loosening `onMerge` alone was meant to prevent (an operator plan of
+ * two reviewers under `either` means "either ONE of two can flag it"; drop to one reviewer and there is only
+ * ever one vote to begin with, silently narrowing the floor without ever touching `onMerge`).
+ *
+ * When the operator has NOT set an `either` floor, every field resolves unclamped (repo override, else
+ * operator's own value) -- there is nothing to protect. When the operator HAS set `either`, a repo override
+ * that would reduce the effective reviewer count below the operator's own count (via a shorter `reviewers`
+ * list or a `combine: "single"` switch) is clamped: the repo's `combine`/`reviewers` overrides are ignored
+ * entirely and the operator's own values are used instead, while `onMerge` still resolves normally through
+ * resolveEffectiveAiReviewOnMerge. `clamped` is true if EITHER the onMerge clamp or this reviewer-count clamp
+ * fired, so the caller can surface either kind identically.
+ */
+export function resolveEffectiveAiReviewPlan(
+  repoOverride: AiReviewPlanShape,
+  operatorPlan: AiReviewPlanShape | null | undefined,
+): { combine: CombineStrategy | null | undefined; onMerge: OnMerge | null | undefined; reviewers: AiReviewPlanShape["reviewers"]; clamped: boolean } {
+  const onMergeResolution = resolveEffectiveAiReviewOnMerge(repoOverride.onMerge, operatorPlan?.onMerge);
+  const hasOperatorFloor = operatorPlan?.onMerge === "either";
+  if (hasOperatorFloor) {
+    // The operator's OWN effective reviewer count under their plan -- absent reviewers falls back to the
+    // built-in default pair (2), the historical dual-reviewer behavior (see GittensoryAiReviewInput.reviewers).
+    const operatorReviewerCount = operatorPlan?.reviewers?.length ?? 2;
+    const repoReviewerCount = repoOverride.reviewers?.length ?? operatorReviewerCount;
+    const reducesReviewerCount = repoOverride.reviewers != null && repoReviewerCount < operatorReviewerCount;
+    // Must be the REPO'S OWN combine value, not `repoOverride.combine ?? operatorPlan?.combine` -- that
+    // fallback made an operator plan that itself sets `combine: "single"` (no repo override at all) spuriously
+    // report `clamped: true` on every call, since there is nothing for the repo to have bypassed.
+    const collapsesToSingleReviewer = repoOverride.combine === "single" && operatorReviewerCount > 1;
+    if (reducesReviewerCount || collapsesToSingleReviewer) {
+      return { combine: operatorPlan?.combine, onMerge: onMergeResolution.onMerge, reviewers: operatorPlan?.reviewers, clamped: true };
+    }
+  }
+  return {
+    combine: repoOverride.combine ?? operatorPlan?.combine,
+    onMerge: onMergeResolution.onMerge,
+    reviewers: repoOverride.reviewers ?? operatorPlan?.reviewers,
+    clamped: onMergeResolution.clamped,
+  };
+}
+
 export type GittensoryAiReviewInput = {
   repoFullName: string;
   prNumber: number;
@@ -1115,12 +1166,22 @@ export async function runGittensoryAiReview(
   // combined by `consensus` — byte-identical to today. The self-host boot plan (`env.AI_REVIEW_PLAN`) supplies
   // named providers (e.g. claude-code + codex) and a strategy; an explicit `input` field overrides it. `single`
   // (or a single configured reviewer) runs ONE opinion; consensus/synthesis run two.
+  //
+  // combine/onMerge/reviewers are a per-repo REFINEMENT of the operator's plan, never a bypass (#2567): a repo
+  // can only TIGHTEN the operator's `either` floor, never loosen it by shrinking the reviewer count or
+  // switching to `combine: "single"` either (a floor of "either ONE of two reviewers can flag it" is just as
+  // bypassed by dropping to one reviewer as by flipping onMerge itself). resolveEffectiveAiReviewPlan enforces
+  // the clamp across all three fields together; a fired clamp increments a metric so it is surfaced, not
+  // silently ignored (mirrors the gittensory_ai_review_inconclusive_total pattern below).
   const plan = env.AI_REVIEW_PLAN;
+  const planResolution = resolveEffectiveAiReviewPlan(
+    { combine: input.combine, onMerge: input.onMerge, reviewers: input.reviewers },
+    plan,
+  );
   const configured: ReadonlyArray<{
     model: string;
     fallback?: string | null | undefined;
-  }> | null =
-    (input.reviewers?.length ? input.reviewers : plan?.reviewers) ?? null;
+  }> | null = planResolution.reviewers?.length ? planResolution.reviewers : null;
   const primary = configured?.[0] ?? {
     model: BEST_REVIEW_MODELS[0],
     fallback: RELIABLE_FALLBACK_MODELS[0] as string | null,
@@ -1133,15 +1194,9 @@ export async function runGittensoryAiReview(
   // i.e. runWorkersOpinion's single-model path).
   const primaryFallback = primary.fallback ?? primary.model;
   const secondaryFallback = secondary.fallback ?? secondary.model;
-  const combine: CombineStrategy =
-    input.combine ?? plan?.combine ?? "consensus";
-  // `onMerge` is a per-repo REFINEMENT of the operator's plan, never a bypass (#2567): a repo can only TIGHTEN
-  // the operator's floor (never loosen `either` down to `both`). resolveEffectiveAiReviewOnMerge enforces the
-  // clamp; a fired clamp increments a metric so it is surfaced, not silently ignored (mirrors the
-  // gittensory_ai_review_inconclusive_total pattern below).
-  const onMergeResolution = resolveEffectiveAiReviewOnMerge(input.onMerge, plan?.onMerge);
-  const onMerge = onMergeResolution.onMerge;
-  if (onMergeResolution.clamped) {
+  const combine: CombineStrategy = planResolution.combine ?? "consensus";
+  const onMerge = planResolution.onMerge;
+  if (planResolution.clamped) {
     incr("gittensory_ai_review_onmerge_clamped_total", { mode: input.mode });
   }
   const dual = combine !== "single" && (!configured || configured.length > 1);

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

@@ -12,6 +12,9 @@ const baseInput = (): AiReviewCacheInput => ({
   model: null,
   aiReviewAllAuthors: false,
   aiReviewCloseConfidence: null,
+  aiReviewCombine: null,
+  aiReviewOnMerge: null,
+  aiReviewReviewers: null,
   gatePack: null,
   reviewerPlan: null,
   selfHostProviderConfig: null,
@@ -243,6 +246,35 @@ describe("aiReviewCacheInputFingerprint", () => {
     expect(repeated).toBe(original);
   });
 
+  // #2567 gate-review follow-up: these directly shape the EFFECTIVE combine/onMerge/reviewers plan
+  // (resolveEffectiveAiReviewPlan), which drives whether/how a consensus defect is computed -- a repo
+  // flipping any of them must miss the cache, mirroring aiReviewCloseConfidence's own reasoning above.
+  it("changes when aiReviewCombine, aiReviewOnMerge, or aiReviewReviewers change", async () => {
+    const original = await aiReviewCacheInputFingerprint(baseInput());
+    const combineChanged = await aiReviewCacheInputFingerprint({ ...baseInput(), aiReviewCombine: "synthesis" });
+    const onMergeChanged = await aiReviewCacheInputFingerprint({ ...baseInput(), aiReviewOnMerge: "either" });
+    const reviewersChanged = await aiReviewCacheInputFingerprint({ ...baseInput(), aiReviewReviewers: [{ model: "claude-code" }] });
+    const repeated = await aiReviewCacheInputFingerprint(baseInput());
+
+    expect(combineChanged).not.toBe(original);
+    expect(onMergeChanged).not.toBe(original);
+    expect(reviewersChanged).not.toBe(original);
+    expect(repeated).toBe(original);
+  });
+
+  // REGRESSION (#2567 gate-review follow-up): nullish (no repo override, falls through to the built-in default
+  // reviewers per resolveEffectiveAiReviewPlan) and an explicit [] (a real, empty override) are DIFFERENT
+  // effective plans -- collapsing both to the same fingerprint would let a same-SHA cache hit replay a verdict
+  // produced under the other plan.
+  it("fingerprints aiReviewReviewers: null and aiReviewReviewers: [] DIFFERENTLY -- runtime semantics differ", async () => {
+    const nullish = await aiReviewCacheInputFingerprint({ ...baseInput(), aiReviewReviewers: null });
+    const undef = await aiReviewCacheInputFingerprint({ ...baseInput(), aiReviewReviewers: undefined });
+    const explicitEmpty = await aiReviewCacheInputFingerprint({ ...baseInput(), aiReviewReviewers: [] });
+
+    expect(nullish).toBe(undef);
+    expect(explicitEmpty).not.toBe(nullish);
+  });
+
   it("changes when securityFocus toggles, independently of profile (#review-security-focus)", async () => {
     const original = await aiReviewCacheInputFingerprint(baseInput());
     const securityFocusOn = await aiReviewCacheInputFingerprint({ ...baseInput(), securityFocus: true });

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

@@ -11,6 +11,9 @@ const baseFingerprintInput = (): AiReviewCacheInput => ({
   model: null,
   aiReviewAllAuthors: false,
   aiReviewCloseConfidence: null,
+  aiReviewCombine: null,
+  aiReviewOnMerge: null,
+  aiReviewReviewers: null,
   gatePack: null,
   reviewerPlan: null,
   selfHostProviderConfig: null,

test/unit/ai-review.test.ts

@@ -4,6 +4,7 @@ import {
   BEST_REVIEW_MODELS,
   buildTestEvidencePromptSection,
   resolveEffectiveAiReviewOnMerge,
+  resolveEffectiveAiReviewPlan,
   runGittensoryAiReview,
   type GittensoryAiReviewInput,
 } from "../../src/services/ai-review";
@@ -1133,6 +1134,64 @@ describe("resolveEffectiveAiReviewOnMerge (#2567, pure precedence logic)", () =>
   });
 });
 
+describe("resolveEffectiveAiReviewPlan (#2567 gate-review follow-up: combine/reviewers can't bypass the onMerge floor)", () => {
+  const TWO_REVIEWERS = [{ model: "claude-code" }, { model: "codex" }];
+  const OPERATOR_FLOOR = { combine: "synthesis" as const, onMerge: "either" as const, reviewers: TWO_REVIEWERS };
+
+  it("no operator either-floor ⇒ combine/reviewers resolve unclamped, exactly like a direct override", () => {
+    const noFloor = resolveEffectiveAiReviewPlan({ combine: "single", reviewers: [{ model: "claude-code" }] }, { combine: "synthesis", onMerge: "both", reviewers: TWO_REVIEWERS });
+    expect(noFloor).toEqual({ combine: "single", onMerge: "both", reviewers: [{ model: "claude-code" }], clamped: false });
+
+    const noOperatorPlan = resolveEffectiveAiReviewPlan({ combine: "single", reviewers: [{ model: "claude-code" }] }, null);
+    expect(noOperatorPlan).toEqual({ combine: "single", onMerge: undefined, reviewers: [{ model: "claude-code" }], clamped: false });
+  });
+
+  it("gate finding: an either-floor operator plan cannot be neutered by a repo override reducing reviewer count", () => {
+    const reduced = resolveEffectiveAiReviewPlan({ reviewers: [{ model: "claude-code" }] }, OPERATOR_FLOOR);
+    expect(reduced).toEqual({ combine: "synthesis", onMerge: "either", reviewers: TWO_REVIEWERS, clamped: true });
+  });
+
+  it("gate finding: an either-floor operator plan cannot be neutered by a repo override switching to combine: single", () => {
+    const collapsed = resolveEffectiveAiReviewPlan({ combine: "single" }, OPERATOR_FLOOR);
+    expect(collapsed).toEqual({ combine: "synthesis", onMerge: "either", reviewers: TWO_REVIEWERS, clamped: true });
+  });
+
+  it("an either-floor operator plan with an UNCONFIGURED reviewers list (implicit default pair of 2) is still protected", () => {
+    const collapsed = resolveEffectiveAiReviewPlan({ combine: "single" }, { combine: "consensus", onMerge: "either", reviewers: undefined });
+    expect(collapsed).toEqual({ combine: "consensus", onMerge: "either", reviewers: undefined, clamped: true });
+  });
+
+  it("a repo override that keeps (or increases) the reviewer count and does not collapse to single passes through unclamped", () => {
+    const sameCount = resolveEffectiveAiReviewPlan({ combine: "consensus", reviewers: [{ model: "claude-code" }, { model: "ollama" }] }, OPERATOR_FLOOR);
+    expect(sameCount).toEqual({ combine: "consensus", onMerge: "either", reviewers: [{ model: "claude-code" }, { model: "ollama" }], clamped: false });
+  });
+
+  it("a repo tightening onMerge to either under an either floor is unaffected by the reviewer-count clamp (no reviewers/combine override at all)", () => {
+    const tightened = resolveEffectiveAiReviewPlan({ onMerge: "either" }, OPERATOR_FLOOR);
+    expect(tightened).toEqual({ combine: "synthesis", onMerge: "either", reviewers: TWO_REVIEWERS, clamped: false });
+  });
+
+  it("the onMerge clamp still fires independently when combine/reviewers are untouched", () => {
+    const onMergeOnly = resolveEffectiveAiReviewPlan({ onMerge: "both" }, OPERATOR_FLOOR);
+    expect(onMergeOnly).toEqual({ combine: "synthesis", onMerge: "either", reviewers: TWO_REVIEWERS, clamped: true });
+  });
+
+  // REGRESSION (gate-review follow-up on this same PR): the reviewer-count clamp must only fire on a REPO'S OWN
+  // combine override -- an operator plan that itself already sets combine: "single" (no repo override at all)
+  // must NOT be reported as clamped, since there is nothing for a repo to have bypassed.
+  it("an operator plan whose OWN combine is 'single' does not spuriously report clamped when the repo has no combine override at all", () => {
+    const operatorSingle = { combine: "single" as const, onMerge: "either" as const, reviewers: TWO_REVIEWERS };
+    const noRepoOverride = resolveEffectiveAiReviewPlan({}, operatorSingle);
+    expect(noRepoOverride).toEqual({ combine: "single", onMerge: "either", reviewers: TWO_REVIEWERS, clamped: false });
+  });
+
+  it("an operator plan whose OWN combine is 'single' is STILL clamped when the repo separately tries to reduce the reviewer count", () => {
+    const operatorSingle = { combine: "single" as const, onMerge: "either" as const, reviewers: TWO_REVIEWERS };
+    const reduced = resolveEffectiveAiReviewPlan({ reviewers: [{ model: "claude-code" }] }, operatorSingle);
+    expect(reduced).toEqual({ combine: "single", onMerge: "either", reviewers: TWO_REVIEWERS, clamped: true });
+  });
+});
+
 describe("pure helpers", () => {
   it("toPublicSafe drops forbidden public text and neutralizes markdown, mentions, links, and control characters", () => {
     expect(toPublicSafe("This change is solid.")).toBe("This change is solid.");