From 661d04acd46f5cb2b4373d8d5970480523a1c66e Mon Sep 17 00:00:00 2001 From: Jim Meyer Date: Thu, 3 Sep 2026 17:04:20 -0700 Subject: [PATCH 1/9] feat(ci): add maintainer approval decision helper Signed-off-by: Jim Meyer --- tasks/scripts/core_approval.py | 185 ++++++++++++++++++++++++++++ tasks/scripts/core_approval_test.py | 118 ++++++++++++++++++ tasks/test.toml | 6 + 3 files changed, 309 insertions(+) create mode 100644 tasks/scripts/core_approval.py create mode 100644 tasks/scripts/core_approval_test.py diff --git a/tasks/scripts/core_approval.py b/tasks/scripts/core_approval.py new file mode 100644 index 0000000000..ec3ab175f6 --- /dev/null +++ b/tasks/scripts/core_approval.py @@ -0,0 +1,185 @@ +#!/usr/bin/env python3 +# /// script +# requires-python = ">=3.11" +# dependencies = [] +# /// + +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Decide whether a pull request carries an approval from a listed maintainer. + +The logic here is pure so it can be unit tested. The calling workflow does the +I/O: it fetches MAINTAINERS.md pinned to the default branch, lists the pull +request's reviews, and passes both in as files. + +Runs as bare `python3` on the Actions runner, so it must stay stdlib-only. +""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +# Only a login that appears as a link to a GitHub profile counts. A bare +# "[@someone]" in prose must never widen the approver set. +MAINTAINER_RE = re.compile( + r"\[@([A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?)\]\(https://github\.com/" +) + +# States that express a standing position. COMMENTED and PENDING leave a +# reviewer's earlier approval intact, which is how GitHub itself treats them. +DECISIVE_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED", "DISMISSED"}) + +# GitHub truncates commit status descriptions past this length. +DESCRIPTION_LIMIT = 140 + +COMMENT_MARKER = "" + + +def parse_maintainers(markdown: str) -> set[str]: + """Return the lowercased GitHub logins listed in a MAINTAINERS.md table.""" + return {match.group(1).lower() for match in MAINTAINER_RE.finditer(markdown)} + + +def latest_positions(reviews: list[dict]) -> dict[str, str]: + """Map each reviewer's lowercased login to their most recent decisive state. + + The reviews API returns reviews in ascending submission order, so a later + entry for the same login supersedes an earlier one. + """ + positions: dict[str, str] = {} + for entry in reviews: + state = str(entry.get("state") or "").upper() + if state not in DECISIVE_STATES: + continue + login = str((entry.get("user") or {}).get("login") or "").lower() + if login: + positions[login] = state + return positions + + +def approving_maintainers( + reviews: list[dict], maintainers: set[str], author: str +) -> list[str]: + """Return the listed maintainers whose standing position is an approval.""" + author = author.lower() + return sorted( + login + for login, state in latest_positions(reviews).items() + if state == "APPROVED" and login in maintainers and login != author + ) + + +def decide(markdown: str, reviews: list[dict], author: str) -> tuple[str, str]: + """Return the (state, description) to publish as a commit status.""" + maintainers = parse_maintainers(markdown) + if not maintainers: + # Fail closed. An unparseable or empty list must never satisfy the gate. + return "failure", "Could not parse any maintainers from MAINTAINERS.md" + + approvers = approving_maintainers(reviews, maintainers, author) + if not approvers: + return "failure", "Needs approval from a maintainer listed in MAINTAINERS.md" + + shown = ", ".join(f"@{login}" for login in approvers[:3]) + remainder = len(approvers) - 3 + if remainder > 0: + shown = f"{shown} and {remainder} more" + return "success", f"Approved by {shown}"[:DESCRIPTION_LIMIT] + + +def format_delta(before: str, after: str) -> str: + """Render a review comment describing how the approver set changes.""" + old, new = parse_maintainers(before), parse_maintainers(after) + added, removed = sorted(new - old), sorted(old - new) + + lines = [COMMENT_MARKER, "## Maintainer list change", ""] + if not added and not removed: + lines.append( + "This pull request edits `MAINTAINERS.md` but does not change the set " + "of logins the approval gate recognises." + ) + else: + if added: + lines += ["**Gains approval rights:**", ""] + lines += [f"- @{login}" for login in added] + lines.append("") + if removed: + lines += ["**Loses approval rights:**", ""] + lines += [f"- @{login}" for login in removed] + lines.append("") + lines.append( + "Confirm every change is intended. Anyone listed here can single-handedly " + "satisfy `OpenShell / Core Approval`." + ) + + if not new: + lines += [ + "", + "> [!WARNING]", + "> No logins parse from the updated file. Merging this would make the " + "approval gate fail closed on every pull request.", + ] + return "\n".join(lines) + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description=__doc__) + subcommands = parser.add_subparsers(dest="command", required=True) + + decide_cmd = subcommands.add_parser( + "decide", help="print the commit status to publish, as 'statedescription'" + ) + decide_cmd.add_argument( + "--maintainers", + required=True, + type=Path, + help="MAINTAINERS.md fetched from the default branch", + ) + decide_cmd.add_argument( + "--reviews", + required=True, + type=Path, + help="JSON array returned by the list-reviews API", + ) + decide_cmd.add_argument( + "--author", default="", help="pull request author, excluded from approvers" + ) + + diff_cmd = subcommands.add_parser( + "diff", help="print a review comment describing the approver set change" + ) + diff_cmd.add_argument( + "--before", required=True, type=Path, help="MAINTAINERS.md at the base commit" + ) + diff_cmd.add_argument( + "--after", required=True, type=Path, help="MAINTAINERS.md at the head commit" + ) + return parser + + +def main(argv: list[str] | None = None) -> int: + args = build_parser().parse_args(argv) + + if args.command == "decide": + reviews = json.loads(args.reviews.read_text(encoding="utf-8")) + state, description = decide( + args.maintainers.read_text(encoding="utf-8"), reviews, args.author + ) + print(f"{state}\t{description}") + else: + print( + format_delta( + args.before.read_text(encoding="utf-8"), + args.after.read_text(encoding="utf-8"), + ) + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tasks/scripts/core_approval_test.py b/tasks/scripts/core_approval_test.py new file mode 100644 index 0000000000..4a4c45dbbf --- /dev/null +++ b/tasks/scripts/core_approval_test.py @@ -0,0 +1,118 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for tasks/scripts/core_approval.py. + +Run via `mise run test:core-approval`, which provides pytest through +`uv run --with pytest`. pytest puts this file's directory on sys.path, so the +sibling script imports directly as `core_approval`. +""" + +from __future__ import annotations + +import core_approval as ca + +TABLE = """# Maintainers + +| Name | GitHub ID | Company/Organization | +| --- | --- | --- | +| Derek Carr | [@derekwaynecarr](https://github.com/derekwaynecarr) | Red Hat | +| Jim Meyer | [@purp](https://github.com/purp) | NVIDIA | +| Mrunal Patel | [@mrunalp](https://github.com/mrunalp) | Red Hat | +""" + + +def review(login: str, state: str) -> dict: + return {"user": {"login": login}, "state": state} + + +def test_parse_maintainers_extracts_linked_logins() -> None: + assert ca.parse_maintainers(TABLE) == {"derekwaynecarr", "purp", "mrunalp"} + + +def test_parse_maintainers_ignores_unlinked_mentions() -> None: + # A prose mention must not silently grant approval rights. + prose = TABLE + "\nThanks to [@drive-by](mailto:nobody@example.com) too.\n" + assert "drive-by" not in ca.parse_maintainers(prose) + + +def test_parse_maintainers_returns_empty_when_table_is_reformatted() -> None: + assert ca.parse_maintainers("# Maintainers\n\n- derekwaynecarr\n- purp\n") == set() + + +def test_decide_fails_closed_on_unparseable_list() -> None: + state, description = ca.decide("# Maintainers\n", [review("purp", "APPROVED")], "x") + assert state == "failure" + assert "MAINTAINERS.md" in description + + +def test_decide_succeeds_on_maintainer_approval() -> None: + state, description = ca.decide(TABLE, [review("purp", "APPROVED")], "contributor") + assert state == "success" + assert "@purp" in description + + +def test_decide_fails_on_non_maintainer_approval() -> None: + state, _ = ca.decide(TABLE, [review("outsider", "APPROVED")], "contributor") + assert state == "failure" + + +def test_decide_matches_logins_case_insensitively() -> None: + state, _ = ca.decide(TABLE, [review("PuRp", "APPROVED")], "contributor") + assert state == "success" + + +def test_comment_after_approval_does_not_revoke_it() -> None: + reviews = [review("purp", "APPROVED"), review("purp", "COMMENTED")] + state, _ = ca.decide(TABLE, reviews, "contributor") + assert state == "success" + + +def test_dismissed_review_revokes_approval() -> None: + reviews = [review("purp", "APPROVED"), review("purp", "DISMISSED")] + state, _ = ca.decide(TABLE, reviews, "contributor") + assert state == "failure" + + +def test_changes_requested_after_approval_revokes_it() -> None: + reviews = [review("purp", "APPROVED"), review("purp", "CHANGES_REQUESTED")] + state, _ = ca.decide(TABLE, reviews, "contributor") + assert state == "failure" + + +def test_author_cannot_satisfy_the_gate() -> None: + state, _ = ca.decide(TABLE, [review("purp", "APPROVED")], "purp") + assert state == "failure" + + +def test_another_maintainer_still_satisfies_a_maintainer_authored_pr() -> None: + state, _ = ca.decide(TABLE, [review("mrunalp", "APPROVED")], "purp") + assert state == "success" + + +def test_description_stays_within_the_github_limit() -> None: + reviews = [ + review(login, "APPROVED") for login in ("purp", "mrunalp", "derekwaynecarr") + ] + _, description = ca.decide(TABLE, reviews, "contributor") + assert len(description) <= 140 + + +def test_format_delta_names_added_and_removed_logins() -> None: + after = TABLE.replace( + "| Mrunal Patel | [@mrunalp](https://github.com/mrunalp) | Red Hat |\n", + "| New Person | [@newbie](https://github.com/newbie) | NVIDIA |\n", + ) + body = ca.format_delta(TABLE, after) + assert "@newbie" in body + assert "@mrunalp" in body + + +def test_format_delta_reports_no_change_when_only_prose_moves() -> None: + body = ca.format_delta(TABLE, TABLE + "\nSee also CONTRIBUTING.md.\n") + assert "does not change" in body + + +def test_format_delta_warns_when_the_result_parses_empty() -> None: + body = ca.format_delta(TABLE, "# Maintainers\n\n- purp\n") + assert "WARNING" in body diff --git a/tasks/test.toml b/tasks/test.toml index 4a5cda0890..c6c03800f6 100644 --- a/tasks/test.toml +++ b/tasks/test.toml @@ -15,6 +15,7 @@ depends = [ "test:packaging-assets", "test:codex-security-release-range", "test:docs-website", + "test:core-approval", ] ["test:docs-website"] @@ -51,6 +52,11 @@ description = "Test Codex Security release-range resolution" run = "uv run --no-project --with pytest pytest -o \"python_files=*_test.py\" tasks/scripts/codex_security_range_test.py" hide = true +["test:core-approval"] +description = "Test the maintainer approval gate helper" +run = "uv run --no-project --with pytest pytest -o \"python_files=*_test.py\" tasks/scripts/core_approval_test.py" +hide = true + [e2e] description = "Run all end-to-end tests (Rust + Python + MCP)" depends = ["e2e:rust", "e2e:python", "e2e:mcp"] From cc44618061f2dde0a0c78897e5a878f2ff16e953 Mon Sep 17 00:00:00 2001 From: Jim Meyer Date: Thu, 3 Sep 2026 17:10:29 -0700 Subject: [PATCH 2/9] fix(ci): order maintainer reviews by review id Signed-off-by: Jim Meyer --- tasks/scripts/core_approval.py | 6 +++--- tasks/scripts/core_approval_test.py | 11 +++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/tasks/scripts/core_approval.py b/tasks/scripts/core_approval.py index ec3ab175f6..c23a53cb35 100644 --- a/tasks/scripts/core_approval.py +++ b/tasks/scripts/core_approval.py @@ -48,11 +48,11 @@ def parse_maintainers(markdown: str) -> set[str]: def latest_positions(reviews: list[dict]) -> dict[str, str]: """Map each reviewer's lowercased login to their most recent decisive state. - The reviews API returns reviews in ascending submission order, so a later - entry for the same login supersedes an earlier one. + Ordering comes from the review id, not from input order: a later entry + for the same login, by ascending review id, supersedes an earlier one. """ positions: dict[str, str] = {} - for entry in reviews: + for entry in sorted(reviews, key=lambda r: r.get("id") or 0): state = str(entry.get("state") or "").upper() if state not in DECISIVE_STATES: continue diff --git a/tasks/scripts/core_approval_test.py b/tasks/scripts/core_approval_test.py index 4a4c45dbbf..ab5c3ac085 100644 --- a/tasks/scripts/core_approval_test.py +++ b/tasks/scripts/core_approval_test.py @@ -74,6 +74,17 @@ def test_dismissed_review_revokes_approval() -> None: assert state == "failure" +def test_out_of_order_reviews_still_respect_the_latest_position() -> None: + # Ordering comes from the review id, not the order the caller happened + # to assemble the pages in. + reviews = [ + {"id": 2, "user": {"login": "purp"}, "state": "DISMISSED"}, + {"id": 1, "user": {"login": "purp"}, "state": "APPROVED"}, + ] + state, _ = ca.decide(TABLE, reviews, "contributor") + assert state == "failure" + + def test_changes_requested_after_approval_revokes_it() -> None: reviews = [review("purp", "APPROVED"), review("purp", "CHANGES_REQUESTED")] state, _ = ca.decide(TABLE, reviews, "contributor") From b6c434419d724bab51aa58e43fd97730b98a6dbb Mon Sep 17 00:00:00 2001 From: Jim Meyer Date: Thu, 3 Sep 2026 17:18:11 -0700 Subject: [PATCH 3/9] feat(ci): publish the core approval status check Signed-off-by: Jim Meyer --- .github/workflows/core-approval.yml | 110 ++++++++++++++++++++++++++++ .github/zizmor.yml | 1 + CONTRIBUTING.md | 4 + 3 files changed, 115 insertions(+) create mode 100644 .github/workflows/core-approval.yml diff --git a/.github/workflows/core-approval.yml b/.github/workflows/core-approval.yml new file mode 100644 index 0000000000..d71cd37403 --- /dev/null +++ b/.github/workflows/core-approval.yml @@ -0,0 +1,110 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +name: Core Approval + +on: + merge_group: + types: [checks_requested] + pull_request_target: + types: [opened, reopened, synchronize, ready_for_review] + pull_request_review: + types: [submitted, dismissed] + workflow_dispatch: + inputs: + pr_number: + description: Pull request number to re-evaluate + required: true + type: string + +permissions: + contents: read + pull-requests: read + statuses: write + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || inputs.pr_number || github.sha || github.run_id }} + cancel-in-progress: true + +jobs: + core-approval: + name: Publish core approval status + if: github.repository_owner == 'NVIDIA' + runs-on: ubuntu-latest + steps: + # Check out the default branch, never the pull request head. This job runs + # with a write-capable token, so it must not fetch or execute contributor + # code. Only the approval helper is needed. + - name: Check out the approval helper + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: main + sparse-checkout: tasks/scripts/core_approval.py + sparse-checkout-cone-mode: false + persist-credentials: false + + - name: Publish core approval status + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + PR_NUMBER_FROM_EVENT: ${{ github.event.pull_request.number }} + PR_NUMBER_FROM_INPUT: ${{ inputs.pr_number }} + MERGE_GROUP_SHA: ${{ github.event.merge_group.head_sha }} + shell: bash + run: | + set -euo pipefail + + STATUS_CONTEXT="OpenShell / Core Approval" + RUN_URL="https://github.com/$GH_REPO/actions/runs/$GITHUB_RUN_ID" + + post_status() { + local sha="$1" state="$2" description="$3" target_url="$4" + echo "$STATUS_CONTEXT: $state - $description" + gh api --method POST "repos/$GH_REPO/statuses/$sha" \ + -f "state=$state" \ + -f "context=$STATUS_CONTEXT" \ + -f "description=$description" \ + -f "target_url=$target_url" >/dev/null + } + + # A merge group only forms after the pull request satisfied this gate, + # and approvals cannot change while an entry sits in the queue. Publish + # success so the queue's required-check evaluation resolves instead of + # waiting out check_response_timeout_minutes. + if [ "$EVENT_NAME" = "merge_group" ]; then + post_status "$MERGE_GROUP_SHA" success \ + "Approval enforced at pull request" "$RUN_URL" + exit 0 + fi + + PR_NUMBER="${PR_NUMBER_FROM_EVENT:-$PR_NUMBER_FROM_INPUT}" + PR=$(gh api "repos/$GH_REPO/pulls/$PR_NUMBER") + + if [ "$(jq -r '.state' <<< "$PR")" != "open" ]; then + echo "PR #$PR_NUMBER is not open; nothing to publish." + exit 0 + fi + + HEAD_SHA=$(jq -r '.head.sha' <<< "$PR") + AUTHOR=$(jq -r '.user.login' <<< "$PR") + TARGET_URL="https://github.com/$GH_REPO/pull/$PR_NUMBER" + + # Pinned to main on purpose. Reading this file from the pull request + # ref would let a contributor add themselves and self-approve. + gh api -H "Accept: application/vnd.github.raw" \ + "repos/$GH_REPO/contents/MAINTAINERS.md?ref=main" > maintainers.md + + gh api --paginate "repos/$GH_REPO/pulls/$PR_NUMBER/reviews" --jq '.[]' \ + | jq -s '.' > reviews.json + + if ! RESULT=$(python3 tasks/scripts/core_approval.py decide \ + --maintainers maintainers.md \ + --reviews reviews.json \ + --author "$AUTHOR"); then + post_status "$HEAD_SHA" failure \ + "Could not evaluate maintainer approval" "$RUN_URL" + exit 1 + fi + + post_status "$HEAD_SHA" "${RESULT%%$'\t'*}" "${RESULT#*$'\t'}" "$TARGET_URL" diff --git a/.github/zizmor.yml b/.github/zizmor.yml index 7b1e035b77..9264f7e139 100644 --- a/.github/zizmor.yml +++ b/.github/zizmor.yml @@ -6,6 +6,7 @@ rules: ignore: # These base-branch workflows never check out or execute pull request # head code. Keep each suppression scoped to its reviewed trigger block. + - core-approval.yml:6 - dco.yml:3 - e2e-label-help.yml:13 - release-canary.yml:3 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ab1fa2e7a5..f8acf92364 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,6 +68,10 @@ Do not start substantial issue-backed work until a maintainer has accepted the i Use agents and the repository skills as needed to understand the affected code, evaluate tradeoffs, implement the smallest coherent change, and verify it. The pull request should explain what changed and how it was tested; it should not substitute an agent transcript for the contributor's understanding. +Every pull request must be approved by someone listed in [MAINTAINERS.md](MAINTAINERS.md) before it can merge. This is enforced by the `OpenShell / Core Approval` status check, which turns green once one of those reviewers approves. Reviews from other contributors are welcome and count toward the general approval requirement, but they do not satisfy this check. + +Maintainers are not requested automatically. If your pull request has been idle, ask for a reviewer in the pull request or in the CNCF Slack channel rather than waiting. + ## Agent Skills OpenShell keeps skills for using the product separate from skills for developing the repository. From 5c8ac00eee750b2fc5f504f5e4840b1a59f8a287 Mon Sep 17 00:00:00 2001 From: Jim Meyer Date: Thu, 3 Sep 2026 17:26:59 -0700 Subject: [PATCH 4/9] fix(ci): publish a red core approval status on abort Signed-off-by: Jim Meyer --- .github/workflows/core-approval.yml | 32 ++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/.github/workflows/core-approval.yml b/.github/workflows/core-approval.yml index d71cd37403..27bfbcc09d 100644 --- a/.github/workflows/core-approval.yml +++ b/.github/workflows/core-approval.yml @@ -31,10 +31,13 @@ jobs: name: Publish core approval status if: github.repository_owner == 'NVIDIA' runs-on: ubuntu-latest + timeout-minutes: 10 steps: # Check out the default branch, never the pull request head. This job runs # with a write-capable token, so it must not fetch or execute contributor - # code. Only the approval helper is needed. + # code. Only the approval helper is needed. "main" is hardcoded here and + # in the MAINTAINERS.md fetch below; both must change together if the + # default branch is ever renamed. - name: Check out the approval helper uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -66,6 +69,11 @@ jobs: -f "context=$STATUS_CONTEXT" \ -f "description=$description" \ -f "target_url=$target_url" >/dev/null + # A status was published, so the guard step has nothing to add. + # $GITHUB_ENV is read once after the step finishes and the last + # write wins, so this correctly reflects whether any status was + # posted. + echo "STATUS_SHA=" >> "$GITHUB_ENV" } # A merge group only forms after the pull request satisfied this gate, @@ -73,6 +81,7 @@ jobs: # success so the queue's required-check evaluation resolves instead of # waiting out check_response_timeout_minutes. if [ "$EVENT_NAME" = "merge_group" ]; then + echo "STATUS_SHA=$MERGE_GROUP_SHA" >> "$GITHUB_ENV" post_status "$MERGE_GROUP_SHA" success \ "Approval enforced at pull request" "$RUN_URL" exit 0 @@ -87,6 +96,7 @@ jobs: fi HEAD_SHA=$(jq -r '.head.sha' <<< "$PR") + echo "STATUS_SHA=$HEAD_SHA" >> "$GITHUB_ENV" AUTHOR=$(jq -r '.user.login' <<< "$PR") TARGET_URL="https://github.com/$GH_REPO/pull/$PR_NUMBER" @@ -108,3 +118,23 @@ jobs: fi post_status "$HEAD_SHA" "${RESULT%%$'\t'*}" "${RESULT#*$'\t'}" "$TARGET_URL" + + # If the step above aborted before publishing anything — a transient API + # failure, a timeout — the required check would otherwise sit at + # "Expected" forever. Publish red so the state is visible and the job can + # be re-run. + - name: Publish a failure status if none was published + if: failure() && env.STATUS_SHA != '' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + shell: bash + run: | + set -euo pipefail + gh api --method POST "repos/$GH_REPO/statuses/$STATUS_SHA" \ + -f "state=failure" \ + -f "context=OpenShell / Core Approval" \ + -f "description=Could not evaluate maintainer approval" \ + -f "target_url=https://github.com/$GH_REPO/actions/runs/$GITHUB_RUN_ID" \ + >/dev/null + From 703becd940d3ccb551f3dc9c799311371d38cbf7 Mon Sep 17 00:00:00 2001 From: Jim Meyer Date: Thu, 3 Sep 2026 17:46:37 -0700 Subject: [PATCH 5/9] feat(ci): comment the approver set delta on MAINTAINERS.md changes Signed-off-by: Jim Meyer --- .../workflows/maintainers-change-alert.yml | 73 +++++++++++++++++++ .github/zizmor.yml | 1 + 2 files changed, 74 insertions(+) create mode 100644 .github/workflows/maintainers-change-alert.yml diff --git a/.github/workflows/maintainers-change-alert.yml b/.github/workflows/maintainers-change-alert.yml new file mode 100644 index 0000000000..12cd63e724 --- /dev/null +++ b/.github/workflows/maintainers-change-alert.yml @@ -0,0 +1,73 @@ +# 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: github.repository_owner == 'NVIDIA' + runs-on: ubuntu-latest + steps: + # Default branch only. The helper must be the reviewed version, not + # whatever the pull request happens to contain. + - name: Check out the approval helper + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: main + sparse-checkout: tasks/scripts/core_approval.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 }} + shell: bash + run: | + set -euo pipefail + + # Fetching file contents is reading data, not executing it. The head + # revision is never checked out or run. + fetch_maintainers() { + gh api -H "Accept: application/vnd.github.raw" \ + "repos/$GH_REPO/contents/MAINTAINERS.md?ref=$1" > "$2" 2>/dev/null \ + || : > "$2" + } + + fetch_maintainers "$BASE_SHA" before.md + fetch_maintainers "$HEAD_SHA" after.md + + python3 tasks/scripts/core_approval.py diff \ + --before before.md --after after.md > body.md + 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("")) | .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 diff --git a/.github/zizmor.yml b/.github/zizmor.yml index 9264f7e139..3caa50ff8a 100644 --- a/.github/zizmor.yml +++ b/.github/zizmor.yml @@ -9,6 +9,7 @@ rules: - core-approval.yml:6 - dco.yml:3 - e2e-label-help.yml:13 + - maintainers-change-alert.yml:6 - release-canary.yml:3 - required-ci-gates.yml:3 - vouch-check.yml:3 From 91c6b8964255dd5ffdb879947b516cc93604e761 Mon Sep 17 00:00:00 2001 From: Jim Meyer Date: Thu, 3 Sep 2026 18:14:36 -0700 Subject: [PATCH 6/9] fix(ci): fail loudly when the MAINTAINERS.md fetch errors Signed-off-by: Jim Meyer --- .../workflows/maintainers-change-alert.yml | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/.github/workflows/maintainers-change-alert.yml b/.github/workflows/maintainers-change-alert.yml index 12cd63e724..a108f149d6 100644 --- a/.github/workflows/maintainers-change-alert.yml +++ b/.github/workflows/maintainers-change-alert.yml @@ -46,10 +46,30 @@ jobs: # 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() { - gh api -H "Accept: application/vnd.github.raw" \ - "repos/$GH_REPO/contents/MAINTAINERS.md?ref=$1" > "$2" 2>/dev/null \ - || : > "$2" + 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 From 94ccb2aa4c49e952eef1ed1148b88d3b6b497d9c Mon Sep 17 00:00:00 2001 From: Jim Meyer Date: Thu, 3 Sep 2026 18:26:37 -0700 Subject: [PATCH 7/9] fix(ci): let the approval guard fire on early failures Signed-off-by: Jim Meyer --- .github/workflows/core-approval.yml | 21 ++++++++++++------- .../workflows/maintainers-change-alert.yml | 5 +++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/.github/workflows/core-approval.yml b/.github/workflows/core-approval.yml index 27bfbcc09d..6e98b03bcb 100644 --- a/.github/workflows/core-approval.yml +++ b/.github/workflows/core-approval.yml @@ -31,7 +31,15 @@ jobs: name: Publish core approval status if: github.repository_owner == 'NVIDIA' runs-on: ubuntu-latest - timeout-minutes: 10 + # Seeded so the guard step below can still publish a red status when the + # job fails before the publishing step resolves the head SHA itself — a + # checkout failure, or a transient API error. Step-level writes to + # $GITHUB_ENV override this for later steps, so post_status clearing it + # still works. Empty on merge_group and workflow_dispatch, where the + # publishing step sets it instead. + env: + STATUS_CONTEXT: OpenShell / Core Approval + STATUS_SHA: ${{ github.event.pull_request.head.sha }} steps: # Check out the default branch, never the pull request head. This job runs # with a write-capable token, so it must not fetch or execute contributor @@ -47,6 +55,7 @@ jobs: persist-credentials: false - name: Publish core approval status + timeout-minutes: 10 env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_REPO: ${{ github.repository }} @@ -58,7 +67,6 @@ jobs: run: | set -euo pipefail - STATUS_CONTEXT="OpenShell / Core Approval" RUN_URL="https://github.com/$GH_REPO/actions/runs/$GITHUB_RUN_ID" post_status() { @@ -120,9 +128,9 @@ jobs: post_status "$HEAD_SHA" "${RESULT%%$'\t'*}" "${RESULT#*$'\t'}" "$TARGET_URL" # If the step above aborted before publishing anything — a transient API - # failure, a timeout — the required check would otherwise sit at - # "Expected" forever. Publish red so the state is visible and the job can - # be re-run. + # failure, a failed checkout, the step timing out — the required check + # would otherwise sit at "Expected" forever. Publish red so the state is + # visible and the job can be re-run. - name: Publish a failure status if none was published if: failure() && env.STATUS_SHA != '' env: @@ -133,8 +141,7 @@ jobs: set -euo pipefail gh api --method POST "repos/$GH_REPO/statuses/$STATUS_SHA" \ -f "state=failure" \ - -f "context=OpenShell / Core Approval" \ + -f "context=$STATUS_CONTEXT" \ -f "description=Could not evaluate maintainer approval" \ -f "target_url=https://github.com/$GH_REPO/actions/runs/$GITHUB_RUN_ID" \ >/dev/null - diff --git a/.github/workflows/maintainers-change-alert.yml b/.github/workflows/maintainers-change-alert.yml index a108f149d6..0969b1cd30 100644 --- a/.github/workflows/maintainers-change-alert.yml +++ b/.github/workflows/maintainers-change-alert.yml @@ -22,6 +22,7 @@ jobs: name: Comment on the approver set change 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. @@ -80,6 +81,10 @@ jobs: cat body.md >> "$GITHUB_STEP_SUMMARY" # Update the existing comment rather than stacking one per push. + # This marker must stay identical to COMMENT_MARKER in + # tasks/scripts/core_approval.py, which emits it as the first line of + # the body. If they drift, the lookup below silently stops matching + # and every push stacks another comment. COMMENT_ID=$(gh api --paginate "repos/$GH_REPO/issues/$PR_NUMBER/comments" \ --jq '.[] | select(.body | startswith("")) | .id' \ | head -n 1) From 4c73f85242a7284a1f710614b3084adf06c2e608 Mon Sep 17 00:00:00 2001 From: Jim Meyer Date: Thu, 3 Sep 2026 18:31:49 -0700 Subject: [PATCH 8/9] fix(ci): seed the merge group SHA for the approval guard A checkout or API failure on a merge_group run left STATUS_SHA empty, so the guard skipped and the required check sat at Expected until the queue timed out. Signed-off-by: Jim Meyer --- .github/workflows/core-approval.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/core-approval.yml b/.github/workflows/core-approval.yml index 6e98b03bcb..39d5b7d54b 100644 --- a/.github/workflows/core-approval.yml +++ b/.github/workflows/core-approval.yml @@ -35,11 +35,11 @@ jobs: # job fails before the publishing step resolves the head SHA itself — a # checkout failure, or a transient API error. Step-level writes to # $GITHUB_ENV override this for later steps, so post_status clearing it - # still works. Empty on merge_group and workflow_dispatch, where the - # publishing step sets it instead. + # still works. Empty only on workflow_dispatch, where no SHA is known until + # the publishing step looks the pull request up. env: STATUS_CONTEXT: OpenShell / Core Approval - STATUS_SHA: ${{ github.event.pull_request.head.sha }} + STATUS_SHA: ${{ github.event.pull_request.head.sha || github.event.merge_group.head_sha }} steps: # Check out the default branch, never the pull request head. This job runs # with a write-capable token, so it must not fetch or execute contributor From 0fe0a76acdd6326d4ad9241ae662c436789b0367 Mon Sep 17 00:00:00 2001 From: Jim Meyer Date: Thu, 3 Sep 2026 20:27:47 -0700 Subject: [PATCH 9/9] fix(ci): pass the approval status SHA as a step output Writing to $GITHUB_ENV under pull_request_target trips zizmor's github-env rule, which fails the code-scanning check. Step outputs carry the same values without granting later steps an attacker-shaped environment, and the resolved head SHA now also covers a workflow_dispatch run that fails after the pull request lookup. Signed-off-by: Jim Meyer --- .github/workflows/core-approval.yml | 32 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/.github/workflows/core-approval.yml b/.github/workflows/core-approval.yml index 39d5b7d54b..ced2820011 100644 --- a/.github/workflows/core-approval.yml +++ b/.github/workflows/core-approval.yml @@ -31,15 +31,8 @@ jobs: name: Publish core approval status if: github.repository_owner == 'NVIDIA' runs-on: ubuntu-latest - # Seeded so the guard step below can still publish a red status when the - # job fails before the publishing step resolves the head SHA itself — a - # checkout failure, or a transient API error. Step-level writes to - # $GITHUB_ENV override this for later steps, so post_status clearing it - # still works. Empty only on workflow_dispatch, where no SHA is known until - # the publishing step looks the pull request up. env: STATUS_CONTEXT: OpenShell / Core Approval - STATUS_SHA: ${{ github.event.pull_request.head.sha || github.event.merge_group.head_sha }} steps: # Check out the default branch, never the pull request head. This job runs # with a write-capable token, so it must not fetch or execute contributor @@ -55,6 +48,7 @@ jobs: persist-credentials: false - name: Publish core approval status + id: publish timeout-minutes: 10 env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -78,10 +72,9 @@ jobs: -f "description=$description" \ -f "target_url=$target_url" >/dev/null # A status was published, so the guard step has nothing to add. - # $GITHUB_ENV is read once after the step finishes and the last - # write wins, so this correctly reflects whether any status was - # posted. - echo "STATUS_SHA=" >> "$GITHUB_ENV" + # Step outputs are collected after the step finishes, including + # when it fails, so this reaches the guard either way. + echo "posted=true" >> "$GITHUB_OUTPUT" } # A merge group only forms after the pull request satisfied this gate, @@ -89,7 +82,6 @@ jobs: # success so the queue's required-check evaluation resolves instead of # waiting out check_response_timeout_minutes. if [ "$EVENT_NAME" = "merge_group" ]; then - echo "STATUS_SHA=$MERGE_GROUP_SHA" >> "$GITHUB_ENV" post_status "$MERGE_GROUP_SHA" success \ "Approval enforced at pull request" "$RUN_URL" exit 0 @@ -104,7 +96,7 @@ jobs: fi HEAD_SHA=$(jq -r '.head.sha' <<< "$PR") - echo "STATUS_SHA=$HEAD_SHA" >> "$GITHUB_ENV" + echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT" AUTHOR=$(jq -r '.user.login' <<< "$PR") TARGET_URL="https://github.com/$GH_REPO/pull/$PR_NUMBER" @@ -132,13 +124,25 @@ jobs: # would otherwise sit at "Expected" forever. Publish red so the state is # visible and the job can be re-run. - name: Publish a failure status if none was published - if: failure() && env.STATUS_SHA != '' + if: failure() && steps.publish.outputs.posted != 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_REPO: ${{ github.repository }} + # The step output covers a failure after the pull request was looked + # up, including on workflow_dispatch. The event payload covers a + # failure before that — a failed checkout, or a transient API error. + STATUS_SHA: ${{ steps.publish.outputs.head_sha || github.event.pull_request.head.sha || github.event.merge_group.head_sha }} shell: bash run: | set -euo pipefail + + # workflow_dispatch aborting before the lookup leaves no SHA to + # address. Nothing can be published; say so rather than failing here. + if [ -z "$STATUS_SHA" ]; then + echo "::warning::No head SHA resolved; cannot publish a failure status." + exit 0 + fi + gh api --method POST "repos/$GH_REPO/statuses/$STATUS_SHA" \ -f "state=failure" \ -f "context=$STATUS_CONTEXT" \