diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 7b73d8e54..230175d7b 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,26 +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 a new HTTP route or an existing route query-parameter contract change, -read `docs/engineering/skills/api-routes.md`. - -For API v2 migration contract, route-group metadata, generated migration docs, -or migration guard changes, read -`docs/engineering/skills/migration_contracts.md`. - -For tests, read `docs/engineering/skills/testing.md` before adding, moving, or -reviewing test files. - -For SQLAlchemy model or Alembic migration work, read -`docs/engineering/skills/alembic-migrations.md`. - -For API v2 route, service, or database-access module additions or moves, read -`docs/engineering/skills/v2-code-organization.md`. - -For `Makefile` changes, read -`docs/engineering/skills/repository-maintenance.md`. - -For pull requests, read `docs/engineering/skills/github-prs.md` before opening, -replacing, or sharing a PR. +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 aa39f207c..117b1ee5e 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -121,6 +121,8 @@ jobs: python-version: "3.12" - 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/.github/workflows/push.yml b/.github/workflows/push.yml index 115615047..014c61797 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -41,6 +41,8 @@ jobs: uses: actions/setup-python@v5 with: python-version: "3.12" + - 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 9b9bdc82a..f47b70413 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,6 +32,9 @@ names, logging, or observability failure handling, read When modifying the `Makefile`, read `docs/engineering/skills/repository-maintenance.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`. @@ -50,6 +53,12 @@ python scripts/run_quality_guards.py python scripts/export_migration_contracts.py ``` +## Safety Boundaries + +Do not claim a route, database table, compute path, or deployment surface has +migrated unless the relevant contract tests, migration flags, and generated +migration docs identify that state. + ## Rules @@ -121,13 +130,21 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. - `api.observability.id-is-diagnostic-only`: observability_id is used only to query diagnostic records, never for idempotency, cache ownership, authorization, database identity, routing or calculation behavior. (doc-only, not checked: follow it yourself) - `api.observability.registered-stages-only`: Span names come from the stage plans in policyengine_api.observability.stages; route and service code never adds span-name string literals. (doc-only, not checked: follow it yourself) +### CI workflows and scripts + +- `api.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`) +- `api.ci.shell-scripts-pass-shellcheck`: Every shell script under .github/scripts/ passes shellcheck. (blocking: quality guard `ci-scripts`) +- `api.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 +- `api.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`) - `api.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) - `api.process.changelog-fragment`: Every PR adds a changelog.d fragment. (blocking: CI job `check-changelog`) - `api.process.cloud-run-image-builds`: The Cloud Run container image builds. (blocking: CI job `test_cloud_run_container_builds`) - `api.process.docker-image-builds`: The API Docker image builds. (blocking: CI job `test_container_builds`) - `api.process.makefile-phony`: Command-only Makefile targets are declared in .PHONY, and .PHONY stays in sync when targets are renamed or removed. (doc-only, not checked: follow it yourself) +- `api.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) - `api.process.rule-machinery-code-owned`: Changes to the rule machinery (rules/, scripts/rules/, scripts/guards/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. (doc-only, not checked: follow it yourself) - `api.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`) - `api.process.stacked-prs-full-ci-at-bottom`: In a stack of pull requests, only the lowest unmerged one runs the full PR workflow. 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 `tests/unit/test_ci_complete_gate.py`) diff --git a/CLAUDE.md b/CLAUDE.md index a07174039..e31fcf566 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,43 +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 adding an HTTP route or changing an existing route's query-parameter -contract, read `docs/engineering/skills/api-routes.md`. - -Before opening, replacing, or sharing a PR, read -`docs/engineering/skills/github-prs.md`. - -When changing API v2 migration contracts, route-group migration metadata, -generated migration docs, or migration guard scripts, read -`docs/engineering/skills/migration_contracts.md`. - -When adding, moving, or reviewing tests, read -`docs/engineering/skills/testing.md`. - -When changing SQLAlchemy models, Alembic configuration, database schemas, or -migration revisions, read -`docs/engineering/skills/alembic-migrations.md`. - -When adding or moving API v2 route, service, or database-access modules, read -`docs/engineering/skills/v2-code-organization.md`. - -When modifying the `Makefile`, read -`docs/engineering/skills/repository-maintenance.md`. - -## Safety Boundaries - -Do not claim a route, database table, compute path, or deployment surface has -migrated unless the relevant contract tests, migration flags, and generated -migration docs identify that state. +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.d/rules-followups.added.md b/changelog.d/rules-followups.added.md new file mode 100644 index 000000000..e85089203 --- /dev/null +++ b/changelog.d/rules-followups.added.md @@ -0,0 +1 @@ +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. diff --git a/docs/engineering/skills/README.md b/docs/engineering/skills/README.md index 769731dc1..47001dd72 100644 --- a/docs/engineering/skills/README.md +++ b/docs/engineering/skills/README.md @@ -13,6 +13,8 @@ Current skills: routes and existing query-contract changes. - `alembic-migrations.md`: mandatory autogenerated Alembic revision workflow, deployment safeguards, and migration validation. +- `ci-workflows.md`: one line of code per workflow `run:` block, tested + workflow scripts, and shellcheck. - `github-prs.md`: PR workflow and migration PR handoff expectations. - `migration_contracts.md`: API v2 migration route contracts, route-group metadata, generated migration artifacts, and quality guards. diff --git a/docs/engineering/skills/ci-workflows.md b/docs/engineering/skills/ci-workflows.md new file mode 100644 index 000000000..94955090b --- /dev/null +++ b/docs/engineering/skills/ci-workflows.md @@ -0,0 +1,51 @@ +# 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: Install ruff (version pinned in uv.lock) + run: bash .github/scripts/install_locked_ruff.sh +``` + +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: `tests/unit/test_install_locked_ruff_script.py` and +`tests/unit/test_cloud_run_deploy_scripts.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 518c80fe2..458f36e46 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,11 +8,11 @@ Every rule that governs `policyengine-api` is listed here with how it is enforce | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 36 | 0 | 2 | 11 | 49 | +| mechanical | 37 | 3 | 2 | 12 | 54 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 36 of 49 rules. +**Blocking:** 40 of 54 rules. ## Gaps @@ -23,6 +23,7 @@ Every rule that governs `policyengine-api` is listed here with how it is enforce - `api.observability.registered-stages-only`: Span names come from the stage plans in policyengine_api.observability.stages; route and service code never adds span-name string literals. - `api.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. - `api.process.makefile-phony`: Command-only Makefile targets are declared in .PHONY, and .PHONY stays in sync when targets are renamed or removed. +- `api.process.pr-tracking-issue`: Every PR has a tracking issue in this repository, and the first line of its description is Fixes #ISSUE_NUMBER. - `api.process.rule-machinery-code-owned`: Changes to the rule machinery (rules/, scripts/rules/, scripts/guards/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. - `api.routes.shared-query-parameters`: Routes that accept query parameters define them through the shared typed query models, never with a second manual parser. - `api.v2.connectors-never-commit`: v2 connectors receive a session from the service and never open, commit, roll back or close it. @@ -63,7 +64,9 @@ Every rule that governs `policyengine-api` is listed here with how it is enforce ### Ratcheted (existing violations still tolerated) -- None. +- `api.ci.multiline-run-is-script`: 10 remaining in `rules/baselines/ci-multiline-run.txt` +- `api.ci.shell-scripts-pass-shellcheck`: 1 remaining in `rules/baselines/ci-shellcheck.txt` +- `api.ci.workflow-scripts-tested`: 1 remaining in `rules/baselines/ci-untested-scripts.txt` ### Exceptions and review dates @@ -320,21 +323,47 @@ flowchart LR r_api_observability_registered_stages_only -.-> gap_unenforced ``` +## CI workflows and scripts + +| Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | +| --- | --- | --- | --- | --- | --- | --- | --- | +| `api.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 | +| `api.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 | +| `api.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_api_ci_multiline_run_is_script("api.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_api_ci_shell_scripts_pass_shellcheck("api.ci.shell-scripts-pass-shellcheck (ratcheted)") + r_api_ci_workflow_scripts_tested("api.ci.workflow-scripts-tested (ratcheted)") + r_api_ci_multiline_run_is_script --> e_quality_guard_ci_scripts + e_quality_guard_ci_scripts --> j_CI_pr_yml_quality_guards + r_api_ci_shell_scripts_pass_shellcheck --> e_quality_guard_ci_scripts + r_api_ci_workflow_scripts_tested --> e_quality_guard_ci_scripts +``` + ## Process and repository hygiene | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | +| `api.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 | | `api.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 | — | — | | `api.process.changelog-fragment` | Every PR adds a changelog.d fragment. | mechanical | enforced | yes | CI job `check-changelog` | `pr.yml:check-changelog` | — | | `api.process.cloud-run-image-builds` | The Cloud Run container image builds. | mechanical | enforced | yes | CI job `test_cloud_run_container_builds` | `pr.yml:test_cloud_run_container_builds` | — | | `api.process.docker-image-builds` | The API Docker image builds. | mechanical | enforced | yes | CI job `test_container_builds` | `pr.yml:test_container_builds` | — | | `api.process.makefile-phony` | Command-only Makefile targets are declared in .PHONY, and .PHONY stays in sync when targets are renamed or removed. | mechanical | doc-only | no | nothing | — | — | +| `api.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 | — | — | | `api.process.rule-machinery-code-owned` | Changes to the rule machinery (rules/, scripts/rules/, scripts/guards/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. | mechanical | doc-only | no | nothing | — | — | | `api.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** | | `api.process.stacked-prs-full-ci-at-bottom` | In a stack of pull requests, only the lowest unmerged one runs the full PR workflow. 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 `tests/unit/test_ci_complete_gate.py` | `pr.yml:test`, `pr.yml:ci-complete` | yes | ```mermaid flowchart LR + r_api_agents_adapters_point_to_agents_md("api.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_api_process_agents_never_bypass("api.process.agents-never-bypass (doc-only)") gap_unenforced>"not enforced"] r_api_process_changelog_fragment("api.process.changelog-fragment (enforced)") @@ -347,14 +376,15 @@ flowchart LR e_CI_job_test_container_builds["CI job test_container_builds"] j_CI_pr_yml_test_container_builds["CI pr.yml:test_container_builds"] r_api_process_makefile_phony("api.process.makefile-phony (doc-only)") + r_api_process_pr_tracking_issue("api.process.pr-tracking-issue (doc-only)") r_api_process_rule_machinery_code_owned("api.process.rule-machinery-code-owned (doc-only)") r_api_process_rules_registry("api.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_api_process_stacked_prs_full_ci_at_bottom("api.process.stacked-prs-full-ci-at-bottom (enforced)") e_pytest_tests_unit_test_ci_complete_gate_py["pytest tests/unit/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_api_agents_adapters_point_to_agents_md --> e_quality_guard_rules_registry + e_quality_guard_rules_registry --> j_CI_pr_yml_quality_guards r_api_process_agents_never_bypass -.-> gap_unenforced r_api_process_changelog_fragment --> e_CI_job_check_changelog e_CI_job_check_changelog --> j_CI_pr_yml_check_changelog @@ -363,9 +393,9 @@ flowchart LR r_api_process_docker_image_builds --> e_CI_job_test_container_builds e_CI_job_test_container_builds --> j_CI_pr_yml_test_container_builds r_api_process_makefile_phony -.-> gap_unenforced + r_api_process_pr_tracking_issue -.-> gap_unenforced r_api_process_rule_machinery_code_owned -.-> gap_unenforced r_api_process_rules_registry --> e_quality_guard_rules_registry - e_quality_guard_rules_registry --> j_CI_pr_yml_quality_guards r_api_process_stacked_prs_full_ci_at_bottom --> e_pytest_tests_unit_test_ci_complete_gate_py e_pytest_tests_unit_test_ci_complete_gate_py --> j_CI_pr_yml_test e_pytest_tests_unit_test_ci_complete_gate_py --> j_CI_pr_yml_ci_complete diff --git a/rules/baselines/ci-multiline-run.txt b/rules/baselines/ci-multiline-run.txt new file mode 100644 index 000000000..6013bfefa --- /dev/null +++ b/rules/baselines/ci-multiline-run.txt @@ -0,0 +1,11 @@ +# Tolerated existing violations. This list may only shrink. +.github/workflows/alembic-v1-check.yml :: mysql-lifecycle :: Test Alembic configuration and MySQL lifecycle +.github/workflows/pr.yml :: test :: Install system dependencies +.github/workflows/push.yml :: deploy-cloud-run-candidate :: Compute Cloud Run candidate metadata +.github/workflows/push.yml :: deploy-cloud-run-candidate :: Restore previous Cloud Run production revision +.github/workflows/push.yml :: deploy-cloud-run-staging :: Compute Cloud Run staging metadata +.github/workflows/push.yml :: deploy-cloud-run-staging :: Install release test dependencies +.github/workflows/push.yml :: deploy-cloud-run-staging :: Run release tests +.github/workflows/push.yml :: promote-cloud-run-staging :: Restore previous Cloud Run staging revision +.github/workflows/push.yml :: versioning :: Build changelog +.github/workflows/sync-cloud-run-secrets.yml :: sync-cloud-run-secrets :: Require master branch diff --git a/rules/baselines/ci-shellcheck.txt b/rules/baselines/ci-shellcheck.txt new file mode 100644 index 000000000..de7e00a3d --- /dev/null +++ b/rules/baselines/ci-shellcheck.txt @@ -0,0 +1,2 @@ +# Tolerated existing violations. This list may only shrink. +.github/scripts/deploy_observability_collector.sh diff --git a/rules/baselines/ci-untested-scripts.txt b/rules/baselines/ci-untested-scripts.txt new file mode 100644 index 000000000..b2651e812 --- /dev/null +++ b/rules/baselines/ci-untested-scripts.txt @@ -0,0 +1,2 @@ +# Tolerated existing violations. This list may only shrink. +.github/scripts/cleanup_cloud_run_images.sh diff --git a/rules/registry.toml b/rules/registry.toml index f5ed4d07c..1a1994188 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -39,6 +39,10 @@ title = "Code quality and types" id = "observability" title = "Observability" +[[category]] +id = "ci" +title = "CI workflows and scripts" + [[category]] id = "process" title = "Process and repository hygiene" @@ -58,6 +62,18 @@ extra_guard_tests = [ "tests/unit/v2/test_report_v1_runtime_selection.py::test_existing_report_route_and_service_import_only_v1_persistence", ] +[agents] +instructions = "AGENTS.md" +adapters = ["CLAUDE.md", ".github/copilot-instructions.md"] + +[ci_scripts] +workflows = ".github/workflows/*.yml" +scripts_dir = ".github/scripts" +test_globs = ["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" + # --- API v2 layer boundaries ---------------------------------------------- [[rule]] @@ -735,6 +751,89 @@ workflow = ".github/workflows/pr.yml" job = "test_env_vars" invocation = "tests/env_variables" +# --- CI workflows and scripts ---------------------------------------------- + +[[rule]] +id = "api.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 = 10 +[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 = "api.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 = 1 +[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 = "api.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]] @@ -754,6 +853,42 @@ workflow = ".github/workflows/pr.yml" job = "quality-guards" invocation = "run_quality_guards.py" +[[rule]] +id = "api.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 = "api.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#tracking-issues" +scope = [".github/pull_request_template.md"] +tier = "mechanical" +status = "doc-only" +autofix = false +[rule.enforcer] +kind = "none" + [[rule]] id = "api.process.changelog-fragment" category = "process" 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