diff --git a/.loopover.yml.example b/.loopover.yml.example index a32890c11..15d1ab316 100644 --- a/.loopover.yml.example +++ b/.loopover.yml.example @@ -272,6 +272,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 @@ -426,6 +444,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. diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 53ed127f6..f36c1fe30 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -10146,6 +10146,51 @@ "appSlug" ] } + }, + "aiReviewEffort": { + "type": "string", + "nullable": true, + "enum": [ + "low", + "medium", + "high", + "xhigh", + "max", + null + ] + }, + "aiReviewSelfConsistencyRuns": { + "type": "number", + "nullable": true + }, + "guardrailEscalationProvider": { + "type": "string", + "nullable": true, + "enum": [ + "anthropic", + "openai", + null + ] + }, + "guardrailEscalationModel": { + "type": "string", + "nullable": true + }, + "guardrailEscalationEffort": { + "type": "string", + "nullable": true, + "enum": [ + "low", + "medium", + "high", + "xhigh", + "max", + null + ] + }, + "guardrailEscalationSelfConsistencyRuns": { + "type": "number", + "nullable": true } }, "required": [ diff --git a/config/examples/loopover.full.yml b/config/examples/loopover.full.yml index 24056fc0c..30bb77ce2 100644 --- a/config/examples/loopover.full.yml +++ b/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. diff --git a/packages/loopover-contract/src/api-schemas.ts b/packages/loopover-contract/src/api-schemas.ts index 2664a44c3..d92b26f14 100644 --- a/packages/loopover-contract/src/api-schemas.ts +++ b/packages/loopover-contract/src/api-schemas.ts @@ -453,6 +453,12 @@ export const RepositorySettingsSchema = z expectedCiContexts: z.array(z.string()).readonly().nullable().optional(), advisoryCheckRuns: z.array(z.object({ name: z.string(), appSlug: z.string() })).readonly().nullable().optional(), ignoredCheckRuns: z.array(z.object({ name: z.string(), appSlug: z.string() })).readonly().nullable().optional(), + aiReviewEffort: z.enum(["low", "medium", "high", "xhigh", "max"]).nullable().optional(), + aiReviewSelfConsistencyRuns: z.number().nullable().optional(), + guardrailEscalationProvider: z.enum(["anthropic", "openai"]).nullable().optional(), + guardrailEscalationModel: z.string().nullable().optional(), + guardrailEscalationEffort: z.enum(["low", "medium", "high", "xhigh", "max"]).nullable().optional(), + guardrailEscalationSelfConsistencyRuns: z.number().nullable().optional(), copycatGateMode: z.enum(["off", "warn", "label", "block"]).optional(), copycatGateMinScore: z.number().nullable().optional(), gateDryRun: z.boolean().optional(), diff --git a/packages/loopover-engine/src/focus-manifest.ts b/packages/loopover-engine/src/focus-manifest.ts index f01546976..8c9fe7209 100644 --- a/packages/loopover-engine/src/focus-manifest.ts +++ b/packages/loopover-engine/src/focus-manifest.ts @@ -114,6 +114,22 @@ export type FocusManifestGateConfig = { aiReviewByok: boolean | null; aiReviewProvider: "anthropic" | "openai" | null; aiReviewModel: string | null; + /** `gate.aiReview.effort` (#9821): reasoning effort for the review pass — low | medium | high | xhigh | max. + * Per-repo parity with the global CLAUDE_AI_EFFORT/CODEX_AI_EFFORT env vars, which were previously the ONLY + * way to set it: provider and model were already per-repo, effort was not. Unset ⇒ the env default. */ + aiReviewEffort: "low" | "medium" | "high" | "xhigh" | "max" | null; + /** `gate.aiReview.selfConsistencyRuns` (#9821): TOTAL evaluations per review, primary included. Per-repo + * parity with AI_REVIEW_SELF_CONSISTENCY_RUNS. Clamped downstream to {0,2,3} -- one run cannot measure + * agreement and the benefit saturates by three. Unset ⇒ the env default. */ + aiReviewSelfConsistencyRuns: number | null; + /** `gate.guardrailEscalation` (#9808/#9821): the review settings to use INSTEAD of the repo defaults when a + * PR touches a `hardGuardrailGlobs` path. Any subset may be given; each unset field falls through to the + * repo/global value. This is what makes a guarded path buy MORE SCRUTINY rather than a manual hold -- + * today a guardrail hit reviews identically to any other file and merely stops the merge. */ + guardrailEscalationProvider: "anthropic" | "openai" | null; + guardrailEscalationModel: string | null; + guardrailEscalationEffort: "low" | "medium" | "high" | "xhigh" | "max" | null; + guardrailEscalationSelfConsistencyRuns: number | null; aiReviewAllAuthors: boolean | null; /** `gate.aiReview.closeConfidence` (#7): minimum calibrated AI-reviewer confidence (0-1) for an AI defect to BLOCK * under `aiReview.mode: block`. null (unset) ⇒ the gate's 0.93 default. Clamped to [0,1] at parse time. */ @@ -631,6 +647,12 @@ export type FocusManifestSettings = Partial< | "aiReviewByok" | "aiReviewProvider" | "aiReviewModel" + | "aiReviewEffort" + | "aiReviewSelfConsistencyRuns" + | "guardrailEscalationProvider" + | "guardrailEscalationModel" + | "guardrailEscalationEffort" + | "guardrailEscalationSelfConsistencyRuns" | "aiReviewAllAuthors" | "aiReviewConfirmedContributorsOnly" | "closeOwnerAuthors" @@ -1342,6 +1364,12 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = { aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, + aiReviewEffort: null, + aiReviewSelfConsistencyRuns: null, + guardrailEscalationProvider: null, + guardrailEscalationModel: null, + guardrailEscalationEffort: null, + guardrailEscalationSelfConsistencyRuns: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewSalvageabilityMinScore: null, @@ -1815,6 +1843,7 @@ const GATE_TOP_LEVEL_KEYS = new Set([ "expectedCiContexts", "advisoryCheckRuns", "ignoredCheckRuns", + "guardrailEscalation", "aiJudgmentBlockers", "copycat", ]); @@ -1837,6 +1866,10 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu } const aiReview = record.aiReview; const aiReviewRecord = aiReview !== null && typeof aiReview === "object" && !Array.isArray(aiReview) ? (aiReview as Record) : undefined; + // #9821: `gate.guardrailEscalation` — same "nested mapping, each field optional" shape as gate.aiReview above. + const escalation = record.guardrailEscalation; + const escalationRecord = + escalation !== null && typeof escalation === "object" && !Array.isArray(escalation) ? (escalation as Record) : undefined; if (aiReview !== undefined && aiReview !== null && aiReviewRecord === undefined) { warnings.push(`Manifest gate field "gate.aiReview" must be a mapping; ignoring it.`); } @@ -1881,6 +1914,12 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu aiReviewByok: normalizeOptionalBoolean(aiReviewRecord?.byok, "gate.aiReview.byok", warnings), aiReviewProvider: normalizeOptionalEnum(aiReviewRecord?.provider, "gate.aiReview.provider", ["anthropic", "openai"] as const, warnings), aiReviewModel: normalizeOptionalString(aiReviewRecord?.model, "gate.aiReview.model", warnings), + aiReviewEffort: normalizeOptionalEnum(aiReviewRecord?.effort, "gate.aiReview.effort", ["low", "medium", "high", "xhigh", "max"] as const, warnings), + aiReviewSelfConsistencyRuns: normalizeOptionalNonNegativeInt(aiReviewRecord?.selfConsistencyRuns, "gate.aiReview.selfConsistencyRuns", warnings), + guardrailEscalationProvider: normalizeOptionalEnum(escalationRecord?.provider, "gate.guardrailEscalation.provider", ["anthropic", "openai"] as const, warnings), + guardrailEscalationModel: normalizeOptionalString(escalationRecord?.model, "gate.guardrailEscalation.model", warnings), + guardrailEscalationEffort: normalizeOptionalEnum(escalationRecord?.effort, "gate.guardrailEscalation.effort", ["low", "medium", "high", "xhigh", "max"] as const, warnings), + guardrailEscalationSelfConsistencyRuns: normalizeOptionalNonNegativeInt(escalationRecord?.selfConsistencyRuns, "gate.guardrailEscalation.selfConsistencyRuns", warnings), aiReviewAllAuthors: normalizeOptionalBoolean(aiReviewRecord?.allAuthors, "gate.aiReview.allAuthors", warnings), aiReviewCloseConfidence: normalizeOptionalConfidence(aiReviewRecord?.closeConfidence, "gate.aiReview.closeConfidence", warnings), aiReviewSalvageabilityMinScore: normalizeOptionalScore(aiReviewRecord?.salvageabilityMinScore, "gate.aiReview.salvageabilityMinScore", warnings), @@ -1947,6 +1986,8 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu gate.aiReviewByok !== null || gate.aiReviewProvider !== null || gate.aiReviewModel !== null || + gate.aiReviewEffort !== null || + gate.aiReviewSelfConsistencyRuns !== null || gate.aiReviewAllAuthors !== null || gate.aiReviewCloseConfidence !== null || gate.aiReviewSalvageabilityMinScore !== null || @@ -1970,6 +2011,12 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu gate.claCheckRunAppSlug !== null || gate.expectedCiContexts !== null || gate.advisoryCheckRuns !== null || + gate.aiReviewEffort !== null || + gate.aiReviewSelfConsistencyRuns !== null || + gate.guardrailEscalationProvider !== null || + gate.guardrailEscalationModel !== null || + gate.guardrailEscalationEffort !== null || + gate.guardrailEscalationSelfConsistencyRuns !== null || gate.ignoredCheckRuns !== null || gate.aiJudgmentBlockersMode !== null || gate.copycatMode !== null || @@ -2029,13 +2076,17 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue { gate.aiReviewLowConfidenceDisposition !== null || gate.aiReviewCombine !== null || gate.aiReviewOnMerge !== null || - gate.aiReviewReviewers !== null + gate.aiReviewReviewers !== null || + gate.aiReviewEffort !== null || + gate.aiReviewSelfConsistencyRuns !== null ) { const aiReview: Record = {}; if (gate.aiReviewMode !== null) aiReview.mode = gate.aiReviewMode; if (gate.aiReviewByok !== null) aiReview.byok = gate.aiReviewByok; if (gate.aiReviewProvider !== null) aiReview.provider = gate.aiReviewProvider; if (gate.aiReviewModel !== null) aiReview.model = gate.aiReviewModel; + if (gate.aiReviewEffort !== null) aiReview.effort = gate.aiReviewEffort; + if (gate.aiReviewSelfConsistencyRuns !== null) aiReview.selfConsistencyRuns = gate.aiReviewSelfConsistencyRuns; if (gate.aiReviewAllAuthors !== null) aiReview.allAuthors = gate.aiReviewAllAuthors; if (gate.aiReviewCloseConfidence !== null) aiReview.closeConfidence = gate.aiReviewCloseConfidence; if (gate.aiReviewSalvageabilityMinScore !== null) aiReview.salvageabilityMinScore = gate.aiReviewSalvageabilityMinScore; @@ -2049,6 +2100,20 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue { } out.aiReview = aiReview; } + // #9821: guardrail escalation round-trips as its own nested mapping, emitted only when something is set. + if ( + gate.guardrailEscalationProvider !== null || + gate.guardrailEscalationModel !== null || + gate.guardrailEscalationEffort !== null || + gate.guardrailEscalationSelfConsistencyRuns !== null + ) { + const escalation: Record = {}; + if (gate.guardrailEscalationProvider !== null) escalation.provider = gate.guardrailEscalationProvider; + if (gate.guardrailEscalationModel !== null) escalation.model = gate.guardrailEscalationModel; + if (gate.guardrailEscalationEffort !== null) escalation.effort = gate.guardrailEscalationEffort; + if (gate.guardrailEscalationSelfConsistencyRuns !== null) escalation.selfConsistencyRuns = gate.guardrailEscalationSelfConsistencyRuns; + out.guardrailEscalation = escalation; + } if (gate.mergeReadiness !== null) out.mergeReadiness = gate.mergeReadiness; if (gate.manifestPolicy !== null) out.manifestPolicy = gate.manifestPolicy; if (gate.selfAuthoredLinkedIssue !== null) out.selfAuthoredLinkedIssue = gate.selfAuthoredLinkedIssue; diff --git a/packages/loopover-engine/src/types/manifest-deps-types.ts b/packages/loopover-engine/src/types/manifest-deps-types.ts index 0c270443c..d4a55249e 100644 --- a/packages/loopover-engine/src/types/manifest-deps-types.ts +++ b/packages/loopover-engine/src/types/manifest-deps-types.ts @@ -296,6 +296,19 @@ export type RepositorySettings = { /** Config-as-code model override for the BYOK advisory write-up (e.g. "claude-3-5-sonnet-latest"). * `null` = use the key record's model, else a conservative per-provider default. */ aiReviewModel?: string | null | undefined; + /** `gate.aiReview.effort` (#9821): reasoning effort for the review pass (low | medium | high | xhigh | max). + * Per-repo parity with the global CLAUDE_AI_EFFORT / CODEX_AI_EFFORT env vars. Unset ⇒ the env default. */ + aiReviewEffort?: "low" | "medium" | "high" | "xhigh" | "max" | null | undefined; + /** `gate.aiReview.selfConsistencyRuns` (#9821): TOTAL evaluations per review, primary included. Per-repo + * parity with AI_REVIEW_SELF_CONSISTENCY_RUNS; clamped downstream to {0,2,3}. Unset ⇒ the env default. */ + aiReviewSelfConsistencyRuns?: number | null | undefined; + /** `gate.guardrailEscalation.*` (#9808/#9821): review settings used INSTEAD of the repo defaults when a PR + * touches a hardGuardrailGlobs path, so a guarded path buys MORE SCRUTINY rather than a manual hold. Each + * field falls through to the repo/global value when unset. */ + guardrailEscalationProvider?: "anthropic" | "openai" | null | undefined; + guardrailEscalationModel?: string | null | undefined; + guardrailEscalationEffort?: "low" | "medium" | "high" | "xhigh" | "max" | null | undefined; + guardrailEscalationSelfConsistencyRuns?: number | null | undefined; /** Review EVERY PR's author, not only confirmed Gittensor contributors. Only meaningful when * {@link aiReviewConfirmedContributorsOnly} is also `true` (that field opts INTO confirmed-only * scoping in the first place — see its own doc comment for the full invariant: AI review runs for diff --git a/packages/loopover-engine/test/focus-manifest-review-knobs.test.ts b/packages/loopover-engine/test/focus-manifest-review-knobs.test.ts new file mode 100644 index 000000000..8a716e3d9 --- /dev/null +++ b/packages/loopover-engine/test/focus-manifest-review-knobs.test.ts @@ -0,0 +1,90 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { gateConfigToJson, parseFocusManifest } from "../dist/focus-manifest.js"; + +// #9821 added per-repo `gate.aiReview.effort`/`selfConsistencyRuns` and the `gate.guardrailEscalation` block. +// These live in the ENGINE, which has its own c8 coverage run over dist/ with `--all` — so every line counts +// whether or not a test loads it, and the root vitest suite's coverage of the same source does NOT satisfy it +// (that report only instruments the 11 lines it actually executes; the engine report instruments all 66). +// Codecov merges both, so the engine's own suite has to exercise these branches or they read as uncovered. + +test("parses the new aiReview knobs and the guardrailEscalation block", () => { + const parsed = parseFocusManifest({ + gate: { + aiReview: { effort: "high", selfConsistencyRuns: 3 }, + guardrailEscalation: { provider: "anthropic", model: "claude-opus-5", effort: "xhigh", selfConsistencyRuns: 2 }, + }, + }); + assert.equal(parsed.gate.present, true); + assert.equal(parsed.gate.aiReviewEffort, "high"); + assert.equal(parsed.gate.aiReviewSelfConsistencyRuns, 3); + assert.equal(parsed.gate.guardrailEscalationProvider, "anthropic"); + assert.equal(parsed.gate.guardrailEscalationModel, "claude-opus-5"); + assert.equal(parsed.gate.guardrailEscalationEffort, "xhigh"); + assert.equal(parsed.gate.guardrailEscalationSelfConsistencyRuns, 2); +}); + +test("each new field ALONE makes the gate present", () => { + // The #9813 presence-gap class, per field: a gate block setting only one of these must not parse as absent, + // or the whole block is silently discarded. + for (const gate of [ + { aiReview: { effort: "low" } }, + { aiReview: { selfConsistencyRuns: 0 } }, + { guardrailEscalation: { provider: "openai" } }, + { guardrailEscalation: { model: "m" } }, + { guardrailEscalation: { effort: "max" } }, + { guardrailEscalation: { selfConsistencyRuns: 3 } }, + ]) { + assert.equal(parseFocusManifest({ gate }).gate.present, true, JSON.stringify(gate)); + } +}); + +test("serialize round-trips the new fields, full and partial", () => { + // The serialize guard was genuinely broken until a round-trip assertion caught it: an aiReview block setting + // ONLY effort/selfConsistencyRuns emitted nothing, so the setting vanished on the next snapshot reload. + for (const gate of [ + { aiReview: { effort: "high", selfConsistencyRuns: 3 }, guardrailEscalation: { effort: "xhigh", selfConsistencyRuns: 2, provider: "anthropic", model: "m" } }, + { aiReview: { effort: "medium" } }, + { aiReview: { selfConsistencyRuns: 2 } }, + { guardrailEscalation: { effort: "high" } }, + { guardrailEscalation: { selfConsistencyRuns: 3 } }, + ]) { + const parsed = parseFocusManifest({ gate }); + const round = parseFocusManifest({ gate: gateConfigToJson(parsed.gate) }); + assert.deepEqual(round.gate, parsed.gate, JSON.stringify(gate)); + } +}); + +test("invalid values warn and stay null rather than being coerced", () => { + const parsed = parseFocusManifest({ + gate: { + aiReview: { effort: "ultra", selfConsistencyRuns: -1 }, + guardrailEscalation: { provider: "grok", effort: 7, selfConsistencyRuns: "three", model: 42 }, + }, + }); + assert.equal(parsed.gate.aiReviewEffort, null); + assert.equal(parsed.gate.aiReviewSelfConsistencyRuns, null); + assert.equal(parsed.gate.guardrailEscalationProvider, null); + assert.equal(parsed.gate.guardrailEscalationEffort, null); + assert.equal(parsed.gate.guardrailEscalationSelfConsistencyRuns, null); + assert.equal(parsed.gate.guardrailEscalationModel, null); + assert.ok(parsed.warnings.some((w) => /gate\.aiReview\.effort/.test(w))); + assert.ok(parsed.warnings.some((w) => /gate\.guardrailEscalation\.provider/.test(w))); +}); + +test("a non-mapping guardrailEscalation is ignored wholesale, and absence leaves every field null", () => { + const notMapping = parseFocusManifest({ gate: { guardrailEscalation: "high", claMode: "advisory" } }); + assert.equal(notMapping.gate.guardrailEscalationEffort, null); + + const absent = parseFocusManifest({ gate: { claMode: "advisory" } }); + assert.equal(absent.gate.aiReviewEffort, null); + assert.equal(absent.gate.aiReviewSelfConsistencyRuns, null); + assert.equal(absent.gate.guardrailEscalationProvider, null); + assert.equal(absent.gate.guardrailEscalationModel, null); + assert.equal(absent.gate.guardrailEscalationEffort, null); + assert.equal(absent.gate.guardrailEscalationSelfConsistencyRuns, null); + // Absent ⇒ the serializer emits no escalation block at all (not an empty one). + const json = gateConfigToJson(absent.gate) as Record; + assert.equal(json.guardrailEscalation, undefined); +}); diff --git a/scripts/check-docs-drift.ts b/scripts/check-docs-drift.ts index 05c456485..c0503ab33 100644 --- a/scripts/check-docs-drift.ts +++ b/scripts/check-docs-drift.ts @@ -174,6 +174,14 @@ export const SETTINGS_ALIAS_MANIFEST: AliasManifestRow[] = [ { field: "aiReviewByok", aliases: ["byok"] }, { field: "aiReviewProvider", aliases: ["aiReview:"] }, { field: "aiReviewModel", aliases: ["aiReview:"] }, + // #9821: nested under the same `aiReview:` mapping as provider/model above. + { field: "aiReviewEffort", aliases: ["aiReview:"] }, + { field: "aiReviewSelfConsistencyRuns", aliases: ["selfConsistencyRuns"] }, + // #9808/#9821: the guardrail-escalation block is its own nested mapping; every field documents under it. + { field: "guardrailEscalationProvider", aliases: ["guardrailEscalation:"] }, + { field: "guardrailEscalationModel", aliases: ["guardrailEscalation:"] }, + { field: "guardrailEscalationEffort", aliases: ["guardrailEscalation:"] }, + { field: "guardrailEscalationSelfConsistencyRuns", aliases: ["guardrailEscalation:"] }, { field: "aiReviewAllAuthors", aliases: ["allAuthors"] }, { field: "aiReviewCloseConfidence", aliases: ["closeConfidence"] }, { field: "aiReviewSalvageabilityMinScore", aliases: ["salvageabilityMinScore"] }, diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index b50132276..a6e565eba 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -748,6 +748,12 @@ export const RepositorySettingsSchema = z expectedCiContexts: z.array(z.string()).readonly().nullable().optional(), advisoryCheckRuns: z.array(z.object({ name: z.string(), appSlug: z.string() })).readonly().nullable().optional(), ignoredCheckRuns: z.array(z.object({ name: z.string(), appSlug: z.string() })).readonly().nullable().optional(), + aiReviewEffort: z.enum(["low", "medium", "high", "xhigh", "max"]).nullable().optional(), + aiReviewSelfConsistencyRuns: z.number().nullable().optional(), + guardrailEscalationProvider: z.enum(["anthropic", "openai"]).nullable().optional(), + guardrailEscalationModel: z.string().nullable().optional(), + guardrailEscalationEffort: z.enum(["low", "medium", "high", "xhigh", "max"]).nullable().optional(), + guardrailEscalationSelfConsistencyRuns: z.number().nullable().optional(), copycatGateMode: z.enum(["off", "warn", "label", "block"]).optional(), copycatGateMinScore: z.number().nullable().optional(), gateDryRun: z.boolean().optional(), diff --git a/src/queue/ai-review-orchestration.ts b/src/queue/ai-review-orchestration.ts index 5d2d2dd97..b2319f5bb 100644 --- a/src/queue/ai-review-orchestration.ts +++ b/src/queue/ai-review-orchestration.ts @@ -14,6 +14,8 @@ // (staying) and this file's own runAiReviewForAdvisory, so processors.ts imports it back rather than this // file importing it from processors.ts, keeping the dependency one-directional. +import { describeReviewEscalation, resolveReviewKnobs } from "../review/review-knobs"; +import { isGuardrailHit } from "../signals/change-guardrail"; import { claimTransientLock, releaseTransientLockIfOwner, @@ -833,7 +835,41 @@ export async function runAiReviewForAdvisory( args.repoFullName, args.reviewInlineComments, ); + // #9808/#9821: resolve the effective review knobs for THIS PR before invoking. A PR touching a + // `hardGuardrailGlobs` path escalates -- higher reasoning effort, more independent runs, optionally a + // different model/provider -- instead of reviewing identically to every other file and merely being held + // for a human afterwards. isGuardrailHit is the SHARED matcher (src/signals/change-guardrail.ts), never a + // second copy. Every layer unset ⇒ null throughout ⇒ byte-identical to the previous behavior. + const reviewKnobs = resolveReviewKnobs({ + guardrailHit: isGuardrailHit(files.map((file) => file.path), args.settings.hardGuardrailGlobs ?? []), + escalation: { + provider: args.settings.guardrailEscalationProvider ?? null, + model: args.settings.guardrailEscalationModel ?? null, + effort: args.settings.guardrailEscalationEffort ?? null, + selfConsistencyRuns: args.settings.guardrailEscalationSelfConsistencyRuns ?? null, + }, + repo: { + provider: args.settings.aiReviewProvider ?? null, + model: args.settings.aiReviewModel ?? null, + effort: args.settings.aiReviewEffort ?? null, + selfConsistencyRuns: args.settings.aiReviewSelfConsistencyRuns ?? null, + }, + }); + const escalationNote = describeReviewEscalation(reviewKnobs); + if (escalationNote) { + console.log( + JSON.stringify({ + event: "ai_review_guardrail_escalated", + repo: args.repoFullName, + pr: args.pr.number, + fields: reviewKnobs.escalatedFields, + effort: reviewKnobs.effort, + selfConsistencyRuns: reviewKnobs.selfConsistencyRuns, + }), + ); + } const result = await runLoopOverAiReview(env, { + reviewKnobs, repoFullName: args.repoFullName, prNumber: args.pr.number, title: args.pr.title, @@ -842,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, @@ -861,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, diff --git a/src/review/review-knobs.ts b/src/review/review-knobs.ts new file mode 100644 index 000000000..2ec981966 --- /dev/null +++ b/src/review/review-knobs.ts @@ -0,0 +1,93 @@ +// The effective AI-review knobs for one PR — provider, model, reasoning effort, self-consistency runs +// (#9808/#9821). +// +// (Named "knobs", not "effort", because src/review/review-effort.ts already owns a different meaning of that +// word: the deterministic estimate of how many MINUTES a HUMAN should budget. The `effort` field here is the +// model's own reasoning effort — `claude --effort` / Codex `model_reasoning_effort`.) +// +// WHY THIS EXISTS: a `hardGuardrailGlobs` hit used to change exactly one thing — it suppressed auto-merge and +// held the PR for a human. It bought no extra analysis whatsoever: a PR touching `.github/workflows/**` got the +// same single-pass, same-model, same-effort review as a README typo. Measured on the production ORB that was 74 +// distinct PRs held in 14 days, on repos whose guardrail lists had already been narrowed twice. Automation was +// being replaced by a maintainer queue without the scrutiny actually going up. +// +// A guardrail hit can now ESCALATE instead: higher reasoning effort, more independent runs, optionally a +// different model or provider. Manual review remains the fallback for genuine uncertainty — the disposition +// still holds when the escalated review is not clean — but it is no longer the automatic price of touching a +// protected path. +// +// PRECEDENCE, most specific wins: +// guardrail escalation (ONLY when the PR hits a guarded path) +// -> per-repo `gate.aiReview.*` (.loopover.yml) +// -> global env (CLAUDE_AI_EFFORT / AI_REVIEW_SELF_CONSISTENCY_RUNS / CLAUDE_AI_MODEL / AI_PROVIDER) +// +// Each field resolves INDEPENDENTLY: an escalation block that sets only `effort` still inherits the repo's +// model and the global provider. That matters because the common request is "same model, think harder" — a +// maintainer raising scrutiny usually does not want to change which model reviews their code. + +/** The four knobs a maintainer can choose at any layer. `null`/absent ⇒ inherit the next layer down. */ +export type ReviewKnobs = { + provider: string | null; + model: string | null; + effort: string | null; + selfConsistencyRuns: number | null; +}; + +export type ResolvedReviewKnobs = ReviewKnobs & { + /** True when the escalation layer supplied at least one value — i.e. this PR is reviewed MORE heavily + * because it touches a protected path. Surfaced so the panel and the decision record can say so: + * "escalated review on a guarded path: effort=high, 3 runs" is what makes the gate legible to a + * contributor, and what makes an auto-merge on a guarded path defensible in the audit trail. */ + escalated: boolean; + /** Exactly which fields the escalation layer changed, for that same surfacing. Empty when not escalated. */ + escalatedFields: string[]; +}; + +const KNOB_ORDER = ["provider", "model", "effort", "selfConsistencyRuns"] as const; + +/** + * PURE: resolve the effective knobs for one review. + * + * `guardrailHit` is the caller's own `isGuardrailHit(changedPaths, hardGuardrailGlobs)` result. This module + * deliberately does NOT re-derive it, so there is exactly one definition of "does this PR touch a guarded + * path" (src/signals/change-guardrail.ts, shared with the engine) rather than a second one free to drift. + */ +export function resolveReviewKnobs(args: { + guardrailHit: boolean; + escalation?: Partial | undefined; + repo?: Partial | undefined; + global?: Partial | undefined; +}): ResolvedReviewKnobs { + const escalatedFields: string[] = []; + const resolved: ReviewKnobs = { provider: null, model: null, effort: null, selfConsistencyRuns: null }; + + // Fixed order so escalatedFields reads in a stable, reportable sequence rather than in whatever order a + // consumer happens to destructure. + for (const field of KNOB_ORDER) { + const escalated = args.guardrailHit ? args.escalation?.[field] : null; + if (escalated !== null && escalated !== undefined) { + escalatedFields.push(field); + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- one assignment across a union of field types + (resolved as any)[field] = escalated; + continue; + } + const repo = args.repo?.[field]; + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- as above + (resolved as any)[field] = repo !== null && repo !== undefined ? repo : (args.global?.[field] ?? null); + } + + return { ...resolved, escalated: escalatedFields.length > 0, escalatedFields }; +} + +/** Human-readable summary of an escalation for the panel and the decision record. Null when not escalated, so + * a caller can fold it into a surface without a branch of its own. */ +export function describeReviewEscalation(resolved: ResolvedReviewKnobs): string | null { + const parts: string[] = []; + if (resolved.escalatedFields.includes("provider") && resolved.provider) parts.push(`provider=${resolved.provider}`); + if (resolved.escalatedFields.includes("model") && resolved.model) parts.push(`model=${resolved.model}`); + if (resolved.escalatedFields.includes("effort") && resolved.effort) parts.push(`effort=${resolved.effort}`); + if (resolved.escalatedFields.includes("selfConsistencyRuns") && resolved.selfConsistencyRuns !== null) { + parts.push(`${resolved.selfConsistencyRuns} runs`); + } + return parts.length > 0 ? `escalated review on a guarded path: ${parts.join(", ")}` : null; +} diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 36f5d3ba4..bb36116cb 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -219,6 +219,14 @@ export function resolveEffectiveAiReviewPlan( } export type LoopOverAiReviewInput = { + /** #9808/#9821: the effective review knobs the caller resolved for THIS PR (resolveReviewKnobs) -- already + * layered escalation > per-repo > global, so this module never re-derives precedence. Absent ⇒ the env + * defaults exactly as before. Consumed HERE: `selfConsistencyRuns` outranks + * AI_REVIEW_SELF_CONSISTENCY_RUNS, and `model`/`effort` outrank the review.ai_model fields on the + * AiRunCorrelation that rides to every provider invocation. `provider` is the one field NOT consumed in + * this module: it governs the BYOK key selection, which happens at the orchestration BEFORE this runs + * (a mismatched stored key is dropped there) -- it is carried on this object for logging/replay only. */ + reviewKnobs?: { provider?: string | null; model?: string | null; effort?: string | null; selfConsistencyRuns?: number | null } | undefined; repoFullName: string; prNumber: number; title: string; @@ -3106,18 +3114,21 @@ export async function runLoopOverAiReview( jobId: input.jobId, repoFullName: input.repoFullName, pullNumber: input.prNumber, - claudeModel: input.claudeModel ?? undefined, - claudeEffort: input.claudeEffort ?? undefined, - codexModel: input.codexModel ?? undefined, - codexEffort: input.codexEffort ?? undefined, + // #9821: reviewKnobs (per-repo gate.aiReview.*, or the guardrailEscalation override on a guarded path) + // outrank the review.ai_model fields -- the same precedence the orchestration pre-applies; folding it + // here too makes THIS module honor the knobs for any caller, and is idempotent when both did. + claudeModel: input.reviewKnobs?.model ?? input.claudeModel ?? undefined, + claudeEffort: input.reviewKnobs?.effort ?? input.claudeEffort ?? undefined, + codexModel: input.reviewKnobs?.model ?? input.codexModel ?? undefined, + codexEffort: input.reviewKnobs?.effort ?? input.codexEffort ?? undefined, claudeTimeoutMs: input.claudeTimeoutMs ?? undefined, codexTimeoutMs: input.codexTimeoutMs ?? undefined, claudeFirstOutputTimeoutMs: input.claudeFirstOutputTimeoutMs ?? undefined, codexFirstOutputTimeoutMs: input.codexFirstOutputTimeoutMs ?? undefined, - ollamaModel: input.ollamaModel ?? undefined, - openaiModel: input.openaiModel ?? undefined, - openaiCompatibleModel: input.openaiCompatibleModel ?? undefined, - anthropicModel: input.anthropicModel ?? undefined, + ollamaModel: input.reviewKnobs?.model ?? input.ollamaModel ?? undefined, + openaiModel: input.reviewKnobs?.model ?? input.openaiModel ?? undefined, + openaiCompatibleModel: input.reviewKnobs?.model ?? input.openaiCompatibleModel ?? undefined, + anthropicModel: input.reviewKnobs?.model ?? input.anthropicModel ?? undefined, }; if (input.providerKey) { const outcome = await runProviderReview( @@ -3279,7 +3290,14 @@ export async function runLoopOverAiReview( // review that produced no usable verdict would spend real money measuring nothing. const selfConsistencySamples: { reviewer: string; votedFail: boolean }[] = []; let selfConsistencyDegraded = false; - const configuredSelfConsistencyRuns = resolveSelfConsistencyRuns(env.AI_REVIEW_SELF_CONSISTENCY_RUNS); + // #9821: a caller-resolved value (per-repo, or escalated because this PR touched a guarded path) outranks + // the global env var. `?? undefined` not `|| undefined`: 0 is a real choice ("off"), and truthiness here + // would silently fall back to the env value on a repo that deliberately turned extra runs off. + const configuredSelfConsistencyRuns = resolveSelfConsistencyRuns( + input.reviewKnobs?.selfConsistencyRuns !== null && input.reviewKnobs?.selfConsistencyRuns !== undefined + ? String(input.reviewKnobs.selfConsistencyRuns) + : env.AI_REVIEW_SELF_CONSISTENCY_RUNS, + ); if (configuredSelfConsistencyRuns >= 2 && reviewerVotes.length > 0) { const plan = planSelfConsistencyRuns({ configuredTotalRuns: configuredSelfConsistencyRuns, diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 833e52a04..6bdd700fb 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -510,6 +510,12 @@ function applyGateConfigOverrides(effective: RepositorySettings, gate: FocusMani if (gate.aiReviewByok !== null) effective.aiReviewByok = gate.aiReviewByok; if (gate.aiReviewProvider !== null) effective.aiReviewProvider = gate.aiReviewProvider; if (gate.aiReviewModel !== null) effective.aiReviewModel = gate.aiReviewModel; + if (gate.aiReviewEffort !== null) effective.aiReviewEffort = gate.aiReviewEffort; + if (gate.aiReviewSelfConsistencyRuns !== null) effective.aiReviewSelfConsistencyRuns = gate.aiReviewSelfConsistencyRuns; + if (gate.guardrailEscalationProvider !== null) effective.guardrailEscalationProvider = gate.guardrailEscalationProvider; + if (gate.guardrailEscalationModel !== null) effective.guardrailEscalationModel = gate.guardrailEscalationModel; + if (gate.guardrailEscalationEffort !== null) effective.guardrailEscalationEffort = gate.guardrailEscalationEffort; + if (gate.guardrailEscalationSelfConsistencyRuns !== null) effective.guardrailEscalationSelfConsistencyRuns = gate.guardrailEscalationSelfConsistencyRuns; if (gate.aiReviewAllAuthors !== null) effective.aiReviewAllAuthors = gate.aiReviewAllAuthors; if (gate.aiReviewCloseConfidence !== null) effective.aiReviewCloseConfidence = gate.aiReviewCloseConfidence; if (gate.aiReviewSalvageabilityMinScore !== null) effective.aiReviewSalvageabilityMinScore = gate.aiReviewSalvageabilityMinScore; diff --git a/src/types.ts b/src/types.ts index 5331a2846..2a980bbfd 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1117,6 +1117,19 @@ export type RepositorySettings = { /** Config-as-code model override for the BYOK advisory write-up (e.g. "claude-3-5-sonnet-latest"). * `null` = use the key record's model, else a conservative per-provider default. */ aiReviewModel?: string | null | undefined; + /** `gate.aiReview.effort` (#9821): reasoning effort for the review pass (low | medium | high | xhigh | max). + * Per-repo parity with the global CLAUDE_AI_EFFORT / CODEX_AI_EFFORT env vars. Unset ⇒ the env default. */ + aiReviewEffort?: "low" | "medium" | "high" | "xhigh" | "max" | null | undefined; + /** `gate.aiReview.selfConsistencyRuns` (#9821): TOTAL evaluations per review, primary included. Per-repo + * parity with AI_REVIEW_SELF_CONSISTENCY_RUNS; clamped downstream to {0,2,3}. Unset ⇒ the env default. */ + aiReviewSelfConsistencyRuns?: number | null | undefined; + /** `gate.guardrailEscalation.*` (#9808/#9821): review settings used INSTEAD of the repo defaults when a PR + * touches a hardGuardrailGlobs path, so a guarded path buys MORE SCRUTINY rather than a manual hold. Each + * field falls through to the repo/global value when unset. */ + guardrailEscalationProvider?: "anthropic" | "openai" | null | undefined; + guardrailEscalationModel?: string | null | undefined; + guardrailEscalationEffort?: "low" | "medium" | "high" | "xhigh" | "max" | null | undefined; + guardrailEscalationSelfConsistencyRuns?: number | null | undefined; /** Review EVERY PR's author, not only confirmed Gittensor contributors. Only meaningful when * {@link aiReviewConfirmedContributorsOnly} is also `true` (that field opts INTO confirmed-only * scoping in the first place — see its own doc comment for the full invariant: AI review runs for diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index acc35e294..849eeaabe 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -341,6 +341,12 @@ describe(".loopover.yml.example field-exhaustiveness (#1670)", () => { claCheckRunAppSlug: "checkRunAppSlug:", expectedCiContexts: "expectedCiContexts:", advisoryCheckRuns: "advisoryCheckRuns:", + aiReviewEffort: "effort:", + aiReviewSelfConsistencyRuns: "selfConsistencyRuns:", + guardrailEscalationProvider: "guardrailEscalation:", + guardrailEscalationModel: "guardrailEscalation:", + guardrailEscalationEffort: "guardrailEscalation:", + guardrailEscalationSelfConsistencyRuns: "guardrailEscalation:", ignoredCheckRuns: "ignoredCheckRuns:", aiJudgmentBlockersMode: "aiJudgmentBlockers:", copycatMode: "copycat:", @@ -433,6 +439,12 @@ describe(".loopover.yml.example field-exhaustiveness (#1670)", () => { unlinkedIssueGuardrail: "unlinkedIssueGuardrail:", screenshotTableGate: "screenshotTableGate:", advisoryAiRouting: "advisoryAiRouting:", + aiReviewEffort: "effort:", + aiReviewSelfConsistencyRuns: "selfConsistencyRuns:", + guardrailEscalationProvider: "guardrailEscalation:", + guardrailEscalationModel: "guardrailEscalation:", + guardrailEscalationEffort: "guardrailEscalation:", + guardrailEscalationSelfConsistencyRuns: "guardrailEscalation:", } satisfies Record, string>; it.each(Object.entries(SETTINGS_FIELD_TOKENS))("documents settings.%s", (_field, token) => { @@ -999,7 +1011,7 @@ describe("compileFocusManifestPolicy", () => { issueDiscoveryPolicy: "neutral", maintainerNotes: [], publicNotes: ["Keep PRs focused.", "Maximize your reward payout"], - gate: { present: false, enabled: null, checkMode: null, pack: null, closeAuditHoldoutPct: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, sizeMaxFiles: null, sizeMaxLines: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewSalvageabilityMinScore: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, contentLaneDeliverable: null, backtestRegression: null, manifestPolicy: null, dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, staleBaseAheadByThreshold: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, ignoredCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }, + gate: { present: false, enabled: null, checkMode: null, pack: null, closeAuditHoldoutPct: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, sizeMaxFiles: null, sizeMaxLines: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewEffort: null, aiReviewSelfConsistencyRuns: null, guardrailEscalationProvider: null, guardrailEscalationModel: null, guardrailEscalationEffort: null, guardrailEscalationSelfConsistencyRuns: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewSalvageabilityMinScore: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, contentLaneDeliverable: null, backtestRegression: null, manifestPolicy: null, dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, staleBaseAheadByThreshold: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, ignoredCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }, settings: {}, review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, sweepWatchdog: null, prReconciliation: null, activeReviewReconciliation: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null, sharedConfigSource: null }, features: { present: false, rag: null, reputation: null, safety: null, grounding: null, e2eTests: null, screenshots: null, improvementSignal: null, amsReputationBridge: null }, @@ -1193,7 +1205,7 @@ describe("parseFocusManifest gate config", () => { // the block→advisory deprecation-downgrade behavior itself is covered separately below. const m = parseFocusManifest({ gate: { linkedIssue: "block", duplicates: "advisory", readiness: { mode: "advisory", minScore: 70 } } }); expect(m.present).toBe(true); - expect(m.gate).toEqual({ present: true, enabled: null, checkMode: null, pack: null, closeAuditHoldoutPct: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, sizeMaxFiles: null, sizeMaxLines: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewSalvageabilityMinScore: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, contentLaneDeliverable: null, backtestRegression: null, manifestPolicy: null, dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, staleBaseAheadByThreshold: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, ignoredCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }); + expect(m.gate).toEqual({ present: true, enabled: null, checkMode: null, pack: null, closeAuditHoldoutPct: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, sizeMaxFiles: null, sizeMaxLines: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewEffort: null, aiReviewSelfConsistencyRuns: null, guardrailEscalationProvider: null, guardrailEscalationModel: null, guardrailEscalationEffort: null, guardrailEscalationSelfConsistencyRuns: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewSalvageabilityMinScore: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, contentLaneDeliverable: null, backtestRegression: null, manifestPolicy: null, dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, staleBaseAheadByThreshold: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, ignoredCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }); }); it("parses gate.mergeReadiness, round-trips it, and warns on a bad value (#822)", () => { @@ -1630,6 +1642,76 @@ describe("parseFocusManifest gate config", () => { expect(over.warnings.some((w) => /gate\.aiReview\.reviewers" is capped/.test(w))).toBe(true); }); + it("parses gate.aiReview.effort/selfConsistencyRuns and gate.guardrailEscalation with full branch coverage (#9821)", () => { + // Every field set: parse, presence, round-trip, resolve. + const full = parseFocusManifest({ + gate: { + aiReview: { effort: "high", selfConsistencyRuns: 3 }, + guardrailEscalation: { provider: "anthropic", model: "claude-opus-5", effort: "xhigh", selfConsistencyRuns: 2 }, + }, + }); + expect(full.gate.present).toBe(true); + expect(full.gate.aiReviewEffort).toBe("high"); + expect(full.gate.aiReviewSelfConsistencyRuns).toBe(3); + expect(full.gate.guardrailEscalationProvider).toBe("anthropic"); + expect(full.gate.guardrailEscalationModel).toBe("claude-opus-5"); + expect(full.gate.guardrailEscalationEffort).toBe("xhigh"); + expect(full.gate.guardrailEscalationSelfConsistencyRuns).toBe(2); + expect(parseFocusManifest({ gate: gateConfigToJson(full.gate) }).gate).toEqual(full.gate); // serialize round-trips + + // Each of the six fields ALONE flips presence — the #9813 presence-gap class, per field. + for (const gate of [ + { aiReview: { effort: "low" } }, + { aiReview: { selfConsistencyRuns: 0 } }, + { guardrailEscalation: { provider: "openai" } }, + { guardrailEscalation: { model: "m" } }, + { guardrailEscalation: { effort: "max" } }, + { guardrailEscalation: { selfConsistencyRuns: 3 } }, + ]) { + expect(parseFocusManifest({ gate }).gate.present).toBe(true); + } + + // Partial escalation: unset fields stay null (each falls through to repo/global downstream). + const partial = parseFocusManifest({ gate: { guardrailEscalation: { effort: "high" } } }); + expect(partial.gate.guardrailEscalationEffort).toBe("high"); + expect(partial.gate.guardrailEscalationProvider).toBeNull(); + expect(partial.gate.guardrailEscalationModel).toBeNull(); + expect(partial.gate.guardrailEscalationSelfConsistencyRuns).toBeNull(); + expect(parseFocusManifest({ gate: gateConfigToJson(partial.gate) }).gate).toEqual(partial.gate); + + // Invalid values warn and stay null — never silently coerced. + const bad = parseFocusManifest({ + gate: { aiReview: { effort: "ultra", selfConsistencyRuns: -1 }, guardrailEscalation: { provider: "grok", effort: 7, selfConsistencyRuns: "three", model: 42 } }, + }); + expect(bad.gate.aiReviewEffort).toBeNull(); + expect(bad.gate.aiReviewSelfConsistencyRuns).toBeNull(); + expect(bad.gate.guardrailEscalationProvider).toBeNull(); + expect(bad.gate.guardrailEscalationEffort).toBeNull(); + expect(bad.gate.guardrailEscalationSelfConsistencyRuns).toBeNull(); + expect(bad.gate.guardrailEscalationModel).toBeNull(); + expect(bad.warnings.some((w) => /gate\.aiReview\.effort/.test(w))).toBe(true); + expect(bad.warnings.some((w) => /gate\.guardrailEscalation\.provider/.test(w))).toBe(true); + + // A non-mapping guardrailEscalation is ignored wholesale (same contract as gate.aiReview). + const notMap = parseFocusManifest({ gate: { guardrailEscalation: "high", claMode: "advisory" } }); + expect(notMap.gate.guardrailEscalationEffort).toBeNull(); + + // Resolution: each field lands on effective settings; absent leaves the DB value untouched. + const eff = resolveEffectiveSettings({} as unknown as RepositorySettings, full); + expect(eff.aiReviewEffort).toBe("high"); + expect(eff.aiReviewSelfConsistencyRuns).toBe(3); + expect(eff.guardrailEscalationProvider).toBe("anthropic"); + expect(eff.guardrailEscalationModel).toBe("claude-opus-5"); + expect(eff.guardrailEscalationEffort).toBe("xhigh"); + expect(eff.guardrailEscalationSelfConsistencyRuns).toBe(2); + const untouched = resolveEffectiveSettings( + { aiReviewEffort: "low", guardrailEscalationModel: "existing" } as unknown as RepositorySettings, + parseFocusManifest({ gate: { claMode: "advisory" } }), + ); + expect(untouched.aiReviewEffort).toBe("low"); + expect(untouched.guardrailEscalationModel).toBe("existing"); + }); + it("parses gate.ignoredCheckRuns, makes the gate present, round-trips + resolves it, and drops spoofable entries (#9810)", () => { const m = parseFocusManifest({ gate: { ignoredCheckRuns: [{ name: "Contributor trust", appSlug: "example-security-app" }] } }); expect(m.gate.present).toBe(true); diff --git a/test/unit/review-knobs.test.ts b/test/unit/review-knobs.test.ts new file mode 100644 index 000000000..1794961ff --- /dev/null +++ b/test/unit/review-knobs.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, it } from "vitest"; +import { describeReviewEscalation, resolveReviewKnobs } from "../../src/review/review-knobs"; + +// #9808/#9821: a hardGuardrailGlobs hit used to buy NO extra analysis — same model, same effort, single pass — +// it only suppressed auto-merge and queued a human. 74 distinct PRs held in 14 days on the production ORB. + +const GLOBAL = { provider: "claude-code", model: "claude-sonnet-5", effort: "medium", selfConsistencyRuns: 0 }; + +describe("resolveReviewKnobs", () => { + it("REGRESSION: a guarded path escalates instead of reviewing identically to any other file", () => { + const r = resolveReviewKnobs({ + guardrailHit: true, + escalation: { effort: "high", selfConsistencyRuns: 3 }, + repo: {}, + global: GLOBAL, + }); + expect(r.effort).toBe("high"); + expect(r.selfConsistencyRuns).toBe(3); + expect(r.escalated).toBe(true); + // The common ask: "same model, think harder" — model/provider inherit rather than being forced along. + expect(r.model).toBe("claude-sonnet-5"); + expect(r.provider).toBe("claude-code"); + }); + + it("INVARIANT: the escalation block is INERT when the PR touches no guarded path", () => { + const r = resolveReviewKnobs({ + guardrailHit: false, + escalation: { effort: "max", selfConsistencyRuns: 3 }, + repo: {}, + global: GLOBAL, + }); + expect(r.effort).toBe("medium"); + expect(r.selfConsistencyRuns).toBe(0); + expect(r.escalated).toBe(false); + expect(r.escalatedFields).toEqual([]); + }); + + it("precedence: escalation > per-repo > global, per field independently", () => { + const r = resolveReviewKnobs({ + guardrailHit: true, + escalation: { effort: "xhigh" }, + repo: { model: "repo-model", effort: "high", selfConsistencyRuns: 2 }, + global: GLOBAL, + }); + expect(r.effort).toBe("xhigh"); // escalation wins + expect(r.model).toBe("repo-model"); // repo wins over global + expect(r.selfConsistencyRuns).toBe(2); // repo, untouched by escalation + expect(r.provider).toBe("claude-code"); // global fallback + expect(r.escalatedFields).toEqual(["effort"]); + }); + + it("per-repo effort/runs apply with no guardrail involved — the parity the manifest was missing", () => { + const r = resolveReviewKnobs({ guardrailHit: false, repo: { effort: "high", selfConsistencyRuns: 3 }, global: GLOBAL }); + expect({ effort: r.effort, runs: r.selfConsistencyRuns, escalated: r.escalated }).toEqual({ effort: "high", runs: 3, escalated: false }); + }); + + it("INVARIANT: selfConsistencyRuns of 0 is a real value, not 'unset'", () => { + // 0 means "off". A truthiness check here would silently fall through to the global value and re-enable + // multi-run reviews on a repo that explicitly turned them off. + const r = resolveReviewKnobs({ guardrailHit: true, escalation: { selfConsistencyRuns: 0 }, global: { ...GLOBAL, selfConsistencyRuns: 3 } }); + expect(r.selfConsistencyRuns).toBe(0); + expect(r.escalated).toBe(true); + }); + + it("everything absent resolves to nulls rather than throwing", () => { + expect(resolveReviewKnobs({ guardrailHit: true })).toEqual({ + provider: null, model: null, effort: null, selfConsistencyRuns: null, escalated: false, escalatedFields: [], + }); + }); +}); + +describe("describeReviewEscalation", () => { + it("names exactly what changed, for the panel and the decision record", () => { + const r = resolveReviewKnobs({ guardrailHit: true, escalation: { effort: "high", selfConsistencyRuns: 3 }, global: GLOBAL }); + expect(describeReviewEscalation(r)).toBe("escalated review on a guarded path: effort=high, 3 runs"); + }); + + it("returns null when nothing was escalated, so a caller needs no branch of its own", () => { + expect(describeReviewEscalation(resolveReviewKnobs({ guardrailHit: false, global: GLOBAL }))).toBeNull(); + }); + + it("reports a model/provider escalation too", () => { + const r = resolveReviewKnobs({ guardrailHit: true, escalation: { provider: "anthropic", model: "claude-opus-5" }, global: GLOBAL }); + 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); + }); +}); diff --git a/test/unit/self-consistency.test.ts b/test/unit/self-consistency.test.ts index b0a6142e3..d1b2f5a7f 100644 --- a/test/unit/self-consistency.test.ts +++ b/test/unit/self-consistency.test.ts @@ -236,6 +236,106 @@ describe("rotated-exemplar self-consistency (#8834)", () => { expect(usage?.n).toBe(2); // every extra run rides the SAME budget sum the quota gate reads }); + it("REGRESSION (#9821): a guardrail hit ESCALATES runs end-to-end — env off, gate.guardrailEscalation.selfConsistencyRuns=3 delivers 2 extra judged calls", async () => { + // The review blocker on #9821 was that resolveReviewKnobs was computed and dropped. This drives the + // whole pipeline: manifest -> settings -> orchestration (isGuardrailHit on src/a.ts) -> reviewKnobs -> + // runLoopOverAiReview's runs override (the `input.reviewKnobs?.selfConsistencyRuns` branch) -> the + // planner -> two extra rotated-exemplar calls. AI_REVIEW_SELF_CONSISTENCY_RUNS is deliberately UNSET: + // every extra call here exists only because the escalation reached the invocation. + const prompts: string[] = []; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async (_model: string, options: { messages?: Array<{ role: string; content: string }> }) => { + prompts.push(options.messages?.find((m) => m.role === "system")?.content ?? ""); + return { response: JSON.stringify({ assessment: "Fine.", blockers: [], nits: [], suggestions: [] }) }; + } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seed(env); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + settings: { + commentMode: "all_prs", publicSurface: "comment_only", checkRunMode: "off", reviewCheckMode: "required", aiReviewMode: "block", + hardGuardrailGlobs: ["src/**"], // the PR's one changed file is src/a.ts -- a guardrail hit + }, + gate: { guardrailEscalation: { selfConsistencyRuns: 3 } }, + }); + + await reReviewStoredPullRequest(env, "sc-escalated-900", 123, "JSONbored/gittensory", 900); + + const judgeCalls = prompts.filter((prompt) => prompt.length > 0); + expect(judgeCalls.length).toBeGreaterThanOrEqual(3); // 1 primary + 2 escalated extras + expect(judgeCalls.filter((prompt) => prompt.includes("Calibration examples"))).toHaveLength(2); + }); + + it("REGRESSION (#9821 review blocker): an escalated EFFORT/MODEL reaches the provider options, not just runs", async () => { + // The reviewer's exact objection: only selfConsistencyRuns was consumed, so "choose model, effort" was + // unimplemented where it takes effect. runLoopOverAiReview folds reviewKnobs into the AiRunCorrelation, + // which is what becomes the provider's per-call options (claudeModel/claudeEffort/... at ai-review.ts's + // provider dispatch). Capturing those options is therefore the honest end-to-end assertion. + const seenOptions: Array> = []; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async (_model: string, options: Record) => { + seenOptions.push(options); + return { response: JSON.stringify({ assessment: "Fine.", blockers: [], nits: [], suggestions: [] }) }; + } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seed(env); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + settings: { + commentMode: "all_prs", publicSurface: "comment_only", checkRunMode: "off", reviewCheckMode: "required", aiReviewMode: "block", + hardGuardrailGlobs: ["src/**"], + }, + gate: { guardrailEscalation: { effort: "xhigh", model: "escalated-model-x" } }, + }); + + await reReviewStoredPullRequest(env, "sc-effort-900", 123, "JSONbored/gittensory", 900); + + // At least one provider call carries BOTH escalated values. + const escalated = seenOptions.filter((o) => o.claudeEffort === "xhigh" && o.claudeModel === "escalated-model-x"); + expect(escalated.length).toBeGreaterThan(0); + // And the HTTP-API provider fields carry the same resolved model (whichever provider actually runs). + expect(escalated[0]?.anthropicModel).toBe("escalated-model-x"); + expect(escalated[0]?.codexEffort).toBe("xhigh"); + }); + + it("INVARIANT (#9821): the same escalation block is INERT when no changed file hits a guardrail glob", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { + aiCalls += 1; + return { response: JSON.stringify({ assessment: "Fine.", blockers: [], nits: [], suggestions: [] }) }; + } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seed(env); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + settings: { + commentMode: "all_prs", publicSurface: "comment_only", checkRunMode: "off", reviewCheckMode: "required", aiReviewMode: "block", + hardGuardrailGlobs: ["migrations/**"], // src/a.ts does NOT match -- no hit, no escalation + }, + gate: { guardrailEscalation: { selfConsistencyRuns: 3 } }, + }); + + await reReviewStoredPullRequest(env, "sc-inert-900", 123, "JSONbored/gittensory", 900); + // Counted via the selfConsistency usage marker, not raw AI.run calls: other pipeline consumers (the + // slop advisory, summaries) legitimately also call AI.run -- the precise claim is that the ESCALATED + // runs never fired, same discipline as the flag-off baseline above. + const usage = await env.DB.prepare( + "select count(*) as n from ai_usage_events where json_extract(metadata_json, '$.selfConsistency') = 1", + ).first<{ n: number }>(); + expect(usage?.n).toBe(0); + expect(aiCalls).toBeGreaterThan(0); // the pass itself still reviewed + }); + it("INVARIANT: flag off (default) spends nothing extra and adds no self-consistency usage rows", async () => { let aiCalls = 0; const env = createTestEnv({