Repository navigation
Fix vendored Poetry revert after a re-lock (#1615) - #1662
Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
A poetry lock by another Poetry version re-renders the unit socket-patch wired (2.x adds groups, 1.4+ drop category, a lock-1.1 re-lock moves [metadata.files] into the unit) but keeps the vendored source and patched hash. vendor --revert, rollback and remove read that as drift forever, so the patch could never be removed. When the verbatim splice can't place the unit, restore it from the live lock if it still carries exactly the wiring we wrote. Edited wiring is still drift. Fixes #1615 Assisted-by: Claude Code:claude-opus-5-5
A second revert after a re-lock restore must stay quiet, and a lock-1.1 unit keeps its patched hash in [metadata.files], so read the patched file list from there. An already restored unit counts as done only when nothing in the lock still names the vendored wheel. Assisted-by: Claude Code:claude-opus-5-5
vendor --revert, rollback, remove and the vendored -> hosted takeover each restore a unit Poetry 1.8.5 re-rendered from a 2.4.1 lock, and reclaim the vendored artifact. Assisted-by: Claude Code:claude-opus-5-5
Only call a unit restored when the lock names neither the vendored wheel nor its patched hash, so a unit that lost its source but kept the patched hash still counts as drift. Never restore a recorded source, and only re-render a lock toml_edit reproduces byte for byte, so no other unit's bytes change. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 268bd9a. Configure here.
|
Ready for review — burn-down agent.
Generated by Claude Code |
Final review briefWhat it does. Fixes #1615: after a Risk: medium. It rewrites Look here
Verified
Changes I made: none. Open questions: none blocking. Follow-up already noted in the PR: PDM's vendored revert uses the same byte-exact splice and likely has the same re-render problem. Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1615
Summary
After a
poetry lockby a different Poetry version, a vendored Poetry patch could never be removed.vendor --revertdrift-kept it (and still exited 0),rollbackandremoveexited 1,scan --mode hostedrefused the takeover, and the "re-vendor" remedy did nothing. Now all four restore the registry entry and reclaim the vendored wheel, as long as the lock still carries exactly the wiring socket-patch wrote.Root cause
revert_poetry(vendor/pypi_poetry.rs) undid the wiring only throughrevert_lock_fragment_splice_atomic, which needs the recorded vendored[[package]]text byte for byte. Each Poetry version renders that unit a little differently:groups.category.groups.[metadata.files]into the unit.The vendored source and patched hash survive every one of these re-renders, but the byte match fails, so the revert read it as drift forever. The vendored → hosted takeover reverts through the same function, so it was stuck too.
Fix
revert_poetrystill tries the verbatim splice first. When that can't place the unit,restore_re_renderedhandles it on the parsed live lock:name@versionunit has afilesource at the recorded.socket/vendor/pypi/<uuid>/…URL andfilesequal to the recorded patched list, drop the source and put back the recorded original file list. That list comes from the unit for lock 2.x originals and from[metadata.files]for 1.0/1.1 originals ([]stays[]). Every field the re-lock added or dropped is kept as rendered.Tests
Each test below failed before the fix and passes with it. The red runs are at the end of this section.
groupsadded, lock 2.0 → 2.1)pypi_poetry::tests::revert_restores_a_unit_another_poetry_version_re_rendered(case1.8 -> 2.5, LF + CRLF, dry run, second revert stays quiet)groupsdropped)2.4 -> 1.8categorydropped)1.3 -> 1.8revert_restores_a_legacy_unit_re_locked_into_lock_2(empty and listed[metadata.files])vendor --revert/rollback/removethrough the CLImode_migration_pypi::poetry_unwinds_after_a_relock_by_another_poetry_versionmode_migration_pypi::poetry_vendored_to_hosted_after_a_relockrevert_converges_after_a_relock_onto_another_versionre_rendered_unit_with_edited_wiring_is_still_drift,re_rendered_restore_refuses_an_original_with_a_sourceCommands run locally:
cargo fmt --all -- --check: clean.cargo clippy --workspace --all-features -- -D warnings: clean.cargo test -p socket-patch-core --all-features --lib: 6594 passed, 4 failed. The 4 are permission tests (copy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched,pypi_requirements::wire_failure_rolls_back_already_written_files). They fail only because this sandbox runs as root, which ignoreschmod 0o555. None are in code this PR changes.cargo test -p socket-patch-cli --all-features --test mode_migration_pypi --test in_process_redirect_poetry: 66 + 9 passed.cargo test --workspacedidn't finish locally because the sandbox ran out of disk; CI runs it.Red runs, without the fix:
vendor_lock_entry_drifted.pypi_poetry.rsreset tomain, both new CLI tests failed.Code review (
/code-review high)common::lock_units_named.Follow-ups
pdm lockby another PDM version, through the samerevert_lock_fragment_splicepath. Not covered here.# socket-patch vendor:comment:vendor --revertdrift-keeps forever (exit 0), and the suggested "re-vendor" remedy is a no-op or adds a duplicate line #977:vendor --revertstill exits 0 when it drift-keeps every package (the requirements.txt case). This PR removes that outcome for Poetry re-renders but doesn't change the exit code.🤖 Generated with Claude Code
https://claude.ai/code/session_01W4vKB5zgAMhBnjCJU9vVv9
Note
Medium Risk
Changes Poetry vendored revert and hosted takeover paths that rewrite
poetry.lock; behavior is heavily tested but lock parsing must stay conservative to avoid silent corruption.Overview
Fixes #1615: vendored Poetry patches could not be removed after someone ran
poetry lockwith a different Poetry version, because revert relied on a byte-exact match of the recorded[[package]]fragment.revert_poetrystill uses the verbatim lock splice first; when that fails,restore_re_renderedparses lock 2.x, confirms the target unit still has the expected vendoredfilesource and patchedfiles, then drops the source and restores the recorded registry file list while keeping fields the re-lock added or removed (groups,category, lock version, etc.). Unchanged wiring edits, legacy locks, and tampered units still count as drift.multiline_filesinpoetry_lock.rsis exposed for consistentfilesrendering on restore. New CLI and core tests cover re-lock scenarios (including vendored→hosted after re-lock).Reviewed by Cursor Bugbot for commit 268bd9a. Configure here.
Generated by Claude Code