From 25422cd71b5e9837025082f187679b40f0c58d6b Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Wed, 8 Jul 2026 17:31:10 +0100 Subject: [PATCH] fix: API-gate bypass, broken-stack conflation, and duplicated import walls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three confirmed tooling bugs from the phase-A audit (issue #35): 1. The documented per-command bypass (PYAUTO_SKIP_API_GATE=1 as a command prefix) could never work — the hook read only its own process env. The hook now also detects the prefix in the command text. 2. A stack that fails to import (e.g. the workspace version check) marked every symbol STALE and repeated the full multi-paragraph import error once per symbol. validate_source now separates import-failed roots (truncated, reported once) from genuine staleness; the gate exits 3 (INSTALL_IMPORT_FAILED) instead of 2 and the hook fails open on it — the command surfaces the same import error itself, and a broken env is not API drift. 3. --check-install/--check-version printed the identical import wall once per library (3x for a version mismatch); identical errors are now grouped and printed once, naming all affected modules. Also: Makefile gains audit/test targets; the gate skill documents the exit-3 contract and both bypass forms. Three new regression tests pin the fixes; full tooling suite 39 passed. The version pin (2026.5.29.4 vs installed dev build) is deliberately untouched — version.txt is release-owned and hand-bumping it would fake a release state. Co-Authored-By: Claude Fable 5 --- .claude/hooks/validate_pyauto_code.py | 19 ++++++++-- Makefile | 10 +++++- autoassistant/audit_skill_apis.py | 51 ++++++++++++++++++++++----- autoassistant/tests/test_api_gate.py | 47 ++++++++++++++++++++++++ skills/al_audit_skill_apis.md | 10 ++++-- 5 files changed, 123 insertions(+), 14 deletions(-) diff --git a/.claude/hooks/validate_pyauto_code.py b/.claude/hooks/validate_pyauto_code.py index 38bb33e..036c8b6 100644 --- a/.claude/hooks/validate_pyauto_code.py +++ b/.claude/hooks/validate_pyauto_code.py @@ -19,8 +19,13 @@ re-grounds against the live API / skills. Otherwise allow. Fail-open by design: any internal error (unparseable command, missing validator, etc.) -allows the call — the gate must never block legitimate work because of its own bugs. -Escape hatch: set PYAUTO_SKIP_API_GATE=1 to allow unconditionally. +allows the call — the gate must never block legitimate work because of its own bugs. A +broken stack (validator exit 3: imports fail, e.g. the workspace version check) also +fails open: the command will surface the same import error itself, and blocking it +would conflate a broken environment with symbol drift. +Escape hatch: set PYAUTO_SKIP_API_GATE=1 — in the hook's environment, or as a prefix on +the command itself (`PYAUTO_SKIP_API_GATE=1 python …`); the hook process env is the +session's, not the command's, so the prefix form is detected in the command text. The same script backs both the autolens_assistant hook and the PyAutoLabs-monorepo hook; the validator path is resolved relative to this file, so it works from either project. @@ -149,6 +154,12 @@ def main() -> None: command = (payload.get("tool_input") or {}).get("command") or "" cwd = Path(payload.get("cwd") or ".") + # Per-command escape hatch: an env-var prefix on the command sets the variable + # for the *command's* process, not this hook's, so it must be detected in the + # command text itself. + if re.search(r"\bPYAUTO_SKIP_API_GATE=1\b", command): + _allow() + # Fast reject: not a Python invocation, or no PyAuto* symbol anywhere in the command. if "python" not in command or not _has_symbol(command): # `_has_symbol(command)` misses symbols that live only inside a referenced .py @@ -178,6 +189,10 @@ def main() -> None: ) except (OSError, subprocess.SubprocessError): continue # validator unrunnable — fail open for this source + # Only returncode 2 (stale symbols/idioms) denies. Exit 3 — the stack itself + # failed to import (broken env / workspace version check) — falls through to + # allow: the command will raise the same import error on its own, and the + # gate only guards symbol drift. if proc.returncode == 2: stale = [ ln.strip() diff --git a/Makefile b/Makefile index c88a434..edbb81a 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,12 @@ -.PHONY: validate-literature-citations +.PHONY: validate-literature-citations audit test validate-literature-citations: python -m autoassistant.literature validate-citations + +# Symbol/idiom audit of skills + wiki against the installed stack. +audit: + python autoassistant/audit_skill_apis.py + +# Assistant tooling test suite (slow: the gate tests import autolens per case). +test: + python -m pytest autoassistant/tests -q diff --git a/autoassistant/audit_skill_apis.py b/autoassistant/audit_skill_apis.py index c4f1968..f497b32 100644 --- a/autoassistant/audit_skill_apis.py +++ b/autoassistant/audit_skill_apis.py @@ -703,8 +703,14 @@ def render_installation_check(check: InstallationCheck) -> str: lines.append(f" install type: {check.install_kind}") if check.missing: lines.append(f" missing from this interpreter: {', '.join(check.missing)}") + # Group identical errors: with e.g. a workspace-version mismatch, autofit, + # autogalaxy and autolens all fail with the same multi-paragraph message — + # print it once, naming every module it applies to. + grouped: dict[str, list[str]] = {} for name, error in check.errors.items(): - lines.append(f" {name} import failed: {error}") + grouped.setdefault(error, []).append(name) + for error, names in grouped.items(): + lines.append(f" {', '.join(names)} import failed: {error}") if check.cache_defaults: defaults = ", ".join( f"{name}={value}" for name, value in check.cache_defaults.items() @@ -1121,7 +1127,7 @@ def write_provenance(root: Path, only: Optional[list[Path]] = None) -> int: # --------------------------------------------------------------------------- # Code gate (--code / --file) # --------------------------------------------------------------------------- -def validate_source(text: str) -> tuple[int, list[str]]: +def validate_source(text: str) -> tuple[int, list[str], dict[str, str]]: """Resolve every alias-rooted symbol in raw Python `text` against the installed stack. @@ -1129,21 +1135,33 @@ def validate_source(text: str) -> tuple[int, list[str]]: version baseline), this is a cheap, version-independent gate over a snippet or a single file the agent is *about to run* — the place a stale symbol like `al.Kernel2D` or `aplt.FitImagingPlotter` actually crashes. Returns - `(n_stale, report_lines)`; `report_lines` is empty when everything resolves. + `(n_stale, report_lines, import_failed)`; `report_lines` is empty when every + resolvable symbol resolves, and `import_failed` maps each root module that + failed to import to a truncated first error — a broken/blocked stack is an + environment problem, never counted as symbol staleness. """ symbols = extract_symbols_code(text) lines: list[str] = [] n_stale = 0 + import_failed: dict[str, str] = {} for sym in sorted(symbols, key=lambda s: s.text): res = resolve(sym) if res.status == "ok": continue + if res.status == "import_failed": + # The root module didn't import — an environment problem, not API + # drift. Marking every symbol under it STALE conflates the two and + # repeats the (often multi-paragraph) import error once per symbol. + module = ALIAS_TO_MODULE[sym.alias] + err = res.error or "import failed" + if len(err) > 200: + err = err[:200] + "…" + import_failed.setdefault(module, err) + continue n_stale += 1 if res.status == "missing_attr": tail = ".".join(sym.chain[res.resolved_depth :]) status = f"not in installed stack (missing `.{tail}`)" - elif res.status == "import_failed": - status = f"import failed ({res.error})" else: status = f"error ({res.error})" # `candidates` are fuzzy/cross-module name matches, NOT a verified rename map @@ -1156,14 +1174,15 @@ def validate_source(text: str) -> tuple[int, list[str]]: "no close match — ground against skills/ or `dir()` of the live module" ) lines.append(f"STALE {sym.text} — {status}; {hint}") - return n_stale, lines + return n_stale, lines, import_failed def run_code_gate(*, code: str | None, file: str | None) -> int: """Validate a snippet (`--code`) or a single `.py` file (`--file`) and return an exit code: 0 = all symbols resolve, 2 = at least one stale symbol (distinct from - the report mode's 1). Self-contained — needs neither `sources.yaml` nor the - baseline, only the installed library.""" + the report mode's 1), 3 (= INSTALL_IMPORT_FAILED) = the stack itself failed to + import so no symbol judgement is possible. Self-contained — needs neither + `sources.yaml` nor the baseline, only the installed library.""" if code is not None: text, label = code, "<--code>" else: @@ -1173,7 +1192,7 @@ def run_code_gate(*, code: str | None, file: str | None) -> int: return 2 text, label = p.read_text(encoding="utf-8"), str(p) - n_stale, report = validate_source(text) + n_stale, report, import_failed = validate_source(text) # Idiom lint runs alongside symbol resolution: the gate must catch a dead # *construction* (`analysis + analysis`) as well as a dead *symbol*. Emit the @@ -1197,6 +1216,20 @@ def run_code_gate(*, code: str | None, file: str | None) -> int: for line in report: print(" " + line, file=sys.stderr) return 2 + if import_failed: + print( + f"[gate] cannot validate {label}: the PyAuto* stack failed to import — " + "an environment problem, not API drift:", + file=sys.stderr, + ) + for module, err in import_failed.items(): + print(f" {module}: {err}", file=sys.stderr) + print( + " Fix the environment first (skills/al_setup_environment.md); if this is " + "the workspace version check, set PYAUTO_SKIP_WORKSPACE_VERSION_CHECK=1.", + file=sys.stderr, + ) + return INSTALL_IMPORT_FAILED print(f"[gate] {label}: all PyAuto* symbols and idioms resolve.", file=sys.stderr) return 0 diff --git a/autoassistant/tests/test_api_gate.py b/autoassistant/tests/test_api_gate.py index 78b9db7..aae75ef 100644 --- a/autoassistant/tests/test_api_gate.py +++ b/autoassistant/tests/test_api_gate.py @@ -175,6 +175,14 @@ def test_hook_escape_hatch(): assert _decision(proc) is None +def test_hook_escape_hatch_command_prefix(): + # The documented per-command form: the env var is a prefix on the command itself, + # so it never reaches the hook's own environment — the hook must detect it in the + # command text. + proc = _run_hook(f"PYAUTO_SKIP_API_GATE=1 {sys.executable} -c '{STALE_PLOTTER}'") + assert _decision(proc) is None + + def test_hook_validates_py_file_argument(tmp_path): good = tmp_path / "good.py" good.write_text(f"import autolens.plot as aplt\n{GOOD}\n", encoding="utf-8") @@ -237,5 +245,44 @@ def test_lint_idioms_clean_tree(tmp_path): assert _run_validator("--lint-idioms", "--root", str(tmp_path)).returncode == 0 +# --- broken-stack handling (import failure is an env problem, not symbol drift) -------- +def _load_validator_module(): + import importlib.util + + spec = importlib.util.spec_from_file_location("audit_skill_apis_under_test", VALIDATOR) + mod = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = mod # dataclasses resolve fields via sys.modules + spec.loader.exec_module(mod) + return mod + + +def test_validate_source_reports_broken_stack_once_not_stale(): + mod = _load_validator_module() + wall = "WorkspaceVersionMismatchError('pinned != installed" + " x" * 200 + "')" + mod._module_cache["autolens"] = None + mod._import_errors["autolens"] = wall + + n_stale, lines, import_failed = mod.validate_source("al.Tracer; al.Galaxy; al.FitImaging") + + assert n_stale == 0 and lines == [] # broken stack is never "stale symbols" + assert list(import_failed) == ["autolens"] # reported once, not per symbol + assert len(import_failed["autolens"]) <= 201 # truncated, not the full wall + + +def test_render_installation_check_groups_identical_errors(): + mod = _load_validator_module() + check = mod.InstallationCheck( + status="import_failed", python="py", prefix="env", + versions={}, locations={}, missing=[], + errors={name: "Boom: identical wall" for name in ("autofit", "autogalaxy", "autolens")}, + install_kind="unknown", cache_defaults={}, + ) + + text = mod.render_installation_check(check) + + assert text.count("Boom: identical wall") == 1 + assert "autofit, autogalaxy, autolens import failed" in text + + if __name__ == "__main__": sys.exit(pytest.main([__file__, "-v"])) diff --git a/skills/al_audit_skill_apis.md b/skills/al_audit_skill_apis.md index 60c83a5..7b2d2f6 100644 --- a/skills/al_audit_skill_apis.md +++ b/skills/al_audit_skill_apis.md @@ -168,12 +168,18 @@ on a snippet or a file: ```bash python autoassistant/audit_skill_apis.py --code "import autolens as al; al.FitImagingPlotter" # exit 2 -python autoassistant/audit_skill_apis.py --file scripts/my_script.py # exit 0/2 +python autoassistant/audit_skill_apis.py --file scripts/my_script.py # exit 0/2/3 ``` +Exit 2 = stale symbol(s)/idiom(s) — the deny case. Exit 3 = the stack itself failed to +import (broken env, or the workspace version check), reported once as an environment +problem — the hook fails open on it, since the command would raise the same import +error itself. + When the gate blocks you, **do not guess a replacement** — grep `skills/` for the task or introspect `dir()` of the live module, then re-run. Escape hatch for deliberate -pre-refactor/debugging work: set `PYAUTO_SKIP_API_GATE=1`. +pre-refactor/debugging work: set `PYAUTO_SKIP_API_GATE=1` — in the environment, or as a +prefix on the command itself (`PYAUTO_SKIP_API_GATE=1 python …`). ## Ask — narrow the scope before fixing