jpmschweitzerandClaude Opus 5.5
caea76387b
Two failures T-1261 found and fixed by hand, left unguarded. They now fail the conformance gate: - (8) An interpreter-prefixed invocation — `python -m tooling`, `python3 -m tooling`, `.venv/bin/reach` — anywhere in the Makefile, the hooks, .claude or docs. One such line silently forks the PATH guarantee. Lines that FORBID the pattern are exempt, since docs/DEVOPS.md states the rule in those words and a check that flags its own documentation gets muted. - (9) The installed reach (read from its shebang) and the .venv the gate tests run in must be the same Python minor version, and it must be the one the Makefile pins. uv once installed reach on 3.11 against a 3.14 venv, so the tests were proving nothing about the interpreter the hooks run. Both proven to fail: an appended `python3 -m tooling.main` make target is caught at its line, and a fake `reach` with a 3.12 shebang first on PATH trips both version comparisons. The invariant list in the docstring now covers all nine, not the first five. Also: the canvas-version units ran under system python3 while every other gate test uses $(VENV_PY). Now they match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
475 lines
21 KiB
Python
475 lines
21 KiB
Python
#!/usr/bin/env python3
|
|
"""The D-263 invariants, as a test rather than a style guide (T-1270).
|
|
|
|
D-263 lists the rules that make the layering real and then says plainly that a
|
|
contract nothing checks is a style guide. This is the check.
|
|
|
|
It matters most *later*. With one domain, every rule here is obvious and nobody
|
|
would break one. Once T-1250 lands ~120 commands, nobody re-reads a decision
|
|
record before adding a verb — this is what tells them, at the moment it is cheap
|
|
to fix rather than after the pattern has been copied forty times.
|
|
|
|
Invariants:
|
|
|
|
1. Transport stays out of the logic layers. No typer/click import in any
|
|
service.py, schemas.py or helpers.py — ever. Transport lives in main.py,
|
|
router.py, and core/cli.py (the single designated transport module).
|
|
2. Nothing prints but console. No bare print()/sys.stdout.write outside
|
|
core/console.py — two output paths drift, and the second is always the one
|
|
that ends up unformatted on stdout inside a hook.
|
|
3. Every registered command carries @command, so no command can have logging
|
|
without error handling or vice versa.
|
|
4. Every command has help at its own level.
|
|
5. Every ReachError names a remedy — a `fix=` on every raise site. The hardest
|
|
to enforce and the most valuable: an error that only says "no" is the thing
|
|
D-263 exists to replace.
|
|
6. Discovery succeeds and being wrong fails (exit codes).
|
|
7. The Blender carve-out stays outside the package, and stays populated.
|
|
8. reach is invoked by its bare name — never `python -m tooling` or a venv
|
|
path — in the Makefile, hooks, .claude and docs (T-1257, from T-1261).
|
|
9. The installed reach and the .venv the gate tests run in are the same
|
|
Python minor version, and it is the one the Makefile pins (T-1257).
|
|
|
|
The import-graph invariant (nothing heavy reachable from main.py) lives in
|
|
test_lazy_domains.py rather than being duplicated here.
|
|
|
|
Run: python3 tooling/test_conformance.py
|
|
"""
|
|
|
|
import ast
|
|
import shutil
|
|
import subprocess
|
|
import sys
|
|
from pathlib import Path
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
|
PACKAGE = REPO_ROOT / "tooling"
|
|
|
|
# Files permitted to import the CLI framework. See core/cli.py for why it is on
|
|
# the list: a shared group class is transport by definition, and the alternative
|
|
# is a copy of it in every router that drifts.
|
|
TRANSPORT_FILES = {"main.py", "router.py", "cli.py"}
|
|
LOGIC_FILES = {"service.py", "schemas.py", "helpers.py", "dependencies.py"}
|
|
FORBIDDEN_IMPORTS = {"typer", "click"}
|
|
|
|
|
|
# The invariants govern the PACKAGE, not the legacy tree. The ~123 loose scripts
|
|
# under tooling/ predate all of this and use bare print() throughout; holding
|
|
# them to a contract they were never written against would mean 500 failures on
|
|
# day one and a suite nobody runs.
|
|
#
|
|
# This is not a permanent carve-out. As T-1250 moves each script into
|
|
# domains/<name>/, it lands inside this scope and the invariants start applying
|
|
# automatically — so the test's reach grows with the migration rather than
|
|
# needing to be widened by hand.
|
|
PACKAGE_ROOTS = ("main.py", "__init__.py", "core", "domains")
|
|
|
|
# tooling/scripts/blender/ is DELIBERATELY not in that tuple, and must never be
|
|
# added (T-1273). Those 35 files run under Blender's BUNDLED Python, which has
|
|
# no access to the repo venv — they physically cannot `import tooling.core`, so
|
|
# they cannot carry `@command` or print through `console`. Holding them to the
|
|
# contract would either fail this gate forever or force the contract to be
|
|
# weakened for everyone, and the second is how a gate stops meaning anything.
|
|
#
|
|
# It reads like an oversight, so `check_carve_out_stays_carved` below asserts
|
|
# the exclusion on purpose: widening PACKAGE_ROOTS to cover them fails loudly
|
|
# instead of quietly redefining what conformance means.
|
|
PAYLOAD_ROOTS = ("scripts",)
|
|
|
|
|
|
def _package_files() -> list[Path]:
|
|
"""Every .py that is part of the reach package — not the legacy scripts."""
|
|
files: list[Path] = []
|
|
for entry in PACKAGE_ROOTS:
|
|
target = PACKAGE / entry
|
|
if target.is_dir():
|
|
files.extend(target.rglob("*.py"))
|
|
elif target.is_file():
|
|
files.append(target)
|
|
return sorted(path for path in files if not path.name.startswith("test_"))
|
|
|
|
|
|
def _imports(tree: ast.AST) -> set[str]:
|
|
found: set[str] = set()
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Import):
|
|
found.update(alias.name.split(".")[0] for alias in node.names)
|
|
elif isinstance(node, ast.ImportFrom) and node.module and node.level == 0:
|
|
found.add(node.module.split(".")[0])
|
|
return found
|
|
|
|
|
|
def check_transport_isolation(failures: list[str]) -> None:
|
|
"""(1) No typer/click outside the designated transport files."""
|
|
for path in _package_files():
|
|
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
|
|
offending = _imports(tree) & FORBIDDEN_IMPORTS
|
|
if not offending:
|
|
continue
|
|
if path.name in TRANSPORT_FILES:
|
|
continue
|
|
rel = path.relative_to(REPO_ROOT)
|
|
failures.append(
|
|
f"[transport] {rel} imports {', '.join(sorted(offending))} — "
|
|
f"only {', '.join(sorted(TRANSPORT_FILES))} may. "
|
|
"A service must not know it was called from a CLI."
|
|
)
|
|
if path.name in LOGIC_FILES:
|
|
failures.append(
|
|
f"[transport] {rel} is a LOGIC file — this is the invariant that "
|
|
"makes services callable from tests and from each other"
|
|
)
|
|
|
|
|
|
def check_jobs_stay_ambient(failures: list[str]) -> None:
|
|
"""No domain imports core.jobs — job identity is the decorator's business.
|
|
|
|
The whole point of carrying the job ambiently is that a command never has to
|
|
open one, tag one, or remember to close one. The moment a domain imports
|
|
`core.jobs`, that has become call-site discipline again, and it will fail the
|
|
same way: one command forgets and its output loses correlation silently.
|
|
|
|
Exempt: core/ (command.py and console.py implement the ambience) and
|
|
main.py. main.py is not a command — it is the entry point, and it already
|
|
owns the invocation-level concerns --detach, --verbose and --no-input.
|
|
Recording a detached child's completion at the PROCESS's exit belongs there
|
|
for the same reason, and is far less coupled than the --detach flag it
|
|
already carries.
|
|
"""
|
|
for path in _package_files():
|
|
if path.parent.name == "core" or path.name == "main.py":
|
|
continue
|
|
source = path.read_text(encoding="utf-8")
|
|
tree = ast.parse(source, filename=str(path))
|
|
for node in ast.walk(tree):
|
|
# Three forms reach the same module and all three must be caught:
|
|
# from tooling.core import jobs -> module, names
|
|
# from tooling.core.jobs import ... -> module
|
|
# import tooling.core.jobs -> names
|
|
hit = False
|
|
if isinstance(node, ast.ImportFrom) and node.module:
|
|
hit = node.module == "tooling.core.jobs" or (
|
|
node.module == "tooling.core"
|
|
and any(alias.name == "jobs" for alias in node.names)
|
|
)
|
|
elif isinstance(node, ast.Import):
|
|
hit = any(alias.name == "tooling.core.jobs" for alias in node.names)
|
|
if hit:
|
|
failures.append(
|
|
f"[jobs] {path.relative_to(REPO_ROOT)}:{node.lineno} imports core.jobs — "
|
|
"job identity is ambient and belongs to @command; a command that "
|
|
"touches it has reintroduced the call-site discipline this replaced"
|
|
)
|
|
|
|
|
|
def check_one_guarded_exec(failures: list[str]) -> None:
|
|
"""Only core/process.py may touch subprocess or os.system.
|
|
|
|
A guarded exec is the right answer for genuine OS work (D-263) — but the
|
|
guards only hold if there is one door. Every per-domain `subprocess.run` is
|
|
a place where the argv-list rule, the missing-binary message, or turning a
|
|
non-zero exit into a remedy quietly goes missing, and none of those
|
|
omissions announces itself.
|
|
|
|
core/process.py is exempt because it IS the door. Test scripts are outside
|
|
the package scope already.
|
|
"""
|
|
for path in _package_files():
|
|
if path.name == "process.py" and path.parent.name == "core":
|
|
continue
|
|
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
|
|
if "subprocess" in _imports(tree):
|
|
failures.append(
|
|
f"[exec] {path.relative_to(REPO_ROOT)} imports subprocess — go through "
|
|
"core/process.run, which is where the argv-list rule, the "
|
|
"missing-binary message and the failure remedy live"
|
|
)
|
|
for node in ast.walk(tree):
|
|
# The receiver has to be checked, not just the attribute name:
|
|
# `platform.system()` is a legitimate call and shares a name with
|
|
# `os.system()`. A check that fires on the wrong thing gets muted.
|
|
if (
|
|
isinstance(node, ast.Call)
|
|
and isinstance(node.func, ast.Attribute)
|
|
and isinstance(node.func.value, ast.Name)
|
|
and node.func.value.id == "os"
|
|
and node.func.attr in {"system", "popen", "execv", "execvp"}
|
|
):
|
|
failures.append(
|
|
f"[exec] {path.relative_to(REPO_ROOT)}:{node.lineno} calls "
|
|
f"os.{node.func.attr} — unguarded, and shell-interpreting in the "
|
|
"case of system()"
|
|
)
|
|
|
|
|
|
def check_single_output_path(failures: list[str]) -> None:
|
|
"""(2) Nothing prints but console."""
|
|
for path in _package_files():
|
|
if path.name == "console.py":
|
|
continue
|
|
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
|
|
for node in ast.walk(tree):
|
|
if not isinstance(node, ast.Call):
|
|
continue
|
|
func = node.func
|
|
if isinstance(func, ast.Name) and func.id == "print":
|
|
failures.append(
|
|
f"[output] {path.relative_to(REPO_ROOT)}:{node.lineno} calls print() — "
|
|
"core/console.py is the single output path (D-263)"
|
|
)
|
|
elif (
|
|
isinstance(func, ast.Attribute)
|
|
and func.attr == "write"
|
|
and isinstance(func.value, ast.Attribute)
|
|
and func.value.attr in {"stdout", "stderr"}
|
|
):
|
|
failures.append(
|
|
f"[output] {path.relative_to(REPO_ROOT)}:{node.lineno} writes to "
|
|
"sys.stdout/stderr directly — go through core/console.py"
|
|
)
|
|
|
|
|
|
def check_commands_decorated(failures: list[str]) -> None:
|
|
"""(3) and (4): every registered command carries @command and has help."""
|
|
probe = """
|
|
import json, sys
|
|
from tooling.core.command import MARKER
|
|
from tooling.main import DOMAINS, _load_domain
|
|
|
|
report = []
|
|
|
|
def walk(name, group, prefix):
|
|
# A nested group (`atlas planet`) is not a verb: its callback only keeps it
|
|
# a group. Recurse into it instead, so the verbs underneath are held to the
|
|
# contract too -- a one-level walk reported the group and skipped all ten.
|
|
for verb in group.list_commands(None):
|
|
cmd = group.get_command(None, verb)
|
|
if hasattr(cmd, "list_commands"):
|
|
walk(name, cmd, prefix + verb + " ")
|
|
continue
|
|
callback = getattr(cmd, "callback", None)
|
|
report.append({
|
|
"domain": name,
|
|
"verb": prefix + verb,
|
|
"decorated": bool(getattr(callback, MARKER, False)),
|
|
"help": (cmd.help or cmd.short_help or "").strip(),
|
|
})
|
|
|
|
for name in sorted(DOMAINS):
|
|
walk(name, _load_domain(name), "")
|
|
print(json.dumps(report))
|
|
"""
|
|
result = subprocess.run(
|
|
[sys.executable, "-c", probe], capture_output=True, text=True, cwd=REPO_ROOT
|
|
)
|
|
if result.returncode != 0:
|
|
failures.append(f"[commands] could not introspect the CLI:\n{result.stderr}")
|
|
return
|
|
|
|
import json
|
|
|
|
report = json.loads(result.stdout)
|
|
if not report:
|
|
failures.append(
|
|
"[commands] no commands found — every assertion here would pass vacuously"
|
|
)
|
|
for entry in report:
|
|
where = f"{entry['domain']} {entry['verb']}"
|
|
if not entry["decorated"]:
|
|
failures.append(
|
|
f"[commands] `reach {where}` is missing @command — it would run "
|
|
"without the error contract or the invocation record"
|
|
)
|
|
if not entry["help"]:
|
|
failures.append(f"[commands] `reach {where}` has no help text")
|
|
|
|
|
|
def check_errors_name_a_remedy(failures: list[str]) -> None:
|
|
"""(5) Every ReachError raise site passes fix=."""
|
|
for path in _package_files():
|
|
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
|
|
for node in ast.walk(tree):
|
|
if not isinstance(node, ast.Raise) or not isinstance(node.exc, ast.Call):
|
|
continue
|
|
func = node.exc.func
|
|
name = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", "")
|
|
if name != "ReachError":
|
|
continue
|
|
if not any(kw.arg == "fix" for kw in node.exc.keywords):
|
|
failures.append(
|
|
f"[remedy] {path.relative_to(REPO_ROOT)}:{node.lineno} raises "
|
|
"ReachError without fix= — an error that only says 'no' is "
|
|
"what D-263 exists to replace"
|
|
)
|
|
|
|
|
|
def check_exit_codes(failures: list[str]) -> None:
|
|
"""(6) Discovery succeeds; being wrong fails.
|
|
|
|
The distinction is easy to break in either direction, and both directions
|
|
are bad in ways nothing else would catch. Make discovery a usage error and
|
|
an agent reads its own onboarding as a failure. Make a wrong name succeed
|
|
and a typo in a hook passes silently — the exit-0 trap D-263 opens with.
|
|
"""
|
|
cases = [
|
|
([], 0, "bare `reach` is discovery, not a usage error"),
|
|
(["check"], 0, "bare `reach <domain>` is discovery, not a usage error"),
|
|
(["--help"], 0, "--help succeeds"),
|
|
(["definitely-not-a-domain"], 2, "an unknown domain is a usage error"),
|
|
(["check", "definitely-not-a-verb"], 2, "an unknown verb is a usage error"),
|
|
]
|
|
for args, expected, why in cases:
|
|
result = subprocess.run(
|
|
["reach", *args], capture_output=True, text=True, cwd=REPO_ROOT
|
|
)
|
|
if result.returncode != expected:
|
|
failures.append(
|
|
f"[exit] `reach {' '.join(args)}` exited {result.returncode}, "
|
|
f"expected {expected} — {why}"
|
|
)
|
|
|
|
|
|
def check_carve_out_stays_carved(failures: list[str]) -> None:
|
|
"""(7) The Blender payloads stay outside package scope, and stay populated.
|
|
|
|
Two failure modes, opposite directions:
|
|
|
|
- Someone widens PACKAGE_ROOTS to cover `scripts/` because the exclusion
|
|
looks like an oversight. Every payload then fails five invariants it
|
|
cannot satisfy, and the likely repair is to weaken the invariants.
|
|
- The payload directory quietly empties — a move, a bad merge — and the
|
|
exclusion goes on passing because excluding nothing is trivially fine.
|
|
An exception that guards nothing should not read as healthy.
|
|
"""
|
|
for root in PAYLOAD_ROOTS:
|
|
if root in PACKAGE_ROOTS:
|
|
failures.append(
|
|
f"[carve-out] '{root}' was added to PACKAGE_ROOTS — the Blender "
|
|
"payloads run under Blender's bundled Python and cannot import "
|
|
"tooling.core; see the comment above PAYLOAD_ROOTS (T-1273)"
|
|
)
|
|
|
|
payloads = list((PACKAGE / "scripts" / "blender").glob("*.py"))
|
|
if not payloads:
|
|
failures.append(
|
|
"[carve-out] tooling/scripts/blender/ holds no payloads — either the "
|
|
"carve-out was undone or they moved; an empty exclusion proves nothing"
|
|
)
|
|
|
|
# The payloads must not be reachable as modules either: an __init__.py would
|
|
# make them importable and invite exactly the coupling the carve-out prevents.
|
|
if (PACKAGE / "scripts" / "__init__.py").exists():
|
|
failures.append(
|
|
"[carve-out] tooling/scripts/__init__.py exists — that makes the "
|
|
"payload tree an importable package, which is what the carve-out avoids"
|
|
)
|
|
|
|
|
|
def check_invoked_through_path(failures: list[str]) -> None:
|
|
"""(8) reach is invoked by NAME, never through an interpreter or a venv path.
|
|
|
|
The PATH guarantee (T-1261) is that `reach` means the same program in a
|
|
shell, a hook and an agent. One `python -m tooling …` or `.venv/bin/reach`
|
|
in a make target or a hook silently forks that — it runs whichever
|
|
interpreter happens to be first, with whichever dependencies it has — and
|
|
nothing fails until a version-specific bug shows in one path and not the
|
|
other. T-1261 verified this by hand; this makes it permanent (T-1257).
|
|
"""
|
|
patterns = ("python -m tooling", "python3 -m tooling", ".venv/bin/reach")
|
|
roots = [REPO_ROOT / "Makefile", REPO_ROOT / ".config" / "hooks", REPO_ROOT / ".claude", REPO_ROOT / "docs"]
|
|
for root in roots:
|
|
files = [root] if root.is_file() else [p for p in root.rglob("*") if p.is_file()]
|
|
for path in files:
|
|
try:
|
|
text = path.read_text(encoding="utf-8")
|
|
except (UnicodeDecodeError, OSError):
|
|
continue
|
|
for lineno, line in enumerate(text.splitlines(), 1):
|
|
# A line that FORBIDS the pattern is the rule, not a breach of
|
|
# it (docs/DEVOPS.md: "Never `python -m tooling`"). A check that
|
|
# flags its own documentation gets muted.
|
|
if "never" in line.lower():
|
|
continue
|
|
for pattern in patterns:
|
|
if pattern in line:
|
|
failures.append(
|
|
f"[path] {path.relative_to(REPO_ROOT)}:{lineno} invokes reach through "
|
|
f"'{pattern}' — call the bare name `reach` (T-1261's PATH guarantee)"
|
|
)
|
|
|
|
|
|
def check_one_python(failures: list[str]) -> None:
|
|
"""(9) The installed reach and the .venv run the same Python minor version.
|
|
|
|
T-1261 found uv had installed the reach tool on CPython 3.11 while .venv
|
|
and system python were 3.14 — uv picks the LOWEST interpreter satisfying
|
|
requires-python. The Makefile now pins PYTHON_VERSION for both, but nothing
|
|
noticed a divergence: the gate tests run under .venv, the hooks run reach,
|
|
and a bug in one interpreter would pass the tests that ran on the other.
|
|
"""
|
|
import re
|
|
import shutil
|
|
|
|
reach = shutil.which("reach")
|
|
if reach is None:
|
|
return # check_exit_codes already reports a missing reach
|
|
shebang = Path(reach).read_text(encoding="utf-8", errors="replace").splitlines()[0]
|
|
if not shebang.startswith("#!"):
|
|
failures.append(f"[python] {reach} has no shebang — cannot tell which Python runs reach")
|
|
return
|
|
interpreter = shebang[2:].strip().split()[0]
|
|
probe = subprocess.run(
|
|
[interpreter, "-c", "import sys; print('%d.%d' % sys.version_info[:2])"],
|
|
capture_output=True, text=True,
|
|
)
|
|
tool_version = probe.stdout.strip()
|
|
venv_version = "%d.%d" % sys.version_info[:2]
|
|
makefile = (REPO_ROOT / "Makefile").read_text(encoding="utf-8")
|
|
pinned = re.search(r"^PYTHON_VERSION \?= (\S+)", makefile, re.M)
|
|
pinned_version = pinned.group(1) if pinned else None
|
|
|
|
if tool_version != venv_version:
|
|
failures.append(
|
|
f"[python] reach runs on Python {tool_version} ({interpreter}) but the gate tests run on "
|
|
f"{venv_version} — tests prove nothing about the interpreter the hooks use. "
|
|
"Fix: make install-reach && make setup-venv (both pin PYTHON_VERSION)"
|
|
)
|
|
if pinned_version and tool_version != pinned_version:
|
|
failures.append(
|
|
f"[python] reach runs on Python {tool_version}, the Makefile pins {pinned_version}. "
|
|
"Fix: make install-reach"
|
|
)
|
|
|
|
|
|
def main() -> int:
|
|
if shutil.which("reach") is None:
|
|
print("test_conformance: `reach` is not on PATH.\n Fix: make install-reach", file=sys.stderr)
|
|
return 1
|
|
|
|
failures: list[str] = []
|
|
check_transport_isolation(failures)
|
|
check_jobs_stay_ambient(failures)
|
|
check_one_guarded_exec(failures)
|
|
check_single_output_path(failures)
|
|
check_commands_decorated(failures)
|
|
check_errors_name_a_remedy(failures)
|
|
check_exit_codes(failures)
|
|
check_carve_out_stays_carved(failures)
|
|
check_invoked_through_path(failures)
|
|
check_one_python(failures)
|
|
|
|
if failures:
|
|
print("test_conformance: FAIL", file=sys.stderr)
|
|
for failure in failures:
|
|
print(f" - {failure}", file=sys.stderr)
|
|
return 1
|
|
|
|
print("test_conformance: OK — transport isolated, one output path, "
|
|
"every command decorated and helped, every error names a remedy, "
|
|
"reach called by name, one Python")
|
|
return 0
|
|
|
|
|
|
if __name__ == "__main__":
|
|
sys.exit(main())
|