fix(vendor): keep artifacts and ledger when revert drift-skips the lock restore

[Mikola Lysenko (mikolalysenko)]

Root cause (residual #131, adversarially re-confirmed on main 2026-08-18)

Two-stage lossiness when vendor --revert drift-skips the lock restore:

1. Backend — crates/socket-patch-core/src/vendor/yarn_classic_lock.rs:352-354: revert_yarn_classic ran remove_tree(.socket/vendor/npm/<uuid>/) unconditionally after the restore loop, even when revert_recorded_block (:404-413) drift-skipped EVERY wiring record with vendor_lock_entry_drifted "left alone" (the ownership gate fails for any resolved URL outside our uuid dir — a hosted patch.socket.dev overlay and a registry re-resolve alike). Same shape in the npm-family twins: npm_lock.rs:460, pnpm_lock.rs:593, yarn_berry_lock.rs:631, bun_lock.rs:345.
2. Orchestrator — crates/socket-patch-cli/src/commands/vendor.rs (run_revert :1366-1370, reconcile_dropped :1319-1326): RevertOutcome had no "kept" channel, so the all-skipped case was plain success ⇒ PatchAction::Removed + state.entries.remove(purl) + save_state. Sub-bug: the skip events pushed by record_warning bypass env.record, so summary.skipped stayed 0 while the events array showed the skip.

End state (matrix cell E1, edgey1-layer-revert): yarn.lock still pointed at patch.socket.dev, but the artifacts, the ledger entry holding the only pre-vendor original lock fragments, and the tarball that redirect-state.json's recorded edit original references were all deleted — silent-success data loss plus a dangling-replay hazard for any future hosted-edit restore.

Fix

Reuses the orphan sweep's invariant (vendor.rs:1392-1413): never delete what something still references.

• RevertOutcome (core vendor/mod.rs) grows kept_artifact: bool + drift_skipped() / keep_artifact() helpers. keep_artifact() pushes an honest vendor_artifact_kept warning naming the kept dir and the remediation ("undo the drift and re-run vendor --revert").
• All five npm-family backends with the unconditional-remove shape (yarn_classic_lock, npm_lock, pnpm_lock, yarn_berry_lock, bun_lock) skip the remove_tree and mark the keep whenever any wiring record was drift-skipped.
• run_revert / reconcile_dropped honor the signal: state.json entry retained, outcome recorded as a counted Skipped (vendor_revert_kept) via env.record — fixing summary.skipped == 0 — never a Removed; counts stay uninflated per the fix(vendor): count skipped packages, not advisory warnings #166 precedent (advisory warnings keep bypassing the counters). Human output gains a Kept N drifted package(s)… line.
• Other RevertOutcome construction sites get kept_artifact: false (zero behavior change outside the npm family). Once the drift is undone, the next vendor --revert completes exactly as before (lock restored byte-for-byte, artifacts + ledger pruned) — proven by test legs.

socket-patch remove <purl> (the hosted-mode-prescribed cleanup) still prunes the manifest + ledger by explicit user intent; with this fix the kept artifact dir simply survives until the next vendor run's orphan sweep instead of being deleted while still referenced.

Tests (TDD, RED first — commit 841f8a4, fix 7a2013f)

• Flipped the deliberate fix(redirect): fail closed on non-npm: berry descriptors + yarn layering regression suite #131 pinning test classic_vendor_revert_after_hosted_overlay_is_drift_skipped_and_lossy → …_keeps_blob (yarn_layering_tests.rs), per its own doc-comment instruction; now also proves the follow-up revert completes once the drift is undone.
• Extended the five backends' revert_leaves_drifted_* unit tests: artifact dir kept + vendor_artifact_kept surfaced.
• New CLI orchestration tests (in_process_vendor.rs): revert_after_drift_keeps_artifact_and_ledger_then_completes and reconcile_keeps_drifted_entry — ledger retained, counted Skipped (summary.skipped == 1, removed == 0), no Removed event, second revert completes.
• New e2e capstone (e2e_vendor_yarn_classic_dev_flow.rs): real yarn 1.22.22 via corepack — vendored → hosted-URL overlay → vendor --revert keeps tarball/ledger → drift undone → kept tarball still installs the patched bytes → second revert completes → pristine registry install.

RED evidence: 6 core + 2 CLI + 1 e2e tests failed on main behavior (summary.skipped: 0, removed: 1, artifact dirs deleted).

Matrix / manual evidence (old vs new binary, throwaway copies)

Replayed the brief's verify repro (matrix2/verify/C-revert-driftskip-lossy-131, real prod scan --vendor of minimist@1.2.2, anonymous free tier) on throwaway copies:

binary	revert exit	summary	.socket/vendor after
old (main)	0	removed 1, skipped 0	empty — tarball, inventory, state.json all deleted; lock still drifted
this PR	0	removed 0, skipped 1	minimist-1.2.2.tgz, socket-patch.vendor.json, state.json all kept

Synthetic E1 (hosted patch.socket.dev overlay over vendored left-pad) shows the same flip, and the recovery leg on the new binary (restore vendored lock → revert) ends with removed 1, vendor tree fully pruned, registry lock restored, yarn install exit 0 with pristine bytes.

Gates

• cargo test -p socket-patch-core — clean (22/22 binaries ok)
• cargo test -p socket-patch-cli — clean
• cargo clippy --workspace --all-features -- -D warnings — clean
• fmt: only touched files formatted

Sibling-PR conflict notes (parallel campaign)

• Lane B fix/npm-vendored-over-hosted-prerevert: touches cli vendor.rs and core redirect takeover — this PR edits run_revert/reconcile_dropped in cli vendor.rs (lines ~1319-1345, ~1366-1400) and the human summary print; expect textual overlap in crates/socket-patch-cli/src/commands/vendor.rs.
• Lane ADE fix/mode-conversion-visibility-warnings: no shared files expected (scan/mod.rs, redirect/mod.rs berry branch, CLI_CONTRACT.md).
• PR fix(scan): surface yarn PnP refusal instead of silent success-0 across all modes #201 (scan/mod.rs, discovery.rs, lock_inventory.rs, apply.rs, e2e_safety_yarn_pnp.rs) and PR test(yarn): version-matrix e2e capstones — yarn2/3 refusal pins, berry workspaces + pnpm-linker install proofs #200 (three new e2e_yarn* test files): no overlap — this PR does not touch those files' changed regions.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes vendoring revert semantics and ledger persistence for drift-skipped lock restores across npm-family backends and CLI orchestration; behavior is well-tested but affects data retention and JSON summary counts on a sensitive path.

Overview
Fixes residual #131: vendor --revert used to leave drifted lock entries untouched but still delete .socket/vendor/ artifacts and prune state.json, reporting success with Removed / summary.removed instead of an honest skip.

Core: RevertOutcome adds kept_artifact plus drift_skipped() / keep_artifact() (with vendor_artifact_kept warnings). The npm-family revert backends (yarn_classic, npm_lock, pnpm, yarn_berry, bun) skip remove_tree when any wiring record was drift-skipped.

CLI: run_revert and reconcile_dropped retain ledger entries and emit a counted Skipped (vendor_revert_kept) via env.record—not Removed—and human output adds a “Kept N drifted package(s)…” line when summary.skipped > 0.

After undoing drift, a follow-up vendor --revert still completes as before (lock restored, vendor tree pruned). Tests flip the prior lossy pinning test and add e2e/in-process coverage for keep + recovery.

^{Reviewed by Cursor Bugbot for commit 7a2013f. Configure here.}

[cursor]

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

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: GC ignores artifact-keep signal
  • Added kept_artifact checks to both manifest-dropped and lockfile-unused paths in run_vendor_gc to prevent ledger removal when backends deliberately retain artifacts during drift-skip keeps.

Or push these changes by commenting:

@cursor push dfde23a211

Preview (dfde23a211)

diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs
--- a/crates/socket-patch-cli/src/commands/vendor.rs
+++ b/crates/socket-patch-cli/src/commands/vendor.rs
@@ -1568,10 +1568,13 @@
                 continue;
             }
             let entry = state.entries.get(&purl).cloned().expect("listed above");
