Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/scripts/review-gate.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
23 changes: 23 additions & 0 deletions .github/scripts/review-gate.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
4 changes: 2 additions & 2 deletions docs/autonomous-dev.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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).
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.
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
Expand All @@ -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):
Expand Down
Loading