Skip to content

Fix pip rollback refusing all-hosted requirements (#410) - #827

Merged
Mikola Lysenko (mikolalysenko) merged 9 commits into
mainfrom
agent/fix-pypi-restore-hash-mode
Oct 7, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 9 commits into
mainfrom
agent/fix-pypi-restore-hash-mode

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #410

Summary

Hosted rollback, remove <purl> and the hosted → vendored takeover (vendor / scan --mode vendored over a hosted pin) no longer refuse a requirements.txt in which every requirement is a hosted pin. Before this fix, the simplest project shape (six==1.16.0, or -e . plus a pin) could be switched to hosted but never switched back. All three commands exited 1 with … is not derivable; restore it from version control instead.

Root cause

restore_requirements (crates/socket-patch-core/src/patch/redirect/upstream/pypi.rs) works out pip's hash-checking mode for the restored line from the other requirement lines in the file:

  • With no other requirement line it hit the (false, false) arm and refused.
  • It skipped every --prefixed line before counting, so an -e . line, which pip refuses in hash-checking mode, never counted as evidence that the file is unhashed.

Fix

Test evidence

Issue variant Test Without fix With fix
six==1.16.0 only (LF + CRLF, plus comment/option/blank lines) upstream::pypi::tests::requirements_with_only_a_hosted_pin_restores_unhashed FAILED (refused, "not derivable") ok
six==1.16.0 --hash=… only line upstream::pypi::tests::requirements_with_only_a_hashed_hosted_pin_restores_hashed FAILED ok (every release file's hash)
-e . + pin (5 editable spellings × fragment / --hash hosted line) upstream::pypi::tests::an_editable_line_settles_requirements_as_unhashed FAILED ok
Controls: other lines still decide; mixed file still refused upstream::pypi::tests::other_requirement_lines_still_settle_the_mode ok except the new --require-hashes + -e refusal ok
Golden: lone unhashed and lone hashed pin round-trip; mixed file still refused upstream_restore_golden::requirements_hash_mode_ambiguity_is_refused (updated: it asserted the #410 refusal) FAILED on the fix's first push (asserted refusal) ok
CLI: hosted scan then rollback / remove / vendor takeover, six==1.16.0 mode_migration_pypi::requirements_sole_hosted_pin_unwinds FAILED ("not derivable") ok
CLI: same, -e . + six==1.16.0 mode_migration_pypi::requirements_editable_beside_hosted_pin_unwinds FAILED ok

Commands run locally:

  • cargo test -p socket-patch-core --all-features --lib --test upstream_restore_golden: 4846 passed. 4 tests failed, all unrelated to this change: copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_… and pypi_requirements::wire_failure_rolls_back_…. They inject write failures with chmod 0555, which root ignores, and the sandbox runs as uid 0.
  • cargo test -p socket-patch-core --all-features --test upstream_restore_golden: 42 passed.
  • cargo test -p socket-patch-cli --all-features --test in_process_rollback_hosted --test mode_migration_pypi --test in_process_get_hosted_ecosystems: 22 + 14 + 8 passed.
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo fmt: the changed hunks are formatted. cargo fmt --all -- --check already fails on main (about 130 files; CI runs no fmt job), so those files are left alone here.
  • A full cargo test --workspace ran out of the sandbox's disk allowance (30 GB of test binaries), so the full matrix is left to CI.

CI

  • f4033e3: 482 check runs succeeded and 6 were skipped. Bugbot found no issues and there are no review threads.
  • PDM patch compatibility / native (ubuntu-latest, 2.1.5) and (…, 2.10.4) failed once on f4033e3 in agent-mode cells (marker agent FAIL appliedExactlyOne). That path doesn't touch the requirements restore, and the same workflow passed on 191ff15, whose code is identical except for one test file. One rerun of the failed jobs passed.
  • 9eb4381 (main merged + Fix vex alias tests broken by store-copy merge #851's tests-only fix ported): all 488 check runs succeeded or were skipped. PDM patch compatibility / native (ubuntu-latest, 2.17.3) and (…, 2.22.4) failed once in a pdm.lock hosted cell this PR doesn't touch; one rerun passed. Bugbot found no issues on this head, and the PR is approved.

Note

Medium Risk
Changes hosted unwind behavior for PyPI requirements files (user-visible restore output and hash mode), though scope is narrow and heavily tested; digest helper swap is low-risk plumbing.

Overview
Fixes #410: hosted unwind (rollback, remove, vendored takeover) no longer refuses a requirements.txt where every line is a hosted pin.

PyPI upstream restore now derives pip hash-checking mode when sibling lines cannot: it reads the hosted line itself (--hash vs URL #sha256= fragment, per the post-#376 rewriter), and treats editable requirements (-e / --editable) as evidence the file is unhashed. Genuinely mixed hashed/unhashed files are still refused. CLI_CONTRACT.md documents this behavior.

Tests cover sole-pin and -e + pin unwind via core unit tests, golden round-trips, and CLI migration tests.

Minor refactor: Gradle cache, JVM jar patching, and Maven sidecars use shared utils::digest::{sha1_hex_of, sha256_hex_of} instead of inline sha1/sha2 calls.

Reviewed by Cursor Bugbot for commit 09364ea. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
Hosted rollback, remove and the hosted-to-vendored takeover refused a
requirements.txt in which every requirement was a hosted pin (a lone
`six==1.16.0`, or one beside `-e .`). They couldn't tell whether the
original line used pip's hash-checking mode, so the only way back was
version control.

The restore now counts an editable line as unhashed evidence (pip
refuses editables in hash-checking mode). When no other line settles
the mode, it reads the hosted line itself: the rewriter writes
`--hash` only into an already hashed file and otherwise pins by the
url's `#sha256=` fragment. With nothing else in the file to conflict
with, either restored form installs.

Fixes #410

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

The golden test asserted that a requirements.txt holding only the
hosted pin is refused as ambiguous, which is the #410 bug. It now
asserts that both the unhashed and hashed sole-pin files round-trip,
and keeps the mixed hashed/unhashed refusal.

Refs #410

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

Copy link
Copy Markdown
Collaborator Author

[agent] CI status on f4033e3:

  • test (ubuntu-latest), test (macos-latest), test-release and coverage failed on 191ff15. All four failed in upstream_restore_golden::requirements_hash_mode_ambiguity_is_refused, which asserted the exact refusal Hosted rollback, remove and the vendored takeover refuse a requirements.txt whose only requirements are hosted pins (six==1.16.0 alone can be patched but never unpatched) #410 reports as the bug. f4033e3 updates it to expect the lone pin to round-trip, hashed and unhashed, and keeps its mixed-file refusal. All four pass now.
  • PDM patch compatibility / native (ubuntu-latest, 2.1.5) and (…, 2.10.4) failed once on f4033e3, in agent-mode cells (marker agent FAIL appliedExactlyOne). This isn't this PR's failure: agent mode never runs the requirements.txt upstream restore, and the same workflow passed on 191ff15, whose code differs only in that one test file. I re-ran the failed jobs once and they passed.

The head is now green (482 succeeded, 6 skipped), Bugbot found no issues, and there are no open threads. It's waiting on human review.


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

Copy link
Copy Markdown
Collaborator Author

Ready for review — head f4033e3bbc.

  • CI: 96/96 green (4 skipped) on the current head; up to date with main, no conflicts.
  • Bugbot: reviewed f4033e3 with no new issues; no unresolved review threads.
  • Reviewer focus: the hash-mode inference in restore_requirements (pypi.rs) — editable lines now count as unhashed evidence, and an all-hosted file follows the hosted line's own --hash / #sha256= form. Mixed hashed/unhashed files still refuse.

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 5, 2026
Resolve the CLI_CONTRACT.md pypi restore bullet: keep main's uv
upload_time spelling note and this branch's hash-checking-mode (#410)
description.

Co-Authored-By: Claude <noreply@anthropic.com>
@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.

Assisted-by: Claude Code:claude-opus-5-5
#605 taught the name-keyed npm resolver to probe bundled store
trees, so it now finds aliased copies (node_modules/lp) and a nested
host's store peers itself. Two vex_consumed tests from #738 assumed
that set never held aliases, so main's CI went red after both merged.

The tests now feed the alias-free set explicitly to keep covering
alias expansion, and also check the resolver's own set reaches the
same copies with no duplicates. No production code changes.

Assisted-by: Claude Code:claude-opus-5-5
(cherry picked from commit 40dac07)
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] CI on a7b427d (the merge of main): coverage, test (macos-latest) and test-release failed in socket-patch-cli --lib, in commands::vex_consumed::tests::hosted_expands_alias_only_copies and …::hosted_reuses_expanded_npm_copies_and_merges_alias_variants.

This isn't this PR's failure: both tests fail the same way on bare main 4646693. It's the semantic conflict between #738 and #605 that #851 describes. I've ported #851's tests-only fix: 9eb4381 cherry-picks 40dac07 after merging current main, and the change will no-op once #851 lands.

Local results on 9eb4381: cargo test -p socket-patch-cli --all-features --lib passes 840/840, upstream_restore_golden -- requirements passes 3/3, and cargo clippy --workspace --all-features -- -D warnings is clean.


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.

Stale Bugbot comment from a previous run.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] PDM patch compatibility / native (ubuntu-latest, 2.17.3) and (…, 2.22.4) failed on 9eb4381: 2.17.3 crlf hosted FAIL appliedExactlyOne, a pdm.lock hosted cell.

This isn't this PR's failure. The PR changes only the requirements.txt upstream restore, which never handles pdm.lock. The same workflow passed on a7b427d. Today the same 2.17.3 cell also failed on gem-only #849, and the workflow failed on #731, #750 and #806's branch, so it's an intermittent failure in the PDM harness's appliedExactlyOne check that isn't tied to any diff. I don't know of a fix for the harness yet. I'm re-running the failed jobs once when the run finishes.


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

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review.

  • Head: 9eb43818401fa1bad27d1908516a49ddf2cf4cb2
  • CI: 488/488 check runs green (success/skipped) on this head; mergeable clean
  • Bugbot: reviewed 9eb4381, no new issues; all threads resolved
  • Reviewer focus: restore_requirements path for all-hosted requirements.txt in crates/socket-patch-core/src/patch/redirect; already approved by Tanmay182003

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 5, 2026
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.

Assisted-by: Claude Code:claude-opus-5-5
main has failed socket-patch-core's lib tests since Gradle support
(#646) and the digest helpers (#865) both landed. The guard test
production_digests_go_through_the_helpers flags three files #646 added
that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and
patch/sidecars/maven.rs. That breaks test, test-release and coverage on
every open PR.

Each inline sha1/sha256 call now goes through sha1_hex_of or
sha256_hex_of, which compute the same lowercase hex. Behaviour is
unchanged.

Assisted-by: Claude Code:claude-opus-5-5
(cherry picked from commit 659ac2c)
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] coverage failed on db053af in socket-patch-core --lib, in utils::digest::tests::production_digests_go_through_the_helpers.

This isn't this PR's failure. The test fails the same way on bare main c644ab0. It's a semantic conflict between Gradle support (#646) and the digest helpers (#865): the guard test flags crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs, which #646 added, for hashing inline. No fix existed, so I opened #878, which routes those calls through utils::digest with no behaviour change. 09364ea ports it here after merging current main, and it will no-op once #878 lands.

Local results on 09364ea: utils::digest + upstream::pypi pass 11/11, upstream_restore_golden 48/48, mode_migration_pypi 29/29, and cargo clippy --workspace --all-features -- -D warnings is clean.


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 09364ea. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at 09364ea (09364eaac5fa46daa33d190ee248289cca7aa990).

  • CI: 538/538 green on the head commit (6 skipped by matrix rule).
  • Bugbot: reviewed 09364ea with no findings; no open review threads.
  • Mergeable against main (clean).
  • Reviewer note: the earlier approval was on 9eb4381; since then main was merged in and Route Gradle digests through utils::digest #878's digest-guard fix was ported, so this needs a fresh approval.

Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit 7ce869e into main Oct 7, 2026
545 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-pypi-restore-hash-mode branch October 7, 2026 12:01
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