diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index c10c24527..230175d7b 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,11 +1,8 @@ # Copilot Instructions -Follow the repository's canonical engineering skills under -`docs/engineering/skills/`. +Repository-wide guidance for AI agents lives in `AGENTS.md`. Read it first: it +lists the engineering skill to read for each kind of change, and its generated +Rules section lists every rule that governs this repository. -For tests, read `docs/engineering/skills/testing.md` before adding, moving, or -reviewing test files. Do not duplicate or override that testing guidance here. - -For pull requests, read `docs/engineering/skills/github-prs.md` before opening, -replacing, or sharing a PR. This repository only accepts same-repository PRs -from branches in `PolicyEngine/policyengine-sim-api`; never create fork PRs. +This file is a Copilot adapter. Keep it a pointer: put guidance in `AGENTS.md` +or in a skill under `docs/engineering/skills/`, never here. diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 64de3f752..96cc7e737 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -127,6 +127,9 @@ jobs: - name: Test the rules registry tool run: python -m unittest discover -s scripts/rules/tests -t . + - name: Install shellcheck (for the ci-scripts guard) + run: pip install "shellcheck-py==0.11.0.1" + - name: Run quality guards run: python scripts/run_quality_guards.py diff --git a/AGENTS.md b/AGENTS.md index 52c8bc159..5a457a752 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,6 +18,9 @@ Before changing correlation identifiers, telemetry transport, spans, stage names, logging, or observability failure handling, read `docs/engineering/skills/observability.md`. +When adding or changing a GitHub Actions workflow step or a script under +`.github/scripts/`, read `docs/engineering/skills/ci-workflows.md`. + When adding, changing, or removing a rule, a guard test, a checker configuration, or a CI job that enforces one, read `docs/engineering/skills/rules-registry.md`. @@ -67,6 +70,13 @@ same-repository draft PR. (including the rules registry), Docker build, and local integration tests. - There is currently no repository-wide changelog fragment requirement. +## Safety Boundaries + +Do not commit directly to `main`. + +Do not make production deployment changes or publish clients unless the user +explicitly asks for that operation. + ## Rules @@ -128,12 +138,20 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. - `sim.quality.ruff-lint`: Python code passes ruff check with the rules selected in ruff.toml (ruff's defaults, made explicit); CI does not run ruff check yet. (advisory: ruff check (E4, E7, E9, F), not blocking) - `sim.quality.tests-pass`: Each project's unit tests pass in its own locked environment. (blocking: CI job `test`) +### CI workflows and scripts + +- `sim.ci.multiline-run-is-script`: A GitHub Actions run: block holds one line of code and no chained commands (&&, ||, ;); anything more lives in a tested script under .github/scripts/, called from a single-line step. (blocking: quality guard `ci-scripts`) +- `sim.ci.shell-scripts-pass-shellcheck`: Every shell script under .github/scripts/ passes shellcheck. (blocking: quality guard `ci-scripts`) +- `sim.ci.workflow-scripts-tested`: Every shell script a workflow calls is run by at least one test. (blocking: quality guard `ci-scripts`) + ### Process and repository hygiene +- `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. (blocking: quality guard `rules-registry`) - `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.rules-registry`: Every rule is registered in rules/registry.toml, is enforced where it claims to be, and its rendered views are current. (blocking: quality guard `rules-registry`) -- `sim.process.same-repo-draft-prs`: PRs are opened as drafts from branches in PolicyEngine/policyengine-sim-api, never from forks, and start with 'Fixes #'. (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) - `sim.process.stacked-prs-full-ci-at-bottom`: In a stack of pull requests, only the lowest unmerged one runs the full PR workflow and the Modal image smoke. Every pr.yml job is needed by the CI complete gate, which fails on the pull requests above it and otherwise passes only when every job passed, so a stack merges bottom-up and nothing merges untested. (blocking: pytest `projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py`) diff --git a/CLAUDE.md b/CLAUDE.md index 8a6908e88..e31fcf566 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,27 +1,8 @@ # Claude Instructions -These instructions apply repository-wide. +Repository-wide guidance for AI agents lives in `AGENTS.md`. Read it first: it +lists the engineering skill to read for each kind of change, and its generated +Rules section lists every rule that governs this repository. -## Canonical Guidance - -Repository-wide AI-facing engineering guidance lives in `AGENTS.md`. -Canonical skills live under `docs/engineering/skills/`. - -Use those files as the source of truth. This file is a Claude adapter and should -stay thin; do not duplicate detailed testing, CI, formatting, or architecture -rules here. - -## Required Skill Lookup - -Before opening, replacing, or sharing a PR, read -`docs/engineering/skills/github-prs.md`. - -When adding, moving, or reviewing tests, read -`docs/engineering/skills/testing.md`. - -## Safety Boundaries - -Do not commit directly to `main`. - -Do not make production deployment changes or publish clients unless the user -explicitly asks for that operation. +This file is a Claude adapter. Keep it a pointer: put guidance in `AGENTS.md` or +in a skill under `docs/engineering/skills/`, never here. diff --git a/changelog_entry.yaml b/changelog_entry.yaml index fbc6287fd..979e3506e 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -1,6 +1,7 @@ - bump: patch changes: added: + - Made CLAUDE.md and the Copilot instructions thin pointers to AGENTS.md, required workflow logic longer than one line to live in tested and shellchecked scripts, and registered the tracking-issue rule - Added a rules registry (rules/registry.toml) with a standard-library tool that validates it, checks every rule is enforced where it claims, and renders docs/generated/rules.md and the Rules block in AGENTS.md 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 diff --git a/docs/engineering/skills/README.md b/docs/engineering/skills/README.md index 180b4799e..f3f02b96c 100644 --- a/docs/engineering/skills/README.md +++ b/docs/engineering/skills/README.md @@ -9,6 +9,8 @@ first, then keep adapters thin. Current skills: +- `ci-workflows.md`: one line of code per workflow `run:` block, tested + workflow scripts, and shellcheck. - `github-prs.md`: same-repository draft PR workflow, issue linkage, PR head verification, and title conventions. - `testing.md`: test placement, dependency boundaries, and expected validation diff --git a/docs/engineering/skills/ci-workflows.md b/docs/engineering/skills/ci-workflows.md new file mode 100644 index 000000000..ad00fc324 --- /dev/null +++ b/docs/engineering/skills/ci-workflows.md @@ -0,0 +1,52 @@ +# CI Workflows + +Read this before adding or changing a GitHub Actions workflow step, or a script +under `.github/scripts/`. + +## One line of code per `run:` block + +A workflow `run:` block may hold only one line of code, with no chained +commands. Anything more goes in a shell script under `.github/scripts/`, called +from a single-line step: + +```yaml +- name: Extract package versions + id: versions + run: .github/scripts/modal-extract-versions.sh projects/policyengine-simulation-executor +``` + +Inline shell in YAML can't be linted, unit-tested or run locally; a script can. + +Every non-empty, non-comment line counts, including each line of a folded +(`>`) block and each backslash-continued line. Chaining commands on one line +with `&&`, `||` or `;` also counts as more than one line. Operators inside +quotes or `${{ }}` expressions don't count, and a single pipeline (`a | b`) is +one command. + +## Writing a workflow script + +- Start with `#!/usr/bin/env bash` and `set -euo pipefail`. +- Take inputs from arguments or environment variables, not from `${{ }}` + expressions inside the script. +- It must pass `shellcheck` (the `ci-scripts` quality guard runs it). + +## Testing a workflow script + +Every script a workflow calls must be referenced by name in at least one test. +Test it the way the existing scripts are tested: start it with `subprocess`, +put fake versions of the commands it calls (`gcloud`, `pip`, `curl`, ...) first +on `PATH`, and assert on what they were called with. Cover the success path and +at least one failure path. + +Examples: `projects/policyengine-simulation-executor/tests/test_modal_scripts.py` +and `projects/policyengine-simulation-entry/tests/test_deployment_assets.py`. + +## Enforcement and baselines + +The `ci-scripts` quality guard (`python scripts/run_quality_guards.py`, or +`python -m scripts.rules ci-scripts`) checks all three rules above. Existing +violations are listed in `rules/baselines/ci-*.txt`. Anything new fails, and so +does a baseline entry that no longer applies, so each list only shrinks. When +you convert an existing multi-line block into a script, or add a missing test, +remove its baseline entry and lower the rule's `baseline.count` in +`rules/registry.toml`. diff --git a/docs/generated/rules.md b/docs/generated/rules.md index bb5851ca9..94ce0fd09 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -1,6 +1,6 @@ # Rules -Generated by `python -m scripts.rules render` (scripts/rules v0.1.0, tool sha256 `803fdc2cb511`) from `rules/registry.toml`. Do not edit by hand. +Generated by `python -m scripts.rules render` (scripts/rules v0.1.0, tool sha256 `afc55ba219f1`) from `rules/registry.toml`. Do not edit by hand. Every rule that governs `policyengine-sim-api` is listed here with how it is enforced. Blocking rules fail CI. The Gaps section lists what is written down but not yet enforced or proven. To add or change a rule, see `docs/engineering/skills/rules-registry.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 | 25 | 0 | 1 | 13 | 39 | +| mechanical | 26 | 3 | 1 | 14 | 44 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 25 of 39 rules. +**Blocking:** 29 of 44 rules. ## Gaps @@ -27,8 +27,9 @@ 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, and start with 'Fixes #'. +- `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. - `sim.quality.ruff-format-tests`: Test code is formatted with ruff format too; CI currently checks only src directories. @@ -59,7 +60,9 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf ### Ratcheted (existing violations still tolerated) -- None. +- `sim.ci.multiline-run-is-script`: 20 remaining in `rules/baselines/ci-multiline-run.txt` +- `sim.ci.shell-scripts-pass-shellcheck`: 1 remaining in `rules/baselines/ci-shellcheck.txt` +- `sim.ci.workflow-scripts-tested`: 4 remaining in `rules/baselines/ci-untested-scripts.txt` ### Exceptions and review dates @@ -259,33 +262,60 @@ flowchart LR e_CI_job_test --> j_CI_pr_yml_test ``` +## CI workflows and scripts + +| Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | +| --- | --- | --- | --- | --- | --- | --- | --- | +| `sim.ci.multiline-run-is-script` | A GitHub Actions run: block holds one line of code and no chained commands (&&, \|\|, ;); anything more lives in a tested script under .github/scripts/, called from a single-line step. | mechanical | ratcheted | yes | quality guard `ci-scripts` | `pr.yml:quality-guards` | yes | +| `sim.ci.shell-scripts-pass-shellcheck` | Every shell script under .github/scripts/ passes shellcheck. | mechanical | ratcheted | yes | quality guard `ci-scripts` | `pr.yml:quality-guards` | yes | +| `sim.ci.workflow-scripts-tested` | Every shell script a workflow calls is run by at least one test. | mechanical | ratcheted | yes | quality guard `ci-scripts` | `pr.yml:quality-guards` | yes | + +```mermaid +flowchart LR + r_sim_ci_multiline_run_is_script("sim.ci.multiline-run-is-script (ratcheted)") + e_quality_guard_ci_scripts["quality guard ci-scripts"] + j_CI_pr_yml_quality_guards["CI pr.yml:quality-guards"] + r_sim_ci_shell_scripts_pass_shellcheck("sim.ci.shell-scripts-pass-shellcheck (ratcheted)") + r_sim_ci_workflow_scripts_tested("sim.ci.workflow-scripts-tested (ratcheted)") + r_sim_ci_multiline_run_is_script --> e_quality_guard_ci_scripts + e_quality_guard_ci_scripts --> j_CI_pr_yml_quality_guards + r_sim_ci_shell_scripts_pass_shellcheck --> e_quality_guard_ci_scripts + r_sim_ci_workflow_scripts_tested --> e_quality_guard_ci_scripts +``` + ## Process and repository hygiene | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | +| `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.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, and start with 'Fixes #'. | mechanical | doc-only | no | nothing | — | — | +| `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 | — | — | | `sim.process.stacked-prs-full-ci-at-bottom` | In a stack of pull requests, only the lowest unmerged one runs the full PR workflow and the Modal image smoke. Every pr.yml job is needed by the CI complete gate, which fails on the pull requests above it and otherwise passes only when every job passed, so a stack merges bottom-up and nothing merges untested. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py` | `pr.yml:test`, `pr.yml:ci-complete` | yes | ```mermaid flowchart LR + r_sim_agents_adapters_point_to_agents_md("sim.agents.adapters-point-to-agents-md (enforced)") + e_quality_guard_rules_registry["quality guard rules-registry"] + 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_rule_machinery_code_owned("sim.process.rule-machinery-code-owned (doc-only)") r_sim_process_rules_registry("sim.process.rules-registry (enforced)") - e_quality_guard_rules_registry["quality guard rules-registry"] - j_CI_pr_yml_quality_guards["CI pr.yml:quality-guards"] r_sim_process_same_repo_draft_prs("sim.process.same-repo-draft-prs (doc-only)") r_sim_process_stacked_prs_full_ci_at_bottom("sim.process.stacked-prs-full-ci-at-bottom (enforced)") e_pytest_projects_policyengine_simulation_executor_tests_test_ci_complete_gate_py["pytest projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py"] j_CI_pr_yml_test["CI pr.yml:test"] j_CI_pr_yml_ci_complete["CI pr.yml:ci-complete"] + 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_rule_machinery_code_owned -.-> gap_unenforced r_sim_process_rules_registry --> e_quality_guard_rules_registry - e_quality_guard_rules_registry --> j_CI_pr_yml_quality_guards r_sim_process_same_repo_draft_prs -.-> gap_unenforced r_sim_process_stacked_prs_full_ci_at_bottom --> e_pytest_projects_policyengine_simulation_executor_tests_test_ci_complete_gate_py e_pytest_projects_policyengine_simulation_executor_tests_test_ci_complete_gate_py --> j_CI_pr_yml_test diff --git a/rules/baselines/ci-multiline-run.txt b/rules/baselines/ci-multiline-run.txt new file mode 100644 index 000000000..bb27165c7 --- /dev/null +++ b/rules/baselines/ci-multiline-run.txt @@ -0,0 +1,21 @@ +# Tolerated existing violations. This list may only shrink. +.github/workflows/pr-image-smoke.yml :: image-smoke :: Run image smokes +.github/workflows/pr.yml :: lint :: Run ruff format check +.github/workflows/pr.yml :: test :: Install dependencies +.github/workflows/pr.yml :: test :: Run tests +.github/workflows/pr.yml :: test-integration :: Run integration tests (local services only) +.github/workflows/pr.yml :: test-integration :: Start services +.github/workflows/publish-clients.yml :: publish :: Build simulation API client from the entrypoint contract +.github/workflows/simulation-deploy.reusable.yml :: deploy_entrypoint :: Build and push entrypoint image +.github/workflows/simulation-deploy.reusable.yml :: deploy_entrypoint :: Capture stable service state before deployment +.github/workflows/simulation-deploy.reusable.yml :: deploy_entrypoint :: Resolve exact candidate metadata +.github/workflows/simulation-deploy.reusable.yml :: deploy_executor :: Select versioned executor app +.github/workflows/simulation-deploy.reusable.yml :: deploy_stage12_v2 :: Select the bundle-derived v2 application name +.github/workflows/simulation-deploy.reusable.yml :: deploy_stage12_v2 :: Validate installed bundles, secrets, datasets, and calculations +.github/workflows/simulation-deploy.reusable.yml :: deploy_stage12_v2 :: Validate the packaged PolicyEngine.py bundle +.github/workflows/simulation-deploy.reusable.yml :: deployment_ready :: Check deployment result +.github/workflows/simulation-deploy.reusable.yml :: prepare :: Synchronize the Modal UK private-data credential +.github/workflows/simulation-deploy.reusable.yml :: publish_stage12_v2_manifest :: Publish only the separate v2 manifest +.github/workflows/simulation-deploy.reusable.yml :: stage12_v2_ready :: Check Stage 12 deployment result +.github/workflows/simulation-deploy.reusable.yml :: update_routing :: Publish active routing state +.github/workflows/simulation-deploy.yml :: summary :: Report promotion status diff --git a/rules/baselines/ci-shellcheck.txt b/rules/baselines/ci-shellcheck.txt new file mode 100644 index 000000000..9bc2c636a --- /dev/null +++ b/rules/baselines/ci-shellcheck.txt @@ -0,0 +1,2 @@ +# Tolerated existing violations. This list may only shrink. +.github/scripts/modal-precompute.sh diff --git a/rules/baselines/ci-untested-scripts.txt b/rules/baselines/ci-untested-scripts.txt new file mode 100644 index 000000000..bcc106dcb --- /dev/null +++ b/rules/baselines/ci-untested-scripts.txt @@ -0,0 +1,5 @@ +# Tolerated existing violations. This list may only shrink. +.github/scripts/dependency-update-commit-and-push.sh +.github/scripts/dependency-update-create-pr.sh +.github/scripts/dependency-update-prepare-branch.sh +.github/scripts/update-policyengine-package.sh diff --git a/rules/registry.toml b/rules/registry.toml index c6c339387..603b88d34 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -36,6 +36,10 @@ title = "Observability" id = "quality" title = "Code quality and tests" +[[category]] +id = "ci" +title = "CI workflows and scripts" + [[category]] id = "process" title = "Process and repository hygiene" @@ -50,6 +54,18 @@ guard_test_globs = [ "projects/policyengine-simulation-gateway/tests/test_openapi_golden.py", ] +[agents] +instructions = "AGENTS.md" +adapters = ["CLAUDE.md", ".github/copilot-instructions.md"] + +[ci_scripts] +workflows = ".github/workflows/*.yml" +scripts_dir = ".github/scripts" +test_globs = ["projects/*/tests/**/test_*.py", "libs/*/tests/**/test_*.py"] +multiline_baseline = "rules/baselines/ci-multiline-run.txt" +untested_baseline = "rules/baselines/ci-untested-scripts.txt" +shellcheck_baseline = "rules/baselines/ci-shellcheck.txt" + # --- Service and package boundaries ---------------------------------------- [[rule]] @@ -634,6 +650,89 @@ autofix = false [rule.enforcer] kind = "none" +# --- CI workflows and scripts ---------------------------------------------- + +[[rule]] +id = "sim.ci.multiline-run-is-script" +category = "ci" +statement = "A GitHub Actions run: block holds one line of code and no chained commands (&&, ||, ;); anything more lives in a tested script under .github/scripts/, called from a single-line step." +rationale = "docs/engineering/skills/ci-workflows.md#one-line-of-code-per-run-block" +scope = [".github/workflows/*.yml"] +tier = "mechanical" +status = "ratcheted" +autofix = false +[rule.enforcer] +kind = "quality-guard" +name = "ci-scripts" +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "quality-guards" +invocation = "run_quality_guards.py" +[rule.baseline] +file = "rules/baselines/ci-multiline-run.txt" +count = 20 +[rule.proof] +bad_fixture = "scripts/rules/tests/fixtures/ci_scripts/multiline_bad" +good_fixture = "scripts/rules/tests/fixtures/ci_scripts/multiline_good" +test = "scripts/rules/tests/test_ci_scripts.py::TestCiScriptsProof::test_rule_proof_multiline_run" +workflow = ".github/workflows/pr.yml" +job = "quality-guards" +invocation = "unittest discover -s scripts/rules/tests" + +[[rule]] +id = "sim.ci.workflow-scripts-tested" +category = "ci" +statement = "Every shell script a workflow calls is run by at least one test." +rationale = "docs/engineering/skills/ci-workflows.md#testing-a-workflow-script" +scope = [".github/scripts/*.sh"] +tier = "mechanical" +status = "ratcheted" +autofix = false +[rule.enforcer] +kind = "quality-guard" +name = "ci-scripts" +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "quality-guards" +invocation = "run_quality_guards.py" +[rule.baseline] +file = "rules/baselines/ci-untested-scripts.txt" +count = 4 +[rule.proof] +bad_fixture = "scripts/rules/tests/fixtures/ci_scripts/untested_bad" +good_fixture = "scripts/rules/tests/fixtures/ci_scripts/untested_good" +test = "scripts/rules/tests/test_ci_scripts.py::TestCiScriptsProof::test_rule_proof_untested_script" +workflow = ".github/workflows/pr.yml" +job = "quality-guards" +invocation = "unittest discover -s scripts/rules/tests" + +[[rule]] +id = "sim.ci.shell-scripts-pass-shellcheck" +category = "ci" +statement = "Every shell script under .github/scripts/ passes shellcheck." +rationale = "docs/engineering/skills/ci-workflows.md#writing-a-workflow-script" +scope = [".github/scripts/*.sh"] +tier = "mechanical" +status = "ratcheted" +autofix = false +[rule.enforcer] +kind = "quality-guard" +name = "ci-scripts" +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "quality-guards" +invocation = "run_quality_guards.py" +[rule.baseline] +file = "rules/baselines/ci-shellcheck.txt" +count = 1 +[rule.proof] +bad_fixture = "scripts/rules/tests/fixtures/ci_scripts/shellcheck/bad_script.sh" +good_fixture = "scripts/rules/tests/fixtures/ci_scripts/shellcheck/good_script.sh" +test = "scripts/rules/tests/test_ci_scripts.py::TestCiScriptsProof::test_rule_proof_shellcheck" +workflow = ".github/workflows/pr.yml" +job = "quality-guards" +invocation = "unittest discover -s scripts/rules/tests" + # --- Process and repository hygiene ---------------------------------------- [[rule]] @@ -704,10 +803,46 @@ autofix = false [rule.enforcer] kind = "none" +[[rule]] +id = "sim.agents.adapters-point-to-agents-md" +category = "process" +statement = "CLAUDE.md and .github/copilot-instructions.md only point to AGENTS.md; skills and rules are listed in AGENTS.md alone." +rationale = "AGENTS.md" +scope = ["CLAUDE.md", ".github/copilot-instructions.md"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "quality-guard" +name = "rules-registry" +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "quality-guards" +invocation = "run_quality_guards.py" +[rule.proof] +bad_fixture = "scripts/rules/tests/fixtures/adapters/bad_CLAUDE.md" +good_fixture = "scripts/rules/tests/fixtures/adapters/good_CLAUDE.md" +test = "scripts/rules/tests/test_adapters.py::TestAdaptersProof::test_rule_proof_adapters" +workflow = ".github/workflows/pr.yml" +job = "quality-guards" +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"] +tier = "mechanical" +status = "doc-only" +autofix = false +[rule.enforcer] +kind = "none" + [[rule]] id = "sim.process.same-repo-draft-prs" category = "process" -statement = "PRs are opened as drafts from branches in PolicyEngine/policyengine-sim-api, never from forks, and start with 'Fixes #'." +statement = "PRs are opened as drafts from branches in PolicyEngine/policyengine-sim-api, never from forks." rationale = "docs/engineering/skills/github-prs.md" scope = [".github/pull_request_template.md"] tier = "mechanical" diff --git a/scripts/rules/adapters.py b/scripts/rules/adapters.py new file mode 100644 index 000000000..b22bd112a --- /dev/null +++ b/scripts/rules/adapters.py @@ -0,0 +1,60 @@ +"""Check that per-tool agent instruction files stay thin pointers. + +``AGENTS.md`` is the one source of agent instructions: it lists the skills and +carries the generated Rules block. Tool adapters such as ``CLAUDE.md`` and +``.github/copilot-instructions.md`` only point to it. When an adapter starts +listing skills or rules itself, the copies drift apart, so this rejects it. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +from scripts.rules.model import Agents, Violation +from scripts.rules.render import BEGIN + +_DOC_REFERENCE = re.compile(r"[\w./-]+\.md\b") + + +def check_adapters(agents: Agents, root: Path) -> list[Violation]: + violations = [] + instructions = Path(agents.instructions).name + for adapter in agents.adapters: + path = root / adapter + if not path.is_file(): + continue + text = path.read_text() + + def v(message: str, fix: str) -> None: + violations.append(Violation(None, "adapters", f"{adapter} {message}", fix)) + + if instructions not in text: + v( + f"does not point to {instructions}", + f"say that the guidance lives in {instructions}", + ) + others = sorted( + { + ref + for ref in _DOC_REFERENCE.findall(text) + if Path(ref).name != instructions + } + ) + if others: + v( + f"references other docs ({', '.join(others)})", + f"list skills and rules in {instructions} only; keep {adapter} a pointer", + ) + if BEGIN in text: + v( + "contains the generated rules block", + f"the block belongs in {instructions} only", + ) + lines = sum(1 for line in text.splitlines() if line.strip()) + if lines > agents.max_adapter_lines: + v( + f"has {lines} non-empty lines (limit {agents.max_adapter_lines})", + f"move the guidance into {instructions} or a skill", + ) + return violations diff --git a/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py new file mode 100644 index 000000000..02b5f3f47 --- /dev/null +++ b/scripts/rules/ci_scripts.py @@ -0,0 +1,216 @@ +"""The ci-scripts quality guard: workflow logic lives in tested shell scripts. + +Three checks, each ratcheted against a baseline file listed in ``[ci_scripts]``: + +1. A workflow ``run:`` block holds one line of code and no chained commands. +2. Every shell script a workflow calls is referenced by at least one test. +3. Every shell script in the scripts directory passes ``shellcheck``. + +Anything found that isn't in the baseline fails, and so does a baseline entry +that no longer applies, so baselines only shrink. +""" + +from __future__ import annotations + +import os +import re +import shutil +import subprocess +from dataclasses import dataclass, field +from pathlib import Path + +from scripts.rules.load import load_registry +from scripts.rules.model import CiScripts, Registry, Violation +from scripts.rules.resolve import clear_caches, workflow_jobs + +REPO_ROOT = Path(__file__).resolve().parents[2] +SEPARATOR = " :: " + + +@dataclass +class Findings: + multiline: set[str] = field(default_factory=set) + untested: set[str] = field(default_factory=set) + shellcheck: set[str] | None = None # None when shellcheck could not run + notes: list[str] = field(default_factory=list) + + +def multiline_runs(config: CiScripts, root: Path) -> set[str]: + """``workflow :: job :: step`` for every run: block with more than one line of + code, or with commands chained by ``&&``, ``||`` or ``;``.""" + found = set() + for workflow in sorted(root.glob(config.workflows)): + relative = workflow.relative_to(root).as_posix() + for job in workflow_jobs(workflow)[0].values(): + for block in job.run_blocks: + if not block.single_line: + found.add(SEPARATOR.join((relative, job.id, block.step))) + return found + + +def called_scripts(config: CiScripts, root: Path) -> set[str]: + """Shell scripts in the scripts directory that a workflow calls.""" + pattern = re.compile(re.escape(config.scripts_dir.rstrip("/")) + r"/[\w.-]+\.sh") + called = set() + for workflow in root.glob(config.workflows): + for job in workflow_jobs(workflow)[0].values(): + called.update(pattern.findall(job.run_text)) + return {path for path in called if (root / path).is_file()} + + +def untested_scripts(config: CiScripts, root: Path) -> set[str]: + """Called scripts whose file name appears in no test file.""" + tests = [ + path.read_text(errors="replace") + for pattern in config.test_globs + for path in root.glob(pattern) + if path.is_file() and ".venv" not in path.parts + ] + return { + script + for script in called_scripts(config, root) + if not any(Path(script).name in text for text in tests) + } + + +def shellcheck_failures(config: CiScripts, root: Path) -> set[str] | None: + """Scripts with any shellcheck finding, or None if shellcheck isn't installed.""" + shellcheck = shutil.which("shellcheck") + if shellcheck is None: + return None + scripts = sorted( + path.relative_to(root).as_posix() + for path in (root / config.scripts_dir).glob("*.sh") + ) + if not scripts: + return set() + result = subprocess.run( + [shellcheck, "--format=gcc", *scripts], + cwd=root, + capture_output=True, + text=True, + ) + return {line.split(":", 1)[0] for line in result.stdout.splitlines() if ":" in line} + + +def find(config: CiScripts, root: Path) -> Findings: + clear_caches() + findings = Findings( + multiline=multiline_runs(config, root), + untested=untested_scripts(config, root), + shellcheck=shellcheck_failures(config, root), + ) + if findings.shellcheck is None: + findings.notes.append( + "shellcheck is not installed; the shellcheck check was skipped" + ) + return findings + + +def _baseline(root: Path, path: str) -> set[str]: + file = root / path + if not file.is_file(): + return set() + return { + line.strip() + for line in file.read_text().splitlines() + if line.strip() and not line.lstrip().startswith("#") + } + + +def _rule_for(registry: Registry, baseline: str) -> str | None: + for rule in registry.rules: + if rule.baseline is not None and rule.baseline.file == baseline: + return rule.id + return None + + +def compare(registry: Registry, findings: Findings, root: Path) -> list[Violation]: + config = registry.ci_scripts + assert config is not None + checks = ( + ( + "multi-line run: block", + findings.multiline, + config.multiline_baseline, + "move the commands into a tested script under " + f"{config.scripts_dir} and call it from a single-line run:", + ), + ( + "untested workflow script", + findings.untested, + config.untested_baseline, + "add a test that runs the script (see docs/engineering/skills/ci-workflows.md)", + ), + ( + "shellcheck finding in", + findings.shellcheck, + config.shellcheck_baseline, + "fix what `shellcheck