Skip to content

fix(cargo): fully migrate hosted<->vendored modes (C1-C7) - #196

Open
Mikola Lysenko (mikolalysenko) wants to merge 10 commits into
mainfrom
fix/cargo-mode-takeover-cleanup
Open

fix(cargo): fully migrate hosted<->vendored modes (C1-C7)#196
Mikola Lysenko (mikolalysenko) wants to merge 10 commits into
mainfrom
fix/cargo-mode-takeover-cleanup

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

What

Makes a cargo mode migration (vendored ↔ hosted) a real takeover: the flow reverts the displaced mode's wiring per purl BEFORE applying the new mode, so the project ends FULLY in the new mode — or the purl is refused with an actionable error. Fixes audit findings C1–C7.

  • Hosted scan over a vendored crate (C1): run_redirect now runs the exact per-purl vendor --revert machinery first (restore the Cargo.lock originals from the vendored ledger, drop the [patch.crates-io] entry, remove the committed tree and the ledger entry), persisting the ledger per purl, and only then redirects. A purl whose vendored state cannot be cleanly reverted (revert failure, ledger unreadable/unwritable) is refused — skipped with redirect_vendored_revert_failed — never half-migrated. Dry runs preview the takeover with redirect_would_revert_vendored.
  • Vendored flows over a hosted redirect (C2, C7): both the standalone vendor command and scan --mode vendored now revert the purl's hosted edits from the redirect ledger's recorded FileEdits first — Cargo.toml loses its registry = "socket-patch-…" pin, Cargo.lock gets the original crates.io source/checksum back, the now-unused [registries.socket-patch-…] block is dropped — then drop the ledger record and vendor. New core module patch/redirect/takeover.rs (revert_cargo_redirect_purl) unwinds re-redirect chains newest-first and fails closed on drifted files (neither the recorded redirected fragment nor the original matches ⇒ Err ⇒ the purl is refused with redirect_revert_failed). The plain vendor command also emits the same vendor_supersedes_redirect envelope advisory the scan flow does (C7).
  • Missing/corrupt ledger backstop: vendor_cargo_crate itself refuses (hosted_redirect_live) when live hosted wiring is present with no redirect-ledger record to revert it — with manual remediation steps — instead of baking the hosted registry values into the vendor ledger as "originals".
  • Lock-originals preservation (C3): the pre-revert ordering means the vendor detach records the PRISTINE crates.io fragment; additionally persist_vendor_entry carries the cargo lock originals forward on re-vendor (entry.lock was previously dropped whenever the re-detach found nothing to detach), so an A→B→A round trip can no longer overwrite the ledger's only copy of the unrecoverable crates.io source/checksum, and vendor --revert restores the true pre-vendor lock.
  • Classifier cargo blindness (C4/C4b): classify_overlap_takeover gains a cargo-specific probe (classify_cargo_overlap) that reads the Cargo.lock entry's actual shape — source matching a config-declared [registries.socket-patch-*] sparse index (or patch.socket.dev) proves hosted; a detached entry with [patch.crates-io] pointing into the entry's committed .socket/vendor/cargo/<uuid>/ copy proves vendored; anything else stays silent. The previous generic probe could never prove hosted for cargo (lock_inventory records resolved: None), so it inverted the direction and told users to delete the live redirect ledger.
  • GC/prune reclamation (C5): dispatch_in_use_one gains the cargo probe (vendored_entry_in_use): a detached lock entry routed through the entry's [patch] copy is in use; a registry-sourced or missing entry is reclaimable, so a vendored entry displaced by a hosted takeover no longer survives every scan --prune forever.
  • Remediation text (C6): both takeover warnings now recommend only the safe sequence (vendor --revert / re-run the vendored flow) and explicitly warn against hand-deleting .socket/vendor/ (the [patch.crates-io] entry may still reference it — deleting it hard-fails every cargo command) or redirect-state.json (it may hold the only revert data and VEX records for other still-redirected packages).

Why

Before this change, every cargo mode migration exited 0 while leaving the project broken — the exact fail-open bug class the audit flagged:

  • vendored→hosted left both wirings in place; cargo refuses to update the lock over the now-unused [patch] entry, so every CI cargo build --locked fails (C1) — with warnings: [] because the classifier could not see hosted-liveness for cargo (C4);
  • hosted→vendored left the Cargo.toml registry pin that [patch.crates-io] cannot patch over — unbuildable online and offline (C2), and completely silent via the primary vendor entry point (C7);
  • A→B→A silently destroyed the crates.io lock originals, the one piece of state documented as "not recoverable offline; the ledger entry is their only home", so revert restored a dead grant-tokenized sparse-index URL (C3);
  • the inverted warning actively advised deleting the live hosted ledger — destroying the VEX record store and the only hosted revert path (C4b/C6);
  • and nothing (GC, prune, repair) could ever reclaim the stale vendored residue (C5).

