Add Python Tooling to CI and Move the Script Tests to scripts/tests - #704
Conversation
CI carried no ruff, mypy, or coverage step, so the hub failed the zero-warnings bar it asks downstream Python for: 88 ruff findings and 5 mypy errors at the configured contract. This fixes every finding, adds the Python gates to validate-task.yml, moves the unit tests to scripts/tests/, documents the gate-versus-utility naming split, and amends the lint-only profile law to carry Python tooling in CI, coverage included. Closes #700
There was a problem hiding this comment.
Pull request overview
This PR brings the repository’s Python “Scripts profile” in line with its stated tooling contract by wiring ruff + mypy + coverage into CI, and by moving the script unit tests into a dedicated scripts/tests/ directory with the accompanying doc/spec updates across the repo.
Changes:
- Add Python linting/type-checking and coverage-driven self-test execution to
.github/workflows/validate-task.ymlviauvx. - Relocate script tests to
scripts/tests/and update import paths,.gitattributes/.editorconfigpins, and prose/skill documentation references accordingly. - Update spec/governance docs and refactor Python utilities (e.g.,
spec/audit.py) to satisfy lint/type rules; addspec/fidelity_honesty.pyfor fleet divergence reporting.
Reviewed changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | Updates a prose reference to reflect the new scripts/tests/ layout. |
| spec/validate.py | Renames loop variable in error printing for lint cleanliness. |
| spec/project-types.json | Updates coverage/Codecov contract wording for the scripts profile. |
| spec/host-tools.json | Updates host tooling note and Python floor rationale to reflect CI running Python tooling. |
| spec/fidelity_honesty.py | Adds a new fleet “fidelity-honesty” analysis/report generator script. |
| spec/audit.py | Refactors for ruff cleanliness (e.g., check=False, pairwise, tuple startswith). |
| scripts/tests/test_skills_install.py | Moves/updates tests and adjusts imports for new scripts/tests/ location. |
| scripts/tests/test_repo_gate.py | Moves/updates tests and adjusts repo-root calculations/imports. |
| scripts/tests/test_prose_lint.py | Moves/updates tests and adjusts repo-root calculations/imports. |
| scripts/tests/test_pr_review.py | Moves/updates tests and refactors context manager usage; adjusts imports. |
| scripts/tests/test_host_gate.py | Moves/updates tests and fixes sys.path injection for new location. |
| scripts/tests/test_build_dist.py | Moves/updates tests and adjusts imports for new scripts/tests/ location. |
| scripts/tests/test_bootstrap.py | Moves/updates tests and adds typing annotation to satisfy lint/type rules. |
| scripts/skills_install.py | Adds check=False to subprocess calls for explicit behavior and lint cleanliness. |
| scripts/repo_gate.py | Adds check=False to subprocess calls for explicit behavior and lint cleanliness. |
| scripts/README.md | Documents the new naming/layout split and updates test invocation examples. |
| scripts/prose_lint.py | Adds check=False and refactors string construction for lint cleanliness. |
| scripts/pr_review.py | Adds check=False, refactors parsing logic, and tightens boolean checks for lint/type rules. |
| scripts/host_gate.py | Adds check=False and simplifies conditional structure for lint cleanliness. |
| OPERATIONS.md | Updates the local verification runbook to match the new CI Python tooling and test layout. |
| host-setup/windows/README.md | Updates references to the bootstrap self-test path under scripts/tests/. |
| host-setup/README.md | Updates references to the bootstrap self-test path under scripts/tests/. |
| host-setup/agent-safety/test_install.py | Removes an import noqa and adds check=False to subprocess calls; updates behavior. |
| host-setup/agent-safety/install.py | Adds check=False, removes the Python version guard, and updates UTC stamping. |
| host-setup/agent-safety/gh-write-guard.py | Adds check=False and scopes broad exception handling with rationale comments. |
| GOVERNANCE.md | Notes scripts/tests/ as the home for unit tests in the repo layout section. |
| docs/fleet-map.md | Updates closing-test references to point at scripts/tests/.... |
| CODESTYLE.md | Updates shell guidance to reference the scripts tree’s tests/ directory. |
| .github/workflows/validate-task.yml | Adds uvx-driven ruff/mypy and coverage-based self-test execution in CI. |
| .gitattributes | Updates LF pin entries to point at moved tests under scripts/tests/. |
| .editorconfig | Updates Python script LF pin section to include scripts/tests/ via scripts/**.py. |
| .claude-plugin/fleet-skills/skills/skill-lifecycle/SKILL.md | Updates references to moved tests under scripts/tests/. |
| .claude-plugin/fleet-skills/skills/shell-codestyle/SKILL.md | Updates guidance to reference tests living under <scripts-dir>/tests/. |
| .claude-plugin/fleet-skills/skills/python-codestyle/SKILL.md | Updates Scripts-profile rules to require CI Python tooling + coverage and new tests layout. |
| .claude-plugin/fleet-skills/.source-digest | Updates the generated digest after regenerating the plugin tree. |
| .agents/skills/skill-lifecycle/SKILL.md | Updates references to moved tests under scripts/tests/. |
| .agents/skills/shell-codestyle/SKILL.md | Updates guidance to reference tests living under <scripts-dir>/tests/. |
| .agents/skills/python-codestyle/SKILL.md | Updates Scripts-profile rules to require CI Python tooling + coverage and new tests layout. |
Suppressed comments (6)
scripts/tests/test_repo_gate.py:24
- The
sys.path.insert(...)followed by a module import triggers ruff E402 (module level import not at top of file) with the repo’s default ruff rules. Add# noqa: E402to the late import (or refactor to avoid it) soruff check .passes.
scripts/tests/test_prose_lint.py:25 - The
sys.path.insert(...)followed by a module import triggers ruff E402 (module level import not at top of file) with the repo’s default ruff rules. Add# noqa: E402to the late import (or refactor to avoid it) soruff check .passes.
scripts/tests/test_pr_review.py:26 - The
sys.path.insert(...)followed by a module import triggers ruff E402 (module level import not at top of file) with the repo’s default ruff rules. Add# noqa: E402to the late import (or refactor to avoid it) soruff check .passes.
scripts/tests/test_host_gate.py:18 - The
sys.path.insert(...)followed by a module import triggers ruff E402 (module level import not at top of file) with the repo’s default ruff rules. Add# noqa: E402to the late import (or refactor to avoid it) soruff check .passes.
scripts/tests/test_build_dist.py:14 - The
sys.path.insert(...)followed by a module import triggers ruff E402 (module level import not at top of file) with the repo’s default ruff rules. Add# noqa: E402to the late import (or refactor to avoid it) soruff check .passes.
scripts/tests/test_skills_install.py:18 - The
sys.path.insert(...)followed by a module import triggers ruff E402 (module level import not at top of file) with the repo’s default ruff rules. Add# noqa: E402to the late import (or refactor to avoid it) soruff check .passes.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review round 1: the uv-run CI steps now pin python 3.13 to match the pyproject target, and the agent-safety installer refuses a pre-3.11 interpreter by capability probe, since the datetime.UTC cleanup raised its runtime floor and the removed version-tuple guard was the friendly error.
|
Answering the six suppressed low-confidence findings, which share one claim: that Declined as a class: E402 is not enabled at this repo's configured contract (ruff latest defaults plus |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 38 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/validate-task.yml:108
- This comment is now misleading: the step runs tooling via
uvx(coverage, and earlier ruff/mypy), so it’s not accurate to say there’s “no dependency to pin”. If the intent is “repo code is stdlib-only and CI uses uvx so there’s no project install step / manual pins”, clarify that to match the new behavior.
# Each gate here is proven by a case that reintroduces the fault it catches, wherever it lives.
# A test sits with the layer that owns it rather than beside the file it names, so a read-only gate over the spec and the host tooling lives in scripts/tests/ even where its subject is shell, and the agent-safety kit carries its own because it ships as a unit.
# Standard library only, so no install step and no dependency to pin.
# The audit engine self-test is offline, so it runs here rather than only on an owner sweep.
Review round 2: the step runs uvx coverage now, so the standard-library claim narrows to the tests and names the one tool the step fetches.
|
Answering the round-2 suppressed finding on |
Closes #707, the follow-up #704 deliberately deferred. ## What - **`df12931` is the one-time reformat**: `uvx ruff@latest format` over `scripts/`, `spec/`, and `host-setup/` (19 files, ~12k diff lines), format-only except for the two source-scan test assertions the new quoting exposed in `scripts/tests/test_pr_review.py`. The `add_argument` option scan now matches either quote and a wrapped call and holds a floor of five options, since the quoting change had silently emptied it to one match and an empty scan reads exactly like a pass. The `--match` assertion reads the formatted double quotes. - **`52884ab` holds the tree there**: a `uvx ruff@latest format --check .` step in `validate-task.yml` beside the lint step, the same invocation in the OPERATIONS.md runbook, and the `python-codestyle` skill's Scripts profile naming `format --check` in its CI tooling sentence, with the plugin regenerated. ## Verification `format --check` reports 117 files already formatted, `ruff check`, mypy, and the full unittest suite pass, `test_install.py` passes on the clean tree, and `build_dist.py --check`, `repo_gate.py` (eol, eol-coverage, sha-pin), the nine-rule prose gate, actionlint, and markdownlint are all clean. Every `.py` stays LF. After merge the machines need `python3 scripts/skills_install.py` re-run, since `python-codestyle` changed. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Merge origin/develop (#704, #705, #709 landed since this branch forked) to clear the DIRTY merge state. Only .claude-plugin's .source-digest conflicted; resolved by regenerating via scripts/build_dist.py, same as every prior round on this branch. host-setup/linux/README.md still pointed at scripts/test_bootstrap.py, which #704 moved to scripts/tests/test_bootstrap.py on develop without updating this reference. Pre-existing drift, not caused by this branch, but merging develop in surfaces it as a live dead-path finding in prose_lint's CI-gated check, so fixed the reference (link target and both visible mentions) to keep the gate green. Ran the full local verification set post-merge: test_prose_lint.py, test_repo_gate.py, test_pr_review.py, spec/audit.py --selftest, gh-write-guard.py --selftest, repo_gate.py, and the full-tree prose_lint CI invocation. All green.
…Python CI Gates (#718) Thirty-one squashes, `56f4d7d..d54862a`. 115 files, +20436/-5298. **Merge with a merge commit, never a squash, and never with `--delete-branch`.** This pull request's head is `develop` itself. ## What lands **Fleet Skills.** The `.agents/skills/` source tree, the generated `.claude-plugin/` distribution, `scripts/build_dist.py` with its `--check` gate, and `scripts/skills_install.py` with its host stamp (#676). Packaged as skills on top of the scaffold: PR review conduct and Copilot instructions upkeep (#677), comment and doc style (#678), resync-a-repo and fleet-conformance-check (#679), the per-language codestyles (#680), git commit conventions and operational vs release workflow (#681), stand up a repo (#683), and repo-worktree (#717). Coverage gaps closed in three passes (#690, #691, #692) plus the P4 sentence-length opt-in (#697). **Host setup.** The Windows host-setup tooling and its PowerShell gate (#674), the Windows bootstrap loader (#682), Docker install and upgrade on Linux and Windows with a version floor (#701, #705), a `uv` floor in `spec/host-tools.json` (#698), self-healing of a shadowing `uv`, `jq`, or `git-restore-mtime` copy (#689), node's real winget package id (#696), and a README for the Linux host-setup nuances (#710). **Python and CI.** Python tooling in CI with the script tests moved to `scripts/tests` (#704), `ruff format` adopted and gated (#709), and the PSScriptAnalyzer claim conditioned on repos that carry `.ps1` files (#686). **Conduct rules.** Triage-order and scope guardrails in pr-review-conduct (#684), `pr_review.py wait` requesting a review rather than only polling for one (#685), a tech-agnostic signed-commit verification (#708), execution rather than analogy to verify platform-specific code (#715), and a unique worktree for every task (#717). **Docs.** The fleet map and gap register with peer messaging declared (#687), mermaid flow diagrams in the kept-authority docs (#702), and the map pointed at the shipped diagrams and current tooling (#703). ## Issues this promotion closes Each landed on `develop` on its own pull request. The keyword fires only on a merge into `main`, so it sits here rather than on the feature pull requests. Closes #700 Closes #707 Closes #711 Closes #712 Closes #714 Closes #688 #699 stays open on purpose: #717 shipped the layout convention and the skill, and the physical migration of existing checkouts is still tracked there. ## Review record Every squash closed its own Copilot loop on its own pull request before merging to `develop`. This promotion carries no new content of its own, so its review is the merged tree as a whole. ## Consequence worth stating The `GOVERNANCE.md` and `AGENTS.md` sections these squashes changed become the canonical the moment this reaches `main`, and every carrying repository reads as drifted from that point until it resyncs. That is the ordinary consequence of a canonical moving rather than a defect. The Skills installer added here is also how a machine picks the new skills up, so a session that keeps restating a rule already packaged as a skill is the signal to run it.
Closes #700.
What
PLW1510,SIM117,EXE001,BLE001,ISC004, and others) and 5 mypy errors at the configured contract (latest defaults plusextend-select = ["I"]) are at zero.subprocess.runcalls carry an explicitcheck=False(behavior-preserving), nestedwith/ifblocks are merged, shebang files carry the exec bit, and the deliberate blind catches (hook and sweep isolation boundaries) carry scoped# noqa: BLE001with their why comments.validate-task.yml):uvx ruff@latest check .,uvx mypy@latest, and the self-test surface underuvx coverage@latestwith--appendand a report, informational with no threshold.astral-sh/setup-uvis SHA-pinned per the catalog precedent.scripts/tests/, with imports, repo-root constants,.gitattributespins, and the.editorconfigLF section (scripts/**.py, since an EditorConfig*stops at/) updated, and every prose reference swept (OPERATIONS.md,scripts/README.md,docs/fleet-map.md, host-setup READMEs, the skill sources plus regenerated plugin)._lint/_gatesuffix (prose_lint,repo_gate,host_gate), a utility carries none (build_dist,pr_review,skills_install).python-codestyleskill's Scripts profile now requires Python tooling in CI, coverage included, and states thetests/layout and naming split fleet-wide.CODESTYLE.mdShell,GOVERNANCE.mdRepository Layout,spec/project-types.json(python.coverage.codecovnote), andspec/host-tools.json(the python floor's why no longer claims no Python linter runs in CI) follow.Decisions left open, deliberately
ruff formatis not adopted: aformat --checkgate would reformat 19 files (~12k diff lines) in the same PR. Measured and left for a separate decision.scripts/prose_lint.py/scripts/repo_gate.pyrather than moving to ascripts/lint/directory, because those paths are named in carried text (.github/copilot-instructions.mdpoints every fleet repo at them), so a move stales the whole fleet. The issue's naming-separation floor is met instead.Verification
All gates run locally the way CI runs them: ruff, mypy, the unittest suite under coverage (all seven files, plus the audit, write-guard, and agent-safety self-tests),
build_dist.py --check,repo_gate.py, the nine-rule prose gate,spec/validate.py, actionlint, editorconfig-checker, markdownlint, cspell, shellcheck, and PSScriptAnalyzer.test_install.pypasses on a clean tree, and OPERATIONS.md now documents its dirty-checkout STALE verdict as by design.After merge the machines need
python3 scripts/skills_install.pyre-run per the documented cadence, since thepython-codestyle,shell-codestyle, andskill-lifecycleskills changed.🤖 Generated with Claude Code