Refuse a report that does not say who wrote it (#627)

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

@@ -35,7 +35,8 @@ Always include:
 
 ```text
 Verdict: PASS | FAIL | INC at <full or 12+ hex>
-Written by: Reviewer. Posted by: <who actually posted it>.
+Written-by: Reviewer
+Posted-by: <who actually posted it, when the writer could not post it themselves>
 
 Ran:
   - <command> → pass|fail
@@ -45,7 +46,9 @@ Not run:
 ```
 
 The first line is the whole conclusion (#637): nothing may come before it, because the gates read the
-first line and treat any other shape as no conclusion.
+first line and treat any other shape as no conclusion. The `Written-by:` line is the attribution every write carries (#627): it names a Bot from the policy's
+`writers`, the gates refuse a report without it, and it is a self-report, not proof — see
+[AGENTS.md](../../../AGENTS.md#every-state-changing-write-says-who-wrote-it-627), which is the single source.
 
 What the gates read as evidence (`review-gate.mjs` `parseReport`; #660–#663 refused at the queue for
 the first point):

.github/agent/review-policy.json

@@ -1,6 +1,19 @@
 {
   "version": 1,
-  "note": "Who may record a PASS that counts, for the `reviewed tip` report (Phase 1: report only; see docs/autonomous-dev.md). A login listed here is not a merge right: the report is read when a tip is judged, and a PASS is required only for T2/T3 (at the tip, with green CI); T0/T1 merge on green CI without one (AGENTS.md step 6). A pull request's own author does not count unless the author is listed in `selfReview` (owner decision 2026-10-19: the owner's Bot/agent, which acts as Birfy, may PASS its own pull requests). A self-PASS never replaces green CI: the agent that gated a T2/T3 pull request merges it itself once a PASS sits at its tip (owner decision 2026-10-21). Editing this file is itself T3.",
+  "note": "Who may record a PASS that counts, for the `reviewed tip` report (Phase 1: report only; see docs/autonomous-dev.md). A login listed here is not a merge right: the report is read when a tip is judged, and a PASS is required only for T2/T3 (at the tip, with green CI); T0/T1 merge on green CI without one (AGENTS.md step 6). A pull request's own author does not count unless the author is listed in `selfReview` (owner decision 2026-10-19: the owner's Bot/agent, which acts as Birfy, may PASS its own pull requests). A self-PASS never replaces green CI: the agent that gated a T2/T3 pull request merges it itself once a PASS sits at its tip (owner decision 2026-10-21). Editing this file is itself T3. `writers` lists the names a report's `Written-by:` line may carry (#627): every Bot shares one GitHub identity, so that line is a claim, and the gate refuses a write whose line is missing or names something outside this list.",
   "reviewers": ["hsliuustc0106", "Birfy"],
-  "selfReview": ["Birfy"]
+  "selfReview": ["Birfy"],
+  "writers": [
+    "Chief",
+    "Triage",
+    "Reviewer",
+    "Release",
+    "SelfTest",
+    "Maintainer",
+    "Dev",
+    "Dev2",
+    "Dev3",
+    "Dev4",
+    "Birfy"
+  ]
 }

.github/scripts/merge-gate.mjs

@@ -170,10 +170,10 @@ export function main({ gh = realGh, env = process.env, repo: repoOption, pr: prO
   const author = pull.user?.login
   const labels = (pull.labels ?? []).map((l) => l.name)
   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 })),
+    ...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 })),
+      .map((v) => ({ kind: 'review', login: v.user?.login, body: v.body, at: v.submitted_at, association: v.author_association })),
   ]
 
   // The workflow passes both events through this same entry point: `merge_group` judges, `pull_request`

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

@@ -8,12 +8,12 @@ import { fileURLToPath } from 'node:url'
 const HEAD = 'abaf938b0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f'
 const OLD = '175328d0c1b2a3d4e5f60718293a4b5c6d7e8f90'
 const ENTRY = `gh-readonly-queue/main/pr-566-${HEAD}`
