Skip to content

Fix vendored requirements revert on edited comment (#977) - #1825

Merged
Mikola Lysenko (mikolalysenko) merged 5 commits into
mainfrom
agent/fix-pypi-requirements-revert-comment
Oct 29, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 5 commits into
mainfrom
agent/fix-pypi-requirements-revert-comment

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

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

Refs #977

Summary

A vendored requirements.txt line could not be reverted once its
trailing # socket-patch vendor: comment was touched: respaced, a note
appended, or stripped by a formatter. pip reads the line the same and
vendor / vendor --check still called it wired, but vendor --revert,
remove and rollback drift-kept the wheel forever. With the comment
stripped, re-running vendor (the remedy the CLI prints) also appended a
second (transitive) line for the same wheel, and the next revert
stranded the user's line.

Root cause

The requirements flavor decided whether a line was "ours" two ways: the
forward paths keyed on the code part of the line, while revert (and the
superseding re-wire) needed the whole line, comment included, to equal
the recorded one. Separately, every reader named a vendored wheel line
only by its comment tag, so a line with the tag stripped was invisible
to vendor (hence the duplicate) and to the lockfile inventory.

Fix

  • pair_recorded_lines (vendor/pypi_requirements.rs) pairs each
    recorded line with an exact copy first, then with a line whose code
    part (what pip reads) is unchanged. Records that wrote the same line
    then take their lines in file order, so identical lines never swap
    originals. Used by both revert_requirements and plan_rewire.
  • vendor_lines and the lock inventory
    (lock_inventory/pypi.rs) name an untagged vendored wheel line by
    its wheel (the existing socket_reference_coords parser), so
    vendor reports it in sync instead of appending a duplicate.
  • The vendor_artifact_kept hint now also names
    vendor_revert_line_drifted, the code this flavor emits.

Not in this PR (why Refs, not Fixes)

The issue also notes that vendor --revert exits 0 / success on a
drift-keep while remove exits 1, and that the human summary says
"lock entries were re-resolved". Both are shared by every vendored
ecosystem, so changing them is a cross-ecosystem contract decision, not
a requirements fix. The {:?} key formatting it mentions was already
fixed on main (#1608). #977 stays open for those.

Review notes (from /code-review high, not changed)

  • An untagged .socket/vendor/pypi/... line now counts as wiring
    everywhere. A superseding vendor or a fresh wire now refuses
    (pypi_requirements_already_vendored) when such a line has no ledger
    record. Before, it went ahead and left that line pointing at a wheel
    dir the run deletes, or appended a second competing wheel line. The
    refusal is the safer outcome.
  • A superseding re-wire rewrites an edited line to the standard shape,
    so a note the user appended is dropped. The rewire already replaces
    the whole line, and the note never affected pip.

Tests (red on main, green here)

Variant from #977 Unit test E2E (real pip 24.0)
(a) respaced, (b) note appended, (c) stripped: revert restores the pin revert_ignores_edits_to_the_vendor_comment (plain, hashed, transitive) pip_requirements_revert_after_the_vendor_comment_is_edited
(c) re-vendor appends a duplicate line an_edited_vendor_comment_is_still_in_sync same e2e (re-vendor leaves the file alone)
superseding patch after an edit rewire_and_revert_after_an_edited_vendor_comment
identical lines keep their own originals revert_pairs_identical_vendor_lines_in_order_after_an_edit
exact or still-tagged line wins over a bare manual line revert_prefers_an_edited_vendor_line_over_a_bare_copy, revert_prefers_the_exact_vendor_line_over_a_bare_copy
scan inventory after the tag is stripped vendored_requirements_lines_without_their_tag_stay_in_the_inventory

Before the fix the e2e fails at the first edit: vendor --revert
reports summary.removed: 0 with vendor_revert_line_drifted.

Commands run locally:

  • cargo fmt --all -- --check: clean
  • cargo clippy --workspace --all-features -- -D warnings: clean
  • cargo test -p socket-patch-core --all-features --lib: 6929 passed,
    4 failed. All 4 are read-only permission tests that cannot fail as
    uid 0, which this container runs as. They fail the same way without
    this change.
  • cargo test -p socket-patch-cli --all-features --test e2e_vendor_pypi_build -- --include-ignored pip_requirements: 2 passed
  • The full cargo test --workspace did not fit the container's disk;
    CI runs it.

No wrapper (npm/, pypi/, gem/) changes: they only dispatch to
the binary.

https://claude.ai/code/session_011f9aeTBVmHpGBarPd1Gbh2


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A vendored requirements.txt line could not be reverted once its
trailing "# socket-patch vendor:" comment was touched: respaced, a
note appended, or stripped by a formatter. pip reads the line the
same, and vendor and vendor --check still called it wired, but
vendor --revert, remove and rollback wanted the whole line verbatim,
so they kept the wheel and reported drift forever. With the comment
stripped, re-running vendor also appended a second line for the
same wheel.

Revert and the superseding re-wire now pair each recorded line with
an exact copy first, then with a line whose code part (what pip
reads) is unchanged. A vendor path line with no tag is recognised by
its wheel file name, so vendor finds it in sync instead of
duplicating it. The kept-artifact hint now names the requirements
drift warning too.

Refs #977

Assisted-by: Claude Code:claude-opus-5-5
End-to-end with real pip: vendor six, then respace, extend or strip
the "# socket-patch vendor:" comment. A re-run of vendor must leave
requirements.txt alone, and vendor --revert must restore the
registry pin and remove the vendored wheel.

Refs #977

Assisted-by: Claude Code:claude-opus-5-5
Review follow-ups for the requirements revert fix:

- Two byte-identical vendor lines with different originals no longer
  swap originals on revert when only one line's comment was edited:
  records that wrote the same line take their lines in file order.
- The lockfile inventory also names an untagged vendored wheel line by
  its wheel, so scan keeps seeing the package after the comment is
  stripped, matching what vendor itself now reads.
- The untagged-line name comes from the existing vendored-path parser
  instead of a new one.

Refs #977

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 29, 2026 12:29
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

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.

After a comment-only edit, revert paired the recorded vendor line with
the bottom-most line pip reads the same. A bare manual copy of that
wheel line below the edited one won, so revert restored the pin over
the user's copy and left the vendor line in place. A same-code line
that still carries the vendor tag is now paired first.

Refs #977

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

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 50be7f8. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

Ready for review (burn-down agent).


Generated by Claude Code

Merged via the queue into main with commit e006861 Oct 29, 2026
50 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-pypi-requirements-revert-comment branch October 29, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants