Skip to content

fix(review): decide whether AI-judgment blockers should gate the verdict, not just annotate it #2592

Description

@JSONbored

Parent: #1936

Problem

deriveUnifiedStatus (src/review/unified-comment.ts) sets status = "ready" whenever input.decision === "merge", regardless of input.blockers — by explicit design (see the code comment: "a gate merge verdict WITH advisory blockers stays authoritative-ready"), confirmed by a dedicated test (deriveUnifiedStatus({decision: "merge", blockers: ["minor"]})"ready").

This is because the gate's decision/verdict (from evaluateGateCheckCore, src/rules/advisory.ts) only reaches conclusion: "success" when its own narrow, deterministic blockers array (secret leak, missing linked issue, slop, manifest violations) is empty. The renderer's separate input.blockers (shown in the PR comment's "Blockers" section) comes from a broader channel — the AI reviewer's free-text notes / consensusDefectFromFindings — which can surface a judgment concern (e.g. a dependency-health misclassification) that never became a gate-recognized hard blocker.

So a PR can legitimately show decision: merge (0 gate-recognized blockers) while the rendered "Blockers" section lists an AI-flagged concern, and the PR still merges — the exact scenario originally reported in #2002.

Requirements

This is a product decision, not a bugfix — the current behavior is intentional. Decide one of:

  1. Tighten evaluateGateCheckCore's hard-blocker classification so a sufficiently-confident AI-judgment finding gets promoted into the gate's own blockers array (making it actually block merge, not just annotate the comment).
  2. Add a renderer-level veto: deriveUnifiedStatus downgrades mergeheld/hold when input.blockers is nonempty, even if the gate's own decision was merge — mirroring how the preflight-hold fix (fix(review): hold PRs whose review cannot complete, never "safe to merge" #2008) already downgrades ready on ctx.preflightHeld.
  3. Confirm current behavior is correct as designed and close this with a rationale documented for future readers (the existing code comment explains WHY, but not a considered maintainer decision that this tradeoff is acceptable long-term).

Acceptance criteria

  • A maintainer decision is recorded (either a merged PR implementing 1 or 2, or an explicit close-as-intended with rationale).
  • If implementing 1 or 2: a regression test proving a nonempty AI-judgment blockers list changes the merge disposition, without breaking the existing "gate merge with advisory blockers stays ready" test that encodes the CURRENT intentional behavior (that test would need to be updated to reflect the new decision, not silently left contradicting it).

Notes

Supersedes #2002, which conflated this open design question with the already-fixed preflight-hold rendering bug (#2008, merged). Investigated 2026-07-02; see #2002 for the original investigation trail.

Metadata

Metadata

Assignees

No one assigned

    Labels

    maintainer-onlyOwner-only work — yields no Gittensor points.roadmapOn the Wave-2 agent-layer roadmap board (project 9)

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions