From 5c30cc24b5369f489bb838139fa5cb97cb7c898c Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:43:24 +0400 Subject: [PATCH 1/6] Add CODEOWNERS for the rule machinery Changes to the rules registry, the guard runner and CODEOWNERS itself need review from a core developer other than the author. Deliberately narrow: pyproject.toml files and workflow files are not owned. Co-Authored-By: Claude Opus 5.5 --- .github/CODEOWNERS | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 000000000..60e98ef7f --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,7 @@ +# Rule machinery: changes need review from a core developer other than the +# author. Deliberately narrow: pyproject.toml files 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/run_quality_guards.py @PolicyEngine/core-developers +/.github/CODEOWNERS @PolicyEngine/core-developers From ea30b47986f1a871915e8c2fd4eb5752743ccf05 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:44:08 +0400 Subject: [PATCH 2/6] Pin ruff in CI and make its rule set explicit The lint job installed whatever ruff was latest. Pin it to 0.15.22 (the newest version in the project locks), and add a root ruff.toml selecting ruff's current default rules so an upgrade cannot change which rules apply. ruff format output is unchanged across every src and tests directory. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pr.yml | 3 ++- ruff.toml | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 ruff.toml diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index f4b4916f1..aa2fe3413 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -64,7 +64,8 @@ jobs: uses: astral-sh/setup-uv@v8.1.0 - name: Install ruff - run: uv tool install ruff + # Pinned so a ruff release cannot change CI results; see ruff.toml. + run: uv tool install ruff==0.15.22 - name: Run ruff format check run: | diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 000000000..21a4f7848 --- /dev/null +++ b/ruff.toml @@ -0,0 +1,4 @@ +# 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. +[lint] +select = ["E4", "E7", "E9", "F"] From 64f275cf162a1808aaa7a80fd78a2d14b303c3d2 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:44:08 +0400 Subject: [PATCH 3/6] Close two CI blind spots: Stage 12 image smoke and fastapi tests The image smoke skipped PRs that only change policyengine-stage12-persistence, although the v2 Modal images mount it; add it to the path filter. Add libs/policyengine-fastapi to the PR test matrix and make test, so its tests run in CI like every other library. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pr-image-smoke.yml | 1 + .github/workflows/pr.yml | 1 + Makefile | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-image-smoke.yml b/.github/workflows/pr-image-smoke.yml index 23d3a875f..13ee720d7 100644 --- a/.github/workflows/pr-image-smoke.yml +++ b/.github/workflows/pr-image-smoke.yml @@ -18,6 +18,7 @@ on: - 'libs/policyengine-simulation-contract/**' - 'libs/policyengine-simulation-observability/**' - 'libs/policyengine-fastapi/**' + - 'libs/policyengine-stage12-persistence/**' - '.github/workflows/pr-image-smoke.yml' - '.github/scripts/modal-image-smoke.sh' diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index aa2fe3413..bf8491c31 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -24,6 +24,7 @@ jobs: - libs/policyengine-simulation-contract - libs/policyengine-simulation-observability - libs/policyengine-stage12-persistence + - libs/policyengine-fastapi steps: - uses: actions/checkout@v6 diff --git a/Makefile b/Makefile index d4e751181..dd81d034b 100644 --- a/Makefile +++ b/Makefile @@ -83,7 +83,8 @@ test: projects/policyengine-simulation-gateway \ libs/policyengine-simulation-contract \ libs/policyengine-simulation-observability \ - libs/policyengine-stage12-persistence; do \ + libs/policyengine-stage12-persistence \ + libs/policyengine-fastapi; do \ echo "Testing $$proj..."; \ (cd "$$proj" && uv sync --extra test && uv run pytest tests/ -v) || exit 1; \ done From 757391af8aed4cae854cbe62523b96dcc9fafa9d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:44:21 +0400 Subject: [PATCH 4/6] Document that agents never bypass branch rules Agents must not merge with `gh pr merge --admin` or use GitHub's option to bypass rules, even with admin credentials; a blocked merge is reported to a human instead. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 3 ++- docs/engineering/skills/github-prs.md | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 6efa4b64c..0f75d5d42 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,7 +30,8 @@ names, logging, or observability failure handling, read ## GitHub PRs Never open `policyengine-sim-api` PRs from forks. CI and deployment checks are -designed for same-repository branches. +designed for same-repository branches. Agents must never bypass branch rules +(for example with `gh pr merge --admin`), even with admin credentials. Before creating or sharing any PR, all developers and agents must: diff --git a/docs/engineering/skills/github-prs.md b/docs/engineering/skills/github-prs.md index a7b2e71ff..d1931d801 100644 --- a/docs/engineering/skills/github-prs.md +++ b/docs/engineering/skills/github-prs.md @@ -33,6 +33,13 @@ repository, stop and ask for access. Do not create a fork PR as a fallback. If you accidentally create one, close it immediately and replace it with a same-repository draft PR. +## 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. + ## PR Title Do not add `[codex]`, `[claude]`, `[copilot]`, or other agent labels to PR From 08248189f1b7a7151724db9ef2edad4035b5ab50 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:44:21 +0400 Subject: [PATCH 5/6] Add changelog entries for Phase 0 enforcement changes Co-Authored-By: Claude Opus 5.5 --- changelog_entry.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changelog_entry.yaml b/changelog_entry.yaml index 066e85eb1..04d21c5ce 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -1,5 +1,8 @@ - bump: patch changes: + changed: + - Added CODEOWNERS for the rule machinery, pinned CI's ruff to 0.15.22 with an explicit rule selection, and documented that agents never bypass branch rules + - Ran the Modal image smoke for Stage 12 persistence changes and added policyengine-fastapi to the PR test matrix fixed: - Preserved bounded reform stop dates in UK and US simulations while retaining current-law baseline artifact reuse - Resolved PolicyEngine-managed regional datasets using bundled data versions instead of release-manifest commit hashes when loading from GCS From ab377aea32b0e7a451ade3bb749a871bc828544d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:44:05 +0400 Subject: [PATCH 6/6] 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, and the Modal image smoke, nine times. The root jobs of pr.yml and the image smoke now carry the stack condition, and the jobs that need them are skipped with them. A new CI complete job needs every other pr.yml 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; the executor's 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-image-smoke.yml | 4 +- .github/workflows/pr.yml | 30 ++++ changelog_entry.yaml | 1 + docs/engineering/skills/github-prs.md | 23 +++ .../tests/test_ci_complete_gate.py | 167 ++++++++++++++++++ 6 files changed, 263 insertions(+), 1 deletion(-) create mode 100755 .github/scripts/check_ci_complete.sh create mode 100644 projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py diff --git a/.github/scripts/check_ci_complete.sh b/.github/scripts/check_ci_complete.sh new file mode 100755 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-image-smoke.yml b/.github/workflows/pr-image-smoke.yml index 13ee720d7..d1f91e667 100644 --- a/.github/workflows/pr-image-smoke.yml +++ b/.github/workflows/pr-image-smoke.yml @@ -30,7 +30,9 @@ jobs: image-smoke: name: Image smoke (staging) runs-on: ubuntu-latest - if: github.event.pull_request.head.repo.full_name == github.repository + # Same-repository pull requests only, and in a stack only the lowest + # unmerged one (see docs/engineering/skills/github-prs.md). + if: ${{ github.event.pull_request.head.repo.full_name == github.repository && (!github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref) }} # Warm cache: ~2 min. After a relock the executor smoke pays the # certified dataset download layer (~15-20 min). timeout-minutes: 45 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index bf8491c31..c71f1c4a2 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -9,8 +9,36 @@ concurrency: cancel-in-progress: true 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: [test, lint, docker-build, test-integration] + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - name: Checkout the gate script + uses: actions/checkout@v6 + 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 + test: name: Test + # 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 strategy: matrix: @@ -51,6 +79,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: diff --git a/changelog_entry.yaml b/changelog_entry.yaml index 04d21c5ce..ff1c790ca 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -3,6 +3,7 @@ changed: - Added CODEOWNERS for the rule machinery, pinned CI's ruff to 0.15.22 with an explicit rule selection, and documented that agents never bypass branch rules - Ran the Modal image smoke for Stage 12 persistence changes and added policyengine-fastapi to the PR test matrix + - Ran the full PR workflow and the Modal image smoke only on the lowest unmerged pull request in a stack, behind a new CI complete gate job that fails on the others fixed: - Preserved bounded reform stop dates in UK and US simulations while retaining current-law baseline artifact reuse - Resolved PolicyEngine-managed regional datasets using bundled data versions instead of release-manifest commit hashes when loading from GCS diff --git a/docs/engineering/skills/github-prs.md b/docs/engineering/skills/github-prs.md index d1931d801..2dce64f4c 100644 --- a/docs/engineering/skills/github-prs.md +++ b/docs/engineering/skills/github-prs.md @@ -40,6 +40,29 @@ 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`) 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. + +- **`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 + 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. + `projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py` + checks both. + ## PR Title Do not add `[codex]`, `[claude]`, `[copilot]`, or other agent labels to PR diff --git a/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py b/projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py new file mode 100644 index 000000000..b1f45dfd2 --- /dev/null +++ b/projects/policyengine-simulation-executor/tests/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[3] +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 = "main", + 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="main", + stack_base_ref="main", + ) + + 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="main", + ) + + 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", "docker-build": "cancelled"}, + ) + + assert result.returncode == 1 + assert result.stdout.splitlines() == [ + "::error::Job did not pass: test: failure", + "::error::Job did not pass: docker-build: cancelled", + ] + + +def test_gate_fails_when_a_job_was_skipped_on_full_ci() -> None: + result = _run_gate({"lint": "success", "test": "skipped"}, stack_base_ref="main") + + 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": "main"}, + text=True, + capture_output=True, + check=False, + ) + + assert result.returncode != 0 + assert "JOB_RESULTS is required" in result.stderr