Skip to content

Fix extras pin un-hosted beside a pylock (#1794) - #1796

Open
Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
agent/fix-pypi-unwirable-sibling-pin
Open

Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
agent/fix-pypi-unwirable-sibling-pin

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #1794

Summary

A pip project whose requirements.txt pins a package with extras (six[foo]==1.16.0) beside the pylock.toml that pip lock wrote lost its patch on a hosted → vendored switch (plain vendor eject or scan --mode vendored). The switch un-hosted requirements.txt, wired only the pylock, said requirements.txt "retains its existing sources", and exited 0. pip install -r requirements.txt then installed unpatched six, and vendor --check failed straight afterwards.

Vendored mode now refuses that package with pypi_extras_unsupported, the code the requirements-only wiring already uses for the same pin. The takeover rolls its restore back, so both files keep their hosted pins byte for byte and the run exits 1 with the reason. This matches the contract's "a takeover the vendored backend does not carry through keeps the hosted pin".

Root cause

#1759 co-wires a requirements pin with the lock only when sibling_pin_wirable accepts it. A pin it rejects fell through as a pypi_multiple_lockfiles "loser". That is accurate for a fresh project, but wrong after a takeover, because the upstream restore has just rewritten the file's source. An exact pin with extras is the one shape hosted mode pins (six[foo] @ https://…) that the vendored wiring can't express.

Change

Tests (red → green)

Issue / case Test Before fix After
#1794 eject and scan --mode vendored over hosted six[foo]==1.16.0 + pylock mode_migration_pypi::pip_lock_hosted_to_vendored_refuses_an_extras_pin failed: exit 0, applied pass: exit 1, pypi_extras_unsupported, both files byte-identical to hosted, nothing vendored
Already-wired pylock with an extras pin added later mode_migration_pypi::pip_lock_rerun_keeps_a_wired_lock_beside_an_extras_pin fails with the first cut of this PR (refused on re-run) pass: in sync, exit 0
Helper: == / === / normalized name and markers / -r include / UTF-16 refuse; other version, range, wirable pin, already vendored don't pypi_requirements::tests::fresh_sibling_pin_refuses_an_exact_extras_pin n/a (new helper) pass

Commands run locally on the branch:

  • cargo fmt --all -- --check: clean
  • cargo clippy --workspace --all-features -- -D warnings: clean
  • cargo test -p socket-patch-cli --all-features --test mode_migration_pypi: 76 passed
  • cargo test --workspace --all-features: 15197 passed, 11 failed, all environmental: 9 read-only-directory tests that cannot fail as root in this container (copy_tree, vlt_heal, pypi_poetry/pypi_requirements wire-failure rollback, in_process_redirect, repair, vlt), and 2 e2e_bun_lockb tests that need patches-api.socket.dev, which this sandbox can't reach. None of them touch the changed paths.
  • cargo test -p socket-patch-cli --all-features --test e2e_vendor_pypi_build -- --include-ignored (uv 0.11.32): 56 passed

No wrapper changes are needed. npm/, pypi/ and gem/ only dispatch to the binary.

/code-review high

I ran it on the first cut and fixed these findings in 2d6811c: a refusal on an in-sync re-run or supersede, === pins missed, UTF-16 files missed, the vendor-line skip, the refusal coming before the lock's own guards, and duplicated parsing. Not changed: on a fresh wiring the requirements tree is now read twice (fresh_sibling_pin, then sibling_pin_wirable). That happens once per vendored PyPI package, on small files, so I left the extra read rather than restructure sibling_pin_wirable.

Follow-ups

  • Poetry and PDM don't co-wire an exported requirements.txt at all, so an extras pin beside them is still a loser after a takeover, as is any plain exact pin. That gap predates this PR and belongs to the broader "wire exports for every tool lock" work.

https://claude.ai/code/session_011f9aeR8BmvdRuS4i2YzgEV


Note

Medium Risk
Changes PyPI vendored wiring and hosted→vendored takeover failure modes, but behavior is narrow (extras-shaped requirements pins), guarded by new tests, and in-sync projects are explicitly preserved.

Overview
Fixes hosted → vendored takeover when requirements.txt beside a pylock.toml / uv / Pipenv lock pins the same release with extras (e.g. six[foo]==1.16.0) or has a single unpinned extras line (six[foo]). Vendored wheel lines cannot express extras, so the switch used to un-host the requirements pin, wire only the lock, exit 0, and leave pip install -r on unpatched bytes.

Fresh lock wiring now goes through fresh_sibling_pin, which fails the package with pypi_extras_unsupported (same as requirements-only wiring) so the takeover rolls back and both files keep their hosted pins. PinScan.exact_extras drives that check (including UTF-16 roots the planner cannot read). In-sync re-runs and supersede paths still treat an extras sibling as a pypi_multiple_lockfiles loser, so projects already vendored before this change do not start failing.

CLI_CONTRACT.md documents the refusal; integration and unit tests cover hosted→vendored refusal, in-sync re-run, and pin-shape edge cases.

Reviewed by Cursor Bugbot for commit b6e2d88. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A pip project whose requirements.txt pins a package with extras
(six[foo]==1.16.0) beside the pylock.toml `pip lock` wrote from it lost
its patch on a hosted -> vendored switch. The switch un-hosted
requirements.txt, wired only the pylock (a wheel path line can't carry
extras), warned that requirements.txt "retains its existing source",
and exited 0, so `pip install -r requirements.txt` went back to the
unpatched PyPI release and `vendor --check` then failed.

Vendored mode now refuses such a package with pypi_extras_unsupported,
as it already does for the same pin with no lock beside it. The switch
rolls its restore back, so both files keep their hosted pins and the
run exits 1 with the reason. The same applies to a requirements.txt
exported beside Pipfile.lock or uv.lock, which is wired with its lock
the same way.

Fixes #1794

Assisted-by: Claude Code:claude-opus-5-5
The first cut refused a package whenever requirements.txt pinned it
with extras beside a pylock, Pipfile.lock or uv.lock. That also failed
every re-run of a project whose lock was already vendored, and blocked
a newer patch from replacing it. The refusal now applies only when the
lock is about to be wired fresh; an already-wired lock re-runs in sync
and the extras pin is still named as a file that installs unpatched.

It also now catches an `===` pin, a pin in an -r include, and a
UTF-16 requirements.txt, and skips a file that already carries a
vendored line for the package.

Refs #1794

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

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] e2e (ubuntu-latest, e2e_vendor_composer_build …) on 2d6811c failed in "Setup Python" before any test ran: getaddrinfo EAI_AGAIN raw.githubusercontent.com, a runner DNS failure. That leg covers Composer, uv redirect and NuGet suites, none of which this diff touches. I re-ran the failed job once.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


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.

