diff --git a/active.md b/active.md index 3b4dcb22..cfc31d94 100644 --- a/active.md +++ b/active.md @@ -74,3 +74,26 @@ - repo-unclaimed: PyAutoReduce is the only affected repo, listed on this single line deliberately and NOT as a 2-space ` - PyAutoReduce` sub-bullet, because worktree_check_conflict reads any such bullet as a live claim — this task holds no claim and must not block other PyAutoReduce work. - ordering: `draft/research/pyautoreduce/acceptance_noise_rebaseline.md` must run AFTER this lands — a bits change moves the IVM weights and therefore the noise maps, so its parity numbers would need redoing otherwise. Related but distinct, do not fold in: #61 (driz_cr flux erosion / LACosmic) and #62 (tier-1 ePSF from the CR-rejected mosaic). - prompt: active/hst_dq_bits_dial.md + +## hygiene-coverage-drift +- issue: https://github.com/PyAutoLabs/PyAutoBrain/issues/197 +- status: library-dev — IMPLEMENTED AND PUSHED to `claude/hygiene-coverage-drift-kso7h1` (PyAutoBrain cb20ecf, PyAutoMind ea0ac81). NO PR opened (not requested). Cloud session: no worktree, no `gh` CLI; issue filed via the GitHub MCP surface. +- verified: PyAutoBrain suite 212 passed (39 pre-existing hygiene tests unchanged + 8 new); PyAutoMind suite 88 passed (9 new). `repos_sync.py --check` gains one passing line and nothing else — diffed against a stashed baseline to prove the 6 tenant-firewall mismatches it still reports are PRE-EXISTING PyAutoHands drift, not ours. +- measured effect (17 repos scanned, was 9): crlf 5 -> 167 cosmetic .py; deps 5 -> 8 pyproject.toml; docs 3 -> 4 repos with docs/api. +- MUTATION-TESTED, not just green: re-hardcoding LIB_REPOS to the stale five makes 2 of the new tests fail; a deliberately narrowed fallback regex makes the coverage check report 4 dropped repos; a re-hardcoded array trips leg B. Each leg was driven with failing input before being trusted. +- LEG A IS NARROWER THAN IT LOOKS, do not oversell it: the conductor reads the same repos.yaml the check does, so a manifest edit moves both sides together and they CANNOT desynchronise. Leg A really guards the READER — specifically the PyYAML-free fallback, used only where PyYAML is absent and verified nowhere else. Both readers are run via a `--parser` flag added for exactly that purpose. +- firewall entry for hygiene.sh DELETED rather than updated: the conductor now names no instance fact at all (verified by running the token scan over both files). Re-adding an entry would re-permit the drift the new check exists to catch. +- SCOPE CORRECTION made mid-implementation: the first cut marked EVERY mode `unscanned` on an empty root, which broke 20 tests and was wrong — docstrings/refs/optdeps/extras/config discover their own targets by walking the root and can legitimately find material the body map never names. `unscanned` is now scoped to the six repo-array modes (tidy/crlf/artifacts/deps/docs/packaging). A real finding still leads the recommendation, with the partial-ranking caveat appended. +- ALSO REVERTED mid-implementation: filtering `packaging` to repos with a pyproject.toml. It narrowed detection semantics for no benefit and broke a valid existing test; the existing ignore/untracked/depth guards already establish a hit. Left as a comment in the source so it is not re-attempted. +- ADJACENT DEFECT SPOTTED, deliberately NOT fixed (separate task): `HYGIENE_PERF_LIBS` still defaults to `autoconf ...`, the pre-rename name of the config package, so perf silently reports n/a for it. Same silent-under-report family, different defect — not repo-array coverage. +- follow-up owed: the widened scan exposes a real backlog (167 cosmetic CRLF, 122 in the CTI library which has an LF-only rule; 41 dep caps across 8 manifests). Triage is the separate task the prompt asked for and has NOT been filed yet. +- what it is: the hygiene conductor hardcodes `LIB_REPOS`/`ORG_REPOS`/`DOC_REPOS` in `agents/conductors/hygiene/hygiene.sh:64-66`. repos.yaml declares 6 libraries and 7 organs, so the conductor skips PyAutoCTI + PyAutoReduce, classes PyAutoNerves as a library where the manifest calls it an organ, and covers 4 of 7 organs. +- reproduced before planning (PYAUTO_ROOT=/home/user): `hygiene crlf` prints `5 library .py w/ CRLF`; `git -C PyAutoCTI grep -Il $'\r$' -- '*.py' | wc -l` = 122, so the true count for the current repo set is 127. deps audits 5 pyproject.toml of 6; tidy scans 9 of ~17 managed checkouts. +- why repos_sync can't catch it: the tenant-firewall entry for hygiene.sh (`scripts/repos_sync.py:525`) pins the drifted set as an ALLOWLIST, so the stale names are permitted rather than checked for coverage. The fix adds a real coverage check beside it. +- DESIGN TRAP, do not "simplify" it away: PyAutoNerves is `category: organ` but ships a pyproject.toml. A straight category mapping would move it out of LIB_REPOS and thereby DROP it from deps/packaging — trading one coverage hole for another. Those two modes key off "repo ships a pyproject.toml", not off the category. Same reasoning makes `docs` key off `docs/api/` presence (which adds PyAutoCTI). +- human decisions 2026-08-05: (1) derive the workspace set from `category: workspace` too, accepting +40 cosmetic CRLF from autocti_workspace (127 -> 167) so no repo name is left hardcoded; (2) deps/packaging cover any managed repo with a pyproject.toml. +- scope: COVERAGE REPAIR ONLY. Widening surfaces a large backlog of genuine findings (CRLF, artifacts, dep caps) — triaging that backlog is a separate task and no finding is fixed here. +- third defect folded in per the prompt: an empty/absent scan root currently reports `clean` across every array-driven mode with no warning. Adds an `unscanned` status + a banner naming the root, covering both "no repos present" and "body map unresolvable". +- prompt: active/hygiene_under_reports_debt_by_25x_because.md +- worktree: (none — cloud session, working in the canonical /home/user checkouts on the mandated branch) +- repos-claimed-on-one-line: PyAutoBrain (primary) and PyAutoMind, named here deliberately and NOT as 2-space ` - Repo` bullets, because worktree_check_conflict reads any such bullet as a live claim. diff --git a/active/hygiene_under_reports_debt_by_25x_because.md b/active/hygiene_under_reports_debt_by_25x_because.md new file mode 100644 index 00000000..d2977a97 --- /dev/null +++ b/active/hygiene_under_reports_debt_by_25x_because.md @@ -0,0 +1,14 @@ +# Hygiene under-reports debt by 25x because its repo arrays skip + +Type: bug +Target: PyAutoBrain +Repos: +- PyAutoBrain +Difficulty: medium +Autonomy: supervised +Priority: normal +Status: formalised + +Hygiene under-reports debt by 25x because its repo arrays skip two libraries. The hygiene conductor scans a hardcoded list of checkouts in PyAutoBrain agents/conductors/hygiene/hygiene.sh. That list is stale: the LIB_REPOS array holds five entries where the body map (repos.yaml) has six, silently skipping the CTI and Reduce libraries, and it mislabels the config layer as a library; ORG_REPOS covers four of seven organs. The result is wrong output, not stale prose. On a real run the crlf mode printed '5 library .py w/ CRLF' when the true count is 127 — 122 of them in the skipped CTI library, breaking that repo's LF-only rule with nobody watching. The deps mode audits five pyproject.toml instead of six; tidy inspects nine of roughly seventeen managed checkouts. Every clean bill of health the conductor has issued understates reality. This is an internal inconsistency, since the sibling scanners _hygiene_config.py and _hygiene_refs.py already reach the CTI library. The drift checker cannot catch the gap: its tenant-firewall entry for hygiene.sh pins the current broken set as an allowlist instead of verifying coverage. The repair should derive the arrays from the body map rather than re-hardcoding them. Widening coverage will surface a large backlog of genuine new findings, so land the coverage repair and the triage of what it uncovers as separate tasks. + + diff --git a/scripts/repos_sync.py b/scripts/repos_sync.py index 926a3b56..b324b9a8 100644 --- a/scripts/repos_sync.py +++ b/scripts/repos_sync.py @@ -30,6 +30,8 @@ * PyAutoHeart/config/repos.yaml — polled repos exist, owners match * PyAutoHands/pre_build.sh — run_workspace repos exist * PyAutoBrain/bin/ensure_workspace_labels.sh — owner/name pairs match + * the hygiene conductor — the repo sets it scans are derived from this + manifest, and no repo name has been hardcoded back into an array * the `origin` remote of every local checkout — manifest matches reality * the tenant firewall — no instance fact (satellite repo name, GitHub owner, workspace path) in Brain/Heart/Build *.py / *.sh outside the @@ -39,6 +41,8 @@ """ import argparse +import json +import os import re import subprocess import sys @@ -311,6 +315,86 @@ def check_pre_build(root, repos): ] +HYGIENE_DIR = "PyAutoBrain/agents/conductors/hygiene" +HYGIENE_SCRIPT = f"{HYGIENE_DIR}/hygiene.sh" +HYGIENE_HELPER = f"{HYGIENE_DIR}/_hygiene_repos.py" + +# The hygiene conductor scans repositories, so its repo sets must equal this +# manifest's. They used to be bash arrays, and they drifted: five libraries +# where the manifest declared six, four organs of seven. The drift was invisible +# because an unscanned repo yields no findings — the conductor reported clean and +# was believed. The tenant firewall could not catch it either; its allowlist +# PERMITTED the stale names rather than checking coverage. +# +# So this check has two legs, because either alone is escapable: +# +# A. every reader the conductor might use returns exactly the sets declared +# here. Note what this can and cannot prove: the conductor reads THIS file, +# so a manifest edit moves both sides together and can never desynchronise +# them — that is the whole point of deriving. What leg A really guards is +# the READER, and specifically the PyYAML-free fallback, which is used only +# where PyYAML is absent and would otherwise be verified nowhere. A +# fallback parser that quietly drops a repo is precisely this bug's class, +# so both readers are run and both must agree with the manifest. +# B. no repo name is written back into a *_REPOS=(...) array literal — what +# stops a future edit from "simplifying" the derivation away. +HYGIENE_ARRAY = re.compile(r"^[ \t]*[A-Za-z_]*REPOS=\(([^)]*)\)", re.M) +HYGIENE_CATEGORIES = ("library", "organ", "workspace") + + +def check_hygiene_coverage(root, repos, mind_root): + helper, script = root / HYGIENE_HELPER, root / HYGIENE_SCRIPT + if not helper.exists() or not script.exists(): + return [] # Brain not checked out in this environment + + problems = [] + for reader in ("auto", "minimal"): + result = subprocess.run( + [sys.executable, str(helper), "--json", "--parser", reader], + capture_output=True, + text=True, + env={**os.environ, "PYAUTO_MIND": str(mind_root)}, + ) + if result.returncode != 0: + problems.append( + f"{HYGIENE_HELPER} ({reader} reader): cannot read the body map " + f"(exit {result.returncode}) — the conductor would scan nothing: " + f"{result.stderr.strip()}" + ) + continue + try: + derived = json.loads(result.stdout) + except json.JSONDecodeError as exc: + problems.append( + f"{HYGIENE_HELPER} ({reader} reader): output is not JSON — {exc}" + ) + continue + for category in HYGIENE_CATEGORIES: + declared = {n for n, r in repos.items() if r["category"] == category} + seen = set(derived.get(category, [])) + for name in sorted(declared - seen): + problems.append( + f"hygiene ({reader} reader) does not scan '{name}' ({category}) " + f"— declared in the manifest but missing from the derived set" + ) + for name in sorted(seen - declared): + problems.append( + f"hygiene ({reader} reader) scans '{name}' ({category}) " + f"— not in the manifest" + ) + + for match in HYGIENE_ARRAY.finditer(script.read_text()): + hardcoded = sorted( + {tok.strip("\"'") for tok in match.group(1).split()} & set(repos) + ) + if hardcoded: + problems.append( + f"{HYGIENE_SCRIPT}: repo name(s) hardcoded in an array — " + f"{', '.join(hardcoded)}; derive them from the body map instead" + ) + return problems + + def check_labels(root, repos): script = root / "PyAutoBrain/bin/ensure_workspace_labels.sh" if not script.exists(): @@ -522,7 +606,10 @@ def write_claude_md_pointers(root, repos): "PyAutoBrain/agents/conductors/hygiene/_hygiene_config.py": {"PyAutoArray", "PyAutoCTI", "PyAutoFit", "PyAutoGalaxy", "PyAutoLabs", "PyAutoLens", "autofit_workspace", "autogalaxy_workspace", "autolens_workspace"}, "PyAutoBrain/agents/conductors/hygiene/_hygiene_optdeps.py": {"HowToFit", "HowToGalaxy", "HowToLens", "autocti_workspace", "autofit_workspace", "autogalaxy_workspace", "autolens_workspace"}, "PyAutoBrain/agents/conductors/hygiene/_hygiene_refs.py": {"PyAutoArray", "PyAutoCTI", "PyAutoFit", "PyAutoGalaxy", "PyAutoLens", "autolens_workspace"}, - "PyAutoBrain/agents/conductors/hygiene/hygiene.sh": {"PyAutoArray", "PyAutoNerves", "PyAutoFit", "PyAutoGalaxy", "PyAutoLabs", "PyAutoLens", "autofit_workspace", "autogalaxy_workspace", "autolens_workspace"}, + # hygiene.sh and _hygiene_repos.py carry NO entry on purpose: the conductor + # now derives its repo sets from the body map, so it names no instance fact + # at all. Re-adding an entry here would re-permit the drift that + # check_hygiene_coverage exists to catch. "PyAutoBrain/agents/conductors/clone/_clone.py": {"HowToFit", "PyAutoFit", "PyAutoLabs", "PyAutoLens", "autofit_assistant", "autofit_workspace", "autolens_assistant"}, "PyAutoBrain/agents/conductors/clone/clone.sh": {"HowToFit", "PyAutoFit", "autofit_workspace", "autolens_assistant"}, "PyAutoBrain/agents/conductors/community/_community.py": {"Jammy2211", "PyAutoLabs"}, @@ -751,6 +838,7 @@ def main(): "PyAutoHeart/config/repos.yaml": check_heart(root, repos), "PyAutoHands/pre_build.sh": check_pre_build(root, repos), "ensure_workspace_labels.sh": check_labels(root, repos), + "hygiene conductor coverage": check_hygiene_coverage(root, repos, mind_root), "local checkout origins": check_origins(root, repos), "tenant firewall (organ code)": check_tenant_firewall(root, repos), "organism-map blocks (generated)": check_map_blocks(root, repos, smap), diff --git a/tests/test_repos_sync_hygiene_coverage.py b/tests/test_repos_sync_hygiene_coverage.py new file mode 100644 index 00000000..9f67bd59 --- /dev/null +++ b/tests/test_repos_sync_hygiene_coverage.py @@ -0,0 +1,153 @@ +"""The hygiene conductor's repo coverage must not drift from the body map. + +The conductor once held its scanned repos as bash arrays. They drifted — five +libraries where the map declared six, four organs of seven — and nothing caught +it, because a repo that is never scanned produces no findings and so reads as a +clean bill of health. The tenant firewall could not catch it either: its entry +for the conductor ALLOWLISTED the stale names rather than checking coverage. + +`check_hygiene_coverage` is the check that closes that gap, and these are its +contract tests. Two things they deliberately do: + +1. **Fictional fixtures only.** `tests/**` is KEEP-copied verbatim into the + public template (see `test_spawn_privacy.py`), so nothing here names a real + repository. It also keeps the tests hermetic — they assert the check's logic, + not the state of whatever happens to be checked out. +2. **Prove each leg FAILS.** A drift check that cannot fail is decoration. Every + leg below is driven with input that must trip it. +""" + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "scripts")) + +import repos_sync # noqa: E402 + +HYGIENE_REL = "PyAutoBrain/agents/conductors/hygiene" + +# A fictional organism: two libraries, one organ, one workspace. +MANIFEST = { + "LibAlpha": {"category": "library"}, + "LibBeta": {"category": "library"}, + "OrganCore": {"category": "organ"}, + "wsalpha_workspace": {"category": "workspace"}, +} + +DERIVED_OK = { + "library": ["LibAlpha", "LibBeta"], + "organ": ["OrganCore"], + "workspace": ["wsalpha_workspace"], +} + +# A conductor that derives its sets — no repo name in any array literal. +CLEAN_SCRIPT = """\ +#!/usr/bin/env bash +mapfile -t LIB_REPOS < <(_body_map library) +mapfile -t ORG_REPOS < <(_body_map organ) +CODE_REPOS=("${LIB_REPOS[@]}" "${ORG_REPOS[@]}") +""" + + +def _stub_helper(derived, exit_code=0): + """A stand-in for _hygiene_repos.py that prints whatever we hand it.""" + return ( + "import json, sys\n" + f"if {exit_code}:\n" + f" sys.exit({exit_code})\n" + f"print(json.dumps({derived!r}))\n" + ) + + +def _tree(tmp_path, *, derived=None, script=CLEAN_SCRIPT, exit_code=0, helper=True): + hygiene = tmp_path / HYGIENE_REL + hygiene.mkdir(parents=True) + (hygiene / "hygiene.sh").write_text(script) + if helper: + (hygiene / "_hygiene_repos.py").write_text( + _stub_helper(DERIVED_OK if derived is None else derived, exit_code) + ) + mind = tmp_path / "PyAutoMind" + mind.mkdir() + return tmp_path, mind + + +def _check(tmp_path, **kwargs): + root, mind = _tree(tmp_path, **kwargs) + return repos_sync.check_hygiene_coverage(root, MANIFEST, mind) + + +def test_a_conductor_deriving_the_declared_sets_is_clean(tmp_path): + assert _check(tmp_path) == [] + + +def test_a_repo_missing_from_the_derived_set_is_drift(tmp_path): + # The original bug: a declared library the conductor never scans. + thinned = {**DERIVED_OK, "library": ["LibAlpha"]} + + problems = _check(tmp_path, derived=thinned) + + assert problems + assert any("does not scan 'LibBeta'" in p for p in problems) + + +def test_a_repo_the_manifest_does_not_declare_is_drift(tmp_path): + padded = {**DERIVED_OK, "organ": ["OrganCore", "OrganGhost"]} + + problems = _check(tmp_path, derived=padded) + + assert any("scans 'OrganGhost'" in p for p in problems) + + +def test_both_readers_are_exercised_so_the_pyyaml_free_path_cannot_rot(tmp_path): + # The fallback runs only where PyYAML is absent, so nothing else verifies it. + # Each reader is invoked separately and each is reported by name. + problems = _check(tmp_path, derived={**DERIVED_OK, "library": []}) + + assert any("(auto reader)" in p for p in problems) + assert any("(minimal reader)" in p for p in problems) + + +def test_a_hardcoded_repo_name_in_an_array_is_drift(tmp_path): + # Leg B: what stops a future edit from "simplifying" the derivation away. + script = 'LIB_REPOS=(LibAlpha LibBeta)\nORG_REPOS=(OrganCore)\n' + + problems = _check(tmp_path, script=script) + + assert any("hardcoded in an array" in p and "LibAlpha" in p for p in problems) + + +def test_an_array_built_from_other_arrays_is_not_drift(tmp_path): + # Composition is how the derived sets are assembled; it must stay legal. + script = 'SCAN_REPOS=("${LIB_REPOS[@]}" "${WS_REPOS[@]}")\n' + + assert _check(tmp_path, script=script) == [] + + +def test_a_helper_that_cannot_read_the_body_map_is_drift(tmp_path): + # Exiting non-zero means the conductor would scan nothing at all — the + # loudest possible version of this bug, and it must not pass silently. + problems = _check(tmp_path, exit_code=3) + + assert problems + assert any("cannot read the body map" in p for p in problems) + + +def test_the_check_skips_when_the_brain_is_not_checked_out(tmp_path): + # Partial/web checkouts are normal; missing organs are skipped, never failed. + mind = tmp_path / "PyAutoMind" + mind.mkdir() + + assert repos_sync.check_hygiene_coverage(tmp_path, MANIFEST, mind) == [] + + +def test_the_real_conductor_matches_the_real_body_map(): + """The live tree, if it is checked out here.""" + mind_root = Path(__file__).resolve().parents[1] + root = mind_root.parent + if not (root / HYGIENE_REL / "hygiene.sh").exists(): + return # Brain not present in this environment + _, repos = repos_sync.load_manifest(mind_root) + + assert repos_sync.check_hygiene_coverage(root, repos, mind_root) == []