-            if dispatch_revert_one(&entry, &common.cwd, false)
-                .await
-                .success
-            {
+            let outcome = dispatch_revert_one(&entry, &common.cwd, false).await;
+            if outcome.success {
+                if outcome.kept_artifact {
+                    // Drift-skip keep: backend kept the uuid dir, so the ledger
+                    // entry must survive too (same contract as reconcile_dropped).
+                    continue;
+                }
                 state.entries.remove(&purl);
                 out.dropped_reverted.push(purl);
             } else {
@@ -1601,13 +1604,16 @@
             out.unused_reverted.push(purl);
             continue;
         }
-        if !dispatch_revert_one(&entry, &common.cwd, false)
-            .await
-            .success
-        {
+        let outcome = dispatch_revert_one(&entry, &common.cwd, false).await;
+        if !outcome.success {
             out.failed.push(purl);
             continue;
         }
+        if outcome.kept_artifact {
+            // Drift-skip keep: backend kept the uuid dir, so the ledger
+            // entry must survive too (same contract as reconcile_dropped).
+            continue;
+        }
         state.entries.remove(&purl);
         if let Some(m) = manifest.as_mut() {
             let base = strip_purl_qualifiers(&entry.base_purl).to_string();

_{You can send follow-ups to the cloud agent here.}

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

^{Reviewed by Cursor Bugbot for commit 7a2013f. Configure here.}

[cursor]

GC ignores artifact-keep signal

High Severity

run_vendor_gc still treats any successful revert as a full removal and prunes the ledger whenever success is true. It never checks kept_artifact, unlike reconcile_dropped and run_revert. On a drift-skip keep, the backend retains the uuid dir, GC drops the ledger entry, then the trailing orphan sweep deletes the artifacts because the drifted lock no longer points into that dir — recreating residual #131 via scan --prune.

 

^{Reviewed by Cursor Bugbot for commit 7a2013f. Configure here.}