diff --git a/.github/scripts/review-gate.mjs b/.github/scripts/review-gate.mjs index 3dc517d9..edd97b35 100644 --- a/.github/scripts/review-gate.mjs +++ b/.github/scripts/review-gate.mjs @@ -159,7 +159,12 @@ export function decide({ head, author, reports, policy }) { } 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)) { + // #928 (owner decision 2026-10-24): a login the policy lists in `selfReview` may be a CONTRIBUTOR + // and still count, but only on its own pull request. That is one exception for one person, not a + // second reviewers list: the same login judging someone else's pull request keeps every rule + // below, and #676 still refuses a login nobody listed. + const selfOnly = selfReview.has(login(r)) && login(r) === authorLogin + if (!ASSOCIATIONS.has(association) && !selfOnly) { reasons.push(`${where(r)}: author association ${association || 'missing'} is not OWNER, MEMBER or COLLABORATOR, so it is not a conclusion`) continue } diff --git a/.github/scripts/review-gate.test.mjs b/.github/scripts/review-gate.test.mjs index ac9c4e21..b7014018 100644 --- a/.github/scripts/review-gate.test.mjs +++ b/.github/scripts/review-gate.test.mjs @@ -493,3 +493,26 @@ test('#676: a conclusion from outside the repository counts for nothing', () => assert.equal(missing.ok, false, 'an association the API did not send is not assumed') assert.match(missing.reason, /author association missing/) }) + +// #928 (owner decision 2026-10-24): a login the policy lists in `selfReview` may be a CONTRIBUTOR and +// still count, but only on its own pull request. One exception for one person, never a second +// reviewers list: the same login on someone else's pull request stays outside the association rule, +// and #676 still refuses everyone nobody listed. +test('#928: a listed self-reviewer who is a CONTRIBUTOR counts, only on their own pull request', () => { + const selfPolicy = { reviewers: ['reviewer-agent', 'hsliuustc0106'], selfReview: ['hsliuustc0106'], writers: ['Reviewer', 'hsliuustc0106'] } + const own = (login = 'hsliuustc0106') => comment(login, report({ writer: 'hsliuustc0106' }), '2026-10-18T01:00:00Z', 'comment', 'CONTRIBUTOR') + + const mine = decide({ head: HEAD, author: 'hsliuustc0106', reports: [own()], policy: selfPolicy }) + assert.equal(mine.ok, true, 'listed in selfReview on its own pull request: the PASS counts') + assert.equal(mine.by, 'hsliuustc0106') + assert.equal(mine.judged[0].counts, true) + + const unlisted = decide({ head: HEAD, author: 'opener', reports: [own('passer-by')], policy: selfPolicy }) + assert.equal(unlisted.ok, false, 'a CONTRIBUTOR nobody listed is still not a conclusion') + assert.match(unlisted.reason, /author association CONTRIBUTOR/) + assert.equal(unlisted.judged[0].counts, false) + + const elsewhere = decide({ head: HEAD, author: 'opener', reports: [own()], policy: selfPolicy }) + assert.equal(elsewhere.ok, false, 'a listed self-reviewer judging someone else\'s pull request is not exempt') + assert.equal(elsewhere.judged[0].counts, false) +}) diff --git a/AGENTS.md b/AGENTS.md index bf61e5a7..12bceb1b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -147,7 +147,7 @@ The owner kept one shared GitHub identity for every Bot (2026-10-21), so attribu 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. -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. +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. 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. diff --git a/docs/autonomous-dev.md b/docs/autonomous-dev.md index 69cd241a..3deff63d 100644 --- a/docs/autonomous-dev.md +++ b/docs/autonomous-dev.md @@ -144,7 +144,7 @@ What counts as a reviewed tip, all of it: Bot/agent, so no independent reviewer is required. That is an exception about the shared *account*, not about the Bot: an implementer never reviews its own pull request ([`AGENTS.md` → Merge gates](../AGENTS.md#merge-gates)). Any other author still needs someone else. A PASS does not replace green CI, and a new commit voids it. -3. The conclusion format is fixed (#637): the **first line** is `Verdict: PASS at ` (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). +3. The conclusion format is fixed (#637): the **first line** is `Verdict: PASS at ` (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. 4. The sha on that first line **is the head being judged**. A later commit moves the tip and the old PASS stops counting; that report is then listed as "not counted", with the sha it named. 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). | --- | --- | | `.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 | | `.github/scripts/review-gate.mjs` | the rule above, for one pull request at a time | -| `.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 | +| `.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 | | `.github/workflows/review-gate.yml` | reports it in the job summary; `continue-on-error`, never required | Run it by hand (read-only API calls with your `gh` login):