Testing

New real-binary + real-cargo migration suite crates/socket-patch-cli/tests/mode_migration_cargo.rs (adapted from the audit probes in mode_migration_cargo_audit.rs), red on the pre-fix code and green here. Each scenario proves the terminal state with a fresh-checkout cargo build --locked:

  • vendored_then_hosted_takeover_leaves_pure_hosted — vendored state fully gone (no [patch], no tree, no ledger entry), redirect live, fresh checkout builds --locked (C1/C4);
  • hosted_then_vendored_takeover_leaves_pure_vendored — registry pin and [registries] block gone, redirect record dropped, fresh checkout builds --locked --offline (C2/C7);
  • double_takeover_a_b_a_preserves_lock_originals — ledger keeps the crates.io fragment through A→B→A, and vendor --revert restores the pristine lock byte-identically (C3);
  • vendor_over_hosted_without_ledger_is_refused — live hosted wiring with a deleted ledger: vendor refuses (hosted_redirect_live), project untouched (fail-closed).

Plus a GC unit test (vendor_gc_reclaims_cargo_entry_displaced_by_hosted_takeover, C5) and classifier/remediation coverage in the scan tests.

Gates run locally, all green:

  • cargo test -p socket-patch-cli --all-features --test mode_migration_cargo (4/4, no skips)
  • cargo test -p socket-patch-cli --all-features --test in_process_redirect --test rollback_invariants --test repair_invariants --test e2e_vendor_cargo_build
  • cargo clippy --workspace --all-features -- -D warnings
  • cargo test --workspace --all-features

Note

High Risk
Changes critical cargo wiring, lockfile originals, and ledger persistence across hosted/vendored migrations; incorrect revert or classifier logic could leave projects unbuildable or destroy unrecoverable lock state.

Overview
Cargo hosted ↔ vendored mode switches are now takeovers: the displaced wiring is reverted per package before applying the new mode, so the repo ends fully in one mode—or that package is refused with an actionable error instead of reporting success on a broken --locked build.

Vendored → hosted: scan --mode hosted runs the same per-purl revert as vendor --revert (lock originals, [patch.crates-io], committed tree, ledger) before redirecting; corrupt/missing vendored ledger with live wiring is skipped.

Hosted → vendored: vendor and vendored scan call new revert_cargo_redirect_purl to unwind hosted edits from the redirect ledger, then vendor; drift or missing ledger refuses the purl (hosted_redirect_live in the cargo backend).

Classifier & GC: Cargo overlap direction uses Cargo.lock shape (sparse index vs detached + patch path), not generic patch.socket.dev resolution; cargo vendored entries displaced by hosted wiring are reclaimable on prune/GC.

Originals chain: Re-vendor carries forward cargo lock originals in the ledger; remediation text warns against hand-deleting vendor trees or redirect-state.json.

New e2e mode_migration_cargo.rs proves fresh-checkout cargo build --locked after each scenario.

Reviewed by Cursor Bugbot for commit 8e784ca. Configure here.

A cargo mode takeover used to exit 0 while leaving the project
unbuildable in both modes (audit findings C1-C7):

- hosted scan over a vendored crate left the vendored [patch.crates-io]
  entry behind, so every `cargo build --locked` failed (C1);
- vendor over a hosted redirect left the Cargo.toml
  registry = "socket-patch-..." pin, which [patch.crates-io] cannot
  apply over - unbuildable online and offline (C2), silently via the
  plain `vendor` command (C7);
- a vendored->hosted->vendored round trip overwrote the unrecoverable
  crates.io lock originals in the vendored ledger with the hosted
  sparse-index URL + patched checksum, so revert restored a dead
  grant-tokenized URL (C3);
- the takeover classifier could never prove hosted for cargo
  (lock-inventory resolved is None), so it INVERTED the direction and
  told users to delete the LIVE redirect ledger (C4/C4b);
- the displaced vendored entry was unreclaimable by GC/prune forever
  (no cargo in-use probe, C5);
- the remediation text offered deleting the .socket/vendor/<eco>/ tree
  while [patch.crates-io] still referenced it - hard resolution
  failure on every cargo command (C6).

A takeover now leaves the project FULLY in the new mode, or refuses:

- hosted scan reverts each claimed purl's vendored state per purl
  (lock originals restored, [patch] entry dropped, tree + ledger entry
  removed) before redirecting, and refuses the purl when the vendored
  ledger is missing/corrupt;
- every vendored flow (vendor, scan --mode vendored) reverts the
  purl's hosted edits from the redirect ledger FileEdits first
  (Cargo.toml pin, Cargo.lock source/checksum, registries block) and
  drops the ledger record; drifted files refuse fail-closed, and the
  cargo backend refuses (hosted_redirect_live) when hosted wiring is
  live with no ledger to revert it;
