Skip to content

Commit db1b8b5

Browse files
purppurp
committed
feat(ci): add the maintainer approval decision tools
Adds the two stdlib-only helpers that back the maintainer approval gate, ahead of the workflows that call them. check_maintainer_approval.py parses the linked handles out of MAINTAINERS.md, folds a review list to each reviewer's latest decisive position, and exits non-zero unless a maintainer's latest position is an approval. It fails closed when the list yields no handles. alert_maintainer_change.py renders the approver-set delta between two versions of MAINTAINERS.md, and prints nothing when the set is unchanged. Landing these first lets the gate workflow, which reads both the list and the decision logic from main rather than from the pull request ref, actually execute on the pull request that introduces it. Signed-off-by: Jim Meyer <jimeyer@nvidia.com>
1 parent dfef088 commit db1b8b5

5 files changed

Lines changed: 380 additions & 0 deletions

File tree

‎tasks/scripts/alert_maintainer_change.py‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
#!/usr/bin/env python3
2+
# /// script
3+
# requires-python = ">=3.11"
4+
# dependencies = []
5+
# ///
6+
7+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
8+
# SPDX-License-Identifier: Apache-2.0
9+
10+
"""Print a review comment describing how a pull request changes the approver set
11+
to make sure that reviewers notice the change.
12+
13+
Prints nothing when the approver set is unchanged, and exits non-zero when the
14+
updated file yields no maintainers at all.
15+
16+
Runs as bare `python3` on the Actions runner, so it must stay stdlib-only.
17+
"""
18+
19+
from __future__ import annotations
20+
21+
import argparse
22+
import os
23+
import re
24+
import sys
25+
from pathlib import Path
26+
27+
# Only a login that appears as a link to a GitHub profile counts. A bare
28+
# "[@someone]" in prose must never widen the approver set. Keep this in step
29+
# with tasks/scripts/check_maintainer_approval.py, the gate this reports on.
30+
MAINTAINER_RE = re.compile(
31+
r"\[@([A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?)\]\(https://github\.com/"
32+
)
33+
34+
35+
def parse_maintainers(markdown: str) -> set[str]:
36+
"""Return the lowercased GitHub logins listed in a MAINTAINERS.md table."""
37+
return {match.group(1).lower() for match in MAINTAINER_RE.finditer(markdown)}
38+
39+
40+
def format_delta(marker: str, before: str, after: str) -> str:
41+
"""Render a review comment, or an empty string when nothing changed."""
42+
old, new = parse_maintainers(before), parse_maintainers(after)
43+
added, removed = sorted(new - old), sorted(old - new)
44+
if not added and not removed:
45+
return ""
46+
47+
lines = [marker, "## Maintainer list change", ""]
48+
if added:
49+
lines += ["**Gains approval rights:**", ""]
50+
lines += [f"- @{login}" for login in added]
51+
lines.append("")
52+
if removed:
53+
lines += ["**Loses approval rights:**", ""]
54+
lines += [f"- @{login}" for login in removed]
55+
lines.append("")
56+
lines.append(
57+
"Confirm every change is intended. Anyone listed here can single-handedly "
58+
"satisfy `OpenShell / Maintainer Approval`."
59+
)
60+
61+
if not new:
62+
lines += [
63+
"",
64+
"> [!WARNING]",
65+
"> No logins parse from the updated file. Merging this would make the "
66+
"approval gate fail closed on every pull request.",
67+
]
68+
return "\n".join(lines)
69+
70+
71+
def main(argv: list[str] | None = None) -> int:
72+
parser = argparse.ArgumentParser(description=__doc__)
73+
parser.add_argument(
74+
"--before", required=True, type=Path, help="MAINTAINERS.md at the base commit"
75+
)
76+
parser.add_argument(
77+
"--after", required=True, type=Path, help="MAINTAINERS.md at the head commit"
78+
)
79+
args = parser.parse_args(argv)
80+
81+
marker = os.environ.get("COMMENT_MARKER")
82+
if not marker:
83+
print("COMMENT_MARKER is not set", file=sys.stderr)
84+
return 1
85+
86+
after = args.after.read_text(encoding="utf-8")
87+
body = format_delta(marker, args.before.read_text(encoding="utf-8"), after)
88+
if body:
89+
print(body)
90+
91+
if not parse_maintainers(after):
92+
print(
93+
"No logins parse from the updated MAINTAINERS.md; the approval gate "
94+
"would fail closed on every pull request.",
95+
file=sys.stderr,
96+
)
97+
return 1
98+
return 0
99+
100+
101+
if __name__ == "__main__":
102+
sys.exit(main())

