GeezerVison is showing github.com — Test cargo takeover over unrewritable deps (#1020) by mikolalysenko · Pull Request #1314 · SocketDev/socket-patch
Conversation
LLM Description written by Claude Code:claude-opus-5-5
Refs #1020 (closed as already fixed on main by #1039; this PR adds the regression test).
Summary
#1020 reported that scan --mode hosted over a vendored cargo crate reverted the vendored wiring before the hosted Cargo.toml rewriter refused the dependency spelling (dotted keys, cfg-if = '1.0', registry = "crates-io"), leaving the crate in neither mode, while --dry-run previewed redirected: 1.
#1039 (823810a, "Make the vendored-to-hosted takeover atomic") staged the takeover in the run's GroupCommit overlay: a staged purl the hosted rewrite does not pin is retracted (Takeover::retract), so the purl stays vendored byte for byte and the dry run runs the same steps. That fixes #1020 on main. Nothing pinned that cargo behavior, so this PR adds a real-cargo e2e.
Test
crates/socket-patch-cli/tests/mode_migration_cargo.rs::takeover_over_unrewritable_spelling_keeps_vendored. It runs once for each of the three spellings:
vendor --offline, then cargo build --locked --offline links the patched copy (a compile oracle that calls cfg_if::socket_patched())- hosted
--dry-run gives redirected: 0, redirect_takeover_kept_vendored and redirect_cargo_toml_dep_unrewritable, with no redirect_would_revert_vendored and no writes
- the wet run gives the same exit and status as the dry run,
redirected: 0 and no redirect_takeover_unpatched. Every project byte is unchanged.
- a fresh checkout still builds the patched crate with
--locked
Commands run
cargo test -p socket-patch-cli --test mode_migration_cargo takeover_over_unrewritable: passes on current main (cargo 1.93.1)rustfmt on the changed file
🤖 Generated with Claude Code
Note Low Risk
Test-only change; no runtime or CLI behavior is modified.
Overview
Adds a regression e2e test for issue #1020 in mode_migration_cargo.rs: takeover_over_unrewritable_spelling_keeps_vendored.
The test runs vendored → hosted scan when Cargo.toml uses dependency spellings the hosted rewriter cannot rewrite (dotted keys, single-quoted version, explicit registry = "crates-io"). It asserts redirected: 0, redirect_takeover_kept_vendored and redirect_cargo_toml_dep_unrewritable, and that --dry-run matches the wet run (no redirect_would_revert_vendored, no filesystem writes). After a refused takeover the project stays byte-identical and a fresh cargo build --locked still links the vendored patched crate.
No production code changes—only pins behavior already fixed on main (#1039).
Reviewed by Cursor Bugbot for commit e18a8af. Configure here.
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
A vendored cargo crate whose Cargo.toml spelling the hosted rewriter
refuses (dotted keys, a single-quoted literal, registry = "crates-io")
must stay vendored when scan --mode hosted runs over it, and the dry
run must preview that refusal instead of a takeover. The atomic
takeover from #1039 already does this; pin it with a real-cargo e2e so
the crate can never again land in neither mode (#1020).
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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 e18a8af. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Ready for review (burn-down agent).
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[final reviewer] I disarmed auto-merge at e18a8af2 because ci-ok is red on this head. Only hosted-e2e and e2e (ubuntu-latest, e2e_safety_pnpm) fail, which is the main-wide [email protected] failure (#1293), not this PR. #1302 fixes it and is in the merge queue now. Once main carries it, merge main in here (no other commits needed) and I'll re-arm auto-merge after CI goes green. The approval still covers this head.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|