Skip to content

Fix pip constraints files broken by link pins (#740) - #1726

Merged
Mikola Lysenko (mikolalysenko) merged 6 commits into
mainfrom
agent/fix-requirements-constraints-link
Oct 26, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 6 commits into
mainfrom
agent/fix-requirements-constraints-link

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #740

Summary

A common pip layout reuses requirements.txt as constraints (requirements-dev.txt with -c requirements.txt, or docs/requirements.txt with -c ../requirements.txt). Hosted and vendored modes rewrite the patched pin in requirements.txt into a link: a hosted name @ <url> or a vendored ./.socket/vendor/...whl path. pip 20.3.x and 21.0.x refuse any link in a constraints file ("Links are not allowed as constraints"), so after the rewrite every install that reads the file as constraints failed on those pips, while the scan reported plain success.

Both modes still rewrite the pin (pip 20.2 and 21.1+ install it fine), but now they name every file that reads an edited requirements file as constraints:

  • hosted (scan / get --mode hosted): redirect_requirements_constraints_link in warnings[]
  • vendored (vendor, scan --mode vendored, dry run included): vendor_requirements_constraints_link advisory

The warning comes on every run while the link pin is in place (re-runs and in-sync vendored runs too), not only on the run that writes it. Both codes are documented in CLI_CONTRACT.md.

Root cause

Neither writer (patch/redirect/requirements.rs::rewrite for hosted, vendor/pypi_requirements.rs for vendored) checked whether a sibling requirements file pulls the edited file in as -c / --constraint. The vendored walker already declined to edit constraints files, but nothing looked in the other direction.

Change

  • vendor/pypi_requirements.rs: the pip option-line reader now returns the line's nested file as Requirements or Constraints (NestedFile). It follows pip's handle_option_line: the first -r if the line has one, else the first -c, so -c a -r b is still an include of b only. -r and -c targets resolve through one helper (nested_files). New constraint_readers_in(view, targets) finds the readers in the root *.txt / *.in files, in root requirements*/ directories, and in the requirements-named files of other top-level directories (docs/requirements.txt). It skips hidden and dependency directories (node_modules, venv, build, ...) and never parses a file without the bytes -c.
  • hosted/engine.rs: rewrite() runs the check once, after the veto loop settles, for the confirmed requirements pins.
  • vendor/pypi.rs: on a wet run, the check uses the requirements_line wiring records. On an in-sync re-run it uses the ledger entry's records. On a dry run it uses the planner's files (or the superseded entry's records for a re-wire), including the uv / Pipenv exported-sibling cases.
  • Not detected: a constraints file given only on the pip command line (pip install -c requirements.txt). This is stated in the contract.
  • No wrapper (npm/, pypi/, gem/) changes needed: they only dispatch to the binary.

Test evidence

  • New e2e mode_migration_pypi::requirements_read_as_constraints_warn_in_both_modes: covers hosted scan + re-run, vendor --dry-run, vendor and the in-sync vendor re-run. Readers are requirements-dev.txt and requirements/test.txt. Non-readers (include.txt with -r, both.txt with -c x -r x) are not reported, and there is a control project with no reader.
    • Without the fix (core src/ stashed): FAILED, left: [] right: ["requirements-dev.txt", "requirements/test.txt"]
    • With the fix: ok
  • New unit tests: nested_file_reads_constraints_like_pip (all -c/--constraint spellings, -r precedence, -e, comments, empty --constraint=) and constraint_readers_resolve_against_the_reading_file (path resolution, docs/, hidden/dependency dirs, non-requirements files).
  • 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: 70 passed.
  • cargo test --workspace --all-features could not finish locally: building every test binary ran out of this sandbox's disk allowance twice. CI does the full run.
  • cargo test -p socket-patch-core --lib -- pypi_requirements: 59 passed, 1 failed. The failure is wire_failure_rolls_back_already_written_files, which also fails on unmodified main in this sandbox: it runs as root, so the read-only-directory trick doesn't make the write fail. It is not related to this change.

Per-issue checklist

Bugbot

Round 1 (at d74e734): 2 findings, both fixed in e0b290a with replies on the threads: the in-sync sibling dry run gave no advisory, and the in-sync rebuild gave no advisory.
Round 2 (at e0b290a): no issues found. CI on e0b290a: 37 success, 19 skipped, ci-ok success.

/code-review high

Fixed all 8 findings: dry-run/wet mismatch for sibling, rewire and supersede plans; dry-run targets guessed rather than planned; no warning on re-runs; docs/requirements.txt not searched; unbounded reads repeated per veto pass; duplicate package names; duplicated path resolver; an empty --constraint= swallowing the next word.

https://claude.ai/code/session_01Ty4yv3Ya36MGmarjxXTJTv


Note

Low Risk
Advisory warnings only; requirements rewrite behavior is unchanged and detection is read-only over project requirements files.

Overview
Fixes #740: when requirements.txt is reused as a constraints file (-c / --constraint from sibling *.txt / *.in), hosted and vendored rewrites still turn the patched pin into a link (hosted URL or vendored wheel path), but pip 20.3.x and 21.0.x reject links in constraint files.

The PR adds discovery of those constraint readers (root and requirements*/ trees, requirements-named files under other top-level dirs; skips hidden/dependency dirs) by extending requirements-file option parsing to distinguish -r vs -c, matching pip’s first--r-else--c rule. Hosted scan/get emits redirect_requirements_constraints_link in warnings[] for confirmed URL pins; vendored emits vendor_requirements_constraints_link on dry run, first wire, and every in-sync re-run while the link remains. Rewrites are unchanged; installs on affected pips still fail unless users upgrade pip or stop using -c on that file. CLI_CONTRACT.md documents both codes; integration and unit tests cover both modes and reader resolution.

Reviewed by Cursor Bugbot for commit d2f5cf2. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A common pip layout reuses requirements.txt as constraints, e.g. a
requirements-dev.txt with `-c requirements.txt`. Hosted and vendored
modes rewrite the patched pin there into a link (a URL or a vendored
wheel path), which pip 20.3.x and 21.0.x refuse in a constraints file
("Links are not allowed as constraints"). Installs on those pips then
failed with no hint from the scan.

Both modes still rewrite the pin, but now name every sibling file that
reads an edited requirements file as constraints:
redirect_requirements_constraints_link (hosted) and
vendor_requirements_constraints_link (vendored, dry run included).
The `-c` reader shares pip's option grammar with the `-r` include
reader, so `-c a -r b` still follows only `b`, as pip does.

Fixes #740

Assisted-by: Claude Code:claude-opus-5-5
Review follow-ups for the constraints-link warning:

- Hosted and vendored re-runs keep naming each `-c` reader while the
  link pin stays in place, not only the run that writes it. The hosted
  check now runs once after the rewrite settles, from the confirmed
  requirements pins, with each package named once.
- A vendored dry run takes its target files from the real plan (the
  planner's files, or the superseded entry's), so it matches the wet
  run for uv / Pipenv exports and re-wires.
- Readers are also found in other top-level directories when named
  like requirements (docs/requirements.txt), skipping hidden and
  dependency dirs; files without `-c` bytes are not parsed.
- `-r` and `-c` targets resolve through one helper, and an empty
  `--constraint=` no longer swallows the next word.

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

Comment thread crates/socket-patch-core/src/vendor/pypi.rs Outdated
Comment thread crates/socket-patch-core/src/vendor/pypi.rs
Bugbot follow-ups: an already-wired uv / Pipenv lock whose exported
requirements.txt is wired (or, in a dry run, would be) now names its
-c readers too, and so does an in-sync run that rebuilds a missing
wheel.

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.

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 26, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Labeled Ready for review (burn-down agent).

  • Head: e0b290aa9895d95120fc334046213a45e6e3e64e
  • CI: 56/56 checks passed or skipped (37 success, 19 skipped); no main-wide failures.
  • Bugbot: round 1 (d74e734) had 2 findings, both fixed in e0b290a and resolved; round 2 at e0b290a found no new issues.
  • Mergeable with main, no CHANGELOG.md change.
  • Reviewer focus: constraint_readers_in in vendor/pypi_requirements.rs (which directories are searched for -c readers) and the dry-run/re-run advisory paths in vendor/pypi.rs.

Slack announcement not sent this run (the Slack send tool was unavailable); the next run will retry.


Generated by Claude Code

constraint_readers_in also scans requirements-named files in other
top-level directories (docs/requirements.txt, the Read the Docs layout),
but both contract rows said only the root and requirements*/ dirs were
searched, so a user reading the contract would not expect the warning
for a docs/ reader.

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 26, 2026
@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.

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

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

Copy link
Copy Markdown
Collaborator Author

Labeled Ready for review again (burn-down agent).

  • Head: d2f5cf24a9ec1b25ff4d1c699882a8258b05b06f. The only change since the last label is the final reviewer's CLI_CONTRACT.md doc commit, which covers the docs/ constraint readers.
  • CI: 56/56 checks passed or skipped (37 success, 19 skipped). No main-wide failures.
  • Bugbot: reviewed d2f5cf2 with a clean check run. Both earlier review threads are resolved.
  • Mergeable with no conflicts. CHANGELOG.md is not touched.
  • Slack: not announced, because the Slack send tool was not available in this run.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. When a pip project reuses requirements.txt as a constraints file (-c requirements.txt in requirements-dev.txt, docs/requirements.txt, ...), socket-patch's hosted URL pin or vendored wheel path in that file breaks installs on pip 20.3.x / 21.0.x ("Links are not allowed as constraints"). The rewrite itself is unchanged; both modes now name each such reader in a warning (redirect_requirements_constraints_link hosted, vendor_requirements_constraints_link vendored) on every run while the link pin is in place.

Risk: low. Warnings only, and the reader scan is read-only. It is bounded to the root plus one level of top-level dirs, skips hidden and dependency dirs, and parses only files containing -c. No change to what gets written.

Look here

  • pypi_requirements.rs:1478:`` nested_file_with. The `-r` parser now also tracks `-c` and follows pip's "first `-r`, else first `-c`" rule. `requirements_includes` (used by the planner) still returns only `-r` targets.
  • pypi_requirements.rs:1326:``` constraint_readers_in, which decides which files are searched.
  • engine.rs:1789: hosted warning, emitted once after the veto loop settles, only for confirmed requirements pins.
  • pypi.rs:2374: dry-run targets per wiring plan (fresh, uv/Pipenv exported sibling, rewire, supersede).
  • mode_migration_pypi.rs:445: e2e over hosted scan + re-run, vendor dry run, wet run and in-sync re-run, with non-reader controls.

Verified

  • Read the full diff against the PR description. I checked the -c byte prefilter (it catches --constraint/--constr, and UTF-16 bypasses it), self-reference exclusion, and /-joined relative paths (fine on Windows). A -c given only on the pip command line is not detected, and the contract says so.
  • Ran cargo test -p socket-patch-core --lib -- constraint nested_file: 13/13 pass.
  • CI: 56/56 passed or skipped on d2f5cf2, ci-ok and clippy green. Bugbot reviewed d2f5cf2 clean; both earlier threads resolved. CHANGELOG.md is untouched.

Changes I made: none this pass. An earlier final-review pass added the docs/ reader wording to CLI_CONTRACT.md (d2f5cf2).

Open questions: none.

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


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

3 participants