Skip to content

Commit 69a9f30

Browse files
committed
fix(gate): count a listed self-reviewer's PASS on its own pull request
The association gate ran before selfReview was consulted, so a login listed there still counted for nothing if GitHub called it a CONTRIBUTOR: #926 adds exactly that login and its PASS stayed not-a-conclusion. One exemption, only when the login is the pull request's own author; every other check is unchanged, and the same login judging someone else's pull request is still refused like any outsider. Fixes #928 Docs-not-needed: no docs/subsystems page owns .github/scripts/, AGENTS.md or docs/autonomous-dev.md (checked against their owned_paths); the rule itself is updated in AGENTS.md and docs/autonomous-dev.md in this branch.
1 parent 137cd2f commit 69a9f30

4 files changed

Lines changed: 32 additions & 4 deletions

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,12 @@ export function decide({ head, author, reports, policy }) {
159159
}
160160
if (!parsed.verdict) continue // a `Tip SHA` block with no verdict is not a conclusion
161161
const association = String(r.association ?? '').toUpperCase()
162-
if (!ASSOCIATIONS.has(association)) {
162+
// #928 (owner decision 2026-10-24): a login the policy lists in `selfReview` may be a CONTRIBUTOR
163+
// and still count, but only on its own pull request. That is one exception for one person, not a
164+
// second reviewers list: the same login judging someone else's pull request keeps every rule
165+
// below, and #676 still refuses a login nobody listed.
166+
const selfOnly = selfReview.has(login(r)) && login(r) === authorLogin
167+
if (!ASSOCIATIONS.has(association) && !selfOnly) {
163168
reasons.push(`${where(r)}: author association ${association || 'missing'} is not OWNER, MEMBER or COLLABORATOR, so it is not a conclusion`)
164169
continue
165170
}

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,3 +493,26 @@ test('#676: a conclusion from outside the repository counts for nothing', () =>
493493
assert.equal(missing.ok, false, 'an association the API did not send is not assumed')
494494
assert.match(missing.reason, /author association missing/)
495495
})
496+
497+
// #928 (owner decision 2026-10-24): a login the policy lists in `selfReview` may be a CONTRIBUTOR and
498+
// still count, but only on its own pull request. One exception for one person, never a second
499+
// reviewers list: the same login on someone else's pull request stays outside the association rule,
500+
// and #676 still refuses everyone nobody listed.
501+
test('#928: a listed self-reviewer who is a CONTRIBUTOR counts, only on their own pull request', () => {
502+
const selfPolicy = { reviewers: ['reviewer-agent', 'hsliuustc0106'], selfReview: ['hsliuustc0106'], writers: ['Reviewer', 'hsliuustc0106'] }
503+
const own = (login = 'hsliuustc0106') => comment(login, report({ writer: 'hsliuustc0106' }), '2026-10-18T01:00:00Z', 'comment', 'CONTRIBUTOR')
504+
505+
const mine = decide({ head: HEAD, author: 'hsliuustc0106', reports: [own()], policy: selfPolicy })
506+
assert.equal(mine.ok, true, 'listed in selfReview on its own pull request: the PASS counts')
507+
assert.equal(mine.by, 'hsliuustc0106')
508+
assert.equal(mine.judged[0].counts, true)
509+
510+
const unlisted = decide({ head: HEAD, author: 'opener', reports: [own('passer-by')], policy: selfPolicy })
511+
assert.equal(unlisted.ok, false, 'a CONTRIBUTOR nobody listed is still not a conclusion')
512+
assert.match(unlisted.reason, /author association CONTRIBUTOR/)
513+
assert.equal(unlisted.judged[0].counts, false)
514+
515+
const elsewhere = decide({ head: HEAD, author: 'opener', reports: [own()], policy: selfPolicy })
516+
assert.equal(elsewhere.ok, false, 'a listed self-reviewer judging someone else\'s pull request is not exempt')
517+
assert.equal(elsewhere.judged[0].counts, false)
518+
})

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ The owner kept one shared GitHub identity for every Bot (2026-10-21), so attribu
147147

148148
The gate refuses what it can check, and fails closed. [`review-gate.mjs`](.github/scripts/review-gate.mjs) — and [`merge-gate.mjs`](.github/scripts/merge-gate.mjs), which imports its `decide`, so a queue entry is judged by the same rule — refuses a report that carries **no** `Written-by:` line, and refuses one whose name is **not** in the policy's `writers` list. Two separate refusals, so the log says which defect fired. Triage names a write that carries no such line in one comment and never repairs it by editing someone else's text.
149149

150-
The gate reads a conclusion only from someone GitHub ties to this repository: the comment's `author_association` must be `OWNER`, `MEMBER` or `COLLABORATOR`, or it is not a conclusion and counts for nothing — otherwise any passer-by could hold a merge with one `Verdict: INC` line, because the newest conclusion decides (#676). An association the API did not send is not assumed.
150+
The gate reads a conclusion only from someone GitHub ties to this repository: the comment's `author_association` must be `OWNER`, `MEMBER` or `COLLABORATOR`, or it is not a conclusion and counts for nothing — otherwise any passer-by could hold a merge with one `Verdict: INC` line, because the newest conclusion decides (#676). An association the API did not send is not assumed. One exception (owner decision 2026-10-24, #928): a login the policy lists in `selfReview` may be a `CONTRIBUTOR` and still count, but **only on its own pull request** — the same login on someone else's pull request is refused like any other outsider, and every other check (tip, `Written-by:`, `Ran:`, reviewer list, newest word) is unchanged.
151151

152152
What this buys is presence and a name from a closed list, not truth: with one shared identity the line is a self-report that can be left out (refused) and can name the wrong Bot (undetectable here). The write layer knowing the writer is the DSH-side half, #633.
153153

‎docs/autonomous-dev.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ What counts as a reviewed tip, all of it:
144144
Bot/agent, so no independent reviewer is required. That is an exception about the shared *account*, not about the
145145
Bot: an implementer never reviews its own pull request ([`AGENTS.md` → Merge gates](../AGENTS.md#merge-gates)). Any
146146
other author still needs someone else. A PASS does not replace green CI, and a new commit voids it.
147-
3. The conclusion format is fixed (#637): the **first line** is `Verdict: PASS at <sha>` (or `INC` / `REQUEST_CHANGES`), the second is the attribution (`Written-by: Reviewer`; a relay adds `Posted-by: Chief (proxy…)` on the third line, and the older prose `Written by: …` reads the same way), then the evidence — `Ran:` (or a `Ran …:` heading over a fenced block of `$ ` commands, #660–#663) with at least one command; `Not run:` naming any gap is recommended but not checked (owner decision 2026-10-22 09:57 Asia/Shanghai). A body whose first line is not the verdict is no conclusion. The comment's `author_association` must be `OWNER`, `MEMBER` or `COLLABORATOR`: a conclusion from anyone else counts for nothing, so it can neither pass a tip nor hold one (#676).
147+
3. The conclusion format is fixed (#637): the **first line** is `Verdict: PASS at <sha>` (or `INC` / `REQUEST_CHANGES`), the second is the attribution (`Written-by: Reviewer`; a relay adds `Posted-by: Chief (proxy…)` on the third line, and the older prose `Written by: …` reads the same way), then the evidence — `Ran:` (or a `Ran …:` heading over a fenced block of `$ ` commands, #660–#663) with at least one command; `Not run:` naming any gap is recommended but not checked (owner decision 2026-10-22 09:57 Asia/Shanghai). A body whose first line is not the verdict is no conclusion. The comment's `author_association` must be `OWNER`, `MEMBER` or `COLLABORATOR`: a conclusion from anyone else counts for nothing, so it can neither pass a tip nor hold one (#676). One exception (owner decision 2026-10-24, #928): a login the policy lists in `selfReview` may be a `CONTRIBUTOR` and still count, but **only on its own pull request** — the same login judging someone else's pull request is refused like any other outsider, and every other check is unchanged.
148148
4. The sha on that first line **is the head being judged**. A later commit moves the tip and the old PASS stops
149149
counting; that report is then listed as "not counted", with the sha it named.
150150
5. The **newest** report that names this tip decides, so a later FAIL at the same tip vetoes an
@@ -160,7 +160,7 @@ makes the line a self-report, so the gate guarantees presence, not truth (#633).
160160
| --- | --- |
161161
| `.github/agent/review-policy.json` | the logins whose PASS counts, `selfReview`, the authors who may PASS their own PR, and `writers`, the Bot names a `Written-by:` line may carry (#627); editing it is T3 |
162162
| `.github/scripts/review-gate.mjs` | the rule above, for one pull request at a time |
163-
| `.github/scripts/review-gate.test.mjs` | stale tip, self-review (allowed only for a `selfReview` author), unlisted login, FAIL after PASS, missing `Ran:` (a missing `Not run:` no longer refuses), a missing `Written-by:` line and one naming an unlisted writer (#627), and the workflow's shape |
163+
| `.github/scripts/review-gate.test.mjs` | stale tip, self-review (allowed only for a `selfReview` author), unlisted login, FAIL after PASS, missing `Ran:` (a missing `Not run:` no longer refuses), a missing `Written-by:` line and one naming an unlisted writer (#627), a `selfReview` login with a `CONTRIBUTOR` association counting only on its own pull request (#928), and the workflow's shape |
164164
| `.github/workflows/review-gate.yml` | reports it in the job summary; `continue-on-error`, never required |
165165

166166
Run it by hand (read-only API calls with your `gh` login):

0 commit comments

Comments
 (0)