From cec4515c2d21369291f11ce94ab8d59c3c514d7b Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Thu, 22 Oct 2026 19:18:44 +0800 Subject: [PATCH] Let Reviewer2 record a report the gate counts (#627) The gate reads `writers` from .github/agent/review-policy.json and refuses a `Written-by:` line that names something outside the list, so every PASS Reviewer2 wrote today was refused. The team gained that Bot after the file was last touched, and nothing noticed: the refusal only shows up when a T2/T3 pull request waits for its conclusion. The test now writes the roster out and checks each Bot in it: the name must be in `writers`, and a PASS carrying it must count. A Bot added to the team later, or a name dropped from the policy, turns the test red. --- .github/agent/review-policy.json | 1 + .github/scripts/review-gate.test.mjs | 26 ++++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/.github/agent/review-policy.json b/.github/agent/review-policy.json index 648a8cb9d..e57e565b4 100644 --- a/.github/agent/review-policy.json +++ b/.github/agent/review-policy.json @@ -7,6 +7,7 @@ "Chief", "Triage", "Reviewer", + "Reviewer2", "Release", "SelfTest", "Maintainer", diff --git a/.github/scripts/review-gate.test.mjs b/.github/scripts/review-gate.test.mjs index 75efa5ac5..ac9c4e21f 100644 --- a/.github/scripts/review-gate.test.mjs +++ b/.github/scripts/review-gate.test.mjs @@ -8,10 +8,10 @@ const HEAD = 'a4d506a1b2c3d4e5f60718293a4b5c6d7e8f9012' const OLD = '175328d0c1b2a3d4e5f60718293a4b5c6d7e8f90' const policy = { reviewers: ['reviewer-agent', 'hsliuustc0106'], writers: ['Reviewer', 'Chief'] } -const report = ({ tip = HEAD, verdict = 'PASS', ran = [' - npm run verify → pass'], notRun = true } = {}) => +const report = ({ tip = HEAD, verdict = 'PASS', ran = [' - npm run verify → pass'], notRun = true, writer = 'Reviewer' } = {}) => [ `Tip SHA: ${tip}`, - 'Written-by: Reviewer', + `Written-by: ${writer}`, 'Ran:', ...ran, ...(notRun ? ['Not run:', ' - smoke — host code unchanged'] : []), @@ -166,6 +166,28 @@ test('the policy file lists at least one reviewer', () => { for (const login of p.selfReview ?? []) assert.ok(reviewers.includes(login.toLowerCase()), `selfReview ${login} must also be a reviewer`) }) +// #627: every Bot on the team may carry a `Written-by:` line. The roster is written out on +// purpose: when the team gains a Bot, this list and the policy must gain the name together, or that +// Bot's PASS is refused and nobody notices until it is needed (Reviewer2, 2026-10-22). Dropping a +// name from the policy turns this red; so does adding a Bot to the team and not to `writers`. +const TEAM_WRITERS = [ + 'Chief', 'Triage', 'Reviewer', 'Reviewer2', 'Release', 'SelfTest', 'Maintainer', + 'Dev', 'Dev2', 'Dev3', 'Dev4', 'Birfy', +] + +test('the policy lets every Bot on the team write a report', () => { + const p = loadPolicy(new URL('../agent/review-policy.json', import.meta.url)) + const writers = new Set((p.writers ?? []).map((w) => String(w).toLowerCase())) + const missing = TEAM_WRITERS.filter((name) => !writers.has(name.toLowerCase())) + assert.deepEqual(missing, [], `these Bots would have every PASS refused: ${missing.join(', ')}`) + const gate = { ...p, reviewers: ['reviewer-agent'] } + for (const name of TEAM_WRITERS) { + const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('reviewer-agent', report({ writer: name }))], policy: gate }) + assert.equal(r.ok, true, `a PASS written by ${name} must count`) + assert.equal(r.by, 'reviewer-agent') + } +}) + test('workflow: pull_request, read-only, not required, acts on nothing', () => { const wf = readFileSync(new URL('../workflows/review-gate.yml', import.meta.url), 'utf8') const testYml = readFileSync(new URL('../workflows/test.yml', import.meta.url), 'utf8')