Skip to content

review-gate: a CONTRIBUTOR listed in selfReview cannot back its own PASS, because the association check runs first #928

Closed

Description

@Birfy

What happens

review-policy.json is supposed to be able to let a login back a Verdict: on its own pull request (selfReview), but a CONTRIBUTOR never reaches that check: the author_association gate runs first and drops the report.

Evidence, not inference — the enqueue-guard output on the head of #926 (b80823ef, one line added to selfReview) says verbatim:

review by hsliuustc0106: author association CONTRIBUTOR is not OWNER, MEMBER or COLLABORATOR, so it is not a conclusion

Replaying that head's real comments and reviews through decide() gives ok=false with main's policy and with the new selfReview alike, so the line added in #926 cannot change the answer on its own.

Why

On main 137cd2fd, .github/scripts/review-gate.mjs reads the list at :144 (selfReview), but the association gate at :162-165 — ASSOCIATIONS is :36 — refuses the report before anything uses it; the self-review author check that would consult selfReview is at :178. Order is the whole bug.

What to change

Exempt exactly one case from the :162 gate: the report's login is listed in selfReview and is the pull request's own author (login(r) === authorLogin). Everything else stays: hold, tip match, merge_group, Written-by in writers, and the reviewers list are untouched, and any other CONTRIBUTOR is refused exactly as today (#676: a conclusion must not be anyone's to write).

Owner decision this implements: 2026-10-24, recorded on #926 in #926 (comment) — the grant is "a Verdict that counts", never a merge right, and it is to live in the repository so it can be reviewed, tested and reverted (the alternative, making the account look like a COLLABORATOR through GitHub settings, was rejected).

Acceptance

  1. A report from a login listed in selfReview, on its own pull request, with author_association: CONTRIBUTOR → counts: true (a Verdict: at the tip).
  2. The same report from a login not listed in selfReview → counts: false, with the association reason unchanged.
  3. A listed login reporting on someone else's pull request as CONTRIBUTOR → still counts: false (the exemption is self-review, not a second reviewers list).
  4. Mutations: removing the exemption turns (1) red; widening it to "any listed login, any pull request" turns (3) red.
  5. .github/scripts/review-gate.test.mjs keeps passing, and no existing check is relaxed.

Not in scope

The GitHub-side membership change (adding the account so the API answers COLLABORATOR): rejected on 2026-10-24 because the rule would leave the repository. #926 itself stays a one-line JSON change and is not to carry code from this issue; it merges only once this lands, otherwise the policy would read as if it took effect when it did not.

Activity

  1. added
    bugSomething isn't working
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    area:automationScheduled tasks and automation
    on Oct 24, 2026
  2. Birfy commented on Oct 24, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Dev3

    Claiming this one. Branch: fix/928-contributor-self-review, from current main 137cd2fd.

    The change is the one the issue names and nothing else: in .github/scripts/review-gate.mjs, at the association gate (:162), a login that the policy lists in selfReview and that equals the pull request's own author is exempt from the OWNER/MEMBER/COLLABORATOR check. Every other check stays: hold, stale tip, Written-by:, Ran:, the reviewer list and the newest-word rule are untouched. A login in selfReview judging someone else's pull request is still refused like any outsider — that is the third case the issue adds, so the exemption cannot become a second reviewers list.

    Three cases in review-gate.test.mjs: listed + CONTRIBUTOR + own pull request → counts: true; unlisted + CONTRIBUTOR → counts: false; listed + CONTRIBUTOR + someone else's pull request → counts: false. The two mutations are both red on that test: removing the exemption, and widening it to "anyone on the list".

  3. added
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 24, 2026
  4. self-assigned this
    on Oct 24, 2026
  5. removed
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    on Oct 24, 2026
  6. added 3 commits that reference this issue on Oct 24, 2026
    dfa25a2
    c812e5d
    d20cceb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:automationScheduled tasks and automationbugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions