diff --git a/.claude/commands/upgrade.md b/.claude/commands/upgrade.md index ef1cf86..d624b67 100644 --- a/.claude/commands/upgrade.md +++ b/.claude/commands/upgrade.md @@ -46,8 +46,8 @@ kit's copy against this repo: `uv run /tmp/agentic-dev-kit/scripts/kit_doctor.py --manifest /tmp/agentic-dev-kit/kit-manifest.json`. The report gives you, per kit-owned file: `unchanged` / `differs` / `missing` / -`unknown-version`, plus four installation-level checks. **Read all four** — each is a -silent failure mode: +`missing-required` / `unknown-version`, plus four installation-level checks. +**Read all four** — each is a silent failure mode: - **config schema version** — unversioned or behind means migrations are pending. - **`paths.engines` resolves to a directory that actually holds engines.** A `✗` here is @@ -112,12 +112,36 @@ Work through `kit_doctor`'s file list. You are already on the branch from Step 2 `init.sh` refreshed itself and migrated the config there, so those changes are captured too. Confirm with `git branch --show-current` before the first copy. +**Install every `missing-required` file first, before any other copy in this step.** +Those are the kit's own libraries — `lib/kitconfig.py` above all, which every Python +engine imports — and refreshing a component on top of an absent one produces a broken +install: `check_doc_budget.py` dies with `ModuleNotFoundError`, and `pr_watch.py` warns +and silently falls back to built-in defaults, leaving the adopter's entire `review.*` +config inert. `kit_doctor` derives this set from the Python import graph, so it is +answering "what do *this* tree's installed components need", not a fixed list. + +**Then re-run `kit_doctor` after installing anything.** The set is computed against the +components present *when the report ran*: a file is `missing-required` only if something +that depends on it is already installed. So installing a previously-`missing` engine or +hook can introduce requirements the first report had no reason to classify. Re-run +before you rely on the list again, and treat the report as converged only when a run +that installed nothing still shows no `missing-required`. + +- **`missing-required`** → install it. This is the one absent-file case that is **not** + an operator decision: an installed component depends on it, and the report names + which. Do not carry it into the `missing` conversation below. - **`unchanged`** → copy the new version straight in. It is provably untouched, so there is nothing to lose. - **`missing`** → decide, don't assume. A sized-down adoption omits engines deliberately (one surveyed repo installs 2 of 6 on purpose). Ask the operator whether each missing piece is wanted before installing it. If a piece stays out, note it in the PR body so - the next upgrade doesn't re-litigate it. + the next upgrade doesn't re-litigate it. Nothing installed here depends on these **by + the graph `kit_doctor` derives**, which is what separates them from the bullet above. + That graph covers **Python imports only** — it does not read shell `source`, so + `lib/repo_root.sh` (which `dev_session.sh` and `reconcile_sessions.sh` both source) + will appear here rather than above. It is a much better prior than the old blanket + "decide, don't assume", not a proof: if a piece you are declining is a library a + shell component plausibly reaches for, check before dropping it. - **`differs`** → `diff` the local file against the kit's, and read the diff: - Only kit-authored changes (the local copy is simply older) → replace it. - Local edits present → for each, find where that value now lives in diff --git a/kit-manifest.json b/kit-manifest.json index 31a7a12..97b30c4 100644 --- a/kit-manifest.json +++ b/kit-manifest.json @@ -96,9 +96,12 @@ }, "scripts/kit_doctor.py": { "role": "engine", - "sha256": "78ff1093fb5f243b05d0f32b7946552883322f4869bd42730fee3315867d4605" + "sha256": "a70626b4f784f56ec683d2239150a2e39ed866ff0f28f599cc877c4abbf13b6f" }, "scripts/lib/atomic_write.py": { + "required_by": [ + "scripts/archive_plan_sessions.py" + ], "role": "engine", "sha256": "f9657f36f8bbcf3741382b616ae19be19cf516fc5fa4f36c53384579e96202b3" }, @@ -107,6 +110,16 @@ "sha256": "a4c29fb1808e0b3ec0a2ba80ffa97ba6c894f1a784463a5d126169542c300ef2" }, "scripts/lib/kitconfig.py": { + "required_by": [ + "scripts/archive_plan_sessions.py", + "scripts/check_doc_budget.py", + "scripts/check_memory_budget.py", + "scripts/hooks/pr_followup_hook.py", + "scripts/hooks/pre-push", + "scripts/kit_doctor.py", + "scripts/panel_prompt.py", + "scripts/pr_watch.py" + ], "role": "engine", "sha256": "4ab496661883d8f4ad590a6612a48b31f8cbf770283bb09794096149276634e6" }, @@ -119,14 +132,23 @@ "sha256": "691a7df06a3bf9019b5faca8b8ebdb9d54463d231d492a632fd54c16cd51a2c6" }, "scripts/lib/state_paths/paths.py": { + "required_by": [ + "scripts/lib/state_paths/resolver.py" + ], "role": "engine", "sha256": "f54fd7d970402f8581f6c5080aa9614ce41cb2dafa2010c08406129731799397" }, "scripts/lib/state_paths/repo_root.py": { + "required_by": [ + "scripts/lib/state_paths/resolver.py" + ], "role": "engine", "sha256": "d9412f1fe395b944a86126bcc39f85d6838382b0fc29d68c8b29b965cd530b5a" }, "scripts/lib/state_paths/resolver.py": { + "required_by": [ + "scripts/lib/state_paths/__init__.py" + ], "role": "engine", "sha256": "e39fca241d6fbf28596ed3954f28a55e65831181cedb02c7c15e372895bb5bd2" }, diff --git a/scripts/kit_doctor.py b/scripts/kit_doctor.py index 3974c1c..7c7b3db 100755 --- a/scripts/kit_doctor.py +++ b/scripts/kit_doctor.py @@ -27,8 +27,21 @@ schema version, and either way the required action is the same: diff before replacing, never clobber. ``missing`` - Not installed. Either a deliberately sized-down adoption or an incomplete - one; the report can't tell, so it says so rather than guessing. + Not installed, and nothing installed here needs it. Either a deliberately + sized-down adoption or an incomplete one; the report can't tell, so it says + so rather than guessing. +``missing-required`` + Not installed, but an engine that IS installed needs it — so this install + is broken, not sized down. The distinction exists because the old report had + only ``missing``, which files a hard dependency under "sized-down adoption, + or incomplete" alongside `docs/templates/*.tmpl`, which genuinely are + optional. `/upgrade` then tells the operator that a missing piece may be a + deliberate omission and to ask before installing it — so the documented path + invited someone to decline `lib/kitconfig.py`, which every Python engine + imports (issue #41). Which files these are is DERIVED at + ``--generate-manifest`` time from the PYTHON import graph, not restated by + hand. Shell ``source`` is deliberately NOT scanned — see + `derive_dependencies` for the gap that leaves and why it is left open. ``unknown-version`` The manifest has no entry for this file, so drift can't be judged. @@ -50,7 +63,10 @@ Exit codes: 0 — every kit-owned file is `unchanged` (or intentionally absent) - 1 — at least one file `differs` or is `unknown-version` + 1 — at least one file `differs`, is `unknown-version`, or is + `missing-required`. The last one is not drift, but it is a broken + install, and the exit code an adopter gates CI on should not be green + for a tree whose engines cannot load their own library. 2 — usage error (no config, no manifest, unreadable input) — including a `kit.version` that is present but not a number. That is deliberately NOT a warning-and-exit-0: CI gates on this exit code, and a config the @@ -60,11 +76,13 @@ from __future__ import annotations import argparse +import ast import hashlib import json +import re import sys from dataclasses import dataclass, field -from pathlib import Path +from pathlib import Path, PurePosixPath sys.path.insert(0, str(Path(__file__).resolve().parent / "lib")) from kitconfig import get, load_config, repo_root # noqa: E402 @@ -191,6 +209,7 @@ # adopter's configured engines directory. KIT_ENGINE_PREFIX = "scripts" + def _derive_engine_names(kit_owned: tuple[tuple[str, str], ...]) -> tuple[str, ...]: """Engine paths as they sit UNDER `paths.engines`, derived from KIT_OWNED. @@ -225,6 +244,156 @@ def _derive_engine_names(kit_owned: tuple[tuple[str, str], ...]) -> tuple[str, . _ENGINE_NAMES: tuple[str, ...] = _derive_engine_names(KIT_OWNED) +# Import statements in a file Python cannot parse. Applied ONLY to `engine` and +# `hook` roles, never to docs or templates: markdown cannot import anything, and +# a doctrine file quoting `from kitconfig import get` inside a code fence would +# otherwise manufacture a dependency edge out of prose. Its user is +# `scripts/hooks/pre-push`, a bash file whose kitconfig import lives inside a +# `python3 - <<'PY'` heredoc. +_TEXT_IMPORT_RE = re.compile(r"^\s*(?:from|import)\s+([A-Za-z_][A-Za-z0-9_.]*)", re.MULTILINE) + + +# Bash `source` / `.` of another kit file. A shell engine's dependency is a PATH, +# not a module name, so it needs its own scanner — and without one the graph +# fails open on exactly the pair #41 is about: `dev_session.sh` and +# `reconcile_sessions.sh` both `source "$SCRIPT_DIR/lib/repo_root.sh"`, so a tree +# missing `lib/repo_root.sh` reported `0 missing`-that-matters and exit 0 while +# `bash scripts/dev_session.sh` died on line 63. The Python-only version of this +# function shipped that hole and the /upgrade wording built on it went further +# than the text it replaced — "declining one is safe" — which is a worse false +# assurance than the "decide, don't assume" it superseded. Found by the +# adversarial lens on PR #225. +def _imported_modules(text: str) -> set[tuple[int, str]]: + """``(relative_level, dotted_name)`` for every import in `text`. + + `ast` first, and the choice matters rather than being stylistic: two library + modules here open with a usage example in the module docstring — the literal + line ``from devmodel_config import get, load_config, resolve_path`` sits in + `devmodel_config.py`'s own docstring. A text scan reads that as an import + and marks the module required by itself; `ast` sees a string constant. The + regex is the fallback for files Python cannot parse at all. + """ + modules: set[tuple[int, str]] = set() + try: + tree = ast.parse(text) + except SyntaxError: + return {(0, m.group(1)) for m in _TEXT_IMPORT_RE.finditer(text)} + for node in ast.walk(tree): + if isinstance(node, ast.Import): + modules.update((0, alias.name) for alias in node.names) + elif isinstance(node, ast.ImportFrom): + if node.module: + modules.add((node.level, node.module)) + elif node.level: + # `from . import resolver` — the imported NAME is the module. + modules.update((node.level, alias.name) for alias in node.names) + return modules + + +def _module_targets(module: str, package_dir: str) -> tuple[str, ...]: + """Kit-layout paths a dotted module name could resolve to under `package_dir`.""" + base = "/".join(module.split(".")) + return (f"{package_dir}/{base}.py", f"{package_dir}/{base}/__init__.py") + + +def derive_dependencies( + root: Path, kit_owned: tuple[tuple[str, str], ...] = KIT_OWNED +) -> dict[str, list[str]]: + """Map each kit-owned file to the kit-owned files that import it. + + This is the ``required`` axis #41 asks for, DERIVED rather than declared — + for the same reason `_derive_engine_names` is derived (#59, #67): a + hand-maintained list of hard dependencies goes stale exactly when a new + engine starts importing something, which is the moment it needed to be + right. A boolean would also be less true than this mapping. "Required" is + not a property of a file, it is a property of a PAIR: `lib/kitconfig.py` + matters to a repo that installed an engine and does not matter to one that + installed none, and both are supported adoptions. + + Resolution mirrors what the engines actually do at run time: every Python + one does ``sys.path.insert(0, /"lib")`` before importing, so an + absolute import resolves against ``scripts/lib``. Relative imports resolve + against the importing file's own package, which is what keeps the + `state_paths` package's internal edges (``from .resolver import …``) in the + graph rather than only its top-level name. + + **Shell `source` is NOT scanned, and this is the one real gap.** + `dev_session.sh` and `reconcile_sessions.sh` both + `source "$SCRIPT_DIR/lib/repo_root.sh"` — a hard dependency expressed as a + path rather than a module — so a tree missing `lib/repo_root.sh` reports it + as an ordinary `missing` while `bash scripts/dev_session.sh` dies on its + `source` line. That is a live instance of #41's own bug class and it is + knowingly left open; #228 carries it. + + A scanner for it was built across three review rounds and withdrawn. The + short version: preventing FALSE edges requires recognising every real + heredoc opener, and preventing MISSED edges requires knowing whether a + `source` is in command position — both are tokenizer problems, and each + regex approximation leaked in one direction or the other. A false edge is + the harmful direction (it tells an adopter their working install is broken), + and shipping a mechanism that can produce them is worse than shipping the + Python-only graph plus a documented hole. #228 has the full account, + including the four constructs that defeated the last version. + + Non-Python files ARE still scanned for Python-style imports: `pre-push` is + bash, and its `from kitconfig import …` lives inside a `python3 - <<'PY'` + heredoc that genuinely executes. + + LIMITS, because a graph that overstates its coverage is worse than a short + one: + + - Only `engine` and `hook` roles are scanned; see `_TEXT_IMPORT_RE`. + - Only imports resolving to a KIT_OWNED path are recorded. Third-party and + stdlib imports are not this function's business, and an engine importing + an UNTRACKED kit file produces no edge at all — that is #37's class, not + one this can see. + - A dynamic import (``importlib``, an ``__import__`` call) is invisible, as + is a `source` whose path is computed at run time. No file THIS FUNCTION + SCANS does either today; the qualifier is load-bearing, because several + TEST modules do use `importlib.util`, and they are neither `engine` nor + `hook` so they are out of scope. A future ENGINE that did either would get + a silently thin graph, and no test here would catch it. + + This sentence has now been wrong twice — first claiming "nothing in the + tree does this", then naming three test modules when there are four — so + it no longer enumerates them. `grep -rln importlib scripts/` is the + answer, and unlike a list in a docstring it cannot go stale. + (Correctness lens, PR #225 rounds 1 and 2.) + - Shell `source` is not scanned at all, so `lib/repo_root.sh`'s two + dependents are absent from the graph. Stated above; #228 carries it. + - Every one of these bounds errs toward NO edge. That direction is chosen, + not incidental: a missing edge degrades a file to plain `missing`, which + is the pre-#41 behaviour and merely unhelpful, while a false edge tells an + adopter their working install is broken and to install something they do + not need. `missing-required` firing wrongly is worse than it not firing, + and that asymmetry is what decided the shell-scan question above. + """ + owned = {rel for rel, _ in kit_owned} + dependents: dict[str, set[str]] = {} + + def record(target: str, importer: str) -> None: + if target in owned and target != importer: + dependents.setdefault(target, set()).add(importer) + + for rel, role in kit_owned: + if role not in ("engine", "hook"): + continue + source = root / rel + if not source.is_file(): + continue + text = source.read_text(encoding="utf-8", errors="replace") + for level, module in _imported_modules(text): + if level: + package = PurePosixPath(rel).parent + for _ in range(level - 1): + package = package.parent + package_dir = str(package) + else: + package_dir = f"{KIT_ENGINE_PREFIX}/lib" + for candidate in _module_targets(module, package_dir): + record(candidate, rel) + return {path: sorted(deps) for path, deps in sorted(dependents.items())} + @dataclass class FileStatus: @@ -266,6 +435,16 @@ def drifted(self) -> list[FileStatus]: def missing(self) -> list[FileStatus]: return [f for f in self.files if f.state == "missing"] + @property + def broken(self) -> list[FileStatus]: + """Files an installed engine needs and that are not installed. + + Deliberately NOT folded into `drifted`: a file that is absent has not + drifted from anything, and this report's whole position is that it does + not claim more than it knows. They meet only at the exit code. + """ + return [f for f in self.files if f.state == "missing-required"] + def sha256_of(path: Path) -> str: digest = hashlib.sha256() @@ -399,7 +578,18 @@ def generate_manifest(root: Path, kit_version: int) -> dict: absent here is recorded as ``None`` rather than skipped, so a packaging mistake shows up as an explicit hole instead of silently narrowing what later gets checked. + + ``required_by`` is written only where the derived dependent set is non-empty + — most kit files are needed by nothing, and an entry per file would be + twenty-six empty lists to read past in every manifest diff (32 KIT_OWNED + entries, 6 with a dependent; the earlier figure of "thirty" was a guess and + the correctness lens on PR #225 computed the real one). A reader must + therefore treat an ABSENT key as "no known dependents", which is also what + an older manifest (written before this field existed) yields: it reports + every missing file as an ordinary `missing`, exactly as it did before. + Degrading to the previous behaviour is the intended failure mode. """ + dependents = derive_dependencies(root) files: dict[str, dict] = {} for rel, role in KIT_OWNED: target = root / rel @@ -407,6 +597,8 @@ def generate_manifest(root: Path, kit_version: int) -> dict: "sha256": sha256_of(target) if target.is_file() else None, "role": role, } + if dependents.get(rel): + files[rel]["required_by"] = dependents[rel] return { "kit_version": kit_version, "files": files, @@ -418,6 +610,11 @@ def inspect(root: Path, manifest: dict, config: dict) -> Report: engines_dir = str(get(config, "paths.engines", KIT_ENGINE_PREFIX)) manifest_files = manifest.get("files") or {} + # Presence of EVERY kit-owned file, resolved before the status loop: whether + # a missing file is a broken install or a sized-down one is a question about + # its dependents, and those can sit anywhere in KIT_OWNED order. + present = {rel: (root / _remap(rel, engines_dir)).is_file() for rel, _ in KIT_OWNED} + statuses: list[FileStatus] = [] for rel, role in KIT_OWNED: local_rel = _remap(rel, engines_dir) @@ -425,12 +622,21 @@ def inspect(root: Path, manifest: dict, config: dict) -> Report: entry = manifest_files.get(rel) or {} expected = entry.get("sha256") if not target.is_file(): - statuses.append(FileStatus(local_rel, role, "missing")) + # Filtered to INSTALLED dependents. A repo that installed no engine + # is a supported sized-down adoption and must not be told its + # missing library breaks it — the same distinction `_ENGINE_NAMES` + # exists to keep the engines probe from getting wrong (#59). + needed_by = [dep for dep in (entry.get("required_by") or []) if present.get(dep)] + if needed_by: + names = ", ".join(PurePosixPath(dep).name for dep in needed_by) + statuses.append( + FileStatus(local_rel, role, "missing-required", f"needed by {names}") + ) + else: + statuses.append(FileStatus(local_rel, role, "missing")) continue if expected is None: - statuses.append( - FileStatus(local_rel, role, "unknown-version", "no manifest entry") - ) + statuses.append(FileStatus(local_rel, role, "unknown-version", "no manifest entry")) continue actual = sha256_of(target) if actual == expected: @@ -552,11 +758,20 @@ def render(report: Report) -> str: for f in report.files: by_state.setdefault(f.state, []).append(f) + # `missing` counts BOTH absent states, so the total does not change meaning + # for anyone reading this line as "how much of the kit is not here"; the + # parenthetical is what says how much of that absence is breakage. Rendered + # only when non-zero, so a healthy install's summary line is unchanged. + n_required_missing = len(by_state.get("missing-required", [])) + n_absent = len(by_state.get("missing", [])) + n_required_missing + absent_note = ( + f" ({n_required_missing} required by an installed engine)" if n_required_missing else "" + ) lines.append("") lines.append( f" files: {len(by_state.get('unchanged', []))} unchanged, " f"{len(by_state.get('differs', []))} differ, " - f"{len(by_state.get('missing', []))} missing, " + f"{n_absent} missing{absent_note}, " f"{len(by_state.get('unknown-version', []))} unknown" ) # Narrow "differs" using the schema version rather than asserting a cause. @@ -598,6 +813,14 @@ def render(report: Report) -> str: "diff before replacing" ) for state, label in ( + # First, and phrased to leave no room for the "deliberate omission" + # reading: this is the one absent-file case where the answer is not + # "decide, don't assume" but "install it". + ( + "missing-required", + "✗ NOT INSTALLED, and needed by an engine that is — this install is " + "broken, not sized down. Install these before refreshing any engine", + ), ("differs", differs_label), ("unknown-version", "no manifest entry — drift cannot be judged"), ("missing", "not installed (sized-down adoption, or incomplete)"), @@ -687,11 +910,15 @@ def main(argv: list[str] | None = None) -> int: # a manifest git TRACKS. Tracked is the property that makes that work, not # location: `--manifest` and `--root` can both aim at an untracked path, # inside the repo or outside it. - manifest_path.write_text(json.dumps(manifest, indent=2, sort_keys=True) + "\n", encoding="utf-8") + manifest_path.write_text( + json.dumps(manifest, indent=2, sort_keys=True) + "\n", encoding="utf-8" + ) holes = [p for p, e in manifest["files"].items() if e["sha256"] is None] print(f"wrote {manifest_path} ({len(manifest['files'])} files, kit_version={version})") if holes: - print(f"warning: {len(holes)} listed file(s) absent from this checkout:", file=sys.stderr) + print( + f"warning: {len(holes)} listed file(s) absent from this checkout:", file=sys.stderr + ) for h in holes: print(f" · {h}", file=sys.stderr) return 0 @@ -742,7 +969,7 @@ def main(argv: list[str] | None = None) -> int: # the adopter sees the ⚠ line, then a non-zero status so CI cannot go # green on a config this very run called UNREADABLE. return 2 - return 1 if report.drifted else 0 + return 1 if report.drifted or report.broken else 0 if __name__ == "__main__": diff --git a/scripts/tests/test_kit_doctor.py b/scripts/tests/test_kit_doctor.py index 8ae6890..d3c9a65 100644 --- a/scripts/tests/test_kit_doctor.py +++ b/scripts/tests/test_kit_doctor.py @@ -58,7 +58,9 @@ def _manifest(entries: dict[str, str | None], version: int = 2) -> dict: def test_remap_follows_configured_engines_dir(): assert kit_doctor._remap("scripts/pr_watch.py", "scripts") == "scripts/pr_watch.py" - assert kit_doctor._remap("scripts/pr_watch.py", "scripts/devkit") == "scripts/devkit/pr_watch.py" + assert ( + kit_doctor._remap("scripts/pr_watch.py", "scripts/devkit") == "scripts/devkit/pr_watch.py" + ) assert ( kit_doctor._remap("scripts/lib/state_paths/resolver.py", "tools/devkit") == "tools/devkit/lib/state_paths/resolver.py" @@ -441,7 +443,9 @@ def test_a_manifest_without_a_usable_version_is_not_a_silent_checkmark( report = kit_doctor.inspect(root, manifest, config) assert report.kit_version_manifest is None rendered = kit_doctor.render(report) - assert f"manifest kit_version is {shown} — cannot tell whether this config is behind" in rendered + assert ( + f"manifest kit_version is {shown} — cannot tell whether this config is behind" in rendered + ) assert "cannot narrow this" in rendered assert "LOCAL EDITS" not in rendered assert "OLDER version" not in rendered @@ -583,4 +587,297 @@ def test_generate_manifest_refuses_an_unreadable_version(tmp_path, capsys): def test_remap_tolerates_a_trailing_slash(): - assert kit_doctor._remap("scripts/pr_watch.py", "scripts/devkit/") == "scripts/devkit/pr_watch.py" + assert ( + kit_doctor._remap("scripts/pr_watch.py", "scripts/devkit/") == "scripts/devkit/pr_watch.py" + ) + + +# --- the required-by axis (#41) ------------------------------------------- +# +# The bug these pin: `missing` filed a hard dependency under "sized-down +# adoption, or incomplete", and /upgrade tells the operator that such a piece +# may be a deliberate omission and to ask before installing it. So the +# documented path invited someone to decline `lib/kitconfig.py`, which every +# Python engine imports. + + +def _tree(tmp_path: Path, files: dict[str, str]) -> Path: + for rel, text in files.items(): + _write(tmp_path / rel, text) + return tmp_path + + +def test_dependency_graph_of_the_real_kit_names_kitconfigs_importers(): + """Measured against the kit's own tree, not a fixture. + + What this pins that a fixture cannot: `scripts/hooks/pre-push` is BASH, and + its kitconfig import lives inside a `python3 - <<'PY'` heredoc, so an + ast-only scan drops it; and the equality assertion below fails on any NEW + importer the derivation misses, which is the staleness the whole axis exists + to prevent. + + **What it does NOT pin, corrected from an earlier version of this docstring + that claimed otherwise.** The `devmodel_config.py` assertion looks like it + proves `ast` beats a text scan on a docstring usage example — that module's + docstring opens with the literal line `from devmodel_config import get, + load_config, resolve_path`. It does not: that example names the module's own + file, so `derive_dependencies`'s `candidate != rel` self-loop guard drops it + whether ast or the regex produced the match. Forcing the regex branch leaves + this test GREEN. The ast-vs-prose property is real and IS pinned — by + `test_a_docstring_usage_example_is_not_an_import`, whose fixture names a + DIFFERENT module and so is not rescued by the self-loop guard. The assertion + is kept here as a cheap regression check on the graph's shape, not as + evidence of the mechanism. Found by the correctness lens on PR #225. + """ + graph = kit_doctor.derive_dependencies(REPO_ROOT) + importers = set(graph.get("scripts/lib/kitconfig.py", [])) + assert "scripts/hooks/pre-push" in importers, "the bash heredoc import was dropped" + assert "scripts/pr_watch.py" in importers, "a function-level import was dropped" + assert "scripts/lib/devmodel_config.py" not in graph, "a docstring example became an edge" + # Every engine that imports it, by the kit's own grep. Equality, not a + # subset: a NEW importer that this misses is exactly the staleness the + # derivation exists to prevent. + assert importers == { + "scripts/archive_plan_sessions.py", + "scripts/check_doc_budget.py", + "scripts/check_memory_budget.py", + "scripts/hooks/pr_followup_hook.py", + "scripts/hooks/pre-push", + "scripts/kit_doctor.py", + "scripts/panel_prompt.py", + "scripts/pr_watch.py", + } + + +def test_importing_a_package_by_name_resolves_to_its_init(tmp_path): + """The `/__init__.py` candidate in `_module_targets`. Nothing in the kit + imports a KIT_OWNED package by bare name today, so deleting that candidate + left the whole suite green — flagged by the adversarial lens on PR #225 as + an untested resolution rule, and pinned here rather than left to a future + engine to discover.""" + root = _tree( + tmp_path, + { + "scripts/engine.py": "import pkg\n", + "scripts/lib/pkg/__init__.py": "value = 1\n", + }, + ) + owned = (("scripts/engine.py", "engine"), ("scripts/lib/pkg/__init__.py", "engine")) + assert kit_doctor.derive_dependencies(root, owned) == { + "scripts/lib/pkg/__init__.py": ["scripts/engine.py"] + } + + +def test_the_shell_source_dependency_is_a_KNOWN_GAP_not_an_oversight(): + """`dev_session.sh` and `reconcile_sessions.sh` both + `source "$SCRIPT_DIR/lib/repo_root.sh"`, and that edge is deliberately NOT + in the graph. Asserted so the gap stays stated. + + A scanner for it was built and withdrawn across three review rounds. Both + directions failed: preventing FALSE edges needs every real heredoc opener + recognised (`cmd <