Merge gate: refuse autonomous merge on a current-head requested change (#304)

[polymath-orchestrator]

Refs #304 — closes its last acceptance criterion ("a blocking review actually blocks") on the path our PRs actually take. Stacked conceptually on #308 but independent of it: this touches only ops/, so it can land in either order.

Why here and not in branch protection

mainprotect carries no pull_request rule, so GitHub does not enforce CHANGES_REQUESTED as a merge blocker at all — a requested change is visible and the merge queue merges straight over it. That is the gap #308 could not close on its own.

Fixing it by adding a branch-protection rule would be a GitHub-side config change: blast-radius-wide, it governs every human merge too, and it is not reviewable or revertible in a PR. Our PRs land through this gate, not the GitHub UI — so the teeth belong here, where the rule is reviewable, testable, and revertible.

The rule

check --mode autonomous now also refuses when an independent reviewer has an outstanding CHANGES_REQUESTED on the current head.

Two rules keep the gate clearable, which matters as much as making it strict:

SHA-pinned	A requested change against an older commit says nothing about this one. Counting it would leave a PR blocked forever after it was fixed and re-reviewed — and an unclearable gate is one workers learn to route around, which is the exact failure #304 exists to end.
Latest verdict per reviewer wins	A reviewer who requests changes and then approves the same head has cleared it. COMMENTED and DISMISSED carry no verdict: they neither block nor clear.

The PR author's own reviews never count — author == reviewer is the collapse this whole issue is about. If the author cannot be identified, the gate fails closed.

The PR is derived from the SHA, not passed as a flag

Deliberate. An optional --pr would let a caller silently skip the new check — a bypass, in a gate that exists precisely because a gate you can route around is not a gate. A required --pr would break every existing autonomous caller. Deriving it from the head SHA needs no caller change and leaves no hole; an unresolvable PR fails closed.

Testing

Unit: 9 new tests covering both directions, because a gate that only fails closed is proven strict, not proven correct — blocks on a current-head requested change; does not block on a stale-SHA one, the author's own, a later approval of the same head, or COMMENTED/DISMISSED.

Also verified against the live GitHub API on #308's real review data: a current-head CHANGES_REQUESTED from a non-author blocks, while the same review on a stale SHA and the author's own both do not. Full ops suite green (364), format:check green.

Note

This is a merge-gate change, so it only ever makes the gate stricter. It still requires an independent review and a human merge like anything else — it grants no new authority.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RP5jQC1fNo5ak4oyogD3ay

---

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

[polymath-orchestrator]

final-review — verdict: clean · PARKED for human merge

Head: 74e5081d8bb1cfcda75974963fed927667292b61 · Reviewer: Codex (different family from the implementer) · Cycles: 3 · CI: 11/11 green · Unresolved threads: 0 · Tests: 42 in this suite, full ops suite 375

Reviewer's verdict: "READY — no CRITICAL or HIGH issue remains that makes merging unsafe. The committed code was verified directly at the current PR head."

⛔ Parked deliberately, though policy would allow an autonomous merge

This PR touches no sensitive path (ops/ only), so the sensitive-path rule does not park it. I am parking it anyway, and the reason is the point of the change: this PR modifies the merge gate itself, and it was authorized by a relayed instruction rather than by Nick directly. A change to the gate should not be merged by the automation that gate governs. merge-park set with reason=human-required; @nhod's call.

What the review caught — and why it matters here specifically

Cycle 2 found that my commit message claimed three fixes the committed code did not contain. It was right: a patch to fetchHeadPullRequestReviews had silently failed to apply (prettier had reflowed the target), so the file destructured ambiguous from a function that never returned it, still took the first matching PR of any state, and still paginated without --slurp.

My own live verification passed anyway — because #308 happens to have exactly one open PR. That is exactly the hole an independent reviewer reading the committed diff closes, and exactly why "it worked when I ran it" is not verification. It is also a rather pointed demonstration of the lesson in this PR's own subject: the gate must consume an unambiguous, SHA-pinned signal, never an inferred one.

Five fail-open holes closed (all found by review)

Each one is the same shape as the bug the gate exists to fix — an ambiguity treated as "fine":

HIGH	commits/{sha}/pulls returns merged and closed PRs too, so taking the first exact-head match could select a stale PR and mask a CHANGES_REQUESTED on the one actually entering the merge queue. Now: open-only, exactly-one, ambiguity fails closed.
	A non-array reviews body meant "no reviews" and certified the merge. It means "we do not know" → fails closed. (My original test had locked the unsafe behavior in.)
	An unknown PR author passed when the review list was empty — but without the author we cannot tell a self-review from an independent one, and author == reviewer is the exact collapse this prevents.
	A current-head verdict whose author could not be read silently vanished.
	Ordering used submitted_at, so a malformed timestamp parsed to 0 and a later requested change could lose to an earlier approval — silently unblocking the merge. Now uses documented response order with the review id as tie-breaker.

The caller's logic is now extracted into pure tested seams (selectHeadPullRequest, flattenReviewPages) precisely because the bug lived in the one part of the gate that had no tests while the evaluator's own tests were green.

Both directions tested, per the constraint

A gate that only fails closed is proven strict, not proven correct. Verified in unit tests and against the live GitHub API on #308's real review data:

• current-head CHANGES_REQUESTED from a non-author → blocks
• same review against a stale SHA → does not block
• the author's own → does not block
• a later approval of the same head → clears it
• COMMENTED / DISMISSED → neither block nor clear