From 8b264030324b68f51aa7dc5615b66eb016fe4534 Mon Sep 17 00:00:00 2001 From: galuis116 Date: Wed, 15 Jul 2026 20:56:42 -0700 Subject: [PATCH 1/2] fix(pr-bot): classify renames of core paths as core trust-gate.yml, auto-merge.yml, and comment-command.yml built the changed-file list from `gh pr view --json files`, which is backed by the GraphQL PullRequestFile type and carries no previous-filename field. When GitHub detects a rename, only the new path appears there, so `git mv src/vouch/http_server.py src/vouch/web_server.py` (plus any edit inside it) made the changed core file invisible to pr_bot's exact- match CORE_GLOBS check: trust-gate reported no core paths touched for an untrusted author, and the PR was eligible for auto-merge arming as klass=code instead of being permanently blocked as klass=core. switch all three workflows to the REST pulls/{n}/files endpoint, which does populate previous_filename on renamed entries, and add a `changed-files` pr_bot subcommand that flattens that payload into both the new and previous filename so a rename can no longer drop a core path off the radar. Fixes #505 --- .github/workflows/auto-merge.yml | 5 +++- .github/workflows/comment-command.yml | 5 +++- .github/workflows/trust-gate.yml | 10 +++++-- CHANGELOG.md | 7 +++++ src/vouch/pr_bot.py | 28 ++++++++++++++++++ tests/test_pr_bot.py | 41 +++++++++++++++++++++++++++ 6 files changed, 91 insertions(+), 5 deletions(-) diff --git a/.github/workflows/auto-merge.yml b/.github/workflows/auto-merge.yml index 67d2c51f..777e17b1 100644 --- a/.github/workflows/auto-merge.yml +++ b/.github/workflows/auto-merge.yml @@ -68,7 +68,10 @@ jobs: REPO: ${{ github.repository }} PR: ${{ github.event.pull_request.number }} run: | - gh pr view "$PR" --repo "$REPO" --json files --jq '.files[].path' > changed.txt + # REST files endpoint carries previous_filename on renames; the + # GraphQL-backed `gh pr view --json files` shortcut does not. + gh api "repos/$REPO/pulls/$PR/files" --paginate > files.json + PYTHONPATH=src python -m vouch.pr_bot changed-files --json-file files.json > changed.txt klass=$(PYTHONPATH=src python -m vouch.pr_bot classify --files-file changed.txt --print-klass) echo "klass=$klass" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/comment-command.yml b/.github/workflows/comment-command.yml index 018bd5d9..96c6b8b2 100644 --- a/.github/workflows/comment-command.yml +++ b/.github/workflows/comment-command.yml @@ -51,7 +51,10 @@ jobs: PR: ${{ github.event.issue.number }} run: | head_sha="$(gh pr view "$PR" --repo "$REPO" --json headRefOid --jq .headRefOid)" - gh pr view "$PR" --repo "$REPO" --json files --jq '.files[].path' > changed.txt + # REST files endpoint carries previous_filename on renames; the + # GraphQL-backed `gh pr view --json files` shortcut does not. + gh api "repos/$REPO/pulls/$PR/files" --paginate > files.json + PYTHONPATH=src python -m vouch.pr_bot changed-files --json-file files.json > changed.txt klass="$(PYTHONPATH=src python -m vouch.pr_bot classify --files-file changed.txt --print-klass)" # mark authorized (visible, and enables deauthorize-on-push). a label # added via GITHUB_TOKEN does not re-trigger auto-merge.yml (GitHub's diff --git a/.github/workflows/trust-gate.yml b/.github/workflows/trust-gate.yml index e56ea0b9..3b3d85b7 100644 --- a/.github/workflows/trust-gate.yml +++ b/.github/workflows/trust-gate.yml @@ -20,10 +20,14 @@ jobs: - name: list changed files env: GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ github.event.pull_request.number }} run: | - gh pr view "${{ github.event.pull_request.number }}" \ - --repo "${{ github.repository }}" \ - --json files --jq '.files[].path' > changed.txt + # the REST files endpoint (unlike `gh pr view --json files`) carries + # previous_filename on renames — required so a rename that lands a + # core path under a new name still classifies as core. + gh api "repos/$REPO/pulls/$PR/files" --paginate > files.json + PYTHONPATH=src python -m vouch.pr_bot changed-files --json-file files.json > changed.txt - name: fail if an untrusted author touched core env: ASSOC: ${{ github.event.pull_request.author_association }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ff39a63..7924d670 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,13 @@ All notable changes to vouch are documented here. Format follows in config.yaml (#476). ### Fixed +- `pr_bot` core-path classification: `trust-gate.yml`, `auto-merge.yml`, + and `comment-command.yml` now source the changed-file list from the REST + `pulls/{n}/files` endpoint instead of `gh pr view --json files`, and feed + both the new and previous filename into classification. the GraphQL-backed + shortcut carries no rename metadata, so a `git mv` of a `CORE_GLOBS` path + (e.g. `http_server.py`) made the file invisible to the trust gate and + auto-merge arm check. (#505) - approve/reject/expire record the audit event *before* moving the proposal to decided/. a crash between the two used to leave a durable decision with no authoritative history; it now leaves a pending diff --git a/src/vouch/pr_bot.py b/src/vouch/pr_bot.py index 31b52b3b..8177973a 100644 --- a/src/vouch/pr_bot.py +++ b/src/vouch/pr_bot.py @@ -104,6 +104,26 @@ def _read_lines(path: str) -> list[str]: return [ln.strip() for ln in fh if ln.strip()] +def extract_changed_paths(files_json: str) -> list[str]: + """Flatten a REST ``/pulls/{n}/files`` payload into a path list. + + Emits both ``filename`` and, for a renamed entry, ``previous_filename`` — + a rename that lands a core path under a new name must still classify as + core. ``gh pr view --json files`` (the GraphQL-backed shortcut) carries no + previous-filename field and silently drops this; callers must use the + REST files endpoint (``gh api repos/{o}/{r}/pulls/{n}/files``) instead. + """ + paths: list[str] = [] + for entry in json.loads(files_json): + filename = entry.get("filename") + if filename: + paths.append(filename) + previous = entry.get("previous_filename") + if previous: + paths.append(previous) + return paths + + def main(argv: Sequence[str] | None = None) -> int: p = argparse.ArgumentParser(prog="vouch.pr_bot") sub = p.add_subparsers(dest="cmd", required=True) @@ -112,6 +132,9 @@ def main(argv: Sequence[str] | None = None) -> int: c.add_argument("--files-file", required=True) c.add_argument("--print-klass", action="store_true") + cf = sub.add_parser("changed-files") + cf.add_argument("--json-file", required=True) + for name in ("core-touched", "ui-touched"): sp = sub.add_parser(name) sp.add_argument("--files-file", required=True) @@ -135,6 +158,11 @@ def main(argv: Sequence[str] | None = None) -> int: changed = _read_lines(ns.files_file) sys.stdout.write(klass(changed) if ns.print_klass else json.dumps(classify(changed))) return 0 + if ns.cmd == "changed-files": + with open(ns.json_file, encoding="utf-8") as fh: + paths = extract_changed_paths(fh.read()) + sys.stdout.write("\n".join(paths)) + return 0 if ns.cmd == "core-touched": return 0 if classify(_read_lines(ns.files_file))["is_core"] else 1 if ns.cmd == "ui-touched": diff --git a/tests/test_pr_bot.py b/tests/test_pr_bot.py index 9d26baf8..b995f4f2 100644 --- a/tests/test_pr_bot.py +++ b/tests/test_pr_bot.py @@ -97,6 +97,47 @@ def test_cli_trust_exit_codes(): assert ok.returncode == 0 and bad.returncode == 1 +def test_extract_changed_paths_plain_file(): + files_json = '[{"filename": "src/vouch/context.py"}]' + assert pr_bot.extract_changed_paths(files_json) == ["src/vouch/context.py"] + + +def test_extract_changed_paths_includes_previous_filename_on_rename(): + files_json = ( + '[{"filename": "src/vouch/web_server.py", "status": "renamed", ' + '"previous_filename": "src/vouch/http_server.py"}]' + ) + assert pr_bot.extract_changed_paths(files_json) == [ + "src/vouch/web_server.py", "src/vouch/http_server.py", + ] + + +def test_rename_of_core_path_still_classifies_core(): + # a rename that lands a core path under a new name must not slip past + # trust-gate — the pre-rename path has to stay in the classified list. + files_json = ( + '[{"filename": "src/vouch/web_server.py", "status": "renamed", ' + '"previous_filename": "src/vouch/http_server.py"}]' + ) + changed = pr_bot.extract_changed_paths(files_json) + assert pr_bot.classify(changed)["is_core"] is True + + +def test_cli_changed_files_emits_previous_filename(tmp_path): + f = tmp_path / "files.json" + f.write_text( + '[{"filename": "src/vouch/web_server.py", "status": "renamed", ' + '"previous_filename": "src/vouch/http_server.py"}]', + encoding="utf-8", + ) + out = subprocess.run( + [sys.executable, "-m", "vouch.pr_bot", "changed-files", "--json-file", str(f)], + capture_output=True, text=True, check=True) + assert out.stdout.splitlines() == [ + "src/vouch/web_server.py", "src/vouch/http_server.py", + ] + + def test_codeowners_covers_every_core_glob(): text = Path(".github/CODEOWNERS").read_text(encoding="utf-8") for glob in pr_bot.CORE_GLOBS: From 0ab0e1443c5d7f8e16c54a77b4f747296e9ae0a8 Mon Sep 17 00:00:00 2001 From: galuis116 Date: Thu, 16 Jul 2026 07:32:40 -0700 Subject: [PATCH 2/2] style(pr-bot): lowercase extract_changed_paths docstring prose matches house style per CodeRabbit review on #506. --- src/vouch/pr_bot.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vouch/pr_bot.py b/src/vouch/pr_bot.py index 66b84650..bed94924 100644 --- a/src/vouch/pr_bot.py +++ b/src/vouch/pr_bot.py @@ -168,9 +168,9 @@ def _read_lines(path: str) -> list[str]: def extract_changed_paths(files_json: str) -> list[str]: - """Flatten a REST ``/pulls/{n}/files`` payload into a path list. + """flatten a REST ``/pulls/{n}/files`` payload into a path list. - Emits both ``filename`` and, for a renamed entry, ``previous_filename`` — + emits both ``filename`` and, for a renamed entry, ``previous_filename`` — a rename that lands a core path under a new name must still classify as core. ``gh pr view --json files`` (the GraphQL-backed shortcut) carries no previous-filename field and silently drops this; callers must use the