merge_pr.py refuses EVERY merge since 13:28 today — ruleset set required_approving_review_count to 0, so reviewDecision is now always null nf-zh26 ← Beads

closed priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/646 by tools/beads/import_github.py's reconcile pass.

⚠️ This blocks every session, not one PR. Two approved PRs are already stuck: #604 and #638. Every future merge hits the same wall.

Symptom

``` $ ./tools/pr/merge_pr.py 604 refuse: PR #604 reviewDecision=(none) ```

…on a PR @aedanpope has explicitly APPROVED (visible in latestReviews), ledger clean, --all green, MERGEABLE.

Cause — found, not guessed

The repository ruleset "pull requests required" (id 19101787) was updated today at 2026-08-06T13:28:13Z and now carries:

```json "required_approving_review_count": 0, "required_review_thread_resolution": true ```

GitHub only computes reviewDecision when a PR actually *requires* review. With the count at 0, reviewDecision is null for every PR in the repo — permanently, not transiently.

merge_pr.py:423 gates on exactly that field:

```python if decision != "APPROVED": raise MergeRefused(f"PR #{number} reviewDecision={decision or "(none)"}") ```

so it can no longer pass for anything.

The timing is conclusive:

| event | time | |---|---| | #615 merged | 13:20:31Z | | #600 merged | 13:26:15Z | | #626 merged | 13:26:21Z | | ruleset updated | 13:28:13Z | | #604 refused | 14:5xZ |

Three merges succeeded in the eight minutes before the change; nothing has merged since.

⚠️ Is the ruleset change intentional?

That is the first question and it is @aedanpope's to answer — do not "fix" this by reverting his repo config.

The code fix, either way: gate on the FACT, not the derived field

reviewDecision is a derived field whose existence depends on repo configuration. The fact is the review itself, which is present and unambiguous:

``` latestReviews: aedanpope APPROVED ```

So the gate should read latestReviews for a non-dismissed APPROVED from a human reviewer (excluding ninefold-reviewer, whose verdicts are advisory), and treat reviewDecision == "APPROVED" as a fast path when the repo does require reviews. That is the same "derive from the fact, not from a restatement of it" lesson #604 spent its whole review cycle on, applied one level up.

⚠️ Whatever the fix, keep the gate. There is deliberately no SKIP_APPROVAL env var next to SKIP_LEDGER / SKIP_PRESUBMIT / SKIP_PROBE_GATE, and the absence is load-bearing: "did a human approve this" is the one gate a session must not be able to talk itself past. Adding a bypass would be the wrong fix.

Second, smaller: required_review_thread_resolution: true is also new

merge_pr.py does not check unresolved review threads, so GitHub can now reject a merge the tool believes is fine. #604 happens to have 0 unresolved, so it is not the current blocker — but it is a second way for the tool and the server to disagree, and the tool would report the server's rejection rather than pre-empting it.

Not doing

I have not merged #604 around the gate, and have not touched the ruleset. Reporting rather than working around, because both the config and the safety gate are @aedanpope's call.

Filed unclaimed. Refs #604, #638, #470

Dependencies

None.

Comments

No comments.

Add a comment