gate: AI blocker silently collapses to a clean pass when its text trips the public-safe filter (false merge, live config)

[JSONbored]

Summary

In the single combine strategy — the strategy the live deployment uses — a reviewer that names a real blocker produces a clean pass whenever the blocker's text happens to contain an ordinary word the public-safe filter rejects (score, ranking, reward, cohort, payout, farming, reviewability). The PR auto-merges with the defect unreported.

Mechanism (verified at HEAD, 776c414)

synthesizeDefect drops the entire defect when the blocker title is not publishable:

src/services/ai-review.ts:2561-2577

const title = toPublicSafe(primary);
if (!title) return null; // unsafe title → drop the block entirely (fail-safe)

combineReviews, single branch — src/services/ai-review.ts:2598-2608:

const r = present[0];
if (!r) return { defect: null, split: false, inconclusive: true };
return {
  defect: r.blockers.length > 0 ? synthesizeDefect([r]) : null,
  split: false,
  inconclusive: false,
};

When synthesizeDefect returns null, the trio is {defect: null, split: false, inconclusive: false} — indistinguishable from "the reviewer found nothing". runAiReviewForAdvisory then pushes no finding at all (src/queue/ai-review-orchestration.ts:897-988), notes are composed from the surviving sentences, and the row is written durably cacheable (:1032-1042, no cacheable field ⇒ true) — so the wrong verdict is replayed on later passes.

The comment calls this "fail-safe". In the consensus branch it is; in single/synthesis it is fail-open.

Why the existing guards do not cover it

• The consensus branch is fail-closed for exactly this case: src/services/ai-review.ts:2642 sets split = !defect && realBlockers > 0, converting a dropped defect into a hold. single and synthesis have no equivalent.
• composeAdvisoryNotes passes allowBareScoreTerm (:2029-2035), but synthesizeDefect and consensusDefectOf call toPublicSafe with no options (:2242, :2573) — so even a repo on the score-terms allowlist gets strict filtering on the one string that carries the gate decision.

Why this is live, and why it got worse two commits ago

• AI_PROVIDER=claude-code,ollama ⇒ one reviewer slot, combine: "single" (src/selfhost/ai.ts:1596-1613). The live path is the vulnerable one.
• The rejection set is ordinary review vocabulary matched with .includes(), plus /\bscore\w*\b/i unless the repo is in LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS. In a codebase whose domain is scoring, ranking and rewards, blocker titles like "computeScore divides by zero when items is empty" or "the ranking comparator drops the tie-break" are routine.
• fix(review): filter the AI narrative assessment per sentence, not all-or-nothing #9445 (6269d27a9, two commits before this audit) widened reachability. Before per-sentence filtering, a filtered review often lost its whole narrative, leaving no public notes and routing the PR to an ai_review_inconclusive hold. Now the narrative survives, notes exist, and the pass completes cleanly with no defect. That commit's own data — the filter engaging on roughly 40% of reviews on this repo — is the base rate for how often blocker text carries these words.

Requirements

1. A blocker that exists but cannot be published must never resolve to "no defect".
2. The fix must apply to both single and synthesis (both call synthesizeDefect and both currently collapse); consensus already handles it and must not change behaviour.
3. The unpublishable case must be observable, not silent.

Deliverables

• In combineReviews, when blockers.length > 0 && synthesizeDefect(...) === null, return a hold rather than a pass — {defect: null, split: true, splitConfidence: <flagging reviewer's confidence>} (reusing the existing split→hold path) or inconclusive: true. Pick one and state the rationale in the code comment.
• Same treatment in the synthesis branch, both the either and both arms.
• A counter (e.g. loopover_ai_review_unpublishable_blocker_total) incremented at the drop site, so the rate is measurable.
• Correct the synthesizeDefect doc comment: it is fail-safe only for consensus.
• Consider whether the gate-bearing title should be filtered with allowBareScoreTerm for allowlisted repos, matching composeAdvisoryNotes. If not, document why the two differ.

Tests (must fail against current main)

• single + one reviewer + one blocker whose title contains score (repo NOT allowlisted) ⇒ result holds; asserts defect === null && (split || inconclusive).
• Same for each of reward, ranking, cohort, farming, payout.
• synthesis/either and synthesis/both equivalents.
• consensus unchanged (regression guard).
• A publishable blocker still produces a defect in all three strategies.
• The cache row written for a dropped-blocker review is not durably cacheable, or is keyed so the hold is reproduced.

Expected outcome

A reviewer-named blocker always produces either a published defect or an explicit hold. It can never produce a silent pass. The rate of unpublishable blockers becomes visible in metrics rather than invisible in merges.

Verification

Reproduce on a scratch PR by configuring a repo off the score-terms allowlist and inducing a blocker whose title contains "score"; confirm the PR holds instead of merging.

[linear-code]

JSO-6524

[JSONbored]

Correction while implementing the fix: the example "computeScore divides by zero when items is empty" in the description would not actually be filtered.

The bare-score term uses BARE_SCORE_TERM_PATTERN = /\bscore\w*\b/i (src/queue-intelligence.ts), so a word boundary is required — computeScore has none before Score and passes through. The comment there also notes "scoring" is not matched either (it drops the trailing e, so it is not a literal score substring).

What genuinely trips it, verified by test:

• plain .includes() substrings from FORBIDDEN_PUBLIC_COMMENT_WORDS: reward, rewards, payout, farming, ranking, rankings, cohort, reviewability, and the trust/wallet terms.
• bare score with a word boundary: score, scores, scored, scorer.

So realistic blocker titles that hit this are e.g. "score is NaN when the input list is empty", "the ranking comparator drops the tie-break", "cohort assignment leaks across tenants", "reward calculation overflows on a large diff" — all now pinned as regression tests.

The severity is unchanged: these are ordinary blocker phrasings in this codebase. One detail is worth adding, because it makes the issue slightly worse than described — synthesizeDefect calls toPublicSafe with no options, so the gate-bearing title is filtered even on a repo in LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS (unlike composeAdvisoryNotes, which passes allowBareScoreTerm). All three repos are on that allowlist, and it does not rescue this path.

The camelCase non-match is now pinned by its own test so any future widening of the pattern is a deliberate, visible decision.