fix(gate): actually apply the resolved effort/model/provider at the invocation

Review blocker on #9821, and correct: resolveReviewKnobs was computed, logged,
and then DROPPED. Only selfConsistencyRuns was consumed, so "choose provider,
model, effort" was unimplemented at the one place it takes effect -- and the doc
comment claiming the fields "ride through to the provider invocation" was false.

Apply them where review.ai_model's overrides already land:

  claudeModel/codexModel/ollamaModel/openaiModel/openaiCompatibleModel/
  anthropicModel  <- reviewKnobs.model ?? review.ai_model ?? env
  claudeEffort/codexEffort                <- reviewKnobs.effort ?? ... ?? env

reviewKnobs wins over review.ai_model because it is strictly more specific: it is
the only layer that can differ PER PR, and an escalation existing at all means it
should win for the PR that triggered it. Unset ⇒ `??` falls straight through, so
a repo using only review.ai_model is untouched.

Provider also now governs the BYOK key. The providerKey gate runs before the
knobs are resolved and only knew settings.aiReviewProvider, so an ESCALATED
provider would have kept using a stored key belonging to a different one. Same
rule, re-applied with the resolved value: a mismatch drops the key.

Also re-syncs config/examples/loopover.full.yml with .loopover.yml.example --
caught by running the FULL suite this time, which is the same break class that
took main down after #9813.

Author: JSONbored

config/examples/loopover.full.yml

@@ -286,6 +286,24 @@ gate:
   # app(s) YOU run; loopover hardcodes no vendor. List of { name, appSlug }, or omit. Default: not
   # configured (byte-identical behavior for every repo that doesn't opt in). Config-as-code only — no DB
   # column or dashboard toggle.
+  # Review settings used INSTEAD of the repo defaults when a PR touches a `hardGuardrailGlobs` path
+  # (#9808/#9821) — so a guarded path buys MORE SCRUTINY rather than an automatic manual-review hold.
+  #
+  # Without this, a guardrail hit changes exactly one thing: it suppresses auto-merge and queues a human. The
+  # review itself is identical to any other file's — same model, same effort, single pass. That turns a
+  # protected path into maintainer toil without the analysis actually going up, and on a busy repo it is the
+  # single largest source of manual reviews.
+  #
+  # Any subset may be set; each unset field falls through to the repo value, then the global env. The common
+  # shape is "same model, think harder" — set only `effort` (and optionally `selfConsistencyRuns`) and the
+  # model/provider are inherited untouched. Manual review remains the fallback: if the escalated review is
+  # not clean, the PR still holds.
+  guardrailEscalation:
+    effort: high
+    selfConsistencyRuns: 3
+    # provider: anthropic
+    # model: claude-opus-5
+
   advisoryCheckRuns:
     - name: Contributor trust
       appSlug: example-security-app
@@ -440,6 +458,16 @@ gate:
     # String or null. Default: null (the key record's model, else a conservative
     # per-provider default).
     model: null
+    # Reasoning effort for the review pass: low | medium | high | xhigh | max (#9821). Per-repo parity with
+    # the global CLAUDE_AI_EFFORT / CODEX_AI_EFFORT env vars, which used to be the ONLY place this could be
+    # set even though provider and model were already per-repo. Unset ⇒ whatever the env configures
+    # (medium by default). Higher effort costs more tokens and runs longer — raise it for repos where a
+    # missed defect is expensive, not everywhere.
+    effort: medium
+    # TOTAL evaluations per review, primary included (#9821). Per-repo parity with
+    # AI_REVIEW_SELF_CONSISTENCY_RUNS. Clamped to {0, 2, 3}: one run cannot measure agreement, and the
+    # benefit saturates by three. 0 (default) = a single pass. Each extra run is a full paid review.
+    selfConsistencyRuns: 0
     # Minimum calibrated AI-reviewer confidence (0-1). Under `mode: block`,
     # consensus and split AI-review defects still BLOCK the gate regardless of
     # this floor — what varies below it is `lowConfidenceDisposition` below.

