|
| 1 | +# CI Enforcement: Block Merge When Specialist Reviewers Are Outstanding |
| 2 | + |
| 3 | +## Problem |
| 4 | + |
| 5 | +The "all reviewers must complete before merge" requirement (`.claude/rules/25-review-merge-gate.md`) is purely behavioral guidance with no CI enforcement. Agents under context compaction lose track of outstanding reviewers and self-merge. This caused PR #568 (5 CRITICAL findings shipped) and recurred with PR #600. |
| 6 | + |
| 7 | +## Research Findings |
| 8 | + |
| 9 | +### Existing Pattern: `check-preflight-evidence.ts` |
| 10 | +- Located at `scripts/quality/check-preflight-evidence.ts` |
| 11 | +- Reads PR body from `GITHUB_EVENT_PATH` JSON payload |
| 12 | +- Parses sections using HTML comment markers and regex |
| 13 | +- Exits with code 1 on failure, 0 on success |
| 14 | +- CI job: `preflight-evidence` in `.github/workflows/ci.yml`, runs only on `pull_request` events |
| 15 | +- Package script: `quality:preflight` in root `package.json` |
| 16 | + |
| 17 | +### PR Template Structure |
| 18 | +- The "Specialist Review Evidence" section is in `.github/pull_request_template.md` |
| 19 | +- Contains a markdown table: `| Reviewer | Status | Outcome |` |
| 20 | +- Status values: `PASS`, `ADDRESSED`, `DISPATCHED`, `FAILED`, `DEFERRED` |
| 21 | +- Has two checkboxes about reviewer completion |
| 22 | +- Has HTML comment markers for guidance but NO start/end block markers like preflight has |
| 23 | + |
| 24 | +### CI Configuration |
| 25 | +- `.github/workflows/ci.yml` runs on `push` to main and `pull_request` to main |
| 26 | +- Preflight evidence job uses: checkout, pnpm setup, node 22, install, then `pnpm quality:preflight` |
| 27 | +- The new job should follow the same pattern |
| 28 | + |
| 29 | +### Agent-Authored Detection |
| 30 | +- Agent PRs include `Co-Authored-By: Claude` in commit messages |
| 31 | +- The check needs to determine if commits are agent-authored to decide whether to require the table |
| 32 | +- Can check the PR body or commit messages from the event payload |
| 33 | + |
| 34 | +## Implementation Checklist |
| 35 | + |
| 36 | +- [ ] Create `scripts/quality/check-specialist-review-evidence.ts` following `check-preflight-evidence.ts` pattern |
| 37 | + - [ ] Parse PR body from `GITHUB_EVENT_PATH` |
| 38 | + - [ ] Find the "Specialist Review Evidence" section |
| 39 | + - [ ] Parse the reviewer table rows |
| 40 | + - [ ] Fail if any row has `DISPATCHED` or `FAILED` status |
| 41 | + - [ ] Check for `needs-human-review` label via event payload labels |
| 42 | + - [ ] Check if PR is agent-authored (Co-Authored-By: Claude in body or detect from commit messages) |
| 43 | + - [ ] Fail if agent-authored and table is missing/empty |
| 44 | + - [ ] Pass for human-authored PRs without the table |
| 45 | + - [ ] Pass when all reviewers show `PASS` or `ADDRESSED` |
| 46 | +- [ ] Add `quality:specialist-review` script to root `package.json` |
| 47 | +- [ ] Add `specialist-review-evidence` CI job in `.github/workflows/ci.yml` |
| 48 | +- [ ] Write tests covering all parsing edge cases |
| 49 | + - [ ] All PASS/ADDRESSED -> pass |
| 50 | + - [ ] DISPATCHED status -> fail |
| 51 | + - [ ] FAILED status -> fail |
| 52 | + - [ ] Missing table, agent-authored -> fail |
| 53 | + - [ ] Missing table, human-authored -> pass |
| 54 | + - [ ] Empty table (only header) -> fail for agent PRs |
| 55 | + - [ ] Mixed statuses (some PASS, one DISPATCHED) -> fail |
| 56 | + - [ ] Malformed table -> fail gracefully |
| 57 | + - [ ] `needs-human-review` label -> fail |
| 58 | + - [ ] DEFERRED status -> pass (with warning) |
| 59 | + - [ ] N/A: human-authored PR -> pass |
| 60 | + |
| 61 | +## Acceptance Criteria |
| 62 | + |
| 63 | +- [ ] CI fails if any reviewer in the evidence table shows `DISPATCHED` or `FAILED` |
| 64 | +- [ ] CI fails if `needs-human-review` label is present on the PR |
| 65 | +- [ ] CI fails if evidence table is missing/empty on agent-authored PRs |
| 66 | +- [ ] CI passes for human-authored PRs without the table (don't block human PRs) |
| 67 | +- [ ] CI passes when all reviewers show `PASS` or `ADDRESSED` |
| 68 | +- [ ] Tests cover all parsing edge cases (malformed table, missing table, mixed statuses) |
| 69 | + |
| 70 | +## References |
| 71 | + |
| 72 | +- `.claude/rules/25-review-merge-gate.md` |
| 73 | +- `.claude/rules/14-do-workflow-persistence.md` |
| 74 | +- `.github/pull_request_template.md` |
| 75 | +- `scripts/quality/check-preflight-evidence.ts` |
| 76 | +- `.github/workflows/ci.yml` |
| 77 | +- `docs/notes/2026-03-31-pr568-premature-merge-postmortem.md` |
0 commit comments