fix(installer): route setup.sh and setup.py through the launcher's scratch-and-commit install (#573) - #576

ZETETIC-REVIEW: APPROVE

Summary

Routes scripts/setup.sh and scripts/setup.py through the launcher's existing scratch-and-commit install (launcher_deps.install_requirements calling launcher_pip.install_requirements calling launcher_deps_install._commit_install/_commit_resolved_entries), eliminating the direct pip install --target "$DEPS_DIR" that left old package code under new *.dist-info on upgrade (#573). Also fixes launcher_deps_fs.dist_info_versions to omit any distribution with more than one *.dist-info (previously it silently picked whichever os.listdir order returned last), so the idempotence guard can no longer read a mixed/damaged deps/ as already satisfied. Verified against ADR-0749/1059/1063 and confirmed with real pip runs. No regression found in dist_info_satisfies, _entry_already_satisfied, ensure_deps, or ensure_all_deps.

Concurrency contract (ADR-0749/1059)

• Lock: launcher_deps.install_requirements (scripts/launcher_deps.py:232-238) wraps the call in _deps_lock(deps_dir), the same lock ensure_deps/ensure_all_deps use. Held.
• Scratch on same filesystem: launcher_pip.scratch_dir(deps_dir) returns f"{deps_dir}.tmp-{pid}" (scripts/launcher_pip.py:79-81), a sibling of deps_dir, so os.replace in commit_entry stays same-filesystem. Held.
• Per-entry os.replace with rollback: unchanged commit_entry/_entry_already_satisfied (scripts/launcher_deps_install.py:73-141). Held.
• Prune only after the whole batch commits: _commit_resolved_entries (scripts/launcher_deps_install.py:143-177) collects committed_dist_infos during the loop and only calls prune_superseded_dist_info once every entry in tmp_dir has committed. A mid-loop failure returns (False, entry) with zero pruning. Held.
• The new pip_install/install_requirements split (scripts/launcher_deps_install.py:200-250) factors the shared tail into _commit_install, which both call identically, so there is no drift between the two call paths.

dist_info_versions regression check (as requested)

scripts/launcher_deps_fs.py:84-110 now builds dict[key, set[version]] and returns only keys with exactly one version; an ambiguous distribution is entirely absent from the map.

• dist_info_satisfies (fs.py:122-134): .get(dist_key) == version on a missing key is None == version, so False, meaning "not satisfied." That correctly forces a reinstall of an ambiguous package rather than treating it as done. No caller change needed.
• _entry_already_satisfied (launcher_deps_install.py:108-126): same reasoning. dest_versions.get(key) is None for an ambiguous dest entry, so tmp_v != None reads as "not satisfied," the entry is replaced via commit_entry, and, being a .dist-info entry, it lands in committed_dist_infos and triggers prune_superseded_dist_info, which removes the other stale dist-info(s). This is the actual repair path and is exercised end to end by test_pip_install_repairs_old_code_under_new_metadata (tests_py/scripts/test_launcher_deps.py) and by the two "from-4.22.0-mix" parametrizations in test_installer_upgrade_single_version.py.
• ensure_deps/ensure_all_deps (launcher_deps.py, bodies unchanged by this diff) rely on _pins_satisfied (a separate JSON stamp comparing pinned specs, not dist_info_versions) as the fast path, and on _dist_info_satisfies for the missing-list computation on the slow path. Neither can loop forever: once a package is ambiguous it is always selected for reinstall, and the next successful commit prunes the duplicate, resolving the ambiguity for the following call. I could not construct a case where a stamp gets written on a broken state: _write_stamp in ensure_deps is gated on _importable(...), which imports the actual code rather than counting metadata, and ensure_all_deps additionally returns early before writing the ML stamp if _pip_install fails. Note that ensure_deps ignores _pip_install's return value before checking importability while ensure_all_deps checks it; that asymmetry predates this diff and is untouched by it, so it is not a regression introduced here.
• No infinite-reinstall loop is possible under normal operation, per the unit test test_dist_info_versions_omits_a_distribution_with_several_dist_infos (parametrized both listing orders) and the real-pip end-to-end tests.

setup.sh / setup.py wiring

• scripts/setup.sh runs under set -euo pipefail. ok/fail/warn are defined at lines 18-20, before the new source "$SCRIPT_DIR/lib/install_python_deps.sh" at line 219. The sourced library only defines a function (install_python_deps_step); it does not execute anything at source time, so set -e is not an issue there. The library's own if ! python3 "$scripts_dir/launcher_deps.py" ...; then fail ...; fi correctly fails loudly on a non-zero exit from launcher_deps.py, verified by test_setup_sh_step_fails_loudly_when_the_install_fails, which asserts a non-zero exit, the [FAIL] line, and that the old version is left in place.
• scripts/setup.py's install_deps() (scripts/setup.py:206-222) correctly invokes sys.executable scripts/launcher_deps.py --requirement <file> <deps_dir> (a real subprocess, no --target) and checks result.returncode. On failure, though, it only calls warn(...) and continues to setup_database(); it never calls fail()/sys.exit(1). This is unchanged from origin/main (confirmed via git show origin/main:scripts/setup.py): the diff only changed the argv passed to run(), not this control flow, so it is not a new defect introduced by this PR and I am not blocking on it. See the non-blocking note below.
• scripts/launcher_deps.py:main (new) parses --requirement/-r and a positional deps_dir, returns 0/1 based on install_requirements's boolean, and sys.exit(main(...)) at the bottom. Correct exit-code contract for both the bash if ! check and the subprocess.run(...).returncode check in setup.py.

