Repository navigation
feat(ci): gate merges on maintainer approval via a required status check #3176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,35 @@ | ||
| # Broad ownership — core team reviews everything | ||
| ## Broad ownership — core team reviews everything | ||
| * @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
|
|
||
| # Vouch list — maintainers only (bot commits bypass, but manual edits need review) | ||
| ## Vouch list — maintainers only (bot commits bypass, but manual edits need review) | ||
| .github/VOUCHED.td @NVIDIA/openshell-codeowners | ||
|
|
||
| ## Merge enforcement — these paths decide who can merge, so they notify the full | ||
| # list of maintainers to raise scrutiny on changes here. | ||
| # | ||
| # PR checks run local to the branch, so unless the check specifically avoids | ||
| # using branch-local files, a PR could change its own gates and appear to pass | ||
| # them. Covering the required checks here closes that gap. | ||
| # | ||
| # The whole workflows directory is covered, not just the approval gate, because | ||
| # any workflow can claim a required check context or ask for a broader token. | ||
| # | ||
| # Normally, we'd use a GitHub team to cover all maintainers, but org-level | ||
| # policies prevent outside collaborators from being on a GH team at this time | ||
| # so we spell them out explicitly. | ||
|
|
||
| # Core check mechanisms | ||
| /.github/workflows/ @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
| /.github/actions/ @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
| /.github/CODEOWNERS @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
| /.github/zizmor.yml @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
|
|
||
| # `MAINTAINERS.md` maintainer approval checks | ||
| /MAINTAINERS.md @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
| /tasks/scripts/check_maintainer_approval.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
| /tasks/scripts/alert_maintainer_change.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
| /tasks/scripts/check_maintainer_approval_test.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
| /tasks/scripts/alert_maintainer_change_test.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
|
|
||
| # Auditable SBOM check | ||
| /tasks/scripts/verify-image-sbom.sh @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| name: Maintainer Approval | ||
|
|
||
| on: | ||
| merge_group: | ||
| types: [checks_requested] | ||
| pull_request_review: | ||
| types: [submitted, dismissed] | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| maintainer-approval: | ||
| # This name is the required status check context. Changing it silently | ||
| # breaks the ruleset entry that gates merges on this job. | ||
| name: OpenShell / Maintainer Approval | ||
| if: github.repository_owner == 'NVIDIA' | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 5 | ||
| steps: | ||
| # Check out the default branch, never the pull request head. Both the | ||
| # maintainer list and the decision helper must come from main: reading | ||
| # either from the pull request ref would let a contributor add themselves | ||
| # to the list, or rewrite the decision logic, and self-approve. | ||
| - name: Check out the maintainer list and helper | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| ref: main | ||
| sparse-checkout: | | ||
| MAINTAINERS.md | ||
| tasks/scripts/check_maintainer_approval.py | ||
| sparse-checkout-cone-mode: false | ||
| persist-credentials: false | ||
|
|
||
| - name: Require an approving review from a maintainer | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| GH_REPO: ${{ github.repository }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| # merge_group carries no pull request in its payload, but the queue | ||
| # branch names the entry: gh-readonly-queue/main/pr-3027-<base sha>. | ||
| MERGE_GROUP_REF: ${{ github.ref_name }} | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| if [ -z "${PR_NUMBER:-}" ]; then | ||
| if [[ "$MERGE_GROUP_REF" =~ /pr-([0-9]+)-[0-9a-f]+$ ]]; then | ||
| PR_NUMBER="${BASH_REMATCH[1]}" | ||
| else | ||
| # Fail closed: an unrecognised ref must never satisfy the gate. | ||
| echo "::error::No pull request resolved from '$MERGE_GROUP_REF'." | ||
| exit 1 | ||
| fi | ||
| fi | ||
|
|
||
| gh api --paginate "repos/$GH_REPO/pulls/$PR_NUMBER/reviews" --jq '.[]' \ | ||
| | jq -s '.' > reviews.json | ||
|
|
||
| # Exits non-zero when no maintainer's latest decisive review is an | ||
| # approval, and when MAINTAINERS.md yields no handles. That exit code | ||
| # is the check result; nothing is posted anywhere. | ||
| python3 tasks/scripts/check_maintainer_approval.py \ | ||
| --maintainers MAINTAINERS.md \ | ||
| --reviews reviews.json |
|
purp marked this conversation as resolved.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| name: Maintainers Change Alert | ||
|
|
||
| on: | ||
| pull_request_target: | ||
| types: [opened, reopened, synchronize] | ||
| paths: | ||
| - MAINTAINERS.md | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.event.pull_request.number }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| describe-change: | ||
| name: Comment on the approver set change if MAINTAINERS.md has changed | ||
| if: github.repository_owner == 'NVIDIA' | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| steps: | ||
| # Default branch only. The helper must be the reviewed version, not | ||
| # whatever the pull request happens to contain. | ||
| - name: Check out the change-alert helper | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| ref: main | ||
| sparse-checkout: tasks/scripts/alert_maintainer_change.py | ||
| sparse-checkout-cone-mode: false | ||
| persist-credentials: false | ||
|
|
||
| - name: Post the maintainer delta | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| GH_REPO: ${{ github.repository }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| BASE_SHA: ${{ github.event.pull_request.base.sha }} | ||
| HEAD_SHA: ${{ github.event.pull_request.head.sha }} | ||
| # Single source of truth for the comment marker: the script emits | ||
| # it as the first line of the body, the lookup below matches on it. | ||
| COMMENT_MARKER: "<!-- maintainer-approval-delta -->" | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| # Fetching file contents is reading data, not executing it. The head | ||
| # revision is never checked out or run. | ||
| # | ||
| # A 404 means the file genuinely does not exist at that revision — a | ||
| # pull request that adds or deletes MAINTAINERS.md — and yields an | ||
| # empty side of the comparison. Every other failure is fatal: an empty | ||
| # file from a rate limit or a 5xx would render as "every maintainer | ||
| # was just added" or "nothing changed", both of which mislead the | ||
| # reviewer about who can merge code. | ||
| fetch_maintainers() { | ||
| local ref="$1" out="$2" err | ||
| err="$(mktemp)" | ||
| if gh api -H "Accept: application/vnd.github.raw" \ | ||
| "repos/$GH_REPO/contents/MAINTAINERS.md?ref=$ref" > "$out" 2>"$err"; then | ||
| rm -f "$err" | ||
| return 0 | ||
| fi | ||
| if grep -q 'HTTP 404' "$err"; then | ||
| rm -f "$err" | ||
| : > "$out" | ||
| return 0 | ||
| fi | ||
| echo "::error::Could not fetch MAINTAINERS.md at $ref" | ||
| cat "$err" >&2 | ||
| rm -f "$err" | ||
| return 1 | ||
| } | ||
|
|
||
| fetch_maintainers "$BASE_SHA" before.md | ||
| fetch_maintainers "$HEAD_SHA" after.md | ||
|
|
||
| # An unparseable result exits non-zero, but the comment explaining | ||
| # why still has to be posted before this job fails. | ||
| status=0 | ||
| python3 tasks/scripts/alert_maintainer_change.py \ | ||
| --before before.md --after after.md > body.md || status=$? | ||
|
|
||
| # No output means the approver set did not change. A comment saying | ||
| # so is noise, so post nothing. | ||
| if [ -s body.md ]; then | ||
| cat body.md >> "$GITHUB_STEP_SUMMARY" | ||
|
|
||
| # Update the existing comment rather than stacking one per push. | ||
| COMMENT_ID=$(gh api --paginate "repos/$GH_REPO/issues/$PR_NUMBER/comments" \ | ||
| --jq '.[] | select(.body | startswith($ENV.COMMENT_MARKER)) | .id' \ | ||
| | head -n 1) | ||
|
|
||
| if [ -n "$COMMENT_ID" ]; then | ||
| gh api --method PATCH "repos/$GH_REPO/issues/comments/$COMMENT_ID" \ | ||
| -F "body=@body.md" >/dev/null | ||
| else | ||
| gh api --method POST "repos/$GH_REPO/issues/$PR_NUMBER/comments" \ | ||
| -F "body=@body.md" >/dev/null | ||
| fi | ||
| fi | ||
|
|
||
| exit "$status" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.