chore(ci): report a review round that calls someone other than the declared reviewer (#911)

.github/scripts/agent-audience.mjs

@@ -0,0 +1,136 @@
+// agent-audience (#911): a pull request names its owner and its reviewer in the body, and the review
+// round calls that reviewer. When a round calls somebody else, the named reviewer is never asked and
+// the pull request sits with no verdict while everyone believes review is in flight — #907 did that
+// for four tips. This is a report, not a gate: it reads the two body lines and the comments written
+// after the current tip, and says where they disagree. Silence is not detectable here (the repository
+// has no "who read this" field); a comment that calls nobody is invisible to this check.
+//
+//   node .github/scripts/agent-audience.mjs --repo ThinkFlowLab/FlyBot --pr 907
+//   node .github/scripts/agent-audience.mjs --repo ThinkFlowLab/FlyBot        # every open PR
+//
+// Report only: the process exits 0 whatever it finds. Every line has the shape
+// `#<n> @<sha7>: <sentence>` so a round can read it without opening the pull request.
+import { execFileSync } from 'node:child_process'
+import { readFileSync } from 'node:fs'
+import { fileURLToPath } from 'node:url'
+
+export const POLICY_PATH = new URL('../agent/review-policy.json', import.meta.url)
+// The Chief escalates a stalled review; its call is not a wrong call.
+export const ALWAYS_ALLOWED = ['Chief']
+
+const escape = value => String(value).replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+
+export function loadPolicy(path = POLICY_PATH) {
+  return JSON.parse(readFileSync(path, 'utf8'))
+}
+
+// The roster the review policy already carries. A declared name outside it is a typo or a member who
+// cannot write a verdict at all, and either way the round calls nobody who can answer.
+export function rosterOf(policy = {}) {
+  const names = new Set()
+  for (const key of ['writers', 'reviewers', 'maintainers']) {
+    for (const name of policy[key] ?? []) if (typeof name === 'string' && name) names.add(name)
+  }
+  for (const name of policy.selfReview ?? []) if (typeof name === 'string' && name) names.add(name)
+  return names
+}
+
+// `Owner: Dev2` / `Reviewer: Reviewer2` on their own line. Deliberately not a search anywhere in the
+// body: quoting the requirement in prose must not satisfy it.
+export function parseDeclarations(body = '') {
+  const line = name => {
+    const match = new RegExp(`^${name}:\\s*(\\S+)\\s*$`, 'm').exec(String(body ?? ''))
+    return match ? match[1] : undefined
+  }
+  return { owner: line('Owner'), reviewer: line('Reviewer') }
+}
+
+// The policy names a member's `@` handle exactly, so a mention is matched whole: `@Reviewer` must not
+// match inside `@Reviewer2`, and a handle longer than a roster name is not that member.
+export function mentionsIn(text = '', names) {
+  const found = []
+  for (const name of names) {
+    const pattern = new RegExp(`(^|[^A-Za-z0-9_.-])@${escape(name)}(?![A-Za-z0-9_.-])`, 'g')
+    if (pattern.test(String(text ?? ''))) found.push(name)
+  }
+  return found.sort()
+}
+
+const sha7 = sha => String(sha ?? '').slice(0, 7)
+const report = (pr, sentence) => `#${pr.number} @${sha7(pr.headSha)}: ${sentence}`
+
+// One report per defect, not per comment: three calls to the wrong member are one disagreement.
+export function evaluate({ pr, comments = [], policy = {}, tipCommittedAt, allow = ALWAYS_ALLOWED } = {}) {
+  const { owner, reviewer } = parseDeclarations(pr.body)
+  const roster = rosterOf(policy)
+  const out = []
+  const missing = []
+  if (!owner) missing.push('`Owner:`')
+  if (!reviewer) missing.push('`Reviewer:`')
+  if (missing.length) out.push(report(pr, `pull request body has no ${missing.join(' and ')} line`))
+  for (const [label, name] of [['Owner', owner], ['Reviewer', reviewer]]) {
+    if (!name || roster.has(name)) continue
+    out.push(report(pr, `${label}: names \`${name}\`, which is not in the review-policy roster`))
+  }
+  // Only calls written after the tip can be about this review round; an earlier call belongs to the
+  // tip it was written for, and its tip is already void.
+  const since = tipCommittedAt ? Date.parse(tipCommittedAt) : 0
+  const calls = new Map()
+  for (const comment of comments) {
+    const at = Date.parse(comment.createdAt ?? '')
+    if (!Number.isFinite(at) || (Number.isFinite(since) && since > 0 && at <= since)) continue
+    for (const name of mentionsIn(comment.body, roster)) {
+      if (name === reviewer || allow.includes(name)) continue
+      const entry = calls.get(name) ?? { count: 0, ids: [] }
+      entry.count += 1
+      if (comment.id !== undefined) entry.ids.push(comment.id)
+      calls.set(name, entry)
+    }
+  }
+  for (const [name, call] of [...calls].sort(([a], [b]) => a.localeCompare(b))) {
+    const where = call.ids.length ? ` (comment ${call.ids.join(', ')})` : ''
+    const named = reviewer ? `the body names \`Reviewer: ${reviewer}\`` : 'the body names no `Reviewer:`'
+    out.push(report(pr, `${call.count} call${call.count === 1 ? '' : 's'} to @${name} after this tip${where}, but ${named}`))
+  }
+  return out
+}
+
+const realGh = (...args) => execFileSync('gh', args, { encoding: 'utf8', maxBuffer: 16 << 20 })
+
+// `gh` and `env` are injectable so the run can be driven from fixtures, the way label-hygiene.mjs is.
+export function main({ gh = realGh, env = process.env, repo: repoOption, pr: prOption, policy = loadPolicy() } = {}) {
+  const arg = (name) => { const i = process.argv.indexOf(name); return i > 0 ? process.argv[i + 1] : undefined }
+  const repo = repoOption || env.GH_REPO || arg('--repo')
+  if (!repo) throw new Error('set GH_REPO or pass --repo owner/name')
+  const single = prOption ?? arg('--pr')
+  const page = (path) => gh('api', '--paginate', '--jq', '.[]', path).split('\n').filter(Boolean).map(line => JSON.parse(line))
+
+  const numbers = single
+    ? [Number(single)]
+    : page(`repos/${repo}/pulls?state=open&per_page=100`).map(pull => pull.number)
+  const reports = []
+  for (const number of numbers) {
+    const pull = JSON.parse(gh('api', `repos/${repo}/pulls/${number}`))
+    const comments = page(`repos/${repo}/issues/${number}/comments?per_page=100`).map(c => ({ id: c.id, body: c.body, createdAt: c.created_at }))
+    const tipCommittedAt = gh('api', '--jq', '.commit.committer.date', `repos/${repo}/commits/${pull.head.sha}`).trim()
+    reports.push(...evaluate({
+      pr: { number, body: pull.body, headSha: pull.head.sha },
+      comments,
+      policy,
+      tipCommittedAt,
+    }))
+  }
+  for (const line of reports) console.log(line)
+  console.log(`agent-audience: ${reports.length} finding(s) on ${numbers.length} pull request(s); report only`)
+  const summary = env.GITHUB_STEP_SUMMARY
+  if (summary) {
+    const body = reports.length ? `${reports.join('\n')}\n` : 'agent-audience: no findings\n'
+    // Best effort: a missing summary must not change the check's result.
+    try { execFileSync('sh', ['-c', `cat >> "$1"`, 'sh', summary], { input: `## agent-audience\n\n${body}`, encoding: 'utf8' }) } catch {}
+  }
+  return { ok: true, reports, numbers }
+}
+
+if (process.argv[1] === fileURLToPath(import.meta.url)) {
+  try { main() } catch (error) { console.error(`agent-audience: ${error.message}`); process.exit(1) }
+}

.github/scripts/agent-audience.test.mjs

@@ -0,0 +1,131 @@
+// agent-audience (#911): the check reads the two lines a pull request body must carry and the calls
+// written after its current tip, and reports where they disagree. The first case replays the real
+// #907 record: the body of that pull request carried neither line, and the review rounds called
+// `@Reviewer` three times while the declared reviewer (in the team chat) was Reviewer2. Those calls
+// were written in the delivery group rather than on the pull request, which is why the replay feeds
+// them as comment bodies; the live run sees only the GitHub comments, and reports the missing lines.
+import { test } from 'node:test'
+import assert from 'node:assert/strict'
+import { readFileSync } from 'node:fs'
+import { evaluate, loadPolicy, main, mentionsIn, parseDeclarations, rosterOf } from './agent-audience.mjs'
+
+const policy = loadPolicy()
+const TIP = '0c7b33a7b817eddc2c15c7edaeb20eca9f938c85'
+const TIP_AT = '2026-10-24T01:27:48Z'
+
+// The first lines of the real #907 body: no `Owner:`/`Reviewer:` anywhere.
+const BODY_907 = `Written-by: Dev3
+
+Fixes #904
+
+A guest's copy of a linked room now lists the host's Bots and shows the room's group notice.`
+
+// The three rounds that called `@Reviewer` for #907, at the times the group recorded them.
+const callsTo907 = [
+  { id: 9101, createdAt: '2026-10-24T01:27:56Z', body: '@Reviewer 请按 `0c7b33a7` 判' },
+  { id: 9102, createdAt: '2026-10-24T01:28:30Z', body: '@Reviewer 这张归你判，不是 Reviewer2' },
+  { id: 9103, createdAt: '2026-10-24T01:29:10Z', body: '@Reviewer 等结论' },
+]
+
+const pr907 = { number: 907, body: BODY_907, headSha: TIP }
+
+test('#911: the #907 record is two findings — no declared reviewer, and three calls to @Reviewer', () => {
+  const reports = evaluate({ pr: pr907, comments: callsTo907, policy, tipCommittedAt: TIP_AT })
+  assert.deepEqual(reports, [
+    '#907 @0c7b33a: pull request body has no `Owner:` and `Reviewer:` line',
+    '#907 @0c7b33a: 3 calls to @Reviewer after this tip (comment 9101, 9102, 9103), but the body names no `Reviewer:`',
+  ])
+})
+
+test('#911: negative control — the declared reviewer being called produces no finding', () => {
+  const body = `Owner: Dev3\nReviewer: Reviewer2\n\nFixes #904\nTip SHA: ${TIP}`
+  const comments = [
+    { id: 9201, createdAt: '2026-10-24T01:28:00Z', body: '@Reviewer2 please judge `0c7b33a7`' },
+    { id: 9202, createdAt: '2026-10-24T01:28:40Z', body: 'Written-by: Reviewer2 — running the suite now' },
+  ]
+  assert.deepEqual(evaluate({ pr: { number: 904, body, headSha: TIP }, comments, policy, tipCommittedAt: TIP_AT }), [])
+})
+
+test('#911: a call written before the current tip belongs to the tip it was written for', () => {
+  const body = `Owner: Dev3\nReviewer: Reviewer2`
+  const before = [{ id: 9301, createdAt: '2026-10-24T00:10:00Z', body: '@Reviewer old round' }]
+  assert.deepEqual(evaluate({ pr: { number: 5, body, headSha: TIP }, comments: before, policy, tipCommittedAt: TIP_AT }), [])
+})
+
+test('#911: Chief is the escalation exit, not a wrong call', () => {
+  const body = `Owner: Dev3\nReviewer: Reviewer2`
+  const comments = [{ id: 9401, createdAt: '2026-10-24T01:30:00Z', body: '@Chief this PR has waited four rounds' }]
+  assert.deepEqual(evaluate({ pr: { number: 5, body, headSha: TIP }, comments, policy, tipCommittedAt: TIP_AT }), [])
+})
+
+test('#911: a name outside the review policy is its own finding', () => {
+  const body = `Owner: Dev3\nReviewer: Reviewer3`
+  const reports = evaluate({ pr: { number: 5, body, headSha: TIP }, comments: [], policy, tipCommittedAt: TIP_AT })
+  assert.deepEqual(reports, ['#5 @0c7b33a: Reviewer: names `Reviewer3`, which is not in the review-policy roster'])
+})
+
+test('#911: @Reviewer2 is not read as @Reviewer, and the body lines are matched whole', () => {
+  assert.deepEqual(mentionsIn('@Reviewer2 please judge', new Set(['Reviewer', 'Reviewer2'])), ['Reviewer2'])
+  assert.deepEqual(mentionsIn('cite @Reviewer.example in text', new Set(['Reviewer'])), [])
+  assert.deepEqual(parseDeclarations('Owner: Dev2\nReviewer: Reviewer2'), { owner: 'Dev2', reviewer: 'Reviewer2' })
+  // Prose that quotes the requirement does not satisfy it.
+  assert.deepEqual(parseDeclarations('Write `Owner: Dev2` and `Reviewer: Reviewer2` in the body'), { owner: undefined, reviewer: undefined })
+  assert.deepEqual(parseDeclarations('Owner: Dev2 and Reviewer: Reviewer2 on one line'), { owner: undefined, reviewer: undefined })
+})
+
+test('#911: the roster is the policy file, not a second list', () => {
+  const roster = rosterOf(policy)
+  for (const name of ['Chief', 'Dev2', 'Reviewer2', 'Birfy']) assert.ok(roster.has(name), name)
+  assert.equal(roster.has('Reviewer3'), false)
+})
+
+// A fake `gh` over recorded API answers, the way label-hygiene.mjs is tested.
+const fakeGh = (answers) => {
+  const calls = []
+  const gh = (...args) => {
+    calls.push(args.join(' '))
+    const path = args[args.length - 1]
+    const key = Object.keys(answers).find(prefix => path.startsWith(prefix))
+    if (!key) throw new Error(`unexpected gh call: ${args.join(' ')}`)
+    const value = answers[key]
+    return typeof value === 'function' ? value(args) : value
+  }
+  gh.calls = calls
+  return gh
+}
+
+test('#911: the run prints one line per finding and exits success by design', () => {
+  const gh = fakeGh({
+    'repos/x/y/pulls/907': JSON.stringify({ number: 907, body: BODY_907, head: { sha: TIP } }),
+    'repos/x/y/issues/907/comments': callsTo907.map(c => JSON.stringify({ id: c.id, body: c.body, created_at: c.createdAt })).join('\n'),
+    'repos/x/y/commits/': `${TIP_AT}\n`,
+  })
+  const lines = []
+  const original = console.log
+  console.log = (...args) => lines.push(args.join(' '))
+  try {
+    const result = main({ gh, env: {}, repo: 'x/y', pr: '907', policy })
+    assert.equal(result.ok, true)
+    assert.equal(result.reports.length, 2)
+  } finally {
+    console.log = original
+  }
+  assert.match(lines[0], /^#907 @0c7b33a: /)
+  assert.match(lines.at(-1), /2 finding\(s\) on 1 pull request\(s\); report only/)
+})
+
+// #912: this check passed on its own pull request while reading nothing. The workflow checked the
+// script out of `main`, where it does not exist yet, `node` could not load it, and `continue-on-error`
+// turned that crash into a green check -- a check that passes without reading is the failure this job
+// exists to report. It now reads the script from the base commit (a pull request must not swap the
+// checker that audits it) and falls back to the pull request for the run that introduces it. Either
+// half disappearing turns this red, so the guard names both.
+test('#911: the workflow cannot pass without having read the checker', () => {
+  const yml = readFileSync(new URL('../workflows/agent-audience.yml', import.meta.url), 'utf8')
+  assert.match(yml, /ref: \$\{\{ github\.event\.pull_request\.base\.sha \|\| github\.event\.repository\.default_branch \}\}/)
+  assert.match(yml, /if: hashFiles\('base\/\.github\/scripts\/agent-audience\.mjs'\) == ''/)
+  assert.match(yml, /dir="pr"/)
+  assert.match(yml, /node "\$dir\/\.github\/scripts\/agent-audience\.mjs"/)
+  assert.doesNotMatch(yml, /node \.github\/scripts\/agent-audience\.mjs/)
+  assert.doesNotMatch(yml, /^\s+ref: main$/m)
+})

.github/workflows/agent-audience.yml

@@ -0,0 +1,77 @@
+name: Agent audience
+
+# A pull request names its owner and its reviewer in the body, and the review round calls that
+# reviewer. A round that calls somebody else — or a body that names nobody — leaves the pull request
+# waiting for a verdict while it looks like review is in flight (#911: #907 sat that way for four
+# tips, and every call went to a member who was not the declared reviewer).
+#
+# This is a report, not a gate: `continue-on-error` keeps the check green whatever it finds, it only
+# reads, and it writes the findings into the job summary. It never comments and never labels.
+# Silence itself is not detectable (the repository has no "who read this" field); only a call to the
+# wrong member, or a missing declaration, is.
+#
+# The one way this check can lie is by passing without reading anything: `continue-on-error` turns a
+# crash into a green check, so the script must always be found. It is therefore checked out from the
+# BASE commit -- a pull request must not be able to change the checker that audits it -- with the
+# first-run fallback review-gate.yml uses: the pull request that introduces the script has no copy on
+# the base yet, so that one run reads the copy in the pull request, under the same read-only token.
+on:
+  pull_request:
+    types: [opened, edited, synchronize, reopened, ready_for_review]
+  issue_comment:
+    types: [created]
+  workflow_dispatch:
+
+concurrency:
+  group: agent-audience-${{ github.event.pull_request.number || github.event.issue.number || 'all' }}
+  cancel-in-progress: true
+
+permissions:
+  contents: read
+  issues: read
+  pull-requests: read
+
+jobs:
+  audience:
+    name: agent-audience
+    # A comment on a plain issue is not a pull request.
+    if: github.event_name != 'issue_comment' || github.event.issue.pull_request != null
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
+        with:
+          ref: ${{ github.event.pull_request.base.sha || github.event.repository.default_branch }}
+          path: base
+          persist-credentials: false
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
+        if: hashFiles('base/.github/scripts/agent-audience.mjs') == ''
+        with:
+          ref: ${{ github.event.pull_request.head.sha || github.event.repository.default_branch }}
+          path: pr
+          persist-credentials: false
+      - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1  # v7.1.0
+        with:
+          node-version: 24
+      - name: Read the declared audience against the calls written this round
+        continue-on-error: true
+        env:
+          GH_TOKEN: ${{ github.token }}
+          GH_REPO: ${{ github.repository }}
+          PR: ${{ github.event.pull_request.number || github.event.issue.number }}
+        run: |
+          dir="base"
+          if [ ! -f base/.github/scripts/agent-audience.mjs ]; then
+            echo "::notice::The base commit has no agent-audience script yet; using the copy in this pull request."
+            dir="pr"
+          fi
+          if [ ! -f "$dir/.github/scripts/agent-audience.mjs" ]; then
+            # Loud on purpose: a green check that read nothing is the failure this job exists to catch.
+            echo "::error::agent-audience: the checker is on neither the base commit nor this pull request; nothing was read."
+            exit 1
+          fi
+          if [ -n "$PR" ]; then
+            node "$dir/.github/scripts/agent-audience.mjs" --pr "$PR"
+          else
+            node "$dir/.github/scripts/agent-audience.mjs"
+          fi

docs/testing-layers.json

@@ -228,6 +228,15 @@
       "gate": false,
       "runs": "pull_request_review (submitted)",
       "reason": "Says, in one warning and the run summary, that a PASS posted as a review body whose Written-by names someone outside the policy's writers does not count (#801). Read-only by construction (permissions: contents: read), so a pull request that edits it can only change what its own review job prints; a workflow-only layer with no gate."
+    },
+    {
+      "id": "agent-audience",
+      "workflow": ".github/workflows/agent-audience.yml",
+      "kind": "workflow-only",
+      "layer": "none",
+      "gate": false,
+      "runs": "pull_request (opened, edited, synchronize, reopened, ready_for_review), issue_comment (created) and workflow_dispatch",
+      "reason": "Reads the body's Owner:/Reviewer: lines and the calls written after the current tip, and prints where they disagree (#911). Report only: continue-on-error, read-only permissions, no comment and no label, so it gates nothing."
     }
   ]
 }