Comment thread crates/socket-patch-core/src/vendor/pypi_requirements.rs
Hosted mode rewrites a requirements file's one unpinned line for the
package too (six[foo] -> six[foo] @ ...), so after a hosted -> vendored
switch the takeover restore un-hosts that line as well. fresh_sibling_pin
only refused exact ==/=== extras pins, so it wired the lock alone and
pip install -r went back to unpatched bytes, the #1794 failure. Count the
file's sole unpinned extras line as the hosted-rewritable pin too.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011f9aeRBd9SafVxvTmiMFa7
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


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.

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

Copy link
Copy Markdown
Collaborator Author

Ready for review at 8eafd68 (burn-down agent).

  • CI: 50/50 check runs on 8eafd68 green (31 success, 19 skipped); mergeable, no conflicts.
  • Bugbot: on 2d6811c it found 1 real issue: a sole unpinned extras line (six[foo]), which hosted mode also rewrites, was not refused, so the lock was wired alone. Fixed in 8eafd68, with unit cases added, and the thread is resolved. Bugbot re-reviewed 8eafd68 and found no new issues.
  • Reviewer focus: scan_pins in vendor/pypi_requirements.rs. The unpinned-extras refusal applies only when that line is the file's only line naming the package, which matches the hosted rewriter's own uniqueness rule.
  • Local check: wire_failure_rolls_back_already_written_files fails only when run as root; it fails the same way without this change and passes in CI.

