Fix uv script-lock revert per pair (#1285, #890)

[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1285
Fixes #890

Summary

A vendored PyPI entry can wire several PEP 723 scripts (a.py +
a.py.lock, b.py + b.py.lock) or several PEP 751 locks
(pylock.toml, pylock.dev.toml) at once. Its revert now handles each
pair separately, so a change to one script no longer strands the others.
Every unwind benefits: vendor --revert, scan --prune, remove,
rollback, and the vendored → hosted takeover.

Root cause

revert_python_locks (crates/socket-patch-core/src/vendor/pypi_lock.rs)
decided liveness for the whole ledger entry at once:

• Vendored uv script locks: after uv remove --script drops the package from one of two vendored scripts, every unwind keeps the other script wired (revert exits 0, remove/rollback exit 1, hosted takeover refuses) #1285: one unreferenced flag was cleared as soon as any wired
  file still named the uuid. After uv remove --script b.py six, a.py
  still named it, so b.py's records were drift instead of removed. The
  drift then kept every pair, including the untouched a.py, and
  remove / rollback exited 1.
• Vendored uv script locks: once the user deletes one wired script (or its .py.lock), vendor --revert, remove, rollback and the hosted takeover can never unwind the other scripts, and the takeover's suggested fix is the command that fails #890: the same probe, and the per-record read, returned
  RevertOutcome::failed on any read error, including NotFound. One
  deleted script, .py.lock or pylock.dev.toml failed every unwind
  with cannot read ./b.py.lock, and the takeover told users to run the
  same failing command.

Fix

• Every wired file is read once, up front. A file that does not exist
  is None. Any other read error still fails closed.
• "Removed, not drift" (After uv remove --script drops a vendored package from a PEP 723 script lock, scan --prune, vendor --revert, remove, rollback and the hosted takeover still drift-keep it, so vendor --check stays red and its prune remedy loops #1214's rule) is decided per pair: a
  .py.lock goes with its script (script_of_lock), and each
  pylock*.toml is its own pair. A hand edit inside a pair that still
  routes through the wheel is still drift.
• A wired file that no longer exists warns vendor_lockfile_missing once,
  naming the file (the gem backend's precedent), and the files that still
  exist are restored. The pypi dispatcher's residual-reference guard
  still keeps the wheel if any other project file names it.
• The hosted takeover now passes vendor_lockfile_missing through, next to
  vlt's reinstall notice, so scan --mode hosted names the deleted file
  too.
• CLI_CONTRACT.md documents the per-pair rule and the missing-file
  advisory.

Out of scope: #890 has a later comment saying a deleted uv.lock blocks
the project lane. That goes through a different backend
(pypi.rs revert_uv) and is a separate root cause, so it is not claimed
here.

Tests (red → green)

Per issue:

• Vendored uv script locks: after uv remove --script drops the package from one of two vendored scripts, every unwind keeps the other script wired (revert exits 0, remove/rollback exit 1, hosted takeover refuses) #1285: pypi_lock::tests::uv_remove_from_one_of_two_scripts_reverts_the_other
  (core), plus the uv remove b row of
  mode_migration_pypi::multi_script_unwinds_after_one_script_changes (CLI:
  vendor --revert, remove, rollback, hosted takeover). Before the
  fix it fails with vendor_lock_entry_drifted (b.py.lock changed since vendoring).
• Vendored uv script locks: once the user deletes one wired script (or its .py.lock), vendor --revert, remove, rollback and the hosted takeover can never unwind the other scripts, and the takeover's suggested fix is the command that fails #890: deleted_script_or_lock_does_not_block_the_other_scripts
  (deleted both, only .py.lock, or only .py; dry and wet) and
  deleted_pylock_does_not_block_the_other_pylock (core), plus the four
  rm … rows of the CLI test across the same unwinds. Before the fix
  they fail with cannot read …/b.py.lock: No such file or directory.
• Guards: drift_in_a_still_wired_pair_keeps_the_entry and
  unreadable_wired_file_still_fails_closed (a directory where the lock
  was still fails closed).
• Takeover after only b.py is deleted: hosted mode refuses the orphan
  b.py.lock on its own (redirect_uv_script_missing) and keeps the
  project vendored and untouched. The CLI test pins that this is the
  hosted refusal and not a revert failure.

Local runs:

• cargo fmt --all -- --check and cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test --workspace --all-features --no-fail-fast: everything this
  PR touches passes. 13 tests fail in this sandbox and also fail on
  main here. 12 are permission-injection tests (*_write_failure_*,
  *unremovable*, relax_loop_must_not_traverse_symlinked_root) that
  can't fail a write when run as root. The 13th,
  e2e_bun_lockb::binary_shared_bundled_record_hosted_pin_is_managed,
  fails on an npm missing_blob download; I re-ran it on main with the
  same result.
• cargo test -p socket-patch-cli --all-features --test e2e_vendor_pypi_build -- --include-ignored (uv 0.11.32): 52 passed.
• No wrapper changes (npm/, pypi/, gem/): the revert logic is only in the Rust core.

/code-review high ran on the diff. Its findings are fixed in 194f0c3:
one read per file (closes a delete-mid-revert race), a deduplicated
warning in wording that is also true on a dry or drift-kept run,
script_of_lock reused, and the takeover forwarding the advisory.

Bugbot (194f0c3) found that the missing-file probe read the disk, not the
GroupCommit overlay the takeover reverts inside. Fixed in e7fae29, with
the regression test missing_is_judged_through_the_group_commit_overlay
(red before the fix, green after).

Human review (e7fae29) found that a renamed vendored script (git mv a.py tool.py) would lose its wheel, because the residual-reference guard never read
root *.py files. Fixed in b8bd9e6, with the test
mode_migration_pypi::renamed_vendored_script_keeps_its_wheel (red before
the fix, green after).

🤖 Generated with Claude Code

https://claude.ai/code/session_01LSV2D8AnkmcqFDijhzZGhE

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Ready for review at e7fae29.

• CI: 35/35 non-skipped checks green on the head (18 skipped by path filters); mergeable, no conflicts; no main-wide failures.
• Bugbot: reviewed e7fae29 (check passed); its one thread on pypi_lock.rs:720 is resolved.
• Reviewer focus: revert_python_locks in crates/socket-patch-core/src/vendor/pypi_lock.rs now decides liveness per script/lock pair; check the drift handling for a pair whose file changed while siblings stay untouched, and the matching CLI_CONTRACT.md wording.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. revert_python_locks now reads every wired file once, up front, and decides the "removed, not drift" rule from #1214 separately for each pair (a script plus its .py.lock, or one pylock*.toml) instead of for the whole ledger entry. So after uv remove --script b.py, the untouched a.py is still restored (#1285). A deleted wired file now gives one vendor_lockfile_missing warning, and the files that remain are restored, instead of every unwind failing (#890). The vendored → hosted takeover now passes that warning through, and CLI_CONTRACT.md documents it.

Risk: low–medium. The change is confined to one revert function, and any read error other than NotFound still fails closed. One intended behaviour change: an entry whose only wired file was deleted used to fail; now it succeeds with a warning, which matches the bun and gem reverts.

Look here

• pypi_lock.rs#L714-L730: read_existing (overlay-aware, NotFound → None) and pair_of
• pypi_lock.rs#L833-L857: single read and the per-pair referenced set
• pypi_lock.rs#L899-L923: missing-file warning (deduplicated) and the per-pair removed check
• takeover.rs#L281-L284:`` advisory filter that now forwards vendor_lockfile_missing
• mode_migration_pypi.rs#L1083: multi_script_unwinds_after_one_script_changes (5 changes × 4 unwinds)

Verified. Read the full diff. cargo test -p socket-patch-core --lib pypi_lock: 60 passed. mode_migration_pypi: 55 passed. --lib takeover: 12 passed. in_process_vendor_bun_takeover: 40 passed. Red check: with main's pypi_lock.rs logic and the PR's tests, 4 of the 6 new core tests fail; the other 2 are guard tests (drift in a still-wired pair; unreadable file fails closed) and pass on both, as they should. I checked these edge cases and found them correct: only the script deleted, only the lock deleted, both deleted, dry runs, and a hand edit inside a live pair (still drift). Windows paths: wiring entries are bare filenames (allowed_file), and there is no CRLF-specific logic. Each file is now read once instead of twice. CI 53/53 green (35 pass, 18 skipped) on e7fae29; Cursor Bugbot: no new issues. CHANGELOG untouched; no debug leftovers.

Changes I made. None.

Open questions (non-blocking).

• The takeover filter forwards every vendor_lockfile_missing, not only pypi's. Bun's revert also emits that code when bun.lock is missing, so a bun takeover will now show it in warnings[] too. That is probably desirable, but it is unstated and not covered by a test.
• The vendor_lockfile_missing row in the CLI_CONTRACT.md error-code table still lists only the bun preflight placement; the prose covers the new placement. The code is also a bare string literal in two files, where a shared constant would be safer.

Auto-merge is armed: approving sends it straight to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit b8bd9e6. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Final review brief (update for b8bd9e6)

What it does. Same change as the earlier brief: the uv script-lock revert decides "removed vs. drift" per script/lock pair (#1285), and a deleted wired file warns vendor_lockfile_missing instead of failing the unwind (#890). One commit since that brief, b8bd9e6: if a vendored script was renamed (git mv job.py tool.py), the revert used to delete the wheel the renamed script still installs from. The residual-reference guard now also reads root *.py files, so the wheel is kept and vendor_revert_residual_reference names the file.

Risk: low–medium (unchanged). The new commit only widens a fail-closed guard. The worst case is that the artifact is kept when a root .py mentions the uuid dir for some other reason, and the warning names that file.

Look here

• pypi.rs#L2296-L2302: root *.py added to the probe set
• pypi.rs#L2711-L2722: the post-revert guard that now catches the renamed script (it runs after wired files are restored, so restored scripts don't trip it)
• mode_migration_pypi.rs#L1208-L1243:`` renamed_vendored_script_keeps_its_wheel covering `vendor --revert`, `remove` and `rollback`
• Earlier brief's links still cover the main change in pypi_lock.rs.

Verified. Read the delta. mode_migration_pypi: 56/56 pass. Red check: dropping the .py clause makes renamed_vendored_script_keeps_its_wheel fail. socket-patch-core --lib pypi: 527 pass. The 2 failures are chmod-based write-failure tests in pypi_poetry/pypi_requirements. They fail only because my sandbox runs as root, they're untouched by this PR, and they're green in CI. The other callers (guard_unwired_pypi_revert and the dry-run preview) both still fail closed, which is correct. CI 35 pass, 19 skipped, 0 failed on b8bd9e6. Cursor Bugbot: success. Review threads: all resolved. CHANGELOG untouched.

Changes I made. None. I disarmed auto-merge when the new commit landed and re-armed it after this review.

Open questions (non-blocking). The two from the earlier brief still apply (takeover forwards every vendor_lockfile_missing; the CLI_CONTRACT error-code table row).

Auto-merge is armed: approving sends it straight to the merge queue.

---

Generated by Claude Code