Skip to content

Fix pylock wiring ignoring an out-of-root -r pin (#1851) - #1853

Open
Mikola Lysenko (mikolalysenko) wants to merge 7 commits into
mainfrom
agent/fix-pypi-out-of-root-include-pylock
Open

Mikola Lysenko (mikolalysenko) wants to merge 7 commits into
mainfrom
agent/fix-pypi-out-of-root-include-pylock

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

Fixes #1851

Summary

This covers a project whose requirements.txt pins a package only through a -r include outside the project root (-r ../shared/base.txt holding six==1.16.0), next to a pip lock pylock.toml or another tool lock. Before this PR, vendored mode wired only the lock and exited 0. vendor --check passed, and vex attested not_affected, while pip install -r requirements.txt installed the upstream release.

Now:

  • vendor / scan --mode vendored / the hosted → vendored eject refuse the package with pypi_requirements_outside_root, which is the code the requirements-only path already uses for the same pin. They write nothing for it. This applies beside a pylock and beside a governing tool lock (uv, Poetry, PDM, Pipenv). Before, the tool-lock case didn't even name requirements.txt as a loser, because it only checked the root file.
  • vendor --check and vex see a lock already wired beside such a pin (vendored before this fix, or a pin added later) as contested. vendor --check exits 1 and vex omits the package, the same as for an in-root pin (CLI_CONTRACT.md "Contested locks").

Root cause

Two code paths both skip a pin in an out-of-root include:

  • Planner: detect_pypi_flavor (vendor/pypi.rs) gets sibling_pin_wirable == false for the read-only include, so it names requirements.txt a pypi_multiple_lockfiles loser ("retain their existing sources") and wires the lock.
  • Discovery: extract_requirements (vex/discover/pypi_other.rs) reads only requirements_include_names_in, which drops out-of-root includes. Nothing contests the lock's wiring.

Change

  • pypi_requirements::pin_outside_root finds an out-of-root include through which pip installs the release from a non-vendored source:

    • an == / === pin (extras and per-requirement options allowed)
    • a direct reference, or an editable checkout
    • a bare URL / VCS / path line whose filename or #egg= names the package

    It decodes the file the way pip does, so a UTF-16 pip freeze export counts too. The planner and discovery read these lines through the same helpers: utils::requirements::{editable_location, egg_name} and pypi_other::foreign_release. detect_pypi_flavor refuses through refuse_pin_outside_root in both lock branches. The refusal message is shared with plan_requirements (outside_root_refusal).

  • pypi_requirements::requirements_outside_root_names_in lists the out-of-root includes the root reaches, through in-root files and through each other. Discovery reads them in the same install tree as the root file (so, as with Lockfile discovery treats a requirements.txt -r include as a competing lock, so after a hosted rewrite vex attests nothing (exit 2) and rollback refuses (exit 1), although pip install -r requirements.txt installs the patched wheel #1086, they never contest a wiring inside that tree). It reads them as evidence only: registry pins via registry_pin, editable checkouts, and non-Socket direct references and bare lines via foreign_reference. An include that can't be read or decoded is reported as lockfile_unreadable instead of being dropped silently. Nothing in them is read as wiring, because neither writer edits them. They share the MAX_REQUIREMENTS_FILES budget.

No wrapper changes: npm/, pypi/ and gem/ only dispatch to the binary. No new error code.

Tests (red → green)

Issue facet Test Without fix With fix
#1851 planner: exact pin in ../base.txt beside pylock / uv.lock / Pipfile.lock / poetry.lock refuses; another release or an in-root-only package doesn't core vendor::pypi::tests::exact_pin_in_an_out_of_root_include_refuses_beside_a_lock FAIL (unwrap_err on Ok, pypi.rs:4179) pass
#1851 discovery: out-of-root pin (direct and via a nested out-of-root include) contests a vendored pylock; another release / other package doesn't core vex::discover::pypi_locks::tests::out_of_root_include_pin_contests_a_vendored_pylock (also extras/===, direct reference, editable, bare VCS and path lines, and an undecodable include diagnosed) FAIL (refs still holds the pylock wiring) pass
#1851 planner, every shape (review follow-up): extras, ===, --config-settings, direct reference, -e/--editable= egg, bare git+…#egg= / URL / path wheels, UTF-16 export; other-release and other-package bare wheels don't refuse core vendor::pypi::tests::every_installing_shape_in_an_out_of_root_include_refuses FAIL (first cut matched only == in UTF-8; before 713eec9, Ok(PythonLocks) for git+…#egg=six) pass
#1851 CLI: vendor refuses and leaves every file byte-identical; with another release in the include, the pylock vendors and vendor --check passes; once the include pins the vendored release, vendor --check fails as contested cli mode_migration_pypi::pip_lock_beside_an_out_of_root_include_pin_refuses FAIL (vendor exit 0) pass

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: all pass except 4 known permission-denial tests that can't fail a write as root in this sandbox (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_rolls_back_…). The other agent PRs see the same 4, and none of them touches this change.
  • cargo test -p socket-patch-cli --all-features for mode_migration_pypi (75), e2e_vex (45), e2e_vex_vendor (53), e2e_vex_redirect (53), in_process_vendor_pypi_takeover, scan_requirements_lock_only, scan_vendor_requirements_unwired, hosted_superseding_pypi, vendor_ledger_schema_e2e and e2e_pypi: all pass.
  • CI on 713eec9 is green (ci-ok), including the real-pip e2e_vex_build legs. Its first attempt lost 3 jobs to Depot runners (no logs, 10-minute cutoff, Depot runners lost mid-job evicted 2 merge-queue entries in 27h #1660), and one re-run of those jobs passed.

Per-issue checklist

/code-review high

Fixed in the review follow-up commit:

  • Extras / === / per-requirement-option pins, direct references, editables and UTF-16 files in an out-of-root include were missed by the planner, and direct references were also missed by discovery.
  • An unreadable out-of-root include was dropped silently. It's now diagnosed.
  • The dead wired_with_lock parameter is removed.

Bugbot (fixed in 713eec9): bare URL / VCS / path lines in an out-of-root include didn't refuse in the planner, though discovery contested them. Both sides now read them through one helper, pypi_other::foreign_release. Bugbot's re-review of 713eec9 found no new issues.

Not changed:

Notes for review

https://claude.ai/code/session_011f9aeR3uZQzo93CSAMyih2


Note

Medium Risk
Changes PyPI vendoring routing and VEX discovery around requirements includes; incorrect matching could block legitimate vendoring or miss contested installs, but behavior is fail-closed and heavily regression-tested.

Overview
Fixes #1851: when requirements.txt pulls an exact package pin from a -r include outside the project root (e.g. -r ../shared/base.txt) next to a pylock or other governing lock, vendored mode no longer wires only the lock and reports success while pip install -r still installs the upstream release.

Planner (vendor / vendored scan / eject): detect_pypi_flavor now calls refuse_pin_outside_root, which uses pin_outside_root to walk the requirements tree and refuse with pypi_requirements_outside_root (same code as the requirements-only path) when pip would install the target from that unreadable include—covering == / === pins, direct references, editables, bare URL/VCS/path lines, UTF-16 exports, and fail-closed behavior when an include exists but cannot be decoded.

Discovery (vex / vendor --check): requirements_outside_root_names_in plus outside_root_line treat those includes as evidence only (not wiring), so an already-wired lock beside a matching out-of-root pin is contested like an in-root pin. Shared parsing helpers (editable_location, egg_name, bare_line_release / foreign_release) keep planner refusal and discovery contest aligned.

Adds CLI and core tests for refusal, --check contest, and multiple requirement line shapes.

Reviewed by Cursor Bugbot for commit 70b2147. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A requirements.txt that pins a package only through a -r include
outside the project root (-r ../shared/base.txt) was named a
pypi_multiple_lockfiles loser next to a pylock, uv.lock, Pipfile.lock
or other tool lock. The lock was wired, the run exited 0, and
`pip install -r requirements.txt` kept installing the upstream
release.

Vendor can't edit that include, so it now refuses the package with
pypi_requirements_outside_root, the same code the requirements-only
path already used for this pin.

Refs #1851

Assisted-by: Claude Code:claude-opus-5-5
`vendor --check` and `vex` read only the root requirements.txt and its
in-root -r includes. A pin in an include outside the project root
(-r ../shared/base.txt) was never seen, so a vendored pylock that
wired the same release passed `vendor --check`, and `vex` attested
not_affected, while `pip install -r requirements.txt` installed the
upstream release.

Discovery now reads the out-of-root includes the root reaches. Their
registry pins and editable checkouts count as evidence against a
lock's wiring, the same as an in-root pin. Nothing in them is read as
wiring, because neither writer edits them.

Refs #1851

Assisted-by: Claude Code:claude-opus-5-5
Covers the #1851 project end to end through the CLI. `vendor` refuses
and leaves every file untouched. Another release in the include
vendors cleanly. Once the include pins the vendored release,
`vendor --check` fails as contested.

Refs #1851

Assisted-by: Claude Code:claude-opus-5-5
Review follow-up for #1851. The refusal and the contest only matched
a clean `==` pin read as strict UTF-8. pip also installs the release
from these lines in an out-of-root include:
- an extras pin, `===`, or a pin with per-requirement options
- a direct reference, or an editable checkout
- a UTF-16 `pip freeze` export
vendor wired the lock beside any of them, and only an `==` pin
contested it afterwards.

The planner and discovery now decode these files the way pip does
and count every one of these shapes. An include that can't be read
or decoded is now reported as unreadable instead of dropped silently.
editable_location and egg_name move into utils::requirements so both
sides parse lines with the same grammar.

Refs #1851

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 29, 2026 21:09
@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.

Bugbot follow-up for #1851. A bare archive URL, VCS URL
(git+https://...#egg=six) or wheel path in an out-of-root include
installs that artifact. Discovery already counted these lines as
evidence, but the planner didn't refuse them, so `vendor` could still
exit 0 and `vendor --check` would then fail.

Both sides now read these lines through one helper,
pypi_other::foreign_release. A line whose filename or #egg= names
the package refuses: at the filename's version, or at any version
when the line names none.

Refs #1851

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

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] CI on eb7935d didn't produce test results: 14 jobs failed together at 21:23:31 UTC. They span cargo, node-addon, yarn, pnpm, vlt, gradle, maven, gem, composer and pip-vex, most of which this diff doesn't touch. All ran on Depot runners, and none has a log (HTTP 404) or check output. That is the lost-runner pattern from #1660, not a test failure. The push of 713eec9 (Bugbot fix) re-runs the full suite, so that serves as the re-run. I'll treat any failure there as real.


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

Copy link
Copy Markdown
Collaborator Author

[agent] On 713eec9, three jobs failed the same way as on eb7935d: coverage (1), coverage (2) and e2e-build (ubuntu-latest). Each ran on Depot runners and ended exactly 10 minutes after it started (21:29:58 → 21:39:58). None has a log (HTTP 404) or check output, so no test body reported a failure. This matches the lost-runner pattern in #1660, not this diff. I'll re-run the failed jobs once when the run finishes (GitHub refuses a re-run while it's still running). If they fail again with a real log, I'll treat that as this PR's failure.


Generated by Claude Code

@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 713eec90deb121456ebadce913de8da40520c50c.


Generated by Claude Code

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: Fixes #1851. Before, when a pylock.toml or a tool lock (uv, Poetry, PDM or Pipenv) sat beside a requirements.txt whose -r include outside the project root pinned the target release, vendoring wired the lock, called requirements.txt a loser and exited 0, while pip would still install the registry pin. The planner now refuses with the existing pypi_requirements_outside_root code. VEX discovery also reads out-of-root includes, as evidence only, so an already-wired lock shows as contested: vendor --check exits 1 and vex leaves the package out.

Risk: medium. It fails closed: there is no new write path and no new error code. The risk is the user-visible change. Projects that wired a lock beside such a pin used to pass and are now refused or flagged as contested. That is the intended fix, but users will notice it.

Look here:

  • pypi.rs:418 and pypi.rs:473: the refusal in the pylock and tool-lock branches
  • pypi_requirements.rs:463-512:`` pin_outside_root and `installs_outside_vendor`
  • pypi_requirements.rs:1324:``` requirements_outside_root_names_in
  • vex/discover/pypi_other.rs:275-322:```` discovery reads the includes as evidence

Verified: I read the full diff against #1851.

  • Path safety: ../ and absolute includes were already read before this PR. Reads go through the FIFO-safe read_regular_to_bytes, and nothing is written outside the root.
  • Tests: they exercise the change and would fail without it. The planner test would get Ok, and the CLI test checks every file is byte-identical and that vendor --check fails as contested.
  • Moving editable_location and egg_name into utils/requirements.rs is a clean extraction.
  • CHANGELOG.md is untouched.
  • CI is green on 713eec9 (31 passed, 19 skipped, ci-ok and clippy both green). Bugbot found no new issues on the head, and its one earlier thread is resolved.

Changes I made: none.

Open questions (non-blocking; edge cases, none a regression from main):

  • If an out-of-root include can't be read or decoded, the planner skips it and wires the lock, while discovery reports lockfile_unreadable for the same file (pypi_requirements.rs:472).````` Should the planner fail closed here too?
  • The planner refuses a name @ url or editable line at any version, while discovery contests only a matching artifact version. The stricter side is the planner, so this is not a safety gap.
  • The planner's vendor-dir check uses contains(".socket/vendor/"), so Windows .socket\vendor\ paths refuse (fail closed). Discovery normalizes backslashes.
  • The description says "exact pin", but direct references and editables also trigger the refusal. The doc comments say so; the description doesn't.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.


Generated by Claude Code

Final-review follow-up for #1851. If an out-of-root -r include
existed but couldn't be read or decoded, the planner skipped it and
wired the lock anyway, while discovery reported the same file as
unreadable. pip may still install pins from that file, so vendor
now refuses the package with pypi_requirements_outside_root and
names the file.

The vendor-dir check for direct references also normalizes Windows
backslashes now, matching discovery.

Refs #1851

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

Copy link
Copy Markdown
Collaborator Author

[agent] On the final review brief's open questions:

  • Unreadable / undecodable out-of-root include: fixed in 70b2147. The planner now refuses with pypi_requirements_outside_root and names the file, matching discovery's lockfile_unreadable. Before, it skipped the file and wired the lock. Test: every_installing_shape_in_an_out_of_root_include_refuses (undecodable case). It fails without the change and passes with it.
  • Windows .socket\vendor\ paths: the planner's vendor-dir check now normalizes backslashes, as discovery already did (70b2147).
  • Planner stricter than discovery for name @ url / editable at any version: left as is. The planner is the side that fails closed, so the mismatch can only refuse, never attest.
  • Description wording: the "Change" section lists every shape that refuses, including direct references and editables. "Exact pin" appears only in the first test row, which covers that test's case.

BugBot review


Generated by Claude Code

@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 70b2147. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief (updated for 70b2147)

What it does: Fixes #1851. When a pylock.toml or tool lock sits beside a requirements.txt whose -r include outside the project root pins the target release, vendor now refuses with pypi_requirements_outside_root instead of wiring the lock while pip still installs the registry pin. VEX discovery reads the same includes, so an already-wired lock shows as contested. Since the last brief, 70b2147 also refuses when such an include exists but can't be read or decoded, and normalizes Windows backslashes in the vendor-dir check.

Risk: medium. It fails closed, with no new write path and no new error code. The change is user-visible: projects that wired a lock beside such a pin, or beside an unreadable out-of-root include, are now refused.

Look here:

  • pypi_requirements.rs:465-504:`` pin_outside_root, including the new unreadable-include refusal (any non-`NotFound` read or decode error on an out-of-root file)
  • pypi_requirements.rs:527:``` backslash normalization in installs_outside_vendor
  • pypi.rs:261: refuse_pin_outside_root now returns the refusal as built
  • pypi.rs:4245-4261: the new undecodable-include test

Verified:

  • I read the 713eec9..70b2147 diff (2 files, +50/-12). It answers two of the earlier open questions; the other two were answered on the thread and need no code.
  • An unreadable include refuses only when it is outside the root and exists. A missing include and in-root read errors behave as before.
  • Ran cargo test -p socket-patch-core --lib vendor::pypi locally: 514 passed. The 2 failures (wire_failure_rolls_back_already_written_files, wire_write_failure_maps_error_and_leaves_lock_untouched) are permission-based tests that also fail on main when run as root. They are not caused by this PR.
  • CI is green on 70b2147 (ci-ok and clippy both green). Bugbot found no new issues on 70b2147. No unresolved threads. CHANGELOG.md is untouched.

Changes I made: none.

Open questions: none blocking. Note that an unreadable out-of-root include now refuses every PyPI package in that project, not just the one it might pin. That fails closed, but users will see it.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: re-verified Ready for review on the new head 70b2147 (label was applied at 713eec9). CI 50/50 green; mergeable, no conflicts. Bugbot reviewed 70b2147: no findings; no open review threads. Slack re-announcement not sent this run (Slack posting unavailable).


Generated by Claude Code

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

2 participants