- the classifier gains a cargo lock-shape probe (socket-patch registry
  index vs detached-with-[patch]), so the direction follows the lock;
- persist_vendor_entry carries the cargo lock originals forward on
  re-vendor, keeping the crates.io fragment as the ledger's only home;
- dispatch_in_use_one gains the cargo probe so GC/prune reclaims
  genuinely stale vendored entries;
- both takeover remediation texts now recommend only the safe
  sequence (vendor --revert / re-run the vendored flow) and warn
  against hand-deleting ledgers or the vendor tree.

Red-then-green: the new real-cargo migration suite
(tests/mode_migration_cargo.rs, adapted from the audit probes) fails
all four scenarios on the previous code and passes with this change,
proving fresh-checkout `cargo build --locked` in the terminal state of
every migration direction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8e784ca. Configure here.

Comment thread crates/socket-patch-core/src/patch/redirect/takeover.rs
…er-cleanup

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er-cleanup

Resolves the cargo_lock.rs conflict: both sides appended an
independent read-only probe to the same spot. Keeps this branch's
LockEntryProbe/probe_lock_entry (which mode a crate's lock entry
points at, for the takeover logic) alongside main's
count_lock_entries from the vendored fail-closed audit (#194).

Assisted-by: Claude Code:claude-opus-5
…er-cleanup

Picks up #195 (composer redirect confirmation/pinning), which landed on
main while this branch was being reconciled. No overlap with the cargo
takeover work; merged clean.

Assisted-by: Claude Code:claude-opus-5
…er-cleanup

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…v/socket-patch into fix/cargo-mode-takeover-cleanup

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
#174 extracted the re-vendor reconciliation out of the CLI's
persist_vendor_entry into vendor::carry_forward_wiring and extended it
(wiring union for untouched surfaces, pnpm created-surface merge). This
branch had added the cargo lock originals carry-forward to the same
inline block, so both sides rewrote it.

Resolved by keeping main's extraction and moving this branch's cargo
carry-forward into carry_forward_wiring beside the other unconditional,
identity-matched reconciliations: a re-vendor over already-detached
wiring records lock: None, and the crates.io source/checksum pair it
replaced is unrecoverable offline, so the prior entry is its only home.
Placed ahead of the same-uuid early return because that loss happens on
a new-uuid re-vendor too.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
revert_cargo_redirect_purl unwound the ledger's edits newest-first and
wrote each inverse to disk as it went, so a later drifted edit returned
Err with the earlier ones already written. Reverting a hosted redirect
whose Cargo.toml a third party had re-pinned restored Cargo.lock to
crates.io and then refused on the manifest, leaving the crate pinned to
the socket-patch registry with a lock that no longer resolves there --
a project that builds in neither mode. The ledger still recorded the
redirect as live and the caller reported the package as untouched
("cannot vendor over the live hosted redirect"), so nothing pointed at
the half-reverted files and every retry refused on the same drift.

Each inverse is now resolved against a staged view of the files and
nothing is written until all of them resolve. Reads go through the
staged map, which keeps the two orderings the unwind depends on: a
re-redirect chain still sees the previous step's result, and the
registry block still sees the wiring reverts when it probes whether
anything references it. Only an I/O fault during the final flush can
still stop mid-set, and that surfaces as Err naming the file.

The new unit test drifts only Cargo.toml, which unwinds after
Cargo.lock, and asserts all three files are byte-identical after the
refusal; it fails on the previous code with Cargo.lock already
rewritten.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…er-cleanup

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Refines the scan/mod.rs conflict resolution in the preceding merge of
#187 and #191, which crossed with this branch's takeover work.

The vendored-direction remediation had taken #191's text verbatim,
which tells the operator to hand-edit .socket/vendor/redirect-state.json
and delete each superseded package's `records` entry plus its matching
`edits`. #191 could not have known this branch makes that automatic:
re-running `socket-patch vendor` (or `scan --mode vendored`) now reverts
the stale hosted edits from the ledger and drops both halves of the
entry itself. The text leads with that and keeps #191's by-hand
procedure as the fallback, along with its records-AND-edits reasoning
and its never-delete-the-ledger-file warning. Restores the comment
explaining why the ledger is never offered up for hand deletion.

The hosted direction keeps #191's per-package `remove <purl>` guidance;
its comment now also records why `vendor --revert` is not offered -- it
unwinds every vendored package, including ones still live in the
lockfile.

Both sides' tests pass together: #191's
following_the_vendored_remediation_clears_the_warning and
hosted_remediation_states_removes_full_blast_radius alongside this
branch's cargo classifier and GC reclaim tests.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants