Skip to content

fix(review): validate reviewer roster collaborator membership - #5550

Open
purp wants to merge 2 commits into
NVIDIA:mainfrom
purp:fix/5549-reviewer-roster-validation/purp
Open

purp wants to merge 2 commits into
NVIDIA:mainfrom
purp:fix/5549-reviewer-roster-validation/purp

Conversation

@purp

@purp purp commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Remove 10 non-collaborators from REVIEWERS.md. Add a required Branch Checks job that compares every roster handle with GitHub's repository collaborator list and rejects non-collaborators before merge. Read, triage, and outside collaborators remain eligible.

Calls to add reviewers to a PR require that all requested reviewers are "collaborators" which GitHub defines as "outside collaborators, organization members that are direct collaborators, organization members with access through team memberships, organization members with access through default organization permissions, and organization owners" which boils down to being an explictly-named member of the repository. Until the repo moves to the new CNCF org, it's unfortunately hard to add members.

Related Issue

Closes #5549.

Verified target-repository ADMIN access; the issue has no state:accepted label. Replaces closed #5540 and #5539.

Changes

  • Stop reviewer assignment failures by removing 21 memberships belonging to the 10 people who are not collaborators per GitHub definition; preserve category definitions and all eligible memberships.
  • Add a read-only Reviewer Roster job to Branch Checks for PR mirrors and merge groups. Check all collaborator pages, report sorted offending handles, and fail on lookup errors.
  • Reuse the existing roster parser and API helper; include four offline regressions in the existing review-readiness test task.
  • Protect the validator with CODEOWNERS and document the check.

Release Notes

Testing

  • All 49 reviewer-roster and review-readiness tests pass.
  • Ruff format/lint, ty, Actionlint with ShellCheck, Markdown lint, and git diff --check pass.
  • Live validation rejects the original roster with all 10 offending accounts and accepts the cleaned roster's 17 accounts.
  • Independent review found no blocking issues.
  • Actions-token execution: verified after CI runs the new job.
  • E2E tests: not applicable to repository tooling.

Checklist

  • Follows Conventional Commits
  • Commit signature verified and DCO sign-off included

@purp
purp requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners November 14, 2026 01:30
@github-actions github-actions Bot added the state:accepted A maintainer decided OpenShell should pursue this issue label Nov 14, 2026
@purp
purp enabled auto-merge November 14, 2026 04:55
@purp purp added area:build CI/CD workflows, releases, build targets, packaging, and artifact publication. topic:dx Making OpenShell easy to contribute to for humans and agents. labels Nov 14, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:build CI/CD workflows, releases, build targets, packaging, and artifact publication. state:accepted A maintainer decided OpenShell should pursue this issue topic:dx Making OpenShell easy to contribute to for humans and agents.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(review): keep the reviewer roster limited to collaborators

1 participant