contributions: clarify Maintainer team access - #23394
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves brew contributions behavior when GitHub organization teams are not accessible to the current user/token by introducing a dedicated TeamAccessError and adding a fallback that reads the public Maintainers list from Homebrew/brew’s README.md.
Changes:
- Add
GitHub::TeamAccessErrorand updateGitHub.members_by_teamto raise it when a team is missing or inaccessible (instead of a misleading scope message). - Update
brew contributionsto use default maintainers (team-first, README fallback) when no--team/--user/report mode is specified, reusing the existing README maintainer parser. - Add specs covering the inaccessible-team error and the README fallback path.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Library/Homebrew/utils/github.rb | Introduces TeamAccessError and changes members_by_team error handling for inaccessible teams. |
| Library/Homebrew/dev-cmd/contributions.rb | Adds default maintainer resolution and README-based fallback by reusing the README parser. |
| Library/Homebrew/test/utils/github_spec.rb | Adds a unit test asserting members_by_team raises TeamAccessError for inaccessible teams. |
| Library/Homebrew/test/dev-cmd/contributions_spec.rb | Adds a test for the brew contributions README fallback behavior when team access fails. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@SSakutaro why are you trying to run |
|
I originally encountered this while investigating obviously incorrect results from $ brew contributions --user SSakutaro
Fetching latest commits for Homebrew/brew...
Fetching latest commits for Homebrew/homebrew-core...
Fetching latest commits for Homebrew/homebrew-cask...
SSakutaro contributed >=1001 times (merged PRs authored), >=1000 times (PRs merged), >=1001 times (qualifying merged PRs), 39 times (approved-review search matches), 98 times(co-authored commits) and >=1138 times (total) between 2025-08-01 and 2026-08-02.I have not authored or merged anywhere near that many PRs, so as part of the investigation I checked the command's behavior both with and without arguments. That is when I encountered this error. My original goal was not to inspect the Maintainers' contribution totals. I was also confused by this error message: My token already has the I therefore believe the team lookup fails because I am not a member of the Homebrew organization, rather than because the token is missing the required scope. Since the error still says that I proposed the README fallback because the Maintainer list is already public, and the documentation for However, if the no-argument form of |
Interesting. Would love a fix for that!
This would be great, thanks 🙇🏻 |
|
Got it, thanks! I'll narrow this PR to improving the error message and documentation, and remove the README fallback. After that, I'll investigate the incorrect contribution counts separately and follow up with another PR when I have a focused fix. |
099cb7e to
d009aed
Compare
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Thanks, looks good! GitHub Copilot review comment worth a look.
d009aed to
499a148
Compare
|
Got it. I've incorporated the review. |
brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?Codex (GPT-5) assisted with investigating the issue, drafting the implementation and tests. I reviewed the resulting diff, reproduced the issue and fallback locally, and ran the relevant tests and brew lgtm --online.
brew contributionsfetches theHomebrew/maintainersteam by default. When the current GitHub account cannot view organization teams, the command incorrectly reports that the token needsread:org, even when that scope is already present.This change replaces the scope-specific error with an actionable message covering both GitHub account access and
token permissions. It also documents that the default Maintainer search requires access to the
Homebrew/maintainersteam.Reproduction
With this change:
Testing
Added a test covering an inaccessible team without assuming that the token is missing a particular scope.