diff --git a/.gittensory.yml.example b/.gittensory.yml.example index f0bec75a1a..cc45372b08 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -230,6 +230,19 @@ gate: - build - test + # Promote a confident AI-judgment-only finding (one the reviewer itself placed under "Blockers", never + # a "Nit") into a real, deterministic gate blocker instead of leaving it advisory (#3907). Only matters + # for repos already running the registry content lane (see contentLane below) — content/registry repos + # have no schema/lint/codecov net to catch a semantically-wrong-but-structurally-valid defect, so their + # own AI reviewer's judgment is the only thing that ever catches it. + # advisory — today's behavior: a decisive deterministic content-lane merge overrides an AI-judgment-only + # failure (an AI opinion alone never one-shot-closes a structurally-clean submission). + # gate — the AI-judgment finding survives into the gate's own blockers, demoting the decision away + # from merge. Reopens the risk an AI hallucination can one-shot-close a clean PR — an + # explicit, per-repo trade-off, never the default. + # gate | advisory. Default: advisory. Config-as-code only — no DB column or dashboard toggle. + aiJudgmentBlockers: advisory + # Composite merge-readiness gate (no min score). # off | advisory | block. Default: off. mergeReadiness: off diff --git a/config/examples/gittensory.full.yml b/config/examples/gittensory.full.yml index 84ec82f502..8bd3578ea5 100644 --- a/config/examples/gittensory.full.yml +++ b/config/examples/gittensory.full.yml @@ -243,6 +243,19 @@ gate: - build - test + # Promote a confident AI-judgment-only finding (one the reviewer itself placed under "Blockers", never + # a "Nit") into a real, deterministic gate blocker instead of leaving it advisory (#3907). Only matters + # for repos already running the registry content lane (see contentLane below) — content/registry repos + # have no schema/lint/codecov net to catch a semantically-wrong-but-structurally-valid defect, so their + # own AI reviewer's judgment is the only thing that ever catches it. + # advisory — today's behavior: a decisive deterministic content-lane merge overrides an AI-judgment-only + # failure (an AI opinion alone never one-shot-closes a structurally-clean submission). + # gate — the AI-judgment finding survives into the gate's own blockers, demoting the decision away + # from merge. Reopens the risk an AI hallucination can one-shot-close a clean PR — an + # explicit, per-repo trade-off, never the default. + # gate | advisory. Default: advisory. Config-as-code only — no DB column or dashboard toggle. + aiJudgmentBlockers: advisory + # Composite merge-readiness gate (no min score). # off | advisory | block. Default: off. mergeReadiness: off diff --git a/packages/gittensory-engine/src/focus-manifest.ts b/packages/gittensory-engine/src/focus-manifest.ts index 157a956ccd..a3df275b36 100644 --- a/packages/gittensory-engine/src/focus-manifest.ts +++ b/packages/gittensory-engine/src/focus-manifest.ts @@ -162,6 +162,17 @@ export type FocusManifestGateConfig = { * (unset) ⇒ no generic fallback configured — the live-CI aggregate keeps today's fold-all behavior * when branch protection is also unreadable. See {@link RepositorySettings.expectedCiContexts}. */ expectedCiContexts: ReadonlyArray | null; + /** `gate.aiJudgmentBlockers` (#3907): "gate" | "advisory", null (unset) ⇒ "advisory" (byte-identical to + * today everywhere that doesn't opt in). Config-as-code only, YML-only (no DB column, no dashboard + * toggle) — mirrors `contentLane`'s own YML-only shape, since this only has an effect for repos already + * running the registry content lane. When "gate", a confident AI-judgment-only finding that + * applySurfaceGate's default "advisory" behavior would otherwise let a decisive surface merge override + * instead SURVIVES into the deterministic gate's own blockers array, demoting `decision` away from + * `merge` — see content-lane-wire.ts's `applySurfaceGate` guard #3 and `evaluateWithSurfaceLane` for the + * wiring. This deliberately reopens exactly the risk #2592 accepted for the general case (an AI + * hallucination can one-shot-close a structurally-clean PR) as an explicit, per-repo, documented + * trade-off — never the default. */ + aiJudgmentBlockersMode: "gate" | "advisory" | null; }; // The converged per-PR review features a self-host operator toggles PER-REPO under `features:` in the private @@ -845,6 +856,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = { claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, + aiJudgmentBlockersMode: null, }; const EMPTY_FEATURES_CONFIG: FocusManifestFeaturesConfig = { @@ -1176,6 +1188,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu claCheckRunName: parsePublicSafeText(claRecord?.checkRunName, "gate.cla.checkRunName", warnings), claCheckRunAppSlug: parsePublicSafeText(claRecord?.checkRunAppSlug, "gate.cla.checkRunAppSlug", warnings), expectedCiContexts: normalizeOptionalStringList(record.expectedCiContexts, "gate.expectedCiContexts", warnings), + aiJudgmentBlockersMode: normalizeOptionalEnum(record.aiJudgmentBlockers, "gate.aiJudgmentBlockers", ["gate", "advisory"] as const, warnings), }; // #2266: the flag is parsed, clamped, and threaded end-to-end, but the gate evaluator never reads it — a // maintainer who sets it to true believing it softens a blocker for newcomers gets no such effect. Surface @@ -1218,7 +1231,8 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu gate.claConsentPhrase !== null || gate.claCheckRunName !== null || gate.claCheckRunAppSlug !== null || - gate.expectedCiContexts !== null; + gate.expectedCiContexts !== null || + gate.aiJudgmentBlockersMode !== null; return gate; } @@ -1293,6 +1307,7 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue { out.cla = cla; } if (gate.expectedCiContexts !== null) out.expectedCiContexts = gate.expectedCiContexts as JsonValue; + if (gate.aiJudgmentBlockersMode !== null) out.aiJudgmentBlockers = gate.aiJudgmentBlockersMode; return out; } diff --git a/src/review/content-lane-wire.ts b/src/review/content-lane-wire.ts index 106cb4d4be..0c939bc9ac 100644 --- a/src/review/content-lane-wire.ts +++ b/src/review/content-lane-wire.ts @@ -102,6 +102,12 @@ export function surfaceVerdictToGate(result: SurfaceReviewResult): { * result rather than silently dropped — see `evaluateWithSurfaceLane` for the companion `advisory.findings` * cleanup that keeps the public comment from re-surfacing the overridden AI defect via a separate path. * + * `opts.aiJudgmentBlockersMode` (#3907): a per-repo `.gittensory.yml` `gate.aiJudgmentBlockers` opt-in that + * SKIPS this exception when set to `"gate"` — an AI-judgment-only failure then falls through to the union + * below like any other blocker, letting a confidently-flagged content-correctness defect actually gate the + * merge. Default (`null`/`undefined`/`"advisory"`) preserves this exception exactly as documented above, + * byte-identical to pre-#3907 behavior for every repo that doesn't opt in. + * * A second, analogous exception (guard #4) applies when the generic gate's blockers are ALL duplicate-only * (a same-linked-issue `duplicate_pr_risk` finding escalated into a blocker by `duplicatePrGateMode: "block"`, * see `isDuplicateOnlyFailure`): a decisive surface merge downgrades that failure to a HOLD (neutral) rather than @@ -117,6 +123,7 @@ export function surfaceVerdictToGate(result: SurfaceReviewResult): { export function applySurfaceGate( generic: GateCheckEvaluation | undefined, surface: GateCheckEvaluation | null, + opts?: { aiJudgmentBlockersMode?: "gate" | "advisory" | null | undefined }, ): GateCheckEvaluation | undefined { if (surface === null) return generic; if (!generic) return surface; // gate off → surface stands @@ -127,7 +134,11 @@ export function applySurfaceGate( if (surface.conclusion === "success") return generic; return surface; } - if (isAiJudgmentOnlyFailure(generic) && surface.conclusion === "success") { + // #3907: opt-in escape hatch from guard #3 below. Default (null/undefined/"advisory") preserves today's + // behavior byte-identically. "gate" skips the override entirely, so an AI-judgment-only failure falls + // through to the unconditional union+failure return at the bottom of this function like any other + // blocker — the opted-in repo's own AI reviewer becomes a real, deterministic-gate-blocking signal. + if (opts?.aiJudgmentBlockersMode !== "gate" && isAiJudgmentOnlyFailure(generic) && surface.conclusion === "success") { return { ...surface, warnings: [...generic.warnings, ...surface.warnings] }; } if (isDuplicateOnlyFailure(generic) && surface.conclusion === "success") { @@ -295,8 +306,16 @@ export async function evaluateWithSurfaceLane( advisory: args.advisory, files: await args.getChangedFiles(), }); - const result = applySurfaceGate(gateEvaluation, surfaceGate); - if (gateEvaluation && surfaceGate?.conclusion === "success" && isAiJudgmentOnlyFailure(gateEvaluation)) { + // #3907: null/undefined manifest.gate is treated the same as an explicit "advisory" — see + // applySurfaceGate's own doc comment for what "gate" mode does. + const aiJudgmentBlockersMode = manifest?.gate.aiJudgmentBlockersMode ?? undefined; + const result = applySurfaceGate(gateEvaluation, surfaceGate, { aiJudgmentBlockersMode }); + if ( + aiJudgmentBlockersMode !== "gate" && + gateEvaluation && + surfaceGate?.conclusion === "success" && + isAiJudgmentOnlyFailure(gateEvaluation) + ) { args.advisory.findings = args.advisory.findings.filter((finding) => !AI_JUDGMENT_BLOCKER_CODES.has(finding.code)); } return result; diff --git a/test/unit/content-lane-wire.test.ts b/test/unit/content-lane-wire.test.ts index 7807d92794..aafd9d1620 100644 --- a/test/unit/content-lane-wire.test.ts +++ b/test/unit/content-lane-wire.test.ts @@ -130,6 +130,32 @@ describe("applySurfaceGate", () => { expect(out?.conclusion).toBe("failure"); expect(out?.blockers).toEqual([split, ...surfaceClose.blockers]); // union — the AI-only exception only applies to a surface MERGE }); + it("#3907: aiJudgmentBlockersMode 'gate' skips the AI-judgment-only override — the finding survives into the failure union, reproducing PR #3910's shape", () => { + // The exact repro this issue is about: a confidently-flagged, correct AI-judgment finding (a registry + // provider slug semantically wrong for its domain) that the deterministic surface lane's own schema/shape + // scan can never catch, so only AI judgment ever surfaces it. + const providerMisattribution: AdvisoryFinding = { + code: "ai_consensus_defect", + title: "AI reviewers agree on a likely critical defect", + severity: "critical", + detail: "provider is set to \"gittensory\" (an unrelated tool's slug) instead of \"gittensor\"", + }; + const genericAiOnly = gate({ conclusion: "failure", blockers: [providerMisattribution], warnings: [] }); + const surfaceMerge = gate({ conclusion: "success", title: "Surface", summary: "structurally valid entry" }); + const out = applySurfaceGate(genericAiOnly, surfaceMerge, { aiJudgmentBlockersMode: "gate" }); + // Opted in: the AI-judgment finding is NOT overridden — decision is no longer merge. + expect(out?.conclusion).toBe("failure"); + expect(out?.blockers).toEqual([providerMisattribution]); + }); + it("#3907: aiJudgmentBlockersMode 'advisory' (explicit) behaves identically to the default (unset) — byte-identical override", () => { + const aiConsensusDefect: AdvisoryFinding = { code: "ai_consensus_defect", title: "AI defect", severity: "critical", detail: "" }; + const genericAiOnly = gate({ conclusion: "failure", blockers: [aiConsensusDefect], warnings: [] }); + const surfaceMerge = gate({ conclusion: "success", title: "Surface", summary: "valid entry" }); + const withExplicitAdvisory = applySurfaceGate(genericAiOnly, surfaceMerge, { aiJudgmentBlockersMode: "advisory" }); + const withDefault = applySurfaceGate(genericAiOnly, surfaceMerge); + expect(withExplicitAdvisory).toEqual(withDefault); + expect(withExplicitAdvisory?.conclusion).toBe("success"); + }); it("a MIXED generic failure (an AI-judgment code plus a real blocker) is not AI-judgment-only — still overrides a surface merge", () => { const secret: AdvisoryFinding = { code: "secret_leak", title: "Secret", severity: "critical", detail: "leaked key" }; const aiConsensusDefect: AdvisoryFinding = { code: "ai_consensus_defect", title: "AI defect", severity: "critical", detail: "" }; @@ -366,6 +392,85 @@ describe("evaluateWithSurfaceLane (the processor seam helper)", () => { expect(advisory.findings).toEqual([otherWarning]); }); + it("#3907 REGRESSION: on an opted-in repo (gate.aiJudgmentBlockers: 'gate'), a confident AI-judgment-only finding survives a decisive surface merge — decision is no longer merge, and the finding stays in advisory.findings for the public comment", async () => { + const bodies: Record = { + "HEAD:registry/subnets/foo.json": doc([existing, newEntry]), + "BASE:registry/subnets/foo.json": doc([existing]), + }; + vi.stubGlobal("fetch", async (url: string | URL) => { + const m = /\/contents\/(.+)\?ref=(.+)$/.exec(String(url)); + if (!m) return new Response("nope", { status: 404 }); + const path = m[1]!.split("/").map(decodeURIComponent).join("/"); + const body = bodies[`${decodeURIComponent(m[2]!)}:${path}`]; + return body === undefined ? new Response("missing", { status: 404 }) : new Response(body); + }); + const providerMisattribution: AdvisoryFinding = { + code: "ai_consensus_defect", + title: "AI reviewers agree on a likely critical defect", + severity: "critical", + detail: "provider is set to \"gittensory\" instead of \"gittensor\"", + }; + const advisory = { findings: [providerMisattribution] }; + const genericAiOnly = gate({ conclusion: "failure", blockers: [providerMisattribution], warnings: [] }); + const wiredEnv = { GITTENSORY_REVIEW_CONTENT_LANE: "true", GITTENSORY_REVIEW_REPOS: REPO } as unknown as Env; + const optedInManifest = (): Promise => + Promise.resolve(parseFocusManifest({ wantedPaths: ["src/"], gate: { aiJudgmentBlockers: "gate" } })); + const out = await evaluateWithSurfaceLane( + wiredEnv, + REPO, + true, + genericAiOnly, + { + installationId: null, + pr: { headSha: "HEAD", baseRef: "BASE" }, + repo: { defaultBranch: "main" }, + advisory, + getChangedFiles: async () => [{ path: SUBNET, status: "modified" }], + }, + optedInManifest, + ); + // The core deliverable: opted in, the AI-judgment finding is no longer overridden by the clean surface merge. + expect(out?.conclusion).not.toBe("success"); + expect(out?.blockers).toEqual([providerMisattribution]); + // Unlike the opted-out REGRESSION test above, the finding must NOT be stripped from advisory.findings — + // it's now a real blocker, so the public comment needs to keep showing it. + expect(advisory.findings).toEqual([providerMisattribution]); + }); + + it("#3907: an opted-OUT repo (no gate.aiJudgmentBlockers configured) is unaffected — byte-identical to the pre-#3907 REGRESSION test above", async () => { + const bodies: Record = { + "HEAD:registry/subnets/foo.json": doc([existing, newEntry]), + "BASE:registry/subnets/foo.json": doc([existing]), + }; + vi.stubGlobal("fetch", async (url: string | URL) => { + const m = /\/contents\/(.+)\?ref=(.+)$/.exec(String(url)); + if (!m) return new Response("nope", { status: 404 }); + const path = m[1]!.split("/").map(decodeURIComponent).join("/"); + const body = bodies[`${decodeURIComponent(m[2]!)}:${path}`]; + return body === undefined ? new Response("missing", { status: 404 }) : new Response(body); + }); + const aiConsensusDefect: AdvisoryFinding = { code: "ai_consensus_defect", title: "AI defect", severity: "critical", detail: "hallucinated" }; + const advisory = { findings: [aiConsensusDefect] }; + const genericAiOnly = gate({ conclusion: "failure", blockers: [aiConsensusDefect], warnings: [] }); + const wiredEnv = { GITTENSORY_REVIEW_CONTENT_LANE: "true", GITTENSORY_REVIEW_REPOS: REPO } as unknown as Env; + const out = await evaluateWithSurfaceLane( + wiredEnv, + REPO, + true, + genericAiOnly, + { + installationId: null, + pr: { headSha: "HEAD", baseRef: "BASE" }, + repo: { defaultBranch: "main" }, + advisory, + getChangedFiles: async () => [{ path: SUBNET, status: "modified" }], + }, + noConfigManifest, // same zero-config manifest as the default-behavior tests above + ); + expect(out?.conclusion).toBe("success"); // default advisory behavior: override still fires + expect(advisory.findings).toEqual([]); // stripped, same as the pre-#3907 REGRESSION test + }); + it("does NOT touch advisory.findings when the surface lane defers or the generic gate isn't AI-judgment-only", async () => { const aiConsensusDefect: AdvisoryFinding = { code: "ai_consensus_defect", title: "AI defect", severity: "critical", detail: "" }; const secret: AdvisoryFinding = { code: "secret_leak", title: "Secret", severity: "critical", detail: "leaked" }; diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index d10c08d2d6..77335b8e5b 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -273,6 +273,7 @@ describe(".gittensory.yml.example field-exhaustiveness (#1670)", () => { claCheckRunName: "checkRunName:", claCheckRunAppSlug: "checkRunAppSlug:", expectedCiContexts: "expectedCiContexts:", + aiJudgmentBlockersMode: "aiJudgmentBlockers:", } satisfies Record, string>; it.each(Object.entries(GATE_FIELD_TOKENS))("documents gate.%s", (_field, token) => { @@ -799,7 +800,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 }, + 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 }, 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, 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 }, @@ -1109,7 +1110,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 }); + 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 }); }); it("parses gate.mergeReadiness + gate.firstTimeContributorGrace, round-trips them, and warns on bad values (#822)", () => {