feat: extend self-approval to team membership (self_approval_via_teams)

[daniil-pushin]

Summary / Background

allow_self_approval matches the PR author against individually listed logins only: with a rule like * @org/backend @alice, a backend team member authoring a PR does not satisfy the group, even though an approval from the same person would (approvals are resolved through the user-reviewer map, the author is not).

This implements the behavior requested in #49: an opt-in self_approval_via_teams config option that extends self-approval to team membership.

Behavior

• self_approval_via_teams = true (default false, effective together with allow_self_approval = true): the author's team memberships are resolved through the existing user-reviewer map, and OR groups containing any of those teams are treated as satisfied.
• Teams are not removed from the groups — other team members remain valid reviewers (mirrors how the author's own login is handled, except the group entry stays).
• AuthorModeDefault is unchanged: team memberships are ignored there, since other members of the team must still be able to (and are expected to) review.
• Requires a token able to read org team members — the same requirement as the documented GitHub Teams support.

Implementation notes

• CodeOwners.SetAuthor gains a variadic authorTeams ...Slug parameter — existing two-argument call sites keep compiling.
• New Client.UserReviewers(user) getter over the already-built userReviewerMap; no new GitHub API surface.
• When the option is enabled, InitUserReviewerMap is called once before SetAuthor (with the pre-author owner set) and later re-initialized as before — one extra members fetch per team in the rules, only when the option is on. Happy to add caching in teamFetch if you'd prefer.
• Design question: I kept this as a separate opt-in flag for backward compatibility, but folding it into allow_self_approval itself would arguably match the README wording ("any OR ownership group they belong to") — happy to change if you prefer that.
• Also tightened the README wording for allow_self_approval to make the login-only matching explicit.

Testing

• TestSetAuthorSelfApprovalViaTeamMembership — OR group containing the author's team is auto-satisfied
• TestSetAuthorTeamsIgnoredInDefaultMode — default mode unchanged, team stays in the group
• TestSetAuthorTeamsDoNotAffectUnrelatedGroups — groups without the author's teams remain required
• go test ./..., go vet ./... clean

Closes #49

🤖 Generated with Claude Code

[gemini-code-assist]

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

[daniil-pushin]

Pushed a second commit that tightens the semantics: self-approval (both by login and via teams) no longer satisfies additional-reviewer (&) groups.

The test suite already documents this intent ("Self-approval only applies to OR groups"), but the implementation satisfied any group containing the author — including & groups the author is listed in (this pre-dates this PR; the team extension would have inherited it). Since & rules are the four-eyes mechanism, authorship satisfying them silently disables mandatory review for authors who are members of the co-signing team.

Implementation: ReviewerGroup.Additional flag set for groups created from & rules (memoized under a separate key so the flag can't leak between an owner rule and an & rule with identical names), skipped in SetAuthor self-approval. Emptiness behavior is unchanged (sole-listed author still satisfies the group to avoid deadlock), and MergeCodeOwners keys include the flag.

[BakerNet]

Pushed a second commit that tightens the semantics: self-approval (both by login and via teams) no longer satisfies additional-reviewer (&) groups.

The test suite already documents this intent ("Self-approval only applies to OR groups"), but the implementation satisfied any group containing the author — including & groups the author is listed in (this pre-dates this PR; the team extension would have inherited it). Since & rules are the four-eyes mechanism, authorship satisfying them silently disables mandatory review for authors who are members of the co-signing team.

Implementation: ReviewerGroup.Additional flag set for groups created from & rules (memoized under a separate key so the flag can't leak between an owner rule and an & rule with identical names), skipped in SetAuthor self-approval. Emptiness behavior is unchanged (sole-listed author still satisfies the group to avoid deadlock), and MergeCodeOwners keys include the flag.

Thanks for the contribution!

The base idea of this PR I'm good with, but this is not was "Self-approval only applies to OR groups" was trying to convey; that comment is a bit misleading in hindsight. Already, if an author was the only owner in a review group - whether it was a normal rule or an additional reviewers rule - that group would not be enforced. When an additional owner is in the group (an OR rule exists, again whether it's a normal review group or an additional reviewers rule) then that group would need to be satisfied by the other owner.

So "& rules mean always required" would be new semantics which I don't want to add.

Would you mind reverting the second commit?

[daniil-pushin]

Hi @BakerNet ,
Thank you for the review and for the clear explanation — that all makes sense to me. I agree that "& rules mean always required" is a separate semantic change and shouldn't be bundled into this PR, so I've reverted the second commit and can open a separate issue for the &/self-approval discussion.

That leaves the PR scoped purely to extending self-approval to team membership. A few things I wasn't sure about and would love your guidance on:

1. Separate flag vs. folding into allow_self_approval. I kept it as a separate self_approval_via_teams flag mainly to avoid changing the behavior of the existing allow_self_approval for people already using it — but I could also just fold the team behavior directly into allow_self_approval if you'd prefer that. I wasn't sure which way you'd want to go here, so I left it separate for now.
2. The flag depends on the other one. Right now self_approval_via_teams doesn't do anything unless allow_self_approval = true. Would it be worth printing a warning (or erroring) if someone turns it on by itself, so it's not silently a no-op? Happy to add that if you think it's useful.
3. Possible small optimization. While working on this I noticed InitUserReviewerMap ends up running twice per invocation when the option is on (once before SetAuthor, once in the normal flow), re-fetching the same teams. It looked like something that could be optimized with a bit of caching in teamFetch — I'd be happy to do that, either here or as a follow-up, if it's worth it.