feat(scripts): refuse to enqueue a held or unpassed pull request (#602)

.claude/skills/fix-issue/SKILL.md

@@ -58,7 +58,7 @@ node scripts/ds-bot-patches.mjs generate && node scripts/ds-bot-patches.mjs chec
 ## 6. Merge and cleanup
 
 - Merge only when all hold **on the same tip**, by tier (owner decisions 2026-10-19): **T0/T1:** required checks green, mergeable, design-conformant; no PASS needed. **T2/T3:** required checks green and a PASS at the tip (self-PASS counts: the owner's reviewer agent may PASS a PR opened as `Birfy`); the agent that gated it then enqueues it itself, see AGENTS.md owner decision 2026-10-21. Tip moved (any new commit) → the PASS is void, start over from the review.
-- **Merge.** An agent may enqueue the PR itself once `tests-ok` is green at the tip and the branch is mergeable: always for **T0/T1**, and for **T2/T3** when a PASS sits at that same tip (owner decision 2026-10-21: the gating Bot merges its own PR; no click). `gh pr merge --auto --squash --match-head-commit <sha>`. Never admin-merge, never bypass a gate.
+- **Merge.** An agent may enqueue the PR itself once `tests-ok` is green at the tip and the branch is mergeable: always for **T0/T1**, and for **T2/T3** when a PASS sits at that same tip (owner decision 2026-10-21: the gating Bot merges its own PR; no click). **First run `node .github/scripts/enqueue-guard.mjs --pr <n>`** and only then `gh pr merge --auto --squash --match-head-commit <sha>`: the guard re-reads the live head and exits 1 on a `hold`, on a head whose newest conclusion is not a PASS, and on a report marked "record only — do not merge" (#602). Never admin-merge, never bypass a gate.
 - **Merge queue.** `main` requires one. The command above enqueues the PR; gh may print `The merge strategy for main is set by the merge queue` or `already queued`, which is success, not a refusal. The queue runs `Test` on a `gh-readonly-queue/main/pr-<n>-<sha>` entry and squash-merges it; a plain `gh pr merge --squash` is refused. Do not retry; check `isInMergeQueue` (GraphQL) or wait for the PR to read MERGED.
 - Then: fetch and fast-forward main, delete the branch locally and remotely, remove the worktree, and carry follow-up issues (not TODOs).
 

.github/scripts/enqueue-guard.mjs

@@ -0,0 +1,124 @@
+// The enqueue guard (#602; the rules are AGENTS.md → Merge gates).
+//
+//   node .github/scripts/enqueue-guard.mjs --pr 566 [--repo owner/repo] [--sha <head>]
+//
+// Nothing under `.github/scripts/` enqueues anything today: the queue call is an instruction to the
+// gating agent (AGENTS.md step 6, `.claude/skills/fix-issue` → Merge, `docs/autonomous-dev.md` → How
+// to merge): `gh pr merge <n> --auto --squash --match-head-commit <sha>`. That gap is #602: #566 was
+// enqueued 10 s after its PASS while the standing instruction was "parked — do not merge", and the
+// only check for `hold` ran on the queue entry, after it was already in the queue (merge-gate.yml;
+// #639 tracks making that check required).
+//
+// So this is the step immediately before that call. It refuses — exit 1, the reason printed — when
+// the pull request carries `hold`, when the newest conclusion at its head is not a PASS, or when that
+// conclusion is itself marked "record only — do not merge". It reuses `judge({ mode: 'queue' })`, so
+// "may this enter the queue" keeps one definition: a rule added to the merge gate is enforced here
+// without being restated, unless someone deliberately changes both.
+//
+// It never enqueues and never merges: on success it prints the exact command and exits 0, and the
+// agent that gated the pull request runs it. `hold` is a label and a label carries no signature
+// (#627), so this guard is not a security boundary — it is the routine's own check, kept in the
+// routine's path rather than left to a check that reports after the queue is reached.
+import { execFileSync } from 'node:child_process'
+import { fileURLToPath } from 'node:url'
+import { judge } from './merge-gate.mjs'
+import { loadPolicy, parseReport } from './review-gate.mjs'
+
+const SHORT = 12
+// AGENTS.md → Merge gates rule 2: the `hold` label is paired with a comment saying
+// "Record only — do not merge / 仅记录、禁止入队". The label can be removed or forgotten; the record
+// must still keep the pull request out of the queue.
+//
+// The marker is a whole line, not a phrase somewhere in the text: a PASS that explains the rule — the
+// first real one wrote `- \`RECORD_ONLY\` matches … a record written only in Chinese would not keep the
+// pull request out` — must not freeze itself (#687). A bullet or a quote line is allowed, but nothing
+// may follow the marker on it except its other language half.
+const RECORD_ONLY = /^[\s>*+-]*(?:record only\s*[—–-]+\s*do not (?:merge|queue)(?:\s*[/／]\s*仅记录[、,，]?\s*禁止入队)?|仅记录[、,，]?\s*禁止入队)\s*[.。]?\s*$/im
+const short = (s) => String(s ?? '').slice(0, SHORT)
+
+// The newest conclusion that names this head, if it carries the record-only marker. Only a body that
+// parses as a conclusion counts, so ordinary prose quoting the phrase is no marker; only the newest
+// one counts, so a fresh PASS at the same head supersedes an older parked report.
+export function recordOnlyAt(reports, head) {
+  const target = String(head ?? '').toLowerCase()
+  if (!target) return null
+  const at = (reports ?? [])
+    .map((r, i) => ({ r, i, parsed: parseReport(r.body) }))
+    .filter(({ parsed }) => parsed.verdict && parsed.tip && target.startsWith(parsed.tip))
+    .map((x) => ({ ...x, t: Date.parse(x.r.at ?? 0) || 0 }))
+    .sort((a, b) => a.t - b.t || a.i - b.i)
+  const last = at.at(-1)
+  return last && RECORD_ONLY.test(String(last.r.body ?? '')) ? last.r : null
+}
+
+// The decision, without the network: `result` is merge-gate's `judge` output for this head.
+export function guard({ pr, head, labels, result, reports }) {
+  const quiet = recordOnlyAt(reports, result.head ?? head)
+  const reasons = [...result.reasons]
+  if (quiet) reasons.push('the newest conclusion at this head says "record only — do not merge"')
+  const ok = result.ok && !quiet
+  return {
+    ...result,
+    ok,
+    reasons,
+    recordOnly: Boolean(quiet),
+    labels: labels ?? [],
+    // One loud sentence: a refusal says what to do ("do not enqueue") and why, so it cannot be read
+    // as a warning that some other step may override.
+    message: ok
+      ? `enqueue #${pr}: no \`hold\` and the gate passes`
+      : `do not enqueue #${pr}: ${reasons.join('; ')}`,
+    command: `gh pr merge ${pr} --auto --squash --match-head-commit ${result.head ?? head}`,
+  }
+}
+
+const realGh = (...a) => execFileSync('gh', a, { encoding: 'utf8', maxBuffer: 256 << 20 })
+
+function arg(name) {
+  const i = process.argv.indexOf(name)
+  return i > 0 ? process.argv[i + 1] : undefined
+}
+
+// `gh` and `env` are injectable so a test drives the whole check without the network, the way
+// merge-gate.mjs does.
+export function main({ gh = realGh, env = process.env, repo: repoOption, pr: prOption } = {}) {
+  const list = (path) => gh('api', '--paginate', '--jq', '.[]', path).split('\n').filter(Boolean).map((line) => JSON.parse(line))
+  const repo = repoOption || env.GH_REPO || arg('--repo')
+  if (!repo) throw new Error('set GH_REPO or pass --repo owner/name')
+  const pr = prOption || arg('--pr') || env.PR
+  if (!pr) throw new Error('set PR or pass --pr N')
+  const policyPath = arg('--policy') || env.REVIEW_POLICY || fileURLToPath(new URL('../agent/review-policy.json', import.meta.url))
+
+  const pull = JSON.parse(gh('api', `repos/${repo}/pulls/${pr}`))
+  const head = pull.head?.sha
+  if (!head) throw new Error(`pull request ${pr} has no head sha`)
+  const labels = (pull.labels ?? []).map((l) => l.name)
+  // #676: the association travels with the report, the way review-gate.mjs reads the same two APIs. Without
+  // it every conclusion is refused as "not tied to this repository", so a PASS could never clear the guard.
+  const reports = [
+    ...list(`repos/${repo}/issues/${pr}/comments?per_page=100`).map((c) => ({ kind: 'comment', login: c.user?.login, body: c.body, at: c.created_at, association: c.author_association })),
+    ...list(`repos/${repo}/pulls/${pr}/reviews?per_page=100`)
+      .filter((v) => (v.body ?? '').trim())
+      .map((v) => ({ kind: 'review', login: v.user?.login, body: v.body, at: v.submitted_at, association: v.author_association })),
+  ]
+  const files = list(`repos/${repo}/pulls/${pr}/files?per_page=100`)
+
+  // The live head is judged, never a cached one: a push that moved the tip voids the PASS the caller
+  // believes it has, and `--sha` lets a caller insist on the head it read a moment ago.
+  const result = judge({ labels, files, head, author: pull.user?.login, reports, policy: loadPolicy(policyPath), expectedHead: arg('--sha'), mode: 'queue' })
+  const decision = guard({ pr, head, labels, result, reports })
+
+  console.log(`enqueue-guard: PR #${pr} head ${short(decision.head)}, labels [${labels.join(', ')}]`)
+  console.log(decision.ok
+    ? `enqueue-guard: ALLOW — ${decision.message}; ${decision.exempt ? `a ${decision.tier ?? 'T0/T1'} tier or a mechanical release PR (AGENTS.md 6)` : `${decision.verdict?.by}'s PASS names this tip`}`
+    : `enqueue-guard: REFUSE — ${decision.message}`)
+  if (decision.ok) {
+    console.log('enqueue-guard: this guard never enqueues; run the command yourself, now:')
+    console.log(`  ${decision.command}`)
+  }
+  return decision
+}
+
+if (process.argv[1] === fileURLToPath(import.meta.url)) {
+  try { const decision = main(); process.exit(decision.ok ? 0 : 1) } catch (e) { console.error(`enqueue-guard: ${e.message}`); process.exit(1) }
+}

.github/scripts/enqueue-guard.test.mjs

@@ -0,0 +1,179 @@
+// enqueue-guard (#602): a held pull request, or one whose newest conclusion is not a PASS, is not
+// enqueued — and the guard never enqueues anything itself.
+import { test } from 'node:test'
+import assert from 'node:assert/strict'
+import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { main, recordOnlyAt } from './enqueue-guard.mjs'
+
+const HEAD = 'abaf938b0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f'
+const OLD = '175328d0c1b2a3d4e5f60718293a4b5c6d7e8f90'
+const HOST_FILE = 'core/src/host/turns.js' // T2: needs a PASS
+const TEST_FILE = 'core/test/enqueue-guard.test.js' // T1: needs none
+
+const policyFile = () => {
+  const path = join(mkdtempSync(join(tmpdir(), 'enqueue-guard-policy-')), 'review-policy.json')
+  writeFileSync(path, JSON.stringify({ reviewers: ['reviewer-agent', 'Birfy'], selfReview: ['Birfy'], writers: ['reviewer-agent', 'Reviewer', 'Birfy'] }))
+  return path
+}
+const POLICY = policyFile()
+
+// The fixed format (#637): the first line is the conclusion, the evidence below it. `Ran:` is what
+// makes a PASS evidence rather than a claim (review-gate's rule, reused here).
+const report = ({ tip = HEAD, verdict = 'PASS', extra = [] } = {}) =>
+  [`Verdict: ${verdict} at ${tip}`, 'Written-by: reviewer-agent', '', 'Ran: npm run verify → pass', 'Not run: smoke', ...extra].join('\n')
+
+// #676: only a comment GitHub ties to this repository can be a conclusion, so the fixture carries the
+// association the API sends for the shared `Birfy` account.
+const comment = (body = report(), at = '2026-10-21T10:31:00Z') =>
+  ({ user: { login: 'reviewer-agent' }, body, created_at: at, author_association: 'MEMBER' })
+
+const fakeGh = ({ comments = [], reviews = [], labels = [], files = [], head = HEAD, author = 'opener' } = {}) => {
+  const calls = []
+  const gh = (...args) => {
+    const path = args[args.length - 1]
+    calls.push(path)
+    if (path.includes('/files')) return files.map((f) => JSON.stringify(typeof f === 'string' ? { filename: f } : f)).join('\n')
+    if (path.includes('/comments')) return comments.map((c) => JSON.stringify(c)).join('\n')
+    if (path.includes('/reviews')) return reviews.map((r) => JSON.stringify(r)).join('\n')
+    if (/\/pulls\/\d+$/.test(path)) return JSON.stringify({ head: { sha: head }, user: { login: author }, labels: labels.map((name) => ({ name })) })
+    // Anything else — `gh pr merge` above all — is the guard doing more than checking.
+    throw new Error('fake gh: unexpected ' + path)
+  }
+  gh.calls = calls
+  return gh
+}
+
+const run = (fixture = {}) => main({
+  gh: fakeGh({ files: [HOST_FILE], ...fixture }),
+  env: { REVIEW_POLICY: POLICY, GH_REPO: 'ThinkFlowLab/FlyBot', PR: '566' },
+  repo: 'ThinkFlowLab/FlyBot',
+  pr: 566,
+})
+
+test('#602: a held pull request is not enqueued, whatever a PASS at its tip says', () => {
+  const r = run({ labels: ['hold'], comments: [comment()] })
+  assert.equal(r.ok, false)
+  assert.equal(r.held, true)
+  assert.match(r.reasons.join('; '), /`hold` label/)
+  // The refusal says what to do ("do not enqueue") and names the hold, so it cannot be read as
+  // "the PASS was missing".
+  assert.match(r.message, /^do not enqueue #566: .*`hold` label/)
+})
+
+test('#602: a PASS at the tip lets it through, and the guard only prints the command', () => {
+  const gh = fakeGh({ files: [HOST_FILE], comments: [comment()] })
+  const r = main({ gh, env: { REVIEW_POLICY: POLICY, GH_REPO: 'ThinkFlowLab/FlyBot', PR: '566' }, repo: 'ThinkFlowLab/FlyBot', pr: 566 })
+  assert.equal(r.ok, true, r.reasons.join('; '))
+  assert.equal(r.command, `gh pr merge 566 --auto --squash --match-head-commit ${HEAD}`)
+  // No `gh pr merge` was run: the fake throws on any path it does not recognise, and every call here
+  // is a read.
+  for (const path of gh.calls) assert.match(path, /\/(pulls\/\d+|files|comments|reviews)(\?|$)/)
+})
+
+test('#602: no conclusion at the tip refuses', () => {
+  const r = run({ comments: [comment('LGTM, merging this')] })
+  assert.equal(r.ok, false)
+  assert.match(r.reasons.join('; '), /no Verifier report names the tip/)
+})
+
+test('#602: an INC at the tip refuses even when an older PASS names the same head', () => {
+  const r = run({ comments: [comment(report(), '2026-10-21T10:31:08Z'), comment(report({ verdict: 'INC' }), '2026-10-21T11:24:37Z')] })
+  assert.equal(r.ok, false)
+  assert.equal(r.verdict.verdict, 'INC')
+})
+
+test('#602: a PASS naming an older tip does not clear the live head', () => {
+  const r = run({ comments: [comment(report({ tip: OLD }))] })
+  assert.equal(r.ok, false)
+  assert.match(r.reasons.join('; '), /not the tip/)
+})
+
+test('#602: a "record only — do not merge" PASS at the head keeps it out of the queue', () => {
+  const parked = comment(report({ extra: ['Record only — do not merge / 仅记录、禁止入队'] }))
+  const r = run({ comments: [parked] })
+  assert.equal(r.ok, false)
+  assert.equal(r.recordOnly, true)
+  assert.match(r.reasons.join('; '), /record only/)
+})
+
+test('#602: the newest conclusion decides, so a fresh PASS supersedes an older parked report', () => {
+  const parked = comment(report({ extra: ['Record only — do not merge'] }), '2026-10-21T10:00:00Z')
+  const fresh = comment(report(), '2026-10-21T12:00:00Z')
+  assert.equal(run({ comments: [parked, fresh] }).ok, true)
+  // The same pair the other way round is a fresh freeze over an older PASS: no enqueue.
+  const r = run({ comments: [fresh, comment(report({ extra: ['Record only — do not merge'] }), '2026-10-21T13:00:00Z')] })
+  assert.equal(r.ok, false)
+  assert.equal(r.recordOnly, true)
+})
+
+test('#602: the marker on a different tip is not this head', () => {
+  const parkedOld = comment(report({ tip: OLD, extra: ['Record only — do not merge'] }), '2026-10-21T10:00:00Z')
+  assert.equal(run({ comments: [parkedOld, comment(report(), '2026-10-21T12:00:00Z')] }).ok, true)
+  assert.equal(recordOnlyAt([{ body: report({ tip: OLD, extra: ['Record only — do not merge'] }), at: '2026-10-21T10:00:00Z' }], HEAD), null)
+})
+
+test('#602: prose that quotes the marker without being a conclusion is not a freeze', () => {
+  const note = { user: { login: 'Chief' }, body: 'The routine says "record only — do not merge" when the owner parks one; this one is not parked.', created_at: '2026-10-21T13:00:00Z' }
+  assert.equal(recordOnlyAt([note], HEAD), null)
+  assert.equal(run({ comments: [comment(report(), '2026-10-21T10:00:00Z'), note] }).ok, true)
+})
+
+// The first PASS written for this guard explained the rule and was refused by it: the phrase sat in the
+// middle of a bullet, not as a marker. A conclusion that quotes the rule must not freeze itself (#687).
+test('#602: a PASS that explains the marker in a sentence does not freeze itself', () => {
+  const explaining = [
+    'Verdict: PASS at ' + HEAD,
+    'Written-by: Reviewer',
+    '',
+    'Ran: node --test .github/scripts/enqueue-guard.test.mjs',
+    '',
+    '- `RECORD_ONLY` matches the English marker only. `AGENTS.md` rule 2 pairs `hold` with "Record only — do not merge / 仅记录、禁止入队"; a record written only in Chinese would not keep the pull request out.',
+    '- A bullet saying `Record only — do not merge` in the middle of a sentence is not a marker either.',
+  ].join('\n')
+  assert.equal(recordOnlyAt([{ body: explaining, at: '2026-10-22T06:10:56Z' }], HEAD), null)
+  assert.equal(run({ comments: [{ user: { login: 'reviewer-agent' }, body: explaining, created_at: '2026-10-22T06:10:56Z', author_association: 'MEMBER' }] }).ok, true)
+})
+
+test('#602: the marker is a whole line, and its other language half is allowed', () => {
+  for (const line of ['Record only — do not merge', '- Record only — do not merge', '> Record only — do not merge', '仅记录、禁止入队', 'Record only — do not merge / 仅记录、禁止入队']) {
+    assert.ok(recordOnlyAt([{ body: report({ extra: [line] }), at: '2026-10-21T10:00:00Z' }], HEAD), line)
+  }
+})
+
+test('#602: a T0/T1 pull request still needs no PASS', () => {
+  const r = run({ files: [TEST_FILE], comments: [] })
+  assert.equal(r.ok, true, r.reasons.join('; '))
+  assert.equal(r.exempt, true)
+  assert.equal(r.tier, 'T1')
+})
+
+test('#602: hold refuses a T0/T1 pull request too', () => {
+  const r = run({ files: [TEST_FILE], labels: ['hold'], comments: [] })
+  assert.equal(r.ok, false)
+  assert.equal(r.held, true)
+})
+
+test('#602: the self-gate case (author Birfy, PASS by Birfy) is allowed', () => {
+  const r = main({
+    gh: fakeGh({
+      files: [HOST_FILE],
+      author: 'Birfy',
+      comments: [{ user: { login: 'Birfy' }, body: report(), created_at: '2026-10-21T10:31:00Z', author_association: 'OWNER' }],
+    }),
+    env: { REVIEW_POLICY: POLICY, GH_REPO: 'ThinkFlowLab/FlyBot', PR: '566' },
+    repo: 'ThinkFlowLab/FlyBot',
+    pr: 566,
+  })
+  assert.equal(r.ok, true, r.reasons.join('; '))
+})
+
+// The guard is only in the path if the instructions that tell an agent to enqueue name it; otherwise
+// the next agent runs `gh pr merge` from memory and #602 is back.
+test('#602: the instructions that enqueue point at the guard', () => {
+  const agents = readFileSync(new URL('../../AGENTS.md', import.meta.url), 'utf8')
+  const fixIssue = readFileSync(new URL('../../.claude/skills/fix-issue/SKILL.md', import.meta.url), 'utf8')
+  const plumbing = readFileSync(new URL('../../docs/autonomous-dev.md', import.meta.url), 'utf8')
+  for (const text of [agents, fixIssue, plumbing]) assert.match(text, /enqueue-guard\.mjs/)
+})