‎tasks/scripts/alert_maintainer_change_test.py‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
"""Tests for tasks/scripts/alert_maintainer_change.py.
5+
6+
Run via `mise run test:maintainer-approval`, which provides pytest through
7+
`uv run --with pytest`. pytest puts this file's directory on sys.path, so the
8+
sibling script imports directly.
9+
"""
10+
11+
from __future__ import annotations
12+
13+
import alert_maintainer_change as alert
14+
15+
MARKER = "<!-- maintainer-approval-delta -->"
16+
17+
TABLE = """# Maintainers
18+
19+
| Name | GitHub ID | Company/Organization |
20+
| --- | --- | --- |
21+
| Derek Carr | [@derekwaynecarr](https://github.com/derekwaynecarr) | Red Hat |
22+
| Evan Lezar | [@elezar](https://github.com/elezar) | NVIDIA |
23+
| Piotr Mlocek | [@pimlock](https://github.com/pimlock) | NVIDIA |
24+
"""
25+
26+
27+
def run(tmp_path, monkeypatch, before: str, after: str, marker: str = MARKER):
28+
"""Return the tool's exit code and the comment body it printed."""
29+
monkeypatch.setenv("COMMENT_MARKER", marker)
30+
before_file = tmp_path / "before.md"
31+
before_file.write_text(before, encoding="utf-8")
32+
after_file = tmp_path / "after.md"
33+
after_file.write_text(after, encoding="utf-8")
34+
return alert.main(["--before", str(before_file), "--after", str(after_file)])
35+
36+
37+
def test_parse_maintainers_extracts_linked_logins() -> None:
38+
assert alert.parse_maintainers(TABLE) == {"derekwaynecarr", "elezar", "pimlock"}
39+
40+
41+
def test_names_added_and_removed_logins() -> None:
42+
after = TABLE.replace(
43+
"| Piotr Mlocek | [@pimlock](https://github.com/pimlock) | NVIDIA |\n",
44+
"| Mrunal Patel | [@mrunalp](https://github.com/mrunalp) | Red Hat |\n",
45+
)
46+
body = alert.format_delta(MARKER, TABLE, after)
47+
assert "@mrunalp" in body
48+
assert "@pimlock" in body
49+
50+
51+
def test_says_nothing_when_only_prose_moves() -> None:
52+
# An unchanged approver set is not worth a comment.
53+
assert alert.format_delta(MARKER, TABLE, TABLE + "\nSee CONTRIBUTING.md.\n") == ""
54+
55+
56+
def test_prints_nothing_when_the_approver_set_is_unchanged(
57+
tmp_path, monkeypatch, capsys
58+
) -> None:
59+
assert run(tmp_path, monkeypatch, TABLE, TABLE) == 0
60+
assert capsys.readouterr().out == ""
61+
62+
63+
def test_fails_when_the_result_parses_empty(tmp_path, monkeypatch, capsys) -> None:
64+
# Merging this would make the approval gate fail closed on every PR.
65+
assert run(tmp_path, monkeypatch, TABLE, "# Maintainers\n\n- pimlock\n") != 0
66+
assert "WARNING" in capsys.readouterr().out
67+
68+
69+
def test_fails_when_the_marker_is_unset(tmp_path, monkeypatch) -> None:
70+
assert run(tmp_path, monkeypatch, TABLE, TABLE, marker="") != 0
71+
72+
73+
def test_body_starts_with_the_marker_the_workflow_supplies(
74+
tmp_path, monkeypatch, capsys
75+
) -> None:
76+
# The workflow finds its earlier comment with this prefix.
77+
after = TABLE + "| Jim Meyer | [@purp](https://github.com/purp) | NVIDIA |\n"
78+
assert run(tmp_path, monkeypatch, TABLE, after) == 0
79+
assert capsys.readouterr().out.startswith(MARKER)
80+
81+
82+
def test_login_pattern_matches_the_gate() -> None:
83+
# Each tool parses MAINTAINERS.md on its own. If the patterns drift, this
84+
# alert reports a delta that differs from what the gate enforces.
85+
import check_maintainer_approval as gate
86+
87+
assert alert.MAINTAINER_RE.pattern == gate.MAINTAINER_RE.pattern

