diff --git a/.github/workflows/auto-merge.yml b/.github/workflows/auto-merge.yml index 7893b1c4..d0a28244 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 6f86a57d..dfee025f 100644 --- a/.github/workflows/comment-command.yml +++ b/.github/workflows/comment-command.yml @@ -50,7 +50,10 @@ jobs: REPO: ${{ github.repository }} PR: ${{ github.event.issue.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)" # 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 d405b36e..0c37f7b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,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 2f735c8c..bed94924 100644 --- a/src/vouch/pr_bot.py +++ b/src/vouch/pr_bot.py @@ -167,6 +167,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) @@ -175,6 +195,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) @@ -203,6 +226,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 34a2b9e8..45d11842 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: