diff --git a/.gittensory.yml.example b/.gittensory.yml.example index b597787547..b6b5a1b321 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -344,12 +344,22 @@ gate: # String or null. Default: null (the key record's model, else a conservative # per-provider default). model: null - # Minimum calibrated AI-reviewer confidence (0-1) recorded for cache and - # calibration context. Under `mode: block`, consensus and split AI-review - # defects still block regardless of this floor. Number 0–1, or null. - # Default: null (engine uses 0.93). Config-as-code only — no DB column or - # dashboard toggle; this can only be set here. + # 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. + # Number 0-1, or null. Default: null (engine uses 0.93). Config-as-code + # only — no DB column or dashboard toggle; this can only be set here. closeConfidence: null + # Disposition for a sub-closeConfidence-floor consensus/split defect (#4603). + # one_shot — ignore the floor; always one-shot-close (today's + # pre-#4603 behavior). Opt-in only. + # hold_for_review — DEFAULT. Still blocks the merge, but holds for a + # maintainer instead of one-shot-closing. + # advisory_only — drop to a fully non-blocking advisory below the floor. + # one_shot | hold_for_review | advisory_only, or null. Default: null (engine + # uses hold_for_review). DB-backed (dashboard-settable too, via the + # maintainer AI-review settings); this overrides the stored value. + lowConfidenceDisposition: null # Per-repo override of the self-host operator's dual-AI combine strategy (#2567). # single | consensus | synthesis, or null. Default: null (the operator's own # AI_REVIEW_PLAN.combine env default, itself "consensus" if unset). A refinement diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index 01e8f74883..9237010d48 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -9554,6 +9554,16 @@ }, "agentGlobalFreezeOverride": { "type": "boolean" + }, + "aiReviewLowConfidenceDisposition": { + "type": "string", + "nullable": true, + "enum": [ + "one_shot", + "hold_for_review", + "advisory_only", + null + ] } }, "required": [ diff --git a/config/examples/gittensory.full.yml b/config/examples/gittensory.full.yml index 75fa9c41ae..e7c87c9d3c 100644 --- a/config/examples/gittensory.full.yml +++ b/config/examples/gittensory.full.yml @@ -357,12 +357,22 @@ gate: # String or null. Default: null (the key record's model, else a conservative # per-provider default). model: null - # Minimum calibrated AI-reviewer confidence (0-1) recorded for cache and - # calibration context. Under `mode: block`, consensus and split AI-review - # defects still block regardless of this floor. Number 0–1, or null. - # Default: null (engine uses 0.93). Config-as-code only — no DB column or - # dashboard toggle; this can only be set here. + # 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. + # Number 0-1, or null. Default: null (engine uses 0.93). Config-as-code + # only — no DB column or dashboard toggle; this can only be set here. closeConfidence: null + # Disposition for a sub-closeConfidence-floor consensus/split defect (#4603). + # one_shot — ignore the floor; always one-shot-close (today's + # pre-#4603 behavior). Opt-in only. + # hold_for_review — DEFAULT. Still blocks the merge, but holds for a + # maintainer instead of one-shot-closing. + # advisory_only — drop to a fully non-blocking advisory below the floor. + # one_shot | hold_for_review | advisory_only, or null. Default: null (engine + # uses hold_for_review). DB-backed (dashboard-settable too, via the + # maintainer AI-review settings); this overrides the stored value. + lowConfidenceDisposition: null # Per-repo override of the self-host operator's dual-AI combine strategy (#2567). # single | consensus | synthesis, or null. Default: null (the operator's own # AI_REVIEW_PLAN.combine env default, itself "consensus" if unset). A refinement diff --git a/migrations/0140_ai_review_low_confidence_disposition.sql b/migrations/0140_ai_review_low_confidence_disposition.sql new file mode 100644 index 0000000000..19ce1da48c --- /dev/null +++ b/migrations/0140_ai_review_low_confidence_disposition.sql @@ -0,0 +1,8 @@ +-- AI-review low-confidence disposition (#4603, resolving the dead aiReviewCloseConfidence floor audit +-- finding). Governs what happens when an ai_consensus_defect/ai_review_split finding's confidence is BELOW +-- the configured aiReviewCloseConfidence floor: 'hold_for_review' (default -- flips the undocumented +-- unconditional-close drift from commit 311b7613d/#1781 back to a safe default) routes the would-be close +-- through the existing held-for-manual-review mechanism instead of one-shot-closing; 'one_shot' keeps +-- today's unconditional-close behavior (opt-in); 'advisory_only' drops a sub-floor finding to fully +-- non-blocking. See src/rules/advisory.ts's isConfiguredGateBlocker and gittensory-gate-setting-wiring. +ALTER TABLE repository_settings ADD COLUMN ai_review_low_confidence_disposition TEXT NOT NULL DEFAULT 'hold_for_review'; diff --git a/packages/gittensory-engine/src/advisory/gate-advisory.ts b/packages/gittensory-engine/src/advisory/gate-advisory.ts index 7fc3c61155..f67ea2052a 100644 --- a/packages/gittensory-engine/src/advisory/gate-advisory.ts +++ b/packages/gittensory-engine/src/advisory/gate-advisory.ts @@ -40,10 +40,17 @@ export type GateCheckPolicy = { /** When `block`, a dual-model AI consensus defect (`ai_consensus_defect` finding) becomes a hard * blocker. Defaults to advisory — AI never blocks unless the maintainer opts in. */ aiReviewGateMode?: GateRuleMode | undefined; - /** Minimum calibrated confidence (0-1) configured for AI close calibration. AI defect findings still block under - * `aiReviewGateMode: block` even when below this floor; the floor remains configurable context, never a guardrail - * that downgrades a blocker to manual review. `null`/undefined ⇒ the 0.93 default. */ + /** Minimum calibrated confidence (0-1) configured for AI close calibration. AI defect findings still BLOCK the + * gate under `aiReviewGateMode: block` even when below this floor — the floor never turns a real defect into a + * non-blocker on its own. What varies below the floor is {@link aiReviewLowConfidenceDisposition}. `null`/ + * undefined ⇒ the 0.93 default. */ aiReviewCloseConfidence?: number | null | undefined; + /** Disposition for a sub-floor `ai_consensus_defect`/`ai_review_split` finding (#4603) — see the host copy's + * doc comment (`src/rules/advisory.ts` / `src/types.ts`) for the full semantics. `null`/undefined ⇒ + * `hold_for_review` (the shipped default). Only `advisory_only` changes what `isConfiguredGateBlocker` returns + * for these codes here; `one_shot`/`hold_for_review` are indistinguishable to this predictor (the + * `hold_for_review` vs `one_shot` difference is a disposition-planner concern this predictor doesn't model). */ + aiReviewLowConfidenceDisposition?: "one_shot" | "hold_for_review" | "advisory_only" | undefined; readinessScore?: number | null | undefined; /** When `block`, the deterministic slop score becomes a hard blocker once `slopRisk >= slopGateMinScore` * (default threshold 60, the `high` band). Defaults to off/advisory — slop never blocks unless opted in. */ @@ -542,11 +549,20 @@ function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPoli // most conservative AI signal (two independent models) but still confirmed-contributor gated by // evaluateGateCheck, and advisory by default. // A consensus defect (both reviewers) OR a SPLIT (one reviewer flagged a blocker the other did not) both block - // when aiReviewGateMode is `block`. The configured close-confidence floor remains calibration context; it does - // not turn a blocker into a manual hold for normal contributors. (#ai-review-split) + // when aiReviewGateMode is `block`. (#ai-review-split) The close-confidence floor + disposition (#4603, mirrors + // the host copy in src/rules/advisory.ts -- this predictor package doesn't thread aiReviewLowConfidenceDisposition + // through predicted-gate.ts's own policy-building call yet, same deliberate partial-wiring precedent as + // linkedIssueSatisfactionGateMode, so this branch only ever sees the default `hold_for_review` here today) decide + // what happens to a SUB-floor finding: `one_shot`/`hold_for_review` both still block here; only `advisory_only` + // demotes a sub-floor finding to a non-blocker. if (code === "ai_consensus_defect" || code === "ai_review_split") { - void (policy.aiReviewCloseConfidence ?? DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE); - return gatePolicyBlocks(policy.aiReviewGateMode, "advisory"); + if (!gatePolicyBlocks(policy.aiReviewGateMode, "advisory")) return false; + if ((policy.aiReviewLowConfidenceDisposition ?? "hold_for_review") === "advisory_only") { + const floor = policy.aiReviewCloseConfidence ?? DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE; + const confidence = finding.confidence ?? 1; + if (confidence < floor) return false; + } + return true; } if (code === REVIEW_THREAD_BLOCKER_CODE) return true; // A leaked-secret finding (`secret_leak`) ALWAYS hard-blocks: a committed credential must be removed and diff --git a/packages/gittensory-engine/src/focus-manifest.ts b/packages/gittensory-engine/src/focus-manifest.ts index 46c278def5..0651c06e87 100644 --- a/packages/gittensory-engine/src/focus-manifest.ts +++ b/packages/gittensory-engine/src/focus-manifest.ts @@ -10,6 +10,7 @@ import { parse as parseYaml } from "yaml"; import type { AdvisoryAiRoutingConfig, + AiReviewLowConfidenceDisposition, CombineStrategy, GatePolicyPack, GateRuleMode, @@ -101,6 +102,11 @@ export type FocusManifestGateConfig = { /** `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. */ aiReviewCloseConfidence: number | null; + /** `gate.aiReview.lowConfidenceDisposition` (#4603): disposition for a sub-`closeConfidence`-floor + * `ai_consensus_defect`/`ai_review_split` finding. null (unset) ⇒ `hold_for_review` (the shipped default). + * DB-backed (dashboard-settable too, via the `/ai-review` route); this overrides the stored value -- mirrors + * `aiReviewMode` above, not the config-as-code-only `closeConfidence` sibling field just above. */ + aiReviewLowConfidenceDisposition: AiReviewLowConfidenceDisposition | null; /** `gate.aiReview.combine` (#2567): per-repo override of the self-host operator's `AI_REVIEW_PLAN.combine` * boot default (single/consensus/synthesis). null (unset) ⇒ the operator's plan (or `consensus`). A * REFINEMENT only — see {@link aiReviewOnMerge} for the operator-floor clamp `runGittensoryAiReview` applies @@ -911,6 +917,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = { aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, + aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, @@ -1263,6 +1270,12 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu aiReviewModel: normalizeOptionalString(aiReviewRecord?.model, "gate.aiReview.model", warnings), aiReviewAllAuthors: normalizeOptionalBoolean(aiReviewRecord?.allAuthors, "gate.aiReview.allAuthors", warnings), aiReviewCloseConfidence: normalizeOptionalConfidence(aiReviewRecord?.closeConfidence, "gate.aiReview.closeConfidence", warnings), + aiReviewLowConfidenceDisposition: normalizeOptionalEnum( + aiReviewRecord?.lowConfidenceDisposition, + "gate.aiReview.lowConfidenceDisposition", + ["one_shot", "hold_for_review", "advisory_only"] as const, + warnings, + ), aiReviewCombine: normalizeOptionalEnum(aiReviewRecord?.combine, "gate.aiReview.combine", ["single", "consensus", "synthesis"] as const, warnings), aiReviewOnMerge: normalizeOptionalEnum(aiReviewRecord?.onMerge, "gate.aiReview.onMerge", ["either", "both"] as const, warnings), aiReviewReviewers: normalizeOptionalReviewers(aiReviewRecord?.reviewers, "gate.aiReview.reviewers", warnings), @@ -1309,6 +1322,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu gate.aiReviewModel !== null || gate.aiReviewAllAuthors !== null || gate.aiReviewCloseConfidence !== null || + gate.aiReviewLowConfidenceDisposition !== null || gate.aiReviewCombine !== null || gate.aiReviewOnMerge !== null || gate.aiReviewReviewers !== null || @@ -1365,6 +1379,7 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue { gate.aiReviewModel !== null || gate.aiReviewAllAuthors !== null || gate.aiReviewCloseConfidence !== null || + gate.aiReviewLowConfidenceDisposition !== null || gate.aiReviewCombine !== null || gate.aiReviewOnMerge !== null || gate.aiReviewReviewers !== null @@ -1376,6 +1391,7 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue { if (gate.aiReviewModel !== null) aiReview.model = gate.aiReviewModel; if (gate.aiReviewAllAuthors !== null) aiReview.allAuthors = gate.aiReviewAllAuthors; if (gate.aiReviewCloseConfidence !== null) aiReview.closeConfidence = gate.aiReviewCloseConfidence; + if (gate.aiReviewLowConfidenceDisposition !== null) aiReview.lowConfidenceDisposition = gate.aiReviewLowConfidenceDisposition; if (gate.aiReviewCombine !== null) aiReview.combine = gate.aiReviewCombine; if (gate.aiReviewOnMerge !== null) aiReview.onMerge = gate.aiReviewOnMerge; if (gate.aiReviewReviewers !== null) { diff --git a/packages/gittensory-engine/src/types/manifest-deps-types.ts b/packages/gittensory-engine/src/types/manifest-deps-types.ts index 72fd36c668..b180147d66 100644 --- a/packages/gittensory-engine/src/types/manifest-deps-types.ts +++ b/packages/gittensory-engine/src/types/manifest-deps-types.ts @@ -18,6 +18,10 @@ export type CombineStrategy = "single" | "consensus" | "synthesis"; export type OnMerge = "either" | "both"; +// Disposition for a sub-aiReviewCloseConfidence-floor ai_consensus_defect/ai_review_split finding (#4603) -- +// see src/types.ts's mirror of this type (AiReviewLowConfidenceDisposition) for the full semantics of each value. +export type AiReviewLowConfidenceDisposition = "one_shot" | "hold_for_review" | "advisory_only"; + // #4110: `request_changes`/`comment` were REMOVED (see src/types.ts's mirror of this type for why). // `"advisory"` (#4535) is a NEW, actually-wired value -- see src/types.ts's mirror for the full rationale. export type ScreenshotTableGateAction = "close" | "advisory"; diff --git a/packages/gittensory-engine/test/gate-advisory-ai-review-low-confidence-disposition.test.ts b/packages/gittensory-engine/test/gate-advisory-ai-review-low-confidence-disposition.test.ts new file mode 100644 index 0000000000..c6185e9106 --- /dev/null +++ b/packages/gittensory-engine/test/gate-advisory-ai-review-low-confidence-disposition.test.ts @@ -0,0 +1,75 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { evaluateGateCheck } from "../dist/advisory/gate-advisory.js"; +import type { Advisory, AdvisoryFinding } from "../dist/types/predicted-gate-types.js"; + +const DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE = 0.93; + +function consensusAdvisory(confidence: number): Advisory { + const finding: AdvisoryFinding = { + code: "ai_consensus_defect", + title: "AI reviewers agree on a likely critical defect", + severity: "critical", + detail: "Both reviewers flagged the same blocker.", + confidence, + }; + return { + id: "advisory-1", + targetType: "pull_request", + targetKey: "JSONbored/gittensory#1", + repoFullName: "JSONbored/gittensory", + conclusion: "action_required", + severity: "critical", + title: "Gittensory review", + summary: "", + findings: [finding], + generatedAt: "2026-07-10T00:00:00.000Z", + }; +} + +// #4603 (gate-decision twin of src/rules/advisory.ts, kept in sync per checkGateDecisionVersionBump): mirrors +// the host copy's own regression tests for isConfiguredGateBlocker's aiReviewLowConfidenceDisposition branch. + +test("at-or-above-floor confidence blocks identically across all three dispositions", () => { + const advisory = consensusAdvisory(DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE); + for (const disposition of ["one_shot", "hold_for_review", "advisory_only"] as const) { + const evaluation = evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: disposition }); + assert.equal(evaluation.conclusion, "failure"); + } +}); + +test("sub-floor + one_shot or hold_for_review still blocks (today's behavior, unchanged at the gate level)", () => { + const advisory = consensusAdvisory(DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE - 0.1); + assert.equal(evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "one_shot" }).conclusion, "failure"); + assert.equal(evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "hold_for_review" }).conclusion, "failure"); + // Unset ⇒ hold_for_review is the default. + assert.equal(evaluateGateCheck(advisory, { aiReviewGateMode: "block" }).conclusion, "failure"); +}); + +test("sub-floor + advisory_only drops the finding to fully non-blocking", () => { + const advisory = consensusAdvisory(DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE - 0.1); + const evaluation = evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "advisory_only" }); + assert.equal(evaluation.conclusion, "success"); + assert.equal(evaluation.blockers.length, 0); +}); + +test("respects a custom aiReviewCloseConfidence floor under advisory_only", () => { + const advisory = consensusAdvisory(0.5); + assert.equal( + evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "advisory_only", aiReviewCloseConfidence: 0.6 }).conclusion, + "success", + ); + assert.equal( + evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "advisory_only", aiReviewCloseConfidence: 0.4 }).conclusion, + "failure", + ); +}); + +test("aiReviewGateMode !== block stays non-blocking regardless of disposition", () => { + const advisory = consensusAdvisory(DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE - 0.1); + for (const disposition of ["one_shot", "hold_for_review", "advisory_only"] as const) { + const evaluation = evaluateGateCheck(advisory, { aiReviewGateMode: "advisory", aiReviewLowConfidenceDisposition: disposition }); + assert.equal(evaluation.conclusion, "success"); + } +}); diff --git a/src/api/routes.ts b/src/api/routes.ts index 8adf62def9..cf5904d7c4 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -683,6 +683,7 @@ const repositorySettingsSchema = z.object({ aiReviewProvider: z.enum(["anthropic", "openai"]).nullable().optional(), aiReviewModel: z.string().trim().min(1).max(120).nullable().optional(), aiReviewAllAuthors: z.boolean().default(false), + aiReviewLowConfidenceDisposition: z.enum(["one_shot", "hold_for_review", "advisory_only"]).default("hold_for_review"), closeOwnerAuthors: z.boolean().default(false), autoLabelEnabled: z.boolean().default(true), gittensorLabel: z.string().trim().min(1).max(50).default("gittensor"), @@ -806,6 +807,10 @@ const repositoryAiReviewSchema = z.object({ model: z.string().trim().min(1).max(120).nullable().optional(), allAuthors: z.boolean().default(false), closeOwnerAuthors: z.boolean().optional(), + // Disposition for a sub-aiReviewCloseConfidence-floor ai_consensus_defect/ai_review_split finding (#4603). + // Optional so a caller that only ever cared about mode/byok/provider/model keeps its historical effect -- + // upsertRepositorySettings applies its own "hold_for_review" default when omitted. + lowConfidenceDisposition: z.enum(["one_shot", "hold_for_review", "advisory_only"]).optional(), }); const contributorIssueDraftGenerateSchema = z.object({ @@ -2560,6 +2565,7 @@ export function createApp() { aiReviewProvider: parsed.data.provider, aiReviewModel: parsed.data.model, aiReviewAllAuthors: parsed.data.allAuthors, + aiReviewLowConfidenceDisposition: parsed.data.lowConfidenceDisposition ?? current.aiReviewLowConfidenceDisposition, closeOwnerAuthors: parsed.data.closeOwnerAuthors ?? current.closeOwnerAuthors, }); // getRepositorySettings normalizes these to a concrete value or null (never undefined). @@ -2569,6 +2575,10 @@ export function createApp() { aiReviewProvider: updated.aiReviewProvider ?? null, aiReviewModel: updated.aiReviewModel ?? null, aiReviewAllAuthors: updated.aiReviewAllAuthors, + // parseAiReviewLowConfidenceDisposition's return type is non-nullable and already falls back to the + // literal "hold_for_review" itself, so this side of the `??` can never actually run. + /* v8 ignore next */ + aiReviewLowConfidenceDisposition: updated.aiReviewLowConfidenceDisposition ?? "hold_for_review", closeOwnerAuthors: updated.closeOwnerAuthors, }); }); @@ -3895,6 +3905,7 @@ export function createApp() { aiReviewProvider: parsed.data.aiReviewProvider, aiReviewModel: parsed.data.aiReviewModel, aiReviewAllAuthors: parsed.data.aiReviewAllAuthors, + aiReviewLowConfidenceDisposition: parsed.data.aiReviewLowConfidenceDisposition, closeOwnerAuthors: parsed.data.closeOwnerAuthors, autoLabelEnabled: parsed.data.autoLabelEnabled, gittensorLabel: parsed.data.gittensorLabel, diff --git a/src/db/repositories.ts b/src/db/repositories.ts index a16140c726..2d3491c7c1 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -66,6 +66,7 @@ import { MAX_CONTRIBUTOR_OPEN_ITEM_CAP } from "../types"; import type { Advisory, AdvisoryFinding, + AiReviewLowConfidenceDisposition, AgentActionRecord, AgentActionStatus, AgentActionType, @@ -534,6 +535,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: false, + aiReviewLowConfidenceDisposition: "hold_for_review", closeOwnerAuthors: false, autoLabelEnabled: true, typeLabelsEnabled: true, @@ -614,6 +616,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise aiReviewProvider: normalizeAiReviewProvider(row.aiReviewProvider), aiReviewModel: row.aiReviewModel ?? null, aiReviewAllAuthors: row.aiReviewAllAuthors, + aiReviewLowConfidenceDisposition: parseAiReviewLowConfidenceDisposition(row.aiReviewLowConfidenceDisposition), closeOwnerAuthors: row.closeOwnerAuthors, autoLabelEnabled: row.autoLabelEnabled, typeLabelsEnabled: row.typeLabelsEnabled, @@ -737,6 +740,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial blocker.title), @@ -3247,6 +3253,7 @@ async function runAgentMaintenancePlanAndExecute( }, ...(migrationCollisionHold !== undefined ? { migrationCollisionHold } : {}), ...(unlinkedIssueMatchHold !== undefined ? { unlinkedIssueMatchHold } : {}), + ...(aiReviewLowConfidenceHold !== undefined ? { aiReviewLowConfidenceHold } : {}), ...(unlinkedIssueMatchClose !== undefined ? { unlinkedIssueMatchClose } : {}), pr: { mergeableState: liveMergeState ?? pr.mergeableState, @@ -6860,6 +6867,10 @@ export function gateCheckPolicy( // Calibrated AI close-confidence floor (#7) — config-as-code via `.gittensory.yml gate.aiReview.closeConfidence`, // resolved into settings upstream. `null`/undefined ⇒ advisory.ts applies the 0.93 default. aiReviewCloseConfidence: settings.aiReviewCloseConfidence ?? null, + // Sub-floor AI-judgment disposition (#4603) — DB-backed (dashboard-settable) + `.gittensory.yml + // gate.aiReview.lowConfidenceDisposition` override, resolved into settings upstream. `null`/undefined ⇒ + // advisory.ts applies the "hold_for_review" default. + aiReviewLowConfidenceDisposition: settings.aiReviewLowConfidenceDisposition ?? null, readinessScore: readinessScore ?? null, slopGateMode: settings.slopGateMode, mergeReadinessGateMode: settings.mergeReadinessGateMode, @@ -7642,7 +7653,12 @@ export async function runAiReviewForAdvisory( detail: result.consensusDefect.detail, action: "Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.", - // Calibrated confidence (#8): clears aiReviewCloseConfidence ⇒ block; below it ⇒ human-review hold. + // Calibrated confidence (#8). This finding ALWAYS blocks under aiReviewGateMode: block regardless of + // where it falls relative to aiReviewCloseConfidence (isConfiguredGateBlocker never refutes a blocker + // on confidence alone) -- what varies below the floor is the DISPOSITION (#4603, + // aiReviewLowConfidenceDisposition): hold_for_review (default) routes the would-be close to manual + // review instead of one-shot-closing; advisory_only drops it to non-blocking; one_shot ignores the + // floor. See resolveAiReviewLowConfidenceHold in src/rules/advisory.ts. confidence: result.consensusDefect.confidence, }); } else if (result.split) { @@ -7657,10 +7673,14 @@ export async function runAiReviewForAdvisory( "One AI reviewer independently flagged a concrete must-fix defect in this change (the other did not). Under the quorum rule, a single rejection closes the PR; see the review notes for specifics.", action: "Resolve the flagged defect and open a new pull request, or override if the reviewers are mistaken.", - // Calibrated confidence (#8) of the lone flagging reviewer; clears aiReviewCloseConfidence ⇒ block, - // below it ⇒ human-review hold. A consensus split ALWAYS carries this (combineReviews sets it whenever + // Calibrated confidence (#8) of the lone flagging reviewer. Like the consensus-defect finding above, this + // ALWAYS blocks under aiReviewGateMode: block regardless of the aiReviewCloseConfidence floor -- the + // floor only selects the DISPOSITION of a sub-floor finding (#4603, aiReviewLowConfidenceDisposition): + // hold_for_review (default) holds instead of one-shot-closing; advisory_only drops it to non-blocking; + // one_shot ignores the floor. A consensus split ALWAYS carries this (combineReviews sets it whenever // split is true), so the spread is effectively unconditional; the guard is a defensive belt-and-braces — - // an absent value degrades to 1.0 in the threshold check (advisory.ts `?? 1`), matching today's always-block. + // an absent value degrades to 1.0 in the threshold check (advisory.ts `?? 1`), matching an at-or-above-floor + // confidence. /* v8 ignore next 3 -- a split always carries splitConfidence; the absent arm is an unreachable guard. */ ...(result.splitConfidence !== undefined ? { confidence: result.splitConfidence } diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts index 0541007b36..72dc281bbf 100644 --- a/src/rules/advisory.ts +++ b/src/rules/advisory.ts @@ -3,6 +3,7 @@ import type { AdvisoryConclusion, AdvisoryFinding, AdvisorySeverity, + AiReviewLowConfidenceDisposition, GateRuleMode, IssueRecord, PullRequestFileRecord, @@ -32,10 +33,18 @@ export type GateCheckPolicy = { /** When `block`, a dual-model AI consensus defect (`ai_consensus_defect` finding) becomes a hard * blocker. Defaults to advisory — AI never blocks unless the maintainer opts in. */ aiReviewGateMode?: GateRuleMode | undefined; - /** Minimum calibrated confidence (0-1) configured for AI close calibration. AI defect findings still block under - * `aiReviewGateMode: block` even when below this floor; the floor remains configurable context, never a guardrail - * that downgrades a blocker to manual review. `null`/undefined ⇒ the 0.93 default. */ + /** Minimum calibrated confidence (0-1) configured for AI close calibration. AI defect findings still BLOCK the + * gate under `aiReviewGateMode: block` even when below this floor — the floor never turns a real defect into a + * non-blocker on its own. What varies below the floor is {@link aiReviewLowConfidenceDisposition}: `null`/undefined + * ⇒ the 0.93 default. */ aiReviewCloseConfidence?: number | null | undefined; + /** Disposition for a sub-floor `ai_consensus_defect`/`ai_review_split` finding (#4603) — see the type's own doc + * comment (`src/types.ts`) for the full semantics of `one_shot` / `hold_for_review` / `advisory_only`. + * `null`/undefined ⇒ `hold_for_review` (the shipped default). Only `advisory_only` changes what + * `isConfiguredGateBlocker` returns for these codes; `one_shot` and `hold_for_review` both still block the gate + * identically — `hold_for_review`'s difference is downstream, in the disposition planner (see + * `resolveAiReviewLowConfidenceHold`). */ + aiReviewLowConfidenceDisposition?: AiReviewLowConfidenceDisposition | null | undefined; readinessScore?: number | null | undefined; /** When `block`, the deterministic slop score becomes a hard blocker once `slopRisk >= slopGateMinScore` * (default threshold 60, the `high` band). Defaults to off/advisory — slop never blocks unless opted in. */ @@ -132,6 +141,37 @@ export function isAiJudgmentOnlyFailure(evaluation: GateCheckEvaluation): boolea return evaluation.conclusion === "failure" && evaluation.blockers.length > 0 && evaluation.blockers.every((blocker) => AI_JUDGMENT_BLOCKER_CODES.has(blocker.code)); } +/** + * Resolve the `hold_for_review` disposition (#4603) for a gate evaluation that FAILED solely on a sub-floor + * `ai_consensus_defect`/`ai_review_split` finding — the mirror of `migrationCollisionHold`/`unlinkedIssueMatchHold` + * (`src/settings/agent-actions.ts`) for this specific guardrail. Returns `undefined` (no hold) when: + * - the resolved disposition isn't `hold_for_review` (default) -- `one_shot` and `advisory_only` never hold here + * (`advisory_only` already dropped the finding out of `blockers` entirely in `isConfiguredGateBlocker`, so a + * gate that failed for that reason can't reach this function with the finding still present as a blocker); + * - the failure ISN'T solely AI-judgment (a concrete blocker like `secret_leak` sits alongside it) -- a hold + * here must never soften a genuinely different, non-AI blocker into manual review, mirroring how + * `migrationCollisionHold`/guardrail holds never downgrade a real blocker/conflict/red-CI close either; or + * - every AI-judgment blocker is AT OR ABOVE the floor (nothing below it to hold on). + * PURE — the caller (processors.ts) threads the `{ reason, comment }` result into `AgentActionPlanInput. + * aiReviewLowConfidenceHold`, which the planner uses to route a would-be one-shot close into the existing + * held-for-manual-review path instead (see `willClose` in `src/settings/agent-actions.ts`). + */ +export function resolveAiReviewLowConfidenceHold( + evaluation: GateCheckEvaluation, + policy: Pick, +): { reason: string; comment: string } | undefined { + if ((policy.aiReviewLowConfidenceDisposition ?? "hold_for_review") !== "hold_for_review") return undefined; + if (!isAiJudgmentOnlyFailure(evaluation)) return undefined; + const floor = policy.aiReviewCloseConfidence ?? DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE; + const belowFloor = evaluation.blockers.some((blocker) => (blocker.confidence ?? 1) < floor); + if (!belowFloor) return undefined; + return { + reason: `an AI-reviewer defect finding's confidence is below the configured close-confidence floor (${floor})`, + comment: + "An AI reviewer flagged a likely defect, but its confidence was below this repository's configured close-confidence floor, so this is held for a maintainer to confirm instead of closing automatically. Resolve the flagged defect (see the review notes), or ask a maintainer to override.", + }; +} + // DUPLICATE-ONLY blocker codes: findings whose own severity is always "warning" (advisory by nature — a // same-linked-issue overlap is a lead for a human, not proof of a defect) but that a per-repo gate-mode config // can still escalate into a hard blocker (`duplicate_pr_risk` under `duplicatePrGateMode: "block"`, its ONLY @@ -865,9 +905,11 @@ function isEvaluationBlocker(code: string, policy: GateCheckPolicy): boolean { return false; } -// Default configured close-confidence floor (#7) retained for settings compatibility and public calibration text. -// It must not be used to downgrade an AI defect blocker into manual review. -const DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE = 0.93; +// Default configured close-confidence floor (#7). A sub-floor AI-judgment finding still blocks the gate under +// aiReviewGateMode: block (see isConfiguredGateBlocker) UNLESS the resolved aiReviewLowConfidenceDisposition +// (#4603) is "advisory_only" -- the floor never softens a blocker into a non-blocker on its own. Exported for +// resolveAiReviewLowConfidenceHold (below) and for the packages/gittensory-engine gate-decision twin's own copy. +export const DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE = 0.93; function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPolicy): boolean { const code = finding.code; @@ -879,11 +921,18 @@ function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPoli // most conservative AI signal (two independent models) but still confirmed-contributor gated by // evaluateGateCheck, and advisory by default. // A consensus defect (both reviewers) OR a SPLIT (one reviewer flagged a blocker the other did not) both block - // when aiReviewGateMode is `block`. The configured close-confidence floor remains calibration context; it does - // not turn a blocker into a manual hold for normal contributors. (#ai-review-split) + // when aiReviewGateMode is `block`. (#ai-review-split) The close-confidence floor + disposition (#4603) decide + // what happens to a SUB-floor finding: `one_shot`/`hold_for_review` (both still block here -- hold_for_review's + // difference is downstream, in resolveAiReviewLowConfidenceHold below) leave this branch's return unchanged; + // only `advisory_only` demotes a sub-floor finding to a non-blocker. if (code === "ai_consensus_defect" || code === "ai_review_split") { - void (policy.aiReviewCloseConfidence ?? DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE); - return gateMode(policy.aiReviewGateMode ?? "advisory") === "block"; + if (gateMode(policy.aiReviewGateMode ?? "advisory") !== "block") return false; + if ((policy.aiReviewLowConfidenceDisposition ?? "hold_for_review") === "advisory_only") { + const floor = policy.aiReviewCloseConfidence ?? DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE; + const confidence = finding.confidence ?? 1; + if (confidence < floor) return false; + } + return true; } if (code === REVIEW_THREAD_BLOCKER_CODE) return true; // A leaked-secret finding (`secret_leak`) ALWAYS hard-blocks: a committed credential must be removed and diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 9f6ab585d3..4377f24646 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -392,10 +392,13 @@ export type ModelReview = { blockers: string[]; nits: string[]; suggestions: string[]; - // Calibrated confidence in [0,1] (#8): the reviewer's own probability that its blocker(s) are a REAL defect. Drives - // the gate's `aiReviewCloseConfidence` floor (clear ⇒ block; below ⇒ human-review hold). parseModelReview - // sets it from the model's `confidence` field; an absent/unparseable/out-of-range value degrades to 1.0 (FALLBACK), - // so behavior matches the historical hardcoded `confidence: 1` until a calibrated value is actually present. + // Calibrated confidence in [0,1] (#8): the reviewer's own probability that its blocker(s) are a REAL defect. A + // consensus/split defect blocks the gate regardless of where this falls relative to `aiReviewCloseConfidence`; + // the floor instead selects the DISPOSITION of a sub-floor finding via `aiReviewLowConfidenceDisposition` (#4603) + // -- hold_for_review (default) ⇒ manual-review hold instead of one-shot-close; advisory_only ⇒ non-blocking; + // one_shot ⇒ the floor is ignored. parseModelReview sets it from the model's `confidence` field; an + // absent/unparseable/out-of-range value degrades to 1.0 (FALLBACK), so behavior matches the historical hardcoded + // `confidence: 1` until a calibrated value is actually present. confidence: number; // Line-anchored findings for inline PR review comments (#inline-comments). ALWAYS present (parseModelReview // sets []); populated only when the caller asked for them (input.inlineFindings) AND the model emitted any. @@ -1422,8 +1425,12 @@ export function composeInlineFindings(reviews: ModelReview[]): InlineFinding[] { /** A CONSENSUS defect = BOTH reviews independently name at least one concrete blocker (the severity-disciplined * reviewbot model: a lone blocker in a dual review is a split, not a hard block). Requiring two independent - * models to AGREE is itself the precision mechanism; the calibrated confidence (#8) ADDS a numeric floor on top — - * a consensus is only as strong as its WEAKER reviewer, so the defect carries `min(a.confidence, b.confidence)`. */ + * models to AGREE is itself the precision mechanism; the calibrated confidence (#8) — a consensus is only as + * strong as its WEAKER reviewer, so the defect carries `min(a.confidence, b.confidence)` — feeds the gate's + * `aiReviewLowConfidenceDisposition` (#4603): the defect always blocks under `aiReviewGateMode: block`, but a + * sub-`aiReviewCloseConfidence`-floor confidence changes what happens next (manual-review hold by default, + * non-blocking under `advisory_only`, or ignored under `one_shot`) rather than adding a second floor on top of + * the block decision itself. */ export function consensusDefectOf( a: ModelReview, b: ModelReview, @@ -2204,8 +2211,9 @@ export async function runGittensoryAiReview( advisoryNotes, consensusDefect, split: aiReviewSplit, - // Carry the split's calibrated confidence (#8) so the caller can gate `ai_review_split` on the same floor as a - // consensus defect. Only present on a split (combineReviews leaves it undefined otherwise). + // Carry the split's calibrated confidence (#8) so the caller can apply the same `aiReviewCloseConfidence` + // floor + `aiReviewLowConfidenceDisposition` (#4603) to `ai_review_split` as to a consensus defect. Only + // present on a split (combineReviews leaves it undefined otherwise). ...(splitConfidence !== undefined ? { splitConfidence } : {}), inconclusive, estimatedNeurons, diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index a22385ec9e..e2465e0a90 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -351,6 +351,15 @@ export type AgentActionPlanInput = { // AI semantic-match verdict, and a systematically-wrong match must not become breaker-proof just because // it repeated. Mutually exclusive with unlinkedIssueMatchHold -- the resolver only ever returns one. unlinkedIssueMatchClose?: { reason: string; comment: string } | undefined; + // AI-review low-confidence guardrail (#4603). The trigger (runAgentMaintenancePlanAndExecute) has already run + // resolveAiReviewLowConfidenceHold (src/rules/advisory.ts) against the gate evaluation -- this input is already + // the resolved "yes, hold this close" verdict (or absent, meaning the gate didn't fail solely on a sub-floor + // AI-judgment finding under the `hold_for_review` disposition). UNLIKE migrationCollisionHold/unlinkedIssueMatchHold + // (which suppress a would-MERGE), this suppresses a would-CLOSE: `willClose` below narrows its own + // `conclusion === "failure"` arm on this field so a genuinely different adverse signal (red CI, a base conflict) + // still closes normally -- only the "verdict=failure, driven solely by this AI-judgment blocker" path is held. The + // gate check itself still reports failure (the merge stays blocked) -- only the one-shot CLOSE is suppressed. + aiReviewLowConfidenceHold?: { reason: string; comment: string } | undefined; // Screenshot-table gate (#2006): a DETERMINISTIC verdict (no AI, zero hallucination risk) that an in-scope // visual/frontend PR's body is missing a before/after screenshot table (or has an image outside a table, or // a screenshot committed to the repo instead of uploaded to the PR) AND (#4110) the bot's own visual-capture @@ -809,7 +818,10 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne // Owner/automation PRs are never closed unless owner-close is explicitly enabled. Guardrails do not soften // blockers/conflicts/red CI; they hold only otherwise-ready PRs for manual review. // (Rebase-if-behind already ran above, so a red CI here is on the latest base — not a stale-base artifact.) (#ci-fail-closes-guarded) - const willClose = closeEligible && acting("close") && (ciFailed || conclusion === "failure" || isConflict); + // aiReviewLowConfidenceHold (#4603) narrows ONLY the `conclusion === "failure"` arm -- ciFailed/isConflict still + // close normally even when a sub-floor AI-judgment finding also happens to be present, matching how a real + // guardrail hold never softens those two either (see the comment above). + const willClose = closeEligible && acting("close") && (ciFailed || (conclusion === "failure" && input.aiReviewLowConfidenceHold === undefined) || isConflict); // Unlinked-issue-match REPEAT close (#unlinked-issue-guardrail-followup): a CONFIRMED repeat of the // credibility-gate-farming pattern (tracked via audit_events in resolveUnlinkedIssueMatchDisposition) — not // an immediate close on the first occurrence (that stays a hold, unlinkedIssueMatchHold), only once the same @@ -1237,6 +1249,11 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne reason: manualHoldReason, label: labels.manualReview, labelOp: "add", + // aiReviewLowConfidenceHold (#4603) is the only reason this generic fallback carries a comment -- it + // mirrors the migrationCollisionHold/unlinkedIssueMatchHold fallbacks' own comment-attaching shape (section + // 1c/1d above), but those apply only to a would-MERGE hold (reviewGood); this hold applies to a would-CLOSE + // suppression instead, so it belongs in this generic not-review-good fallback, not sections 1c/1d. + ...(!reviewGood && input.aiReviewLowConfidenceHold !== undefined ? { comment: sanitizePublicComment(input.aiReviewLowConfidenceHold.comment) } : {}), }); } diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index c63b93dd42..8f131310fd 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -462,6 +462,7 @@ function applyGateConfigOverrides(effective: RepositorySettings, gate: FocusMani if (gate.aiReviewModel !== null) effective.aiReviewModel = gate.aiReviewModel; if (gate.aiReviewAllAuthors !== null) effective.aiReviewAllAuthors = gate.aiReviewAllAuthors; if (gate.aiReviewCloseConfidence !== null) effective.aiReviewCloseConfidence = gate.aiReviewCloseConfidence; + if (gate.aiReviewLowConfidenceDisposition !== null) effective.aiReviewLowConfidenceDisposition = gate.aiReviewLowConfidenceDisposition; // Dual-AI combine/onMerge/reviewers overrides (#2567) are projected onto `effective` unclamped here — they are // a REFINEMENT of the operator's AI_REVIEW_PLAN, not a replacement for it, so the actual operator-floor clamp // (onMerge can only TIGHTEN, never loosen) happens where both the per-repo value AND the operator's plan are diff --git a/src/types.ts b/src/types.ts index b8a22429ad..331d0f1f5b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -456,9 +456,13 @@ export type AdvisoryFinding = { publicText?: string; /** Calibrated confidence in [0,1] for an AI-judgment finding (`ai_consensus_defect` / `ai_review_split`) — the * reviewer's own probability that the flagged blocker is a real defect (#8). The gate's `aiReviewCloseConfidence` - * threshold uses it: an AI defect blocks ONLY when this clears the floor. Absent for deterministic findings (they - * carry no model confidence); an absent/unparseable reviewer confidence degrades to 1.0 upstream, so omitting it - * here behaves exactly like today. */ + * floor and `aiReviewLowConfidenceDisposition` (#4603) use it: a sub-floor finding still blocks the gate when + * `aiReviewMode` is `block`, but its DISPOSITION varies — `hold_for_review` (default) routes the PR to a manual + * hold instead of a one-shot close, `advisory_only` drops it to a non-blocking finding entirely, and `one_shot` + * ignores the floor (today's unconditional-close behavior). See `isConfiguredGateBlocker` and + * `resolveAiReviewLowConfidenceHold` in `src/rules/advisory.ts`. Absent for deterministic findings (they carry + * no model confidence); an absent/unparseable reviewer confidence degrades to 1.0 upstream, so omitting it here + * behaves exactly like an at-or-above-floor confidence. */ confidence?: number; }; @@ -656,6 +660,25 @@ export type CombineStrategy = "single" | "consensus" | "synthesis"; * {@link CombineStrategy} for why the canonical definition lives here rather than `services/ai-review.ts`. */ export type OnMerge = "either" | "both"; +/** + * Disposition for an `ai_consensus_defect` / `ai_review_split` finding whose confidence is BELOW the + * configured `aiReviewCloseConfidence` floor (#4603, resolving the dead-floor audit finding from commit + * `311b7613d` / #1781). Only matters under `aiReviewMode: block` — a sub-floor finding is otherwise-identical + * across all three values once confidence clears the floor. + * • `one_shot` — today's live (pre-#4603) behavior: confidence is ignored, the defect always + * one-shot-closes. Opt-in only, for maintainers who want max automation and accept + * the false-positive risk. + * • `hold_for_review` — the SHIPPED DEFAULT. The defect still blocks the merge (the gate check still fails, + * a contributor still cannot merge as-is), but does NOT one-shot-close — it is routed + * through the same held-for-manual-review mechanism the disposition planner already + * uses for `migrationCollisionHold`/`unlinkedIssueMatchHold` + * (`src/settings/agent-actions.ts`), not a second hold mechanism. + * • `advisory_only` — a sub-floor finding drops to a fully non-blocking advisory (never a gate blocker). + * For maintainers who would rather lean on other deterministic gates and never see a + * review-hold queue. + */ +export type AiReviewLowConfidenceDisposition = "one_shot" | "hold_for_review" | "advisory_only"; + /** * A multimodal content block for an AI provider message (#4111 — advisory-only AI-vision analysis of * before/after visual captures). Canonical definition lives here for the same UI-safety reason as @@ -830,11 +853,20 @@ export type RepositorySettings = { * `aiReviewMode`: `off` still means no AI; this only widens WHO an enabled review covers. */ aiReviewAllAuthors: boolean; /** Configured AI-reviewer confidence floor (0-1) for close calibration (#7). Under `aiReviewMode: block`, AI - * defect findings remain blockers even when their confidence is below this floor; the floor is retained as - * configurable context, not a manual-review downgrade. Config-as-code only — set via `.gittensory.yml - * gate.aiReview.closeConfidence` (no dashboard/DB column); unset ⇒ the gate uses the 0.93 default. Clamped to - * [0,1] at parse time. */ + * defect findings remain BLOCKERS even when their confidence is below this floor — the floor never turns a + * real defect into a non-blocker on its own. What DOES vary below the floor is governed by the separate + * {@link aiReviewLowConfidenceDisposition} field (#4603): `hold_for_review` (default) routes a sub-floor + * blocker to manual review instead of one-shot-closing; `advisory_only` drops it to non-blocking; `one_shot` + * ignores the floor entirely. Config-as-code only — set via `.gittensory.yml gate.aiReview.closeConfidence` + * (no dashboard/DB column); unset ⇒ the gate uses the 0.93 default. Clamped to [0,1] at parse time. */ aiReviewCloseConfidence?: number | null | undefined; + /** Disposition for a sub-floor `ai_consensus_defect`/`ai_review_split` finding (#4603) — see + * {@link AiReviewLowConfidenceDisposition} for the full semantics of each value. Default `"hold_for_review"`. + * Unlike {@link aiReviewCloseConfidence}, this IS DB-backed/dashboard-settable (via the `/ai-review` route, + * alongside `aiReviewMode`) and also overridable via `.gittensory.yml gate.aiReview.lowConfidenceDisposition` + * — yml > DB > this default, resolved through the normal `resolveEffectiveSettings` chain like every other + * gate-setting field. */ + aiReviewLowConfidenceDisposition?: AiReviewLowConfidenceDisposition | null | undefined; /** Per-repo dual-AI combine-strategy override (#2567). Config-as-code only — set via `.gittensory.yml * gate.aiReview.combine` (no dashboard/DB column); unset ⇒ the self-host operator's `AI_REVIEW_PLAN.combine` * boot config (or `consensus` if the operator set nothing). A REFINEMENT of the operator's plan, not a diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index 7600ac1ff8..61d31c1b8b 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -571,6 +571,57 @@ describe("planAgentMaintenanceActions (#778)", () => { }); }); + describe("AI-review low-confidence hold (#4603): a sub-floor consensus/split defect under hold_for_review must not one-shot-close", () => { + const held = { + aiReviewLowConfidenceHold: { + reason: "an AI-reviewer defect finding's confidence is below the configured close-confidence floor (0.93)", + comment: "An AI reviewer flagged a likely defect, but its confidence was below this repository's configured close-confidence floor, so this is held for a maintainer to confirm instead of closing automatically.", + }, + }; + + it("BASELINE: without the hold, a failure verdict one-shot-closes a contributor PR (proves the hold is what changes behavior)", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["AI reviewers agree on a likely critical defect"] }))); + expect(plan).toContain("close"); + }); + + it("suppresses the one-shot CLOSE when present — the gate still failed, but the PR is held instead", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["AI reviewers agree on a likely critical defect"], ...held }))); + expect(plan).not.toContain("close"); + }); + + it("labels the held PR manual-review (via the generic manualHoldReason fallback, close autonomy class) with the hold's comment attached", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, manualReviewLabel: "human-review", blockerTitles: ["AI reviewers agree on a likely critical defect"], ...held })); + expect(plan).toEqual([ + expect.objectContaining({ actionClass: "label", autonomyClass: "close", label: "human-review", labelOp: "add", comment: held.aiReviewLowConfidenceHold.comment }), + ]); + }); + + it("does NOT suppress a close driven by red CI, even when the hold is also present", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, ciState: "failed", blockerTitles: ["AI reviewers agree on a likely critical defect"], ...held }))); + expect(plan).toContain("close"); + }); + + it("does NOT suppress a close driven by a base conflict, even when the hold is also present", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["AI reviewers agree on a likely critical defect"], ...held, pr: { labels: [], mergeableState: "dirty" } }))); + expect(plan).toContain("close"); + }); + + it("never applies to a success/neutral verdict — no hold-driven label without a failure to hold", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", merge: "auto" }, ...held, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))); + expect(plan).toContain("merge"); + }); + + it("has no effect for the owner/automation-bot branch (never close-eligible regardless)", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, authorIsOwner: true, blockerTitles: ["AI reviewers agree on a likely critical defect"], ...held }))); + expect(plan).not.toContain("close"); + }); + + it("absent (undefined) is byte-identical to today — a failure verdict still closes", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"] }))); + expect(plan).toContain("close"); + }); + }); + describe("unlinked-issue-match hold (#unlinked-issue-guardrail, credibility-gate-farming defense)", () => { const matched = { unlinkedIssueMatchHold: { reason: "this PR links no issue, but appears to directly solve open issue #42 without linking it (adds the missing dedup key)", comment: "This PR doesn't link an issue, but its diff appears to directly solve #42. Please add a linking reference." } }; diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 65fafcedc7..8639685147 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -259,6 +259,7 @@ describe(".gittensory.yml.example field-exhaustiveness (#1670)", () => { aiReviewModel: "model:", aiReviewAllAuthors: "allAuthors:", aiReviewCloseConfidence: "closeConfidence:", + aiReviewLowConfidenceDisposition: "lowConfidenceDisposition:", aiReviewCombine: "combine:", aiReviewOnMerge: "onMerge:", aiReviewReviewers: "reviewers:", @@ -825,7 +826,7 @@ describe("compileFocusManifestPolicy", () => { issueDiscoveryPolicy: "neutral", maintainerNotes: [], publicNotes: ["Keep PRs focused.", "Maximize your reward payout"], - gate: { present: false, enabled: null, checkMode: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }, + gate: { present: false, enabled: null, checkMode: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: 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, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, e2eTestDelivery: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null, sharedConfigSource: null }, features: { present: false, rag: null, reputation: null, unifiedComment: null, safety: null, grounding: null, e2eTests: null }, @@ -1136,7 +1137,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, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }); + expect(m.gate).toEqual({ present: true, enabled: null, checkMode: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }); }); it("parses gate.mergeReadiness + gate.firstTimeContributorGrace, round-trips them, and warns on bad values (#822)", () => { @@ -1470,6 +1471,29 @@ describe("parseFocusManifest gate config", () => { expect(resolveEffectiveSettings({ aiReviewCloseConfidence: 0.6 } as unknown as RepositorySettings, noFlag).aiReviewCloseConfidence).toBe(0.6); }); + it("parses gate.aiReview.lowConfidenceDisposition, makes the gate present, round-trips + resolves it, and warns on a bad value (#4603)", () => { + // lowConfidenceDisposition alone makes the gate present, serializes back under + // gate.aiReview.lowConfidenceDisposition, and the gate alias projects it onto effective settings. + const m = parseFocusManifest({ gate: { aiReview: { lowConfidenceDisposition: "advisory_only" } } }); + expect(m.gate.present).toBe(true); + expect(m.gate.aiReviewLowConfidenceDisposition).toBe("advisory_only"); + expect((gateConfigToJson(m.gate) as { aiReview: { lowConfidenceDisposition: string } }).aiReview.lowConfidenceDisposition).toBe("advisory_only"); + expect(parseFocusManifest({ gate: gateConfigToJson(m.gate) }).gate).toEqual(m.gate); // round-trips + // Every valid enum value parses. + for (const value of ["one_shot", "hold_for_review", "advisory_only"] as const) { + expect(parseFocusManifest({ gate: { aiReview: { lowConfidenceDisposition: value } } }).gate.aiReviewLowConfidenceDisposition).toBe(value); + } + // An invalid value warns and is dropped (stays null). + expect(parseFocusManifest({ gate: { aiReview: { lowConfidenceDisposition: "sometimes" } } }).warnings.some((w) => /gate\.aiReview\.lowConfidenceDisposition/.test(w))).toBe(true); + expect(parseFocusManifest({ gate: { aiReview: { lowConfidenceDisposition: "sometimes" } } }).gate.aiReviewLowConfidenceDisposition).toBeNull(); + // The gate alias projects it onto effective settings; absent ⇒ null ⇒ the DB value (here "hold_for_review") is untouched. + const eff = resolveEffectiveSettings({ aiReviewLowConfidenceDisposition: "hold_for_review" } as unknown as RepositorySettings, m); + expect(eff.aiReviewLowConfidenceDisposition).toBe("advisory_only"); + const noFlag = parseFocusManifest({ gate: { aiReview: { mode: "advisory" } } }); + expect(noFlag.gate.aiReviewLowConfidenceDisposition).toBeNull(); + expect(resolveEffectiveSettings({ aiReviewLowConfidenceDisposition: "one_shot" } as unknown as RepositorySettings, noFlag).aiReviewLowConfidenceDisposition).toBe("one_shot"); + }); + it("parses gate.aiReview.combine, makes the gate present, round-trips + resolves it, and warns on a bad value (#2567)", () => { const m = parseFocusManifest({ gate: { aiReview: { combine: "synthesis" } } }); expect(m.gate.present).toBe(true); diff --git a/test/unit/gate-check-policy.test.ts b/test/unit/gate-check-policy.test.ts index 61ba1f0a95..c2011a12ce 100644 --- a/test/unit/gate-check-policy.test.ts +++ b/test/unit/gate-check-policy.test.ts @@ -397,6 +397,30 @@ describe("AI close-confidence threshold gate (#7)", () => { expect(eff.aiReviewMode).toBe("block"); expect(evaluateGateCheck(aiDefectWith(0.5), gateCheckPolicy(eff, null, true)).conclusion).toBe("failure"); }); + + it("resolveEffectiveSettings maps gate.aiReview.lowConfidenceDisposition into the policy (#4603)", () => { + const eff = resolveEffectiveSettings( + settings({ aiReviewMode: "off", aiReviewLowConfidenceDisposition: "hold_for_review" } as Partial), + parseFocusManifest({ gate: { aiReview: { mode: "block", lowConfidenceDisposition: "advisory_only" } } }), + ); + expect(eff.aiReviewLowConfidenceDisposition).toBe("advisory_only"); + expect(eff.aiReviewMode).toBe("block"); + // A sub-floor defect drops to non-blocking under the resolved advisory_only disposition. + expect(evaluateGateCheck(aiDefectWith(0.5), gateCheckPolicy(eff, null, true)).conclusion).toBe("success"); + }); + + it("threads aiReviewLowConfidenceDisposition from settings onto the policy unchanged (#4603)", () => { + const policy = gateCheckPolicy(settings({ aiReviewMode: "block", aiReviewLowConfidenceDisposition: "one_shot" } as Partial), null, true); + expect(policy.aiReviewLowConfidenceDisposition).toBe("one_shot"); + // one_shot ignores the floor -- still blocks below it, same as the unset default. + expect(evaluateGateCheck(aiDefectWith(0.5), policy).conclusion).toBe("failure"); + }); + + it("absent aiReviewLowConfidenceDisposition threads as null, matching the advisory.ts hold_for_review default", () => { + const policy = gateCheckPolicy(settings({ aiReviewMode: "block" }), null, true); + expect(policy.aiReviewLowConfidenceDisposition).toBeNull(); + expect(evaluateGateCheck(aiDefectWith(0.5), policy).conclusion).toBe("failure"); + }); }); describe("slop gate (#530/#532)", () => { diff --git a/test/unit/predicted-gate-engine-branch-coverage.test.ts b/test/unit/predicted-gate-engine-branch-coverage.test.ts index afad05107e..63f3bca7bb 100644 --- a/test/unit/predicted-gate-engine-branch-coverage.test.ts +++ b/test/unit/predicted-gate-engine-branch-coverage.test.ts @@ -60,6 +60,36 @@ describe("predicted-gate engine branch coverage (#2283)", () => { expect(gateAdvisoryInternals.isConfiguredGateBlocker(finding("ai_consensus_defect"), advisory)).toBe(false); expect(gateAdvisoryInternals.isConfiguredGateBlocker(finding("ai_consensus_defect"), block)).toBe(true); expect(gateAdvisoryInternals.isConfiguredGateBlocker(finding("ai_review_split"), advisory)).toBe(false); + // #4603: aiReviewLowConfidenceDisposition branches — the default ("hold_for_review", exercised by the + // bare `block` policy above with no confidence set) and "one_shot" both ignore confidence entirely and + // still block; only "advisory_only" demotes a SUB-floor finding to a non-blocker, and only below the + // configured floor -- at/above it, "advisory_only" still blocks like every other disposition. + expect( + gateAdvisoryInternals.isConfiguredGateBlocker( + { ...finding("ai_consensus_defect"), confidence: 0.2 }, + { ...block, aiReviewLowConfidenceDisposition: "one_shot" }, + ), + ).toBe(true); + expect( + gateAdvisoryInternals.isConfiguredGateBlocker( + { ...finding("ai_consensus_defect"), confidence: 0.2 }, + { ...block, aiReviewLowConfidenceDisposition: "advisory_only", aiReviewCloseConfidence: 0.93 }, + ), + ).toBe(false); + expect( + gateAdvisoryInternals.isConfiguredGateBlocker( + { ...finding("ai_review_split"), confidence: 0.99 }, + { ...block, aiReviewLowConfidenceDisposition: "advisory_only", aiReviewCloseConfidence: 0.93 }, + ), + ).toBe(true); + // A finding with no confidence reported at all defaults to fully-confident (?? 1), so it still blocks + // even under advisory_only regardless of the configured floor. + expect( + gateAdvisoryInternals.isConfiguredGateBlocker(finding("ai_consensus_defect"), { + ...block, + aiReviewLowConfidenceDisposition: "advisory_only", + }), + ).toBe(true); expect(gateAdvisoryInternals.isConfiguredGateBlocker(finding("manifest_linked_issue_required"), advisory)).toBe(false); expect(gateAdvisoryInternals.isConfiguredGateBlocker(finding("manifest_linked_issue_required"), block)).toBe(true); expect(gateAdvisoryInternals.isConfiguredGateBlocker(finding("manifest_missing_tests"), advisory)).toBe(false); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index a14a6a3333..cb8e3edf17 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -6292,6 +6292,142 @@ describe("queue processors", () => { expect(mergeAudit?.n).toBe(0); }); + // Shared cache-input-fingerprint builder for the #4603 pair below -- mirrors "#1"'s own inline fingerprint, + // parameterized only by PR title/number/sha so both tests get a genuine cache HIT (aiCalls stays 0) instead of + // silently falling through to a real (unmocked-defect) AI call on a fingerprint mismatch. + async function cachedSubFloorDefectFingerprint(title: string): Promise { + return aiReviewCacheInputFingerprint({ + title, + mode: "block", + byok: false, + provider: null, + model: null, + aiReviewAllAuthors: false, + aiReviewCloseConfidence: undefined, + aiReviewCombine: null, + aiReviewOnMerge: null, + aiReviewReviewers: null, + gatePack: "oss-anti-slop", + reviewerPlan: undefined, + selfHostProviderConfig: null, + selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null, ollamaModel: null, openaiModel: null, openaiCompatibleModel: null, anthropicModel: null }, + reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = value.length;", additions: 1, deletions: 0 }], + profile: null, + securityFocus: false, + inlineComments: false, + pathInstructions: [], + pathGuidance: "", + repoInstructions: null, + excludePaths: [], + pathFilters: [], + changedPaths: ["src/a.ts"], + features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, + }); + } + + it("#4603: a sub-floor cached ai_consensus_defect under hold_for_review (default) still fails the gate but does NOT one-shot-close", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001); + // aiReviewLowConfidenceDisposition left UNSET — the shipped default (hold_for_review) is what's under test. + await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { close: "auto" }, aiReviewMode: "block", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 8, title: "Sub-floor defect PR", state: "open", user: { login: "contributor" }, head: { sha: "b8" }, labels: [], body: "Closes #1" }); + await upsertPullRequestFile(env, { repoFullName: "owner/agent-repo", pullNumber: 8, path: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, payload: { patch: "@@\n+export const ok = value.length;" } }); + const inputFingerprint = await cachedSubFloorDefectFingerprint("Sub-floor defect PR"); + await putCachedAiReview(env, "owner/agent-repo", 8, "b8", "block", { + notes: "cached review", + reviewerCount: 2, + // 0.3 is well below the default 0.93 close-confidence floor. + findings: [{ code: "ai_consensus_defect", severity: "critical", title: "Cached defect", detail: "Cached critical defect.", confidence: 0.3 }], + metadata: { inputFingerprint }, + }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/8/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = value.length;" }]); + if (url.endsWith("/pulls/8") && init?.method === "PATCH") return Response.json({ number: 8, state: "closed" }); + if (url.endsWith("/pulls/8")) return Response.json({ number: 8, title: "Sub-floor defect PR", state: "open", user: { login: "contributor" }, head: { sha: "b8" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/b8/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/b8/status")) return Response.json({ state: "success", statuses: [] }); + if (url.endsWith("/pulls/8/reviews") && init?.method === "POST") return Response.json({ id: 1 }); + if (url.endsWith("/pulls/8/reviews")) return Response.json([]); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + + await sweepAndDrainPerPr(env, "owner/agent-repo"); + + expect(aiCalls).toBe(0); // the cached AI review was reused — the LLM was never called for this head SHA + // The gate still failed on the AI-judgment blocker (the merge stays blocked). + const blocker = await env.DB.prepare("select blocker_codes_json from gate_outcomes where repo_full_name = ? and pull_number = ? order by rowid desc limit 1").bind("owner/agent-repo", 8).first<{ blocker_codes_json: string }>(); + expect(blocker?.blocker_codes_json).toContain("ai_consensus_defect"); + // But it was NOT one-shot-closed -- the hold suppressed the close autonomy would otherwise have taken. + const closeAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and detail like ?").bind("agent.action.close", "%closed%").first<{ n: number }>(); + expect(closeAudit?.n).toBe(0); + const pr8 = await getPullRequest(env, "owner/agent-repo", 8); + expect(pr8?.state).toBe("open"); + }); + + it("#4603: the SAME sub-floor defect one-shot-closes when aiReviewLowConfidenceDisposition is explicitly one_shot", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { pull_requests: "write" }, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001); + await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { close: "auto" }, aiReviewMode: "block", aiReviewLowConfidenceDisposition: "one_shot", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 9, title: "Sub-floor defect PR (one_shot)", state: "open", user: { login: "contributor" }, head: { sha: "c9" }, labels: [], body: "Closes #1" }); + await upsertPullRequestFile(env, { repoFullName: "owner/agent-repo", pullNumber: 9, path: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, payload: { patch: "@@\n+export const ok = value.length;" } }); + const inputFingerprint = await cachedSubFloorDefectFingerprint("Sub-floor defect PR (one_shot)"); + await putCachedAiReview(env, "owner/agent-repo", 9, "c9", "block", { + notes: "cached review", + reviewerCount: 2, + findings: [{ code: "ai_consensus_defect", severity: "critical", title: "Cached defect", detail: "Cached critical defect.", confidence: 0.3 }], + metadata: { inputFingerprint }, + }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/9/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = value.length;" }]); + if (url.endsWith("/pulls/9") && init?.method === "PATCH") return Response.json({ number: 9, state: "closed" }); + if (url.endsWith("/pulls/9")) return Response.json({ number: 9, title: "Sub-floor defect PR (one_shot)", state: "open", user: { login: "contributor" }, head: { sha: "c9" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/c9/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/c9/status")) return Response.json({ state: "success", statuses: [] }); + if (url.endsWith("/pulls/9/reviews") && init?.method === "POST") return Response.json({ id: 1 }); + if (url.endsWith("/pulls/9/reviews")) return Response.json([]); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + + await sweepAndDrainPerPr(env, "owner/agent-repo"); + + expect(aiCalls).toBe(0); + const blocker = await env.DB.prepare("select blocker_codes_json from gate_outcomes where repo_full_name = ? and pull_number = ? order by rowid desc limit 1").bind("owner/agent-repo", 9).first<{ blocker_codes_json: string }>(); + expect(blocker?.blocker_codes_json).toContain("ai_consensus_defect"); + // one_shot ignores the floor: the close autonomy actually fires this time (contrast with the hold_for_review + // test above, whose closeAudit count is 0). The PR row's `state` column only flips once GitHub's own + // `closed` webhook round-trips back through the normal sync path -- a separate delivery this sweep-driven + // test does not simulate (see the identical gap documented at this file's #linked-issue-hard-rule-persistence + // two-pass test), so the disposition planner's own audit record is the observable proof instead. + const close = await env.DB.prepare("select outcome, detail from audit_events where event_type = ? order by rowid desc limit 1").bind("agent.action.close").first<{ outcome: string; detail: string }>(); + expect(close?.outcome).toBe("completed"); + }); + it("posts the 🟪 reviewing placeholder before the AI review runs, then overwrites it with the verdict (#reviewing-placeholder)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), diff --git a/test/unit/repository-settings-ai-review-low-confidence-disposition.test.ts b/test/unit/repository-settings-ai-review-low-confidence-disposition.test.ts new file mode 100644 index 0000000000..be0b3b234e --- /dev/null +++ b/test/unit/repository-settings-ai-review-low-confidence-disposition.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from "vitest"; +import { getRepositorySettings, upsertRepositorySettings } from "../../src/db/repositories"; +import { createTestEnv } from "../helpers/d1"; + +// #4603: aiReviewLowConfidenceDisposition is the DB-backed, dashboard-settable disposition for a sub- +// aiReviewCloseConfidence-floor ai_consensus_defect/ai_review_split finding -- one_shot (today's pre-#4603 +// unconditional close) | hold_for_review (default -- routes the would-be close to manual review instead) | +// advisory_only (drops a sub-floor finding to fully non-blocking). +describe("repository_settings: aiReviewLowConfidenceDisposition default + round-trip (#4603)", () => { + it("getRepositorySettings returns hold_for_review for a repo with no DB row at all (the shipped safe default)", async () => { + const env = createTestEnv(); + const settings = await getRepositorySettings(env, "acme/brand-new-repo"); + expect(settings.aiReviewLowConfidenceDisposition).toBe("hold_for_review"); + }); + + it("upsertRepositorySettings persists hold_for_review when the caller omits the field entirely", async () => { + const env = createTestEnv(); + await upsertRepositorySettings(env, { repoFullName: "acme/omits-field" }); + const settings = await getRepositorySettings(env, "acme/omits-field"); + expect(settings.aiReviewLowConfidenceDisposition).toBe("hold_for_review"); + }); + + it("an explicit one_shot/advisory_only opt-in round-trips through a re-upsert that carries it forward explicitly", async () => { + const env = createTestEnv(); + await upsertRepositorySettings(env, { repoFullName: "acme/round-trip", aiReviewLowConfidenceDisposition: "one_shot" }); + const settings = await getRepositorySettings(env, "acme/round-trip"); + expect(settings.aiReviewLowConfidenceDisposition).toBe("one_shot"); + // A true read-modify-write caller (the route-handler pattern: spread current settings, then override) must + // carry the persisted value forward explicitly -- upsertRepositorySettings never merges against the DB row. + await upsertRepositorySettings(env, { ...settings, repoFullName: "acme/round-trip" }); + const after = await getRepositorySettings(env, "acme/round-trip"); + expect(after.aiReviewLowConfidenceDisposition).toBe("one_shot"); + }); + + it("advisory_only round-trips distinctly from one_shot, including through an UPDATE (onConflictDoUpdate) of an existing row", async () => { + const env = createTestEnv(); + await upsertRepositorySettings(env, { repoFullName: "acme/advisory-mode", aiReviewLowConfidenceDisposition: "one_shot" }); + await upsertRepositorySettings(env, { repoFullName: "acme/advisory-mode", aiReviewLowConfidenceDisposition: "advisory_only" }); + const settings = await getRepositorySettings(env, "acme/advisory-mode"); + expect(settings.aiReviewLowConfidenceDisposition).toBe("advisory_only"); + }); + + it("an invalid persisted DB value fails closed to hold_for_review on read (parseAiReviewLowConfidenceDisposition's shared fallback)", async () => { + const env = createTestEnv(); + await upsertRepositorySettings(env, { repoFullName: "acme/malformed" }); + await env.DB.prepare("UPDATE repository_settings SET ai_review_low_confidence_disposition = ? WHERE repo_full_name = ?").bind("sometimes", "acme/malformed").run(); + const settings = await getRepositorySettings(env, "acme/malformed"); + expect(settings.aiReviewLowConfidenceDisposition).toBe("hold_for_review"); + }); + + it("an explicit null on write falls back to hold_for_review (parseAiReviewLowConfidenceDisposition's null arm)", async () => { + const env = createTestEnv(); + await upsertRepositorySettings(env, { repoFullName: "acme/explicit-null", aiReviewLowConfidenceDisposition: null }); + const settings = await getRepositorySettings(env, "acme/explicit-null"); + expect(settings.aiReviewLowConfidenceDisposition).toBe("hold_for_review"); + }); +}); diff --git a/test/unit/routes-ai-byok.test.ts b/test/unit/routes-ai-byok.test.ts index bc5615c744..35084d2bc9 100644 --- a/test/unit/routes-ai-byok.test.ts +++ b/test/unit/routes-ai-byok.test.ts @@ -93,6 +93,59 @@ describe("maintainer AI-review config route", () => { expect(res.status).toBe(400); }); + it("sets aiReviewLowConfidenceDisposition (#4603) and preserves unrelated settings", async () => { + const app = createApp(); + const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await upsertRepositorySettings(env, { repoFullName: REPO, gateCheckMode: "enabled", gittensorLabel: "custom-label" }); + const res = await app.request( + `/v1/repos/${REPO}/ai-review`, + { method: "PUT", headers: apiHeaders(env), body: JSON.stringify({ mode: "block", byok: false, lowConfidenceDisposition: "advisory_only" }) }, + env, + ); + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ aiReviewMode: "block", aiReviewLowConfidenceDisposition: "advisory_only" }); + const settings = await getRepositorySettings(env, REPO); + expect(settings.aiReviewLowConfidenceDisposition).toBe("advisory_only"); // persisted + read back (DB column round-trip) + expect(settings.gateCheckMode).toBe("enabled"); // preserved + expect(settings.gittensorLabel).toBe("custom-label"); // preserved + }); + + it("defaults aiReviewLowConfidenceDisposition to hold_for_review when the AI-review config omits it (fresh repo, no row)", async () => { + const app = createApp(); + const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + const res = await app.request( + `/v1/repos/${REPO}/ai-review`, + { method: "PUT", headers: apiHeaders(env), body: JSON.stringify({ mode: "advisory", byok: false }) }, + env, + ); + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ aiReviewLowConfidenceDisposition: "hold_for_review" }); + expect((await getRepositorySettings(env, REPO)).aiReviewLowConfidenceDisposition).toBe("hold_for_review"); + }); + + it("preserves aiReviewLowConfidenceDisposition when an AI-review update omits it (read-modify-write, not a reset)", async () => { + const app = createApp(); + const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await upsertRepositorySettings(env, { repoFullName: REPO, aiReviewLowConfidenceDisposition: "one_shot" }); + + const res = await app.request( + `/v1/repos/${REPO}/ai-review`, + { method: "PUT", headers: apiHeaders(env), body: JSON.stringify({ mode: "advisory", byok: false }) }, + env, + ); + + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ aiReviewMode: "advisory", aiReviewLowConfidenceDisposition: "one_shot" }); + expect((await getRepositorySettings(env, REPO)).aiReviewLowConfidenceDisposition).toBe("one_shot"); + }); + + it("rejects an invalid aiReviewLowConfidenceDisposition value", async () => { + const app = createApp(); + const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + const res = await app.request(`/v1/repos/${REPO}/ai-review`, { method: "PUT", headers: apiHeaders(env), body: JSON.stringify({ mode: "block", lowConfidenceDisposition: "sometimes" }) }, env); + expect(res.status).toBe(400); + }); + it("lets maintainer settings set closeOwnerAuthors without resetting unrelated fields", async () => { const app = createApp(); const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); diff --git a/test/unit/rules.test.ts b/test/unit/rules.test.ts index da9765e6a0..2060b9f74a 100644 --- a/test/unit/rules.test.ts +++ b/test/unit/rules.test.ts @@ -5,6 +5,7 @@ import { buildPullRequestAdvisory, buildRepositoryAdvisory, CHECK_RUN_ANNOTATION_LIMIT, + DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE, evaluateGateCheck, firstAddedLineFromPatch, formatCheckRunOutput, @@ -12,6 +13,7 @@ import { isAiJudgmentOnlyFailure, isDuplicateOnlyFailure, reconcileGateEvaluationForGreenCi, + resolveAiReviewLowConfidenceHold, } from "../../src/rules/advisory"; import type { CollisionReport } from "../../src/signals/engine"; import type { IssueRecord, PullRequestRecord, PullRequestFileRecord, RepositoryRecord } from "../../src/types"; @@ -378,6 +380,146 @@ describe("advisory rules", () => { expect(evaluateGateCheck(splitAdvisory).conclusion).toBe("success"); }); + describe("aiReviewLowConfidenceDisposition (#4603)", () => { + const consensusAdvisory = (confidence: number) => ({ + ...buildPullRequestAdvisory(repo, null), + findings: [ + { + code: "ai_consensus_defect", + title: "AI reviewers agree on a likely critical defect", + severity: "critical" as const, + detail: "Both reviewers flagged the same blocker.", + confidence, + }, + ], + }); + const belowFloor = DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE - 0.1; + const atFloor = DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE; + + it("acceptance (4): at-or-above-floor confidence blocks identically across all three dispositions", () => { + const advisory = consensusAdvisory(atFloor); + for (const disposition of ["one_shot", "hold_for_review", "advisory_only"] as const) { + expect(evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: disposition }).conclusion).toBe("failure"); + } + }); + + it("acceptance (2): sub-floor + one_shot closes exactly like today (still a blocker)", () => { + const advisory = consensusAdvisory(belowFloor); + expect(evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "one_shot" }).conclusion).toBe("failure"); + }); + + it("sub-floor + hold_for_review (default, explicit or unset) still blocks the gate — the hold is downstream", () => { + const advisory = consensusAdvisory(belowFloor); + expect(evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "hold_for_review" }).conclusion).toBe("failure"); + // Unset ⇒ hold_for_review is the default. + expect(evaluateGateCheck(advisory, { aiReviewGateMode: "block" }).conclusion).toBe("failure"); + }); + + it("acceptance (3): sub-floor + advisory_only drops the finding to fully non-blocking", () => { + const advisory = consensusAdvisory(belowFloor); + const evaluation = evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "advisory_only" }); + expect(evaluation.conclusion).toBe("success"); + expect(evaluation.blockers).toHaveLength(0); + }); + + it("advisory_only respects a custom aiReviewCloseConfidence floor, not just the 0.93 default", () => { + const advisory = consensusAdvisory(0.5); + // 0.5 is below a custom 0.6 floor ⇒ non-blocking. + expect(evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "advisory_only", aiReviewCloseConfidence: 0.6 }).conclusion).toBe("success"); + // 0.5 clears a custom 0.4 floor ⇒ still blocks. + expect(evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "advisory_only", aiReviewCloseConfidence: 0.4 }).conclusion).toBe("failure"); + }); + + it("an absent confidence degrades to 1.0 (at-or-above any floor), matching an at-or-above-floor confidence", () => { + const advisory = { + ...buildPullRequestAdvisory(repo, null), + findings: [{ code: "ai_consensus_defect", title: "t", severity: "critical" as const, detail: "d" }], + }; + expect(evaluateGateCheck(advisory, { aiReviewGateMode: "block", aiReviewLowConfidenceDisposition: "advisory_only" }).conclusion).toBe("failure"); + }); + + it("aiReviewGateMode !== block stays non-blocking regardless of disposition (unchanged from today)", () => { + const advisory = consensusAdvisory(belowFloor); + for (const disposition of ["one_shot", "hold_for_review", "advisory_only"] as const) { + expect(evaluateGateCheck(advisory, { aiReviewGateMode: "advisory", aiReviewLowConfidenceDisposition: disposition }).conclusion).toBe("success"); + } + }); + }); + + describe("resolveAiReviewLowConfidenceHold (#4603)", () => { + const finding = (code: string, confidence?: number): import("../../src/types").AdvisoryFinding => ({ + code, + severity: "critical", + title: `t:${code}`, + detail: `d:${code}`, + ...(confidence !== undefined ? { confidence } : {}), + }); + const failure = (findings: import("../../src/types").AdvisoryFinding[]): import("../../src/rules/advisory").GateCheckEvaluation => ({ + enabled: true, + conclusion: "failure", + title: "Gittensory Orb Review Agent: blocked", + summary: "A hard blocker was found.", + blockers: findings, + warnings: [], + }); + const belowFloor = DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE - 0.1; + const atFloor = DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE; + + it("acceptance (1): sub-floor consensus defect + hold_for_review (default) → returns the hold", () => { + const evaluation = failure([finding("ai_consensus_defect", belowFloor)]); + const hold = resolveAiReviewLowConfidenceHold(evaluation, {}); + expect(hold).toBeDefined(); + expect(hold?.reason).toContain("confidence"); + expect(hold?.comment.length).toBeGreaterThan(0); + // Explicit hold_for_review is identical to the unset default. + expect(resolveAiReviewLowConfidenceHold(evaluation, { aiReviewLowConfidenceDisposition: "hold_for_review" })).toEqual(hold); + }); + + it("acceptance (2): sub-floor + one_shot → no hold (closes exactly like today)", () => { + const evaluation = failure([finding("ai_consensus_defect", belowFloor)]); + expect(resolveAiReviewLowConfidenceHold(evaluation, { aiReviewLowConfidenceDisposition: "one_shot" })).toBeUndefined(); + }); + + it("acceptance (3): sub-floor + advisory_only → no hold (the finding is non-blocking, not held)", () => { + const evaluation = failure([finding("ai_consensus_defect", belowFloor)]); + expect(resolveAiReviewLowConfidenceHold(evaluation, { aiReviewLowConfidenceDisposition: "advisory_only" })).toBeUndefined(); + }); + + it("acceptance (4): at-or-above-floor confidence → no hold across every disposition (nothing to hold)", () => { + const evaluation = failure([finding("ai_consensus_defect", atFloor)]); + for (const disposition of ["one_shot", "hold_for_review", "advisory_only"] as const) { + expect(resolveAiReviewLowConfidenceHold(evaluation, { aiReviewLowConfidenceDisposition: disposition })).toBeUndefined(); + } + }); + + it("respects a custom aiReviewCloseConfidence floor", () => { + const evaluation = failure([finding("ai_consensus_defect", 0.5)]); + expect(resolveAiReviewLowConfidenceHold(evaluation, { aiReviewCloseConfidence: 0.6 })).toBeDefined(); + expect(resolveAiReviewLowConfidenceHold(evaluation, { aiReviewCloseConfidence: 0.4 })).toBeUndefined(); + }); + + it("never holds a mixed failure — a genuinely different blocker alongside the AI defect must still close", () => { + const evaluation = failure([finding("ai_consensus_defect", belowFloor), finding("secret_leak")]); + expect(resolveAiReviewLowConfidenceHold(evaluation, {})).toBeUndefined(); + }); + + it("applies to ai_review_split exactly like a consensus defect", () => { + const evaluation = failure([finding("ai_review_split", belowFloor)]); + expect(resolveAiReviewLowConfidenceHold(evaluation, {})).toBeDefined(); + }); + + it("an absent confidence degrades to 1.0 — never holds", () => { + const evaluation = failure([finding("ai_consensus_defect")]); + expect(resolveAiReviewLowConfidenceHold(evaluation, {})).toBeUndefined(); + }); + + it("never holds a non-failure conclusion or an empty blocker list", () => { + const evaluation = failure([finding("ai_consensus_defect", belowFloor)]); + expect(resolveAiReviewLowConfidenceHold({ ...evaluation, conclusion: "success" }, {})).toBeUndefined(); + expect(resolveAiReviewLowConfidenceHold({ ...evaluation, blockers: [] }, {})).toBeUndefined(); + }); + }); + it("keeps readiness score advisory even when legacy config says block", () => { const advisory = buildPullRequestAdvisory(repo, { repoFullName: repo.fullName,