⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
evaluateGateCheckCore holds the gate neutral whenever an ai_review_inconclusive finding is present and
no deterministic blocker fired (src/rules/advisory.ts:826-840):
// Fail-CLOSED AI hold (#ai-fail-closed, #audit-3.5): with NO deterministic blocker, a block-mode AI review
// that could not return a usable verdict HOLDS the gate (neutral) for a human rather than passing
// automatically — NEVER a failure, so a contributor PR is never auto-CLOSED because a model hiccupped. …
if (advisoryResult.findings.some((finding) => finding.code === "ai_review_inconclusive")) {
return {
enabled: true,
conclusion: "neutral",
…
The comment says "a block-mode AI review". The condition never reads policy.aiReviewGateMode.
aiReviewGateMode defaults to advisory (src/rules/advisory.ts:1317,
gateMode(policy.aiReviewGateMode ?? "advisory")), and its own doc comment states the contract:
"Defaults to advisory — AI never blocks unless the maintainer opts in" (src/rules/advisory.ts:63-65).
The finding is not mode-gated on the producer side either. aiReviewLockContendedResult
(src/queue/ai-review-orchestration.ts:206-227) pushes it whenever a second pass loses the head-lock race,
and aiReviewAttemptFailedResult (src/queue/ai-review-orchestration.ts:255-278) pushes it on any failed
attempt — neither consults the repo's gate mode. So on a repo that never opted into blocking AI review, a
clean, green PR whose only event was two passes racing for the same head produces
conclusion: "neutral", which gateConclusionToVerdict maps to manual
(src/review/unified-comment-bridge.ts:93-95) — a maintainer hold instead of a merge.
The same file already resolved the identical question the opposite way, with the reasoning written out
(src/rules/advisory.ts:1263-1269):
// cla_check_unresolved (#2564): the CLA-bot check-run's conclusion could not be resolved. Unlike the codes
// above (which are never mode-gated), evaluateClaCheck runs for BOTH claGateMode "advisory" and "block" (so
// the finding surfaces either way) — only "block" should ever HOLD the gate on an unresolved check-run.
// "advisory" mode's whole contract is "surface findings, never affect the verdict"; unconditionally holding
// here would violate that for any advisory-mode repo using check-run-only detection (#2564 gate-review
// finding). advisory mode still gets the finding in the panel via the normal warnings path below.
if (code === CLA_CHECK_UNRESOLVED_CODE) return policy.claGateMode === "block";
ai_review_inconclusive is the same shape — a "could not evaluate" signal produced in every mode — and it
holds unconditionally.
The engine twin carries the identical code (packages/loopover-engine/src/advisory/gate-advisory.ts:545-559),
and evaluateGateCheckCore is one of GATE_DECISION_CORE_MARKERS
(scripts/check-engine-parity.ts:35-40), so the two must stay in lock-step.
Requirements
- The
ai_review_inconclusive hold in evaluateGateCheckCore must fire only when the resolved AI gate mode
is block, i.e. only when isConfiguredGateBlocker would treat an AI-judgment finding as a blocker for
this policy. Under advisory or off, the gate must fall through to the checks below it exactly as if the
finding were absent.
- Under
advisory/off the finding must STILL be visible: it is severity: "warning", so it already flows
into warnings via the normal path — assert that it does, so the fix suppresses the HOLD and never the
signal.
- The mode must be resolved from the SAME
effective policy the rest of evaluateGateCheckCore uses (the
post-applyMergeReadinessGate value, src/rules/advisory.ts:808), not from the raw policy argument.
- The change must be mirrored byte-for-byte in
packages/loopover-engine/src/advisory/gate-advisory.ts's
evaluateGateCheckCore so scripts/check-engine-parity.ts stays green. The engine's GateCheckPolicy
already carries aiReviewGateMode (packages/loopover-engine/src/advisory/gate-advisory.ts:62).
- Behaviour that must NOT change: the block-mode hold itself (a block-mode repo with an inconclusive review
still gets conclusion: "neutral" with the same title and summary); the ordering — this check still runs
only inside blockers.length === 0, AFTER the deterministic blockers, so a real violation still blocks; the
secret_scan_incomplete hold below it, which is deliberately unconditional and must stay so; the
size/guardrail holds; and promoteAdvisoryToBlock's dry-run promotion of aiReviewGateMode
(src/rules/advisory.ts:768-773), which means a dry-run displayConclusion still previews the block-mode
hold.
⚠️ Required pattern: mirror isEvaluationBlocker's CLA case (src/rules/advisory.ts:1263-1269) — a
"could not evaluate" signal holds only under the gate mode that owns it. What does NOT satisfy this issue:
mode-gating the PRODUCERS in src/queue/ai-review-orchestration.ts instead (the finding must still surface
as a warning in every mode, and moving the decision out of the pure evaluator breaks the replay contract in
src/review/decision-replay.ts); adding a new policy field for this hold instead of reading
aiReviewGateMode; changing the host copy without the engine twin (the parity check fails); a test-only PR.
Deliverables
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example
changing the host copy without the engine twin, or without the secret_scan_incomplete co-occurrence test —
does not resolve this issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts and packages/loopover-engine/src/**/*.ts; both touched files are measured and gated.
The change turns a one-condition if into a two-condition one — every combination needs a test: finding
present + block, finding present + advisory, finding present + off, finding absent + block. The
?? "advisory" default arm and the explicit-value arm of the mode resolution both need a case.
Engine lines are credited by two uploads whose hits are unioned — add the test to
packages/loopover-engine/test/** as well as the root test/** coverage, or the patch gate can still fail.
Expected Outcome
A repo that never opted into blocking AI review stops having clean, green PRs diverted to manual review
because two passes raced for the same head lock or a model call failed transiently — while a repo that DID
opt in keeps the fail-closed hold exactly as it is today, and every repo still sees the inconclusive finding
in its panel.
Links & Resources
src/rules/advisory.ts:826-840 — the unconditional hold
src/rules/advisory.ts:63-65, :1316-1327 — aiReviewGateMode's advisory-by-default contract
src/rules/advisory.ts:1257-1271 — isEvaluationBlocker's CLA precedent, the pattern to mirror
src/queue/ai-review-orchestration.ts:206-227, :255-275 — the two mode-agnostic producers
src/review/unified-comment-bridge.ts:87-99 — neutral ⇒ manual
packages/loopover-engine/src/advisory/gate-advisory.ts:545-559 — the engine twin
scripts/check-engine-parity.ts:35-40, :199 — the parity markers that pin the two together
Context
evaluateGateCheckCoreholds the gateneutralwhenever anai_review_inconclusivefinding is present andno deterministic blocker fired (
src/rules/advisory.ts:826-840):The comment says "a block-mode AI review". The condition never reads
policy.aiReviewGateMode.aiReviewGateModedefaults toadvisory(src/rules/advisory.ts:1317,gateMode(policy.aiReviewGateMode ?? "advisory")), and its own doc comment states the contract:"Defaults to advisory — AI never blocks unless the maintainer opts in" (
src/rules/advisory.ts:63-65).The finding is not mode-gated on the producer side either.
aiReviewLockContendedResult(
src/queue/ai-review-orchestration.ts:206-227) pushes it whenever a second pass loses the head-lock race,and
aiReviewAttemptFailedResult(src/queue/ai-review-orchestration.ts:255-278) pushes it on any failedattempt — neither consults the repo's gate mode. So on a repo that never opted into blocking AI review, a
clean, green PR whose only event was two passes racing for the same head produces
conclusion: "neutral", whichgateConclusionToVerdictmaps tomanual(
src/review/unified-comment-bridge.ts:93-95) — a maintainer hold instead of a merge.The same file already resolved the identical question the opposite way, with the reasoning written out
(
src/rules/advisory.ts:1263-1269):ai_review_inconclusiveis the same shape — a "could not evaluate" signal produced in every mode — and itholds unconditionally.
The engine twin carries the identical code (
packages/loopover-engine/src/advisory/gate-advisory.ts:545-559),and
evaluateGateCheckCoreis one ofGATE_DECISION_CORE_MARKERS(
scripts/check-engine-parity.ts:35-40), so the two must stay in lock-step.Requirements
ai_review_inconclusivehold inevaluateGateCheckCoremust fire only when the resolved AI gate modeis
block, i.e. only whenisConfiguredGateBlockerwould treat an AI-judgment finding as a blocker forthis policy. Under
advisoryoroff, the gate must fall through to the checks below it exactly as if thefinding were absent.
advisory/offthe finding must STILL be visible: it isseverity: "warning", so it already flowsinto
warningsvia the normal path — assert that it does, so the fix suppresses the HOLD and never thesignal.
effectivepolicy the rest ofevaluateGateCheckCoreuses (thepost-
applyMergeReadinessGatevalue,src/rules/advisory.ts:808), not from the rawpolicyargument.packages/loopover-engine/src/advisory/gate-advisory.ts'sevaluateGateCheckCoresoscripts/check-engine-parity.tsstays green. The engine'sGateCheckPolicyalready carries
aiReviewGateMode(packages/loopover-engine/src/advisory/gate-advisory.ts:62).still gets
conclusion: "neutral"with the same title and summary); the ordering — this check still runsonly inside
blockers.length === 0, AFTER the deterministic blockers, so a real violation still blocks; thesecret_scan_incompletehold below it, which is deliberately unconditional and must stay so; thesize/guardrail holds; and
promoteAdvisoryToBlock's dry-run promotion ofaiReviewGateMode(
src/rules/advisory.ts:768-773), which means a dry-rundisplayConclusionstill previews the block-modehold.
Deliverables
src/rules/advisory.ts: theai_review_inconclusivebranch inevaluateGateCheckCorefires only whenthe effective
aiReviewGateModeresolves toblock.packages/loopover-engine/src/advisory/gate-advisory.ts: the identical change in itsevaluateGateCheckCore.test/unit/rules.test.ts:evaluateGateCheck(advisoryWith(["ai_review_inconclusive"]), {})(no
aiReviewGateMode) returnsconclusion: "success"and itswarningscontain theai_review_inconclusivefinding.test/unit/rules.test.ts: the same advisory with{ aiReviewGateMode: "advisory" }returnsconclusion: "success", and with{ aiReviewGateMode: "off" }likewise.test/unit/rules.test.ts: the same advisory with{ aiReviewGateMode: "block" }stillreturns
conclusion: "neutral"with the unchanged"LoopOver Orb Review Agent — held for human review"title.test/unit/rules.test.ts: an advisory carrying BOTHai_review_inconclusiveandsecret_scan_incompletewith noaiReviewGateModereturnsconclusion: "neutral"— the secret-scanhold is unconditional and must still fire once the AI hold no longer does.
test/unit/rules.test.ts: with{ aiReviewGateMode: "advisory", dryRun: true }thedisplayConclusionis"neutral"while the postedconclusionstays"success".test/unit/rules.test.tsnamed for this bug (e.g."REGRESSION: an inconclusive review does not hold an advisory-mode repo's otherwise-clean gate").packages/loopover-engine/test/gate-advisory-ai-review-low-confidence-disposition.test.ts(or a newfile at
packages/loopover-engine/test/gate-advisory-ai-inconclusive-hold.test.ts).All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example
changing the host copy without the engine twin, or without the
secret_scan_incompleteco-occurrence test —does not resolve this issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted.
vitest.config.ts'scoverage.includecovers
src/**/*.tsandpackages/loopover-engine/src/**/*.ts; both touched files are measured and gated.The change turns a one-condition
ifinto a two-condition one — every combination needs a test: findingpresent + block, finding present + advisory, finding present + off, finding absent + block. The
?? "advisory"default arm and the explicit-value arm of the mode resolution both need a case.Engine lines are credited by two uploads whose hits are unioned — add the test to
packages/loopover-engine/test/**as well as the roottest/**coverage, or the patch gate can still fail.Expected Outcome
A repo that never opted into blocking AI review stops having clean, green PRs diverted to manual review
because two passes raced for the same head lock or a model call failed transiently — while a repo that DID
opt in keeps the fail-closed hold exactly as it is today, and every repo still sees the inconclusive finding
in its panel.
Links & Resources
src/rules/advisory.ts:826-840— the unconditional holdsrc/rules/advisory.ts:63-65,:1316-1327—aiReviewGateMode's advisory-by-default contractsrc/rules/advisory.ts:1257-1271—isEvaluationBlocker's CLA precedent, the pattern to mirrorsrc/queue/ai-review-orchestration.ts:206-227,:255-275— the two mode-agnostic producerssrc/review/unified-comment-bridge.ts:87-99—neutral⇒manualpackages/loopover-engine/src/advisory/gate-advisory.ts:545-559— the engine twinscripts/check-engine-parity.ts:35-40,:199— the parity markers that pin the two together