From 3a851dd584cd01e4b41cb37d7deb755ee006aa02 Mon Sep 17 00:00:00 2001 From: AztecBot Date: Thu, 10 Sep 2026 14:54:40 +0000 Subject: [PATCH 1/4] chore: request code owners only on pull requests that reach next --- CODEOWNERS => .github/next-code-owners | 18 +++ .github/scripts/next_code_owners.py | 137 ++++++++++++++++++++ .github/workflows/codeowner-notify-next.yml | 58 +++++++++ 3 files changed, 213 insertions(+) rename CODEOWNERS => .github/next-code-owners (80%) create mode 100644 .github/scripts/next_code_owners.py create mode 100644 .github/workflows/codeowner-notify-next.yml diff --git a/CODEOWNERS b/.github/next-code-owners similarity index 80% rename from CODEOWNERS rename to .github/next-code-owners index 269d761491bd..53324d841e7f 100644 --- a/CODEOWNERS +++ b/.github/next-code-owners @@ -1,3 +1,21 @@ +# Code owners, requested for review only on pull requests whose merge reaches +# `next`: pull requests based on `next`, and pull requests into a merge train +# that itself targets `next` (every `merge-train/*` except the `-v` ones, which +# target their release line — the rule in merge-train-create-pr.yml). +# +# This file is deliberately NOT named CODEOWNERS. GitHub reads CODEOWNERS from the +# pull request's base branch and offers no way to scope an entry to particular +# bases, so a native CODEOWNERS also requests owners on pull requests stacked +# onto feature branches — every feature branch cut from `next` carries a copy — +# and re-requests them each time the stack is rebased. Owners here are requested +# by .github/workflows/codeowner-notify-next.yml instead, which applies exactly +# this file with CODEOWNERS semantics (last match wins, per changed file) and +# nothing else. Add or change owners here as you would in CODEOWNERS. +# +# What is given up by not using the native file: the "code owner" badge on +# review requests, owner hints in the Files-changed view, and the +# require_code_owner_review ruleset option, which needs a real CODEOWNERS. + /.devcontainer/ @charlielye @ludamad /.github/ @charlielye @ludamad /build-images/ @charlielye @ludamad diff --git a/.github/scripts/next_code_owners.py b/.github/scripts/next_code_owners.py new file mode 100644 index 000000000000..376f39f8f348 --- /dev/null +++ b/.github/scripts/next_code_owners.py @@ -0,0 +1,137 @@ +"""Apply .github/next-code-owners to a pull request, with CODEOWNERS semantics. + +Reads the owners file and the list of changed paths, and prints a JSON body for +`POST /repos/{owner}/{repo}/pulls/{n}/requested_reviewers` — or nothing at all +when nobody should be requested. Nothing is requested when the pull request's +merge does not reach `next`, when no changed path has an owner, or when the base +branch carries no owners file (a merge train that has not yet pulled `next`). + +Semantics match GitHub's CODEOWNERS: for each changed path the *last* matching +pattern wins, and the pull request's reviewers are the union of those winners +across all changed paths. Patterns follow CODEOWNERS/gitignore rules — a leading +`/` anchors at the repository root, an unanchored pattern matches at any depth, a +trailing `/` means everything beneath a directory, `*` stays within one path +segment and `**` crosses segments. + +The pull request's author is dropped: GitHub refuses to request the author as a +reviewer, and asking it to fails the whole request. +""" + +import json +import re +import sys + +# A merge train's own pull request targets `next`, unless the train is suffixed +# -v, in which case it targets that release line. This mirrors the base_branch +# rule in .github/workflows/merge-train-create-pr.yml; the two must agree about +# where a train ends up. +TRAIN_TO_RELEASE_LINE = re.compile(r"^merge-train/.*-v[0-9]+$") + + +def reaches_next(base_ref: str) -> bool: + if base_ref == "next": + return True + if not base_ref.startswith("merge-train/"): + return False + return not TRAIN_TO_RELEASE_LINE.match(base_ref) + + +def pattern_to_regex(pattern: str) -> re.Pattern: + anchored = pattern.startswith("/") + body = pattern.lstrip("/") + directory_only = body.endswith("/") + body = body.rstrip("/") + + out = [] + i = 0 + while i < len(body): + char = body[i] + if body.startswith("**", i): + out.append(".*") + i += 2 + if i < len(body) and body[i] == "/": + i += 1 + continue + if char == "*": + out.append("[^/]*") + elif char == "?": + out.append("[^/]") + else: + out.append(re.escape(char)) + i += 1 + + # A pattern without a slash in its body (e.g. `*.js`) matches a basename at + # any depth; one with a slash is relative to the root, like gitignore. + if anchored or "/" in body: + prefix = "^" + else: + prefix = "(?:^|.*/)" + # A directory pattern matches everything beneath it; a file pattern matches + # the file itself, or a directory of that name and everything beneath it, + # which is how CODEOWNERS treats `/docs`. + suffix = "/.*" if directory_only else "(?:/.*)?$" + return re.compile(prefix + "".join(out) + suffix) + + +def load_rules(owners_file: str) -> list[tuple[re.Pattern, list[str]]]: + rules = [] + with open(owners_file, encoding="utf-8") as handle: + for line in handle: + line = line.split("#", 1)[0].strip() + if not line: + continue + pattern, *owners = line.split() + rules.append((pattern_to_regex(pattern), owners)) + return rules + + +def owners_for(path: str, rules: list[tuple[re.Pattern, list[str]]]) -> list[str]: + """The owners of the last pattern that matches `path`; empty when none does or + the winning pattern deliberately names nobody.""" + winner: list[str] = [] + for regex, owners in rules: + if regex.match(path): + winner = owners + return winner + + +def main() -> int: + owners_file, changed_file, author, base_ref = sys.argv[1:5] + + if not reaches_next(base_ref): + return 0 + + try: + rules = load_rules(owners_file) + except FileNotFoundError: + return 0 + + with open(changed_file, encoding="utf-8") as handle: + changed = [line.strip() for line in handle if line.strip()] + + users: list[str] = [] + teams: list[str] = [] + for path in changed: + for owner in owners_for(path, rules): + handle = owner.lstrip("@") + if "/" in handle: + slug = handle.split("/", 1)[1] + if slug not in teams: + teams.append(slug) + elif handle != author and handle not in users: + users.append(handle) + + if not users and not teams: + return 0 + + body = {} + if users: + body["reviewers"] = users + if teams: + body["team_reviewers"] = teams + print(json.dumps(body)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/workflows/codeowner-notify-next.yml b/.github/workflows/codeowner-notify-next.yml new file mode 100644 index 000000000000..8a1fd2088286 --- /dev/null +++ b/.github/workflows/codeowner-notify-next.yml @@ -0,0 +1,58 @@ +name: Code-owner notify (next) + +# Requests the owners in .github/next-code-owners for pull requests whose merge +# reaches `next` — `next` itself, and the merge trains that target it. This +# replaces a native CODEOWNERS, which GitHub reads from the base branch with no +# way to scope it, so it also fires on pull requests stacked onto feature +# branches and fires again on every rebase of the stack. +# +# Three moments matter: a pull request is opened non-draft, a draft is marked +# ready, or a stacked pull request is retargeted onto `next` once the one below +# it has merged. `edited` covers the last; the job guard limits it to edits that +# changed the base, so a title or body edit does not re-request anyone. +on: + pull_request_target: + types: [opened, ready_for_review, edited] + branches: + - next + - 'merge-train/**' + +permissions: + pull-requests: write + +jobs: + request-owners: + if: >- + ${{ !github.event.pull_request.draft + && (github.event.action != 'edited' || github.event.changes.base.ref.from) }} + runs-on: ubuntu-latest + steps: + # pull_request_target checks out the base branch, never the pull request's + # head, so the elevated token is not exposed to code from the pull request. + - uses: actions/checkout@v4 + with: + sparse-checkout: | + .github/next-code-owners + .github/scripts/next_code_owners.py + sparse-checkout-cone-mode: false + + - name: Request the owners of the changed paths + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + set -euo pipefail + gh api --paginate "/repos/$REPO/pulls/$PR_NUMBER/files" \ + --jq '.[].filename' > changed-files.txt + body=$(python3 .github/scripts/next_code_owners.py \ + .github/next-code-owners changed-files.txt "$PR_AUTHOR" "$BASE_REF") + if [ -z "$body" ]; then + echo "Nothing to request for base $BASE_REF." + exit 0 + fi + echo "Requesting: $body" + printf '%s' "$body" | gh api --method POST --input - \ + "/repos/$REPO/pulls/$PR_NUMBER/requested_reviewers" From 8de942b5404a0d89cb6ead87eca6b17e7000f4c6 Mon Sep 17 00:00:00 2001 From: AztecBot Date: Thu, 10 Sep 2026 14:56:35 +0000 Subject: [PATCH 2/4] review: fix checkout permission, pin checkout SHA, simplify glob matcher, log why nothing was requested --- .github/next-code-owners | 24 ++-- .github/scripts/next_code_owners.py | 135 +++++++++----------- .github/workflows/codeowner-notify-next.yml | 6 +- 3 files changed, 72 insertions(+), 93 deletions(-) diff --git a/.github/next-code-owners b/.github/next-code-owners index 53324d841e7f..4bf1d4962291 100644 --- a/.github/next-code-owners +++ b/.github/next-code-owners @@ -1,20 +1,16 @@ # Code owners, requested for review only on pull requests whose merge reaches -# `next`: pull requests based on `next`, and pull requests into a merge train -# that itself targets `next` (every `merge-train/*` except the `-v` ones, which -# target their release line — the rule in merge-train-create-pr.yml). +# `next`: those based on `next`, and those into a merge train that targets `next` +# (every `merge-train/*` except the `-v` ones, which target a release line). # -# This file is deliberately NOT named CODEOWNERS. GitHub reads CODEOWNERS from the -# pull request's base branch and offers no way to scope an entry to particular -# bases, so a native CODEOWNERS also requests owners on pull requests stacked -# onto feature branches — every feature branch cut from `next` carries a copy — -# and re-requests them each time the stack is rebased. Owners here are requested -# by .github/workflows/codeowner-notify-next.yml instead, which applies exactly -# this file with CODEOWNERS semantics (last match wins, per changed file) and -# nothing else. Add or change owners here as you would in CODEOWNERS. +# Deliberately NOT named CODEOWNERS. GitHub reads CODEOWNERS from a pull request's +# base branch with no way to scope it, so a native file also requests owners on +# pull requests stacked onto feature branches, and again on every rebase of the +# stack. .github/workflows/codeowner-notify-next.yml applies this file instead, +# with CODEOWNERS semantics (last match wins, per changed path). Edit it exactly +# as you would CODEOWNERS. # -# What is given up by not using the native file: the "code owner" badge on -# review requests, owner hints in the Files-changed view, and the -# require_code_owner_review ruleset option, which needs a real CODEOWNERS. +# Given up by not using the native name: the "code owner" badge, owner hints in +# the Files-changed view, and the require_code_owner_review ruleset option. /.devcontainer/ @charlielye @ludamad /.github/ @charlielye @ludamad diff --git a/.github/scripts/next_code_owners.py b/.github/scripts/next_code_owners.py index 376f39f8f348..07ab5e427cf3 100644 --- a/.github/scripts/next_code_owners.py +++ b/.github/scripts/next_code_owners.py @@ -1,129 +1,110 @@ """Apply .github/next-code-owners to a pull request, with CODEOWNERS semantics. -Reads the owners file and the list of changed paths, and prints a JSON body for -`POST /repos/{owner}/{repo}/pulls/{n}/requested_reviewers` — or nothing at all -when nobody should be requested. Nothing is requested when the pull request's -merge does not reach `next`, when no changed path has an owner, or when the base -branch carries no owners file (a merge train that has not yet pulled `next`). - -Semantics match GitHub's CODEOWNERS: for each changed path the *last* matching -pattern wins, and the pull request's reviewers are the union of those winners -across all changed paths. Patterns follow CODEOWNERS/gitignore rules — a leading -`/` anchors at the repository root, an unanchored pattern matches at any depth, a -trailing `/` means everything beneath a directory, `*` stays within one path -segment and `**` crosses segments. - -The pull request's author is dropped: GitHub refuses to request the author as a -reviewer, and asking it to fails the whole request. +Usage: next_code_owners.py OWNERS_FILE CHANGED_PATHS_FILE PR_AUTHOR BASE_REF + +Prints a JSON body for `POST /repos/{owner}/{repo}/pulls/{n}/requested_reviewers`, +or nothing when nobody should be requested. The reason is logged to stderr. + +Matching follows GitHub's CODEOWNERS: for each changed path the *last* matching +pattern wins, and the reviewers are the union of those winners. Patterns are +gitignore-style — leading `/` anchors at the root, a trailing `/` means the whole +directory, `*` stays within a path segment, `**` crosses segments. """ import json import re import sys -# A merge train's own pull request targets `next`, unless the train is suffixed -# -v, in which case it targets that release line. This mirrors the base_branch -# rule in .github/workflows/merge-train-create-pr.yml; the two must agree about -# where a train ends up. +# Where a merge train's own pull request lands. A train targets `next` unless its +# name is suffixed -v, in which case it targets that release line. This mirrors +# the base_branch rule in .github/workflows/merge-train-create-pr.yml; keep them +# in step. TRAIN_TO_RELEASE_LINE = re.compile(r"^merge-train/.*-v[0-9]+$") -def reaches_next(base_ref: str) -> bool: +def reaches_next(base_ref): if base_ref == "next": return True - if not base_ref.startswith("merge-train/"): - return False - return not TRAIN_TO_RELEASE_LINE.match(base_ref) + return base_ref.startswith("merge-train/") and not TRAIN_TO_RELEASE_LINE.match(base_ref) -def pattern_to_regex(pattern: str) -> re.Pattern: +def pattern_to_regex(pattern): anchored = pattern.startswith("/") - body = pattern.lstrip("/") - directory_only = body.endswith("/") - body = body.rstrip("/") - - out = [] - i = 0 - while i < len(body): - char = body[i] - if body.startswith("**", i): - out.append(".*") - i += 2 - if i < len(body) and body[i] == "/": - i += 1 - continue - if char == "*": - out.append("[^/]*") - elif char == "?": - out.append("[^/]") + directory_only = pattern.endswith("/") + body = pattern.strip("/") + + parts = [] + for token in re.split(r"(\*\*/|\*\*|\*|\?)", body): + if token == "**/": + parts.append("(?:.*/)?") + elif token == "**": + parts.append(".*") + elif token == "*": + parts.append("[^/]*") + elif token == "?": + parts.append("[^/]") else: - out.append(re.escape(char)) - i += 1 - - # A pattern without a slash in its body (e.g. `*.js`) matches a basename at - # any depth; one with a slash is relative to the root, like gitignore. - if anchored or "/" in body: - prefix = "^" - else: - prefix = "(?:^|.*/)" - # A directory pattern matches everything beneath it; a file pattern matches - # the file itself, or a directory of that name and everything beneath it, - # which is how CODEOWNERS treats `/docs`. + parts.append(re.escape(token)) + + # As in gitignore: a pattern with no slash (`*.js`) matches at any depth; one + # with a slash is relative to the root. + prefix = "^" if anchored or "/" in body else "(?:^|.*/)" + # A directory pattern matches everything beneath it. A file pattern also + # matches a directory of that name, which is how CODEOWNERS treats `/docs`. suffix = "/.*" if directory_only else "(?:/.*)?$" - return re.compile(prefix + "".join(out) + suffix) + return re.compile(prefix + "".join(parts) + suffix) -def load_rules(owners_file: str) -> list[tuple[re.Pattern, list[str]]]: +def load_rules(owners_file): rules = [] with open(owners_file, encoding="utf-8") as handle: for line in handle: line = line.split("#", 1)[0].strip() - if not line: - continue - pattern, *owners = line.split() - rules.append((pattern_to_regex(pattern), owners)) + if line: + pattern, *owners = line.split() + rules.append((pattern_to_regex(pattern), owners)) return rules -def owners_for(path: str, rules: list[tuple[re.Pattern, list[str]]]) -> list[str]: - """The owners of the last pattern that matches `path`; empty when none does or - the winning pattern deliberately names nobody.""" - winner: list[str] = [] +def owners_for(path, rules): + """Owners of the last pattern matching `path`; empty when none does.""" + winner = [] for regex, owners in rules: if regex.match(path): winner = owners return winner -def main() -> int: +def main(): owners_file, changed_file, author, base_ref = sys.argv[1:5] if not reaches_next(base_ref): + print(f"base {base_ref} does not reach next", file=sys.stderr) return 0 - try: rules = load_rules(owners_file) except FileNotFoundError: + print(f"{owners_file} is absent on {base_ref}; nothing to apply", file=sys.stderr) return 0 - with open(changed_file, encoding="utf-8") as handle: changed = [line.strip() for line in handle if line.strip()] - users: list[str] = [] - teams: list[str] = [] + users, teams = [], [] for path in changed: for owner in owners_for(path, rules): - handle = owner.lstrip("@") - if "/" in handle: - slug = handle.split("/", 1)[1] - if slug not in teams: - teams.append(slug) - elif handle != author and handle not in users: - users.append(handle) + name = owner.lstrip("@") + if "/" in name: # @org/team + target, name = teams, name.split("/", 1)[1] + else: + target = users + if name == author: # GitHub refuses to request the author + continue + if name not in target: + target.append(name) if not users and not teams: + print("no changed path has an owner", file=sys.stderr) return 0 - body = {} if users: body["reviewers"] = users diff --git a/.github/workflows/codeowner-notify-next.yml b/.github/workflows/codeowner-notify-next.yml index 8a1fd2088286..7b34dbebe9da 100644 --- a/.github/workflows/codeowner-notify-next.yml +++ b/.github/workflows/codeowner-notify-next.yml @@ -18,7 +18,8 @@ on: - 'merge-train/**' permissions: - pull-requests: write + contents: read # checkout + pull-requests: write # requested_reviewers jobs: request-owners: @@ -29,7 +30,8 @@ jobs: steps: # pull_request_target checks out the base branch, never the pull request's # head, so the elevated token is not exposed to code from the pull request. - - uses: actions/checkout@v4 + # Sparse: the repository is large and only two files are needed. + - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 with: sparse-checkout: | .github/next-code-owners From 410c243f51d10d12e4cd5cd1e77546a0ef797e22 Mon Sep 17 00:00:00 2001 From: AztecBot Date: Fri, 11 Sep 2026 09:03:56 +0000 Subject: [PATCH 3/4] tag owners in a comment naming the paths, instead of requesting review --- .github/next-code-owners | 8 +-- .github/scripts/next_code_owners.py | 60 ++++++++++----------- .github/workflows/codeowner-notify-next.yml | 31 ++++++----- 3 files changed, 49 insertions(+), 50 deletions(-) diff --git a/.github/next-code-owners b/.github/next-code-owners index 4bf1d4962291..bc6e995fb3fd 100644 --- a/.github/next-code-owners +++ b/.github/next-code-owners @@ -1,6 +1,7 @@ -# Code owners, requested for review only on pull requests whose merge reaches +# Code owners, tagged in a comment only on pull requests whose merge reaches # `next`: those based on `next`, and those into a merge train that targets `next` # (every `merge-train/*` except the `-v` ones, which target a release line). +# The comment names the path each owner came from, so the author can see why. # # Deliberately NOT named CODEOWNERS. GitHub reads CODEOWNERS from a pull request's # base branch with no way to scope it, so a native file also requests owners on @@ -9,8 +10,9 @@ # with CODEOWNERS semantics (last match wins, per changed path). Edit it exactly # as you would CODEOWNERS. # -# Given up by not using the native name: the "code owner" badge, owner hints in -# the Files-changed view, and the require_code_owner_review ruleset option. +# Given up by not using the native name: owners are not added as requested +# reviewers (no Reviewers-sidebar entry or review-queue item), and the +# require_code_owner_review ruleset option is unavailable. /.devcontainer/ @charlielye @ludamad /.github/ @charlielye @ludamad diff --git a/.github/scripts/next_code_owners.py b/.github/scripts/next_code_owners.py index 07ab5e427cf3..86506b0516c4 100644 --- a/.github/scripts/next_code_owners.py +++ b/.github/scripts/next_code_owners.py @@ -1,9 +1,10 @@ """Apply .github/next-code-owners to a pull request, with CODEOWNERS semantics. -Usage: next_code_owners.py OWNERS_FILE CHANGED_PATHS_FILE PR_AUTHOR BASE_REF +Usage: next_code_owners.py OWNERS_FILE CHANGED_PATHS_FILE BASE_REF -Prints a JSON body for `POST /repos/{owner}/{repo}/pulls/{n}/requested_reviewers`, -or nothing when nobody should be requested. The reason is logged to stderr. +Prints a Markdown comment that @-mentions the owners of the changed paths, one line +per owning rule so the author can see which paths brought in whom. Prints nothing +when nobody should be told, and logs the reason to stderr. Matching follows GitHub's CODEOWNERS: for each changed path the *last* matching pattern wins, and the reviewers are the union of those winners. Patterns are @@ -11,7 +12,6 @@ directory, `*` stays within a path segment, `**` crosses segments. """ -import json import re import sys @@ -56,27 +56,28 @@ def pattern_to_regex(pattern): def load_rules(owners_file): + """Each rule is (regex, pattern text, owners), in file order.""" rules = [] with open(owners_file, encoding="utf-8") as handle: for line in handle: line = line.split("#", 1)[0].strip() if line: pattern, *owners = line.split() - rules.append((pattern_to_regex(pattern), owners)) + rules.append((pattern_to_regex(pattern), pattern, owners)) return rules -def owners_for(path, rules): - """Owners of the last pattern matching `path`; empty when none does.""" - winner = [] - for regex, owners in rules: - if regex.match(path): - winner = owners +def winning_rule(path, rules): + """The last rule whose pattern matches `path`, or None.""" + winner = None + for rule in rules: + if rule[0].match(path): + winner = rule return winner def main(): - owners_file, changed_file, author, base_ref = sys.argv[1:5] + owners_file, changed_file, base_ref = sys.argv[1:4] if not reaches_next(base_ref): print(f"base {base_ref} does not reach next", file=sys.stderr) @@ -89,28 +90,25 @@ def main(): with open(changed_file, encoding="utf-8") as handle: changed = [line.strip() for line in handle if line.strip()] - users, teams = [], [] + # One line per owning rule, in file order, so the author sees which of their + # paths brought in which owners. A rule that names nobody owns nothing. + hit = [] for path in changed: - for owner in owners_for(path, rules): - name = owner.lstrip("@") - if "/" in name: # @org/team - target, name = teams, name.split("/", 1)[1] - else: - target = users - if name == author: # GitHub refuses to request the author - continue - if name not in target: - target.append(name) - - if not users and not teams: + rule = winning_rule(path, rules) + if rule and rule[2] and rule not in hit: + hit.append(rule) + if not hit: print("no changed path has an owner", file=sys.stderr) return 0 - body = {} - if users: - body["reviewers"] = users - if teams: - body["team_reviewers"] = teams - print(json.dumps(body)) + + lines = [ + "This pull request touches code with owners listed in " + "`.github/next-code-owners`. Tagging them for review:", + "", + ] + for _, pattern, owners in sorted(hit, key=rules.index): + lines.append(f"- {' '.join(owners)} — `{pattern}`") + print("\n".join(lines)) return 0 diff --git a/.github/workflows/codeowner-notify-next.yml b/.github/workflows/codeowner-notify-next.yml index 7b34dbebe9da..1edfe5cbe9e8 100644 --- a/.github/workflows/codeowner-notify-next.yml +++ b/.github/workflows/codeowner-notify-next.yml @@ -1,15 +1,16 @@ name: Code-owner notify (next) -# Requests the owners in .github/next-code-owners for pull requests whose merge -# reaches `next` — `next` itself, and the merge trains that target it. This -# replaces a native CODEOWNERS, which GitHub reads from the base branch with no -# way to scope it, so it also fires on pull requests stacked onto feature -# branches and fires again on every rebase of the stack. +# Comments on pull requests whose merge reaches `next` — `next` itself, and the +# merge trains that target it — tagging the owners of the changed paths from +# .github/next-code-owners, with the path each owner came from. This replaces a +# native CODEOWNERS, which GitHub reads from the base branch with no way to scope +# it, so it also fires on pull requests stacked onto feature branches and again +# on every rebase of the stack. # # Three moments matter: a pull request is opened non-draft, a draft is marked # ready, or a stacked pull request is retargeted onto `next` once the one below # it has merged. `edited` covers the last; the job guard limits it to edits that -# changed the base, so a title or body edit does not re-request anyone. +# changed the base, so a title or body edit does not tag anyone again. on: pull_request_target: types: [opened, ready_for_review, edited] @@ -19,7 +20,7 @@ on: permissions: contents: read # checkout - pull-requests: write # requested_reviewers + pull-requests: write # gh pr comment jobs: request-owners: @@ -38,23 +39,21 @@ jobs: .github/scripts/next_code_owners.py sparse-checkout-cone-mode: false - - name: Request the owners of the changed paths + - name: Tag the owners of the changed paths env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} REPO: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }} - PR_AUTHOR: ${{ github.event.pull_request.user.login }} BASE_REF: ${{ github.event.pull_request.base.ref }} run: | set -euo pipefail gh api --paginate "/repos/$REPO/pulls/$PR_NUMBER/files" \ --jq '.[].filename' > changed-files.txt - body=$(python3 .github/scripts/next_code_owners.py \ - .github/next-code-owners changed-files.txt "$PR_AUTHOR" "$BASE_REF") - if [ -z "$body" ]; then - echo "Nothing to request for base $BASE_REF." + python3 .github/scripts/next_code_owners.py \ + .github/next-code-owners changed-files.txt "$BASE_REF" > comment.md + if [ ! -s comment.md ]; then + echo "Nobody to tag for base $BASE_REF." exit 0 fi - echo "Requesting: $body" - printf '%s' "$body" | gh api --method POST --input - \ - "/repos/$REPO/pulls/$PR_NUMBER/requested_reviewers" + cat comment.md + gh pr comment "$PR_NUMBER" --repo "$REPO" --body-file comment.md From e550c182eaafb5b50619220f6b37ac73f3490054 Mon Sep 17 00:00:00 2001 From: AztecBot Date: Fri, 11 Sep 2026 11:57:43 +0000 Subject: [PATCH 4/4] review: drop review-request leftovers, index winning rules instead of re-sorting tuples --- .github/scripts/next_code_owners.py | 24 ++++++++++----------- .github/workflows/codeowner-notify-next.yml | 2 +- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/.github/scripts/next_code_owners.py b/.github/scripts/next_code_owners.py index 86506b0516c4..eaca94012d50 100644 --- a/.github/scripts/next_code_owners.py +++ b/.github/scripts/next_code_owners.py @@ -7,7 +7,7 @@ when nobody should be told, and logs the reason to stderr. Matching follows GitHub's CODEOWNERS: for each changed path the *last* matching -pattern wins, and the reviewers are the union of those winners. Patterns are +pattern wins, and every rule that wins for some path gets a line. Patterns are gitignore-style — leading `/` anchors at the root, a trailing `/` means the whole directory, `*` stays within a path segment, `**` crosses segments. """ @@ -68,11 +68,11 @@ def load_rules(owners_file): def winning_rule(path, rules): - """The last rule whose pattern matches `path`, or None.""" + """Index of the last rule whose pattern matches `path`, or None.""" winner = None - for rule in rules: - if rule[0].match(path): - winner = rule + for i, (regex, _, _) in enumerate(rules): + if regex.match(path): + winner = i return winner @@ -90,14 +90,11 @@ def main(): with open(changed_file, encoding="utf-8") as handle: changed = [line.strip() for line in handle if line.strip()] - # One line per owning rule, in file order, so the author sees which of their + # One line per winning rule, in file order, so the author sees which of their # paths brought in which owners. A rule that names nobody owns nothing. - hit = [] - for path in changed: - rule = winning_rule(path, rules) - if rule and rule[2] and rule not in hit: - hit.append(rule) - if not hit: + winners = {winning_rule(path, rules) for path in changed} - {None} + winners = [i for i in sorted(winners) if rules[i][2]] + if not winners: print("no changed path has an owner", file=sys.stderr) return 0 @@ -106,7 +103,8 @@ def main(): "`.github/next-code-owners`. Tagging them for review:", "", ] - for _, pattern, owners in sorted(hit, key=rules.index): + for i in winners: + _, pattern, owners = rules[i] lines.append(f"- {' '.join(owners)} — `{pattern}`") print("\n".join(lines)) return 0 diff --git a/.github/workflows/codeowner-notify-next.yml b/.github/workflows/codeowner-notify-next.yml index 1edfe5cbe9e8..146bb71087bf 100644 --- a/.github/workflows/codeowner-notify-next.yml +++ b/.github/workflows/codeowner-notify-next.yml @@ -23,7 +23,7 @@ permissions: pull-requests: write # gh pr comment jobs: - request-owners: + tag-owners: if: >- ${{ !github.event.pull_request.draft && (github.event.action != 'edited' || github.event.changes.base.ref.from) }}