fix(gates): give review-gate the same report rows the guard and the queue read

.github/scripts/enqueue-rules.mjs

@@ -3,7 +3,7 @@
 // read the same text: the guard that prints the command, and `merge-gate.mjs`, whose required check is
 // the only gate on a `merge_group` entry today (#960). A second copy of these rules would be two
 // answers to one question, which is #812's kill condition.
-import { parseReport } from './review-gate.mjs'
+import { parseReport, isConclusionSource } from './review-gate.mjs'
 
 const SHORT = 12
 // AGENTS.md → Merge gates rule 2: the `hold` label is paired with a comment saying
@@ -17,9 +17,10 @@ const SHORT = 12
 // #2091: a comment written on one line of the diff (`pulls/{pr}/comments`, `kind: 'review-comment'`) is a
 // request and never a conclusion. `stopRequestAt` reads all three faces, because a person's "do not
 // merge" is the same request in whichever box it is typed; the conclusion readers take the two faces a
-// Verifier block is written in, so adding the third input cannot add a way to record a PASS. One
-// predicate, used by every reader that parses a body for a conclusion.
-export const isConclusionSource = (r) => r?.kind !== 'review-comment'
+// Verifier block is written in, so adding the third input cannot add a way to record a PASS. The
+// predicate itself is `isConclusionSource` in `review-gate.mjs`, re-exported here so the guard and
+// `merge-gate.mjs` keep one import.
+export { isConclusionSource }
 
 const RECORD_ONLY = /^[\s>*+-]*(?:record only\s*[—–-]+\s*do not (?:merge|queue)(?:\s*[/／]\s*仅记录[、,，]?\s*禁止入队)?|仅记录[、,，]?\s*禁止入队)\s*[.。]?\s*$/im
 // AGENTS.md → Claim: a pull request says `Fixes #N` in its body, and one issue carries one pull request.

.github/scripts/review-gate.mjs

@@ -125,6 +125,14 @@ function parseCorrection(line) {
   return { by, tip: tip[1].toLowerCase() }
 }
 
