diff --git a/pyproject.toml b/pyproject.toml index be21077..a8e6676 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -88,6 +88,20 @@ include = ["src/bmad_loop"] # part of the package's import graph); type-checking them is noise. exclude = ["src/bmad_loop/data"] typeCheckingMode = "basic" +# Stage 2 of #245: the pure/leaf modules (no live process, git, or mux I/O) are +# held to strict analysis while the rest stays basic. `strict` overrides +# typeCheckingMode for these paths only — the same effect as a per-file +# `# pyright: strict` comment — so annotation drift in the domain core is caught +# without forcing the I/O-heavy modules over the strict bar. No runtime changes. +strict = [ + "src/bmad_loop/model.py", + "src/bmad_loop/statemachine.py", + "src/bmad_loop/policy.py", + "src/bmad_loop/documents.py", + "src/bmad_loop/machine.py", + "src/bmad_loop/checks.py", + "src/bmad_loop/sanitize.py", +] # Check against the floor of requires-python so we never lean on 3.12+ typing. pythonVersion = "3.11" # Resolve third-party imports from the uv-managed project venv (created by diff --git a/src/bmad_loop/checks.py b/src/bmad_loop/checks.py index 739841b..8460ae1 100644 --- a/src/bmad_loop/checks.py +++ b/src/bmad_loop/checks.py @@ -21,9 +21,17 @@ on runtime data, so it is a lint with a stack trace rather than a validation. """ +# Strict-checked under #245 Stage 2, with one rule relaxed for this file only: +# `ValidationReport.findings` uses the idiomatic `field(default_factory=list)`, +# which pyright can only infer as `list[Unknown]` (it does not fold the declared +# `list[Finding]` annotation back into the bare `list` factory) though the field +# is correctly typed. Every other strict rule stays on. +# pyright: reportUnknownVariableType=false + from __future__ import annotations import sys +from collections.abc import Mapping from dataclasses import dataclass, field from typing import Literal @@ -80,7 +88,7 @@ class Finding: check: str severity: Severity message: str - detail: dict | None = None + detail: Mapping[str, object] | None = None @dataclass @@ -95,17 +103,23 @@ class ValidationReport: findings: list[Finding] = field(default_factory=list) - def add(self, check: str, severity: Severity, message: str, detail: dict | None = None) -> None: + def add( + self, + check: str, + severity: Severity, + message: str, + detail: Mapping[str, object] | None = None, + ) -> None: assert check in VALIDATE_CHECKS, f"unregistered check id: {check!r}" self.findings.append(Finding(check, severity, message, detail)) - def ok(self, check: str, message: str, detail: dict | None = None) -> None: + def ok(self, check: str, message: str, detail: Mapping[str, object] | None = None) -> None: self.add(check, "ok", message, detail) - def warn(self, check: str, message: str, detail: dict | None = None) -> None: + def warn(self, check: str, message: str, detail: Mapping[str, object] | None = None) -> None: self.add(check, "warning", message, detail) - def fail(self, check: str, message: str, detail: dict | None = None) -> None: + def fail(self, check: str, message: str, detail: Mapping[str, object] | None = None) -> None: self.add(check, "problem", message, detail) def extend(self, findings: list[Finding]) -> None: diff --git a/src/bmad_loop/documents.py b/src/bmad_loop/documents.py index ccbe43f..7c335ef 100644 --- a/src/bmad_loop/documents.py +++ b/src/bmad_loop/documents.py @@ -52,7 +52,9 @@ VALIDATE_SCHEMA_VERSION = 1 -def validate_document(report: ValidationReport, stories_on: bool, spec_folder: str) -> dict: +def validate_document( + report: ValidationReport, stories_on: bool, spec_folder: str +) -> dict[str, object]: """The `validate --json` document: the verdict plus every check that produced it. Obeys the pure-document contract in machine.py (additive-only evolution; @@ -211,7 +213,7 @@ def status_document(state: RunState, *, graceful_stop_pending: bool = False) -> status = "stopped" else: status = "in-progress" - tasks = [] + tasks: list[dict[str, object]] = [] for key, task in state.tasks.items(): tokens = task.tokens.to_dict() tokens["raw"] = task.tokens.total diff --git a/src/bmad_loop/machine.py b/src/bmad_loop/machine.py index 5e5cab7..0270b97 100644 --- a/src/bmad_loop/machine.py +++ b/src/bmad_loop/machine.py @@ -118,7 +118,9 @@ def emit_document(rendered: str) -> None: # is a crash with stdout still empty — permitted by the contract above. if hasattr(sys.stdout, "reconfigure"): # hasattr-guarded: reconfigure exists on TextIOWrapper, not the TextIO base. - sys.stdout.reconfigure(encoding="utf-8") # pyright: ignore[reportAttributeAccessIssue] + sys.stdout.reconfigure( # pyright: ignore[reportAttributeAccessIssue, reportUnknownMemberType] + encoding="utf-8" + ) print(document) diff --git a/src/bmad_loop/model.py b/src/bmad_loop/model.py index 3dc3ac6..8e8c6ea 100644 --- a/src/bmad_loop/model.py +++ b/src/bmad_loop/model.py @@ -1,5 +1,16 @@ """Core data model: story lifecycle phases, per-task records, run state.""" +# Strict-checked under #245 Stage 2, with the two rules below relaxed for this +# file only. `reportUnknownVariableType`: the dataclass collection fields use the +# idiomatic `field(default_factory=list|dict)`, which pyright can only infer as +# `list[Unknown]` / `dict[Unknown, Unknown]` (it does not fold the declared +# annotation back into the factory) though the fields are correctly typed. +# `reportUnknownArgumentType`: `from_dict` / snapshot readers pull values out of +# run-persisted `dict[str, Any]`, so isinstance-narrowing an `Any` value yields +# Unknown at that boundary. Both are inherent to the persistence edge, not +# annotation drift; every other strict rule stays on. +# pyright: reportUnknownArgumentType=false, reportUnknownVariableType=false + from __future__ import annotations from dataclasses import dataclass, field diff --git a/src/bmad_loop/policy.py b/src/bmad_loop/policy.py index 39726b5..c636671 100644 --- a/src/bmad_loop/policy.py +++ b/src/bmad_loop/policy.py @@ -1,5 +1,15 @@ """Policy-as-data: .bmad-loop/policy.toml -> immutable Policy dataclasses.""" +# Strict-checked under #245 Stage 2, with the three "expression fully known" +# rules below relaxed for this file only: the snapshot/TOML readers rebuild typed +# Policy objects from loosely-typed persisted data — `tomllib.load` and a run's +# json-round-tripped `asdict(Policy)` both hand back `dict[str, Any]`, and +# isinstance-narrowing an `Any` yields `dict[Unknown, Unknown]` — so `.get(...)` +# chains off those dicts read as Unknown. Clearing them would take a TypedDict +# per persisted shape or runtime coercions (out of scope for a no-runtime-change +# gate); every other strict rule stays on and still catches annotation drift. +# pyright: reportUnknownArgumentType=false, reportUnknownMemberType=false, reportUnknownVariableType=false + from __future__ import annotations import re diff --git a/src/bmad_loop/sanitize.py b/src/bmad_loop/sanitize.py index aceab0c..0beec54 100644 --- a/src/bmad_loop/sanitize.py +++ b/src/bmad_loop/sanitize.py @@ -35,6 +35,14 @@ bug or a future field can therefore never silently ship a secret/PII/path. """ +# Strict-checked under #245 Stage 2, with the two "expression fully known" rules +# below relaxed for this file only: `_scrub` walks arbitrary, foreign JSON whose +# leaves are `Any` by design (it exists to redact unknown/future fields), so +# isinstance-narrowing that `Any` yields dict/list `[Unknown, ...]` and every +# key/value it iterates is Unknown. Typing it away would defeat the point of a +# catch-all scrubber. Every other strict rule stays on. +# pyright: reportUnknownArgumentType=false, reportUnknownVariableType=false + from __future__ import annotations import getpass diff --git a/tests/test_cli.py b/tests/test_cli.py index 317e331..46f255a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3483,6 +3483,50 @@ def test_validate_json_warning_message_carries_no_severity_prefix(project, capsy assert not finding["message"].startswith(" ") +def test_validate_json_detail_round_trips_for_every_real_shape(capsys): + """Every `detail` shape the real gates attach must survive `validate --json`'s + `json.dumps` boundary. + + #268 widened `Finding.detail` to `Mapping[str, object]` for strict-mode + covariance — so `install.py`'s `dict[str, str]` stays assignable — which leaves + implicit the invariant this pins: the production path + `machine.emit(validate_document(...))` still emits one whole, parseable document + for every detail a caller actually builds. Each case below mirrors a real call + site (str values, the nested `dict(role_names)` dict, str+int, install.py's + `{**detail, "marker": ...}`, and the `detail=None` leg). A future caller that + attaches a non-JSON-serializable detail fails here, by name, rather than at + runtime on stdout. + """ + from bmad_loop import machine + + report = cli.ValidationReport() + report.ok("adapter.binary", "codex found", {"binary": "codex"}) # str values (cli.py) + report.ok( # nested plain dict — the `dict(role_names)` shape (cli.py) + "policy", + "policy OK", + {"gates_mode": "warn", "adapters": {"dev": "claude", "review": "claude"}}, + ) + report.ok( # str + int (cli.py) + "queue.stories-manifest", "stories OK", {"spec_folder": "docs", "stories": 3} + ) + report.fail( # install.py's `{**detail, "marker": ...}` shape + "skills.stories-dispatch-stale", + "stale", + {"tree": ".claude", "skill": "s", "file": "f", "marker": "m"}, + ) + report.ok("git.worktree-clean", "clean") # the detail=None leg + + # the exact production path: cli.py does `machine.emit(validate_document(...))`. + machine.emit(cli.validate_document(report, stories_on=False, spec_folder="")) + parsed = json.loads(capsys.readouterr().out) # whole stdout parses => a pure document + + by_check = {f["check"]: f for f in parsed["findings"]} + assert by_check["policy"]["detail"]["adapters"]["dev"] == "claude" # nested dict survives + assert by_check["queue.stories-manifest"]["detail"]["stories"] == 3 # int, not "3" + assert by_check["skills.stories-dispatch-stale"]["detail"]["marker"] == "m" + assert by_check["git.worktree-clean"]["detail"] is None # None -> null round-trips + + @pytest.mark.parametrize( ("spec", "mode", "folder"), [(None, "sprint", ""), (STORIES_SPEC_FOLDER, "stories", STORIES_SPEC_FOLDER)],