Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/loopover-engine/src/advisory/gate-advisory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,13 @@ function evaluateGateCheckCore(advisoryResult: Advisory, policy: GateCheckPolicy
// automatically — NEVER a failure, so a contributor PR is never auto-CLOSED because a model hiccupped. This
// is evaluated AFTER the deterministic blockers above, so a real violation (secret_leak, duplicate,
// missing-issue, slop, quality) still blocks: an inconclusive AI can no longer bury a blocked PR in a hold.
if (advisoryResult.findings.some((finding) => finding.code === "ai_review_inconclusive")) {
// #10016: mirrors isEvaluationBlocker's CLA case -- the finding is produced in every aiReviewGateMode (the
// two producers in ai-review-orchestration.ts don't consult the mode), so only "block" should ever HOLD the
// gate on it. "advisory"/"off" mode's whole contract is "surface findings, never affect the verdict";
// unconditionally holding here diverted a clean, green advisory-mode PR to manual review over a race or a
// transient model failure the repo never opted into blocking on. The finding still reaches the panel via
// `gateWarnings` below either way -- only the HOLD is mode-gated, never the visibility.
if (gatePolicyBlocks(effective.aiReviewGateMode, "advisory") && advisoryResult.findings.some((finding) => finding.code === "ai_review_inconclusive")) {
return {
enabled: true,
conclusion: "neutral",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
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";

function inconclusiveAdvisory(extraFindings: AdvisoryFinding[] = []): Advisory {
const finding: AdvisoryFinding = {
code: "ai_review_inconclusive",
title: "AI review could not complete for this PR head",
severity: "warning",
detail: "The AI review attempt did not produce a result.",
action: "The review is retried automatically after a short cooldown.",
};
return {
id: "advisory-1",
targetType: "pull_request",
targetKey: "JSONbored/loopover#1",
repoFullName: "JSONbored/loopover",
conclusion: "success",
severity: "warning",
title: "LoopOver review",
summary: "",
findings: [finding, ...extraFindings],
generatedAt: "2026-07-31T00:00:00.000Z",
};
}

// #10016 (gate-decision twin of src/rules/advisory.ts, kept in sync per checkGateDecisionVersionBump): mirrors
// the host copy's own regression tests for the ai_review_inconclusive hold's aiReviewGateMode gating.

test("REGRESSION: an inconclusive review does not hold an advisory-mode repo's otherwise-clean gate", () => {
const evaluation = evaluateGateCheck(inconclusiveAdvisory());
assert.equal(evaluation.conclusion, "success");
assert.ok(evaluation.warnings.some((finding) => finding.code === "ai_review_inconclusive"));
});

test("stays non-blocking under an explicit advisory or off mode", () => {
const advisory = inconclusiveAdvisory();
const advisoryMode = evaluateGateCheck(advisory, { aiReviewGateMode: "advisory" });
assert.equal(advisoryMode.conclusion, "success");
assert.ok(advisoryMode.warnings.some((finding) => finding.code === "ai_review_inconclusive"));

const offMode = evaluateGateCheck(advisory, { aiReviewGateMode: "off" });
assert.equal(offMode.conclusion, "success");
assert.ok(offMode.warnings.some((finding) => finding.code === "ai_review_inconclusive"));
});

test("still HOLDS the gate (neutral) under aiReviewGateMode: block, with the unchanged title", () => {
const evaluation = evaluateGateCheck(inconclusiveAdvisory(), { aiReviewGateMode: "block" });
assert.equal(evaluation.conclusion, "neutral");
assert.equal(evaluation.title, "LoopOver Orb Review Agent — held for human review");
assert.equal(evaluation.blockers.length, 0);
});

test("the unconditional secret_scan_incomplete hold still fires once the AI hold no longer does", () => {
const advisory = inconclusiveAdvisory([
{
code: "secret_scan_incomplete",
title: "Patch-less file(s) could not be fully scanned for secrets (1)",
severity: "critical",
detail: "GitHub omitted inline diff for: secrets.env.",
action: "Ensure patch-less files are within scan limits or split the change so secrets can be verified.",
},
]);
const evaluation = evaluateGateCheck(advisory);
assert.equal(evaluation.conclusion, "neutral");
assert.equal(evaluation.blockers.length, 0);
});
11 changes: 10 additions & 1 deletion src/rules/advisory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,16 @@ function evaluateGateCheckCore(advisoryResult: Advisory, policy: GateCheckPolicy
// automatically — NEVER a failure, so a contributor PR is never auto-CLOSED because a model hiccupped. This
// is evaluated AFTER the deterministic blockers above, so a real violation (secret_leak, duplicate,
// missing-issue, slop, quality) still blocks: an inconclusive AI can no longer bury a blocked PR in a hold.
if (advisoryResult.findings.some((finding) => finding.code === "ai_review_inconclusive")) {
// #10016: mirrors isEvaluationBlocker's CLA case -- the finding is produced in every aiReviewGateMode (the
// two producers in ai-review-orchestration.ts don't consult the mode), so only "block" should ever HOLD the
// gate on it. "advisory"/"off" mode's whole contract is "surface findings, never affect the verdict";
// unconditionally holding here diverted a clean, green advisory-mode PR to manual review over a race or a
// transient model failure the repo never opted into blocking on. The finding still reaches the panel via
// `gateWarnings` below either way -- only the HOLD is mode-gated, never the visibility.
if (
gateMode(effective.aiReviewGateMode ?? "advisory") === "block" &&
advisoryResult.findings.some((finding) => finding.code === "ai_review_inconclusive")
) {
return {
enabled: true,
conclusion: "neutral",
Expand Down
24 changes: 18 additions & 6 deletions test/unit/gate-check-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,14 @@ describe(".loopover.yml settings override (resolveEffectiveSettings)", () => {
});

describe("AI fail-closed hold (#ai-fail-closed)", () => {
it("holds the gate NEUTRAL (held for human, never a failure-close) when an AI review is inconclusive", () => {
it("holds the gate NEUTRAL (held for human, never a failure-close) when an AI review is inconclusive under aiReviewGateMode: block", () => {
const adv: Advisory = {
...missingIssueAdvisory(),
findings: [{ code: "ai_review_inconclusive", title: "AI review could not be completed", severity: "warning", detail: "no usable verdict", action: "held for human" }],
};
const result = evaluateGateCheck(adv, gateCheckPolicy(settings(), null, true));
// #10016: the hold is gated on aiReviewGateMode -- this suite's own repo default is unset (resolves
// "advisory"), so the block-mode hold this describe block exercises must opt in explicitly.
const result = evaluateGateCheck(adv, gateCheckPolicy(settings({ aiReviewMode: "block" }), null, true));
expect(result.conclusion).toBe("neutral");
expect(result.blockers).toEqual([]);
});
Expand All @@ -175,13 +177,13 @@ describe("AI fail-closed hold (#ai-fail-closed)", () => {
{ code: "ai_review_inconclusive", title: "AI review could not be completed", severity: "warning", detail: "no usable verdict", action: "held for human" },
],
};
const result = evaluateGateCheck(adv, gateCheckPolicy(settings(), null, true));
const result = evaluateGateCheck(adv, gateCheckPolicy(settings({ aiReviewMode: "block" }), null, true));
// An inconclusive AI can no longer bury a real violation in a "held" state — the secret_leak still hard-blocks.
expect(result.conclusion).toBe("failure");
expect(result.blockers.map((blocker) => blocker.code)).toContain("secret_leak");
});

it("holds the gate NEUTRAL (never a failure-close) when the AI review lock is held by another in-flight pass (#confirmed-bug)", () => {
it("holds the gate NEUTRAL (never a failure-close) when the AI review lock is held by another in-flight pass under aiReviewGateMode: block (#confirmed-bug)", () => {
// Same code, different finding text — the lock-contention finding constructed by runAiReviewForAdvisory's
// new claim-failure branch. advisory.ts only keys on `code`, so this proves the mechanism end-to-end for
// the new finding shape without needing to touch advisory.ts.
Expand All @@ -197,7 +199,7 @@ describe("AI fail-closed hold (#ai-fail-closed)", () => {
},
],
};
const result = evaluateGateCheck(adv, gateCheckPolicy(settings(), null, true));
const result = evaluateGateCheck(adv, gateCheckPolicy(settings({ aiReviewMode: "block" }), null, true));
expect(result.conclusion).toBe("neutral");
expect(result.blockers).toEqual([]);
});
Expand All @@ -216,11 +218,21 @@ describe("AI fail-closed hold (#ai-fail-closed)", () => {
},
],
};
const result = evaluateGateCheck(adv, gateCheckPolicy(settings(), null, true));
const result = evaluateGateCheck(adv, gateCheckPolicy(settings({ aiReviewMode: "block" }), null, true));
expect(result.conclusion).toBe("failure");
expect(result.blockers.map((blocker) => blocker.code)).toContain("secret_leak");
});

it("REGRESSION (#10016): an inconclusive AI review no longer holds the gate when aiReviewGateMode resolves to advisory (the repo default)", () => {
const adv: Advisory = {
...missingIssueAdvisory(),
findings: [{ code: "ai_review_inconclusive", title: "AI review could not be completed", severity: "warning", detail: "no usable verdict", action: "held for human" }],
};
const result = evaluateGateCheck(adv, gateCheckPolicy(settings(), null, true));
expect(result.conclusion).toBe("success");
expect(result.warnings.map((finding) => finding.code)).toContain("ai_review_inconclusive");
});

it("an enforced pre-merge check (pre_merge_check_required) hard-blocks; the advisory variant never does (#review-pre-merge-checks)", () => {
const enforced: Advisory = {
...missingIssueAdvisory(),
Expand Down
3 changes: 2 additions & 1 deletion test/unit/predicted-gate-engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,8 @@ describe("predicted-gate engine module coverage (#2283)", () => {
findings: [{ code: "ai_review_inconclusive", severity: "warning", title: "ai", detail: "ai" }],
generatedAt: "2026-01-01T00:00:00.000Z",
},
{},
// #10016: the hold only fires under aiReviewGateMode: block now -- an unset mode resolves "advisory".
{ aiReviewGateMode: "block" },
);
expect(aiHold.conclusion).toBe("neutral");

Expand Down
63 changes: 62 additions & 1 deletion test/unit/rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
resolveAiReviewLowConfidenceHold,
} from "../../src/rules/advisory";
import type { CollisionReport } from "../../src/signals/engine";
import type { IssueRecord, PullRequestRecord, PullRequestFileRecord, RepositoryRecord } from "../../src/types";
import type { Advisory, IssueRecord, PullRequestRecord, PullRequestFileRecord, RepositoryRecord } from "../../src/types";

const repo: RepositoryRecord = {
fullName: "JSONbored/loopover",
Expand Down Expand Up @@ -738,6 +738,67 @@ describe("advisory rules", () => {
expect(evaluateGateCheck(splitAdvisory).conclusion).toBe("success");
});

describe("ai_review_inconclusive hold is gated by aiReviewGateMode (#10016)", () => {
const inconclusiveAdvisory = (extraFindings: Advisory["findings"] = []) => ({
...buildPullRequestAdvisory(repo, null),
findings: [
{
code: "ai_review_inconclusive" as const,
title: "AI review could not complete for this PR head",
severity: "warning" as const,
detail: "The AI review attempt did not produce a result.",
action: "The review is retried automatically after a short cooldown.",
},
...extraFindings,
],
});

it("REGRESSION: an inconclusive review does not hold an advisory-mode repo's otherwise-clean gate", () => {
const result = evaluateGateCheck(inconclusiveAdvisory());
expect(result.conclusion).toBe("success");
expect(result.warnings.map((finding) => finding.code)).toContain("ai_review_inconclusive");
});

it("stays non-blocking under an explicit advisory or off mode", () => {
const advisory = inconclusiveAdvisory();
const advisoryMode = evaluateGateCheck(advisory, { aiReviewGateMode: "advisory" });
expect(advisoryMode.conclusion).toBe("success");
expect(advisoryMode.warnings.map((finding) => finding.code)).toContain("ai_review_inconclusive");

const offMode = evaluateGateCheck(advisory, { aiReviewGateMode: "off" });
expect(offMode.conclusion).toBe("success");
expect(offMode.warnings.map((finding) => finding.code)).toContain("ai_review_inconclusive");
});

it("still HOLDS the gate (neutral) under aiReviewGateMode: block, with the unchanged title", () => {
const result = evaluateGateCheck(inconclusiveAdvisory(), { aiReviewGateMode: "block" });
expect(result.conclusion).toBe("neutral");
expect(result.title).toBe("LoopOver Orb Review Agent — held for human review");
expect(result.blockers).toEqual([]);
});

it("the unconditional secret_scan_incomplete hold still fires once the AI hold no longer does", () => {
const advisory = inconclusiveAdvisory([
{
code: "secret_scan_incomplete",
title: "Patch-less file(s) could not be fully scanned for secrets (1)",
severity: "critical",
detail: "GitHub omitted inline diff for: secrets.env.",
action: "Ensure patch-less files are within scan limits or split the change so secrets can be verified.",
},
]);
const result = evaluateGateCheck(advisory);
expect(result.conclusion).toBe("neutral");
expect(result.blockers).toEqual([]);
});

it("dry-run: displayConclusion previews the block-mode hold while the posted conclusion stays success", () => {
const result = evaluateGateCheck(inconclusiveAdvisory(), { aiReviewGateMode: "advisory", dryRun: true });
expect(result.conclusion).toBe("success");
expect(result.displayConclusion).toBe("neutral");
});
});

describe("aiReviewLowConfidenceDisposition (#4603)", () => {
const consensusAdvisory = (confidence: number) => ({
...buildPullRequestAdvisory(repo, null),
Expand Down
Loading