+// #2091: a comment written on one line of the diff (`pulls/{pr}/comments`, `kind: 'review-comment'`) is a
+// request and never a conclusion. `stopRequestAt` reads all three faces, because a person's "do not merge"
+// is the same request in whichever box it is typed; a conclusion is written in one of the other two, so
+// adding the third input cannot add a way to record a PASS. One predicate, asked by every reader that
+// parses a body for a conclusion. It lives here because this module owns what a conclusion is;
+// `enqueue-rules.mjs` re-exports it, so its callers keep one import.
+export const isConclusionSource = (r) => r?.kind !== 'review-comment'
+
 // The last Verifier block in one body: a comment may hold an earlier review that a later
 // one replaced, and only the last block is the verdict the reviewer stands behind.
 export function parseReport(body) {
@@ -595,6 +603,29 @@ function arg(name) {
   return i > 0 ? process.argv[i + 1] : undefined
 }
 
+// #2092/#2120/#2870: the rows every gate reader parses, in one place. This file's copy had drifted from
+// `enqueue-guard.mjs:67` and `merge-gate.mjs:224`: it carried no `id` (the handle a decision prints, and
+// the one `conclusionAt` and `stopRequestAt` answer with), no review `state` (the whole of a
+// CHANGES_REQUESTED review with no body, which `stopRequestAt` reads as "asked for changes"), dropped a
+// review whose body is empty (#2119: "Request changes" is the one refusal a person writes without
+// typing), and never read the comments written on one line of the diff (#2091).
+export function reportsFrom({ comments = [], reviews = [], reviewComments = [] } = {}) {
+  return [
+    ...comments.map((c) => ({
+      id: c.id, kind: 'comment', login: c.user?.login, type: c.user?.type, body: c.body,
+      at: c.created_at, association: c.author_association, url: c.html_url,
+    })),
+    ...reviews.map((v) => ({
+      id: v.id, kind: 'review', state: v.state, login: v.user?.login, type: v.user?.type, body: v.body,
+      at: v.submitted_at, association: v.author_association, url: v.html_url,
+    })),
+    ...reviewComments.filter((c) => (c.body ?? '').trim()).map((c) => ({
+      id: c.id, kind: 'review-comment', login: c.user?.login, type: c.user?.type, body: c.body,
+      at: c.created_at, association: c.author_association, url: c.html_url,
+    })),
+  ]
+}
+
 function main() {
   const policyPath = arg('--policy') || process.env.REVIEW_POLICY || fileURLToPath(new URL('../agent/review-policy.json', import.meta.url))
   const repo = arg('--repo') || process.env.GH_REPO
@@ -607,12 +638,11 @@ function main() {
   const author = pull.user?.login
   if (!head) throw new Error(`pull request ${pr} has no head sha`)
 
-  const reports = [
-    ...list(`repos/${repo}/issues/${pr}/comments?per_page=100`).map((c) => ({ kind: 'comment', login: c.user?.login, type: c.user?.type, body: c.body, at: c.created_at, association: c.author_association, url: c.html_url })),
-    ...list(`repos/${repo}/pulls/${pr}/reviews?per_page=100`)
-      .filter((v) => (v.body ?? '').trim())
-      .map((v) => ({ kind: 'review', login: v.user?.login, type: v.user?.type, body: v.body, at: v.submitted_at, association: v.author_association, url: v.html_url })),
-  ]
+  const reports = reportsFrom({
+    comments: list(`repos/${repo}/issues/${pr}/comments?per_page=100`),
+    reviews: list(`repos/${repo}/pulls/${pr}/reviews?per_page=100`),
+    reviewComments: list(`repos/${repo}/pulls/${pr}/comments?per_page=100`),
+  })
 
   const policy = loadPolicy(policyPath)
   // #1317: the changed-file list, read here so a tip that moved only because `main` moved can be told
@@ -623,7 +653,10 @@ function main() {
   // changes only what its own run reads. `loadSigners` is fail-closed on its own; `decide` refuses when the
   // policy lists a roster and this is missing.
   const signers = loadSigners({ policy, path: arg('--signers') || process.env.VERDICT_SIGNERS || fileURLToPath(new URL('../agent/signers.json', import.meta.url)) })
-  const result = decide({ head, author, reports, policy, body: pull.body, admins: adminsFor({ gh, repo, reports, policy, author }), repo, pr, signers, gh, changedPaths })
+  // Only the two faces a Verifier block is written in are conclusions (#2091): the same filter
+  // `merge-gate.mjs:127` applies, so a comment on one line of the diff cannot pass here and be refused
+  // there. `adminsFor` still reads every row — a login is a login wherever it wrote.
+  const result = decide({ head, author, reports: reports.filter(isConclusionSource), policy, body: pull.body, admins: adminsFor({ gh, repo, reports, policy, author }), repo, pr, signers, gh, changedPaths })
   console.log(`review-gate (report only): PR #${pr} tip ${String(head).slice(0, SHORT)} — ${result.ok ? `PASS by ${result.by}` : result.reason}`)
   const text = render(result, { pr, head, author })
   if (process.env.GITHUB_STEP_SUMMARY) appendFileSync(process.env.GITHUB_STEP_SUMMARY, text + '\n')

.github/scripts/review-gate.test.mjs

@@ -2,7 +2,8 @@
 import { test } from 'node:test'
 import assert from 'node:assert/strict'
 import { readFileSync } from 'node:fs'
-import { adminsFor, decide, loadPolicy, parseReport, render } from './review-gate.mjs'
+import { adminsFor, decide, isConclusionSource, loadPolicy, parseReport, render, reportsFrom } from './review-gate.mjs'
+import { stopRequestAt } from './enqueue-rules.mjs'
 
 const HEAD = 'a4d506a1b2c3d4e5f60718293a4b5c6d7e8f9012'
 const OLD = '175328d0c1b2a3d4e5f60718293a4b5c6d7e8f90'
@@ -1008,3 +1009,68 @@ test('#1335: a correction listed before the PASS it withdraws still refuses —
   assert.equal(r.ok, false, 'the PASS it withdraws is listed after it, and the withdrawal still refuses')
   assert.match(r.reason, new RegExp(`the PASS at ${HEAD} was withdrawn by Reviewer`))
 })
+
+// #2870: this file's rows had drifted from `enqueue-guard.mjs:67` and `merge-gate.mjs:224`. A review that
+// asks for changes with no body is the whole of what a person types there ("Request changes" is a button),
+// and the queue gate answers it from the review's own `state`; without `state` (and without the row at all,
+// which an empty-body filter dropped) the report here could say PASS while the guard refused the same pull
+// request. These fixtures are GitHub API objects, the shape `reportsFrom` reads.
+const ghReview = (over = {}) => ({
+  id: 9001,
+  state: 'CHANGES_REQUESTED',
+  body: '',
+  submitted_at: '2026-11-11T10:00:00Z',
+  user: { login: 'reviewer-agent', type: 'User' },
+  author_association: 'MEMBER',
+  html_url: 'https://github.com/ThinkFlowLab/FlyBot/pull/6#pullrequestreview-9001',
+  ...over,
+})
+const ghComment = (over = {}) => ({
+  id: 7001,
+  body: 'do not merge this tip',
+  created_at: '2026-11-11T10:00:00Z',
+  user: { login: 'reviewer-agent', type: 'User' },
+  author_association: 'MEMBER',
+  html_url: 'https://github.com/ThinkFlowLab/FlyBot/pull/6#issuecomment-7001',
+  ...over,
+})
+
+test('#2870: a review with no body still arrives as a row, with its id and state', () => {
+  const rows = reportsFrom({ reviews: [ghReview()] })
+  assert.equal(rows.length, 1, 'an empty body must not drop the row (#2119)')
+  assert.equal(rows[0].id, 9001, 'the id is the handle a decision names')
+  assert.equal(rows[0].kind, 'review')
+  assert.equal(rows[0].state, 'CHANGES_REQUESTED', 'the state is what a request written with no sentence says')
+  assert.equal(rows[0].body, '')
+})
+
+test('#2870: the same rows answer the queue gate\'s stop request for a review that asked for changes', () => {
+  const rows = reportsFrom({ reviews: [ghReview()] })
+  const stop = stopRequestAt(rows, { author: 'opencode-agent' })
+  assert.equal(stop?.phrase, 'asked for changes')
+  assert.equal(stop?.because, 'state', 'it is read from the state, not from a body that is empty')
+  assert.equal(stop?.id, 9001)
+})
+
+test('#2870: an APPROVED review with no body keeps its row and changes no conclusion', () => {
+  const rows = reportsFrom({
+    reviews: [ghReview({ id: 9002, state: 'APPROVED' })],
+    comments: [ghComment({ id: 7002, body: report() })],
+  })
+  assert.equal(rows.length, 2, 'both rows are kept')
+  assert.equal(rows.filter(isConclusionSource).length, 2, 'a review is one of the two conclusion faces')
+  const r = decide({ head: HEAD, author: 'opencode-agent', reports: rows.filter(isConclusionSource), policy })
+  assert.equal(r.ok, true, r.reason)
+  assert.equal(r.by, 'reviewer-agent', 'the PASS in the comment still decides')
+  assert.equal(stopRequestAt(rows, { author: 'opencode-agent' }), null, 'an approval is not a request')
+})
+
+test('#2870: a comment on one line of the diff is a request, never a conclusion', () => {
+  const rows = reportsFrom({ reviewComments: [ghComment({ body: report() })] })
+  assert.equal(rows.length, 1)
+  assert.equal(rows[0].kind, 'review-comment')
+  assert.equal(rows.filter(isConclusionSource).length, 0, 'its Verifier block is not a conclusion (#2091)')
+  assert.equal(decide({ head: HEAD, author: 'opencode-agent', reports: rows.filter(isConclusionSource), policy }).ok, false, 'no conclusion names the tip')
+  const ask = reportsFrom({ reviewComments: [ghComment({ body: 'one more thing — do not merge this tip\n' })] })
+  assert.equal(stopRequestAt(ask, { author: 'opencode-agent' })?.phrase, 'do not merge')
+})