-const policy = { reviewers: ['reviewer-agent'], selfReview: [] }
+const policy = { reviewers: ['reviewer-agent'], selfReview: [], writers: ['Reviewer'] }
 
 const report = ({ tip = HEAD, verdict = 'PASS' } = {}) =>
-  [`Tip SHA: ${tip}`, 'Ran:', '  - npm run verify → pass', 'Not run:', '  - smoke — host code unchanged', `Verdict: ${verdict}`].join('\n')
+  [`Tip SHA: ${tip}`, 'Written-by: Reviewer', 'Ran:', '  - npm run verify → pass', 'Not run:', '  - smoke — host code unchanged', `Verdict: ${verdict}`].join('\n')
 
-const say = (body = report(), at = '2026-10-21T10:31:00Z', kind = 'comment') => ({ login: 'reviewer-agent', body, at, kind })
+const say = (body = report(), at = '2026-10-21T10:31:00Z', kind = 'comment') => ({ login: 'reviewer-agent', body, at, kind, association: 'MEMBER' })
 
 const judgeAt = (over = {}) => judge({ labels: [], head: HEAD, author: 'opener', reports: [say()], policy, ...over })
 
@@ -142,7 +142,7 @@ const fakeGh = ({ comments, reviews = [], labels = [], files = [], head = HEAD }
 
 const runGate = (fixture) => {
   const policy = join(mkdtempSync(join(tmpdir(), 'merge-gate-policy-')), 'review-policy.json')
-  writeFileSync(policy, JSON.stringify({ reviewers: ['reviewer-agent'], selfReview: [] }))
+  writeFileSync(policy, JSON.stringify({ reviewers: ['reviewer-agent'], selfReview: [], writers: ['Reviewer'] }))
   return main({
   gh: fakeGh(fixture),
   env: { GITHUB_STEP_SUMMARY: '', REVIEW_POLICY: policy },
@@ -152,8 +152,8 @@ const runGate = (fixture) => {
   })
 }
 
-const apiComment = (body, at) => ({ user: { login: 'reviewer-agent' }, body, created_at: at })
-const apiReview = (body, at) => ({ user: { login: 'reviewer-agent' }, body, submitted_at: at })
+const apiComment = (body, at) => ({ user: { login: 'reviewer-agent' }, body, created_at: at, author_association: 'MEMBER' })
+const apiReview = (body, at) => ({ user: { login: 'reviewer-agent' }, body, submitted_at: at, author_association: 'MEMBER' })
 
 const firstLine = (verdict, tip = HEAD) => [
   `Verdict: ${verdict} at ${tip}`,
@@ -218,7 +218,7 @@ test('#639: a report names what the entry would refuse and still passes', () =>
 
 test('#639: the pull_request entry point reports without a head_ref and does not throw', () => {
   const policy = join(mkdtempSync(join(tmpdir(), 'merge-gate-policy-')), 'review-policy.json')
-  writeFileSync(policy, JSON.stringify({ reviewers: ['reviewer-agent'], selfReview: [] }))
+  writeFileSync(policy, JSON.stringify({ reviewers: ['reviewer-agent'], selfReview: [], writers: ['Reviewer'] }))
   const result = main({
     gh: fakeGh({ comments: [apiComment(report({ verdict: 'INC' }), '2026-10-21T12:49:00Z')] }),
     env: { GITHUB_STEP_SUMMARY: '', REVIEW_POLICY: policy, MODE: 'report', PR: '639' },
@@ -273,7 +273,7 @@ const RUN_FILES = ['core/src/host/turns.js', 'changelog.d/651.fixed.md']
 
 const runQueue = (fixture) => {
   const policyFile = join(mkdtempSync(join(tmpdir(), 'merge-gate-policy-')), 'review-policy.json')
-  writeFileSync(policyFile, JSON.stringify({ reviewers: ['reviewer-agent'], selfReview: [] }))
+  writeFileSync(policyFile, JSON.stringify({ reviewers: ['reviewer-agent'], selfReview: [], writers: ['Reviewer'] }))
   // Exactly what merge-gate.yml hands the script on a merge_group run, and no PR number.
   return main({
     gh: fakeGh({ head: RUN_HEAD, files: RUN_FILES, ...fixture }),
@@ -324,7 +324,7 @@ test('#654: a T1 queue entry passes without a PASS, as the pull_request report s
   assert.equal(queue.ok, true, queue.reasons.join('; '))
   assert.equal(queue.tier, 'T1')
   const policyFile = join(mkdtempSync(join(tmpdir(), 'merge-gate-policy-')), 'review-policy.json')
-  writeFileSync(policyFile, JSON.stringify({ reviewers: ['reviewer-agent'], selfReview: [] }))
+  writeFileSync(policyFile, JSON.stringify({ reviewers: ['reviewer-agent'], selfReview: [], writers: ['Reviewer'] }))
   const report = main({ gh: fakeGh({ head: RUN_HEAD, files, comments: [] }), env: { GITHUB_STEP_SUMMARY: '', REVIEW_POLICY: policyFile, GH_REPO: 'ThinkFlowLab/FlyBot', PR: '654', MODE: 'report' } })
   assert.deepEqual(report.reasons, queue.reasons, 'both events judge the same head the same way')
   assert.equal(report.head, queue.head)
@@ -347,7 +347,7 @@ const queueReal = (fx, body = fx.comment.body) => main({
   gh: (...args) => {
     const path = args[args.length - 1]
     if (path.includes('/files')) return fx.files.map((filename) => JSON.stringify({ filename })).join('\n')
-    if (path.includes('/comments')) return JSON.stringify({ user: { login: fx.comment.login }, body, created_at: fx.comment.created_at })
+    if (path.includes('/comments')) return JSON.stringify({ user: { login: fx.comment.login }, body, created_at: fx.comment.created_at, author_association: 'OWNER' })
     if (path.includes('/reviews')) return ''
     if (/\/pulls\/\d+$/.test(path)) return JSON.stringify({ head: { sha: fx.head }, user: { login: fx.author }, labels: [] })
     throw new Error('fake gh: unexpected ' + path)

.github/scripts/review-gate.mjs

@@ -13,6 +13,9 @@
 //     its own pull requests; any other author still needs someone else);
 //   - it carries the Verifier block review-pr already writes, with a `Tip SHA:` that is the
 //     head being judged and a `Verdict:`;
+//   - it names its writer in the header: `Written-by: <Bot>` (the older prose `Written by: …` reads
+//     the same way) with a name from the policy's `writers`, or it refuses — a missing line and an
+//     unknown name are separate refusals (#627);
 //   - the verdict is PASS and the report lists a command it `Ran:` (a `- ` bullet, or a `$ ` line in a
 //     fenced block under a `Ran …:` heading): a report that ran nothing is not evidence.
 //   - `Not run:` is recommended but not checked (owner decision 2026-10-22 09:57 Asia/Shanghai, #667):
@@ -27,6 +30,10 @@ import { execFileSync } from 'node:child_process'
 import { fileURLToPath } from 'node:url'
 
 const SHORT = 12
+// #676: a conclusion is read only from someone GitHub ties to this repository. The fail-closed rule
+// above makes any newest conclusion refuse, so without this an outside comment saying `Verdict: INC`
+// could hold a merge. A comment from anyone else is not a conclusion: it can neither pass nor block.
+const ASSOCIATIONS = new Set(['OWNER', 'MEMBER', 'COLLABORATOR'])
 
 // 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.
@@ -35,7 +42,7 @@ export function parseReport(body) {
   // markdown emphasis, line for line, for everything else.
   const raw = String(body ?? '').split('\n')
   const lines = raw.map((line) => line.replace(/[*`]/g, ''))
-  const out = { tip: null, verdict: null, ran: [], hasNotRun: false }
+  const out = { tip: null, verdict: null, ran: [], hasNotRun: false, writer: null }
   // The fixed format (#637): the first non-empty line is the whole conclusion,
   // `Verdict: PASS at <sha>` (or INC / REQUEST_CHANGES). The attribution line and the evidence go
   // below it, so a verdict Chief posts for a Reviewer with no write channel parses the same way.
@@ -80,9 +87,27 @@ export function parseReport(body) {
     }
     if (/^Not run\b[^:\n]*:/i.test(line)) out.hasNotRun = true
   }
+  out.writer = parseWriter(lines)
   return out
 }
 
+// #627: the attribution line, `Written-by: <Bot>` (the older prose `Written by: Reviewer. Posted by:
+// …` reads the same way). Only the header is read — the first few non-empty lines — so a report that
+// arrives without one does not borrow a writer from a block it quotes further down.
+const HEADER_LINES = 3
+function parseWriter(lines) {
+  let seen = 0
+  for (const line of lines) {
+    const text = line.trim()
+    if (!text) continue
+    if (++seen > HEADER_LINES) break
+    const written = /^Written[- ]by\s*:\s*(.+)$/i.exec(text)
+    // The first clause is the name: `Reviewer. Posted by: Chief (proxy…)` names Reviewer.
+    if (written) return written[1].split(/[.,(;]/)[0].trim() || null
+  }
+  return null
+}
+
 // The commands under a `Ran` heading at line `at`: the inline text of a bare `Ran: <command>` (never
 // an empty one or `-`), then either the `- ` bullets that follow, or the `$ ` lines of the fenced
 // block that follows. Output lines in the block are not commands, so a block with no `$ ` line, or
@@ -117,45 +142,65 @@ export function decide({ head, author, reports, policy }) {
   const authorLogin = String(author ?? '').toLowerCase()
   const reviewers = new Set((policy?.reviewers ?? []).map((l) => String(l).toLowerCase()))
   const selfReview = new Set((policy?.selfReview ?? []).map((l) => String(l).toLowerCase()))
-  const reasons = []
+  const writers = new Set((policy?.writers ?? []).map((w) => String(w).toLowerCase()))
+  const reasons = [] // reports that do not name this tip at all
   const judged = []
+  const atTip = [] // conclusions that name this tip, valid or not, in the order the API returned them
 
   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, parsed, counts: false }
+    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 }
     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)}`)
       continue
     }
-    if (login(r) === authorLogin && !selfReview.has(authorLogin)) { reasons.push(`${where(r)}: the pull request's own author is not a reviewer (not in selfReview)`); continue }
-    if (!reviewers.has(login(r))) { reasons.push(`${where(r)}: not a reviewer listed in .github/agent/review-policy.json`); continue }
+    if (!parsed.verdict) continue // a `Tip SHA` block with no verdict is not a conclusion
+    const association = String(r.association ?? '').toUpperCase()
+    if (!ASSOCIATIONS.has(association)) {
+      reasons.push(`${where(r)}: author association ${association || 'missing'} is not OWNER, MEMBER or COLLABORATOR, so it is not a conclusion`)
+      continue
+    }
+    // #676: a conclusion at this tip belongs to the newest-word question even when it fails a check
+    // below, so it joins `atTip` carrying its reason instead of being dropped. Dropping it made a
+    // newer refusal invisible: an INC with no `Written-by:` line left an older PASS in force, and a
+    // human REQUEST_CHANGES from an unlisted login did the same.
+    atTip.push(entry)
+    // #627: a *conclusion* has to say who wrote it, refused on two separate grounds — a report with no
+    // attribution line and one that names a writer the policy does not list are different defects, and
+    // the log has to say which one fired. Both fail closed. An ordinary comment is not a conclusion, so
+    // a missing line is not held against it. With one shared GitHub identity the line is a self-report,
+    // so this buys presence and a name from a closed list, not proof (#633).
+    if (!parsed.writer) { entry.why = `${where(r)}: no Written-by line naming its writer`; continue }
+    if (!writers.has(parsed.writer.toLowerCase())) { entry.why = `${where(r)}: Written-by names "${parsed.writer}", who is not in the writers list of .github/agent/review-policy.json`; continue }
+    if (login(r) === authorLogin && !selfReview.has(authorLogin)) { entry.why = `${where(r)}: the pull request's own author is not a reviewer (not in selfReview)`; continue }
+    if (!reviewers.has(login(r))) { entry.why = `${where(r)}: not a reviewer listed in .github/agent/review-policy.json`; continue }
     entry.counts = true
   }
 
-  const counting = judged.filter((e) => e.counts)
-  if (!counting.length) {
-    const reason = reviewers.size
-      ? reasons.length ? reasons.join('; ') : `no Verifier report names the tip ${headSha.slice(0, SHORT)}`
-      : 'no reviewer is listed in .github/agent/review-policy.json'
+  if (!reviewers.size) return { ok: false, verdict: null, by: null, tip: null, reason: 'no reviewer is listed in .github/agent/review-policy.json', judged }
+  if (!atTip.length) {
+    const reason = reasons.length ? reasons.join('; ') : `no Verifier report names the tip ${headSha.slice(0, SHORT)}`
     return { ok: false, verdict: null, by: null, tip: null, reason, judged }
   }
 
-  // Newest first: the reviewer's latest word at this tip decides. Equal timestamps fall back to
-  // the order the API returned them in (oldest first), so the later comment still wins.
-  const last = counting
+  // The newest word at this tip decides, and that includes a conclusion the checks refused: an invalid
+  // PASS is not evidence, and an invalid INC is still someone saying not to merge (#676). Equal
+  // timestamps fall back to the order the API returned them in (oldest first), so the later one wins.
+  const last = atTip
     .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 }
   const { parsed } = last
-  if (last.verdict === 'PASS' && !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 }
-  }
   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 }
   }
+  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: true, verdict: 'PASS', by: last.login, tip: last.tip, reason: `${last.login}'s PASS names this tip`, judged }
 }
 
@@ -170,7 +215,7 @@ export function render(result, { pr, head, author } = {}) {
     `Pull request #${pr}, author ${author}, head \`${String(head).slice(0, SHORT)}\`.`,
   ]
   const seen = result.judged.filter((e) => e.counts || e.verdict)
-  if (seen.length) out.push('', 'Verifier reports read:', ...seen.slice(0, 10).map((e) => `- ${e.login} (${e.kind}): ${e.verdict ?? 'no verdict'} at ${(e.tip ?? 'no tip').slice(0, SHORT)}${e.counts ? '' : ' (not counted)'}`))
+  if (seen.length) out.push('', 'Verifier reports read:', ...seen.slice(0, 10).map((e) => `- ${e.login} (${e.kind}): ${e.verdict ?? 'no verdict'} at ${(e.tip ?? 'no tip').slice(0, SHORT)}, written-by ${e.writer ?? 'nothing'}${e.counts ? '' : ' (not counted)'}`))
   out.push('', '_Report only (Phase 1): this does not block, label, approve or merge anything. Rules: `.github/agent/review-policy.json`; see [`docs/autonomous-dev.md`](../../docs/autonomous-dev.md)._')
   return out.join('\n')
 }
@@ -200,10 +245,10 @@ function main() {
   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, body: c.body, at: c.created_at, url: c.html_url })),
+    ...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, 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, body: v.body, at: v.submitted_at, url: v.html_url })),
+      .map((v) => ({ kind: 'review', login: v.user?.login, body: v.body, at: v.submitted_at, association: v.author_association, url: v.html_url })),
   ]
 
   const result = decide({ head, author, reports, policy: loadPolicy(policyPath) })