From 4b04fce667aa09b6da78bdbf5bb2c3ee27c503b5 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 21 Aug 2026 20:40:25 +0200 Subject: [PATCH 1/7] Add Skylos dead-code detection Run a strict, local Skylos production scan from `make lint` and CI. Remove the dead controller helper and unused verifier parameters found by the scan, and document each verified runtime false positive in the reviewed Skylos configuration. Keep the lint contract, developer guidance, agent gates, and spelling policy aligned with the new enforcement behaviour. --- .github/workflows/ci.yml | 2 +- .gitignore | 1 + AGENTS.md | 7 ++- Makefile | 5 ++ cmd_mox/controller.py | 4 -- cmd_mox/verifiers.py | 4 -- docs/developers-guide.md | 55 +++++++++++++----- pyproject.toml | 45 +++++++++++++++ tests/test_skylos_lint_contract.py | 93 ++++++++++++++++++++++++++++++ typos.local.toml | 2 +- typos.toml | 62 +++++++++++++++++++- 11 files changed, 253 insertions(+), 27 deletions(-) create mode 100644 tests/test_skylos_lint_contract.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2b9c2a0..7f71d7e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,7 +61,7 @@ jobs: if: matrix.run-windows-smoke == false run: make check-fmt - - name: Run ruff + - name: Run lint and dead-code detection if: matrix.run-windows-smoke == false run: make lint diff --git a/.gitignore b/.gitignore index f6e4d39..6ff5ac8 100644 --- a/.gitignore +++ b/.gitignore @@ -212,6 +212,7 @@ target/ .claude/ .memdb/ .grepai/ +.skylos/ *.swp *.swo *~ diff --git a/AGENTS.md b/AGENTS.md index 49812ae..6c9c33a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -91,7 +91,12 @@ When implementing changes, adhere to the following testing procedures: - **Testing:** Passes all relevant unit and behavioural tests according to the guidelines above (`make test`). - - **Linting:** Passes lint checks (`make lint`). + - **Linting:** Passes the complete `make lint` pipeline: Ruff, the + PyPy-backed Pylint rules, and the blocking Skylos dead-code scan. + Investigate every Skylos finding and remove genuine dead code. After + verifying a false positive, add a reasoned, reviewed exception to the + appropriate `[tool.skylos.dead_code.entrypoints]` or + `[tool.skylos.whitelist.documented]` table in `pyproject.toml`. - **Formatting:** Adheres to formatting standards (`make check-fmt`, formatting can be applied by running `make fmt`). - **Typechecking:** Passes type checking (`make typecheck`). diff --git a/Makefile b/Makefile index 76e65ee..612e7e4 100644 --- a/Makefile +++ b/Makefile @@ -14,6 +14,10 @@ PYLINT_PYPY_SHIM_REF ?= 726d09f968b4d729ee4b29c71fc732e744854f3b PYLINT_PYPY_SHIM = git+https://github.com/leynos/pylint-pypy-shim.git@$(PYLINT_PYPY_SHIM_REF) PYLINT_BASELINE_DISABLE = no-else-return,unnecessary-ellipsis,too-many-lines,too-many-arguments,too-many-positional-arguments,subprocess-run-check,use-implicit-booleaness-not-comparison-to-string,unnecessary-dunder-call,use-implicit-booleaness-not-comparison PYLINT = $(UV_ENV) $(UV) tool run --python $(PYLINT_PYTHON) --from '$(PYLINT_PYPY_SHIM)' pylint-pypy --disable=$(PYLINT_BASELINE_DISABLE) +SKYLOS_VERSION = 4.33.2 +SKYLOS = $(UV_ENV) $(UV) tool run --from 'skylos==$(SKYLOS_VERSION)' skylos \ + --config-file pyproject.toml +SKYLOS_PRODUCTION_TARGETS ?= cmd_mox WINDOWS_SMOKE_ARGS = tests/test_windows_environment.py \ tests/test_windows_support_bdd.py \ --log-file=windows-ipc.log \ @@ -89,6 +93,7 @@ markdownlint-run: ## Run markdownlint-cli2 with pinned fallback lint: build ## Run linters $(RUFF) check $(PYLINT) $(PYLINT_TARGETS) + $(SKYLOS) $(SKYLOS_PRODUCTION_TARGETS) --category dead_code --gate --format concise --no-upload --no-provenance --no-grep-verify +$(MAKE) spelling typecheck: build ## Run typechecking diff --git a/cmd_mox/controller.py b/cmd_mox/controller.py index 84dd40b..5296085 100644 --- a/cmd_mox/controller.py +++ b/cmd_mox/controller.py @@ -150,10 +150,6 @@ def _registered_commands(self) -> set[str]: """Return all commands registered via doubles.""" return set(self._doubles) - def _expected_commands(self) -> set[str]: - """Return commands that must be called during replay.""" - return {name for name, dbl in self._doubles.items() if dbl.is_expected} - # ------------------------------------------------------------------ # Context manager protocol # ------------------------------------------------------------------ diff --git a/cmd_mox/verifiers.py b/cmd_mox/verifiers.py index 8f7ee56..c0c7840 100644 --- a/cmd_mox/verifiers.py +++ b/cmd_mox/verifiers.py @@ -328,8 +328,6 @@ def _validate_expectations_order( self._check_order_violations( ordered_seq, relevant_invocations, - expected_descriptions, - actual_descriptions, ) self._check_extra_invocations( ordered_seq, @@ -364,8 +362,6 @@ def _check_order_violations( self, ordered_seq: list[Expectation], relevant_invocations: list[Invocation], - expected_descriptions: list[str], - actual_descriptions: list[str], ) -> None: for index, (exp, actual_inv) in enumerate( zip(ordered_seq, relevant_invocations, strict=False) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 264ba40..e2f149b 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -6,17 +6,18 @@ where lint policy is configured. ## Linting -CmdMox uses a two-tier linting pipeline. Run it with: +CmdMox uses a three-stage linting pipeline. Run it with: ```bash make lint ``` The `lint` target first builds the development environment through -`make build`. It then runs the two lint tiers in order: +`make build`. It then runs the checks in order: 1. `ruff check` 2. PyPy-backed Pylint through `pylint-pypy-shim` +3. a blocking Skylos dead-code scan of the production package Ruff is the fast first tier. It enforces import order, pycodestyle and Pyflakes rules, pathlib usage, docstring rules, pytest rules, selected Ruff preview @@ -29,21 +30,29 @@ footguns, and module or function shape limits. The Pylint tier is intentionally focused: `pyproject.toml` disables all Pylint messages by default and then enables only the selected messages that complement Ruff. +Skylos is the final production-liveness check. It is separately provisioned at +an exact release, scans `cmd_mox` without treating test references as live +callers, and fails the local gate and Linux CI when it reports unexplained dead +code. The scan uses only local static analysis: uploads, provenance collection, +and grep verification are disabled. + ## Makefile lint variables The `Makefile` exposes the lint runner through variables so developers and Continuous Integration (CI) jobs can override the runtime without editing project files. -| Variable | Default | Purpose | -| ------------------------- | ---------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------- | -| `RUFF` | `$(UV_ENV) $(UV) run ruff` | Runs the Ruff command inside the `uv` environment. | -| `PYLINT_PYTHON` | `pypy` | Selects the Python interpreter used by `uv tool run` for Pylint. | -| `PYLINT_TARGETS` | `cmd_mox conftest.py examples tests` | Lists the directories and files linted by Pylint. | -| `PYLINT_PYPY_SHIM_REF` | `726d09f968b4d729ee4b29c71fc732e744854f3b` | Pins the shim repository revision. | -| `PYLINT_PYPY_SHIM` | `git+https://github.com/leynos/pylint-pypy-shim.git@$(PYLINT_PYPY_SHIM_REF)` | Identifies the shim package used by `uv tool run`. | -| `PYLINT_BASELINE_DISABLE` | Existing cmd-mox baseline | Temporarily disables legacy Pylint findings while keeping the second tier active. | -| `PYLINT` | `$(UV_ENV) $(UV) tool run --python $(PYLINT_PYTHON) --from '$(PYLINT_PYPY_SHIM)' pylint-pypy --disable=$(PYLINT_BASELINE_DISABLE)` | Builds the full PyPy-backed Pylint command. | +| Variable | Default | Purpose | +| --------------------------- | ---------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------- | +| `RUFF` | `$(UV_ENV) $(UV) run ruff` | Runs the Ruff command inside the `uv` environment. | +| `PYLINT_PYTHON` | `pypy` | Selects the Python interpreter used by `uv tool run` for Pylint. | +| `PYLINT_TARGETS` | `cmd_mox conftest.py examples tests` | Lists the directories and files linted by Pylint. | +| `PYLINT_PYPY_SHIM_REF` | `726d09f968b4d729ee4b29c71fc732e744854f3b` | Pins the shim repository revision. | +| `PYLINT_PYPY_SHIM` | `git+https://github.com/leynos/pylint-pypy-shim.git@$(PYLINT_PYPY_SHIM_REF)` | Identifies the shim package used by `uv tool run`. | +| `PYLINT_BASELINE_DISABLE` | Existing cmd-mox baseline | Temporarily disables legacy Pylint findings while keeping the second tier active. | +| `PYLINT` | `$(UV_ENV) $(UV) tool run --python $(PYLINT_PYTHON) --from '$(PYLINT_PYPY_SHIM)' pylint-pypy --disable=$(PYLINT_BASELINE_DISABLE)` | Builds the full PyPy-backed Pylint command. | +| `SKYLOS_VERSION` | `4.33.2` | Pins the separately provisioned dead-code analyser. | +| `SKYLOS_PRODUCTION_TARGETS` | `cmd_mox` | Limits dead-code liveness analysis to production sources. | _Table 1: Makefile variables for the lint pipeline._ @@ -55,8 +64,20 @@ make lint PYLINT_TARGETS=cmd_mox/ipc PYLINT_PYTHON=pypy ``` Do not bypass `make lint` for normal validation. Running the target keeps the -Ruff and Pylint tiers ordered consistently with CI and preserves the shared -`uv` cache configuration. +Ruff, Pylint, and Skylos checks ordered consistently with CI and preserves the +shared `uv` cache configuration. + +## Skylos dead-code policy + +Treat a Skylos report as genuine dead code until a runtime caller has been +verified. Remove confirmed dead code. For a confirmed false positive that +cannot be represented as an ordinary static reference, add a precise typed +entry-point rule to `[tool.skylos.dead_code.entrypoints]` or a named exception +to `[tool.skylos.whitelist.documented]` in `pyproject.toml`. Every exception +must name its verified runtime caller in its reason. Do not add unexplained +exceptions or use the allow list to avoid a removal. The `--no-grep-verify` +configuration is intentional: test references must not keep production symbols +live in the blocking scan. ## Spelling policy @@ -80,6 +101,7 @@ goals: - use focused Pylint checks for problems that Ruff does not cover as well; and - run Pylint under PyPy through the shared [pylint-pypy-shim](https://github.com/leynos/pylint-pypy-shim) approach. +- detect unused production symbols with a local, blocking Skylos scan. The policy is adapted for CmdMox rather than copied blindly. CmdMox targets Python 3.12 in `pyproject.toml`, while Episodic targets a newer interpreter. @@ -131,6 +153,13 @@ The `Makefile` currently supplies `PYLINT_BASELINE_DISABLE` in addition to the the desired selected Pylint policy, while the `Makefile` carries the temporary project baseline required to keep the new tier actionable. +### Skylos tables + +- `[tool.skylos.gate]` enables strict failure for unexplained dead-code + findings. +- `[tool.skylos.whitelist.documented]` stores only reasoned false positives; + each entry must identify the verified runtime caller. + ## Updating lint policy When changing lint policy: diff --git a/pyproject.toml b/pyproject.toml index 3756f5b..5035083 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -355,6 +355,51 @@ also_copy = ["conftest.py", "features/", "docs/"] [tool.uv] package = true +[tool.skylos.gate] +strict = true + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = [ + "cmd_mox.ipc.server._BaseIPCServer._export_environment", + "cmd_mox.ipc.server.IPCServer._prepare_backend_start", + "cmd_mox.ipc.server.IPCServer._wait_until_ready", + "cmd_mox.ipc.server.IPCServer._stop_backend", + "cmd_mox.ipc.server.IPCServer._post_stop_cleanup", + "cmd_mox.ipc.server.NamedPipeServer._prepare_backend_start", + "cmd_mox.ipc.server.NamedPipeServer._wait_until_ready", + "cmd_mox.ipc.server.NamedPipeServer._stop_backend", +] +reason = "The shared IPC lifecycle invokes transport hooks through dynamically dispatched self calls." + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = [ + "cmd_mox.ipc.server.ParsedRequest.validate", + "cmd_mox.ipc.server._NamedPipeState.stop", + "cmd_mox.ipc.server._NamedPipeState._poke_pipe", +] +reason = "The IPC request and named-pipe lifecycle invoke these methods through runtime transport state." + +[[tool.skylos.dead_code.entrypoints]] +type = "variable" +full_name = [ + "cmd_mox.environment._Win32Function.argtypes", + "cmd_mox.environment._Win32Function.restype", +] +reason = "ctypes function pointers expose these runtime-configured attributes through the typed protocol." + +[[tool.skylos.dead_code.entrypoints]] +type = "parameter" +full_name = ["cmd_mox.ipc.server._ServerLifecycle._stop_backend.server"] +reason = "The abstract lifecycle contract receives the backend instance from its start and stop orchestration." + +[tool.skylos.whitelist] +names = ["bootstrap_shim_path"] + +[tool.skylos.whitelist.documented] +bootstrap_shim_path = "cmd_mox.shim re-imports this helper after bootstrap before main invokes it." + [build-system] requires = ["setuptools>=61.0", "wheel"] diff --git a/tests/test_skylos_lint_contract.py b/tests/test_skylos_lint_contract.py new file mode 100644 index 0000000..db5502c --- /dev/null +++ b/tests/test_skylos_lint_contract.py @@ -0,0 +1,93 @@ +"""Contract tests for the blocking Skylos dead-code lint gate.""" + +import shutil +import subprocess +import tomllib +import typing as typ +from pathlib import Path + +REPOSITORY_ROOT = Path(__file__).resolve().parents[1] + + +def _pyproject() -> dict[str, object]: + """Load the repository's Python project configuration.""" + return tomllib.loads( + (REPOSITORY_ROOT / "pyproject.toml").read_text(encoding="utf-8") + ) + + +def test_skylos_is_a_pinned_external_tool() -> None: + """Keep Skylos out of the project environment and pin its tool release.""" + config = _pyproject() + dependency_groups = typ.cast("dict[str, list[str]]", config["dependency-groups"]) + + dependencies = dependency_groups["dev"] + assert not any(dependency.startswith("skylos") for dependency in dependencies), ( + "Expected Skylos to be separately provisioned from the development " + "dependency group." + ) + makefile = (REPOSITORY_ROOT / "Makefile").read_text(encoding="utf-8") + assert "SKYLOS_VERSION = 4.33.2" in makefile, ( + "Expected the separately provisioned Skylos tool version to be exact." + ) + assert "--from 'skylos==$(SKYLOS_VERSION)' skylos" in makefile, ( + "Expected Skylos to run from its separately provisioned tool environment." + ) + + +def test_skylos_configuration_is_strict_and_reasoned() -> None: + """Require reasons for every configured Skylos exception.""" + config = _pyproject() + tool_config = typ.cast("dict[str, object]", config["tool"]) + skylos = typ.cast("dict[str, object]", tool_config["skylos"]) + whitelist = typ.cast("dict[str, object]", skylos["whitelist"]) + documented = typ.cast("dict[str, str]", whitelist["documented"]) + assert all(reason.strip() for reason in documented.values()), ( + "Expected every documented Skylos whitelist entry to have a reason." + ) + + dead_code = typ.cast("dict[str, object]", skylos["dead_code"]) + entrypoints = typ.cast("list[dict[str, object]]", dead_code["entrypoints"]) + assert entrypoints, "Expected verified runtime entry points to be documented." + assert all( + isinstance(reason := entrypoint.get("reason"), str) and reason.strip() + for entrypoint in entrypoints + ), "Expected every Skylos dead-code entry point to have a reason." + + gate = typ.cast("dict[str, object]", skylos["gate"]) + assert gate["strict"] is True, "Expected the Skylos gate to run in strict mode." + + +def test_make_lint_runs_local_blocking_dead_code_scan() -> None: + """Keep the Skylos invocation deterministic and production-scoped.""" + make_executable = shutil.which("make") + assert make_executable is not None, "Expected make to be available for the test." + + result = subprocess.run( # noqa: S603 - test executes make without a shell + [make_executable, "--no-print-directory", "--dry-run", "lint"], + cwd=REPOSITORY_ROOT, + check=False, + capture_output=True, + text=True, + ) + + assert result.returncode == 0, "Expected make lint dry run to succeed." + skylos_commands = [ + line + for line in result.stdout.splitlines() + if "skylos --config-file pyproject.toml" in line + ] + assert len(skylos_commands) == 1, ( + "Expected make lint to expand exactly one blocking Skylos command." + ) + skylos_command = skylos_commands[0] + assert "cmd_mox --category" in skylos_command, ( + "Expected the blocking Skylos command to use production-only targets." + ) + assert " tests" not in skylos_command, ( + "Expected tests to be excluded from the production Skylos graph." + ) + assert ( + "--category dead_code --gate --format concise --no-upload " + "--no-provenance --no-grep-verify" in skylos_command + ), "Expected the blocking Skylos command to retain its gate flags." diff --git a/typos.local.toml b/typos.local.toml index 04e3fa2..4484933 100644 --- a/typos.local.toml +++ b/typos.local.toml @@ -6,7 +6,7 @@ schema = 1 stems = [] [words] -accepted = ["Flavored"] # Formal GitHub Flavored Markdown product name. +accepted = ["Flavored", "color"] # Formal product name and literal US API spelling. [words.corrections] diff --git a/typos.toml b/typos.toml index 3b904d5..0149236 100644 --- a/typos.toml +++ b/typos.toml @@ -6,6 +6,7 @@ extend-exclude = [ ".git", ".hypothesis", ".pytest_cache", + ".terraform", ".tox", ".typos-oxendict-base.json", ".typos-oxendict-base.toml", @@ -30,10 +31,11 @@ extend-exclude = [ locale = "en-gb" extend-ignore-re = [ "(?s)```.*?```", - "`[^`\\n]+`", + "\\brust-analyzer\\b", ] [default.extend-words] +"ASO" = "ASO" "Flavored" = "Flavored" "absolutisable" = "absolutizable" "absolutisation" = "absolutization" @@ -143,8 +145,6 @@ extend-ignore-re = [ "apologizers" = "apologizers" "apologizes" = "apologizes" "apologizing" = "apologizing" -"artifact" = "artifact" -"artifacts" = "artifacts" "atomisable" = "atomizable" "atomisation" = "atomization" "atomisations" = "atomizations" @@ -307,6 +307,7 @@ extend-ignore-re = [ "colonizers" = "colonizers" "colonizes" = "colonizes" "colonizing" = "colonizing" +"color" = "color" "colourisable" = "colourizable" "colourisation" = "colourization" "colourisations" = "colourizations" @@ -831,6 +832,7 @@ extend-ignore-re = [ "globalizers" = "globalizers" "globalizes" = "globalizes" "globalizing" = "globalizing" +"handwritten" = "handwritten" "harmonisable" = "harmonizable" "harmonisation" = "harmonization" "harmonisations" = "harmonizations" @@ -1011,6 +1013,24 @@ extend-ignore-re = [ "internationalizers" = "internationalizers" "internationalizes" = "internationalizes" "internationalizing" = "internationalizing" +"italicisable" = "italicizable" +"italicisation" = "italicization" +"italicisations" = "italicizations" +"italicise" = "italicize" +"italicised" = "italicized" +"italiciser" = "italicizer" +"italicisers" = "italicizers" +"italicises" = "italicizes" +"italicising" = "italicizing" +"italicizable" = "italicizable" +"italicization" = "italicization" +"italicizations" = "italicizations" +"italicize" = "italicize" +"italicized" = "italicized" +"italicizer" = "italicizer" +"italicizers" = "italicizers" +"italicizes" = "italicizes" +"italicizing" = "italicizing" "itemisable" = "itemizable" "itemisation" = "itemization" "itemisations" = "itemizations" @@ -1680,6 +1700,24 @@ extend-ignore-re = [ "pluralizers" = "pluralizers" "pluralizes" = "pluralizes" "pluralizing" = "pluralizing" +"polymerisable" = "polymerizable" +"polymerisation" = "polymerization" +"polymerisations" = "polymerizations" +"polymerise" = "polymerize" +"polymerised" = "polymerized" +"polymeriser" = "polymerizer" +"polymerisers" = "polymerizers" +"polymerises" = "polymerizes" +"polymerising" = "polymerizing" +"polymerizable" = "polymerizable" +"polymerization" = "polymerization" +"polymerizations" = "polymerizations" +"polymerize" = "polymerize" +"polymerized" = "polymerized" +"polymerizer" = "polymerizer" +"polymerizers" = "polymerizers" +"polymerizes" = "polymerizes" +"polymerizing" = "polymerizing" "popularisable" = "popularizable" "popularisation" = "popularization" "popularisations" = "popularizations" @@ -2436,6 +2474,24 @@ extend-ignore-re = [ "uncategorizers" = "uncategorizers" "uncategorizes" = "uncategorizes" "uncategorizing" = "uncategorizing" +"underutilisable" = "underutilizable" +"underutilisation" = "underutilization" +"underutilisations" = "underutilizations" +"underutilise" = "underutilize" +"underutilised" = "underutilized" +"underutiliser" = "underutilizer" +"underutilisers" = "underutilizers" +"underutilises" = "underutilizes" +"underutilising" = "underutilizing" +"underutilizable" = "underutilizable" +"underutilization" = "underutilization" +"underutilizations" = "underutilizations" +"underutilize" = "underutilize" +"underutilized" = "underutilized" +"underutilizer" = "underutilizer" +"underutilizers" = "underutilizers" +"underutilizes" = "underutilizes" +"underutilizing" = "underutilizing" "uninitialisable" = "uninitializable" "uninitialisation" = "uninitialization" "uninitialisations" = "uninitializations" From f8475f67b17b082dbab34bdf1b0255c16fb92179 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 21 Aug 2026 20:58:34 +0200 Subject: [PATCH 2/7] Harden Skylos lint documentation Protect the exact reviewed Skylos exceptions from accidental removal. Record the production scan's scope, flags, consequences, and maintenance work in ADR 001 and its contents entry. --- docs/adr-001-linting-architecture.md | 44 +++++++++++++++++++++++++--- docs/contents.md | 3 +- tests/test_skylos_lint_contract.py | 33 ++++++++++++++++++++- 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/docs/adr-001-linting-architecture.md b/docs/adr-001-linting-architecture.md index eaed7c7..ab936c1 100644 --- a/docs/adr-001-linting-architecture.md +++ b/docs/adr-001-linting-architecture.md @@ -1,9 +1,10 @@ -# Architectural decision record (ADR) 001: Two-tier linting architecture +# Architectural decision record (ADR) 001: Three-stage linting architecture ## Status -Accepted. CmdMox uses Ruff as the first lint tier and PyPy-backed Pylint as the -second lint tier. +Accepted. CmdMox uses Ruff as the first lint tier, PyPy-backed Pylint as the +second lint tier, and a strict Skylos dead-code scan as the final production +liveness check. ## Date @@ -21,6 +22,12 @@ Pylint also needs to run in a way that matches the Episodic approach: through the `pylint-pypy-shim` repository and under PyPy as a second-tier lint action after Ruff. +Ruff and Pylint do not model cross-module symbol liveness. CmdMox therefore +also needs a deterministic dead-code check that identifies unused production +symbols without letting test-only references keep them alive. Framework-style +dispatch, ctypes metadata, and the shim bootstrap include verified runtime +surfaces that require narrowly reasoned exceptions rather than a broad baseline. + ## Decision drivers - Keep `make lint` as the single developer entrypoint for lint validation. @@ -31,6 +38,9 @@ after Ruff. - Keep existing CmdMox lint debt visible without forcing unrelated refactors into the lint architecture change. - Pin the shim revision so lint execution is reproducible. +- Detect genuine unused production symbols in the standard local and CI gate. +- Keep dead-code exceptions explicit, typed where possible, and reviewable in + `pyproject.toml`. ## Options considered @@ -65,18 +75,39 @@ _Table 1: Linting architecture options._ ## Decision outcome -CmdMox adopts the Ruff plus PyPy-backed Pylint architecture. +CmdMox adopts a three-stage Ruff, PyPy-backed Pylint, and Skylos architecture. The `lint` target runs `ruff check` first and then runs Pylint through `pylint-pypy-shim`. Ruff and Pylint policy are configured in `pyproject.toml`, while the Makefile defines the executable composition and the temporary CmdMox-specific Pylint baseline. +The final stage provisions Skylos 4.33.2 separately from the project +environment. It scans only `cmd_mox` with +`--category dead_code --gate +--format concise --no-upload --no-provenance --no-grep-verify`. +The existing Linux CI `make lint` step therefore enforces the same local, +non-interactive production scan. Strict mode fails the gate for unexplained +findings. + +Verified runtime entry points are recorded with symbol type, fully qualified +name, and reason under `[tool.skylos.dead_code.entrypoints]`. Exceptions that +cannot describe an entry point are stored in both +`[tool.skylos.whitelist].names` and `[tool.skylos.whitelist.documented]`, with +a reason. This preserves a narrow, auditable distinction between real dead code +and static-analysis limits. + ## Consequences - Developers continue to run one command: `make lint`. - Ruff remains the fastest feedback path and blocks before Pylint starts. - Pylint adds second-tier checks without becoming a separate manual workflow. +- Skylos removes confirmed dead production code and blocks new unexplained + findings locally and in Linux CI. +- Test references do not influence the dead-code graph, and the scan does not + upload code, collect provenance, or invoke cloud analysis. +- Runtime false positives require reasoned, version-controlled entry-point or + whitelist configuration; the contract test protects the reviewed symbols. - The project carries an explicit baseline for existing findings. This makes future clean-up incremental rather than hiding the stricter policy. - The managed PyPy runtime may lag the syntax used by CmdMox. The Pylint @@ -90,3 +121,8 @@ CmdMox-specific Pylint baseline. - Revisit unsupported Ruff selectors when the pinned Ruff version changes. - Keep `docs/developers-guide.md` synchronized with Makefile and `pyproject.toml` lint policy changes. +- Review Skylos exceptions whenever the runtime lifecycle, ctypes protocol, or + bootstrap behaviour changes, and remove obsolete entries with the code that + made them unnecessary. +- Update the pinned Skylos release only with a clean production scan and the + complete lint contract test. diff --git a/docs/contents.md b/docs/contents.md index 24276ac..3b5f7f2 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -8,4 +8,5 @@ - [Fake Capabilities Design](./cmd-mox-fake-capabilities-design.md): Durable fixture writes and reusable helpers for stateful command fakes. - [Roadmap](./roadmap.md): Planned features and progression. -- [ADR 001](./adr-001-linting-architecture.md): Two-tier linting architecture. +- [ADR 001](./adr-001-linting-architecture.md): Three-stage linting + architecture. diff --git a/tests/test_skylos_lint_contract.py b/tests/test_skylos_lint_contract.py index db5502c..cf766df 100644 --- a/tests/test_skylos_lint_contract.py +++ b/tests/test_skylos_lint_contract.py @@ -7,6 +7,23 @@ from pathlib import Path REPOSITORY_ROOT = Path(__file__).resolve().parents[1] +EXPECTED_WHITELIST_NAMES = frozenset({"bootstrap_shim_path"}) +EXPECTED_ENTRYPOINT_FULL_NAMES = frozenset({ + "cmd_mox.environment._Win32Function.argtypes", + "cmd_mox.environment._Win32Function.restype", + "cmd_mox.ipc.server._BaseIPCServer._export_environment", + "cmd_mox.ipc.server.IPCServer._post_stop_cleanup", + "cmd_mox.ipc.server.IPCServer._prepare_backend_start", + "cmd_mox.ipc.server.IPCServer._stop_backend", + "cmd_mox.ipc.server.IPCServer._wait_until_ready", + "cmd_mox.ipc.server.NamedPipeServer._prepare_backend_start", + "cmd_mox.ipc.server.NamedPipeServer._stop_backend", + "cmd_mox.ipc.server.NamedPipeServer._wait_until_ready", + "cmd_mox.ipc.server.ParsedRequest.validate", + "cmd_mox.ipc.server._NamedPipeState._poke_pipe", + "cmd_mox.ipc.server._NamedPipeState.stop", + "cmd_mox.ipc.server._ServerLifecycle._stop_backend.server", +}) def _pyproject() -> dict[str, object]: @@ -42,13 +59,27 @@ def test_skylos_configuration_is_strict_and_reasoned() -> None: skylos = typ.cast("dict[str, object]", tool_config["skylos"]) whitelist = typ.cast("dict[str, object]", skylos["whitelist"]) documented = typ.cast("dict[str, str]", whitelist["documented"]) + whitelist_names = frozenset(typ.cast("list[str]", whitelist["names"])) + assert whitelist_names == EXPECTED_WHITELIST_NAMES, ( + "Expected the reviewed Skylos whitelist names to stay enabled." + ) + assert frozenset(documented) == whitelist_names, ( + "Expected every documented Skylos whitelist exception to be enabled." + ) assert all(reason.strip() for reason in documented.values()), ( "Expected every documented Skylos whitelist entry to have a reason." ) dead_code = typ.cast("dict[str, object]", skylos["dead_code"]) entrypoints = typ.cast("list[dict[str, object]]", dead_code["entrypoints"]) - assert entrypoints, "Expected verified runtime entry points to be documented." + entrypoint_full_names = frozenset( + full_name + for entrypoint in entrypoints + for full_name in typ.cast("list[str]", entrypoint["full_name"]) + ) + assert entrypoint_full_names == EXPECTED_ENTRYPOINT_FULL_NAMES, ( + "Expected the reviewed Skylos entry points to stay enabled." + ) assert all( isinstance(reason := entrypoint.get("reason"), str) and reason.strip() for entrypoint in entrypoints From 77bf9a35613c11294daaed22bbe7035cdc4f9e48 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 21 Aug 2026 21:02:33 +0200 Subject: [PATCH 3/7] Scope colour spelling exemption Allow the documented external API literal without permitting US spelling throughout Markdown prose. --- typos.local.toml | 4 ++-- typos.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/typos.local.toml b/typos.local.toml index 4484933..6a42ae5 100644 --- a/typos.local.toml +++ b/typos.local.toml @@ -6,12 +6,12 @@ schema = 1 stems = [] [words] -accepted = ["Flavored", "color"] # Formal product name and literal US API spelling. +accepted = ["Flavored"] # Formal GitHub Flavored Markdown product name. [words.corrections] [patterns] -ignore = [] +ignore = ["`color`"] # Literal external API spelling in the style guide. [files] exclude = [] diff --git a/typos.toml b/typos.toml index 0149236..62f242b 100644 --- a/typos.toml +++ b/typos.toml @@ -32,6 +32,7 @@ locale = "en-gb" extend-ignore-re = [ "(?s)```.*?```", "\\brust-analyzer\\b", + "`color`", ] [default.extend-words] @@ -307,7 +308,6 @@ extend-ignore-re = [ "colonizers" = "colonizers" "colonizes" = "colonizes" "colonizing" = "colonizing" -"color" = "color" "colourisable" = "colourizable" "colourisation" = "colourization" "colourisations" = "colourizations" From 3abd94a64b652caed243c65180035c45b2b28d32 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 21 Aug 2026 21:05:33 +0200 Subject: [PATCH 4/7] Document caller-specific Skylos reasons Require Skylos exception reasons to identify the runtime caller or lifecycle and constrain grouped symbols to shared callers across repository guidance and linting docs. --- AGENTS.md | 4 +++- docs/adr-001-linting-architecture.md | 6 ++++-- docs/developers-guide.md | 9 +++++---- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6c9c33a..ce745e8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,7 +96,9 @@ When implementing changes, adhere to the following testing procedures: Investigate every Skylos finding and remove genuine dead code. After verifying a false positive, add a reasoned, reviewed exception to the appropriate `[tool.skylos.dead_code.entrypoints]` or - `[tool.skylos.whitelist.documented]` table in `pyproject.toml`. + `[tool.skylos.whitelist.documented]` table in `pyproject.toml`. Keep + each reason caller-specific; group symbols only when the same runtime + caller or lifecycle reaches all of them. - **Formatting:** Adheres to formatting standards (`make check-fmt`, formatting can be applied by running `make fmt`). - **Typechecking:** Passes type checking (`make typecheck`). diff --git a/docs/adr-001-linting-architecture.md b/docs/adr-001-linting-architecture.md index ab936c1..b18de34 100644 --- a/docs/adr-001-linting-architecture.md +++ b/docs/adr-001-linting-architecture.md @@ -94,8 +94,10 @@ Verified runtime entry points are recorded with symbol type, fully qualified name, and reason under `[tool.skylos.dead_code.entrypoints]`. Exceptions that cannot describe an entry point are stored in both `[tool.skylos.whitelist].names` and `[tool.skylos.whitelist.documented]`, with -a reason. This preserves a narrow, auditable distinction between real dead code -and static-analysis limits. +a caller-specific reason. Symbols are grouped only when the same runtime caller +or lifecycle reaches all of them; separate records describe different callers. +This preserves a narrow, auditable distinction between real dead code and +static-analysis limits. ## Consequences diff --git a/docs/developers-guide.md b/docs/developers-guide.md index e2f149b..1373457 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -74,10 +74,11 @@ verified. Remove confirmed dead code. For a confirmed false positive that cannot be represented as an ordinary static reference, add a precise typed entry-point rule to `[tool.skylos.dead_code.entrypoints]` or a named exception to `[tool.skylos.whitelist.documented]` in `pyproject.toml`. Every exception -must name its verified runtime caller in its reason. Do not add unexplained -exceptions or use the allow list to avoid a removal. The `--no-grep-verify` -configuration is intentional: test references must not keep production symbols -live in the blocking scan. +must name its verified runtime caller in a caller-specific reason. Group +symbols only when the same caller or lifecycle reaches all of them; otherwise, +use separate entries. Do not add unexplained exceptions or use the allow list +to avoid a removal. The `--no-grep-verify` configuration is intentional: test +references must not keep production symbols live in the blocking scan. ## Spelling policy From 06a6106ff5dbb88ee60d50a1206ed1edc1ed139b Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 21 Aug 2026 21:12:40 +0200 Subject: [PATCH 5/7] Specify Skylos runtime callers Split dead-code exceptions by their verified caller and protect each caller-specific reason with the Skylos lint contract. --- pyproject.toml | 80 ++++++++++++++++++-------- tests/test_skylos_lint_contract.py | 92 ++++++++++++++++++++++++------ 2 files changed, 130 insertions(+), 42 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 5035083..febc914 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -360,39 +360,73 @@ strict = true [[tool.skylos.dead_code.entrypoints]] type = "method" -full_name = [ - "cmd_mox.ipc.server._BaseIPCServer._export_environment", - "cmd_mox.ipc.server.IPCServer._prepare_backend_start", - "cmd_mox.ipc.server.IPCServer._wait_until_ready", - "cmd_mox.ipc.server.IPCServer._stop_backend", - "cmd_mox.ipc.server.IPCServer._post_stop_cleanup", - "cmd_mox.ipc.server.NamedPipeServer._prepare_backend_start", - "cmd_mox.ipc.server.NamedPipeServer._wait_until_ready", - "cmd_mox.ipc.server.NamedPipeServer._stop_backend", -] -reason = "The shared IPC lifecycle invokes transport hooks through dynamically dispatched self calls." +full_name = ["cmd_mox.ipc.server._BaseIPCServer._export_environment"] +reason = "_ServerLifecycle.start invokes this inherited hook before backend creation to export the active IPC environment." [[tool.skylos.dead_code.entrypoints]] type = "method" -full_name = [ - "cmd_mox.ipc.server.ParsedRequest.validate", - "cmd_mox.ipc.server._NamedPipeState.stop", - "cmd_mox.ipc.server._NamedPipeState._poke_pipe", -] -reason = "The IPC request and named-pipe lifecycle invoke these methods through runtime transport state." +full_name = ["cmd_mox.ipc.server.IPCServer._prepare_backend_start"] +reason = "_ServerLifecycle.start invokes this Unix transport hook before creating _InnerServer." + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = ["cmd_mox.ipc.server.IPCServer._wait_until_ready"] +reason = "_ServerLifecycle.start invokes this Unix transport hook after starting the backend thread to wait for the socket." + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = ["cmd_mox.ipc.server.IPCServer._stop_backend"] +reason = "_ServerLifecycle.stop invokes this Unix transport hook to shut down _InnerServer." + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = ["cmd_mox.ipc.server.IPCServer._post_stop_cleanup"] +reason = "_ServerLifecycle.stop invokes this Unix transport hook after joining the backend thread to remove the socket." + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = ["cmd_mox.ipc.server.NamedPipeServer._prepare_backend_start"] +reason = "_ServerLifecycle.start invokes this named-pipe hook; it is a no-op because named pipes leave no socket artefact." + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = ["cmd_mox.ipc.server.NamedPipeServer._wait_until_ready"] +reason = "_ServerLifecycle.start invokes this named-pipe hook to wait for the state readiness event." + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = ["cmd_mox.ipc.server.NamedPipeServer._stop_backend"] +reason = "_ServerLifecycle.stop invokes this named-pipe hook to stop the state and join its clients." + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = ["cmd_mox.ipc.server.ParsedRequest.validate"] +reason = "_request_pipeline invokes this validator before dispatch for Unix and named-pipe request ingress." + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = ["cmd_mox.ipc.server._NamedPipeState.stop"] +reason = "NamedPipeServer._wait_until_ready invokes this on timeout and NamedPipeServer._stop_backend invokes it during shutdown." + +[[tool.skylos.dead_code.entrypoints]] +type = "method" +full_name = ["cmd_mox.ipc.server._NamedPipeState._poke_pipe"] +reason = "_NamedPipeState.stop invokes this helper to wake the named-pipe accept loop." [[tool.skylos.dead_code.entrypoints]] type = "variable" -full_name = [ - "cmd_mox.environment._Win32Function.argtypes", - "cmd_mox.environment._Win32Function.restype", -] -reason = "ctypes function pointers expose these runtime-configured attributes through the typed protocol." +full_name = ["cmd_mox.environment._Win32Function.argtypes"] +reason = "_get_short_path assigns this ctypes argument signature to GetShortPathNameW through the typed protocol." + +[[tool.skylos.dead_code.entrypoints]] +type = "variable" +full_name = ["cmd_mox.environment._Win32Function.restype"] +reason = "_get_short_path assigns this ctypes return type to GetShortPathNameW through the typed protocol." [[tool.skylos.dead_code.entrypoints]] type = "parameter" full_name = ["cmd_mox.ipc.server._ServerLifecycle._stop_backend.server"] -reason = "The abstract lifecycle contract receives the backend instance from its start and stop orchestration." +reason = "_ServerLifecycle.stop passes the stored backend instance through this abstract lifecycle hook." [tool.skylos.whitelist] names = ["bootstrap_shim_path"] diff --git a/tests/test_skylos_lint_contract.py b/tests/test_skylos_lint_contract.py index cf766df..ff57625 100644 --- a/tests/test_skylos_lint_contract.py +++ b/tests/test_skylos_lint_contract.py @@ -8,22 +8,64 @@ REPOSITORY_ROOT = Path(__file__).resolve().parents[1] EXPECTED_WHITELIST_NAMES = frozenset({"bootstrap_shim_path"}) -EXPECTED_ENTRYPOINT_FULL_NAMES = frozenset({ - "cmd_mox.environment._Win32Function.argtypes", - "cmd_mox.environment._Win32Function.restype", - "cmd_mox.ipc.server._BaseIPCServer._export_environment", - "cmd_mox.ipc.server.IPCServer._post_stop_cleanup", - "cmd_mox.ipc.server.IPCServer._prepare_backend_start", - "cmd_mox.ipc.server.IPCServer._stop_backend", - "cmd_mox.ipc.server.IPCServer._wait_until_ready", - "cmd_mox.ipc.server.NamedPipeServer._prepare_backend_start", - "cmd_mox.ipc.server.NamedPipeServer._stop_backend", - "cmd_mox.ipc.server.NamedPipeServer._wait_until_ready", - "cmd_mox.ipc.server.ParsedRequest.validate", - "cmd_mox.ipc.server._NamedPipeState._poke_pipe", - "cmd_mox.ipc.server._NamedPipeState.stop", - "cmd_mox.ipc.server._ServerLifecycle._stop_backend.server", -}) +EXPECTED_ENTRYPOINT_REASONS = { + "cmd_mox.environment._Win32Function.argtypes": ( + "_get_short_path assigns this ctypes argument signature to " + "GetShortPathNameW through the typed protocol." + ), + "cmd_mox.environment._Win32Function.restype": ( + "_get_short_path assigns this ctypes return type to GetShortPathNameW " + "through the typed protocol." + ), + "cmd_mox.ipc.server._BaseIPCServer._export_environment": ( + "_ServerLifecycle.start invokes this inherited hook before backend " + "creation to export the active IPC environment." + ), + "cmd_mox.ipc.server.IPCServer._post_stop_cleanup": ( + "_ServerLifecycle.stop invokes this Unix transport hook after joining " + "the backend thread to remove the socket." + ), + "cmd_mox.ipc.server.IPCServer._prepare_backend_start": ( + "_ServerLifecycle.start invokes this Unix transport hook before " + "creating _InnerServer." + ), + "cmd_mox.ipc.server.IPCServer._stop_backend": ( + "_ServerLifecycle.stop invokes this Unix transport hook to shut down " + "_InnerServer." + ), + "cmd_mox.ipc.server.IPCServer._wait_until_ready": ( + "_ServerLifecycle.start invokes this Unix transport hook after " + "starting the backend thread to wait for the socket." + ), + "cmd_mox.ipc.server.NamedPipeServer._prepare_backend_start": ( + "_ServerLifecycle.start invokes this named-pipe hook; it is a no-op " + "because named pipes leave no socket artefact." + ), + "cmd_mox.ipc.server.NamedPipeServer._stop_backend": ( + "_ServerLifecycle.stop invokes this named-pipe hook to stop the state " + "and join its clients." + ), + "cmd_mox.ipc.server.NamedPipeServer._wait_until_ready": ( + "_ServerLifecycle.start invokes this named-pipe hook to wait for the " + "state readiness event." + ), + "cmd_mox.ipc.server.ParsedRequest.validate": ( + "_request_pipeline invokes this validator before dispatch for Unix and " + "named-pipe request ingress." + ), + "cmd_mox.ipc.server._NamedPipeState._poke_pipe": ( + "_NamedPipeState.stop invokes this helper to wake the named-pipe accept loop." + ), + "cmd_mox.ipc.server._NamedPipeState.stop": ( + "NamedPipeServer._wait_until_ready invokes this on timeout and " + "NamedPipeServer._stop_backend invokes it during shutdown." + ), + "cmd_mox.ipc.server._ServerLifecycle._stop_backend.server": ( + "_ServerLifecycle.stop passes the stored backend instance through this " + "abstract lifecycle hook." + ), +} +EXPECTED_ENTRYPOINT_FULL_NAMES = frozenset(EXPECTED_ENTRYPOINT_REASONS) def _pyproject() -> dict[str, object]: @@ -33,6 +75,12 @@ def _pyproject() -> dict[str, object]: ) +def _skylos_config() -> dict[str, object]: + """Return the Skylos configuration from the project file.""" + tool_config = typ.cast("dict[str, object]", _pyproject()["tool"]) + return typ.cast("dict[str, object]", tool_config["skylos"]) + + def test_skylos_is_a_pinned_external_tool() -> None: """Keep Skylos out of the project environment and pin its tool release.""" config = _pyproject() @@ -54,9 +102,7 @@ def test_skylos_is_a_pinned_external_tool() -> None: def test_skylos_configuration_is_strict_and_reasoned() -> None: """Require reasons for every configured Skylos exception.""" - config = _pyproject() - tool_config = typ.cast("dict[str, object]", config["tool"]) - skylos = typ.cast("dict[str, object]", tool_config["skylos"]) + skylos = _skylos_config() whitelist = typ.cast("dict[str, object]", skylos["whitelist"]) documented = typ.cast("dict[str, str]", whitelist["documented"]) whitelist_names = frozenset(typ.cast("list[str]", whitelist["names"])) @@ -80,6 +126,14 @@ def test_skylos_configuration_is_strict_and_reasoned() -> None: assert entrypoint_full_names == EXPECTED_ENTRYPOINT_FULL_NAMES, ( "Expected the reviewed Skylos entry points to stay enabled." ) + entrypoint_reasons = { + full_name: typ.cast("str", entrypoint["reason"]) + for entrypoint in entrypoints + for full_name in typ.cast("list[str]", entrypoint["full_name"]) + } + assert entrypoint_reasons == EXPECTED_ENTRYPOINT_REASONS, ( + "Expected every Skylos entry point to retain its verified runtime caller." + ) assert all( isinstance(reason := entrypoint.get("reason"), str) and reason.strip() for entrypoint in entrypoints From 20471c8b334798e79077d9089b185090e0f613d8 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 21 Aug 2026 23:17:12 +0200 Subject: [PATCH 6/7] Add Skylos whitelist helper Expose the standalone name-only Skylos whitelist command through `make`. Keep the reviewed caller-specific rationale in the documented configuration table, and protect the command shape and required name with contract tests. --- AGENTS.md | 5 +++ Makefile | 12 +++++-- docs/developers-guide.md | 6 ++++ tests/test_skylos_lint_contract.py | 55 ++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ce745e8..27b0b8d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -99,6 +99,11 @@ When implementing changes, adhere to the following testing procedures: `[tool.skylos.whitelist.documented]` table in `pyproject.toml`. Keep each reason caller-specific; group symbols only when the same runtime caller or lifecycle reaches all of them. + For a verified false positive, `make skylos-allow NAME=` invokes + Skylos's name-only whitelist subcommand. It accepts no reason, so the + same reviewed change must add or retain the matching + `[tool.skylos.whitelist.documented]` entry with its caller-specific + reason in `pyproject.toml`. - **Formatting:** Adheres to formatting standards (`make check-fmt`, formatting can be applied by running `make fmt`). - **Typechecking:** Passes type checking (`make typecheck`). diff --git a/Makefile b/Makefile index 612e7e4..691e011 100644 --- a/Makefile +++ b/Makefile @@ -15,8 +15,9 @@ PYLINT_PYPY_SHIM = git+https://github.com/leynos/pylint-pypy-shim.git@$(PYLINT_P PYLINT_BASELINE_DISABLE = no-else-return,unnecessary-ellipsis,too-many-lines,too-many-arguments,too-many-positional-arguments,subprocess-run-check,use-implicit-booleaness-not-comparison-to-string,unnecessary-dunder-call,use-implicit-booleaness-not-comparison PYLINT = $(UV_ENV) $(UV) tool run --python $(PYLINT_PYTHON) --from '$(PYLINT_PYPY_SHIM)' pylint-pypy --disable=$(PYLINT_BASELINE_DISABLE) SKYLOS_VERSION = 4.33.2 -SKYLOS = $(UV_ENV) $(UV) tool run --from 'skylos==$(SKYLOS_VERSION)' skylos \ - --config-file pyproject.toml +SKYLOS_COMMAND = $(UV_ENV) $(UV) tool run --from 'skylos==$(SKYLOS_VERSION)' skylos +SKYLOS = $(SKYLOS_COMMAND) --config-file pyproject.toml +SKYLOS_WHITELIST = $(SKYLOS_COMMAND) whitelist SKYLOS_PRODUCTION_TARGETS ?= cmd_mox WINDOWS_SMOKE_ARGS = tests/test_windows_environment.py \ tests/test_windows_support_bdd.py \ @@ -25,7 +26,7 @@ WINDOWS_SMOKE_ARGS = tests/test_windows_environment.py \ --log-file-format="%(asctime)s %(levelname)s [%(name)s] %(message)s" .PHONY: help all clean build build-release lint fmt check-fmt -.PHONY: markdownlint markdownlint-run nixie spelling test typecheck +.PHONY: markdownlint markdownlint-run nixie spelling skylos-allow test typecheck .PHONY: $(TOOLS) $(VENV_TOOLS) .DEFAULT_GOAL := all @@ -96,6 +97,11 @@ lint: build ## Run linters $(SKYLOS) $(SKYLOS_PRODUCTION_TARGETS) --category dead_code --gate --format concise --no-upload --no-provenance --no-grep-verify +$(MAKE) spelling +skylos-allow: export SKYLOS_NAME = $(value NAME) +skylos-allow: ## Add one named Skylos whitelist exception + @test -n "$${SKYLOS_NAME}" || { printf "Error: NAME is required for a named whitelist exception\\n" >&2; exit 2; } + $(SKYLOS_WHITELIST) "$${SKYLOS_NAME}" + typecheck: build ## Run typechecking $(TY) --version $(TY) check diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 1373457..5d8b543 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -80,6 +80,12 @@ use separate entries. Do not add unexplained exceptions or use the allow list to avoid a removal. The `--no-grep-verify` configuration is intentional: test references must not keep production symbols live in the blocking scan. +For a verified false positive, use `make skylos-allow NAME=` to invoke +Skylos's name-only whitelist subcommand. The subcommand accepts the symbol name +but no reason. Treat its output as a candidate only: in the same reviewed +change, add or retain the matching `[tool.skylos.whitelist.documented]` entry +in `pyproject.toml` with a caller-specific reason. + ## Spelling policy The lint and Markdown gates run a pinned `typos` release with British English diff --git a/tests/test_skylos_lint_contract.py b/tests/test_skylos_lint_contract.py index ff57625..4c2022b 100644 --- a/tests/test_skylos_lint_contract.py +++ b/tests/test_skylos_lint_contract.py @@ -100,6 +100,61 @@ def test_skylos_is_a_pinned_external_tool() -> None: ) +def test_skylos_allow_target_uses_the_standalone_subcommand() -> None: + """Keep the name-only whitelist command separate from the scan command.""" + make_executable = shutil.which("make") + assert make_executable is not None, "Expected make to be available for the test." + + result = subprocess.run( # noqa: S603 - test executes make without a shell + [ + make_executable, + "--no-print-directory", + "--dry-run", + "NAME=bootstrap_shim_path", + "skylos-allow", + ], + cwd=REPOSITORY_ROOT, + check=False, + capture_output=True, + text=True, + ) + + assert result.returncode == 0, "Expected the Skylos allow-list target to expand." + whitelist_commands = [ + line for line in result.stdout.splitlines() if "skylos whitelist" in line + ] + assert len(whitelist_commands) == 1, ( + "Expected the allow-list target to call the standalone Skylos " + "whitelist subcommand without scan options." + ) + whitelist_command = whitelist_commands[0] + assert whitelist_command.endswith('skylos whitelist "${SKYLOS_NAME}"'), ( + "Expected the standalone command to put the whitelist subcommand " + "before its name." + ) + scan_options = ("--config-file", "--category", "--gate") + assert not any(option in whitelist_command for option in scan_options), ( + "Expected the standalone whitelist command not to include scan options." + ) + + +def test_skylos_allow_target_requires_a_name() -> None: + """Prevent an incomplete allow-list operation from invoking Skylos.""" + make_executable = shutil.which("make") + assert make_executable is not None, "Expected make to be available for the test." + + result = subprocess.run( # noqa: S603 - test executes make without a shell + [make_executable, "--no-print-directory", "skylos-allow"], + cwd=REPOSITORY_ROOT, + check=False, + capture_output=True, + text=True, + ) + + assert result.returncode == 2, "Expected an unnamed allow-list operation to fail." + assert "Error: NAME is required for a named whitelist exception" in result.stderr + + def test_skylos_configuration_is_strict_and_reasoned() -> None: """Require reasons for every configured Skylos exception.""" skylos = _skylos_config() From b4afd5bb088f302d9426ee0498b53f252e7c84f0 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 24 Aug 2026 02:06:42 +0200 Subject: [PATCH 7/7] Harden Skylos lint contracts Run Skylos with Python 3.14 and keep its scan configuration separate from the command-only CLI, so the documented whitelist helper dispatches reliably. Parse Makefile and CI contracts with the pinned Makeutil release, install it in each isolated full-suite job, and document the production-only false-positive policy. --- .github/workflows/ci.yml | 13 + .github/workflows/coverage-main.yml | 12 + AGENTS.md | 25 +- Makefile | 26 +- docs/adr-001-linting-architecture.md | 30 +- docs/contents.md | 4 +- docs/developers-guide.md | 61 +++- scripts/tests/test_typos_rollout.py | 22 ++ tests/test_skylos_lint_contract.py | 420 ++++++++++++++++++++------- 9 files changed, 470 insertions(+), 143 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7f71d7e..277a065 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,6 +22,8 @@ jobs: env: CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }} CODESCENE_CLI_SHA256: ${{ vars.CODESCENE_CLI_SHA256 }} + MAKEUTIL_REVISION: '29fc5a1634ffbaa18a773eed9dff1b2838a45d9c' + MAKEUTIL_TOOLCHAIN: 'nightly-2026-05-28' steps: - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -44,6 +46,17 @@ jobs: for tool in mbake ty ruff; do uv tool install ${tool}; done npm install -g markdownlint-cli2 + - name: Install Makefile parser + if: matrix.run-windows-smoke == false + run: | + rustup toolchain install "${MAKEUTIL_TOOLCHAIN}" --profile minimal + RUSTFLAGS="-Zpolonius=next" cargo +"${MAKEUTIL_TOOLCHAIN}" install \ + --git https://github.com/leynos/makeutil \ + --rev "${MAKEUTIL_REVISION}" \ + --locked \ + --force \ + makeutil + - name: Install make if: matrix.run-windows-smoke shell: pwsh diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index 00b3899..5a81bcd 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -20,6 +20,8 @@ jobs: env: CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN || '' }} CODESCENE_CLI_SHA256: ${{ vars.CODESCENE_CLI_SHA256 || '' }} + MAKEUTIL_REVISION: '29fc5a1634ffbaa18a773eed9dff1b2838a45d9c' + MAKEUTIL_TOOLCHAIN: 'nightly-2026-05-28' steps: - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -32,6 +34,16 @@ jobs: - name: Install uv uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 + - name: Install Makefile parser + run: | + rustup toolchain install "${MAKEUTIL_TOOLCHAIN}" --profile minimal + RUSTFLAGS="-Zpolonius=next" cargo +"${MAKEUTIL_TOOLCHAIN}" install \ + --git https://github.com/leynos/makeutil \ + --rev "${MAKEUTIL_REVISION}" \ + --locked \ + --force \ + makeutil + - name: Generate coverage uses: leynos/shared-actions/.github/actions/generate-coverage@19a7f5d1b8d5c1b2236c39720a5744492b3fc129 with: diff --git a/AGENTS.md b/AGENTS.md index 27b0b8d..8139bc0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -91,19 +91,18 @@ When implementing changes, adhere to the following testing procedures: - **Testing:** Passes all relevant unit and behavioural tests according to the guidelines above (`make test`). - - **Linting:** Passes the complete `make lint` pipeline: Ruff, the - PyPy-backed Pylint rules, and the blocking Skylos dead-code scan. - Investigate every Skylos finding and remove genuine dead code. After - verifying a false positive, add a reasoned, reviewed exception to the - appropriate `[tool.skylos.dead_code.entrypoints]` or - `[tool.skylos.whitelist.documented]` table in `pyproject.toml`. Keep - each reason caller-specific; group symbols only when the same runtime - caller or lifecycle reaches all of them. - For a verified false positive, `make skylos-allow NAME=` invokes - Skylos's name-only whitelist subcommand. It accepts no reason, so the - same reviewed change must add or retain the matching - `[tool.skylos.whitelist.documented]` entry with its caller-specific - reason in `pyproject.toml`. + - **Linting:** Passes the complete `make lint` pipeline, including the + fourth Python lint tier: the blocking Skylos production dead-code scan. + Investigate every Skylos finding and remove genuine dead code. For a + verified false positive, first model an implicit runtime caller with a + precise, typed entry-point rule in + `[tool.skylos.dead_code.entrypoints]`; use `type = "method"` for methods + and include the fully qualified symbol and a caller-specific reason. Only + when an entry-point rule cannot describe the boundary may a documented + allow-list exception be added with + `make skylos-allow SYMBOL=handler REASON="Loaded by plugin registry"`. + The helper requires both variables; retain the same caller-specific + reason in `[tool.skylos.whitelist.documented]` in `pyproject.toml`. - **Formatting:** Adheres to formatting standards (`make check-fmt`, formatting can be applied by running `make fmt`). - **Typechecking:** Passes type checking (`make typecheck`). diff --git a/Makefile b/Makefile index 691e011..26e15d4 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,7 @@ MDFORMAT_ALL ?= mdformat-all UV ?= $(shell command -v uv 2>/dev/null || printf '%s' "$$HOME/.local/bin/uv") TOOLS = $(MDFORMAT_ALL) $(UV) VENV_TOOLS = pytest +MAKEUTIL = makeutil UV_ENV = UV_CACHE_DIR=.uv-cache UV_TOOL_DIR=.uv-tools RUFF = $(UV_ENV) $(UV) run ruff TY = $(UV_ENV) $(UV) run ty @@ -15,10 +16,12 @@ PYLINT_PYPY_SHIM = git+https://github.com/leynos/pylint-pypy-shim.git@$(PYLINT_P PYLINT_BASELINE_DISABLE = no-else-return,unnecessary-ellipsis,too-many-lines,too-many-arguments,too-many-positional-arguments,subprocess-run-check,use-implicit-booleaness-not-comparison-to-string,unnecessary-dunder-call,use-implicit-booleaness-not-comparison PYLINT = $(UV_ENV) $(UV) tool run --python $(PYLINT_PYTHON) --from '$(PYLINT_PYPY_SHIM)' pylint-pypy --disable=$(PYLINT_BASELINE_DISABLE) SKYLOS_VERSION = 4.33.2 -SKYLOS_COMMAND = $(UV_ENV) $(UV) tool run --from 'skylos==$(SKYLOS_VERSION)' skylos -SKYLOS = $(SKYLOS_COMMAND) --config-file pyproject.toml -SKYLOS_WHITELIST = $(SKYLOS_COMMAND) whitelist +# Skylos parses source using its own Python AST. Python 3.14 avoids phantom +# dead-code findings from newer syntax that older tool runtimes cannot parse. +SKYLOS_CLI = $(UV_ENV) $(UV) tool run --python 3.14 --from 'skylos==$(SKYLOS_VERSION)' skylos +SKYLOS = $(SKYLOS_CLI) --config-file pyproject.toml SKYLOS_PRODUCTION_TARGETS ?= cmd_mox +SKYLOS_EXCLUDE_FOLDERS ?= tests WINDOWS_SMOKE_ARGS = tests/test_windows_environment.py \ tests/test_windows_support_bdd.py \ --log-file=windows-ipc.log \ @@ -26,7 +29,7 @@ WINDOWS_SMOKE_ARGS = tests/test_windows_environment.py \ --log-file-format="%(asctime)s %(levelname)s [%(name)s] %(message)s" .PHONY: help all clean build build-release lint fmt check-fmt -.PHONY: markdownlint markdownlint-run nixie spelling skylos-allow test typecheck +.PHONY: makeutil markdownlint markdownlint-run nixie spelling skylos-allow test typecheck .PHONY: $(TOOLS) $(VENV_TOOLS) .DEFAULT_GOAL := all @@ -94,13 +97,15 @@ markdownlint-run: ## Run markdownlint-cli2 with pinned fallback lint: build ## Run linters $(RUFF) check $(PYLINT) $(PYLINT_TARGETS) - $(SKYLOS) $(SKYLOS_PRODUCTION_TARGETS) --category dead_code --gate --format concise --no-upload --no-provenance --no-grep-verify +$(MAKE) spelling + $(SKYLOS) $(SKYLOS_PRODUCTION_TARGETS) --exclude $(SKYLOS_EXCLUDE_FOLDERS) --category dead_code --gate --format concise --no-upload --no-provenance --no-grep-verify -skylos-allow: export SKYLOS_NAME = $(value NAME) +skylos-allow: export SKYLOS_SYMBOL = $(value SYMBOL) +skylos-allow: export SKYLOS_REASON = $(value REASON) skylos-allow: ## Add one named Skylos whitelist exception - @test -n "$${SKYLOS_NAME}" || { printf "Error: NAME is required for a named whitelist exception\\n" >&2; exit 2; } - $(SKYLOS_WHITELIST) "$${SKYLOS_NAME}" + @test -n "$${SKYLOS_SYMBOL}" || { printf "Error: SYMBOL is required for a named whitelist exception\\n" >&2; exit 2; } + @test -n "$${SKYLOS_REASON}" || { printf "Error: REASON is required for a named whitelist exception\\n" >&2; exit 2; } + $(SKYLOS_CLI) whitelist "$${SKYLOS_SYMBOL}" --reason "$${SKYLOS_REASON}" typecheck: build ## Run typechecking $(TY) --version @@ -118,7 +123,10 @@ spelling: ## Enforce en-GB-oxendict spelling in Markdown prose nixie: $(NIXIE) ## Validate Mermaid diagrams $(NIXIE) --no-sandbox -test: build $(UV) $(VENV_TOOLS) ## Run tests +makeutil: ## Verify the Makefile parser used by contract tests + $(call ensure_tool,$@) + +test: build $(UV) $(VENV_TOOLS) makeutil ## Run tests $(UV_ENV) $(UV) run pytest -v -n auto windows-smoke: build $(UV) $(VENV_TOOLS) ## Run Windows smoke workflow and capture IPC logs diff --git a/docs/adr-001-linting-architecture.md b/docs/adr-001-linting-architecture.md index b18de34..6fb9a7a 100644 --- a/docs/adr-001-linting-architecture.md +++ b/docs/adr-001-linting-architecture.md @@ -1,4 +1,4 @@ -# Architectural decision record (ADR) 001: Three-stage linting architecture +# Architectural decision record (ADR) 001: Python linting architecture ## Status @@ -75,7 +75,8 @@ _Table 1: Linting architecture options._ ## Decision outcome -CmdMox adopts a three-stage Ruff, PyPy-backed Pylint, and Skylos architecture. +CmdMox adopts Ruff, PyPy-backed Pylint, and Skylos as its source linting +architecture. The `lint` target runs `ruff check` first and then runs Pylint through `pylint-pypy-shim`. Ruff and Pylint policy are configured in `pyproject.toml`, @@ -128,3 +129,28 @@ static-analysis limits. made them unnecessary. - Update the pinned Skylos release only with a clean production scan and the complete lint contract test. + +## Addendum — 2026-08-24: Skylos fourth Python lint tier + +The original two-tier decision is historical. The current Python lint +architecture records Skylos as the fourth tier in the complete quality gate. +The pipeline now runs Ruff first, PyPy-backed Pylint second, the spelling +policy third, and Skylos fourth. This records the full project lint workflow +without altering the historical Pylint decision. +Skylos is blocking: it runs with the pinned Python 3.14 command-only CLI, +scans production modules while excluding test paths, and uses strict gate mode +for unexplained dead-code findings. Scan-only global options such as +`--config-file pyproject.toml` remain separate from that CLI macro so the +command-first `whitelist` helper can dispatch safely. + +Investigate every finding and remove genuine dead code. Model implicit runtime +callers with typed entry-point rules first; use the documented allow list only +when an entry-point rule cannot describe the verified boundary. The helper is: + +```bash +make skylos-allow SYMBOL=handler REASON="Loaded by plugin registry" +``` + +The `SYMBOL` name avoids WSL's `NAME` collision, and both variables are +required. Keep the caller-specific reason in the reviewed +`[tool.skylos.whitelist.documented]` configuration. diff --git a/docs/contents.md b/docs/contents.md index 3b5f7f2..d4e9e15 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -8,5 +8,5 @@ - [Fake Capabilities Design](./cmd-mox-fake-capabilities-design.md): Durable fixture writes and reusable helpers for stateful command fakes. - [Roadmap](./roadmap.md): Planned features and progression. -- [ADR 001](./adr-001-linting-architecture.md): Three-stage linting - architecture. +- [ADR 001](./adr-001-linting-architecture.md): Python linting architecture, + including the dated addendum for the fourth Skylos tier. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 5d8b543..ea00a5f 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -6,7 +6,7 @@ where lint policy is configured. ## Linting -CmdMox uses a three-stage linting pipeline. Run it with: +CmdMox uses a four-tier linting pipeline for its Python project. Run it with: ```bash make lint @@ -17,7 +17,8 @@ The `lint` target first builds the development environment through 1. `ruff check` 2. PyPy-backed Pylint through `pylint-pypy-shim` -3. a blocking Skylos dead-code scan of the production package +3. the en-GB-oxendict spelling policy +4. the blocking Skylos dead-code scan of the production package Ruff is the fast first tier. It enforces import order, pycodestyle and Pyflakes rules, pathlib usage, docstring rules, pytest rules, selected Ruff preview @@ -30,11 +31,11 @@ footguns, and module or function shape limits. The Pylint tier is intentionally focused: `pyproject.toml` disables all Pylint messages by default and then enables only the selected messages that complement Ruff. -Skylos is the final production-liveness check. It is separately provisioned at -an exact release, scans `cmd_mox` without treating test references as live -callers, and fails the local gate and Linux CI when it reports unexplained dead -code. The scan uses only local static analysis: uploads, provenance collection, -and grep verification are disabled. +Skylos is the fourth lint tier and the production-liveness check. It is +separately provisioned at an exact release, scans `cmd_mox` while excluding +tests from the liveness graph, and fails the local gate and Linux CI when it +reports unexplained dead code. The scan uses only local static analysis: +uploads, provenance collection, and grep verification are disabled. ## Makefile lint variables @@ -52,7 +53,10 @@ project files. | `PYLINT_BASELINE_DISABLE` | Existing cmd-mox baseline | Temporarily disables legacy Pylint findings while keeping the second tier active. | | `PYLINT` | `$(UV_ENV) $(UV) tool run --python $(PYLINT_PYTHON) --from '$(PYLINT_PYPY_SHIM)' pylint-pypy --disable=$(PYLINT_BASELINE_DISABLE)` | Builds the full PyPy-backed Pylint command. | | `SKYLOS_VERSION` | `4.33.2` | Pins the separately provisioned dead-code analyser. | +| `SKYLOS_CLI` | `$(UV_ENV) $(UV) tool run --python 3.14 --from 'skylos==$(SKYLOS_VERSION)' skylos` | Command-only CLI; Python 3.14 supplies Skylos's source AST runtime. | +| `SKYLOS` | `$(SKYLOS_CLI) --config-file pyproject.toml` | Adds scan-only global options for the blocking lint target. | | `SKYLOS_PRODUCTION_TARGETS` | `cmd_mox` | Limits dead-code liveness analysis to production sources. | +| `SKYLOS_EXCLUDE_FOLDERS` | `tests` | Prevents test-only references from keeping production symbols live. | _Table 1: Makefile variables for the lint pipeline._ @@ -80,11 +84,44 @@ use separate entries. Do not add unexplained exceptions or use the allow list to avoid a removal. The `--no-grep-verify` configuration is intentional: test references must not keep production symbols live in the blocking scan. -For a verified false positive, use `make skylos-allow NAME=` to invoke -Skylos's name-only whitelist subcommand. The subcommand accepts the symbol name -but no reason. Treat its output as a candidate only: in the same reviewed -change, add or retain the matching `[tool.skylos.whitelist.documented]` entry -in `pyproject.toml` with a caller-specific reason. +For a verified false positive that cannot be modelled with an entry-point rule, +use the command-first helper: + +```bash +make skylos-allow SYMBOL=handler REASON="Loaded by plugin registry" +``` + +The target requires both variables and invokes `skylos whitelist +--reason `. `SYMBOL` avoids WSL's caller-owned `NAME` environment +variable. Treat the helper as a reviewed write: retain the matching +`[tool.skylos.whitelist.documented]` entry in `pyproject.toml`, with a +caller-specific reason, and never use it to avoid removing genuine dead code. + +Skylos parses source with the AST implementation of its own runtime. The +command-only `SKYLOS_CLI` therefore pins Python 3.14 to prevent newer +syntax from producing phantom findings. Scan-only global options such as +`--config-file pyproject.toml` belong in `SKYLOS`, not in the command-only +macro, so the `whitelist` subcommand remains first for helper dispatch. + +The blocking scan targets production modules only, excludes test paths from +the liveness graph, and enables strict gate mode. Investigate every finding; +remove genuine dead code and record only verified false positives. + +The contract test parses the Makefile with the pinned Makeutil executable, and +`make test` verifies that the parser is available before running the suite. CI +installs the same revision independently in each isolated full-suite job. + +For local test runs, install the same parser and toolchain before running +`make test`: + +```bash +rustup toolchain install nightly-2026-05-28 --profile minimal +RUSTFLAGS="-Zpolonius=next" cargo +nightly-2026-05-28 install \ + --git https://github.com/leynos/makeutil \ + --rev 29fc5a1634ffbaa18a773eed9dff1b2838a45d9c \ + --locked --force makeutil +make test +``` ## Spelling policy diff --git a/scripts/tests/test_typos_rollout.py b/scripts/tests/test_typos_rollout.py index 7f4ab1f..a8950b0 100644 --- a/scripts/tests/test_typos_rollout.py +++ b/scripts/tests/test_typos_rollout.py @@ -145,6 +145,28 @@ def test_merge_rejects_conflicting_corrections( rollout.merge_dictionaries(base, local) +def test_committed_local_policy_preserves_api_literal_exemption( + rollout_modules: tuple[types.ModuleType, types.ModuleType, types.ModuleType], + tmp_path: Path, +) -> None: + """The generated configuration retains the reviewed API literal exemption.""" + _, _, generator = rollout_modules + (tmp_path / ".typos-oxendict-base.toml").write_text( + _dictionary_text(), encoding="utf-8" + ) + (tmp_path / "typos.local.toml").write_text( + (SCRIPT_DIRECTORY.parent / "typos.local.toml").read_text(encoding="utf-8"), + encoding="utf-8", + ) + + config = tomllib.loads(generator.render_config(tmp_path)) + + assert "`color`" in config["default"]["extend-ignore-re"], ( + "The committed literal external API spelling exemption must survive " + "generated configuration." + ) + + def test_render_and_write_are_deterministic_valid_toml( rollout_modules: tuple[types.ModuleType, types.ModuleType, types.ModuleType], tmp_path: Path, diff --git a/tests/test_skylos_lint_contract.py b/tests/test_skylos_lint_contract.py index 4c2022b..d8771b6 100644 --- a/tests/test_skylos_lint_contract.py +++ b/tests/test_skylos_lint_contract.py @@ -1,12 +1,47 @@ -"""Contract tests for the blocking Skylos dead-code lint gate.""" +"""Contract tests for Skylos dead-code detection in Make and CI. -import shutil +Skylos scan options must follow its command-only CLI, while the standalone +``whitelist`` subcommand must appear immediately after ``skylos``. Skylos uses +its own Python AST, so the CLI must pin Python 3.14 to understand the project's +syntax. Makeutil provides structured Makefile assertions without depending on +whitespace or nearby source text. +""" + +from __future__ import annotations + +import json +import os +import shlex import subprocess import tomllib import typing as typ from pathlib import Path +import yaml + REPOSITORY_ROOT = Path(__file__).resolve().parents[1] +_MAKEUTIL_COMMAND: typ.Final = ("makeutil", "parse", "Makefile") +_MAKEUTIL_REVISION: typ.Final = "29fc5a1634ffbaa18a773eed9dff1b2838a45d9c" +_MAKEUTIL_TOOLCHAIN: typ.Final = "nightly-2026-05-28" +_MAKEUTIL_INSTALL_TOKENS: typ.Final = ( + "rustup", + "toolchain", + "install", + "${MAKEUTIL_TOOLCHAIN}", + "--profile", + "minimal", + "RUSTFLAGS=-Zpolonius=next", + "cargo", + "+${MAKEUTIL_TOOLCHAIN}", + "install", + "--git", + "https://github.com/leynos/makeutil", + "--rev", + "${MAKEUTIL_REVISION}", + "--locked", + "--force", + "makeutil", +) EXPECTED_WHITELIST_NAMES = frozenset({"bootstrap_shim_path"}) EXPECTED_ENTRYPOINT_REASONS = { "cmd_mox.environment._Win32Function.argtypes": ( @@ -68,6 +103,140 @@ EXPECTED_ENTRYPOINT_FULL_NAMES = frozenset(EXPECTED_ENTRYPOINT_REASONS) +def _mapping(value: object, *, subject: str) -> dict[str, object]: + """Return a JSON object, naming the unexpected ``subject`` on failure.""" + assert isinstance(value, dict), f"Expected {subject} to be a JSON object." + return typ.cast("dict[str, object]", value) + + +def _objects(value: object, *, subject: str) -> list[dict[str, object]]: + """Return a JSON object array, naming the unexpected ``subject`` on failure.""" + assert isinstance(value, list), f"Expected {subject} to be a JSON array." + return [_mapping(item, subject=f"{subject} item") for item in value] + + +def _text_sequence(value: object, *, subject: str) -> tuple[str, ...]: + """Return a JSON string array, naming the unexpected ``subject`` on failure.""" + assert isinstance(value, list), f"Expected {subject} to be a JSON array." + assert all(isinstance(item, str) for item in value), ( + f"Expected {subject} to contain only JSON strings." + ) + return tuple(typ.cast("list[str]", value)) + + +def _makefile_report() -> dict[str, object]: + """Return Makeutil's complete, successfully parsed Makefile report.""" + completed = subprocess.run( # noqa: S603 - fixed parser command. + _MAKEUTIL_COMMAND, + capture_output=True, + check=True, + cwd=REPOSITORY_ROOT, + text=True, + ) + report = typ.cast("dict[str, object]", json.loads(completed.stdout)) + parse = _mapping(report.get("parse"), subject="Makeutil parse report") + assert parse.get("status") == "complete", ( + f"Makeutil must complete the Makefile parse, received {parse!r}." + ) + return report + + +def _sole_variable(name: str) -> dict[str, object]: + """Return Makeutil's sole variable fact for ``name``.""" + variables = _objects(_makefile_report().get("variables"), subject="variables") + matches = [variable for variable in variables if variable.get("name") == name] + assert len(matches) == 1, ( + f"Expected one Makefile variable named {name!r}, found {len(matches)}." + ) + return matches[0] + + +def _sole_recipe_rule(target: str) -> dict[str, object]: + """Return the only parsed rule for ``target`` that has recipes.""" + rules = _objects(_makefile_report().get("rules"), subject="rules") + matches = [ + rule + for rule in rules + if target in _text_sequence(rule.get("targets"), subject="rule targets") + and _objects(rule.get("recipes"), subject="rule recipes") + ] + assert len(matches) == 1, ( + f"Expected one recipe-bearing Makefile rule named {target!r}, found " + f"{len(matches)}." + ) + return matches[0] + + +def _variable_tokens(name: str) -> tuple[str, ...]: + """Return shell-like tokens from Makeutil's raw variable value.""" + value = _sole_variable(name).get("raw_value") + assert isinstance(value, str), f"Expected {name!r} to have a string value." + return tuple(shlex.split(value)) + + +def _recipe_tokens(target: str) -> tuple[tuple[str, ...], ...]: + """Return shell-like tokens for every recipe in ``target``.""" + recipes = _objects( + _sole_recipe_rule(target).get("recipes"), subject=f"{target} recipes" + ) + return tuple( + tuple(shlex.split(recipe_text)) + for recipe in recipes + if isinstance(recipe_text := recipe.get("text"), str) + ) + + +def _workflow_job(workflow_path: str, job_name: str) -> dict[str, object]: + """Return the named job from a repository workflow.""" + workflow = yaml.safe_load((REPOSITORY_ROOT / workflow_path).read_text()) + workflow_mapping = _mapping(workflow, subject=f"{workflow_path} workflow") + jobs = _mapping(workflow_mapping.get("jobs"), subject=f"{workflow_path} jobs") + return _mapping(jobs.get(job_name), subject=f"{workflow_path} job {job_name!r}") + + +def _sole_workflow_step( + workflow_path: str, job_name: str, step_name: str +) -> dict[str, object]: + """Return the sole named CI step from ``job_name``.""" + job = _workflow_job(workflow_path, job_name) + steps = _objects( + job.get("steps"), subject=f"{workflow_path} job {job_name!r} steps" + ) + matches = [step for step in steps if step.get("name") == step_name] + assert len(matches) == 1, ( + f"Expected one {step_name!r} step in {workflow_path} job {job_name!r}, " + f"found {len(matches)}." + ) + return matches[0] + + +def _run_skylos_allow(*arguments: str) -> subprocess.CompletedProcess[str]: + """Run the incomplete whitelist boundary without invoking Skylos.""" + environment: dict[str, str] = dict(os.environ) + environment["NAME"] = "" + environment.pop("REASON", None) + environment.pop("SYMBOL", None) + command = ["make", "skylos-allow", *arguments] + return subprocess.run( # noqa: S603 - fixed local Make boundary command. + command, + capture_output=True, + check=False, + cwd=REPOSITORY_ROOT, + env=environment, + text=True, + ) + + +def _assert_makeutil_installation(command: object, *, contract: str) -> None: + """Assert that ``command`` installs the pinned Makeutil parser.""" + assert isinstance(command, str), ( + f"{contract} must provide a Makeutil installation shell command." + ) + assert ( + tuple(shlex.split(command.replace("\\\n", ""))) == _MAKEUTIL_INSTALL_TOKENS + ), f"{contract} must pin the Makeutil installation command." + + def _pyproject() -> dict[str, object]: """Load the repository's Python project configuration.""" return tomllib.loads( @@ -81,153 +250,194 @@ def _skylos_config() -> dict[str, object]: return typ.cast("dict[str, object]", tool_config["skylos"]) -def test_skylos_is_a_pinned_external_tool() -> None: - """Keep Skylos out of the project environment and pin its tool release.""" +def test_lint_recipe_runs_the_production_dead_code_gate() -> None: + """``make lint`` must scan only production code with Skylos's strict gate.""" config = _pyproject() dependency_groups = typ.cast("dict[str, list[str]]", config["dependency-groups"]) - - dependencies = dependency_groups["dev"] - assert not any(dependency.startswith("skylos") for dependency in dependencies), ( - "Expected Skylos to be separately provisioned from the development " - "dependency group." + assert not any( + dependency.startswith("skylos") for dependency in dependency_groups["dev"] + ), "Skylos dependency contract must keep the detector out of the dev group." + assert _variable_tokens("SKYLOS_VERSION") == ("4.33.2",), ( + "Skylos version contract must pin 4.33.2." ) - makefile = (REPOSITORY_ROOT / "Makefile").read_text(encoding="utf-8") - assert "SKYLOS_VERSION = 4.33.2" in makefile, ( - "Expected the separately provisioned Skylos tool version to be exact." + assert _variable_tokens("SKYLOS_PRODUCTION_TARGETS") == ("cmd_mox",), ( + "Skylos production-target contract must scan cmd_mox." ) - assert "--from 'skylos==$(SKYLOS_VERSION)' skylos" in makefile, ( - "Expected Skylos to run from its separately provisioned tool environment." + assert _variable_tokens("SKYLOS_EXCLUDE_FOLDERS") == ("tests",), ( + "Skylos exclusion contract must omit tests." ) + skylos_commands = [ + command for command in _recipe_tokens("lint") if command[:1] == ("$(SKYLOS)",) + ] + assert skylos_commands == [ + ( + "$(SKYLOS)", + "$(SKYLOS_PRODUCTION_TARGETS)", + "--exclude", + "$(SKYLOS_EXCLUDE_FOLDERS)", + "--category", + "dead_code", + "--gate", + "--format", + "concise", + "--no-upload", + "--no-provenance", + "--no-grep-verify", + ) + ], "Skylos lint command contract must scan production dead code strictly." -def test_skylos_allow_target_uses_the_standalone_subcommand() -> None: - """Keep the name-only whitelist command separate from the scan command.""" - make_executable = shutil.which("make") - assert make_executable is not None, "Expected make to be available for the test." - - result = subprocess.run( # noqa: S603 - test executes make without a shell - [ - make_executable, - "--no-print-directory", - "--dry-run", - "NAME=bootstrap_shim_path", - "skylos-allow", - ], - cwd=REPOSITORY_ROOT, - check=False, - capture_output=True, - text=True, - ) - - assert result.returncode == 0, "Expected the Skylos allow-list target to expand." +def test_whitelist_target_uses_skylos_subcommand_contract() -> None: + """``skylos whitelist`` must precede its arguments and scan options.""" + assert _variable_tokens("SKYLOS_CLI") == ( + "$(UV_ENV)", + "$(UV)", + "tool", + "run", + "--python", + "3.14", + "--from", + "skylos==$(SKYLOS_VERSION)", + "skylos", + ), "Skylos CLI contract must pin Python 3.14 and its tool release." + assert _variable_tokens("SKYLOS") == ( + "$(SKYLOS_CLI)", + "--config-file", + "pyproject.toml", + ), "Skylos scan command contract must add only the configuration file." whitelist_commands = [ - line for line in result.stdout.splitlines() if "skylos whitelist" in line + command + for command in _recipe_tokens("skylos-allow") + if command[:1] == ("$(SKYLOS_CLI)",) ] - assert len(whitelist_commands) == 1, ( - "Expected the allow-list target to call the standalone Skylos " - "whitelist subcommand without scan options." - ) - whitelist_command = whitelist_commands[0] - assert whitelist_command.endswith('skylos whitelist "${SKYLOS_NAME}"'), ( - "Expected the standalone command to put the whitelist subcommand " - "before its name." - ) - scan_options = ("--config-file", "--category", "--gate") - assert not any(option in whitelist_command for option in scan_options), ( - "Expected the standalone whitelist command not to include scan options." - ) + assert whitelist_commands == [ + ( + "$(SKYLOS_CLI)", + "whitelist", + "$${SKYLOS_SYMBOL}", + "--reason", + "$${SKYLOS_REASON}", + ) + ], "Skylos whitelist command contract must dispatch before --reason." -def test_skylos_allow_target_requires_a_name() -> None: - """Prevent an incomplete allow-list operation from invoking Skylos.""" - make_executable = shutil.which("make") - assert make_executable is not None, "Expected make to be available for the test." +def test_skylos_allow_requires_symbol_and_reason() -> None: + """The whitelist target must reject incomplete input without running Skylos.""" + for arguments, expected_error in ( + ((), "Error: SYMBOL is required for a named whitelist exception"), + ( + ("SYMBOL=bootstrap_shim_path",), + "Error: REASON is required for a named whitelist exception", + ), + ): + completed = _run_skylos_allow(*arguments) - result = subprocess.run( # noqa: S603 - test executes make without a shell - [make_executable, "--no-print-directory", "skylos-allow"], - cwd=REPOSITORY_ROOT, - check=False, + assert completed.returncode == 2, ( + "Skylos whitelist boundary must reject missing required arguments." + ) + assert expected_error in completed.stderr, ( + "Skylos whitelist boundary must name the missing required argument." + ) + + +def test_skylos_allow_dry_run_preserves_the_whitelist_command_contract() -> None: + """A valid dry run must reveal the command without writing an exception.""" + completed = subprocess.run( + ( + "make", + "--dry-run", + "skylos-allow", + "SYMBOL=bootstrap_shim_path", + "REASON=Loaded by bootstrap shim", + ), capture_output=True, + check=False, + cwd=REPOSITORY_ROOT, text=True, ) - assert result.returncode == 2, "Expected an unnamed allow-list operation to fail." - assert "Error: NAME is required for a named whitelist exception" in result.stderr + assert completed.returncode == 0, ( + "Skylos whitelist dry-run contract must accept complete input." + ) + assert ( + 'skylos whitelist "${SKYLOS_SYMBOL}" --reason "${SKYLOS_REASON}"' + in completed.stdout + ), "Skylos whitelist dry-run contract must preserve subcommand argument order." def test_skylos_configuration_is_strict_and_reasoned() -> None: - """Require reasons for every configured Skylos exception.""" + """Require exact, caller-specific reasons for every Skylos exception.""" skylos = _skylos_config() - whitelist = typ.cast("dict[str, object]", skylos["whitelist"]) + whitelist = _mapping(skylos.get("whitelist"), subject="Skylos whitelist") documented = typ.cast("dict[str, str]", whitelist["documented"]) whitelist_names = frozenset(typ.cast("list[str]", whitelist["names"])) assert whitelist_names == EXPECTED_WHITELIST_NAMES, ( - "Expected the reviewed Skylos whitelist names to stay enabled." + "Skylos whitelist contract must keep reviewed names enabled." ) assert frozenset(documented) == whitelist_names, ( - "Expected every documented Skylos whitelist exception to be enabled." + "Skylos whitelist contract must document every enabled exception." ) assert all(reason.strip() for reason in documented.values()), ( - "Expected every documented Skylos whitelist entry to have a reason." + "Skylos whitelist contract must give every exception a reason." ) - - dead_code = typ.cast("dict[str, object]", skylos["dead_code"]) - entrypoints = typ.cast("list[dict[str, object]]", dead_code["entrypoints"]) + dead_code = _mapping(skylos.get("dead_code"), subject="Skylos dead-code config") + entrypoints = _objects(dead_code.get("entrypoints"), subject="Skylos entrypoints") entrypoint_full_names = frozenset( full_name for entrypoint in entrypoints - for full_name in typ.cast("list[str]", entrypoint["full_name"]) + for full_name in _text_sequence( + entrypoint.get("full_name"), subject="entrypoint full name" + ) ) assert entrypoint_full_names == EXPECTED_ENTRYPOINT_FULL_NAMES, ( - "Expected the reviewed Skylos entry points to stay enabled." + "Skylos entry-point contract must keep reviewed runtime callers enabled." ) entrypoint_reasons = { full_name: typ.cast("str", entrypoint["reason"]) for entrypoint in entrypoints - for full_name in typ.cast("list[str]", entrypoint["full_name"]) + for full_name in _text_sequence( + entrypoint.get("full_name"), subject="entrypoint full name" + ) } assert entrypoint_reasons == EXPECTED_ENTRYPOINT_REASONS, ( - "Expected every Skylos entry point to retain its verified runtime caller." + "Skylos entry-point contract must retain each verified runtime caller." ) assert all( isinstance(reason := entrypoint.get("reason"), str) and reason.strip() for entrypoint in entrypoints - ), "Expected every Skylos dead-code entry point to have a reason." - - gate = typ.cast("dict[str, object]", skylos["gate"]) - assert gate["strict"] is True, "Expected the Skylos gate to run in strict mode." - - -def test_make_lint_runs_local_blocking_dead_code_scan() -> None: - """Keep the Skylos invocation deterministic and production-scoped.""" - make_executable = shutil.which("make") - assert make_executable is not None, "Expected make to be available for the test." - - result = subprocess.run( # noqa: S603 - test executes make without a shell - [make_executable, "--no-print-directory", "--dry-run", "lint"], - cwd=REPOSITORY_ROOT, - check=False, - capture_output=True, - text=True, + ), "Skylos entry-point contract must give every exception a reason." + gate = _mapping(skylos.get("gate"), subject="Skylos gate config") + assert gate.get("strict") is True, ( + "Skylos gate configuration must enable strict mode." ) - assert result.returncode == 0, "Expected make lint dry run to succeed." - skylos_commands = [ - line - for line in result.stdout.splitlines() - if "skylos --config-file pyproject.toml" in line - ] - assert len(skylos_commands) == 1, ( - "Expected make lint to expand exactly one blocking Skylos command." - ) - skylos_command = skylos_commands[0] - assert "cmd_mox --category" in skylos_command, ( - "Expected the blocking Skylos command to use production-only targets." + +def test_ci_runs_the_lint_target_and_installs_makeutil() -> None: + """Full-suite CI jobs must install the same pinned Makefile parser.""" + lint_step = _sole_workflow_step( + ".github/workflows/ci.yml", "quality", "Run lint and dead-code detection" ) - assert " tests" not in skylos_command, ( - "Expected tests to be excluded from the production Skylos graph." + assert lint_step.get("run") == "make lint", ( + "CI lint-step contract must invoke the shared make lint target." ) - assert ( - "--category dead_code --gate --format concise --no-upload " - "--no-provenance --no-grep-verify" in skylos_command - ), "Expected the blocking Skylos command to retain its gate flags." + for workflow_path, job_name in ( + (".github/workflows/ci.yml", "quality"), + (".github/workflows/coverage-main.yml", "coverage-upload"), + ): + job = _workflow_job(workflow_path, job_name) + environment = _mapping( + job.get("env"), subject=f"{workflow_path} {job_name} environment" + ) + assert environment.get("MAKEUTIL_REVISION") == _MAKEUTIL_REVISION, ( + f"{workflow_path} {job_name} must pin the Makeutil revision." + ) + assert environment.get("MAKEUTIL_TOOLCHAIN") == _MAKEUTIL_TOOLCHAIN, ( + f"{workflow_path} {job_name} must pin the Makeutil toolchain." + ) + parser_step = _sole_workflow_step( + workflow_path, job_name, "Install Makefile parser" + ) + _assert_makeutil_installation( + parser_step.get("run"), + contract=f"{workflow_path} {job_name} Makeutil-install contract", + )