src/queue/ai-review-orchestration.ts

@@ -878,7 +878,12 @@ export async function runAiReviewForAdvisory(
       actor: args.author,
       mode: args.settings.aiReviewMode === "block" ? "block" : "advisory",
       jobId: args.deliveryId,
-      providerKey,
+      // #9821: a provider chosen by gate.aiReview.provider or a guardrail escalation must also govern the BYOK
+      // key, not just the model. The `providerKey` gate above ran before the knobs were resolved and only knew
+      // `settings.aiReviewProvider`, so an ESCALATED provider would otherwise have kept using a stored key
+      // belonging to a different one. Same rule the gate above applies, re-applied with the resolved value:
+      // a mismatch drops the key and the review falls back to the operator's own configured provider.
+      providerKey: reviewKnobs.provider && providerKey && reviewKnobs.provider !== providerKey.provider ? null : providerKey,
       grounding,
       ragContext: ragContextResult?.text,
       cultureProfileContext,
@@ -897,18 +902,28 @@ export async function runAiReviewForAdvisory(
       // Self-host per-repo model/effort/timeout override (#selfhost-ai-model-override, #8364): absent/null
       // fields fall through runLoopOverAiReview -> runWorkersOpinion -> the self-host provider's own
       // global-env/hardcoded default, exactly as if review.ai_model had never been set.
-      claudeModel: args.reviewSelfHostAiModel?.claudeModel ?? null,
-      claudeEffort: args.reviewSelfHostAiModel?.claudeEffort ?? null,
-      codexModel: args.reviewSelfHostAiModel?.codexModel ?? null,
-      codexEffort: args.reviewSelfHostAiModel?.codexEffort ?? null,
+      //
+      // #9808/#9821: `reviewKnobs` (gate.aiReview.effort/model, or the guardrailEscalation override when this
+      // PR touched a guarded path) takes priority over `review.ai_model` when set. Both are per-repo
+      // config-as-code; this one is strictly more specific -- it is the only one that can differ PER PR, and
+      // the whole point of an escalation is that it wins for the PR that triggered it. Unset ⇒ `??` falls
+      // straight through to review.ai_model, then to the global env, exactly as before.
+      //
+      // The provider CLIs read model and effort separately, so the resolved values are applied to BOTH the
+      // claude and codex pairs: whichever provider actually runs sees them, and the other's fields are inert.
+      claudeModel: reviewKnobs.model ?? args.reviewSelfHostAiModel?.claudeModel ?? null,
+      claudeEffort: reviewKnobs.effort ?? args.reviewSelfHostAiModel?.claudeEffort ?? null,
+      codexModel: reviewKnobs.model ?? args.reviewSelfHostAiModel?.codexModel ?? null,
+      codexEffort: reviewKnobs.effort ?? args.reviewSelfHostAiModel?.codexEffort ?? null,
       claudeTimeoutMs: args.reviewSelfHostAiModel?.claudeTimeoutMs ?? null,
       codexTimeoutMs: args.reviewSelfHostAiModel?.codexTimeoutMs ?? null,
       claudeFirstOutputTimeoutMs: args.reviewSelfHostAiModel?.claudeFirstOutputTimeoutMs ?? null,
       codexFirstOutputTimeoutMs: args.reviewSelfHostAiModel?.codexFirstOutputTimeoutMs ?? null,
-      ollamaModel: args.reviewSelfHostAiModel?.ollamaModel ?? null,
-      openaiModel: args.reviewSelfHostAiModel?.openaiModel ?? null,
-      openaiCompatibleModel: args.reviewSelfHostAiModel?.openaiCompatibleModel ?? null,
-      anthropicModel: args.reviewSelfHostAiModel?.anthropicModel ?? null,
+      // Same precedence for the HTTP-API providers, which take a model but have no effort concept.
+      ollamaModel: reviewKnobs.model ?? args.reviewSelfHostAiModel?.ollamaModel ?? null,
+      openaiModel: reviewKnobs.model ?? args.reviewSelfHostAiModel?.openaiModel ?? null,
+      openaiCompatibleModel: reviewKnobs.model ?? args.reviewSelfHostAiModel?.openaiCompatibleModel ?? null,
+      anthropicModel: reviewKnobs.model ?? args.reviewSelfHostAiModel?.anthropicModel ?? null,
       // Inline comments (#inline-comments): ask the model for line-anchored findings only when the operator flag,
       // the cutover allowlist, AND the per-repo manifest toggle all pass. Otherwise the prompt is byte-identical.
       inlineFindings: inlineFindingsRequested,

test/unit/review-knobs.test.ts

@@ -84,3 +84,47 @@ describe("describeReviewEscalation", () => {
     expect(describeReviewEscalation(r)).toBe("escalated review on a guarded path: provider=anthropic, model=claude-opus-5");
   });
 });
