Skip to content

Fix PyPI vendored→hosted takeover stranding unreachable pins (#699) - #708

Open
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-pypi-takeover-reachability
Open

Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-pypi-takeover-reachability

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #699

Root cause

scan --mode hosted over a vendored requirements.txt project (vendored_takeover in scan/hosted.rs) only checks whether the vendored wiring can be reverted. It never checks whether the hosted requirements rewriter can reach the restored entry. That rewriter only edits a pin in the root requirements.txt. A vendored pin in a -r include, or a vendor-appended (transitive) line, gets reverted first. The rewriter then refuses with redirect_requirements_entry_not_found, which leaves the package unpatched (exit 1). The dry run counts every takeover preview as redirected, so it never predicts this.

Change

  • New core gate patch::redirect::preflight_requirements_takeover(&VendorEntry). A requirements-flavored PyPI entry is reachable only when every wiring record is a rewritten pin in the root requirements.txt. Otherwise it returns redirect_requirements_takeover_unreachable. The detail names where the pin lives and gives the remedy with its full reach:
    • vendor --revert reverts every vendored package in the project, not just this one;
    • then move the pin into the root file and delete it from the include, or for a (transitive) line add an exact == pin to the root.
  • vendored_takeover's takeover_refusal runs this gate for PyPI purls. The existing refusal path then skips the purl before any revert, on wet and --dry-run alike, so the vendored wiring, ledger entry and wheel stay byte-identical (exit 0, the package stays vendored and patched).
  • A readable skip reason, plus CLI_CONTRACT.md (a takeover paragraph and a warning-table row).
  • No wrapper changes are needed (npm/, pypi/, gem/ only dispatch to the binary).

Per-issue checklist

Test evidence

  • Red on main (commit 9525830, tests only): cargo test -p socket-patch-cli --all-features --test mode_migration_pypi gives 13 passed, 4 failed. All four new refusal tests fail the "no takeover over an entry hosted mode cannot pin" assertion: the dry run says status: success with redirected: 1, and the wet run says partial_failure with the takeover announced, matching the issue.
  • Green with the fix (c06e3f2, then 5f291a5): mode_migration_pypi 17/17. covgap_commands_scan_hosted 50/50, coverage_fix_scan_hosted_dryrun_vendored 5/5, mode_migration_bun and mode_migration_vlt green, socket-patch-cli --lib 834/834, takeover_reach_tests 5/5.
  • cargo clippy --workspace --all-features -- -D warnings: clean (re-run on 5f291a5).
  • socket-patch-core --lib: 4847 passed, 4 failed. in_process_redirect: 104 passed, 3 failed. All 7 failures are chmod-0o555 write-failure injection tests that can't fail as root (the sandbox runs as uid 0). They're unrelated to this diff, and CI runs them unprivileged.
  • cargo fmt --all -- --check is not clean on main itself with the pinned 1.93.1 rustfmt, and CI doesn't run it. The changed files are rustfmt-clean, and no unrelated files were reformatted.
  • Full CI on 5f291a5: green (no failed check suites).

Review

  • Bugbot (c06e3f2): remediation text omitted vendor --revert's full reach and the step to delete the include pin. Valid, fixed in 5f291a5, and the thread is resolved.
  • Bugbot (5f291a5): no new issues.

Prioritization note

I picked this over older p1 issues because it's a correctness regression (#503) where a documented, previewed migration actively deletes a working vendored patch.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
Hosted mode pins only the root requirements.txt, so taking over a
vendored pin that lives in a -r include, or a (transitive) line that
vendored mode appended, used to revert the vendored patch and then
leave the package unpatched. These tests show the wet run and the
dry run for both layouts, plus the root-pin control.

Assisted-by: Claude Code:claude-opus-5-5
`scan --mode hosted` over a vendored requirements.txt project now
refuses to take over a package whose vendored pin is in a -r include
or is a (transitive) line. Before, it removed the vendored patch
first and then found no root pin to redirect, so the project went
back to installing the unpatched release (exit 1). The dry run
previewed a clean takeover.

The refusal is checked before anything is reverted, on wet and dry
runs alike. The package stays vendored and patched, and the run
reports redirect_requirements_takeover_unreachable with the steps to
switch.

Fixes #699

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 3, 2026 16:58
@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/requirements.rs
The refusal told users to run `vendor --revert` to switch one package.
That command reverts every vendored package in the project, and for a
pin in a -r include the user also has to delete the include's pin, or
the unpatched pin is still installed alongside the hosted one. The
detail and the contract now say both.

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.

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

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

Copy link
Copy Markdown
Collaborator Author

Ready for review (burn-down agent).

  • Head: 5f291a521 (0 commits behind main, mergeable)
  • CI: 491/491 check runs completed with no failures or pending runs
  • Bugbot: the c06e3f2 finding (remediation text understated vendor --revert reach) was fixed in 5f291a5 and its thread is resolved; Bugbot re-reviewed 5f291a5 with no new issues.
  • Reviewers should look at: the reachability gate added to the PyPI vendored→hosted takeover and the new redirect_requirements_takeover_unreachable detail text in patch/redirect/requirements.rs.

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

2 participants