A review asking for changes stops nothing: the review state is dropped and an empty body matches no stop #2119

Written-by: Chief
At: 2026-11-02T06:03Z

A person can reject a pull request by pressing Request changes and typing nothing. Neither GitHub nor our queue gates read that: the review's state is dropped on the way in, and the stop reader looks only at text, so the pull request merges with the gate answering ALLOW.

What is dropped

.github/scripts/enqueue-guard.mjs:69-71 collects reviews as

.map((v) => ({ id: v.id, kind: 'review', login: v.user?.login, type: v.user?.type, body: v.body, at: v.submitted_at, association: v.author_association }))

v.state — APPROVED / CHANGES_REQUESTED / COMMENTED — is never carried. .github/scripts/enqueue-rules.mjs:188-212 (stopRequestAt) then decides from STOP_REQUEST / HOLD_LINE matching the text. An empty body matches nothing, so stop stays null and decide() returns ok: true. Same for merge-gate.mjs:219-221, which reads the same objects.

#2073, #2084 and #2091 taught the same reader to look at issue comments, review bodies and review-comment lines — all three are text. The review state is a fourth face, and it is the one a person uses without writing a word.

Why GitHub does not stop it either

Ruleset 25173372 ("main merge queue", branch main) carries exactly two rules (read 06:02Z): merge_queue and required_status_checks. There is no pull_request rule and no required-review rule, so GitHub's own "Changes requested" merge block does not apply here; enqueuePullRequest accepts the head and the queue merges it.

Expected

A review that requests changes is a stop request, whatever it says:

• carry state on the review object in enqueue-guard.mjs and merge-gate.mjs;
• in stopRequestAt, a review with state === 'CHANGES_REQUESTED' counts as a stop naming that review id (phrase: the state, so the refusal reads review 1234 by <login> asked for changes), with or without a body;
• the existing clearing rules stay: a newer APPROVED review, a dismissal, or a newer comment that lets the tip merge clears it — and, as today, an older stop than the conclusion the decision used does not apply.

Acceptance

1. Red first in .github/scripts/enqueue-guard.test.mjs: a review { kind: 'review', state: 'CHANGES_REQUESTED', body: '' } submitted after the PASS at the tip makes decide() return ok: false naming that review id; the same review with state: 'COMMENTED' and an empty body leaves the decision unchanged.
2. The counterpart in merge-gate is refused for the same head.
3. A later APPROVED review by the same login lets the tip merge, and the test says which review cleared it.
4. No new required check and no change to policy/gates.json: this edits .github/scripts/enqueue-guard.mjs, .github/scripts/enqueue-rules.mjs and .github/scripts/merge-gate.mjs, which are gate files — the change needs its own Policy-sign: line naming a comment that names the tip (see #2093/#2095).

Scope note

Text-based stops (#2073, #2084, #2091) are working and tested; this issue is only about the state that carries no text. #2084 recorded it as out of scope at the time.