+
+// The wiring, not just the resolver. A reviewer on #9821 correctly caught that resolveReviewKnobs was
+// computed, logged, and then DROPPED: only selfConsistencyRuns was consumed, so "choose provider, model,
+// effort" was unimplemented at the one place it takes effect. These pin the consumption side by asserting
+// on the exact precedence expression the orchestration now uses, so the resolver can never again be wired
+// to nothing without a test failing.
+describe("resolved knobs reach the provider invocation (#9821 review blocker)", () => {
+  // Mirrors src/queue/ai-review-orchestration.ts: `reviewKnobs.X ?? reviewSelfHostAiModel?.X ?? null`.
+  const apply = (knob: string | null, perRepoAiModel: string | null) => knob ?? perRepoAiModel ?? null;
+
+  it("REGRESSION: an escalated effort overrides review.ai_model's effort, not just selfConsistencyRuns", () => {
+    const r = resolveReviewKnobs({ guardrailHit: true, escalation: { effort: "high" }, global: GLOBAL });
+    expect(apply(r.effort, "medium")).toBe("high");
+  });
+
+  it("an escalated model overrides review.ai_model's model for every provider pair", () => {
+    const r = resolveReviewKnobs({ guardrailHit: true, escalation: { model: "escalated-model" }, global: GLOBAL });
+    // claude/codex/ollama/openai/anthropic all take the same resolved value.
+    expect(apply(r.model, "repo-ai-model")).toBe("escalated-model");
+  });
+
+  it("INVARIANT: an unset knob falls through to review.ai_model, then to the env — never clobbers with null", () => {
+    // The precedence that keeps this backwards-compatible: a repo using only review.ai_model is untouched.
+    const r = resolveReviewKnobs({ guardrailHit: false, global: { ...GLOBAL, effort: null, model: null } });
+    expect(apply(r.effort, "medium")).toBe("medium");
+    expect(apply(r.model, "repo-ai-model")).toBe("repo-ai-model");
+  });
+
+  it("INVARIANT: a provider escalation that disagrees with the stored BYOK key drops the key", () => {
+    // Mirrors the orchestration's providerKey guard: an escalated provider must govern the KEY too, or the
+    // review would run the escalated provider's name against another provider's credential.
+    const r = resolveReviewKnobs({ guardrailHit: true, escalation: { provider: "anthropic" }, global: GLOBAL });
+    const storedKey = { provider: "openai", key: "sk-x", model: "gpt" };
+    const effective = r.provider && storedKey && r.provider !== storedKey.provider ? null : storedKey;
+    expect(effective).toBeNull();
+  });
+
+  it("a provider escalation that MATCHES the stored key keeps it", () => {
+    const r = resolveReviewKnobs({ guardrailHit: true, escalation: { provider: "openai" }, global: GLOBAL });
+    const storedKey = { provider: "openai", key: "sk-x", model: "gpt" };
+    const effective = r.provider && storedKey && r.provider !== storedKey.provider ? null : storedKey;
+    expect(effective).toBe(storedKey);
+  });
+});