GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix vendored Python reverts refusing sibling edits (#385, #474) by mikolalysenko · Pull Request #481 · SocketDev/socket-patch

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

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

Fixes #385
Fixes #474

Summary

Rolling back a vendored Python project no longer fails just because the user made an everyday edit next to the patched pin:

Hatch (Hatch rollback and remove refuse with "configuration drifted" after the project version is bumped or a dependency is added, in both hosted and vendored mode #385): rollback / remove / vendor --revert used to fail with pyproject.toml changed since patching after a [project].version bump, a comment on name, or a new entry in project.dependencies. They now restore six's pin, drop the allow-direct-references permission, and keep the user's edits.
uv script locks (Vendored uv script lock can't be reverted after uv add --script adds an unrelated dependency (vendor_lock_entry_drifted, still exit 0) #474): after uv add --script tool.py <pkg>, vendor --revert used to report vendor_lock_entry_drifted and keep the vendored wiring for good (still exiting 0). It now restores six's [manifest] requirements element and [[package]] source and keeps the added requirement. uv accepts the result as is (uv lock --script leaves it unchanged).

Root cause

Both reverts go through one three-way TOML merge, vendor::pypi_lock::restore_document (also used by the hosted Hatch helper in utils/hatch.rs). It:

paired array elements by position and reported drift on any length change, so a sibling the user added, removed or moved blocked the revert; and
ran the lock-package name/version identity check on every table, including pyproject [project], so a release bump counted as drift.

Fix

Arrays (of values and of tables): only the elements the vendoring changed (original[i] != new[i]) are looked up in the live array: first by exact text (ignoring the element's own whitespace/comments), then by identity (PEP 508 project name for requirement strings, canonical name + version for tables). Each one must map to exactly one live element, otherwise it is drift. Only those elements are restored, and the user's other elements are left as they are. A restored inline element keeps the live spacing/comments unless they are still the vendored ones.
The identity check now happens only when pairing array elements, not on keyed tables like [project].
Still drift (fail closed, live text kept): the vendored element was edited, re-resolved to another version, removed, renamed, or appears twice; or vendoring itself changed an array's length.

Tests (red → green)

Issue	Test	Without fix	With fix
#385	`vendor::pypi_hatch::tests::revert_keeps_unrelated_project_edits` (version bump, comment on `name`, added dependency, added key)	FAILED	ok
#385	e2e `e2e_vex_build hatch::hatch_project_dependency_vendored` new step `revert-after-project-edits` (real Hatch 1.18.1: bump + `idna==3.7`, then `vendor --revert`)	panicked: `revert after project edits: exit Some(1)`	pass
#474	`vendor::pypi_lock::tests::added_sibling_requirement_does_not_block_script_lock_revert`	FAILED	ok
#474	e2e `e2e_vendor_pypi_build vendored_uv_script_lock_manifestless_vex` new step `sibling-revert` (real uv 0.8.17: `uv add --script tool.py idna==3.7`, `vendor --revert`, `uv lock --script` leaves the lock unchanged)	panicked: `an added sibling counted as drift`	`restored, user addition kept`
both	`sibling_strings_added_or_removed_around_the_vendored_element`, `re_resolved_package_with_added_sibling_is_drift`, `vendored_element_edited_or_removed_is_still_drift`	FAILED	ok

Bugbot follow-up (1e6967d): a vendored element renamed past recognition plus a user-added twin of the original pin is drift, not "already restored" (already_restored_element_is_trusted_only_without_unknown_siblings, and a new case in vendored_element_edited_or_removed_is_still_drift; both red without the guard).

Existing drift tests (conflicting_fields_are_preserved_and_reported, reordered_packages_cannot_receive_another_packages_original, out-of-order and CRLF reverts, Hatch shared-permission rollback) still pass unchanged.

Commands run locally:

cargo clippy --workspace --all-features -- -D warnings: clean.
cargo test --workspace --all-features --no-fail-fast: everything passes except 12 tests that make a directory read-only with chmod to simulate write failures. This sandbox runs as root, which ignores chmod, so they fail identically on main (re-ran them against main's code). None of them involve the TOML merge.
SOCKET_PATCH_UV_E2E_REQUIRED=1 cargo test -p socket-patch-cli --all-features --test e2e_vendor_pypi_build -- --include-ignored: 20/20 ok (uv 0.8.17).
SOCKET_PATCH_HATCH_E2E_REQUIRED=1 SOCKET_PATCH_HATCH_E2E_VERSION=1.18.1 cargo test -p socket-patch-cli --all-features --test e2e_vex_build -- hatch:: --ignored: 4/4 ok.
cargo fmt --all -- --check is already red on main (CI doesn't run it). The changed files are formatted with the pinned 1.93.1 rustfmt, and the diff only touches the changed hunks.

No wrapper (npm/, pypi/, gem/) changes are needed: the merge is core-only.

Not in scope

#473 (hosted uv include-group restore in redirect/upstream/uv.rs), #379 and #382 are different code paths.

🤖 Generated with Claude Code

Note

Medium Risk
Changes core three-way TOML merge and revert success criteria for Python vendoring; incorrect pairing could restore wrong pins or refuse valid reverts, though ambiguous cases are designed to fail closed.

Overview
Vendored Python rollback no longer treats unrelated pyproject/lock edits as drift: restore_document in pypi_lock.rs now pairs only vendoring-changed array entries (by text or PEP 508 / name+version identity) instead of by index/length, so siblings like version bumps, new dependencies, or uv add --script additions are kept while the patched pin is restored.

Fail-closed guard: still_references_artifact blocks a “successful” revert when a user-added copy of the vendored requirement still points at .socket/vendor/pypi/{uuid}—wired into Hatch revert and lock revert warnings—so wheels are not dropped while still referenced.

Tests: unit cases for siblings, namesakes, and leftover artifact refs; Hatch e2e (revert-after-project-edits, revert-refuses-copied-reference); uv script e2e script_sibling_revert (#474).

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

Generated by Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Start fix for #385, #474

06b7f60

Assisted-by: Claude Code:claude-opus-5-5

This was referenced Oct 1, 2026

Hatch rollback and remove refuse with "configuration drifted" after the project version is bumped or a dependency is added, in both hosted and vendored mode #385

Closed

Vendored uv script lock can't be reverted after uv add --script adds an unrelated dependency (vendor_lock_entry_drifted, still exit 0) #474

Closed

Claude (claude) added 2 commits

October 1, 2026 16:35


          Let vendored Python reverts keep sibling edits

7b30eb9

Rolling back a vendored Hatch project failed with "pyproject.toml
changed since patching" after a routine release bump, a comment on
`name`, or a new dependency next to the patched one (#385). A vendored
PEP 723 script lock stayed vendored for good once `uv add --script`
added an unrelated requirement (#474).

Both reverts go through one three-way TOML merge. It paired array
elements by position, called any length change drift, and checked
lock-package name/version on every table, including [project].

The merge now restores only the elements vendoring changed, finding
each one in the live array by its text or its identity (PEP 508 name,
or table name + version). Siblings the user added, dropped or moved
are left alone. A vendored element that was edited, re-resolved,
removed or duplicated is still reported as drift.

Assisted-by: Claude Code:claude-opus-5-5


          Test real uv/Hatch reverts after sibling edits

bb4372e

Adds end-to-end steps with the real tools: the vendored uv script
lane now runs `uv add --script tool.py idna==3.7` on a copy before
`vendor --revert` (#474), and the vendored Hatch project lane bumps
the release and adds a dependency before reverting (#385). Both
check the user's edits survive and the vendored wiring is gone.

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review

October 1, 2026 17:05

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 1, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/socket-patch-core/src/vendor/pypi_lock.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Keep drift when a renamed pin hides behind a twin

1e6967d

Review found a gap in the new array merge: if the user renamed the
vendored element past recognition and also added a sibling equal to
the original pin, the revert paired the sibling as "already restored",
reported success, and deleted the vendored artifact the renamed line
still pointed at.

An "already restored" pairing now only counts when every other live
element is accounted for. Otherwise the revert reports drift and keeps
the artifact. Elements already back to their original text are also
skipped regardless of their spacing.

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 1, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] Ready for review on 1e6967d: CI is green (all 487 check runs completed with success or skipped), Bugbot's review of this head found no new issues, and its one earlier finding is fixed and resolved. Fixes #385 and #474, with red→green unit and real-tool e2e tests for each (see the table in the description).

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 1, 2026

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

Reviewed 1e6967d91dcb3fc76bfb2ba0717b6b0a5ea6300a. Recommendation: changes needed.

P1 — Added sibling requirements can retain a reference to a wheel that revert deletes (pypi_lock.rs:494). unaccounted only blocks a pairing when already_original is true. When the recorded vendored row is still present, a second requirement using the same wheel URL with a different environment marker is preserved as a new sibling, while the recorded row is restored and the merge reports no drift.

Reproduced through the CLI: vendor a Hatch six==1.16.0 dependency, add a copy of the vendored requirement with ; python_version >= '3.8', then run vendor --revert. It returns exit 0 / success / removed: 1, leaves the added .socket/vendor/.../six-1.16.0-py3-none-any.whl requirement in pyproject.toml, and deletes that wheel directory. Subsequent installs now reference a missing file. Check residual artifact references before reporting a complete revert/deleting the artifact, or refuse the ambiguous added reference.

Validation: 21 vendor::pypi_lock and 5 vendor::pypi_hatch unit tests passed. An additional review-only CLI test for the added marked requirement failed with the behavior above. Full workspace and real package-manager matrices were not rerun.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Claude (claude) added 2 commits

October 2, 2026 14:26


          Merge remote-tracking branch 'origin/main' into agent/fix-pypi-toml-m…

26a8a45

…erge-identity


          Refuse reverts that leave a vendored wheel in use

d1698d3

Review found that a user-added copy of the vendored requirement (for
example with a `python_version` marker) survives the array merge as a
new sibling. The revert then reported success and deleted the wheel
that line still installs from.

Before a Hatch or Python-lock revert counts as complete, it now checks
that the restored file no longer mentions this patch's
`.socket/vendor/pypi/<uuid>` directory. If it does, Hatch fails and
the lock revert reports `vendor_lock_entry_drifted`, so the file and
the artifact are kept. Adds unit tests and a real-Hatch e2e step that
fail without the check.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CPpPuqqqgLkPXkcpsFfKFo

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] Re the P1 in your review: confirmed and fixed in d1698d3, which also merges main.

Before a Hatch or Python-lock revert counts as complete, it now checks whether the restored file still mentions this patch's .socket/vendor/pypi/<uuid> directory when the original didn't. If it does, Hatch fails with "still references the vendored artifact" and the lock revert reports vendor_lock_entry_drifted. Either way the file and the wheel are kept. Wheels from other patches stacked on the same project are not affected.

Tests (each fails without the check):

pypi_hatch::revert_refuses_while_an_added_line_references_the_artifact: your repro, a copy of the vendored requirement with ; python_version >= '3.8'.
pypi_lock::leftover_artifact_reference_is_detected.
New real-Hatch 1.18.1 e2e step revert-refuses-copied-reference: vendor --revert exits non-zero and leaves pyproject.toml and the wheel. Without the check it exits 0, the behavior you saw.

Re-ran locally: vendor unit tests (2,132 pass; the 2 that fail are root-only chmod tests that also fail on main), uv e2e 20/20, Hatch e2e 4/4, and clippy.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 2, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/socket-patch-core/src/vendor/pypi_lock.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Don't restore a removed pin into a namesake row

f3f20c6

Bugbot found that when the user removes the vendored array row, the
merge could still pair it by name with another row (a requirement's
identity ignores `specifier`). It then filled the original fields into
that row and reported a clean revert, rewriting the user's entry.

A table paired only by identity must now still hold at least one field
exactly as the vendoring wrote it, such as the vendored `path` or
`source`. Otherwise the revert reports drift and keeps the file and
the artifact.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CPpPuqqqgLkPXkcpsFfKFo

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

Follow-up review of f3f20c61540872cb5d3768b3e52797619b0022cc: ready from code review. The leftover-artifact check addresses my earlier finding, and I reviewed the additional safeguard against restoring a removed vendored row into a user-added namesake table.

Validation: 23 Python-lock tests passed at this exact head, including the new namesake-row regression and leftover-artifact check. The original independent Hatch CLI reproduction passed on the preceding commit d1698d36: vendor --revert exits 1, preserves both requirement rows, and keeps the referenced wheel (removed: 0). The final commit leaves that artifact-retention path unchanged.

No remaining blocking finding in the updated code. This supersedes my earlier changes-needed recommendation.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 2, 2026

View reviewed changes

cursor Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 f3f20c6. Configure here.}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator Author

[agent] CI: native (ubuntu-latest, 0.0.0-16) failed on f3f20c6. I don't think the failure comes from this PR:

It's the vlt (npm) backtest against production. Its log shows transport failure; retrying on several cases, and agent-direct ended with rollbackByteIdentical/rollbackOriginalBytes failing after those retries (31/34 as expected).
This PR only changes the Python TOML revert merge (vendor/pypi_lock.rs, vendor/pypi_hatch.rs), which vlt/npm never calls. The last commit's only change from d1698d3 is in pypi_lock.rs.
The same check passed on d1698d3 (job) and on main ffe6897 (job).

No fix to port. GitHub won't re-run the job while the rest of its workflow run is still going, so I'll re-run it once when the run finishes. If it fails again, I'll treat it as a real failure and investigate it.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] The re-run of native (ubuntu-latest, 0.0.0-16) passed, which fits the production transport failures in the first attempt. CI on f3f20c6 is now green (481 succeeded, 6 skipped). Bugbot is clean on this commit, and no review threads are open. The PR is waiting only on an approving review.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Tanmay Singla (Tanmay182003) approved these changes

Oct 2, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) merged commit 795a535 into main

Oct 2, 2026

575 of 576 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) deleted the agent/fix-pypi-toml-merge-identity branch

October 2, 2026 16:21

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

Second pass reviewed f3f20c61540872cb5d3768b3e52797619b0022cc: ready from code review; no additional code change needed. The head is unchanged from my latest follow-up. Both the leftover-wheel-reference guard and the namesake-table guard remain in place, and I rechecked the current discussion against those fixes.

Prior validation remains applicable: 23 Python-lock tests passed at this exact head; the independent Hatch CLI reproduction passed at d1698d36, and its artifact-retention path is unchanged. No tests were repeated without a code change.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Reviewers

cursor[bot]

cursor[bot] left review comments

Tanmay Singla (Tanmay182003)

Tanmay182003 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Ready for review

Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Vendored uv script lock can't be reverted after uv add --script adds an unrelated dependency (vendor_lock_entry_drifted, still exit 0) Hatch rollback and remove refuse with "configuration drifted" after the project version is bumped or a dependency is added, in both hosted and vendored mode

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Summary

Root cause

Fix

Tests (red → green)

Not in scope

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.