fix(agents): pr-approver gates AUTO_MERGE on review:pass / human-approved #74

Merged

navigator merged 1 commit from fix/pr-approver-gate-on-review into main

2026-05-25 19:47:29 -03:00

navigator commented

2026-05-25 19:46:18 -03:00

Owner

Summary

Follow-up to #70. After pr-reviewer started running, pr-approver was still happy to auto-merge any PR with ready-for-review + green CI on its 60s loop — racing against the reviewer's 120s loop. So a PR could land before any semantic review had been recorded.

This adds one triage row (Row 4b) immediately after the mergeability check:

if labels has 'ready-for-review' and not (review:pass or human-approved):
    return SKIP_AWAITING_REVIEW

review:pass — the new positive verdict from pr-reviewer (#70).
review:findings — a blocking hold; never auto-merge a flagged PR.
human-approved — keeps the governance-path bypass for PRs touching off-limits paths (PLAN.md, Accepted ADRs).

SKIP_AWAITING_REVIEW dispatch is a silent noop (no label churn, no Telegram). pr-reviewer drives the state forward.

Test plan

bash -n clean.
After merge, restart pr-approver. Verify #69 (already labelled review:pass) auto-merges. Verify that a fresh PR without review labels stays open (and pr-reviewer catches it within ~2 min, then approver merges).

Notes

Off-limits per ADR-017 — manually merged after CI green.

## Summary Follow-up to #70. After `pr-reviewer` started running, `pr-approver` was still happy to auto-merge any PR with `ready-for-review` + green CI on its 60s loop — racing against the reviewer's 120s loop. So a PR could land before any semantic review had been recorded. This adds one triage row (Row 4b) immediately after the mergeability check: ``` if labels has 'ready-for-review' and not (review:pass or human-approved): return SKIP_AWAITING_REVIEW ``` - `review:pass` — the new positive verdict from `pr-reviewer` (#70). - `review:findings` — a blocking hold; never auto-merge a flagged PR. - `human-approved` — keeps the governance-path bypass for PRs touching off-limits paths (PLAN.md, Accepted ADRs). `SKIP_AWAITING_REVIEW` dispatch is a silent noop (no label churn, no Telegram). `pr-reviewer` drives the state forward. ## Test plan - [x] `bash -n` clean. - [ ] After merge, restart `pr-approver`. Verify `#69` (already labelled `review:pass`) auto-merges. Verify that a fresh PR without review labels stays open (and pr-reviewer catches it within ~2 min, then approver merges). ## Notes Off-limits per ADR-017 — manually merged after CI green.

navigator added 1 commit

2026-05-25 19:46:18 -03:00

fix(agents): pr-approver gates AUTO_MERGE on review:pass / human-approved

build / scalafmt-check (pull_request) Successful in 3s

Details

build / sbt-compile (pull_request) Successful in 3s

Details

build / shell-lint (pull_request) Successful in 18s

Details

3c321abc00

After #70 added the pr-reviewer role, pr-approver still merged any PR
with `ready-for-review` + green CI, racing against the semantic review
(reviewer poll 120s vs approver poll 60s). This adds a single triage
row immediately after the mergeability check:

  if labels has ready-for-review and not (review:pass or human-approved):
      return SKIP_AWAITING_REVIEW

- `review:pass` is the new positive verdict from pr-reviewer.
- `review:findings` is a blocking hold — never auto-merge a PR the
  reviewer flagged. The negation in the guard catches this implicitly.
- `human-approved` keeps the governance-path bypass available for
  PRs touching off-limits paths (PLAN.md, accepted ADRs, etc.) where
  a human signs off after seeing the PR.

The new SKIP_AWAITING_REVIEW dispatch is a silent noop (no labels,
no Telegram, no churn). pr-reviewer drives the state forward.