fix(review): dont request the human on a shadow-gated APPROVE (auto-merge takes it) #407

Merged

joshtronic merged 1 commit from fix/review-suppress-human-request-on-default into master

2026-07-20 22:47:26 +00:00

igor commented

2026-07-20 22:34:23 +00:00

Collaborator

Follow-up completing the review-gate rollout (#405/#406 merged).

The gap (surfaced by your "does auto-merge bypass an assigned review" question)

#405 made the shadow APPROVE the merge gate on default repos, but do_review_tick still called review_request_human on every APPROVE — so you got a review-request assignment for PRs that then auto-merge a tick later. Noise, and it partly defeats the "out of the loop" goal. Auto-merge does bypass the assignment (it merges on the shadow APPROVE) — you were just being pestered on the way.

Fix

do_review_tick requests you only when the auto-merge won't take it: new automerge_will_take (default repo + APPROVE) short-circuits the request. A COMMENT (won't auto-merge) or a require_human carve-out still routes to you — your review is the gate there.

Tests

automerge_will_take covered in test-automerge.sh: default+APPROVE → take; COMMENT / REQUEST_CHANGES → not; carve-out+APPROVE → human gate. make test green.

Net: on default repos you're neither blocking nor bothered — the PR auto-merges and shows up in the next ship-report. On carve-outs you're still asked.

Follow-up completing the review-gate rollout (#405/#406 merged). ## The gap (surfaced by your "does auto-merge bypass an assigned review" question) #405 made the shadow `APPROVE` the merge gate on default repos, but `do_review_tick` still called `review_request_human` on **every** `APPROVE` — so you got a review-request assignment for PRs that then auto-merge a tick later. Noise, and it partly defeats the "out of the loop" goal. Auto-merge *does* bypass the assignment (it merges on the shadow APPROVE) — you were just being pestered on the way. ## Fix `do_review_tick` requests you only when the auto-merge **won't** take it: new `automerge_will_take` (default repo + `APPROVE`) short-circuits the request. A `COMMENT` (won't auto-merge) or a `require_human` carve-out still routes to you — your review is the gate there. ## Tests `automerge_will_take` covered in `test-automerge.sh`: default+APPROVE → take; COMMENT / REQUEST_CHANGES → not; carve-out+APPROVE → human gate. `make test` green. Net: on default repos you're neither blocking nor bothered — the PR auto-merges and shows up in the next ship-report. On carve-outs you're still asked.

igor added 1 commit

2026-07-20 22:34:23 +00:00

fix(review): dont request the human on a shadow-gated APPROVE (auto-merge takes it)

Lint / check-sync (pull_request) Successful in 5s

Details

Lint / check-sync (push) Successful in 4s

Details

d929cdcb1e

Completes the review-gate rollout. #405 made the shadow APPROVE the merge gate
on default repos, but do_review_tick still requested FORGEJO_REVIEWER on every
APPROVE -- so the human got a review request for PRs that then auto-merge a tick
later. Noise, and it partly defeats "get the human out of the loop."

do_review_tick now skips the request when the auto-merge will take the PR anyway
(new automerge_will_take: default repo + APPROVE). A COMMENT (wont auto-merge) or
a require_human carve-out still routes to the human; awareness of what
auto-merged comes via the daily ship-report. Tested in test-automerge.sh.

joshtronic approved these changes

2026-07-20 22:35:22 +00:00

Dismissed

igor commented

2026-07-20 22:46:37 +00:00

Author

Collaborator

🤖 Review — `APPROVE` (automated)

CI for d929cdcb: success

Focused follow-up that suppresses the redundant human review-request when a default shadow-gated repo will auto-merge an APPROVE anyway. Correct, in scope (~30 lines), tested, CI green.

lib/automerge.sh: automerge_will_take is a clean, correct predicate — returns 0 only for APPROVE on a repo without the require_human carve-out. Comment accurately scopes it to the approval signal (CI/mergeable still gate the real merge).
bin/tick.sh: logic is right across all branches. APPROVE on default repo → log, no request; COMMENT → will_take returns 1 (verdict≠APPROVE) → human asked; carve-out APPROVE → will_take returns 1 → human asked. The log line only fires in the will_take-true branch, which requires APPROVE, so the "shadow APPROVE" wording is never mislabeled for a COMMENT.
bin/test-automerge.sh: covers all four relevant cases (default+APPROVE take, default+COMMENT/REQUEST_CHANGES not, carve-out+APPROVE not).

No security, scope, or correctness concerns. Checklist matches the diff.

_{Independent review by the harness on claude-opus-4-8 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `APPROVE` _(automated)_ CI for `d929cdcb`: **success** Focused follow-up that suppresses the redundant human review-request when a default shadow-gated repo will auto-merge an `APPROVE` anyway. Correct, in scope (~30 lines), tested, CI green. - `lib/automerge.sh`: `automerge_will_take` is a clean, correct predicate — returns 0 only for `APPROVE` on a repo without the `require_human` carve-out. Comment accurately scopes it to the approval signal (CI/mergeable still gate the real merge). - `bin/tick.sh`: logic is right across all branches. `APPROVE` on default repo → log, no request; `COMMENT` → `will_take` returns 1 (verdict≠APPROVE) → human asked; carve-out `APPROVE` → `will_take` returns 1 → human asked. The `log` line only fires in the `will_take`-true branch, which requires `APPROVE`, so the "shadow APPROVE" wording is never mislabeled for a `COMMENT`. - `bin/test-automerge.sh`: covers all four relevant cases (default+APPROVE take, default+COMMENT/REQUEST_CHANGES not, carve-out+APPROVE not). No security, scope, or correctness concerns. Checklist matches the diff. --- <sub>Independent review by the harness on `claude-opus-4-8` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-07-20 22:46:37 +00:00

16 seconds

igor requested review from joshtronic

2026-07-20 22:46:37 +00:00

joshtronic approved these changes

2026-07-20 22:47:22 +00:00