‎tasks/scripts/check_maintainer_approval.py‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
#!/usr/bin/env python3
2+
# /// script
3+
# requires-python = ">=3.11"
4+
# dependencies = []
5+
# ///
6+
7+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
8+
# SPDX-License-Identifier: Apache-2.0
9+
10+
"""Exit non-zero unless a maintainer listed in MAINTAINERS.md has approved.
11+
12+
Runs as bare `python3` on the Actions runner, so it must stay stdlib-only.
13+
"""
14+
15+
from __future__ import annotations
16+
17+
import argparse
18+
import json
19+
import re
20+
import sys
21+
from pathlib import Path
22+
23+
# Only a login that appears as a link to a GitHub profile counts. A bare
24+
# "[@someone]" in prose must never widen the approver set. Keep this in step
25+
# with tasks/scripts/alert_maintainer_change.py, which reports the deltas this
26+
# gate enforces.
27+
MAINTAINER_RE = re.compile(
28+
r"\[@([A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?)\]\(https://github\.com/"
29+
)
30+
31+
# States that express a standing position. COMMENTED and PENDING leave a
32+
# reviewer's earlier approval intact, which is how GitHub itself treats them.
33+
DECISIVE_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED", "DISMISSED"})
34+
35+
36+
def parse_maintainers(markdown: str) -> set[str]:
37+
"""Return the lowercased GitHub logins listed in a MAINTAINERS.md table."""
38+
return {match.group(1).lower() for match in MAINTAINER_RE.finditer(markdown)}
39+
40+
41+
def parse_reviews_to_approvers(reviews: list[dict]) -> set[str]:
42+
"""Return the lowercased logins whose current review state is an approval.
43+
44+
A reviewer's latest decisive review supersedes their earlier ones, ordered
45+
by review id rather than by the order the caller assembled the pages in.
46+
"""
47+
positions: dict[str, str] = {}
48+
for entry in sorted(reviews, key=lambda r: r.get("id") or 0):
49+
state = str(entry.get("state") or "").upper()
50+
if state not in DECISIVE_STATES:
51+
continue
52+
login = str((entry.get("user") or {}).get("login") or "").lower()
53+
if login:
54+
positions[login] = state
55+
return {login for login, state in positions.items() if state == "APPROVED"}
56+
57+
58+
def main(argv: list[str] | None = None) -> int:
59+
parser = argparse.ArgumentParser(description=__doc__)
60+
parser.add_argument(
61+
"--maintainers",
62+
required=True,
63+
type=Path,
64+
help="MAINTAINERS.md read from the default branch",
65+
)
66+
parser.add_argument(
67+
"--reviews",
68+
required=True,
69+
type=Path,
70+
help="JSON array returned by the list-reviews API",
71+
)
72+
args = parser.parse_args(argv)
73+
74+
maintainers = parse_maintainers(args.maintainers.read_text(encoding="utf-8"))
75+
if not maintainers:
76+
# Fail closed: an unparseable list must never satisfy the gate.
77+
print("Could not parse any maintainers from MAINTAINERS.md")
78+
return 1
79+
80+
reviews = json.loads(args.reviews.read_text(encoding="utf-8"))
81+
approvers = sorted(parse_reviews_to_approvers(reviews) & maintainers)
82+
if not approvers:
83+
print("Needs approval from a maintainer listed in MAINTAINERS.md")
84+
return 1
85+
86+
print("Approved by " + ", ".join(f"@{login}" for login in approvers))
87+
return 0
88+
89+
90+
if __name__ == "__main__":
91+
sys.exit(main())

