From d68f9f6c449e86974460d151d68c5eeff7202f5d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:07:02 +0400 Subject: [PATCH 1/9] Read the tracking issue number from a PR description's first line Add .github/scripts/check_pr_description.py with issue_number_from_description(), which returns N when the first line of a pull request description is exactly "Fixes #N" and otherwise raises an error whose message says how to fix the description. - A line ends at CRLF, a lone carriage return or a line feed, as in Markdown, so descriptions edited in the web interface pass. - Trailing spaces and tabs are ignored; leading whitespace fails. - An empty or null description fails. - The number is ASCII digits without a leading zero. - A "Fixes #N" line further down is reported with its line number. Part of #861. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_pr_description.py | 77 ++++++++++ .../tests/test_pr_description_check.py | 143 ++++++++++++++++++ 2 files changed, 220 insertions(+) create mode 100644 .github/scripts/check_pr_description.py create mode 100644 projects/policyengine-simulation-entry/tests/test_pr_description_check.py diff --git a/.github/scripts/check_pr_description.py b/.github/scripts/check_pr_description.py new file mode 100644 index 000000000..b36478cd6 --- /dev/null +++ b/.github/scripts/check_pr_description.py @@ -0,0 +1,77 @@ +"""Check that a pull request's description names its tracking issue. + +Rule sim.process.pr-tracking-issue: the first line of every pull request's +description is ``Fixes #ISSUE_NUMBER``. + +How the first line is read: + +- A line ends at a CRLF, a carriage return or a line feed, as in Markdown, so + descriptions edited in the GitHub web interface (which uses CRLF) pass. +- Trailing spaces and tabs are ignored: they don't show, and GitHub still reads + the closing keyword. +- Leading whitespace fails. Four leading spaces make the line a Markdown code + block, where GitHub doesn't read closing keywords, and any indent is a sign + the line isn't what the author meant. +- An empty or missing (``null``) description fails. +- The number is ASCII digits without a leading zero. +""" + +from __future__ import annotations + +import re + +RULE_ID = "sim.process.pr-tracking-issue" +HOW_TO_FIX = ( + "Edit the description so its first line is exactly Fixes #ISSUE_NUMBER, " + "where ISSUE_NUMBER is an open issue in this repository; editing the " + "description runs this check again. See docs/engineering/skills/github-prs.md " + f"(rule {RULE_ID})." +) +FIXES_LINE = re.compile(r"Fixes #([1-9][0-9]*)") +# Markdown line endings: CRLF, a carriage return alone, or a line feed. +LINE_ENDING = re.compile(r"\r\n|\r|\n") +# How much of a rejected first line a message quotes. +SHOWN_FIRST_LINE_LENGTH = 80 + + +class DescriptionError(Exception): + """The description, or the issue it names, does not satisfy the rule.""" + + +def _shown(line: str) -> str: + """Return ``line`` quoted for a message, shortened when it is long.""" + if len(line) > SHOWN_FIRST_LINE_LENGTH: + line = line[:SHOWN_FIRST_LINE_LENGTH].rstrip() + "..." + return f'"{line}"' + + +def issue_number_from_description(body: str | None) -> int: + """Return the issue number on the description's first line. + + Raise ``DescriptionError``, with a message that says how to fix the + description, unless the first line is ``Fixes #ISSUE_NUMBER``. + """ + if body is None or not body.strip(): + msg = f"The description is empty. {HOW_TO_FIX}" + raise DescriptionError(msg) + + lines = [line.rstrip(" \t") for line in LINE_ENDING.split(body)] + first_line = lines[0] + match = FIXES_LINE.fullmatch(first_line) + if match: + return int(match.group(1)) + + if FIXES_LINE.fullmatch(first_line.lstrip()): + msg = ( + f"The first line starts with whitespace: {_shown(first_line)}. {HOW_TO_FIX}" + ) + raise DescriptionError(msg) + for line_number, line in enumerate(lines[1:], start=2): + if FIXES_LINE.fullmatch(line.strip()): + msg = ( + f"{line.strip()} is on line {line_number}, but it must be the " + f"first line. {HOW_TO_FIX}" + ) + raise DescriptionError(msg) + msg = f"The first line is {_shown(first_line)}. {HOW_TO_FIX}" + raise DescriptionError(msg) diff --git a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py new file mode 100644 index 000000000..e6a833e57 --- /dev/null +++ b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py @@ -0,0 +1,143 @@ +"""Tests for .github/scripts/check_pr_description.py. + +Rule sim.process.pr-tracking-issue: the first line of every pull request's +description is ``Fixes #ISSUE_NUMBER``. These tests load the script in this +process to check how it reads a description. +""" + +from __future__ import annotations + +import importlib.util +from typing import TYPE_CHECKING + +import pytest +from workflow_script_support import SCRIPTS_DIR + +if TYPE_CHECKING: + from types import ModuleType + +CHECK_SCRIPT = SCRIPTS_DIR / "check_pr_description.py" + + +@pytest.fixture(scope="module") +def check_module() -> ModuleType: + """Load the check script in this process, as the workflow runs it.""" + spec = importlib.util.spec_from_file_location("check_pr_description", CHECK_SCRIPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _rejection(check_module: ModuleType, body: str | None) -> str: + """Return the message the check gives for a description it rejects.""" + with pytest.raises(check_module.DescriptionError) as rejected: + check_module.issue_number_from_description(body) + return str(rejected.value) + + +# --- The first line ------------------------------------------------------- + + +@pytest.mark.parametrize( + "body", + [ + "Fixes #861", + "Fixes #861\n\n## Summary\n\n- Adds a check.\n", + ], +) +def test_a_first_line_of_fixes_and_a_number_names_the_issue(check_module, body): + assert check_module.issue_number_from_description(body) == 861 + + +@pytest.mark.parametrize( + "body", + [ + # Descriptions edited in the GitHub web interface use CRLF. + "Fixes #861\r\n\r\n## Summary\r\n", + # A carriage return on its own also ends a line in Markdown. + "Fixes #861\r## Summary", + ], +) +def test_carriage_return_line_endings_end_the_first_line(check_module, body): + assert check_module.issue_number_from_description(body) == 861 + + +def test_trailing_whitespace_after_the_number_is_ignored(check_module): + # From PR #405, edited in the web interface: a space, then CRLF. + body = "Fixes #404 \r\n\r\n## Summary\r\n" + + assert check_module.issue_number_from_description(body) == 404 + + +def test_a_description_without_a_fixes_line_is_rejected(check_module): + message = _rejection(check_module, "## Summary\n\n- Adds a check.\n") + + assert 'The first line is "## Summary"' in message + assert "first line is exactly Fixes #ISSUE_NUMBER" in message + assert "sim.process.pr-tracking-issue" in message + + +def test_a_fixes_line_that_is_not_first_is_rejected(check_module): + message = _rejection(check_module, "## Summary\n\nFixes #861\n") + + assert "Fixes #861 is on line 3" in message + assert "must be the first line" in message + + +def test_a_blank_line_before_the_fixes_line_is_rejected(check_module): + message = _rejection(check_module, "\r\nFixes #861\r\n") + + assert "Fixes #861 is on line 2" in message + + +def test_leading_whitespace_before_fixes_is_rejected(check_module): + # Four leading spaces would make the line a Markdown code block, where + # GitHub does not read closing keywords. + message = _rejection(check_module, " Fixes #861\n") + + assert "starts with whitespace" in message + + +@pytest.mark.parametrize("body", [None, "", " \r\n\t\n"]) +def test_an_empty_or_missing_description_is_rejected(check_module, body): + message = _rejection(check_module, body) + + assert "The description is empty." in message + assert "first line is exactly Fixes #ISSUE_NUMBER" in message + + +@pytest.mark.parametrize( + "first_line", + [ + # From PR #616. + "Fixes #615.", + # From PR #543. + "Refs #434", + # From PR #750. + "Fixes # (no tracking issue exists yet.)", + "fixes #861", + "Closes #861", + "Fixes 861", + "Fixes #", + "Fixes #0", + "Fixes #0861", + "Fixes #861 and #862", + "Fixes PolicyEngine/policyengine-api#3000", + "Fixes https://github.com/PolicyEngine/policyengine-sim-api/issues/861", + # Digits from another script are not issue numbers. + "Fixes #٨٦١", + ], +) +def test_other_first_lines_are_rejected(check_module, first_line): + message = _rejection(check_module, f"{first_line}\n\n## Summary\n") + + assert f'The first line is "{first_line}"' in message + + +def test_a_long_first_line_is_shortened_in_the_message(check_module): + first_line = "Draft, stacked on #743. " + "Gate releases on a sentinel. " * 10 + + message = _rejection(check_module, first_line) + + assert 'The first line is "Draft, stacked on #743.' in message + assert first_line not in message From e1588304b4fc74c8deba6d5a06a80660ad43efb1 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:09:22 +0400 Subject: [PATCH 2/9] Confirm the issue a PR description names is open and in this repository Give check_pr_description.py an entry point. It reads the pull request description from the event payload at GITHUB_EVENT_PATH, reads the issue number from the first line, and asks GitHub for that issue with "gh api repos//issues/". It fails, printing one error annotation, when: - GitHub has no issue or pull request with that number (HTTP 404 or 410); - the number is a pull request (the answer has a pull_request field); - the answer belongs to another repository or has another number, which is what gh prints for an issue transferred out of this repository, because it follows GitHub's redirect; - the issue is closed; - gh fails for any other reason, or prints something other than an issue. The repository comparison ignores case, as GitHub does. Messages are escaped so a multi-line gh error stays one annotation. The tests run the script with a fake gh first on PATH, so they need no network access. The moved-issue case uses #154, which GitHub moved to PolicyEngine/policyengine.py#115. Part of #861. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_pr_description.py | 138 ++++++++- .../tests/test_pr_description_check.py | 277 +++++++++++++++++- 2 files changed, 411 insertions(+), 4 deletions(-) diff --git a/.github/scripts/check_pr_description.py b/.github/scripts/check_pr_description.py index b36478cd6..7d16f9682 100644 --- a/.github/scripts/check_pr_description.py +++ b/.github/scripts/check_pr_description.py @@ -1,7 +1,16 @@ """Check that a pull request's description names its tracking issue. Rule sim.process.pr-tracking-issue: the first line of every pull request's -description is ``Fixes #ISSUE_NUMBER``. +description is ``Fixes #ISSUE_NUMBER``, and ISSUE_NUMBER is an open issue, not a +pull request, in this repository. The "PR description" workflow job runs this +script; it prints one error annotation and exits 1 when the rule is broken. + +Environment: + GITHUB_EVENT_PATH the pull_request event payload, which holds the + description (set by the GitHub Actions runner) + GITHUB_REPOSITORY owner/name of this repository (set by the runner) + GH_TOKEN read by gh, not by this script; the workflow sets it to + the job's github.token, which needs only issues: read How the first line is read: @@ -18,7 +27,12 @@ from __future__ import annotations +import json +import os import re +import subprocess +from pathlib import Path +from urllib.parse import urlsplit RULE_ID = "sim.process.pr-tracking-issue" HOW_TO_FIX = ( @@ -32,6 +46,12 @@ LINE_ENDING = re.compile(r"\r\n|\r|\n") # How much of a rejected first line a message quotes. SHOWN_FIRST_LINE_LENGTH = 80 +# gh api's error for a number GitHub has no issue or pull request for (404), +# or one whose issue was deleted (410). +MISSING_ISSUE = re.compile(r"\(HTTP (?:404|410)\)") +# The path of an issue's repository_url: [api/v3/]repos//. +REPOSITORY_PATH = re.compile(r"(?:.+/)?repos/([^/]+/[^/]+)") +ANNOTATION_TITLE = "PR description" class DescriptionError(Exception): @@ -75,3 +95,119 @@ def issue_number_from_description(body: str | None) -> int: raise DescriptionError(msg) msg = f"The first line is {_shown(first_line)}. {HOW_TO_FIX}" raise DescriptionError(msg) + + +def fetch_issue(repository: str, number: int) -> dict[str, object]: + """Return GitHub's answer for issue ``number`` in ``repository``, from gh api. + + The issues API also answers for pull requests, and gh follows the redirect + GitHub sends for an issue transferred to another repository, so the answer + still has to be checked with ``check_issue``. + """ + completed = subprocess.run( + ["gh", "api", f"repos/{repository}/issues/{number}"], + capture_output=True, + text=True, + check=False, + ) + if completed.returncode != 0: + if MISSING_ISSUE.search(completed.stderr): + msg = f"There is no issue #{number} in {repository}. {HOW_TO_FIX}" + raise DescriptionError(msg) + reported = completed.stderr.strip() or f"exit status {completed.returncode}" + msg = ( + f"Could not look up #{number} in {repository}: {reported}. Re-run this " + "job; if it fails again, check the GH_TOKEN the workflow gives it." + ) + raise DescriptionError(msg) + try: + issue = json.loads(completed.stdout) + except json.JSONDecodeError: + issue = None + if not isinstance(issue, dict): + msg = f"gh api did not print an issue for #{number}. Re-run this job." + raise DescriptionError(msg) + return issue + + +def _repository_of(repository_url: object) -> str | None: + """Return ``owner/name`` from an issue's ``repository_url``, if it has one.""" + if not isinstance(repository_url, str): + return None + match = REPOSITORY_PATH.fullmatch(urlsplit(repository_url).path.strip("/")) + return match.group(1) if match else None + + +def check_issue(repository: str, number: int, issue: dict[str, object]) -> None: + """Raise ``DescriptionError`` unless ``issue`` is open issue ``number`` here.""" + issue_repository = _repository_of(issue.get("repository_url")) + if issue_repository is None: + msg = ( + f"GitHub's answer for #{number} names no repository, so it can't be " + f"confirmed as an issue in {repository}. Re-run this job." + ) + raise DescriptionError(msg) + if issue_repository.lower() != repository.lower() or issue.get("number") != number: + msg = ( + f"#{number} is not an issue in {repository}: it moved to " + f"{issue_repository}#{issue.get('number')}. {HOW_TO_FIX}" + ) + raise DescriptionError(msg) + if issue.get("pull_request") is not None: + msg = f"#{number} is a pull request, not an issue. {HOW_TO_FIX}" + raise DescriptionError(msg) + if issue.get("state") != "open": + msg = ( + f"Issue #{number} is {issue.get('state')}. If this pull request still " + "fixes it, reopen it and re-run this job; otherwise open an issue for " + f"this work and edit the first line to name it (rule {RULE_ID})." + ) + raise DescriptionError(msg) + + +def _required_environment(name: str) -> str: + value = os.environ.get(name) + if not value: + msg = f"{name} is required" + raise SystemExit(msg) + return value + + +def _description_from_event(event_path: str) -> str | None: + """Return the pull request description in the event payload at ``event_path``.""" + event = json.loads(Path(event_path).read_text(encoding="utf-8")) + pull_request = event.get("pull_request") if isinstance(event, dict) else None + if not isinstance(pull_request, dict): + msg = ( + f"The event payload at {event_path} has no pull_request; run this " + "check on pull_request events." + ) + raise SystemExit(msg) + body = pull_request.get("body") + return body if isinstance(body, str) else None + + +def _annotation_text(message: str) -> str: + """Escape ``message`` so the runner reads it as one annotation message.""" + return message.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + +def main() -> int: + """Check the pull request in this run's event; return the exit status.""" + repository = _required_environment("GITHUB_REPOSITORY") + description = _description_from_event(_required_environment("GITHUB_EVENT_PATH")) + try: + number = issue_number_from_description(description) + check_issue(repository, number, fetch_issue(repository, number)) + except DescriptionError as error: + print(f"::error title={ANNOTATION_TITLE}::{_annotation_text(str(error))}") + return 1 + print( + f"The first line is Fixes #{number}, and #{number} is an open issue in " + f"{repository}." + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py index e6a833e57..de8881e54 100644 --- a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py +++ b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py @@ -1,22 +1,46 @@ """Tests for .github/scripts/check_pr_description.py. Rule sim.process.pr-tracking-issue: the first line of every pull request's -description is ``Fixes #ISSUE_NUMBER``. These tests load the script in this -process to check how it reads a description. +description is ``Fixes #ISSUE_NUMBER``, naming an open issue in this +repository. Tests of how the script reads a description load it in this +process; tests of the issue lookup run it as the workflow does, with a fake +``gh`` first on PATH, so no test reaches GitHub. """ from __future__ import annotations import importlib.util +import json +import subprocess +import sys from typing import TYPE_CHECKING import pytest -from workflow_script_support import SCRIPTS_DIR +from workflow_script_support import SCRIPTS_DIR, FakeCommands if TYPE_CHECKING: + from pathlib import Path from types import ModuleType CHECK_SCRIPT = SCRIPTS_DIR / "check_pr_description.py" +REPOSITORY = "PolicyEngine/policyengine-sim-api" +VALID_BODY = "Fixes #861\n\n## Summary\n\n- Adds a check.\n" + +# Prints FAKE_GH_STDOUT, and FAKE_GH_STDERR on stderr, then exits with +# FAKE_GH_EXIT, as gh api does with GitHub's answer. +FAKE_GH = """\ +printf '%s' "${FAKE_GH_STDOUT:-}" +if [[ -n "${FAKE_GH_STDERR:-}" ]]; then + printf '%s\\n' "$FAKE_GH_STDERR" >&2 +fi +exit "${FAKE_GH_EXIT:-0}" +""" +# What gh api prints for a number GitHub has no issue or pull request for. +NOT_FOUND_STDOUT = ( + '{"message":"Not Found","documentation_url":' + '"https://docs.github.com/rest/issues/issues#get-an-issue","status":"404"}' +) +NOT_FOUND_STDERR = "gh: Not Found (HTTP 404)" @pytest.fixture(scope="module") @@ -141,3 +165,250 @@ def test_a_long_first_line_is_shortened_in_the_message(check_module): assert 'The first line is "Draft, stacked on #743.' in message assert first_line not in message + + +# --- The issue lookup ----------------------------------------------------- + + +@pytest.fixture +def fakes(tmp_path: Path) -> FakeCommands: + commands = FakeCommands.create(tmp_path) + commands.add("gh", FAKE_GH) + return commands + + +def _issue( + number: int = 861, *, state: str = "open", repository: str = REPOSITORY, **fields +) -> dict: + """Return the fields gh api prints for an issue, with ``fields`` added.""" + return { + "number": number, + "state": state, + "repository_url": f"https://api.github.com/repos/{repository}", + "html_url": f"https://github.com/{repository}/issues/{number}", + **fields, + } + + +def _write_event(tmp_path: Path, event: dict) -> Path: + event_path = tmp_path / "event.json" + event_path.write_text(json.dumps(event), encoding="utf-8") + return event_path + + +def _pull_request_event(body: str | None) -> dict: + return {"action": "edited", "number": 862, "pull_request": {"body": body}} + + +def _run_check( + fakes: FakeCommands, tmp_path: Path, body: str | None = VALID_BODY, **env: str +) -> subprocess.CompletedProcess[str]: + """Run the check on a pull request event whose description is ``body``.""" + event_path = _write_event(tmp_path, _pull_request_event(body)) + return _run_script( + fakes.env( + GITHUB_EVENT_PATH=str(event_path), GITHUB_REPOSITORY=REPOSITORY, **env + ) + ) + + +def _run_script(env: dict[str, str]) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, str(CHECK_SCRIPT)], + capture_output=True, + text=True, + check=False, + env=env, + ) + + +def _gh_answers(issue: dict) -> dict[str, str]: + """Environment for the fake gh to print ``issue`` and succeed.""" + return {"FAKE_GH_STDOUT": json.dumps(issue)} + + +def _error_annotation(result: subprocess.CompletedProcess[str]) -> str: + """Return the one workflow error annotation the check printed.""" + annotations = [ + output_line + for output_line in result.stdout.splitlines() + if output_line.startswith("::error") + ] + assert len(annotations) == 1, result.stdout + return annotations[0] + + +def test_a_description_naming_an_open_issue_in_this_repository_passes(fakes, tmp_path): + result = _run_check(fakes, tmp_path, **_gh_answers(_issue())) + + assert result.returncode == 0, result.stdout + result.stderr + assert fakes.calls("gh") == [ + ["gh", "api", "repos/PolicyEngine/policyengine-sim-api/issues/861"] + ] + assert result.stdout == ( + "The first line is Fixes #861, and #861 is an open issue in " + "PolicyEngine/policyengine-sim-api.\n" + ) + + +def test_a_rejected_description_is_reported_without_asking_github(fakes, tmp_path): + result = _run_check(fakes, tmp_path, "## Summary\n\nFixes #861\n") + + assert result.returncode == 1 + assert "Fixes #861 is on line 3" in _error_annotation(result) + assert fakes.calls("gh") == [] + + +def test_a_null_description_in_the_event_fails(fakes, tmp_path): + result = _run_check(fakes, tmp_path, None) + + assert result.returncode == 1 + assert "The description is empty." in _error_annotation(result) + assert fakes.calls("gh") == [] + + +def test_a_crlf_description_from_the_event_passes(fakes, tmp_path): + result = _run_check( + fakes, tmp_path, "Fixes #861\r\n\r\n## Summary\r\n", **_gh_answers(_issue()) + ) + + assert result.returncode == 0, result.stdout + result.stderr + + +def test_a_closed_issue_fails(fakes, tmp_path): + result = _run_check(fakes, tmp_path, **_gh_answers(_issue(state="closed"))) + + assert result.returncode == 1 + annotation = _error_annotation(result) + assert "Issue #861 is closed." in annotation + assert "reopen it and re-run this job" in annotation + + +def test_a_pull_request_number_fails(fakes, tmp_path): + pull_request = _issue( + 857, + pull_request={ + "url": f"https://api.github.com/repos/{REPOSITORY}/pulls/857", + }, + ) + + result = _run_check(fakes, tmp_path, "Fixes #857\n", **_gh_answers(pull_request)) + + assert result.returncode == 1 + assert "#857 is a pull request, not an issue." in _error_annotation(result) + + +def test_a_missing_issue_fails(fakes, tmp_path): + result = _run_check( + fakes, + tmp_path, + "Fixes #999999\n", + FAKE_GH_STDOUT=NOT_FOUND_STDOUT, + FAKE_GH_STDERR=NOT_FOUND_STDERR, + FAKE_GH_EXIT="1", + ) + + assert result.returncode == 1 + assert ( + "There is no issue #999999 in PolicyEngine/policyengine-sim-api." + in _error_annotation(result) + ) + + +def test_an_issue_moved_to_another_repository_fails(fakes, tmp_path): + # Issue #154 was transferred to policyengine.py, where it is #115; gh api + # follows GitHub's redirect and prints the moved issue. + moved_issue = _issue(115, state="closed", repository="PolicyEngine/policyengine.py") + + result = _run_check(fakes, tmp_path, "Fixes #154\n", **_gh_answers(moved_issue)) + + assert result.returncode == 1 + annotation = _error_annotation(result) + assert ( + "#154 is not an issue in PolicyEngine/policyengine-sim-api: it moved to " + "PolicyEngine/policyengine.py#115." + ) in annotation + + +def test_an_issue_in_a_repository_whose_name_differs_only_in_case_passes( + fakes, tmp_path +): + issue = _issue(repository="policyengine/PolicyEngine-Sim-API") + + result = _run_check(fakes, tmp_path, **_gh_answers(issue)) + + assert result.returncode == 0, result.stdout + result.stderr + + +@pytest.mark.parametrize( + "repository_url", + [None, "", "https://api.github.com/orgs/PolicyEngine", "not a URL"], +) +def test_an_answer_that_names_no_repository_fails(fakes, tmp_path, repository_url): + issue = _issue() + issue["repository_url"] = repository_url + + result = _run_check(fakes, tmp_path, **_gh_answers(issue)) + + assert result.returncode == 1 + assert "GitHub's answer for #861 names no repository" in _error_annotation(result) + + +def test_a_failed_lookup_fails_with_what_gh_reported(fakes, tmp_path): + result = _run_check( + fakes, + tmp_path, + FAKE_GH_STDERR="gh: Bad credentials (HTTP 401)", + FAKE_GH_EXIT="1", + ) + + assert result.returncode == 1 + annotation = _error_annotation(result) + assert "Could not look up #861 in PolicyEngine/policyengine-sim-api" in annotation + assert "gh: Bad credentials (HTTP 401)" in annotation + + +@pytest.mark.parametrize("gh_stdout", ["not JSON", "[]"]) +def test_an_answer_that_is_not_an_issue_object_fails(fakes, tmp_path, gh_stdout): + result = _run_check(fakes, tmp_path, FAKE_GH_STDOUT=gh_stdout) + + assert result.returncode == 1 + assert "gh api did not print an issue for #861" in _error_annotation(result) + + +def test_messages_stay_on_one_annotation_line(fakes, tmp_path): + result = _run_check( + fakes, + tmp_path, + FAKE_GH_STDERR="first problem\nsecond problem 100%", + FAKE_GH_EXIT="1", + ) + + assert result.returncode == 1 + assert result.stdout.count("\n") == 1 + assert "first problem%0Asecond problem 100%25" in _error_annotation(result) + + +@pytest.mark.parametrize("variable", ["GITHUB_EVENT_PATH", "GITHUB_REPOSITORY"]) +def test_the_check_requires_its_environment(fakes, tmp_path, variable): + event_path = _write_event(tmp_path, _pull_request_event(VALID_BODY)) + env = fakes.env(GITHUB_EVENT_PATH=str(event_path), GITHUB_REPOSITORY=REPOSITORY) + del env[variable] + + result = _run_script(env) + + assert result.returncode == 1 + assert f"{variable} is required" in result.stderr + assert fakes.calls("gh") == [] + + +def test_the_check_requires_a_pull_request_event(fakes, tmp_path): + event_path = _write_event(tmp_path, {"ref": "refs/heads/main"}) + + result = _run_script( + fakes.env(GITHUB_EVENT_PATH=str(event_path), GITHUB_REPOSITORY=REPOSITORY) + ) + + assert result.returncode == 1 + assert "has no pull_request" in result.stderr + assert fakes.calls("gh") == [] From 5f50447b0dda8d4e33fc4061fdbd635c355a3237 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:10:54 +0400 Subject: [PATCH 3/9] Run the PR description check on every pull request event that can change it Add .github/workflows/pr-description.yml with one job, "PR description", which runs check_pr_description.py on pull_request events of type opened, edited, reopened and synchronize. - It is a separate workflow from pr.yml, so editing a description reruns only this job rather than the whole of CI, and pr.yml's rule that every job is needed by "CI complete" and carries the stack condition does not apply to it. - It has no branches or paths filter and no stack condition, so it runs on every pull request, including each one in a stack. - The workflow token can only read contents, issues and pull requests; the check step gets it as GH_TOKEN for gh api. - A newer event for the same pull request cancels a run still in progress. Tests pin the triggers, the absence of a condition, the permissions, and the step that runs the script with the job token. Part of #861. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pr-description.yml | 49 +++++++++++++++ .../tests/test_pr_description_check.py | 60 ++++++++++++++++++- 2 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/pr-description.yml diff --git a/.github/workflows/pr-description.yml b/.github/workflows/pr-description.yml new file mode 100644 index 000000000..1402247cd --- /dev/null +++ b/.github/workflows/pr-description.yml @@ -0,0 +1,49 @@ +name: PR description + +# Rule sim.process.pr-tracking-issue: the first line of every pull request's +# description is "Fixes #ISSUE_NUMBER", naming an open issue in this +# repository. See docs/engineering/skills/github-prs.md. +# +# A separate workflow from pr.yml, so that editing a description reruns only +# this check, not the whole of CI. It has no branches or paths filter and no +# stack condition: it is cheap, so every pull request runs it, including each +# one in a stack. + +on: + pull_request: + types: [opened, edited, reopened, synchronize] + +# The check reads the description from the event payload and looks the issue +# up with the job's token, so it needs only read access. Pull requests from +# forks get a read-only token, which is enough. +permissions: + contents: read + issues: read + pull-requests: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + pr-description: + name: PR description + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - name: Checkout the check script + uses: actions/checkout@v6 + with: + sparse-checkout: .github/scripts/check_pr_description.py + sparse-checkout-cone-mode: false + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: '3.13' + + - name: Check the first line and the tracking issue + env: + GH_TOKEN: ${{ github.token }} + run: python .github/scripts/check_pr_description.py diff --git a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py index de8881e54..080a942e4 100644 --- a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py +++ b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py @@ -16,13 +16,16 @@ from typing import TYPE_CHECKING import pytest -from workflow_script_support import SCRIPTS_DIR, FakeCommands +import yaml +from workflow_script_support import SCRIPTS_DIR, WORKFLOWS_DIR, FakeCommands if TYPE_CHECKING: from pathlib import Path from types import ModuleType CHECK_SCRIPT = SCRIPTS_DIR / "check_pr_description.py" +CHECK_WORKFLOW = WORKFLOWS_DIR / "pr-description.yml" +CHECK_JOB = "pr-description" REPOSITORY = "PolicyEngine/policyengine-sim-api" VALID_BODY = "Fixes #861\n\n## Summary\n\n- Adds a check.\n" @@ -412,3 +415,58 @@ def test_the_check_requires_a_pull_request_event(fakes, tmp_path): assert result.returncode == 1 assert "has no pull_request" in result.stderr assert fakes.calls("gh") == [] + + +# --- The workflow job ----------------------------------------------------- + + +@pytest.fixture(scope="module") +def check_workflow() -> dict: + return yaml.safe_load(CHECK_WORKFLOW.read_text(encoding="utf-8")) + + +def test_the_check_runs_on_every_event_that_can_change_the_description( + check_workflow, +): + # PyYAML reads the key "on" as True. + triggers = check_workflow.get("on", check_workflow.get(True)) + + # No branches or paths filter: every pull request is checked, whatever it + # targets and whatever it changes. + assert triggers == { + "pull_request": {"types": ["opened", "edited", "reopened", "synchronize"]} + } + + +def test_the_check_runs_on_every_pull_request_in_a_stack(check_workflow): + """Unlike pr.yml's jobs, this one has no condition and needs no other job.""" + assert list(check_workflow["jobs"]) == [CHECK_JOB] + job = check_workflow["jobs"][CHECK_JOB] + + assert job["name"] == "PR description" + assert "if" not in job + assert "needs" not in job + + +def test_the_check_can_only_read(check_workflow): + assert check_workflow["permissions"] == { + "contents": "read", + "issues": "read", + "pull-requests": "read", + } + assert "permissions" not in check_workflow["jobs"][CHECK_JOB] + + +def test_the_check_runs_the_tested_script_with_the_job_token(check_workflow): + steps = check_workflow["jobs"][CHECK_JOB]["steps"] + checkout = next( + step for step in steps if step.get("uses", "").startswith("actions/checkout@") + ) + check_steps = [step for step in steps if CHECK_SCRIPT.name in step.get("run", "")] + + assert checkout["with"]["sparse-checkout"] == ( + f".github/scripts/{CHECK_SCRIPT.name}" + ) + assert len(check_steps) == 1 + assert check_steps[0]["run"] == f"python .github/scripts/{CHECK_SCRIPT.name}" + assert check_steps[0]["env"] == {"GH_TOKEN": "${{ github.token }}"} From 0b87d36197c4b7ebc3bd7ea7e914733ce259a06b Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:16:06 +0400 Subject: [PATCH 4/9] Name a tracking issue on the first line of dependency-update PRs The daily dependency-update PR's description started with "Automated daily dependency updates", so the PR description check would fail on every one. dependency-update-create-pr.sh now runs whenever the update branch was pushed, with or without an open update PR: - It uses the oldest open issue titled "Update dependencies", or opens one. "Fixes #N" closes it when the PR merges, so the next PR gets a new issue. The issues API lists pull requests too, so it skips entries that have a pull_request field. - Without an open update PR, it opens one whose description starts with "Fixes #N". - With one, it makes "Fixes #N" the first line: it replaces an earlier "Fixes #" line, as when that issue was closed, or adds the line above a description that has none, as on #583, opened before the rule. - A failed issue lookup stops the script before it opens an issue. The GitHub App token in update-dependencies.yml requests no specific permissions, so it has every permission of the App's installation. That installation can write issues: check-policyengine-updates.yml requests issues: write from the same organization secrets (APP_ID, APP_PRIVATE_KEY), and policyengine[bot] opened #831 and #856 with it. Part of #861. Co-Authored-By: Claude Opus 5.5 --- .../scripts/dependency-update-create-pr.sh | 119 ++++++++- .github/workflows/update-dependencies.yml | 7 +- .../tests/test_workflow_scripts.py | 229 +++++++++++++++++- 3 files changed, 339 insertions(+), 16 deletions(-) diff --git a/.github/scripts/dependency-update-create-pr.sh b/.github/scripts/dependency-update-create-pr.sh index 1084c03e8..b35d4d1df 100755 --- a/.github/scripts/dependency-update-create-pr.sh +++ b/.github/scripts/dependency-update-create-pr.sh @@ -1,14 +1,121 @@ #!/usr/bin/env bash -# Open a dependency-update PR after a new update branch has been pushed. +# After an update branch has been pushed, make sure an open dependency-update +# PR exists and that the first line of its description names its tracking +# issue, as rule sim.process.pr-tracking-issue requires of every PR. +# +# The tracking issue is the oldest open issue titled "Update dependencies", or +# a new one when none is open. "Fixes #N" closes it when the PR merges, so the +# next update PR gets a new issue. +# +# - Without an open update PR, open one whose description starts with +# "Fixes #N". +# - With one, make "Fixes #N" its first line: replace an earlier "Fixes #" +# line, or add the line above a description that has none (such as a PR +# opened before the rule). +# +# Environment: +# GITHUB_REPOSITORY owner/name of this repository (set by the runner) +# GH_TOKEN read by gh: a token that can read and write issues and +# pull requests set -euo pipefail : "${GITHUB_REPOSITORY:?GITHUB_REPOSITORY is required}" -gh pr create \ +ISSUE_TITLE="Update dependencies" +ISSUE_BODY="Tracks the automated daily dependency updates from update-dependencies.yml. The update pull request names this issue on the first line of its description and closes it when it merges." +PR_SUMMARY="Automated daily dependency updates. Subsequent runs append commits to this branch while the pull request remains open." + +# Set ISSUE_NUMBER to the oldest open "Update dependencies" issue, opening one +# when none is open. The issues API lists pull requests too; they have a +# pull_request field. This sets a variable rather than printing, because bash +# turns off set -e inside $(...), and a failed lookup must not open an issue. +find_or_open_tracking_issue() { + local open_issues + local issue_number + local issue_url + + open_issues="$( + gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/issues?state=open&per_page=100" \ + --jq ".[] | select(.pull_request == null and .title == \"${ISSUE_TITLE}\") | .number" + )" + if [[ -n "${open_issues}" ]]; then + issue_number="$(sort -n <<<"${open_issues}")" + issue_number="${issue_number%%$'\n'*}" + else + issue_url="$( + gh issue create \ + --repo "${GITHUB_REPOSITORY}" \ + --title "${ISSUE_TITLE}" \ + --body "${ISSUE_BODY}" + )" + issue_number="${issue_url##*/}" + fi + + if [[ ! "${issue_number}" =~ ^[0-9]+$ ]]; then + echo "Could not resolve the dependency-update tracking issue: ${issue_number}" >&2 + exit 1 + fi + ISSUE_NUMBER="${issue_number}" +} + +# Print the description with "Fixes #" as its first line. +description_naming_issue() { + local description="$1" + local issue_number="$2" + local first_line="${description%%$'\n'*}" + local fixes_line="Fixes #${issue_number}" + + if [[ "${first_line}" =~ ^Fixes\ \#[0-9]+[[:space:]]*$ ]]; then + if [[ "${description}" == *$'\n'* ]]; then + printf '%s\n%s' "${fixes_line}" "${description#*$'\n'}" + else + printf '%s' "${fixes_line}" + fi + else + printf '%s\n\n%s' "${fixes_line}" "${description}" + fi +} + +open_pr="$( + gh pr list \ + --repo "${GITHUB_REPOSITORY}" \ + --base main \ + --head update-dependencies \ + --state open \ + --json number \ + --jq '.[0].number // empty' +)" +ISSUE_NUMBER="" +find_or_open_tracking_issue + +if [[ -z "${open_pr}" ]]; then + gh pr create \ + --repo "${GITHUB_REPOSITORY}" \ + --title "Update dependencies" \ + --body-file - \ + --base main \ + --head update-dependencies \ + <<<"Fixes #${ISSUE_NUMBER}"$'\n\n'"${PR_SUMMARY}" + exit 0 +fi + +description="$( + gh pr view "${open_pr}" \ + --repo "${GITHUB_REPOSITORY}" \ + --json body \ + --jq '.body' +)" +linked_description="$(description_naming_issue "${description}" "${ISSUE_NUMBER}")" +if [[ "${linked_description}" == "${description}" ]]; then + echo "Pull request #${open_pr} already names tracking issue #${ISSUE_NUMBER}." + exit 0 +fi + +gh pr edit "${open_pr}" \ --repo "${GITHUB_REPOSITORY}" \ - --title "Update dependencies" \ - --body "Automated daily dependency updates. Subsequent runs append commits to this branch while the pull request remains open." \ - --base main \ - --head update-dependencies + --body-file - \ + <<<"${linked_description}" +echo "Pull request #${open_pr} now names tracking issue #${ISSUE_NUMBER}." diff --git a/.github/workflows/update-dependencies.yml b/.github/workflows/update-dependencies.yml index caf7b0142..3c4d1d265 100644 --- a/.github/workflows/update-dependencies.yml +++ b/.github/workflows/update-dependencies.yml @@ -61,8 +61,11 @@ jobs: HAS_OPEN_PR: ${{ steps.branch.outputs.has_open_pr }} run: .github/scripts/dependency-update-commit-and-push.sh - - name: Create pull request - if: steps.branch.outputs.has_open_pr == 'false' && steps.push.outputs.branch_pushed == 'true' + # Opens the update PR, or, when one is open, makes sure the first line + # of its description names the tracking issue (rule + # sim.process.pr-tracking-issue). + - name: Open or update the pull request + if: steps.push.outputs.branch_pushed == 'true' env: GH_TOKEN: ${{ steps.app-token.outputs.token }} run: .github/scripts/dependency-update-create-pr.sh diff --git a/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py b/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py index aaa0a816b..a7b4c66f8 100644 --- a/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py +++ b/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py @@ -10,6 +10,7 @@ from pathlib import Path import pytest +import yaml from workflow_script_support import ( REPOSITORY_ROOT, WORKFLOWS_DIR, @@ -253,15 +254,81 @@ def test_commit_and_push_fails_when_the_push_fails(fakes, tmp_path): assert github_output(output) == {} -def test_create_pr_opens_the_update_pr(fakes): - fakes.add("gh") - +# The create-PR script finds the open "Update dependencies" issue, or opens +# one, and names it on the first line of the update PR's description (rule +# sim.process.pr-tracking-issue). The fake gh answers each call from the +# environment and writes each description it is given to FAKE_PR_BODY_LOG. +FAKE_GH_FOR_CREATE_PR = """\ +case "$1 $2" in + "pr list") printf '%s' "${FAKE_OPEN_PR:-}" ;; + "pr view") printf '%s' "${FAKE_OPEN_PR_BODY:-}" ;; + "pr create" | "pr edit") cat >> "$FAKE_PR_BODY_LOG" ;; + "issue create") printf '%s\\n' "${FAKE_CREATED_ISSUE_URL:-}" ;; + "api --paginate") + printf '%s' "${FAKE_OPEN_ISSUES:-}" + exit "${FAKE_GH_API_EXIT:-0}" + ;; +esac +""" +UPDATE_PR_SUMMARY = ( + "Automated daily dependency updates. Subsequent runs append commits to " + "this branch while the pull request remains open." +) +LEGACY_UPDATE_PR_BODY = ( + "Automated dependency updates\n\n" + "This PR was automatically created by the dependency update workflow." +) +OPEN_UPDATE_PR_LOOKUP = [ + "gh", + "pr", + "list", + "--repo", + "PolicyEngine/policyengine-sim-api", + "--base", + "main", + "--head", + "update-dependencies", + "--state", + "open", + "--json", + "number", + "--jq", + ".[0].number // empty", +] +OPEN_TRACKING_ISSUE_LOOKUP = [ + "gh", + "api", + "--paginate", + "repos/PolicyEngine/policyengine-sim-api/issues?state=open&per_page=100", + "--jq", + '.[] | select(.pull_request == null and .title == "Update dependencies") | .number', +] + + +def _create_pr(fakes: FakeCommands, tmp_path: Path, **env: str): + """Run the create-PR script; return its result and the descriptions it set.""" + body_log = tmp_path / "pr-bodies" + fakes.add("gh", FAKE_GH_FOR_CREATE_PR) result = run_script( - CREATE_PR, env=fakes.env(GITHUB_REPOSITORY="PolicyEngine/policyengine-sim-api") + CREATE_PR, + env=fakes.env( + GITHUB_REPOSITORY="PolicyEngine/policyengine-sim-api", + FAKE_PR_BODY_LOG=str(body_log), + **env, + ), ) + # Read bytes: read_text would turn the CRLF line endings under test into LF. + descriptions = body_log.read_bytes().decode("utf-8") if body_log.exists() else "" + return result, descriptions + + +def test_create_pr_opens_the_update_pr_naming_the_open_tracking_issue(fakes, tmp_path): + result, descriptions = _create_pr(fakes, tmp_path, FAKE_OPEN_ISSUES="870\n") assert result.returncode == 0, result.stderr assert fakes.calls() == [ + OPEN_UPDATE_PR_LOOKUP, + OPEN_TRACKING_ISSUE_LOOKUP, [ "gh", "pr", @@ -270,15 +337,146 @@ def test_create_pr_opens_the_update_pr(fakes): "PolicyEngine/policyengine-sim-api", "--title", "Update dependencies", - "--body", - "Automated daily dependency updates. Subsequent runs append commits " - "to this branch while the pull request remains open.", + "--body-file", + "-", "--base", "main", "--head", "update-dependencies", - ] + ], + ] + assert descriptions == f"Fixes #870\n\n{UPDATE_PR_SUMMARY}\n" + + +def test_create_pr_names_the_oldest_open_tracking_issue(fakes, tmp_path): + result, descriptions = _create_pr( + fakes, tmp_path, FAKE_OPEN_ISSUES="875\n870\n1001\n" + ) + + assert result.returncode == 0, result.stderr + assert descriptions.startswith("Fixes #870\n\n") + + +def test_create_pr_opens_a_tracking_issue_when_none_is_open(fakes, tmp_path): + result, descriptions = _create_pr( + fakes, + tmp_path, + FAKE_CREATED_ISSUE_URL=( + "https://github.com/PolicyEngine/policyengine-sim-api/issues/871" + ), + ) + + assert result.returncode == 0, result.stderr + assert fakes.calls()[2] == [ + "gh", + "issue", + "create", + "--repo", + "PolicyEngine/policyengine-sim-api", + "--title", + "Update dependencies", + "--body", + "Tracks the automated daily dependency updates from " + "update-dependencies.yml. The update pull request names this issue on " + "the first line of its description and closes it when it merges.", ] + assert fakes.calls()[3][:3] == ["gh", "pr", "create"] + assert descriptions == f"Fixes #871\n\n{UPDATE_PR_SUMMARY}\n" + + +def test_create_pr_fails_when_the_new_issue_has_no_number(fakes, tmp_path): + result, descriptions = _create_pr( + fakes, tmp_path, FAKE_CREATED_ISSUE_URL="could not create the issue" + ) + + assert result.returncode == 1 + assert "Could not resolve the dependency-update tracking issue" in result.stderr + assert not any(call[1:3] == ["pr", "create"] for call in fakes.calls()) + assert descriptions == "" + + +def test_create_pr_fails_when_the_issue_lookup_fails(fakes, tmp_path): + result, descriptions = _create_pr(fakes, tmp_path, FAKE_GH_API_EXIT="1") + + assert result.returncode == 1 + assert fakes.calls() == [OPEN_UPDATE_PR_LOOKUP, OPEN_TRACKING_ISSUE_LOOKUP] + assert descriptions == "" + + +def test_create_pr_puts_the_tracking_issue_first_on_an_open_update_pr(fakes, tmp_path): + """An update PR opened before the rule, such as #583, gets a Fixes line.""" + result, descriptions = _create_pr( + fakes, + tmp_path, + FAKE_OPEN_PR="583", + FAKE_OPEN_PR_BODY=LEGACY_UPDATE_PR_BODY, + FAKE_OPEN_ISSUES="870\n", + ) + + assert result.returncode == 0, result.stderr + assert fakes.calls() == [ + OPEN_UPDATE_PR_LOOKUP, + OPEN_TRACKING_ISSUE_LOOKUP, + [ + "gh", + "pr", + "view", + "583", + "--repo", + "PolicyEngine/policyengine-sim-api", + "--json", + "body", + "--jq", + ".body", + ], + [ + "gh", + "pr", + "edit", + "583", + "--repo", + "PolicyEngine/policyengine-sim-api", + "--body-file", + "-", + ], + ] + assert descriptions == f"Fixes #870\n\n{LEGACY_UPDATE_PR_BODY}\n" + + +@pytest.mark.parametrize("line_ending", ["\n", "\r\n"]) +def test_create_pr_replaces_an_earlier_fixes_line_on_the_open_update_pr( + fakes, tmp_path, line_ending +): + """An open update PR whose issue was closed names the open one instead.""" + rest = f"{line_ending}{UPDATE_PR_SUMMARY}" + result, descriptions = _create_pr( + fakes, + tmp_path, + FAKE_OPEN_PR="900", + FAKE_OPEN_PR_BODY=f"Fixes #860 {line_ending}{rest}", + FAKE_OPEN_ISSUES="870\n", + ) + + assert result.returncode == 0, result.stderr + assert descriptions == f"Fixes #870\n{rest}\n" + + +def test_create_pr_leaves_an_open_update_pr_that_names_the_tracking_issue( + fakes, tmp_path +): + result, descriptions = _create_pr( + fakes, + tmp_path, + FAKE_OPEN_PR="900", + FAKE_OPEN_PR_BODY=f"Fixes #870\n\n{UPDATE_PR_SUMMARY}", + FAKE_OPEN_ISSUES="870\n", + ) + + assert result.returncode == 0, result.stderr + assert not any( + call[1:3] in (["pr", "edit"], ["pr", "create"]) for call in fakes.calls() + ) + assert descriptions == "" def test_create_pr_fails_when_gh_fails(fakes): @@ -289,6 +487,7 @@ def test_create_pr_fails_when_gh_fails(fakes): ) assert result.returncode == 1 + assert fakes.calls() == [OPEN_UPDATE_PR_LOOKUP] def test_create_pr_requires_the_repository(fakes): @@ -303,6 +502,20 @@ def test_create_pr_requires_the_repository(fakes): assert fakes.calls() == [] +def test_update_dependencies_workflow_updates_the_pr_whenever_it_pushes(): + """With or without an open update PR, a push runs the create-PR script.""" + workflow = yaml.safe_load( + (WORKFLOWS_DIR / "update-dependencies.yml").read_text(encoding="utf-8") + ) + steps = workflow["jobs"]["update-dependencies"]["steps"] + create_pr = next( + step for step in steps if step.get("run") == f".github/scripts/{CREATE_PR}" + ) + + assert create_pr["if"] == "steps.push.outputs.branch_pushed == 'true'" + assert create_pr["env"] == {"GH_TOKEN": "${{ steps.app-token.outputs.token }}"} + + # --- pr.yml --------------------------------------------------------------- START_LOCAL_SERVICES = "start-local-services.sh" From 9feec3e726c2ce421c943d83ecd41ec7de40cd3f Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:19:01 +0400 Subject: [PATCH 5/9] Enforce sim.process.pr-tracking-issue with the PR description job Move the rule from doc-only to enforced. Its enforcer is the "PR description" job in pr-description.yml, and the registry now names the step that runs check_pr_description.py. - Proof: pull_request event fixtures under rules/fixtures/sim.process.pr-tracking-issue. The bad one has the first line of PR #616, "Fixes #615."; the good one has the same description with the period removed. test_rule_proof_pr_tracking_issue runs the check on both, with a fake gh answering that #615 is open. pr.yml's test job runs the proof. - The statement now says the issue must be open and not a pull request, and that automated PRs open or reuse one. - pr-description.yml joins [discovery].workflows, so a new job there must be registered too. - github-prs.md gains a "The Fixes Line" section: what passes and fails, how to fix each failure, and how the automated PRs comply. - Regenerate docs/generated/rules.md and the Rules block in AGENTS.md. Making the job a required check is a repository-ruleset change and is not part of this commit. Fixes #861. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 3 +- docs/engineering/skills/github-prs.md | 44 +++++++++++++++++-- docs/generated/rules.md | 14 +++--- .../tests/test_pr_description_check.py | 38 +++++++++++++++- .../bad_event.json | 12 +++++ .../good_event.json | 12 +++++ rules/registry.toml | 25 ++++++++--- 7 files changed, 130 insertions(+), 18 deletions(-) create mode 100644 rules/fixtures/sim.process.pr-tracking-issue/bad_event.json create mode 100644 rules/fixtures/sim.process.pr-tracking-issue/good_event.json diff --git a/AGENTS.md b/AGENTS.md index 498175b75..803a663ba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,9 +150,8 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. ### Process and repository hygiene -- Checked: `sim.agents.adapters-point-to-agents-md`, `sim.process.rules-registry`, `sim.process.stacked-prs-full-ci-at-bottom` +- Checked: `sim.agents.adapters-point-to-agents-md`, `sim.process.pr-tracking-issue`, `sim.process.rules-registry`, `sim.process.stacked-prs-full-ci-at-bottom` - `sim.process.agents-never-bypass`: Agents never bypass branch rules, even with admin credentials: no gh pr merge --admin and no bypass option; a blocked merge is reported to a human. (doc-only, not checked: follow it yourself) -- `sim.process.pr-tracking-issue`: Every PR has a tracking issue in this repository, and the first line of its description is Fixes #ISSUE_NUMBER. (doc-only, not checked: follow it yourself) - `sim.process.rule-machinery-code-owned`: Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. (doc-only, not checked: follow it yourself) - `sim.process.same-repo-draft-prs`: PRs are opened as drafts from branches in PolicyEngine/policyengine-sim-api, never from forks. (doc-only, not checked: follow it yourself) diff --git a/docs/engineering/skills/github-prs.md b/docs/engineering/skills/github-prs.md index 2dce64f4c..47111226a 100644 --- a/docs/engineering/skills/github-prs.md +++ b/docs/engineering/skills/github-prs.md @@ -15,7 +15,8 @@ Before creating or sharing a PR: 2. Open a GitHub issue for the work, or verify that an appropriate issue already exists. 3. Put `Fixes #ISSUE_NUMBER` as the first line of the PR description, using the - issue number from the issue created or found in the previous step. + issue number from the issue created or found in the previous step. The + `PR description` check fails otherwise; see [The `Fixes` Line](#the-fixes-line). 4. Run formatting and the most relevant tests for the changed surface. If an expected check cannot be run, say so in the PR body. 5. Push the current branch to the canonical repository: @@ -44,8 +45,9 @@ human instead. In a stack of pull requests, only the lowest unmerged one runs the full PR workflow (`.github/workflows/pr.yml`) and the Modal image smoke. The ones above -it run only the `CI complete` job, which fails and says so. A stack therefore -uses one set of runners instead of one per layer. +it run only the `CI complete` job, which fails and says so, and the +`PR description` job, which every pull request runs. A stack therefore uses one +set of runners instead of one per layer. - **`CI complete` is the required check.** It needs every other job in `pr.yml`. On a pull request that isn't stacked, or is the lowest unmerged @@ -63,6 +65,42 @@ uses one set of runners instead of one per layer. `projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py` checks both. +## The `Fixes` Line + +Rule `sim.process.pr-tracking-issue`. The `PR description` job, in +`.github/workflows/pr-description.yml`, checks every pull request whenever it is +opened, reopened or edited and on every push, including each pull request in a +stack. It reads the description from the event that started the run and fails +unless: + +- the first line is exactly `Fixes #ISSUE_NUMBER`; +- `ISSUE_NUMBER` is an open issue in this repository. A pull request's number, + an issue that was moved to another repository, a closed issue and a number + with no issue all fail. + +CRLF line endings, which the GitHub web editor uses, and trailing spaces are +fine. These fail: + +- an empty description; +- a blank first line, or leading spaces before `Fixes`; +- other wording, such as `Fixes #615.`, `Closes #12` or `Refs #12`; +- a reference to another repository, such as `Fixes PolicyEngine/policyengine-api#12`. + +To fix a failure: + +- **Wrong first line:** edit the description. Saving the edit runs the check + again. +- **Closed issue:** if the pull request still fixes it, reopen the issue, then + re-run the job from the pull request's Checks tab. Reopening an issue starts + no run. Re-running checks the description as it was when the run started, so + after changing the description, rely on the run the edit starts. + +Automated pull requests follow the rule too. The policyengine package update +(`update-policyengine-package.sh`) opens an issue for each version. The daily +dependency update (`dependency-update-create-pr.sh`) reuses the open "Update +dependencies" issue or opens one, and also puts its `Fixes` line first on an +update pull request that is already open. + ## PR Title Do not add `[codex]`, `[claude]`, `[copilot]`, or other agent labels to PR diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 7fa416bbc..719c83ca6 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,11 +8,11 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 49 | 5 | 0 | 15 | 69 | +| mechanical | 50 | 5 | 0 | 14 | 69 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 54 of 69 rules. +**Blocking:** 55 of 69 rules. ## Gaps @@ -28,7 +28,6 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf - `sim.observability.id-is-diagnostic-only`: observability_id has diagnostic meaning only: never use it for idempotency, authorization, database identity, routing or business logic. - `sim.observability.registered-stages-only`: Span names come from the stage plans in policyengine_simulation_observability.stages; runtime code never adds span-name string literals. - `sim.process.agents-never-bypass`: Agents never bypass branch rules, even with admin credentials: no gh pr merge --admin and no bypass option; a blocked merge is reported to a human. -- `sim.process.pr-tracking-issue`: Every PR has a tracking issue in this repository, and the first line of its description is Fixes #ISSUE_NUMBER. - `sim.process.rule-machinery-code-owned`: Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. - `sim.process.same-repo-draft-prs`: PRs are opened as drafts from branches in PolicyEngine/policyengine-sim-api, never from forks. - `sim.quality.every-project-tested`: Every project and library with unit tests runs them in the PR test matrix. @@ -430,7 +429,7 @@ flowchart LR | --- | --- | --- | --- | --- | --- | --- | --- | | `sim.agents.adapters-point-to-agents-md` | CLAUDE.md and .github/copilot-instructions.md only point to AGENTS.md; skills and rules are listed in AGENTS.md alone. | mechanical | enforced | yes | quality guard `rules-registry` | `pr.yml:quality-guards` | yes | | `sim.process.agents-never-bypass` | Agents never bypass branch rules, even with admin credentials: no gh pr merge --admin and no bypass option; a blocked merge is reported to a human. | mechanical | doc-only | no | nothing | — | — | -| `sim.process.pr-tracking-issue` | Every PR has a tracking issue in this repository, and the first line of its description is Fixes #ISSUE_NUMBER. | mechanical | doc-only | no | nothing | — | — | +| `sim.process.pr-tracking-issue` | The first line of every PR's description is Fixes #ISSUE_NUMBER, naming an open issue, not a pull request, in this repository; automated PRs open or reuse such an issue. | mechanical | enforced | yes | CI job `pr-description` | `pr-description.yml:pr-description` | yes | | `sim.process.rule-machinery-code-owned` | Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. | mechanical | doc-only | no | nothing | — | — | | `sim.process.rules-registry` | Every rule is registered in rules/registry.toml, is enforced where it claims to be, and its rendered views are current. | mechanical | enforced | yes | quality guard `rules-registry` | `pr.yml:quality-guards` | **unproven** | | `sim.process.same-repo-draft-prs` | PRs are opened as drafts from branches in PolicyEngine/policyengine-sim-api, never from forks. | mechanical | doc-only | no | nothing | — | — | @@ -443,7 +442,9 @@ flowchart LR j_CI_pr_yml_quality_guards["CI pr.yml:quality-guards"] r_sim_process_agents_never_bypass("sim.process.agents-never-bypass (doc-only)") gap_unenforced>"not enforced"] - r_sim_process_pr_tracking_issue("sim.process.pr-tracking-issue (doc-only)") + r_sim_process_pr_tracking_issue("sim.process.pr-tracking-issue (enforced)") + e_CI_job_pr_description["CI job pr-description"] + j_CI_pr_description_yml_pr_description["CI pr-description.yml:pr-description"] r_sim_process_rule_machinery_code_owned("sim.process.rule-machinery-code-owned (doc-only)") r_sim_process_rules_registry("sim.process.rules-registry (enforced)") r_sim_process_same_repo_draft_prs("sim.process.same-repo-draft-prs (doc-only)") @@ -454,7 +455,8 @@ flowchart LR r_sim_agents_adapters_point_to_agents_md --> e_quality_guard_rules_registry e_quality_guard_rules_registry --> j_CI_pr_yml_quality_guards r_sim_process_agents_never_bypass -.-> gap_unenforced - r_sim_process_pr_tracking_issue -.-> gap_unenforced + r_sim_process_pr_tracking_issue --> e_CI_job_pr_description + e_CI_job_pr_description --> j_CI_pr_description_yml_pr_description r_sim_process_rule_machinery_code_owned -.-> gap_unenforced r_sim_process_rules_registry --> e_quality_guard_rules_registry r_sim_process_same_repo_draft_prs -.-> gap_unenforced diff --git a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py index 080a942e4..94a68c3fe 100644 --- a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py +++ b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py @@ -17,7 +17,12 @@ import pytest import yaml -from workflow_script_support import SCRIPTS_DIR, WORKFLOWS_DIR, FakeCommands +from workflow_script_support import ( + REPOSITORY_ROOT, + SCRIPTS_DIR, + WORKFLOWS_DIR, + FakeCommands, +) if TYPE_CHECKING: from pathlib import Path @@ -26,6 +31,9 @@ CHECK_SCRIPT = SCRIPTS_DIR / "check_pr_description.py" CHECK_WORKFLOW = WORKFLOWS_DIR / "pr-description.yml" CHECK_JOB = "pr-description" +PROOF_FIXTURES = ( + REPOSITORY_ROOT / "rules" / "fixtures" / "sim.process.pr-tracking-issue" +) REPOSITORY = "PolicyEngine/policyengine-sim-api" VALID_BODY = "Fixes #861\n\n## Summary\n\n- Adds a check.\n" @@ -470,3 +478,31 @@ def test_the_check_runs_the_tested_script_with_the_job_token(check_workflow): assert len(check_steps) == 1 assert check_steps[0]["run"] == f"python .github/scripts/{CHECK_SCRIPT.name}" assert check_steps[0]["env"] == {"GH_TOKEN": "${{ github.token }}"} + + +# --- Proof ---------------------------------------------------------------- + + +def test_rule_proof_pr_tracking_issue(fakes): + """The check rejects PR #616's first line, "Fixes #615.", and accepts it fixed. + + Both fixtures are pull_request event payloads; the fake gh answers that + #615 is an open issue in this repository. + """ + results = { + fixture: _run_script( + fakes.env( + GITHUB_EVENT_PATH=str(PROOF_FIXTURES / f"{fixture}_event.json"), + GITHUB_REPOSITORY=REPOSITORY, + **_gh_answers(_issue(615)), + ) + ) + for fixture in ("bad", "good") + } + + assert results["bad"].returncode == 1 + assert 'The first line is "Fixes #615."' in _error_annotation(results["bad"]) + assert results["good"].returncode == 0, results["good"].stdout + assert fakes.calls("gh") == [ + ["gh", "api", "repos/PolicyEngine/policyengine-sim-api/issues/615"] + ] diff --git a/rules/fixtures/sim.process.pr-tracking-issue/bad_event.json b/rules/fixtures/sim.process.pr-tracking-issue/bad_event.json new file mode 100644 index 000000000..5ffa821c1 --- /dev/null +++ b/rules/fixtures/sim.process.pr-tracking-issue/bad_event.json @@ -0,0 +1,12 @@ +{ + "action": "opened", + "number": 616, + "pull_request": { + "number": 616, + "title": "Return report-shaped US congressional district impacts", + "body": "Fixes #615.\n\nSummary:\n- Maps congressional district results to canonical district IDs like `AL-01`.\n- Returns district impacts only for US national and state-level reports.\n" + }, + "repository": { + "full_name": "PolicyEngine/policyengine-sim-api" + } +} diff --git a/rules/fixtures/sim.process.pr-tracking-issue/good_event.json b/rules/fixtures/sim.process.pr-tracking-issue/good_event.json new file mode 100644 index 000000000..f3b34f646 --- /dev/null +++ b/rules/fixtures/sim.process.pr-tracking-issue/good_event.json @@ -0,0 +1,12 @@ +{ + "action": "opened", + "number": 616, + "pull_request": { + "number": 616, + "title": "Return report-shaped US congressional district impacts", + "body": "Fixes #615\n\nSummary:\n- Maps congressional district results to canonical district IDs like `AL-01`.\n- Returns district impacts only for US national and state-level reports.\n" + }, + "repository": { + "full_name": "PolicyEngine/policyengine-sim-api" + } +} diff --git a/rules/registry.toml b/rules/registry.toml index dac2bb1a7..b9ef22c4c 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -52,7 +52,7 @@ id = "process" title = "Process and repository hygiene" [discovery] -workflows = [".github/workflows/pr.yml", ".github/workflows/pr-image-smoke.yml"] +workflows = [".github/workflows/pr.yml", ".github/workflows/pr-image-smoke.yml", ".github/workflows/pr-description.yml"] guard_test_globs = [ "projects/*/tests/test_*boundar*.py", "libs/*/tests/test_*boundar*.py", @@ -1485,14 +1485,27 @@ invocation = "unittest discover -s scripts/rules/tests" [[rule]] id = "sim.process.pr-tracking-issue" category = "process" -statement = "Every PR has a tracking issue in this repository, and the first line of its description is Fixes #ISSUE_NUMBER." -rationale = "docs/engineering/skills/github-prs.md#same-repository-draft-prs-only" -scope = [".github/pull_request_template.md"] +statement = "The first line of every PR's description is Fixes #ISSUE_NUMBER, naming an open issue, not a pull request, in this repository; automated PRs open or reuse such an issue." +rationale = "docs/engineering/skills/github-prs.md#the-fixes-line" +scope = [".github/pull_request_template.md", ".github/workflows/pr-description.yml", ".github/scripts/check_pr_description.py", ".github/scripts/dependency-update-create-pr.sh", ".github/scripts/update-policyengine-package.sh"] tier = "mechanical" -status = "doc-only" +status = "enforced" autofix = false [rule.enforcer] -kind = "none" +kind = "workflow-job" +step = "Check the first line and the tracking issue" +[[rule.ci]] +workflow = ".github/workflows/pr-description.yml" +job = "pr-description" +invocation = "check_pr_description.py" +[rule.proof] +bad_fixture = "rules/fixtures/sim.process.pr-tracking-issue/bad_event.json" +good_fixture = "rules/fixtures/sim.process.pr-tracking-issue/good_event.json" +project = "projects/policyengine-simulation-entry" +test = "tests/test_pr_description_check.py::test_rule_proof_pr_tracking_issue" +workflow = ".github/workflows/pr.yml" +job = "test" +invocation = "uv run pytest tests/" [[rule]] id = "sim.process.same-repo-draft-prs" From 34f8097160b299e54cf5fc2ffdba70e2a9874ab1 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:22:31 +0400 Subject: [PATCH 6/9] Skip the PR description check on closed pull requests GitHub also sends the pull_request "edited" event for closed pull requests. Merging a pull request closes the issue its first line names, so editing a merged pull request's description would run the check and report a failure on it. Run the job only when the pull request in the event is open. Every open pull request still runs it, including each one in a stack, so the required check is never skipped where it matters. Part of #861. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pr-description.yml | 7 +++++-- .../tests/test_pr_description_check.py | 12 +++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pr-description.yml b/.github/workflows/pr-description.yml index 1402247cd..f6734077a 100644 --- a/.github/workflows/pr-description.yml +++ b/.github/workflows/pr-description.yml @@ -6,8 +6,8 @@ name: PR description # # A separate workflow from pr.yml, so that editing a description reruns only # this check, not the whole of CI. It has no branches or paths filter and no -# stack condition: it is cheap, so every pull request runs it, including each -# one in a stack. +# stack condition: it is cheap, so every open pull request runs it, including +# each one in a stack. on: pull_request: @@ -28,6 +28,9 @@ concurrency: jobs: pr-description: name: PR description + # GitHub also sends "edited" for closed pull requests. Merging closed the + # issue a merged pull request names, so checking one would fail. + if: ${{ github.event.pull_request.state == 'open' }} runs-on: ubuntu-latest timeout-minutes: 5 diff --git a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py index 94a68c3fe..333bc2f85 100644 --- a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py +++ b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py @@ -446,13 +446,19 @@ def test_the_check_runs_on_every_event_that_can_change_the_description( } -def test_the_check_runs_on_every_pull_request_in_a_stack(check_workflow): - """Unlike pr.yml's jobs, this one has no condition and needs no other job.""" +def test_the_check_runs_on_every_open_pull_request_including_stacked_ones( + check_workflow, +): + """Unlike pr.yml's jobs, this one has no stack condition and needs no job. + + It skips closed pull requests: editing a merged pull request's description + would otherwise fail the check, because merging closed its issue. + """ assert list(check_workflow["jobs"]) == [CHECK_JOB] job = check_workflow["jobs"][CHECK_JOB] assert job["name"] == "PR description" - assert "if" not in job + assert job["if"] == "${{ github.event.pull_request.state == 'open' }}" assert "needs" not in job From 7445d637713ab4cb1ee53c8f11c2aba5f4fea687 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:23:16 +0400 Subject: [PATCH 7/9] Say every PR needs a tracking issue, and test two more lookup paths - README: every PR needs a tracking issue, not only non-trivial work, now that the PR description check fails without one. - AGENTS.md: list the PR description job alongside the rest of PR CI. - Test that gh api's HTTP 410 (a deleted issue) reports a missing issue, like HTTP 404. - Test that the dependency-update script replaces a "Fixes #" line that is the open update PR's whole description. The new tests cover existing behaviour, so they passed when added. Part of #861. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 4 +++- README.md | 5 +++-- .../tests/test_pr_description_check.py | 8 ++++++-- .../tests/test_workflow_scripts.py | 13 +++++++++++++ 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 803a663ba..aa0bf0098 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,7 +83,9 @@ same-repository draft PR. - API integration tests live in `projects/policyengine-apis-integ`. - PR CI runs simulation unit tests, Ruff format and lint checks, Pyright for every package, quality guards (including the rules registry), Docker build, - and local integration tests. + and local integration tests. A separate `PR description` job checks that + every open PR's description starts with `Fixes #ISSUE_NUMBER` for an open + issue in this repository. - There is currently no repository-wide changelog fragment requirement. ## Safety Boundaries diff --git a/README.md b/README.md index 3a3b8dcd8..df4db6273 100644 --- a/README.md +++ b/README.md @@ -147,8 +147,9 @@ See [AGENTS.md](AGENTS.md) and for the canonical same-repository draft PR workflow. 1. Create a feature branch (never commit to `main`). -2. Open a GitHub issue for non-trivial work; put `Fixes #ISSUE_NUMBER` as the - first line of the PR description. +2. Open a GitHub issue for the work; put `Fixes #ISSUE_NUMBER` as the first + line of the PR description. Every PR needs one: the `PR description` check + fails otherwise. 3. Make changes and run the relevant tests locally. 4. Open a same-repository draft PR: `make push-pr-branch`, then `gh pr create --draft`. diff --git a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py index 333bc2f85..b938b4cc1 100644 --- a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py +++ b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py @@ -309,13 +309,17 @@ def test_a_pull_request_number_fails(fakes, tmp_path): assert "#857 is a pull request, not an issue." in _error_annotation(result) -def test_a_missing_issue_fails(fakes, tmp_path): +@pytest.mark.parametrize( + "gh_error", + [NOT_FOUND_STDERR, "gh: This issue was deleted (HTTP 410)"], +) +def test_a_missing_issue_fails(fakes, tmp_path, gh_error): result = _run_check( fakes, tmp_path, "Fixes #999999\n", FAKE_GH_STDOUT=NOT_FOUND_STDOUT, - FAKE_GH_STDERR=NOT_FOUND_STDERR, + FAKE_GH_STDERR=gh_error, FAKE_GH_EXIT="1", ) diff --git a/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py b/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py index a7b4c66f8..a3cae0fd7 100644 --- a/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py +++ b/projects/policyengine-simulation-entry/tests/test_workflow_scripts.py @@ -461,6 +461,19 @@ def test_create_pr_replaces_an_earlier_fixes_line_on_the_open_update_pr( assert descriptions == f"Fixes #870\n{rest}\n" +def test_create_pr_replaces_a_fixes_line_that_is_the_whole_description(fakes, tmp_path): + result, descriptions = _create_pr( + fakes, + tmp_path, + FAKE_OPEN_PR="900", + FAKE_OPEN_PR_BODY="Fixes #860", + FAKE_OPEN_ISSUES="870\n", + ) + + assert result.returncode == 0, result.stderr + assert descriptions == "Fixes #870\n" + + def test_create_pr_leaves_an_open_update_pr_that_names_the_tracking_issue( fakes, tmp_path ): From f491355919fd592d23a77e0ea8906d1fe3d48e3d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:24:23 +0400 Subject: [PATCH 8/9] Say the PR description check runs on open pull requests github-prs.md said the job checks every pull request; since the previous commit it skips closed ones. Also correct a test fixture's docstring: it loads the check script as a module, which is not how the workflow runs it. Part of #861. Co-Authored-By: Claude Opus 5.5 --- docs/engineering/skills/github-prs.md | 12 ++++++------ .../tests/test_pr_description_check.py | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/engineering/skills/github-prs.md b/docs/engineering/skills/github-prs.md index 47111226a..77288057a 100644 --- a/docs/engineering/skills/github-prs.md +++ b/docs/engineering/skills/github-prs.md @@ -46,8 +46,8 @@ human instead. In a stack of pull requests, only the lowest unmerged one runs the full PR workflow (`.github/workflows/pr.yml`) and the Modal image smoke. The ones above it run only the `CI complete` job, which fails and says so, and the -`PR description` job, which every pull request runs. A stack therefore uses one -set of runners instead of one per layer. +`PR description` job, which every open pull request runs. A stack therefore +uses one set of runners instead of one per layer. - **`CI complete` is the required check.** It needs every other job in `pr.yml`. On a pull request that isn't stacked, or is the lowest unmerged @@ -68,10 +68,10 @@ set of runners instead of one per layer. ## The `Fixes` Line Rule `sim.process.pr-tracking-issue`. The `PR description` job, in -`.github/workflows/pr-description.yml`, checks every pull request whenever it is -opened, reopened or edited and on every push, including each pull request in a -stack. It reads the description from the event that started the run and fails -unless: +`.github/workflows/pr-description.yml`, checks every open pull request whenever +it is opened, reopened or edited and on every push, including each pull request +in a stack. It skips closed pull requests. It reads the description from the +event that started the run and fails unless: - the first line is exactly `Fixes #ISSUE_NUMBER`; - `ISSUE_NUMBER` is an open issue in this repository. A pull request's number, diff --git a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py index b938b4cc1..659f356c6 100644 --- a/projects/policyengine-simulation-entry/tests/test_pr_description_check.py +++ b/projects/policyengine-simulation-entry/tests/test_pr_description_check.py @@ -56,7 +56,7 @@ @pytest.fixture(scope="module") def check_module() -> ModuleType: - """Load the check script in this process, as the workflow runs it.""" + """Load the check script as a module in this process.""" spec = importlib.util.spec_from_file_location("check_pr_description", CHECK_SCRIPT) module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) From 21142841a7af2faab3ae37b0df0972cdd29741e7 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:54:09 +0400 Subject: [PATCH 9/9] Point the tracking-issue proof at the locked pytest command #887 changed the test job to run uv run --frozen pytest tests/, so the proof's recorded invocation no longer matched and the rules registry check failed after the rebase. Co-Authored-By: Claude Opus 5.5 --- rules/registry.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/registry.toml b/rules/registry.toml index b9ef22c4c..4e2f60794 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -1505,7 +1505,7 @@ project = "projects/policyengine-simulation-entry" test = "tests/test_pr_description_check.py::test_rule_proof_pr_tracking_issue" workflow = ".github/workflows/pr.yml" job = "test" -invocation = "uv run pytest tests/" +invocation = "uv run --frozen pytest tests/" [[rule]] id = "sim.process.same-repo-draft-prs"