From ea59bd96eb394c8ba78919099114682070a237ff Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:41:48 +0400 Subject: [PATCH 1/8] Add CODEOWNERS for the rule machinery Changes to the rules registry, guard scripts and CODEOWNERS itself need review from a core developer other than the author. Deliberately narrow: pyproject.toml and workflow files are not owned. Co-Authored-By: Claude Opus 5.5 --- .github/CODEOWNERS | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 000000000..67b450a4c --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,8 @@ +# Rule machinery: changes need review from a core developer other than the +# author. Deliberately narrow: pyproject.toml and workflow files are not owned, +# so dependency bots and everyday CI edits are not blocked. +/rules/ @PolicyEngine/core-developers +/scripts/rules/ @PolicyEngine/core-developers +/scripts/guards/ @PolicyEngine/core-developers +/scripts/run_quality_guards.py @PolicyEngine/core-developers +/.github/CODEOWNERS @PolicyEngine/core-developers From aa5911b24cdced5ee2560d751116adde734c61f4 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:42:11 +0400 Subject: [PATCH 2/8] Pin ruff to uv.lock in CI and make its rule set explicit The lint jobs ran `pip install ruff>=0.9.0` unquoted, so the shell read `>` as a redirect and CI installed the latest ruff. Install the version pinned in uv.lock instead, and select ruff's current default rules explicitly so an upgrade cannot change which rules apply. No rules are added or removed. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pr.yml | 9 +++++++-- .github/workflows/push.yml | 9 +++++++-- pyproject.toml | 5 +++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index b2bfb28ed..d4019dad2 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -40,8 +40,13 @@ jobs: uses: actions/checkout@v4 - name: Setup Python uses: actions/setup-python@v5 - - name: Install ruff - run: pip install ruff>=0.9.0 + with: + python-version: "3.12" + - name: Install ruff (version pinned in uv.lock) + run: | + version="$(python -c 'import tomllib; print(next(p["version"] for p in tomllib.load(open("uv.lock", "rb"))["package"] if p["name"] == "ruff"))')" + pip install "ruff==${version}" + ruff --version - name: Format check with ruff run: ruff format --check . typecheck-v2: diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 9213f4fbd..9d48745db 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -23,8 +23,13 @@ jobs: uses: actions/checkout@v4 - name: Setup Python uses: actions/setup-python@v5 - - name: Install ruff - run: pip install ruff>=0.9.0 + with: + python-version: "3.12" + - name: Install ruff (version pinned in uv.lock) + run: | + version="$(python -c 'import tomllib; print(next(p["version"] for p in tomllib.load(open("uv.lock", "rb"))["package"] if p["name"] == "ruff"))')" + pip install "ruff==${version}" + ruff --version - name: Format check with ruff run: ruff format --check . diff --git a/pyproject.toml b/pyproject.toml index 54988ccad..04f8d065a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -79,6 +79,11 @@ packages = ["policyengine_api"] [tool.hatch.metadata] allow-direct-references = true +[tool.ruff.lint] +# Ruff's defaults as of 0.15, made explicit so a ruff upgrade cannot change +# which rules apply. Widen this deliberately, one rule family per PR. +select = ["E4", "E7", "E9", "F"] + [tool.ruff.lint.per-file-ignores] "migrations/*/versions/*.py" = ["F401"] From 4bd5e71c5d78de091fd75df99da87587fcd392a7 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:42:50 +0400 Subject: [PATCH 3/8] Discover v2 modules in the CRUD boundary tests The four boundary checks listed their modules by hand, so the households and user_households packages (12 modules) were never checked. Discover the modules by filename instead, fail at collection if a pattern matches nothing, and require every connector module to be a recognised create, read, update or delete module so a new name cannot escape the checks. Co-Authored-By: Claude Opus 5.5 --- tests/unit/v2/test_data_crud_boundaries.py | 92 +++++++++++----------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/tests/unit/v2/test_data_crud_boundaries.py b/tests/unit/v2/test_data_crud_boundaries.py index 44b49ff3c..4e08780f6 100644 --- a/tests/unit/v2/test_data_crud_boundaries.py +++ b/tests/unit/v2/test_data_crud_boundaries.py @@ -38,55 +38,63 @@ def _imported_modules(path: Path) -> set[str]: return modules -@pytest.mark.parametrize( - "relative_path", - ( - "policies/database_connectors/creates.py", - "comparison_runs/database_connectors/creates.py", - "user_policies/database_connectors/creates.py", - "comparison_runs/database_connectors/updates.py", - "user_policies/database_connectors/updates.py", - "user_policies/database_connectors/deletes.py", - ), +def _discover(*patterns: str) -> list[str]: + """Modules under SERVICES_ROOT matching any pattern, as sorted relative paths. + + Discovering modules (rather than listing them) means a new resource package + is checked automatically. An empty result fails at collection, because + pytest silently skips a test whose parameter list is empty. + """ + paths = sorted( + { + path.relative_to(SERVICES_ROOT).as_posix() + for pattern in patterns + for path in SERVICES_ROOT.glob(pattern) + } + ) + assert paths, f"no modules under {SERVICES_ROOT} match {patterns}" + return paths + + +MUTATION_CONNECTORS = _discover( + "*/database_connectors/creates*.py", + "*/database_connectors/updates*.py", + "*/database_connectors/deletes*.py", ) +READ_CONNECTORS = _discover("*/database_connectors/reads*.py") +VALIDATION_AND_TRANSFORMATION_MODULES = _discover( + "*/validators.py", "*/transformations.py" +) +DATABASE_SESSION_MODULES = _discover("*/database_session.py") + + +def test_every_connector_module_is_classified() -> None: + connectors = { + path + for path in _discover("*/database_connectors/*.py") + if not path.endswith("/__init__.py") + } + classified = set(MUTATION_CONNECTORS) | set(READ_CONNECTORS) + assert connectors - classified == set(), ( + "connector modules must be named creates, reads, updates or deletes " + "(optionally with a _suffix) so the boundary checks below cover them" + ) + + +@pytest.mark.parametrize("relative_path", MUTATION_CONNECTORS) def test_mutation_connectors_contain_no_database_reads(relative_path: str) -> None: calls = _called_names(SERVICES_ROOT / relative_path) assert "select" not in calls assert "get" not in calls -@pytest.mark.parametrize( - "relative_path", - ( - "policies/database_connectors/reads.py", - "comparison_runs/database_connectors/reads.py", - "user_policies/database_connectors/reads.py", - "metadata/database_connectors/reads.py", - "metadata/database_connectors/reads_datasets.py", - "metadata/database_connectors/reads_parameter_tree.py", - "metadata/database_connectors/reads_parameters.py", - "metadata/database_connectors/reads_regions.py", - "metadata/database_connectors/reads_variables.py", - ), -) +@pytest.mark.parametrize("relative_path", READ_CONNECTORS) def test_read_connectors_contain_no_database_mutations(relative_path: str) -> None: calls = _called_names(SERVICES_ROOT / relative_path) assert calls.isdisjoint({"insert", "add", "add_all", "delete"}) -@pytest.mark.parametrize( - "relative_path", - ( - "policies/validators.py", - "policies/transformations.py", - "metadata/validators.py", - "metadata/transformations.py", - "comparison_runs/validators.py", - "comparison_runs/transformations.py", - "user_policies/validators.py", - "user_policies/transformations.py", - ), -) +@pytest.mark.parametrize("relative_path", VALIDATION_AND_TRANSFORMATION_MODULES) def test_validation_and_transformation_modules_have_no_database_query_dependency( relative_path: str, ) -> None: @@ -100,15 +108,7 @@ def test_validation_and_transformation_modules_have_no_database_query_dependency ) -@pytest.mark.parametrize( - "relative_path", - ( - "policies/database_session.py", - "comparison_runs/database_session.py", - "metadata/database_session.py", - "user_policies/database_session.py", - ), -) +@pytest.mark.parametrize("relative_path", DATABASE_SESSION_MODULES) def test_database_session_modules_do_not_construct_queries( relative_path: str, ) -> None: From 6332515416f29a7ae65ee1b6512516f19b1649ee Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:43:09 +0400 Subject: [PATCH 4/8] Document tracking issues, Fixes lines and the no-bypass rule for PRs Every PR needs a tracking issue and `Fixes #ISSUE_NUMBER` as the first line of its description; a PR template starts with that line. Agents must never bypass branch rules, even with admin credentials. Co-Authored-By: Claude Opus 5.5 --- .github/pull_request_template.md | 9 +++++++++ AGENTS.md | 4 +++- docs/engineering/skills/github-prs.md | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..daaae976d --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,9 @@ +Fixes # + +## Summary + +- + +## Testing + +- diff --git a/AGENTS.md b/AGENTS.md index f3034a66d..82b5f1c4f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,7 +35,9 @@ When modifying the `Makefile`, read ## GitHub PRs Read `docs/engineering/skills/github-prs.md` before opening, replacing, or -sharing any pull request. +sharing any pull request. Every PR needs a tracking issue and +`Fixes #ISSUE_NUMBER` as the first line of its description. Agents must never +bypass branch rules (for example with `gh pr merge --admin`). Before creating or sharing a migration PR, run the focused migration guards: diff --git a/docs/engineering/skills/github-prs.md b/docs/engineering/skills/github-prs.md index ddbc83d9a..34fb99cb3 100644 --- a/docs/engineering/skills/github-prs.md +++ b/docs/engineering/skills/github-prs.md @@ -2,6 +2,22 @@ Use this skill before opening, replacing, or sharing a pull request. +## Tracking Issues + +Before opening a PR, all developers and agents must: + +1. Open a GitHub issue for the work in `PolicyEngine/policyengine-api`, or + verify that an appropriate issue already exists. +2. Put `Fixes #ISSUE_NUMBER` as the first line of the PR description, using the + issue number from the previous step. The PR template starts with this line. + +## Branch Rules + +Agents must never bypass branch rules, even when their credentials have admin +rights: do not merge with `gh pr merge --admin`, and do not use GitHub's option +to bypass rules. If a required check or review blocks a merge, report it to a +human instead. + ## Migration PRs Migration PRs should keep the user experience stable unless the PR explicitly From 39a9d76a52a0f3ef62ea443644be2b2297167e64 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:43:09 +0400 Subject: [PATCH 5/8] Add changelog fragment for Phase 0 enforcement changes Co-Authored-By: Claude Opus 5.5 --- changelog.d/phase-0-enforcement.changed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/phase-0-enforcement.changed.md diff --git a/changelog.d/phase-0-enforcement.changed.md b/changelog.d/phase-0-enforcement.changed.md new file mode 100644 index 000000000..0d168901a --- /dev/null +++ b/changelog.d/phase-0-enforcement.changed.md @@ -0,0 +1 @@ +Added CODEOWNERS for the rule machinery, pinned CI's ruff to the version in uv.lock with an explicit rule selection, made the v2 CRUD boundary tests discover modules so households and user_households are checked, and documented the tracking-issue, Fixes line and no-bypass PR rules. From 8cedb09a479a514f58fb3018251044e034955bee Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:57:10 +0400 Subject: [PATCH 6/8] Move the locked ruff install into a tested script Multi-line workflow run blocks become tested shell scripts. The lint jobs now run .github/scripts/install_locked_ruff.sh, which reads ruff's version from uv.lock and installs it, and tests cover the lock lookup, the default lock, a missing pin and a failing pip. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/install_locked_ruff.sh | 30 ++++++ .github/workflows/pr.yml | 5 +- .github/workflows/push.yml | 5 +- tests/unit/test_install_locked_ruff_script.py | 94 +++++++++++++++++++ 4 files changed, 126 insertions(+), 8 deletions(-) create mode 100644 .github/scripts/install_locked_ruff.sh create mode 100644 tests/unit/test_install_locked_ruff_script.py diff --git a/.github/scripts/install_locked_ruff.sh b/.github/scripts/install_locked_ruff.sh new file mode 100644 index 000000000..a93e802c5 --- /dev/null +++ b/.github/scripts/install_locked_ruff.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash + +# Install the ruff version pinned in uv.lock, so CI formats and lints with the +# same ruff as `uv sync --extra dev`. +# +# Usage: install_locked_ruff.sh [path/to/uv.lock] +# PYTHON selects the interpreter that reads the lock (Python 3.11+ for tomllib). + +set -euo pipefail + +lock_file="${1:-uv.lock}" +python_bin="${PYTHON:-python3}" + +version="$( + "${python_bin}" - "${lock_file}" <<'PY' +import sys +import tomllib + +lock_path = sys.argv[1] +with open(lock_path, "rb") as lock: + packages = tomllib.load(lock).get("package", []) +versions = [package["version"] for package in packages if package.get("name") == "ruff"] +if not versions: + sys.exit(f"ruff is not pinned in {lock_path}; add it to the dev extra and run uv lock") +print(versions[0]) +PY +)" + +pip install "ruff==${version}" +ruff --version diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index d4019dad2..22ae6a68b 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -43,10 +43,7 @@ jobs: with: python-version: "3.12" - name: Install ruff (version pinned in uv.lock) - run: | - version="$(python -c 'import tomllib; print(next(p["version"] for p in tomllib.load(open("uv.lock", "rb"))["package"] if p["name"] == "ruff"))')" - pip install "ruff==${version}" - ruff --version + run: bash .github/scripts/install_locked_ruff.sh - name: Format check with ruff run: ruff format --check . typecheck-v2: diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 9d48745db..b153d92ae 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -26,10 +26,7 @@ jobs: with: python-version: "3.12" - name: Install ruff (version pinned in uv.lock) - run: | - version="$(python -c 'import tomllib; print(next(p["version"] for p in tomllib.load(open("uv.lock", "rb"))["package"] if p["name"] == "ruff"))')" - pip install "ruff==${version}" - ruff --version + run: bash .github/scripts/install_locked_ruff.sh - name: Format check with ruff run: ruff format --check . diff --git a/tests/unit/test_install_locked_ruff_script.py b/tests/unit/test_install_locked_ruff_script.py new file mode 100644 index 000000000..78ba3aae6 --- /dev/null +++ b/tests/unit/test_install_locked_ruff_script.py @@ -0,0 +1,94 @@ +"""Tests for .github/scripts/install_locked_ruff.sh.""" + +from __future__ import annotations + +import os +import subprocess +import sys +import tomllib +from pathlib import Path + +REPO = Path(__file__).resolve().parents[2] +SCRIPT = REPO / ".github/scripts/install_locked_ruff.sh" + + +def _fake_commands(tmp_path: Path, *, pip_exit: int = 0) -> tuple[Path, Path]: + """Fake pip and ruff that record their arguments instead of installing.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + calls = tmp_path / "calls.log" + for name, exit_code in (("pip", pip_exit), ("ruff", 0)): + command = bin_dir / name + command.write_text( + f'#!/usr/bin/env bash\necho "{name} $*" >> "{calls}"\nexit {exit_code}\n' + ) + command.chmod(0o755) + return bin_dir, calls + + +def _run( + tmp_path: Path, *args: str, pip_exit: int = 0 +) -> tuple[subprocess.CompletedProcess[str], list[str]]: + bin_dir, calls = _fake_commands(tmp_path, pip_exit=pip_exit) + result = subprocess.run( + ["bash", str(SCRIPT), *args], + cwd=REPO, + env={ + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + "PYTHON": sys.executable, + }, + text=True, + capture_output=True, + check=False, + ) + logged = calls.read_text().splitlines() if calls.exists() else [] + return result, logged + + +def _lock(tmp_path: Path, packages: str) -> Path: + lock = tmp_path / "uv.lock" + lock.write_text(f"version = 1\n{packages}") + return lock + + +def test_installs_the_ruff_version_pinned_in_the_lock(tmp_path: Path) -> None: + lock = _lock( + tmp_path, + '[[package]]\nname = "pytest"\nversion = "8.0.0"\n\n' + '[[package]]\nname = "ruff"\nversion = "0.15.12"\n', + ) + + result, calls = _run(tmp_path, str(lock)) + + assert result.returncode == 0, result.stderr + assert calls == ["pip install ruff==0.15.12", "ruff --version"] + + +def test_defaults_to_the_repository_lock(tmp_path: Path) -> None: + with (REPO / "uv.lock").open("rb") as lock: + packages = tomllib.load(lock)["package"] + locked = next(p["version"] for p in packages if p["name"] == "ruff") + + result, calls = _run(tmp_path) + + assert result.returncode == 0, result.stderr + assert calls[0] == f"pip install ruff=={locked}" + + +def test_fails_without_installing_when_ruff_is_not_locked(tmp_path: Path) -> None: + lock = _lock(tmp_path, '[[package]]\nname = "pytest"\nversion = "8.0.0"\n') + + result, calls = _run(tmp_path, str(lock)) + + assert result.returncode != 0 + assert "ruff is not pinned" in result.stderr + assert calls == [] + + +def test_stops_when_pip_fails(tmp_path: Path) -> None: + lock = _lock(tmp_path, '[[package]]\nname = "ruff"\nversion = "0.15.12"\n') + + result, calls = _run(tmp_path, str(lock), pip_exit=1) + + assert result.returncode != 0 + assert calls == ["pip install ruff==0.15.12"] From 6ddebe37e0a9d40b82fe5b27509fa7d27e55b685 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:43:02 +0400 Subject: [PATCH 7/8] Run full CI only on the lowest unmerged PR in a stack GitHub runs pull_request workflows for every PR in a stack as if each targeted the trunk, so a nine-PR stack ran the whole workflow nine times. Every job without needs now carries the stack condition, and the jobs that need them are skipped with them. A new CI complete job needs every other job and is meant to be the single required check. A skipped job counts as passing, so the gate fails on any stacked PR above the lowest unmerged one, and otherwise passes only when every job passed. Stacks therefore merge bottom-up, and nothing merges untested. check_ci_complete.sh holds the gate logic, and tests/unit/test_ci_complete_gate.py tests it and checks that every job is needed by the gate and carries the condition. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_ci_complete.sh | 39 +++++ .github/workflows/pr.yml | 52 +++++++ changelog.d/phase-0-enforcement.changed.md | 2 +- docs/engineering/skills/github-prs.md | 20 +++ tests/unit/test_ci_complete_gate.py | 167 +++++++++++++++++++++ 5 files changed, 279 insertions(+), 1 deletion(-) create mode 100644 .github/scripts/check_ci_complete.sh create mode 100644 tests/unit/test_ci_complete_gate.py diff --git a/.github/scripts/check_ci_complete.sh b/.github/scripts/check_ci_complete.sh new file mode 100644 index 000000000..bcfb70a92 --- /dev/null +++ b/.github/scripts/check_ci_complete.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash + +# Gate behind the "CI complete" check in pr.yml. +# +# In a stack of pull requests, only the lowest unmerged one runs the full +# workflow; the jobs of the others are skipped to save runners. A skipped job +# counts as passing for a required check, so this gate fails on those pull +# requests instead. On every other pull request it passes only when every job +# it needs passed. +# +# Environment: +# JOB_RESULTS toJSON(needs) for the gate job +# PULL_REQUEST_BASE_REF github.event.pull_request.base.ref +# STACK_BASE_REF github.event.pull_request.stack.base.ref, empty when +# the pull request isn't in a stack + +set -euo pipefail + +job_results="${JOB_RESULTS:?JOB_RESULTS is required}" +pull_request_base_ref="${PULL_REQUEST_BASE_REF:?PULL_REQUEST_BASE_REF is required}" +stack_base_ref="${STACK_BASE_REF:-}" + +if [[ -n "${stack_base_ref}" && "${stack_base_ref}" != "${pull_request_base_ref}" ]]; then + echo "::error::Full CI runs only on the lowest unmerged pull request in a stack. This one targets ${pull_request_base_ref}; it runs once the pull request below it merges and this one is rebased onto ${stack_base_ref}." + exit 1 +fi + +unsuccessful_jobs="$( + jq -r 'to_entries[] | select(.value.result != "success") | "\(.key): \(.value.result)"' <<<"${job_results}" +)" + +if [[ -n "${unsuccessful_jobs}" ]]; then + while IFS= read -r unsuccessful_job; do + echo "::error::Job did not pass: ${unsuccessful_job}" + done <<<"${unsuccessful_jobs}" + exit 1 +fi + +echo "Every job passed." diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 22ae6a68b..1c99a021b 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -11,8 +11,46 @@ env: OLD_SIMULATION_GATEWAY_URL: ${{ vars.OLD_SIMULATION_GATEWAY_URL }} jobs: + ci-complete: + # The required check. Needs every other job: on a stacked pull request + # above the lowest unmerged one it fails, because those jobs were skipped + # and a skipped job would count as passing; otherwise it passes only when + # every job passed. See docs/engineering/skills/github-prs.md. + name: CI complete + if: ${{ always() }} + needs: + - ensure-policyengine-bundle-supported-by-simulation-api + - lint + - typecheck-v2 + - quality-guards + - alembic-v1-check + - alembic-v2-check + - v2-integration-check + - check-changelog + - test_container_builds + - test_cloud_run_container_builds + - test_env_vars + - test + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout the gate script + uses: actions/checkout@v4 + with: + sparse-checkout: .github/scripts/check_ci_complete.sh + sparse-checkout-cone-mode: false + - name: Require full CI, with every job passing + env: + JOB_RESULTS: ${{ toJSON(needs) }} + PULL_REQUEST_BASE_REF: ${{ github.event.pull_request.base.ref }} + STACK_BASE_REF: ${{ github.event.pull_request.stack.base.ref }} + run: bash .github/scripts/check_ci_complete.sh + ensure-policyengine-bundle-supported-by-simulation-api: name: Ensure PolicyEngine bundle is supported by simulation API + # Full CI runs only on the lowest unmerged pull request in a stack. + if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} runs-on: ubuntu-latest environment: staging env: @@ -34,6 +72,8 @@ jobs: lint: name: Lint + # Full CI runs only on the lowest unmerged pull request in a stack. + if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} runs-on: ubuntu-latest steps: - name: Checkout repo @@ -48,6 +88,8 @@ jobs: run: ruff format --check . typecheck-v2: name: Type-check API v2 + # Full CI runs only on the lowest unmerged pull request in a stack. + if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} runs-on: ubuntu-latest steps: - name: Checkout repo @@ -64,6 +106,8 @@ jobs: run: uv run --frozen --extra dev mypy quality-guards: name: Quality guards + # Full CI runs only on the lowest unmerged pull request in a stack. + if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} runs-on: ubuntu-latest steps: - name: Checkout repo @@ -77,17 +121,25 @@ jobs: alembic-v1-check: name: Alembic v1 qualification + # Full CI runs only on the lowest unmerged pull request in a stack. + if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} uses: ./.github/workflows/alembic-v1-check.yml alembic-v2-check: name: Alembic v2 qualification + # Full CI runs only on the lowest unmerged pull request in a stack. + if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} uses: ./.github/workflows/alembic-v2-check.yml v2-integration-check: name: Cross-database integration tests + # Full CI runs only on the lowest unmerged pull request in a stack. + if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} uses: ./.github/workflows/v2-integration-check.yml secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} check-changelog: name: Check changelog fragment + # Full CI runs only on the lowest unmerged pull request in a stack. + if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 diff --git a/changelog.d/phase-0-enforcement.changed.md b/changelog.d/phase-0-enforcement.changed.md index 0d168901a..f2f681554 100644 --- a/changelog.d/phase-0-enforcement.changed.md +++ b/changelog.d/phase-0-enforcement.changed.md @@ -1 +1 @@ -Added CODEOWNERS for the rule machinery, pinned CI's ruff to the version in uv.lock with an explicit rule selection, made the v2 CRUD boundary tests discover modules so households and user_households are checked, and documented the tracking-issue, Fixes line and no-bypass PR rules. +Added CODEOWNERS for the rule machinery, pinned CI's ruff to the version in uv.lock with an explicit rule selection, made the v2 CRUD boundary tests discover modules so households and user_households are checked, and documented the tracking-issue, Fixes line and no-bypass PR rules. In a stack of pull requests, only the lowest unmerged one now runs the full PR workflow, behind a new "CI complete" gate job that fails on the others. diff --git a/docs/engineering/skills/github-prs.md b/docs/engineering/skills/github-prs.md index 34fb99cb3..dcb2ab7d8 100644 --- a/docs/engineering/skills/github-prs.md +++ b/docs/engineering/skills/github-prs.md @@ -18,6 +18,26 @@ rights: do not merge with `gh pr merge --admin`, and do not use GitHub's option to bypass rules. If a required check or review blocks a merge, report it to a human instead. +## Stacked PRs and CI + +In a stack of pull requests, only the lowest unmerged one runs the full PR +workflow (`.github/workflows/pr.yml`). 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. + +- **`CI complete` is the required check.** It needs every other job. On a + pull request that isn't stacked, or is the lowest unmerged one, it passes + only when every job passed. Skipped jobs would otherwise count as passing, so + a stack can only merge bottom-up, and nothing merges untested. +- **When the pull request below merges,** GitHub rebases the next one onto + the trunk, and that push starts its full run. If no run starts, push again + (`gh stack rebase`, then `gh stack push`), or close and reopen the pull + request. Re-running the old workflow doesn't help: it reuses the old event, + which still describes the pull request as higher in the stack. +- **When you add a job to `pr.yml`,** add it to `CI complete`'s `needs`. If + the job has no `needs` of its own, also give it the same stack condition as + the other jobs. `tests/unit/test_ci_complete_gate.py` checks both. + ## Migration PRs Migration PRs should keep the user experience stable unless the PR explicitly diff --git a/tests/unit/test_ci_complete_gate.py b/tests/unit/test_ci_complete_gate.py new file mode 100644 index 000000000..4905be87a --- /dev/null +++ b/tests/unit/test_ci_complete_gate.py @@ -0,0 +1,167 @@ +"""Tests for the stacked-PR CI gate: pr.yml's shape and check_ci_complete.sh. + +Only the lowest unmerged pull request in a stack runs the full PR workflow. +The "CI complete" job is the required check: it needs every other job, fails on +the pull requests above the bottom of a stack, and otherwise passes only when +every job passed. +""" + +from __future__ import annotations + +import json +import os +import subprocess +from pathlib import Path + +import yaml + +REPO = Path(__file__).resolve().parents[2] +PR_WORKFLOW = REPO / ".github" / "workflows" / "pr.yml" +GATE_SCRIPT = ".github/scripts/check_ci_complete.sh" +GATE_JOB = "ci-complete" +GATE_CONDITION = "${{ always() }}" +FULL_CI_CONDITION = ( + "${{ !github.event.pull_request.stack || " + "github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }}" +) + + +def gate_violations(workflow: dict) -> list[str]: + """Return every way a job in ``workflow`` could escape the CI-complete gate.""" + jobs = workflow.get("jobs") or {} + gate = jobs.get(GATE_JOB) + if gate is None: + return [f"the workflow has no {GATE_JOB} job"] + + violations = [] + if gate.get("if") != GATE_CONDITION: + violations.append(f"{GATE_JOB} must have if: {GATE_CONDITION}") + + gated_jobs = gate.get("needs") or [] + if isinstance(gated_jobs, str): + gated_jobs = [gated_jobs] + for job_id in sorted(set(jobs) - {GATE_JOB} - set(gated_jobs)): + violations.append(f"{GATE_JOB} does not need {job_id}") + + for job_id, job in jobs.items(): + if job_id == GATE_JOB: + continue + condition = job.get("if") + if job.get("needs"): + # A job that needs a skipped job is skipped too, unless its own + # condition overrides that. + if condition is not None and condition != FULL_CI_CONDITION: + violations.append(f"{job_id} has its own condition: {condition}") + elif condition != FULL_CI_CONDITION: + violations.append(f"{job_id} must have if: {FULL_CI_CONDITION}") + return violations + + +def test_pr_workflow_is_gated() -> None: + workflow = yaml.safe_load(PR_WORKFLOW.read_text()) + + assert gate_violations(workflow) == [] + + +def test_gate_check_rejects_an_ungated_job() -> None: + workflow = { + "jobs": { + GATE_JOB: {"if": GATE_CONDITION, "needs": ["lint"]}, + "lint": {"if": FULL_CI_CONDITION}, + "test": {}, + } + } + + assert gate_violations(workflow) == [ + f"{GATE_JOB} does not need test", + f"test must have if: {FULL_CI_CONDITION}", + ] + + +def _run_gate( + job_results: dict[str, str], + *, + pull_request_base_ref: str = "master", + stack_base_ref: str = "", +) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["bash", GATE_SCRIPT], + cwd=REPO, + env={ + "HOME": os.environ.get("HOME", ""), + "PATH": os.environ["PATH"], + "JOB_RESULTS": json.dumps( + { + job_id: {"result": result, "outputs": {}} + for job_id, result in job_results.items() + } + ), + "PULL_REQUEST_BASE_REF": pull_request_base_ref, + "STACK_BASE_REF": stack_base_ref, + }, + text=True, + capture_output=True, + check=False, + ) + + +def test_gate_passes_an_unstacked_pull_request_when_every_job_passed() -> None: + result = _run_gate({"lint": "success", "test": "success"}) + + assert result.returncode == 0, result.stdout + assert result.stdout == "Every job passed.\n" + + +def test_gate_passes_the_lowest_unmerged_pull_request_in_a_stack() -> None: + result = _run_gate( + {"lint": "success", "test": "success"}, + pull_request_base_ref="master", + stack_base_ref="master", + ) + + assert result.returncode == 0, result.stdout + + +def test_gate_fails_a_pull_request_higher_in_a_stack() -> None: + result = _run_gate( + {"lint": "skipped", "test": "skipped"}, + pull_request_base_ref="ruff-annotations", + stack_base_ref="master", + ) + + assert result.returncode == 1 + assert "lowest unmerged pull request in a stack" in result.stdout + assert "targets ruff-annotations" in result.stdout + + +def test_gate_fails_and_names_every_job_that_did_not_pass() -> None: + result = _run_gate( + {"lint": "success", "test": "failure", "typecheck-v2": "cancelled"}, + ) + + assert result.returncode == 1 + assert result.stdout.splitlines() == [ + "::error::Job did not pass: test: failure", + "::error::Job did not pass: typecheck-v2: cancelled", + ] + + +def test_gate_fails_when_a_job_was_skipped_on_full_ci() -> None: + result = _run_gate({"lint": "success", "test": "skipped"}, stack_base_ref="master") + + assert result.returncode == 1 + assert "::error::Job did not pass: test: skipped" in result.stdout + + +def test_gate_requires_the_job_results() -> None: + result = subprocess.run( + ["bash", GATE_SCRIPT], + cwd=REPO, + env={"PATH": os.environ["PATH"], "PULL_REQUEST_BASE_REF": "master"}, + text=True, + capture_output=True, + check=False, + ) + + assert result.returncode != 0 + assert "JOB_RESULTS is required" in result.stderr From 299dba8bb0abcd00860fd914706cff27d1699a33 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:59:45 +0400 Subject: [PATCH 8/8] Allow only the stacked-PR condition on the migration checks test_pr_always_runs_reusable_alembic_and_v2_integration_checks forbade any if: on the Alembic v2 and cross-database jobs, so they could never be skipped. They now carry the stacked-PR condition like every other job: they still run on every PR that isn't stacked and on the lowest unmerged PR of a stack, and CI complete, which needs them, fails on the rest. The test now allows exactly that condition and nothing else. Co-Authored-By: Claude Opus 5.5 --- tests/unit/test_alembic_workflows.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_alembic_workflows.py b/tests/unit/test_alembic_workflows.py index 1e2114154..81801d5c4 100644 --- a/tests/unit/test_alembic_workflows.py +++ b/tests/unit/test_alembic_workflows.py @@ -10,10 +10,24 @@ REPO = Path(__file__).resolve().parents[2] +STACKED_PR_CONDITION = ( + "${{ !github.event.pull_request.stack || " + "github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }}" +) + + def _workflow(name: str) -> str: return (REPO / ".github" / "workflows" / name).read_text(encoding="utf-8") +def _job_conditions(job_text: str) -> list[str]: + return [ + line.strip().removeprefix("if:").strip() + for line in job_text.splitlines() + if line.strip().startswith("if:") + ] + + def _long_inline_run_blocks() -> list[str]: offenders = [] block_markers = {"|", "|-", "|+", ">", ">-", ">+"} @@ -74,9 +88,13 @@ def test_pr_always_runs_reusable_alembic_and_v2_integration_checks(): assert "uses: ./.github/workflows/v2-integration-check.yml" in integration_job assert "CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}" in integration_job assert "needs:" not in alembic_job - assert "if:" not in alembic_job assert "needs:" not in integration_job - assert "if:" not in integration_job + # The only condition allowed is the stacked-PR one: these checks run on + # every PR that isn't stacked and on the lowest unmerged PR of a stack, and + # CI complete, which needs them, fails on the others + # (tests/unit/test_ci_complete_gate.py). + assert _job_conditions(alembic_job) == [STACKED_PR_CONDITION] + assert _job_conditions(integration_job) == [STACKED_PR_CONDITION] def test_push_requires_schema_and_v2_integration_checks_before_versioning():