Judge a conclusion against the whole head, and print the head that was judged (#1112)

.claude/skills/review-pr/SKILL.md

@@ -12,7 +12,7 @@ This file stays thin: Verifier process only. On each tip review, re-read these f
 ## Establish the tip
 
 1. Confirm repo identity (`ThinkFlowLab/FlyBot` or the agreed fork) and the commit under review.
-2. Record **tip SHA** (`git rev-parse HEAD` or the PR head SHA). If the tip moves, re-run from scratch.
+2. Record **tip SHA** (`git rev-parse HEAD` or the PR head SHA) — all 40 characters. The gate compares the whole sha: a `Verdict: … at abaf938` names every head that starts that way, and the newest of them is the tip being judged, so a short sha never counts (#1112). If the tip moves, re-run from scratch.
 3. Load the **latest rules from `origin/main`** before each review: `git fetch origin && git show origin/main:<path>` for this skill, `docs/code-standards.md`, `AGENTS.md`, and every `docs/subsystems/` page the diff touches. Prefer the fetched text over a stale local copy. Do not restate those pages here.
 4. Read [LESSONS.md](../../../LESSONS.md) and the hard rules in [CONTRIBUTING.md](../../../CONTRIBUTING.md).
 5. Inventory changed files vs the merge base. Do not treat author narrative as evidence.

.github/scripts/enqueue-guard.mjs

@@ -59,13 +59,22 @@ export function decidePull({ gh = realGh, env = process.env, repo: repoOption, p
   // #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.
   // #943: the account type travels too, for the same reason — a bot account writes no conclusion.
+  // #1112: each report carries the id GitHub gave it, so the summary can name the one the decision used.
+  // With one shared account the id is the only handle on a conclusion that is not a person or a word.
   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 })),
+    ...list(`repos/${repo}/issues/${pr}/comments?per_page=100`).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 })),
     ...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 })),
+      .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 })),
   ]
   const files = list(`repos/${repo}/pulls/${pr}/files?per_page=100`)
+  // #1112: the head is read again after the comments and the diff. A push in the middle of this run
+  // would otherwise let the decision judge one head and print another — the shape in which a pull
+  // request that was PASSed once is pushed again and merges with no conclusion at the new tip. A
+  // moved head is refused rather than resolved: both reads are stale by the time the second lands.
+  const again = JSON.parse(gh('api', `repos/${repo}/pulls/${pr}`))
+  const moved = again.head?.sha
+  if (moved && moved !== head) throw new Error(`the head moved during this run (${short(head)} to ${short(moved)}); run it again and judge the head it prints`)
 
   // 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.
@@ -85,7 +94,11 @@ export function decidePull({ gh = realGh, env = process.env, repo: repoOption, p
 export function main({ gh = realGh, env = process.env, repo: repoOption, pr: prOption } = {}) {
   const { decision, pull, pr } = decidePull({ gh, env, repo: repoOption, pr: prOption })
   const labels = (pull.labels ?? []).map((l) => l.name)
-  console.log(`enqueue-guard: PR #${pr} head ${short(decision.head)}, labels [${labels.join(', ')}]`)
+  // #1112: the full 40-character sha, from the value the decision consumed. The short form let a reader
+  // compare it against a summary that named a different head and still look right.
+  console.log(`enqueue-guard: PR #${pr} head ${decision.head}, labels [${labels.join(', ')}]`)
+  const used = decision.verdict?.id ? `${decision.verdict.kind ?? 'comment'} ${decision.verdict.id}` : 'no conclusion'
+  console.log(`enqueue-guard: conclusion used: ${used}${decision.verdict?.by ? ` by ${decision.verdict.by}` : ''}${decision.verdict?.tip ? ` naming ${decision.verdict.tip}` : ''}`)
   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}`)

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

@@ -36,8 +36,9 @@ const report = ({ tip = HEAD, verdict = 'PASS', extra = [] } = {}) =>
 const comment = (body = report(), at = '2026-10-21T10:31:00Z') =>
   ({ user: { login: 'reviewer-agent' }, body, created_at: at, author_association: 'MEMBER' })
 
-const fakeGh = ({ body = HEADERS + 'Fixes #602\n', comments = [], reviews = [], labels = ['agent-pr'], files = [], head = HEAD, author = 'opener', headRepo = 'ThinkFlowLab/FlyBot', permission = undefined, draft = false, merged = false } = {}) => {
+const fakeGh = ({ body = HEADERS + 'Fixes #602\n', comments = [], reviews = [], labels = ['agent-pr'], files = [], head = HEAD, heads = null, author = 'opener', headRepo = 'ThinkFlowLab/FlyBot', permission = undefined, draft = false, merged = false } = {}) => {
   const calls = []
+  let pullReads = 0
   const gh = (...args) => {
     const path = args[args.length - 1]
     calls.push(path)
@@ -48,7 +49,11 @@ const fakeGh = ({ body = HEADERS + 'Fixes #602\n', comments = [], reviews = [],
     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, repo: { full_name: headRepo } }, user: { login: author }, body, labels: labels.map((name) => ({ name })), draft, merged })
+    if (/\/pulls\/\d+$/.test(path)) {
+      // #1112 (c): a run reads the head twice, so the fixture can answer differently the second time.
+      const sha = heads ? heads[Math.min(pullReads++, heads.length - 1)] : head
+      return JSON.stringify({ head: { sha, repo: { full_name: headRepo } }, user: { login: author }, body, labels: labels.map((name) => ({ name })), draft, merged })
+    }
     // Anything else — `gh pr merge` above all — is the guard doing more than checking.
     throw new Error('fake gh: unexpected ' + path)
   }
@@ -599,3 +604,45 @@ test('#962: Docs is registered in writers; Owner: Docs passes and an unknown own
   assert.equal(refused.ok, false)
   assert.ok(refused.reasons.includes('Owner names "somebody", who is not a writers Bot name'), refused.reasons.join('; '))
 })
+
+
+// ---- #1112: the guard reported a PASS that had not been posted yet, and a re-push could go green again.
+// The three cases below are one per failure mode the issue names; each is red on the code before it.
+
+test('#1112: a 7-character prefix is not this tip, even though the head starts with it', () => {
+  // The guard went green at 05:09 on a conclusion naming a head that was PASSed at 05:12: a tip that is
+  // only a prefix matches every head that begins with it, and the newest one is the tip being judged.
+  const r = run({ comments: [comment(report({ tip: HEAD.slice(0, 7) }))] })
+  assert.equal(r.ok, false, 'a short tip names no head')
+  assert.match(r.reasons.join('; '), /7 characters/)
+})
+
+test('#1112: the summary names the comment id the decision used, and a later edit cannot change it', () => {
+  const reports = [{ id: 4242, user: { login: 'reviewer-agent' }, body: report(), created_at: '2026-10-21T10:31:00Z', author_association: 'MEMBER' }]
+  const r = run({ comments: reports })
+  assert.equal(r.ok, true, r.reasons.join('; '))
+  assert.equal(r.verdict.id, 4242)
+  assert.equal(r.verdict.kind, 'comment')
+  // The decision holds its own copy: an edit after the read is not a second opinion.
+  reports[0].body = report({ verdict: 'INC' })
+  assert.equal(r.verdict.verdict, 'PASS')
+})
+
+test('#1112: the printed head is the decided head, all 40 characters, and it names the conclusion', () => {
+  const lines = []
+  const log = console.log
+  console.log = (...a) => lines.push(a.join(' '))
+  try {
+    run({ comments: [{ id: 77, user: { login: 'reviewer-agent' }, body: report(), created_at: '2026-10-21T10:31:00Z', author_association: 'MEMBER' }] })
+  } finally { console.log = log }
+  assert.match(lines[0], new RegExp(`head ${HEAD},`), 'the head is printed whole, from the value judged')
+  assert.match(lines.join('\n'), /conclusion used: comment 77 by reviewer-agent naming [0-9a-f]{40}/)
+})
+
+test('#1112: a head that moves while the comments are read is refused, not judged', () => {
+  const later = 'c0ffee00'.repeat(5)
+  let thrown = null
+  try { run({ comments: [comment()], heads: [HEAD, later] }) } catch (e) { thrown = e }
+  assert.ok(thrown, 'the run refuses instead of judging a head nobody will merge')
+  assert.match(thrown.message, /head moved during this run/)
+})

.github/scripts/enqueue-rules.mjs

@@ -121,7 +121,7 @@ export function recordOnlyAt(reports, head) {
   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))
+    .filter(({ parsed }) => parsed.verdict && parsed.tip && target === 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)

.github/scripts/gate-check.mjs

@@ -87,7 +87,9 @@ export function summaryOf({ decision, head, pr, entry } = {}) {
   if (decision?.exempt) lines.push(`- no PASS needed: ${decision.tier ? `computed tier ${decision.tier}` : 'a T0/T1 tier or a mechanical release pull request'} (AGENTS.md → Merge gates)`)
   if (decision?.mechanicalRelease) lines.push('- a mechanical release pull request: it carries no issue and no `agent-pr` by design')
   if (decision?.fork) lines.push('- the head lives in a fork: the issue reference and `agent-pr` rules are not asked of it')
-  if (decision?.verdict?.by) lines.push(`- newest conclusion at this tip: ${decision.verdict.verdict ?? 'PASS'} by ${decision.verdict.by}`)
+  // #1112: the summary names the comment the decision read, so a reader can go and look at the text it
+  // used instead of trusting the sentence. The full sha above it is the value that was judged.
+  if (decision?.verdict?.by) lines.push(`- newest conclusion at this tip: ${decision.verdict.verdict ?? 'PASS'} by ${decision.verdict.by} (${decision.verdict.kind ?? 'comment'} ${decision.verdict.id ?? 'without an id'}), naming ${decision.verdict.tip ?? 'no tip'}`)
   if (decision?.recordOnly) lines.push('- the newest conclusion at this head says "record only — do not merge"')
   if (!ok) lines.push('', 'What refuses it:', ...(decision?.reasons?.length ? decision.reasons.map((r) => `- ${r}`) : ['- no reason was recorded — treat this as a bug in the gate itself']))
   lines.push('', '_One decision, two readers: this check and `node .github/scripts/enqueue-guard.mjs --pr <n>` call the same function. Rules: [`AGENTS.md` → Merge gates](../../AGENTS.md#merge-gates). This check reads the repository and writes this one run; it never merges, labels or comments._')

.github/scripts/review-gate.mjs

@@ -206,11 +206,17 @@ export function decide({ head, author, reports, policy, admins } = {}) {
   for (const r of reports ?? []) {
     const parsed = parseReport(r.body)
     if (!parsed.tip && !parsed.verdict) continue // not a Verifier report
-    const entry = { login: r.login, verdict: parsed.verdict, tip: parsed.tip, at: r.at, kind: r.kind, writer: parsed.writer, parsed, counts: false, why: null }
+    const entry = { id: r.id, login: r.login, verdict: parsed.verdict, tip: parsed.tip, at: r.at, kind: r.kind, writer: parsed.writer, parsed, counts: false, why: null }
     judged.push(entry)
     if (!parsed.tip) { reasons.push(`${where(r)}: no Tip SHA`); continue }
-    if (!headSha.startsWith(parsed.tip)) {
-      reasons.push(`${where(r)}: names ${parsed.tip.slice(0, SHORT)}, not the tip ${headSha.slice(0, SHORT)}`)
+    // #1112: the comparison is the whole sha, not a prefix. A conclusion written `at abaf938` is a
+    // mention of one head among every head that starts that way, and one of them is the newest tip — the
+    // guard went green at 05:09 on a PASS naming the tip that was PASSed at 05:12 because a short tip
+    // matched it. A tip that is not 40 characters names no head here; the refusal says how long it was.
+    if (parsed.tip !== headSha) {
+      reasons.push(parsed.tip.length < 40
+        ? `${where(r)}: names ${parsed.tip.slice(0, SHORT)} (${parsed.tip.length} characters), not the full tip ${headSha} — a conclusion names the whole 40-character sha`
+        : `${where(r)}: names ${parsed.tip.slice(0, SHORT)}, not the tip ${headSha.slice(0, SHORT)}`)
       continue
     }
     if (!parsed.verdict) continue // a `Tip SHA` block with no verdict is not a conclusion
@@ -264,15 +270,15 @@ export function decide({ head, author, reports, policy, admins } = {}) {
     .map((e, i) => ({ e, i, t: Date.parse(e.at ?? 0) || 0 }))
     .sort((a, b) => a.t - b.t || a.i - b.i)
     .at(-1).e
-  if (!last.counts) return { ok: false, verdict: last.verdict, by: null, tip: last.tip, reason: last.why, judged }
+  if (!last.counts) return { ok: false, verdict: last.verdict, by: null, tip: last.tip, id: last.id, kind: last.kind, reason: last.why, judged }
   const { parsed } = last
   if (last.verdict !== 'PASS') {
-    return { ok: false, verdict: last.verdict, by: last.login, tip: last.tip, reason: `${last.login} says ${last.verdict ?? 'nothing'} at this tip`, judged }
+    return { ok: false, verdict: last.verdict, by: last.login, tip: last.tip, id: last.id, kind: last.kind, reason: `${last.login} says ${last.verdict ?? 'nothing'} at this tip`, judged }
   }
   if (!parsed.ran.length) {
-    return { ok: false, verdict: 'PASS', by: last.login, tip: last.tip, reason: `${last.login}'s PASS lists no command it ran`, judged }
+    return { ok: false, verdict: 'PASS', by: last.login, tip: last.tip, id: last.id, kind: last.kind, reason: `${last.login}'s PASS lists no command it ran`, judged }
   }
-  return { ok: true, verdict: 'PASS', by: last.login, tip: last.tip, reason: `${last.login}'s PASS names this tip`, judged }
+  return { ok: true, verdict: 'PASS', by: last.login, tip: last.tip, id: last.id, kind: last.kind, reason: `${last.login}'s PASS names this tip`, judged }
 }
 
 export function render(result, { pr, head, author } = {}) {

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

@@ -257,8 +257,11 @@ test('#643: a quoted verdict later in the body does not overwrite the first line
 // report quoted in the body, used to move the tip, so `decide` dropped the INC as "about another
 // tip" and the PASS it refused stayed in force.
 test('#640: a quoted Tip SHA later in the body does not move the first line to another tip', () => {
+  // #1112 made a conclusion name the whole sha, so this fixture names the head it judges in full: the
+  // point of the case is which line names the tip, not how long the tip is.
+  const head = '1234567890abcdef000000000000000000000000'
   const body = [
-    'Verdict: INC at 1234567890abcdef',
+    `Verdict: INC at ${head}`,
     'Written by: Reviewer',
     '',
     'Ran: node --test .github/scripts/review-gate.test.mjs → 43 pass',
@@ -274,12 +277,11 @@ test('#640: a quoted Tip SHA later in the body does not move the first line to a
   ].join('\n')
   const parsed = parseReport(body)
   assert.equal(parsed.verdict, 'INC')
-  assert.equal(parsed.tip, '1234567890abcdef', 'the first line names the tip being judged')
+  assert.equal(parsed.tip, head, 'the first line names the tip being judged')
   assert.deepEqual(parsed.ran, ['node --test .github/scripts/review-gate.test.mjs → 43 pass'])
   assert.equal(parsed.hasNotRun, true)
 
   // And the whole point: the INC is not dropped, so it still refuses the merge.
-  const head = '1234567890abcdef000000000000000000000000'
   const at = (report, when) => ({ kind: 'comment', login: 'reviewer-agent', body: report, at: when, association: 'MEMBER' })
   const policy = { reviewers: ['reviewer-agent'], selfReview: [], writers: ['Reviewer'] }
   const pass = ['Tip SHA: ' + head, 'Ran:', '  - npm test → pass', 'Not run:', '  - smoke', 'Verdict: PASS'].join('\n')

AGENTS.md

@@ -184,7 +184,7 @@ Four rules decide what a queue entry must pass. The mechanical judge runs where
 
 A review's `state` is not read at all: approvals are not a gate any more (user decision 2026-10-22), and the account cannot approve its own pull requests, so its reviews stay `COMMENTED` even for an INC — the INC on #610 was a comment rather than a review.
 
-**One conclusion per tip.** A tip carries at most one conclusion: one `Verdict:` block naming one SHA. Two conclusions at the same tip is an incident, not a tie to be broken — nobody re-verdicts to win, the Reviewer that wrote the later one retracts it in a comment, and the pull request waits. A correction reads `Correction from <Bot>: …` and carries no `Verdict:` line, so it never competes with the standing conclusion; the replacement PASS for a missing `Ran:` ([above](#gates)) is the only other body allowed at one tip. **A GitHub bot account's `Verdict:` is never a conclusion** — `user.type` is `Bot`, or the login ends `[bot]` — whatever its body says (#943, `cursor[bot]` at #940's tip); the rule is the account type and not a login list, because every Bot writes as the shared `Birfy` account and a list containing it would void the whole pipeline, so the real Reviewer voids such a report at that tip with `Correction from Reviewer:`.
+**One conclusion per tip.** A tip carries at most one conclusion: one `Verdict:` block naming one SHA — the whole 40 characters, because a prefix names every head that starts that way and the newest of them is the tip being judged (#1112). Two conclusions at the same tip is an incident, not a tie to be broken — nobody re-verdicts to win, the Reviewer that wrote the later one retracts it in a comment, and the pull request waits. A correction reads `Correction from <Bot>: …` and carries no `Verdict:` line, so it never competes with the standing conclusion; the replacement PASS for a missing `Ran:` ([above](#gates)) is the only other body allowed at one tip. **A GitHub bot account's `Verdict:` is never a conclusion** — `user.type` is `Bot`, or the login ends `[bot]` — whatever its body says (#943, `cursor[bot]` at #940's tip); the rule is the account type and not a login list, because every Bot writes as the shared `Birfy` account and a list containing it would void the whole pipeline, so the real Reviewer voids such a report at that tip with `Correction from Reviewer:`.
 
 **Judged on the merge tree, never on the pull request page.** The page answers "does this commit pass alone"; the queue answers "does this commit pass merged onto the current `main`", and the second one is the merge decision. #759 was green on every check, was kicked out of the queue twice, and went red on `main` at `core/test/follow-through.test.js:264`. A red queue entry that names files you did not touch is the stale base of [step 2](#pipeline-contract), not bad luck: fix the base, do not re-enqueue blindly.