Generated by Claude Code

8eafd68 also refuses a file's only line for the package when it is an
unpinned extras requirement (six[foo]), since hosted mode rewrites that
line too, but the pypi_multiple_lockfiles row only named the exact
six[foo]==1.16.0 pin. Name both shapes so the contract matches the code.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mikolalysenko Mikola Lysenko (mikolalysenko) removed 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

bugbot run


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review of 8eafd68. Nothing blocks merging. I ran mode_migration_pypi (76/76 pass). With the merge-base pypi.rs swapped in, pip_lock_hosted_to_vendored_refuses_an_extras_pin fails, so the test catches the bug.

I pushed b6e2d88, a docs-only change. CLI_CONTRACT.md now also names the lone unpinned six[foo] refusal that 8eafd68 added. I removed Ready for review until CI and Bugbot pass on the new head. I'll post the approver brief after that.

Notes for the approver (non-blocking):

  • The refusal also covers fresh vendors. A lock wired fresh next to an extras pin now exits 1 with pypi_extras_unsupported, even in a project that was never hosted. Before, it succeeded with a pypi_multiple_lockfiles warning. This matches the requirements-only wiring and fails closed, but existing users of that layout will see a new failure.
  • Two variants of Hosted → vendored over an extras pin (six[foo]==X) in requirements.txt beside a pip lock pylock.toml still un-hosts requirements.txt and leaves it unpatched, exit 0 (#1758 residual) #1794 remain, and both already exist on main. These are candidates for a follow-up issue:
    • a lone unpinned six (no extras) that hosted mode rewrites;
    • six[foo] together with six==1.16.0 in the same file. Hosted mode rewrites the exact line, and the lock is still wired alone.
  • Partial coverage. Only the pylock path has an end-to-end test. The uv.lock and Pipfile.lock fresh paths are tested at unit level through fresh_sibling_pin.

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 b6e2d88. 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

Burn-down agent: labeled Ready for review at b6e2d88.

  • CI: 50 check runs: 31 success, 19 skipped, 0 failing on b6e2d88; mergeable, no conflicts.
  • Cursor Bugbot: reviewed b6e2d88, no issues found; no unresolved review threads.
  • No CHANGELOG.md change.
  • Slack announcement could not be sent in this run (no Slack send tool available); the next run will retry.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review: one product question before this can be briefed for approval.

The fix is motivated by the hosted → vendored takeover (#1794), but fresh_sibling_pin runs on every fresh lock wiring (pypi.rs:1120, :1161, :1356). Since 8eafd68 it also refuses when the file's only line for the package is an unpinned extras requirement (pypi_requirements.rs:202).``

Concrete case: a plain project that was never hosted, with requirements.txt = six[foo] as a loose top-level input and a uv.lock / pylock.toml pinning six 1.16.0. On main, vendor wires the lock (uv and pip-lock installs patched) and names requirements.txt as a pypi_multiple_lockfiles loser. With this PR, vendor refuses the package with pypi_extras_unsupported and nothing gets patched. The same file with plain six (no extras) is still just a loser, so the two shapes now disagree.

Question: should the extras refusal apply only when the requirements pin was hosted (the takeover case this PR fixes), or to every fresh vendored wiring, as now? If takeover-only, the narrowing would be to refuse only when the restore just un-hosted that line, or at least to drop the unpinned-extras refusal outside a takeover. The exact six[foo]==1.16.0 refusal on a fresh project is defensible either way, because pip install -r would install the unpatched release.

Everything else checks out: CI green on b6e2d88, Bugbot clean, CHANGELOG.md untouched. I set agent:needs-human. Remove it once this is decided.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

2 participants