Fix PyPI vendored to hosted takeover being refused (#328) - #503
Mikola Lysenko (mikolalysenko) merged 8 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
Switching a vendored Python project to hosted mode leaves it vendored: every PyPI hosted rewriter refuses the vendored source socket-patch wrote itself. Cover requirements.txt, Poetry, Pipenv, uv and Hatch (#328). Assisted-by: Claude Code:claude-opus-5-5
`scan --mode hosted` over a project socket-patch had vendored left it vendored and reported success: the takeover that reverts vendored wiring before redirecting only covered cargo, npm and Go, so the Python rewriters (requirements.txt, Poetry, Pipenv, uv, Hatch) refused socket-patch's own vendored source. PyPI packages now go through the same takeover. Two guards keep the takeover from leaving a package unpatched: - vendored wiring edited since vendoring is left in place by the revert, so the takeover now refuses and keeps the ledger entry instead of dropping it; - a package whose wiring was reverted but that the hosted rewrite then did not pin (e.g. hosted wheel metadata unavailable) now fails the run with redirect_takeover_unpatched instead of passing as success. Fixes #328 Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Follow-ups from review of the PyPI vendored-to-hosted takeover: - `--dry-run` now predicts the drifted-wiring refusal from the same drift and residual-reference signals the wet run uses, instead of previewing a takeover the wet run then refuses. - A takeover left unpatched no longer prints a "Migrated ... to hosted" line or "keep the hosted patches" next steps. - Its redirect_takeover_unpatched error also prints under --silent, so the exit 1 is explained. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
|
[agent] CI: I don't think this PR caused it:
I'll re-run the failed job once when the workflow finishes, which isn't possible while other jobs are still running (403 "already running"). If it fails a second time I'll treat it as real and dig into the job artifact. Generated by Claude Code |
|
[agent] Ready for review at
Generated by Claude Code |
|
Reviewed P1 — A ledger-save failure still reports success after removing the PyPI patch ( Reproduced by vendoring Validation: 11 PyPI migration tests plus 15 npm and 5 Cargo migration tests passed. An additional review-only CLI regression test for this read-only-ledger case failed with the output above. Full workspace and real-package-manager matrices were not rerun. |
|
The vendored→hosted takeover for PyPI reverts the vendored patch before fetching the hosted wheel metadata (vendored_takeover at hosted.rs:802, fetch at :818+). If that fetch fails (network, a uv project whose metadata can't be fetched), the package is left patched in neither mode. The PR catches this and fails loudly (partial_failure, exit 1), and re-running recovers it. But CLI_CONTRACT promises the opposite for yarn berry ("refuses before reverting … never left unpatched in both modes"), and #470 adds exactly that check for berry. Can we file a follow-up to do the same for PyPI: fetch and validate the hosted metadata first, and only revert the vendored copy once that succeeds? |
When a vendored-to-hosted takeover reverts a package's wiring but the vendored ledger then cannot be updated, the package is refused and never redirected. Its vendored wiring and artifact are already gone, so it installs unpatched in both modes, yet the run exited 0 with status "success". It now counts as a stranded takeover: redirect_takeover_unpatched, status "partial_failure", exit 1, also printed under --silent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018kguGYyuuizF1dwKyH6oZh
|
[agent] Replying to both review comments. P1 (ledger-save failure reported as success): fixed in 1a1f2e9. The cause is as described. The
This applies to every takeover-capable ecosystem, not only PyPI. CLI_CONTRACT.md lists the ledger-update failure as one of the stranded causes. Regression test: Tanmay Singla (@Tanmay182003), pre-validating hosted metadata before the revert: agreed. Fetching and validating the hosted wheel METADATA before reverting the vendored copy would let a PyPI takeover refuse up front with nothing written, the same way the berry gates do. It also matches the contract's "never left unpatched in both modes" wording. That reorders the takeover against the metadata fetch for every Python flavor, which is bigger than this PR, so I'd keep it as a follow-up. This PR guarantees the fallback: it fails loudly, and a re-run recovers. I'll leave filing that follow-up issue to the maintainers. Generated by Claude Code |
covgap_commands_scan_hosted pinned exit 0 for a takeover whose revert succeeded but whose ledger save failed. That case is now a stranded takeover: redirect_takeover_unpatched, status "partial_failure", exit 1. The test skips as root, so it only ran in CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018kguGYyuuizF1dwKyH6oZh
|
Follow-up reviewed A takeover whose vendored wiring was removed but whose ledger update failed is now included among stranded packages. It reports Validation: 12 PyPI migration tests and the original independent ledger-write-failure reproduction pass with the fix. The 7 ledger-related hosted coverage tests also pass at this head. These are focused checks; the full workspace suite was not rerun locally. |
Resolve the CLI_CONTRACT.md conflict by keeping this branch's PyPI takeover paragraph and main's corrected BUNDLE_GEMFILE precedence wording (app config outranks the environment variable). Co-Authored-By: Claude <noreply@anthropic.com>
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 9d46d12. Configure here.
|
Second pass reviewed Validation at this exact head: all 12 CI follow-up: this same head now has 478 successful checks and 6 skipped, with no unresolved review threads and a clean merge against main |
|
[agent] CI on 9d46d12: The job hung in I don't think this PR caused it:
No fix exists yet. I've re-run the failed job once. If it hangs again, I'll treat it as real and dig in. Generated by Claude Code |
|
[burn-down agent] Re-verified Ready for review at
Slack announcement not sent (no Slack send tool available this run). Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #328
Root cause
When
scan --mode hostedruns over a project socket-patch has already vendored, it first reverts each purl's vendored wiring ("takeover") and then redirects it. The gate that decides which purls get that treatment (takeover_capableincrates/socket-patch-cli/src/commands/scan/hosted.rs) admitted onlypkg:cargo/,pkg:npm/andpkg:golang/. A vendored PyPI purl therefore went straight to the hosted rewriters. Those rewriters treat any non-registry source as user-authored, including the vendored one socket-patch wrote itself, and refused it:redirect_poetry_lock_unsupportedredirect_pipenv_refusedredirect_hatch_unsupportedredirect_uv_project_unsupportedredirect_requirements_entry_not_foundThe scan still exited 0 with
redirected: 0, leaving the project vendored.Fix
pkg:pypi/to the takeover gate. The per-purlvendor --revertmachinery (revert_pypi_opts, which covers every PyPI flavor) restores the recorded registry entry and removes the ledger entry and wheel. Then the normal hosted rewrite pins it.redirect_vendored_revert_failedand keep the ledger entry and artifact. "In place" means a drift-skipped record, or a reverted file that still references the artifact; the sharedrevert_keeps_wiringpredicate checks both. Previously the takeover dropped the ledger entry anyway, which breaks theRevertOutcomecontract.--dry-runpredicts the same refusal from the same signals. This applies to every takeover-capable ecosystem.redirect_takeover_unpatchedwithstatus: "partial_failure"and exit 1, and it is printed under--silenttoo. Human output gives no "Migrated …" line and no "keep the hosted patches" next steps for it.No wrapper changes are needed: the
npm/,pypi/andgem/wrappers only dispatch to the binary.Follow-up (not in this PR, suggested in review): fetch and validate the hosted wheel metadata before reverting, so a PyPI takeover refuses up front with nothing written, as the yarn berry gates do.
Tests (new suite
crates/socket-patch-cli/tests/mode_migration_pypi.rs, hermetic, wiremock)requirements_vendored_to_hostedredirect_requirements_entry_not_found, redirected 0requirements_sole_pin_vendored_to_hosted(six==1.16.0alone, from the 2026-10-01 comment)poetry_vendored_to_hostedredirect_poetry_lock_unsupportedpipenv_vendored_to_hostedredirect_pipenv_refuseduv_vendored_to_hostedredirect_uv_project_unsupportedhatch_vendored_to_hostedredirect_hatch_unsupporteduv_takeover_without_wheel_metadata_fails_loudlyredirect_takeover_unpatcheddrifted_vendored_line_refuses_takeoverdry_run_predicts_drifted_takeover_refusal(Bugbot)stranded_takeover_human_output_is_not_a_migration(Bugbot)stranded_takeover_is_reported_under_silent(Bugbot)ledger_update_failure_after_revert_is_stranded(review P1; Unix, skips where permissions aren't enforced)successpartial_failureThe existing
covgap_commands_scan_hosted::ledger_save_failure_after_successful_revert_fails_closednow expects the stranded contract too: exit 1,partial_failureandredirect_takeover_unpatched.Each lane test vendors the project, runs
scan --mode hosted, and asserts all of:redirected: 1,redirect_takeover_reverted_vendored, no.socket/vendor/reference in any wiring file, the hosted URL wired, and the vendored artifact reclaimed.Local checks on aada684:
cargo clippy --workspace --all-features -- -D warnings: clean.mode_migration_pypi12/12,mode_migration_npm15/15,mode_migration_cargo5/5,covgap_commands_scan_hosted50/50,covgap_commands_vendor44/44,covgap_commands_rollback62/62,covgap_commands_get75/75,in_process_redirect104/104.cargo test -p socket-patch-cli --all-features --lib,in_process_vendor,in_process_vendor_bun_takeover,coverage_fix_scan_hosted_dryrun_vendored,scan: all pass.cargo test --workspacedidn't fit in this session's disk allowance, so CI runs the rest.cargo fmt --all -- --check: fails onmainalready (about 460 diffs in files this PR doesn't touch; CI doesn't run it). The hunks this PR adds are rustfmt-clean.CI on aada684: all 467 checks pass (461 success, 6 skipped).
Per-issue checklist
*_vendored_to_hostedtests plusrequirements_sole_pin_vendored_to_hosted. The hosted → vendored direction was already fixed on main (see the issue comments).🤖 Generated with Claude Code
https://claude.ai/code/session_018kguGYyuuizF1dwKyH6oZh
Note
Medium Risk
Changes hosted
scanexit semantics (exit 1 /partial_failurewhen revert leaves the project on unpatched registry releases) and cross-mode takeover logic; behavior is contract-tested but affects CI and migration workflows.Overview
Fixes vendored → hosted migration for PyPI (#328) by including
pkg:pypi/in the hosted scan’s vendored takeover gate, soscan --mode hostedreverts socket-patch’s own vendored wiring (via the same per-purlvendor --revertpath as cargo/npm/golang) before lock rewriters run—those rewriters previously treated vendored sources as user-authored and skipped redirect with exit 0.Tightens takeover safety for all takeover-capable ecosystems: if revert would leave vendored wiring in place (drift-skipped records or residual artifact references), the takeover is refused with
redirect_vendored_revert_failedand the ledger/artifact stay put;--dry-runuses the same signals instead of previewing a takeover.“Stranded” takeovers—revert succeeded but the package never got a hosted pin (failed redirect, missing wheel metadata, or ledger save after revert)—now emit
redirect_takeover_unpatched, set JSONstatus: "partial_failure", and exit 1 (including under--silent). Human output skips misleading “Migrated …” and commit/reinstall next steps for those purls.CLI_CONTRACT.mddocuments PyPI takeover, revert refusal, and stranded behavior. New hermetic tests inmode_migration_pypi.rscover requirements/Poetry/Pipenv/uv/Hatch happy paths plus drift, dry-run, stranded UX, and ledger-save failure; an existing covgap hosted test expects the new stranded contract.Reviewed by Cursor Bugbot for commit 9d46d12. Configure here.
Generated by Claude Code