Skip to content

Fix PyPI hosted takeover un-vendoring before refusal (#723, #945) - #946

Open
Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
agent/fix-pypi-hosted-takeover-preflight
Open

Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
agent/fix-pypi-hosted-takeover-preflight

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #723
Fixes #945

Summary

On a uv or Poetry project that socket-patch vendored, scan --mode hosted could delete the vendored patch and then fail to pin the hosted one. The package ended up in neither mode, and the next install got the unpatched release. --dry-run promised a clean takeover in the same situation. With this fix the takeover is refused up front, in the dry run and the wet run alike, and the package stays vendored and patched (exit 0).

Root cause

The vendored → hosted takeover reverts a purl's vendored wiring first, then asks the hosted rewriter to pin it. The takeover_refusal gate in crates/socket-patch-cli/src/commands/scan/hosted.rs is supposed to refuse, before the revert, any purl the hosted rewriter can't reach. For pkg:pypi/ it only ran preflight_requirements_takeover, which covers requirements.txt. Nothing checked the uv or Poetry rewriter:

Fix

  • New core preflight, patch::redirect::preflight_pypi_takeover(root, entry) in pypi_takeover.rs. It dispatches on the ledger entry's flavor:
    • requirements: the existing preflight_requirements_takeover
    • uv: refuses with the new code redirect_uv_takeover_version_unreachable when a recorded uv_lock_package original unit has a version different from the patch's. The comparison is exact, the same as the hosted planner's matching_package.
    • poetry: reads the wired poetry.lock and refuses with redirect_poetry_lock_unsupported when it's a 0.x lock. The revert never changes the lock format.
  • scan/hosted.rs computes these refusals once per PyPI takeover purl, before any revert. A refused purl is never dispatched, so the dry run reports the same refusal as the wet run, and the package keeps its wiring, ledger entry and wheel.
  • CLI_CONTRACT.md: documents the new code and the Poetry takeover refusal.

The npm/PyPI/gem wrappers only dispatch to the binary, so they need no change.

Also in this PR

Per-issue checklist

New mode_migration_pypi e2e tests (they run in plain cargo test). Each test runs a real vendor and then a hosted scan:

Issue Test Without fix (bb3a25d) With fix (8be3951)
#723 direct six>=1.15 uv_pinned_down_direct_takeover_is_refused_before_revert FAILED (redirected: 0, redirect_takeover_unpatched) ok
#723 direct, dry run dry_run_predicts_uv_pinned_down_direct_takeover_refusal FAILED (redirected: 1) ok
#723 transitive (dateutil → six) uv_pinned_down_transitive_takeover_is_refused_before_revert FAILED (redirect_takeover_unpatched) ok
#723 transitive, dry run dry_run_predicts_uv_pinned_down_transitive_takeover_refusal FAILED (redirected: 1) ok
#945 Poetry 0.12 poetry_0_lock_takeover_is_refused_before_revert FAILED (redirect_takeover_unpatched) ok
#945 dry run dry_run_predicts_poetry_0_lock_takeover_refusal FAILED (redirected: 1) ok

Core unit tests in pypi_takeover::tests (6 passed): a pinned-down uv entry is refused, a uv entry at the patch version is admitted, an unparseable original is admitted (the revert's own drift handling owns that case), a Poetry 0 lock is refused, a Poetry 2.1 lock is admitted, and other flavors are admitted.

Local verification

  • cargo test -p socket-patch-cli --all-features --test mode_migration_pypi: 33 passed. That includes the controls uv_vendored_to_hosted and poetry_vendored_to_hosted (where the lock already resolves the patch version) and the existing requirements refusals.
  • cargo test -p socket-patch-cli --all-features --test coverage_fix_scan_hosted_dryrun_vendored --test covgap_commands_scan_hosted: 9 + 52 passed.
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo test -p socket-patch-core --all-features --lib: everything passes except 5 tests that fail the same way on main in this sandbox:
    • utils::digest::tests::production_digests_go_through_the_helpers: the Gradle inline-hash guard that Route Gradle digests through utils::digest #878 fixes.
    • Four chmod-based write-failure tests (copy_tree relax loop, vlt_heal unremovable lock, poetry and requirements wire-failure rollback): they can't fail a write when run as root (uid 0 here).
  • cargo test --workspace --all-features couldn't finish locally because the sandbox ran out of disk (about 21 GB of test binaries). CI ran the full set on 0874e9d: all 547 check runs passed or were skipped, 0 failures.
  • cargo fmt: the new and changed hunks are rustfmt-clean. cargo fmt --all -- --check fails on main itself (466 pre-existing diffs; CI doesn't run it), so I didn't reformat unrelated files.

🤖 Generated with Claude Code


Note

Medium Risk
Changes hosted-scan vendored→hosted migration gates for PyPI (uv/Poetry), which can alter whether takeovers proceed but is designed to prevent leaving packages unpatched; includes contract and e2e coverage.

Overview
Vendored → hosted PyPI takeovers no longer revert wiring first when hosted mode cannot pin afterward. A new preflight_pypi_takeover runs once per pkg:pypi/ candidate (requirements, uv, Poetry) from the ledger and on-disk locks; refused packages stay vendored, exit 0, and --dry-run matches the wet run.

uv (#723): emits redirect_uv_takeover_version_unreachable when vendored mode pinned uv.lock to the patch version but the recorded pre-vendor unit resolved another version—revert would leave hosted with nothing to pin.

Poetry (#945): refuses takeover on Poetry 0.x locks with redirect_poetry_lock_unsupported before revert (hosted already refuses those locks post-revert).

Docs/tests: CLI_CONTRACT.md and mode_migration_pypi e2e cover the new refusal paths; requirements takeover tests share a generalized assert_takeover_refused helper.

Minor: Gradle/JVM/Maven code paths use shared utils::digest sha1_hex_of / sha256_hex_of instead of inline hashing.

Reviewed by Cursor Bugbot for commit 0874e9d. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
Add regression tests for the vendored -> hosted takeover of a uv
project whose lock resolved another version than the patch (#723,
direct and transitive) and of a Poetry 0.12 lock (#945). Each case
runs wet and --dry-run and expects the takeover to be refused before
the revert, keeping the vendored patch. On main they fail: the wet
run leaves the package unpatched and the dry run reports a clean
takeover.

Assisted-by: Claude Code:claude-opus-5-5
`scan --mode hosted` over a vendored uv or Poetry project reverted the
vendored wiring first and only then asked the hosted rewriter to pin
the package. When the rewriter could not, the package ended up in
neither mode and the next install got the unpatched release, while
--dry-run promised a clean takeover.

The PyPI takeover gate now checks the hosted rewriter's reach before
the revert, wet and dry run alike, and keeps the package vendored:

- uv: vendored mode pins the lock entry down to the patch's version.
  If the recorded pre-vendor entry is at another version, the revert
  brings it back and hosted mode can't pin it. Refused with
  redirect_uv_takeover_version_unreachable (#723).
- Poetry: hosted mode refuses every Poetry 0.x lock. Refused with
  redirect_poetry_lock_unsupported (#945).

The requirements.txt check moves into the same core preflight.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 6, 2026 15:47
@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/patch/redirect/pypi_takeover.rs
Comment thread crates/socket-patch-core/src/patch/redirect/pypi_takeover.rs
Port of #878. The digest guard test in socket-patch-core fails on
main because the Gradle cache crawler, jar comparator and Maven
sidecar hash inline. This keeps CI green on this branch and becomes
a no-op once #878 lands.

Assisted-by: Claude Code:claude-opus-5-5
The uv and Poetry takeover refusals told the user to re-lock while
the package was still vendored. That can't clear the uv refusal
(the recorded pre-vendor entry never changes) and makes the later
revert see drift. Both remedies now start with
`socket-patch vendor --revert`, say that it reverts every vendored
package, and only then re-lock and re-run hosted mode, matching the
requirements.txt refusal.

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] coverage failed on 8be3951 in socket-patch-core --lib. The same check, plus test (macos-latest) and test (windows-latest), is red on main (9c43dfc). The cause is utils::digest::tests::production_digests_go_through_the_helpers: the Gradle cache crawler, jar comparator and Maven sidecar hash inline, and open PR #878 fixes that. I ported #878's three-file change here (d11b01c) so this PR can go green. It becomes a no-op once #878 merges.


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 0874e9d. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Ready for review at head 0874e9d.

  • CI: 547/547 check runs green or skipped on 0874e9d; no merge conflicts, 0 commits behind main.
  • Bugbot: reviewed 0874e9d, no new issues; no unresolved review threads.
  • Reviewer focus: the new preflight_pypi_takeover (pypi_takeover.rs) runs before any revert in scan/hosted.rs; check that the uv exact-version comparison matches the hosted planner's matching_package, and the new redirect_uv_takeover_version_unreachable code in CLI_CONTRACT.md.
  • Slack announcement not sent this run (no Slack send tool available); the next run will retry.

Generated by Claude Code

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

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

3 participants