Skip to content

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

Open
Mikola Lysenko (mikolalysenko) wants to merge 6 commits into
mainfrom
agent/fix-pypi-restore-hash-mode
Open

Mikola Lysenko (mikolalysenko) wants to merge 6 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.

Note

Low Risk
Narrows when PyPI requirements upstream restore refuses vs succeeds; behavior is covered by new unit, golden, and CLI tests with no auth or network policy changes beyond existing restore paths.

Overview
Fixes #410: hosted rollback, remove, and vendor can unwind a requirements.txt whose only requirements are hosted pins (e.g. lone six==1.16.0 or -e . plus a pin). Previously upstream restore always refused with “hash-checking mode … not derivable.”

Upstream restore (restore_requirements in pypi.rs) now infers pip hash-checking mode when no other requirement line settles it: use whether the hosted line used --hash vs a URL #sha256= fragment (as the hosted rewriter recorded). Editable lines (-e / --editable) count as evidence the file is unhashed, since pip disallows editables in hash-checking mode. Truly mixed hashed/unhashed files are still refused.

CLI_CONTRACT.md documents this behavior. Tests add core unit coverage, golden round-trips for sole-pin files, and CLI migration tests for rollback/remove/vendor. vex_consumed npm hosted tests are adjusted so alias expansion is still exercised after #605’s resolver changes.

Reviewed by Cursor Bugbot for commit 9eb4381. 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.

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

This branch has not been deployed

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

Labels

None yet

Projects

None yet

2 participants