‎tasks/scripts/check_maintainer_approval_test.py‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
"""Tests for tasks/scripts/check_maintainer_approval.py.
5+
6+
Run via `mise run test:maintainer-approval`, which provides pytest through
7+
`uv run --with pytest`. pytest puts this file's directory on sys.path, so the
8+
sibling script imports directly.
9+
"""
10+
11+
from __future__ import annotations
12+
13+
import json
14+
15+
import check_maintainer_approval as gate
16+
17+
TABLE = """# Maintainers
18+
19+
| Name | GitHub ID | Company/Organization |
20+
| --- | --- | --- |
21+
| Derek Carr | [@derekwaynecarr](https://github.com/derekwaynecarr) | Red Hat |
22+
| Evan Lezar | [@elezar](https://github.com/elezar) | NVIDIA |
23+
| Piotr Mlocek | [@pimlock](https://github.com/pimlock) | NVIDIA |
24+
"""
25+
26+
27+
def review(login: str, state: str) -> dict:
28+
return {"user": {"login": login}, "state": state}
29+
30+
31+
def run(tmp_path, table: str, reviews: list[dict]) -> int:
32+
maintainers = tmp_path / "MAINTAINERS.md"
33+
maintainers.write_text(table, encoding="utf-8")
34+
reviews_file = tmp_path / "reviews.json"
35+
reviews_file.write_text(json.dumps(reviews), encoding="utf-8")
36+
return gate.main(
37+
["--maintainers", str(maintainers), "--reviews", str(reviews_file)]
38+
)
39+
40+
41+
def test_parse_maintainers_extracts_linked_logins() -> None:
42+
assert gate.parse_maintainers(TABLE) == {"derekwaynecarr", "elezar", "pimlock"}
43+
44+
45+
def test_parse_maintainers_ignores_unlinked_mentions() -> None:
46+
# A prose mention must not silently grant approval rights.
47+
prose = TABLE + "\nThanks to [@drive-by](mailto:nobody@example.com) too.\n"
48+
assert "drive-by" not in gate.parse_maintainers(prose)
49+
50+
51+
def test_comment_after_approval_does_not_revoke_it() -> None:
52+
reviews = [review("elezar", "APPROVED"), review("elezar", "COMMENTED")]
53+
assert gate.parse_reviews_to_approvers(reviews) == {"elezar"}
54+
55+
56+
def test_dismissed_review_revokes_approval() -> None:
57+
reviews = [review("elezar", "APPROVED"), review("elezar", "DISMISSED")]
58+
assert gate.parse_reviews_to_approvers(reviews) == set()
59+
60+
61+
def test_changes_requested_after_approval_revokes_it() -> None:
62+
reviews = [review("elezar", "APPROVED"), review("elezar", "CHANGES_REQUESTED")]
63+
assert gate.parse_reviews_to_approvers(reviews) == set()
64+
65+
66+
def test_out_of_order_reviews_still_respect_the_latest_position() -> None:
67+
# Ordering comes from the review id, not the order the caller happened
68+
# to assemble the pages in.
69+
reviews = [
70+
{"id": 2, "user": {"login": "elezar"}, "state": "DISMISSED"},
71+
{"id": 1, "user": {"login": "elezar"}, "state": "APPROVED"},
72+
]
73+
assert gate.parse_reviews_to_approvers(reviews) == set()
74+
75+
76+
def test_exits_zero_when_a_maintainer_approved(tmp_path) -> None:
77+
assert run(tmp_path, TABLE, [review("pimlock", "APPROVED")]) == 0
78+
79+
80+
def test_matches_logins_case_insensitively(tmp_path) -> None:
81+
assert run(tmp_path, TABLE, [review("PiMlOcK", "APPROVED")]) == 0
82+
83+
84+
def test_exits_nonzero_with_no_reviews(tmp_path) -> None:
85+
# The exit code is the check result, so this is the gate's actual contract.
86+
assert run(tmp_path, TABLE, []) != 0
87+
88+
89+
def test_exits_nonzero_on_non_maintainer_approval(tmp_path) -> None:
90+
assert run(tmp_path, TABLE, [review("outsider", "APPROVED")]) != 0
91+
92+
93+
def test_fails_closed_on_unparseable_list(tmp_path) -> None:
94+
assert run(tmp_path, "# Maintainers\n", [review("pimlock", "APPROVED")]) != 0

‎tasks/test.toml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ depends = [
2020
"test:codex-security-release-range",
2121
"test:docs-website",
2222
"test:docs-nav",
23+
"test:maintainer-approval",
2324
]
2425

2526
["test:docs-website"]
@@ -78,6 +79,11 @@ description = "Test Codex Security release-range resolution"
7879
run = "uv run --no-project --with pytest pytest -o \"python_files=*_test.py\" tasks/scripts/codex_security_range_test.py"
7980
hide = true
8081

82+
["test:maintainer-approval"]
83+
description = "Test the maintainer approval gate and change-alert tools"
84+
run = "uv run --no-project --with pytest pytest -o \"python_files=*_test.py\" tasks/scripts/check_maintainer_approval_test.py tasks/scripts/alert_maintainer_change_test.py"
85+
hide = true
86+
8187
[e2e]
8288
description = "Run all end-to-end tests (Rust + Python + MCP)"
8389
depends = ["e2e:rust", "e2e:python", "e2e:mcp"]

0 commit comments

Comments
 (0)