Skip to content

Fix requirements pin spellings not contesting VEX (#1602) - #1632

Merged
Mikola Lysenko (mikolalysenko) merged 6 commits into
mainfrom
agent/fix-requirements-pin-contest-pep440
Oct 21, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 6 commits into
mainfrom
agent/fix-requirements-pin-contest-pep440

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #1602

Summary

A requirements.txt line that pins the wired release as ===1.2.3, ==1.2.3.0, ==v1.2.3 (or six==1.16 for 1.16.0) installs the same upstream wheel as ==1.2.3. Before this PR, only the canonical spelling contested another lock's Socket wiring, so vex attested not_affected (exit 0) for a project whose pip install -r requirements.txt ships the unpatched release. Now every one of those spellings withholds the statement with patched_ref_unattributable, the same way ==1.2.3 already did.

Root cause

The requirements reader (vex/discover/pypi_other.rs, registry_pin) took evidence only through utils::requirements::exact_pin. That function rejects === and versions that don't start with a digit, and it returns 1.2.3.0 verbatim. The cross-lock contest (ResolvedElsewhere::covers) then compared canonical purl strings, so @1.2.3.0 never matched @1.2.3.

Fix

  • registry_pin records any requirement that can only select one release:
    • an == pin in any valid PEP 440 spelling;
    • an === pin, recorded with the new ResolvedElsewhere::as_spelled flag because pip compares === as a string.
  • ResolvedElsewhere::covers compares PyPI versions under PEP 440 equality (purl_key::pypi_same_release), except for as_spelled entries. It checks names first, so the PEP 440 parse runs only for pairs with the same name.
  • The contest's "this tree wires the release itself" exclusion uses the same equality. Without that, a tree that wires six@1.16.0 while another lock wires six@1.16 would now be counted as a contester.
  • Ranges and wildcards (~=1.2.3, ==1.2.*) are still not evidence. What they install depends on the index, which the issue also calls a design question.
  • CLI_CONTRACT.md "Contested locks" now documents this. The exact_pin comment in the lock inventory says how the two readers differ.

The npm, PyPI and gem wrappers don't need changes: this is Rust discovery logic only.

Tests

Issue case Test Without fix With fix
#1602 ===X, ==X.0, ==vX, padded + marker (real CLI, Pipfile.lock wired) e2e_vex_lockfile pipenv::requirements_pin_spellings_contest_the_lock_wiring FAILED (vex exit 0, at vex_pipenv_pip_common/mod.rs:1385) ok
#1602 every spelling (===, ==1.16.0.0, ==v1.16.0, ==1.16, case/marker/extras/hash) contests; ==1.16.1, ~=, ==1.16.*, ===1.16, other name don't socket-patch-core vex::discover::pypi_other::tests::issue_1602_requirements_pin_spellings_contest_wiring (new, same file as the fix) ok
A tree that wires another spelling of the release isn't a contester ...::issue_1602_a_tree_wiring_another_spelling_contests_nothing FAILED with the old exact-string wiring check ok

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 -- vex::discover production_code_classifies_purls: 416 passed on 1b96ed7
  • cargo test --workspace --all-features --no-fail-fast (on 65efed5): 232 test targets ok. The 10 failures are sandbox-only. Nine are read-only-permission / write-failure tests that can't fail when running as root (uid 0): copy_tree::relax_loop_must_not_traverse_symlinked_root ("removal cannot succeed under a read-only parent"), vlt_heal::an_unremovable_hidden_lock…, pypi_poetry::wire_write_failure…, pypi_requirements::wire_failure_rolls_back…, 3 in in_process_redirect, 2 in repair. The tenth, e2e_bun_lockb::binary_shared_bundled_record_hosted_pin_is_managed, failed because the sandbox proxy blocked patches-api.socket.dev. The one real failure, the purl-type lint, is fixed in 1b96ed7.

/code-review high

Fixed: the wiring exclusion compared strings exactly; === was read under PEP 440; duplicate helper; the hard-coded version in the e2e case; the stale exact_pin comment.
Not fixed, with reasons:

  • A public ==X pin also matches X+local under PEP 440. That gap already existed before this PR (a canonical ==2.1.0 never contested a wired 2.1.0+cpu). Lock entries resolve exact versions, so folding local labels into covers would be wrong for them. It needs a requirements-only rule, which is a separate follow-up.
  • Move PEP 440 identity into PurlKey. PurlKey keys maps across every ecosystem, and PyPI is kept spelling-exact there on purpose (pypi_equivalent_purls exists for this). Changing it would touch much more than this fix needs.
  • Regex cost in the contest loop. The names are compared first, so the parse runs only for ref and entry pairs of the same package.
  • Scan can't rewrite an === line, so the contest persists. That's correct: pip installs upstream for that line, so the attestation must stay withheld until the user changes it. === now matches only its exact string.

https://claude.ai/code/session_01FuB4p2UQANHrpx4dHRwddS


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A requirements.txt line that pins the wired release as `===1.2.3`,
`==1.2.3.0` or `==v1.2.3` installs the same upstream wheel as
`==1.2.3`, but only the canonical spelling contested another lock's
Socket wiring. `vex` attested `not_affected` for a project whose
`pip install -r requirements.txt` ships the unpatched release.

The requirements reader now records any pin that can only select one
release (an `==` pin in any valid PEP 440 spelling, or `===`), and
the cross-lock contest compares PyPI versions under PEP 440 equality,
the way pip resolves the pin. Ranges and wildcards are still not
evidence, since what they install depends on the index.

Fixes #1602

Assisted-by: Claude Code:claude-opus-5-5
Review follow-ups for the pin-spelling contest:

- pip compares a `===X` pin as a string, so it now contests only a
  wiring of exactly that version string. `===1.16` no longer withholds
  an attestation for release 1.16.0, which pip can't install from it.
- A lock tree that wires the release itself, under another PEP 440
  spelling, is no longer counted as a contester of the other lock.
- Reuse purl_key::pypi_same_release instead of a second copy of it.

Assisted-by: Claude Code:claude-opus-5-5
The repo's lint test requires purl types to be read through
Ecosystem::from_purl rather than an inline prefix check.

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

Copy link
Copy Markdown
Collaborator Author

Labeled Ready for review (burn-down agent).

  • Head: 1b96ed7
  • CI: 86/86 green (36 success, 50 skipped). No main-wide failures.
  • Bugbot: reviewed 1b96ed7, no findings. 0 open threads.
  • Mergeable with no conflicts. CHANGELOG.md is not touched.
  • Reviewer focus: requirements pin spelling normalization in vex/discover/pypi_other.rs and vendor/lock_inventory/pypi.rs, plus the CLI_CONTRACT.md wording.

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


Generated by Claude Code

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 20, 2026
uv 0.13.2 and later refuse the `environments = []` pylock that Pipenv
2026.8.0 writes ("The target environment is not supported by the
lockfile"). setup-uv had no version pin, so the merge-queue
`e2e_vex_build pipenv::` leg started failing with no change on main,
and that dequeued this PR. Pin uv to 0.13.1, as #1639 does. The change
becomes a no-op once #1639 lands.

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

Copy link
Copy Markdown
Collaborator Author

The merge queue dequeued this PR because a CI check failed. The failure isn't this PR's.

The PR needs to be re-queued after CI on d72ed0d passes. The push may also have dismissed the earlier approval.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Wenxin Jiang (@Wenxin-Jiang) this changed after your approval (on 1b96ed74), so it needs another look before it goes through the merge queue.

Besides merges from main, the only new commit is d72ed0d: the CI-only uv pin from #1639 (version: "0.13.1" on the setup-uv steps in ci.yml and pipenv-compatibility.yml, 12 added lines, byte-for-byte the same as #1639). No crate code changed. Without it the pipenv:: e2e leg fails in the merge queue on uv 0.13.2+. It no-ops once #1639 lands.

If that's fine with you, re-approving d72ed0d0 is all this needs.


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.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Ready for review re-confirmed at the new head (burn-down agent).

  • Head: d72ed0d
  • CI: 54/54 green (35 success, 19 skipped). No main-wide failures.
  • Bugbot: reviewed d72ed0d, no findings; no unresolved review threads.
  • Mergeable: clean. The head changed after the earlier approval (main merges plus the setup-uv pin port), so it needs a fresh approval before re-queueing.
  • Slack: not announced this run (send tool unavailable); the next run will retry.

Generated by Claude Code

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 21, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

The merge queue dequeued this PR again. The uv pin worked: the pipenv:: leg passed. This time a different check failed, and it isn't this PR's either.

  • Failing check: coverage (run 39655972927). Test vendor::pypi_hatch::tests::hatch_on_an_absolute_path_entry_passes_the_version_gate failed with pypi_hatch_unsupported ("require Hatch >=1.2 on PATH").
  • Why it isn't this PR's: this PR doesn't touch vendor/pypi_hatch.rs. In the same queue the run for Fix uv restore ignoring lock file filters (#1604, #1613) #1614 passed coverage, and Fix uv restore ignoring lock file filters (#1604, #1613) #1614 sits behind this PR, so its merge group contains this PR's changes. The same code passed once and failed once.
  • Likely cause: fake_hatch writes a shell script, and the probe then execs it right away. If a parallel test thread forks while the script's write fd is still open, the exec fails with ETXTBSY ("Text file busy"). require_environment_context_support_with turns a spawn error into None via .ok(), which produces exactly this refusal. The repo already handles this race in other places (utils/process.rs output_of_fresh_shim, update/download.rs), but not in the Hatch tests.
  • No fix exists yet (CI: pin floating toolchain versions (#1645) #1647 only pins CI tools). Proposed patch, test-only, in crates/socket-patch-core/src/vendor/pypi_hatch.rs:
     fn fake_hatch(dir: &Path, marker: &Path) {
         use std::os::unix::fs::PermissionsExt;
         let script = dir.join("hatch");
         std::fs::write(
             &script,
             format!(
                 "#!/bin/sh\n: > '{}'\necho 'Hatch, version 1.13.0'\n",
                 marker.display()
             ),
         )
         .unwrap();
         std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).unwrap();
+        // Wait out the fork/exec ETXTBSY window (a sibling test thread can
+        // still hold our write fd), so the probe under test never sees it.
+        let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
+        loop {
+            match std::process::Command::new(&script).output() {
+                Ok(_) => break,
+                Err(e)
+                    if e.kind() == std::io::ErrorKind::ExecutableFileBusy
+                        && std::time::Instant::now() < deadline =>
+                {
+                    std::thread::sleep(std::time::Duration::from_millis(20));
+                }
+                Err(e) => panic!("spawn the fake hatch: {e:?}"),
+            }
+        }
+        let _ = std::fs::remove_file(marker);
     }

Removing the marker after the priming run keeps the "the resolved hatch was not run" assertion meaningful.

Re-queueing this PR is the one re-run. If coverage fails the same way again, the race is real and needs the patch above.


Generated by Claude Code

@mikolalysenko Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 21, 2026
#1599 (bare requirements wheel paths contest VEX) landed on main and
added its own test next to this PR's #1602 tests in pypi_other.rs.
Both sides only add tests; keep all of them.

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: #1599 landed on main and conflicted with this PR in crates/socket-patch-core/src/vex/discover/pypi_other.rs. Both sides only added tests, so I kept all of them and merged main (b7fe9a2) in as 3417797. vex::discover tests pass (417/417). I removed Ready for review; it goes back on once CI is green on the new head.


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.

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

Merged via the queue into main with commit 3b6a85c Oct 21, 2026
100 of 102 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-requirements-pin-contest-pep440 branch October 21, 2026 19:58
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 21, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: re-labeled Ready for review at 3417797 (the merge of main described above).

  • CI: 34/34 green, 19 skipped.
  • Bugbot: its check passed on this head and no review threads are open.
  • No code changes since the last review besides the conflict resolution (tests only). The pypi_hatch ETXTBSY flake patch proposed above is still unmerged, so a re-queue may trip it again.

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