fix(launcher): read deps entry owners from the scratch RECORD files (#575) - #577

ZETETIC-REVIEW: REQUEST_CHANGES

Stakes: High (PR size 514 additions/16 deletions across 9 files, over the 400-line threshold; touches the bootstrap installer path every install-plugin.sh/setup.sh/setup.py run goes through, per ADR-1063). Reviewed the diff plus surrounding context (launcher_deps_fs.py, ADR-1064, ADR-1061, issue #575, the prior #576 review), and ran the repo's own gates from the worktree.

Blocking

• scripts/launcher_deps_install.py:144-187 (_commit_resolved_entries): new craftsmanship-gate violation, method-size (repo rule: 40 lines/method, docs/agent-guidance.md / CLAUDE.md "Craftsmanship gate"). Demonstrated:
  • python3 scripts/check_craftsmanship.py --base origin/main on this branch (head 17dcc87) exits 1 with [method-size] scripts/launcher_deps_install.py: _commit_resolved_entries.
  • AST span check confirms this is newly introduced by this diff, not pre-existing: end_lineno - lineno is 38 on origin/main (under the limit) and 43 on this branch (over it). The added owners = _record.entry_owners(tmp_dir) line plus the entry_owners = owners.get(entry, frozenset()) lookup and passthrough pushed the function past the threshold.
  • The PR body's own "Local gates" section claims check_craftsmanship.py ran clean; it does not on this branch against origin/main. Not a seen-defect rationalization (no dismissal language present), just an inaccurate claim, but it means the stated gate evidence for merge is wrong.
  • Fix: extract the per-entry lookup+decide step (entry_owners = owners.get(...); if _entry_already_satisfied(...): continue) into a small helper, or fold owners.get into _entry_already_satisfied's call site via a one-line helper, to bring _commit_resolved_entries back under 40 lines. No ADR exemption is present for this file, and it wasn't over the limit before this PR, so this blocks per this repo's own non-negotiable gate.

Verified correct (no defect found)

• record_top_level (scripts/launcher_deps_record.py:36-48) correctly handles the pip --target ../../<entry> prefix (console scripts, include, share), rejects paths climbing further (../../../bin/tqdm maps to None), rejects absolute paths (/usr/lib/... maps to None), and rejects empty paths, all four covered by test_record_top_level's parametrize table (tests_py/scripts/test_launcher_deps_record.py:48-63). RECORD paths use / unconditionally per the packaging spec regardless of host OS (confirmed against this repo's own installed deps/*.dist-info/RECORD files, e.g. truststore-0.10.4.dist-info records truststore/_windows.py with a forward slash), so there is no unhandled Windows-path-form case here.
• Mixed-version dist-info handling (ADR-1063): dist_info_versions already excludes any dist key with more than one version on either side; _entry_already_satisfied correctly falls through to "not satisfied" (replace) when dest_versions.get(dist_key) != tmp_v, including the None != tmp_v case for an ambiguous or absent dest key. Traced this path manually; matches ADR-1064's "unchanged" and "decision" sections.
• ABI guard (ADR-1061) ordering preserved: the version/owner check (cheap) still runs before the os.walk-based entry_has_foreign_abi_extension call; unchanged from the prior guard shape.
• os.path.lexists (not os.path.exists) is now used for the destination presence check (launcher_deps_install.py:139), correctly restoring a deleted entry rather than treating it as present, confirmed by test_rerun_restores_an_entry_missing_under_matching_metadata, parametrized over all three test entries (probeyaml, probens, bin).
• Test adequacy against issue [bug] deps idempotence guard replaces entries whose name is not their distribution's on every install #575's "Evidence required to close": tests_py/scripts/test_installer_rerun_keeps_entries.py runs real, offline pip through the actual installer entry point and covers exactly what the issue demanded: an import-name-vs-distribution mismatch (cortexprobe-yaml/probeyaml) and a shared-namespace directory across two distributions (cortexprobe-ns-a/-b sharing probens), with both a same-version re-run (inode-stable) and a version-bump re-run (selectively replaced) asserted by inode identity, not just exit code. This is mutation-strong: flipping the guard's owner-version comparison or removing the lexists check would fail these tests, confirmed by re-reading the pre-fix guard logic against the same assertions.
• Ran pytest tests_py/scripts/test_installer_rerun_keeps_entries.py tests_py/scripts/test_launcher_deps_record.py tests_py/scripts/test_launcher_deps.py -q: 93 passed. ruff check / ruff format --check on the changed files: clean. check_no_deps_invariant.py: clean.
• Layer check: scripts/ bootstrap modules remain stdlib-only (launcher_deps_record.py imports only csv, os, sys, Path, and its sibling launcher_deps_fs); no import of mcp_server.* or third-party packages. No layer-boundary violation.
• Documented, non-hidden limitation (ADR-1064 "Unchanged" section): a shared entry with only some of its owners changed in scratch still has the whole directory replaced, potentially dropping files from owners scratch didn't reinstall on a partial install. This is called out explicitly in the ADR as a known, unresolved consequence rather than dismissed, acceptable per "review the delta," since the prior guard had the same behavior on every run, not just this narrower case.

Non-blocking

• scripts/launcher_deps_record.py:28-33: the comment block documenting the ../../ prefix rationale is 6 lines of # comments (under the 8-line "no 8+ line comment run" ceiling), fine as-is, but if a future edit adds even one more line here it will need splitting or moving into the docstring.

Minimum change to unblock merge

Extract enough of _commit_resolved_entries's per-entry decision logic (the owners.get plus _entry_already_satisfied call) into a helper function so the function's AST span (end_lineno - lineno) is 40 or less, and re-run python3 scripts/check_craftsmanship.py --base origin/main to confirm zero new violations before merge.

ZETETIC-REVIEW: APPROVE

Re-review of commit 8764e29 on top of the previously reviewed 17dcc87 (review comment #5688590477, REQUEST_CHANGES, blocking on the craftsmanship method-size gate).

Diff since 17dcc87

$ git diff 17dcc873 8764e29c --stat
 scripts/launcher_deps_install.py | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Only scripts/launcher_deps_install.py changed; nothing else moved since the previously reviewed commit. The change moves the post-commit pruning (prune_superseded_dist_info over every freshly committed .dist-info, then prune_foreign_abi_extensions) out of _commit_resolved_entries into a new _prune_after_full_commit(deps_dir, committed_dist_infos), called once at the same point _commit_resolved_entries used to run it inline, right before return True, None; that is, only after the whole tmp_dir commit loop finished without error. The except OSError branch still does return False, entry before that call is ever reached, so pruning still never runs on the failure path. Confirmed by reading the function body directly (lines 144-196 of the head revision).

Gate: craftsmanship (the blocking finding from the prior review)

$ cd .claude/worktrees/review-577   # worktree checked out at 8764e29c
$ python3 scripts/check_craftsmanship.py --base origin/main
Craftsmanship gate: OK
EXIT:0

$ git checkout 17dcc873 -- scripts/launcher_deps_install.py
$ python3 scripts/check_craftsmanship.py --base origin/main
Craftsmanship gate: NEW violations (not in the base-ref baseline):
  - [method-size] scripts/launcher_deps_install.py: _commit_resolved_entries
EXIT:1
$ git checkout 8764e29c -- scripts/launcher_deps_install.py   # worktree restored

Matches the expectation exactly: 17dcc87 fails with the same _commit_resolved_entries method-size violation this review previously blocked on; 8764e29 passes clean. The gate was run with --base origin/main (origin/main tip 4265241 == the PR's baseRefOid, so this is the same comparison CI runs), not against the working tree.

Gate: ruff

$ uvx [email protected] check .
All checks passed!
CHECK_EXIT:0

$ uvx [email protected] format --check .
3307 files already formatted
FORMAT_EXIT:0

Gate: tests_py/scripts

$ .../fix-575/.venv/bin/python -m pytest tests_py/scripts -q
1001 passed, 359 subtests passed in 43.75s

Base currency

gh pr view 577 --json baseRefOid = 42652417a7a51b70a3a43c43aca203e61b7c77d4, which is the current tip of origin/main; the PR branch has not drifted behind main since the prior review.

Other

• PR body's "Local gates" line now correctly names --base origin/main and describes the 17dcc87 -> 8764e29 fix, matching what I independently reproduced above.
• No seen-defect rationalizations found in the diff or PR body.
• All logic verified in the prior review (RECORD-path parsing, record_top_level boundary rejection, ADR-1063 mixed-version fallthrough, ABI-guard ordering, os.path.lexists restore) is untouched by this commit; confirmed by the file-level stat above showing zero other files changed since 17dcc87.

Verdict: the previously-blocking craftsmanship violation is resolved without weakening the gate or moving logic off the failure-safe path. APPROVE.