Test adequacy

• tests_py/scripts/test_installer_upgrade_single_version.py builds two real wheels of a probe distribution and drives both setup.sh's step-3 library and setup.py's install_deps() through real, offline pip (--no-index --find-links), from a clean N and from a reproduced 4.22.0-mixed state, asserting exactly one *.dist-info remains, matching the pin, with the pinned code on disk and no .tmp-/.bak- leftovers. It also asserts a hash mismatch fails setup.sh's step and leaves N in place, and that setup.sh routes through the library with no --target left in the file.
• tests_py/scripts/test_launcher_deps.py unit-covers dist_info_versions's ambiguous-distribution omission in both listing orders, and the actual repair (test_pip_install_repairs_old_code_under_new_metadata, mocked pip, monkeypatched os.listdir order).
• tests_py/scripts/test_setup_py_installs_from_lock.py asserts the argv install_deps() hands to the subprocess (no --target, correct --requirement value, no hand-written loose specs) and the pip argv launcher_pip.install_requirements builds (--no-deps, --require-hashes, -r, scratch --target, no --upgrade).
• Ran the whole layer locally: pytest tests_py/scripts -q returned 980 passed, 359 subtests passed, 0 failed.
• These tests do catch the regression. Reverting dist_info_versions to the old "last listed wins" behavior fails the three new test_launcher_deps.py cases; I did not re-run the revert myself, but the logic checks out because _listdir_with_last forces both listing orders, which is exactly the axis the original bug depended on.
• Portability: sys.executable -m pip and prepending Path(sys.executable).parent to PATH for a python3 shim match the pattern already used elsewhere in this launcher family (launcher_pip.py's own _pip_command uses sys.executable -m pip). Nothing OS-specific was added. This should hold on Linux CI as long as the interpreter running pytest ships a python3 name in its own bin directory, which every CPython venv and actions/setup-python interpreter does.

Gates run

• pytest tests_py/scripts -q -p no:cacheprovider: 980 passed, 359 subtests passed, 0 failed.
• python scripts/check_craftsmanship.py --base origin/main: OK.
• python scripts/check_no_deps_invariant.py: clean (120 files scanned), confirming no bare --target install of a hashed constraint file survives anywhere in scope, matching the PR's own claim.
• ruff check .: all checks passed. ruff format --check .: all formatted.
• shellcheck scripts/setup.sh scripts/lib/install_python_deps.sh: only a pre-existing SC2235 style note (line 213, untouched by this diff) and two informational SC1091s about not following sourced files. Nothing new or blocking.

PR-body / ADR accuracy

• The claims about dist_info_versions, the scratch/commit routing, the audit note (27/213 entries churned, filed as [bug] deps idempotence guard replaces entries whose name is not their distribution's on every install #575, confirmed that issue exists and matches), and the manual verification numbers are all consistent with the code and ADR-1063.
• One overstatement: the test-plan line "It also checks that a hash mismatch fails the step and leaves N in place" is demonstrated only for setup.sh's step (test_setup_sh_step_fails_loudly_when_the_install_fails). There is no equivalent assertion for setup.py's install_deps(), and install_deps() does not in fact fail the step on a bad exit code; it only warns and lets setup.py continue (see above). Non-blocking, but the sentence reads as if it covers both installers.

Issues

Blocking

None.

Non-blocking

• scripts/setup.py:206-222 (install_deps): on a non-zero launcher_deps.py exit, only warn() is called and the script proceeds to setup_database(). This is pre-existing behavior, unchanged by this diff, but it now sits behind a more consequential failure mode (a real scratch/commit subprocess, not just a --target no-op on skip). Since setup.sh's equivalent step does fail loudly, and this PR's own audit-note discipline otherwise treats the two installers as parity paths, consider filing an issue to make setup.py call fail()/sys.exit(1) on failure, matching setup.sh.
• The PR body's test-plan sentence about "the step" failing loudly on a hash mismatch should be qualified to setup.sh only, or the parity gap above should be closed first.

Verdict

APPROVE. The concurrency contract is held, the dist_info_versions change is a net-positive fix with no new regression in any of its four callers (verified by reading and by tests), both installers are correctly wired to the launcher with no direct --target left, the tests are real (actual pip, actual wheels) and would catch the original regression, and all requested gates (pytest, craftsmanship, no-deps-invariant, ruff, shellcheck) are clean. The one gap found, setup.py not failing loudly, is pre-existing and outside this diff's blast radius, so it is a non-blocking follow-up rather than a reason to withhold approval.