Fix vendored revert reading a removed dependency as drift (#1132, #1140, #1142)

crates/socket-patch-cli/tests/e2e_bun_lockb.rs

@@ -997,6 +997,47 @@ async fn native_binary_scan_vendored() {
     fixture.frozen("reverted", &fixture.original, "minimist");
 }
 
+/// #1132: `bun remove minimist` after a vendored scan leaves neither the
+/// vendored nor the pre-vendor record in bun.lockb. `vendor --revert` must
+/// retire the entry and delete its tarball; reading the removal as drift
+/// kept both forever and left `vendor --check` red. Named outside the
+/// `native_binary_` prefix: `scripts/backtest-bun-lockb.py` pins that set.
+#[tokio::test(flavor = "multi_thread")]
+#[serial_test::serial]
+async fn binary_vendored_revert_after_bun_remove() {
+    let Some(fixture) = Fixture::new("direct") else {
+        return;
+    };
+    let server = MockServer::start().await;
+    mock_api(&server, &fixture, "minimist").await;
+    let result = scan(&fixture.project, &server, "vendored", &[]);
+    assert_eq!(
+        result["vendor"]["summary"]["applied"], 1,
+        "scan vendored: {result}"
+    );
+    let mut remove = command(&fixture.reader, &fixture.project);
+    remove
+        .args(["remove", "minimist", "--ignore-scripts"])
+        .env(
+            "BUN_INSTALL_CACHE_DIR",
+            fixture.temp.path().join("remove-cache"),
+        )
+        .env("BUN_INSTALL", fixture.temp.path().join("remove-home"));
+    require_success(remove.output().unwrap(), "bun remove minimist");
+    assert!(!fixture.project.join("bun.lock").exists());
+
+    let result = cli(&fixture.project, &["vendor", "--revert"]);
+    assert_eq!(result["status"], "success", "vendor revert: {result}");
+    assert!(
+        !result.to_string().contains("vendor_lock_entry_drifted"),
+        "a removed dependency is not drift: {result}"
+    );
+    assert!(
+        !fixture.project.join(".socket/vendor").exists(),
+        "the vendored tarball must be deleted: {result}"
+    );
+}
+
 #[tokio::test(flavor = "multi_thread")]
 #[serial_test::serial]
 async fn native_binary_alias_and_transitive() {

crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs

@@ -954,6 +954,15 @@ fn uv_vendor_revert_sub_table_sources() {
 /// the pair (#806, #821). Before the fix the pyproject side reverted alone
 /// and `uv sync --locked` failed while revert reported success.
 fn uv_relock_then_revert(tag: &str, pyproject: &str, relock: &[&str]) {
+    uv_relock_then_unwind(tag, pyproject, relock, true);
+}
+
+/// The shared body of [`uv_relock_then_revert`]. `keeps_vendored` is false
+/// for a relock that drops the dependency altogether (`uv remove six`,
+/// #1140): uv deletes every fragment that routed through the wheel, so the
+/// revert has nothing to restore, and must still converge instead of
+/// drift-keeping the entry (which kept `vendor --check` red forever).
+fn uv_relock_then_unwind(tag: &str, pyproject: &str, relock: &[&str], keeps_vendored: bool) {
     let Some((uv, python)) = capstone_uv(tag) else {
         return;
     };
@@ -1000,9 +1009,11 @@ fn uv_relock_then_revert(tag: &str, pyproject: &str, relock: &[&str]) {
         return;
     }
     let relocked = std::fs::read_to_string(proj.join("uv.lock")).unwrap();
-    assert!(
+    assert_eq!(
         relocked.contains(".socket/vendor/pypi/"),
-        "the relock must keep six vendored: {relocked}"
+        keeps_vendored,
+        "the relock must {} six vendored: {relocked}",
+        if keeps_vendored { "keep" } else { "drop" }
     );
 
     let (code, stdout, stderr) = run_socket(
@@ -1040,6 +1051,21 @@ fn uv_relock_then_revert(tag: &str, pyproject: &str, relock: &[&str]) {
     );
 }
 
+/// #1140: `uv remove six` after vendoring drops the dependency, its
+/// `[tool.uv.sources]` line and every uv.lock fragment that pointed at the
+/// wheel. `vendor --revert` must retire the entry and delete the wheel
+/// instead of keeping both as drift.
+#[test]
+#[serial_test::serial]
+fn uv_vendor_revert_after_uv_remove() {
+    uv_relock_then_unwind(
+        "uv-remove",
+        "[project]\nname = \"vendor-capstone\"\nversion = \"0.1.0\"\nrequires-python = \">=3.9\"\ndependencies = [\"six==1.16.0\", \"attrs>=20\"]\n",
+        &["remove", "-q", "six"],
+        false,
+    );
+}
+
 /// #821: six in a PEP 735 dev group, then `uv add --dev zipp` rewrites the
 /// whole `requires-dev` group line.
 #[test]

crates/socket-patch-core/src/vendor/bun_binary.rs

@@ -451,6 +451,17 @@ pub(crate) async fn revert(entry: &VendorEntry, root: &Path, opts: RevertOpts) -
             }
         }
     }
+    // REMOVED, not drift (#1132): `bun remove <pkg>` (or an upgrade off the
+    // patched version) leaves neither snapshot in the lock. When no package
+    // resolves through this uuid dir any more, there is nothing to restore
+    // and nothing an install needs the artifact for. Probed once, before any
+    // record is restored; an unreadable package table fails closed (drift).
+    let uuid_lower = entry.uuid.to_ascii_lowercase();
+    let unreferenced = lock.packages().is_ok_and(|packages| {
+        !packages
+            .iter()
+            .any(|p| p.resolution.to_ascii_lowercase().contains(&uuid_lower))
+    });
     let mut mirrors_to_remove = Vec::new();
     for rec in entry.wiring.iter().rev() {
         if rec.kind == MIRROR_KIND {
@@ -500,6 +511,7 @@ pub(crate) async fn revert(entry: &VendorEntry, root: &Path, opts: RevertOpts) -
             }
             continue;
         }
+        // `Ok(true)`: the record left the lock (see `unreferenced`).
         let restore = (|| {
             if rec.file != LOCK || rec.kind != KIND {
                 return Err("unexpected binary wiring file or kind".to_string());
@@ -522,15 +534,25 @@ pub(crate) async fn revert(entry: &VendorEntry, root: &Path, opts: RevertOpts) -
             // original never makes this record appear already reverted.
             let id = match lock.find_snapshot_id(id, new)? {
                 Some(id) => id,
-                None if lock.find_snapshot_id(id, original)?.is_some() => return Ok(()),
+                None if lock.find_snapshot_id(id, original)?.is_some() => return Ok(false),
+                None if unreferenced => return Ok(true),
                 None => return Err("binary package resolution has drifted".into()),
             };
-            lock.restore(id, original)
+            lock.restore(id, original).map(|()| false)
         })();
-        if let Err(e) = restore {
-            outcome
+        match restore {
+            Ok(true) => outcome.warnings.push(VendorWarning::new(
+                super::LOCK_ENTRY_REMOVED_CODE,
+                format!(
+                    "{LOCK} no longer resolves {} through {dir} (the dependency was removed or \
+                     re-resolved); nothing to restore",
+                    entry.base_purl
+                ),
+            )),
+            Ok(false) => {}
+            Err(e) => outcome
                 .warnings
-                .push(VendorWarning::new("vendor_lock_entry_drifted", e));
+                .push(VendorWarning::new("vendor_lock_entry_drifted", e)),
         }
     }
     if outcome.drift_skipped() {
@@ -1014,6 +1036,58 @@ mod rebuild_tests {
         assert_eq!(ts::request_count(&server).await, 0);
     }
 
+    /// #1132: once `bun remove minimist` (or `bun add minimist@1.2.8`)
+    /// moves the vendored record off its vendored resolution, neither the
+    /// rewritten nor the pre-vendor snapshot is in bun.lockb and nothing
+    /// resolves through the uuid dir. There is nothing to restore, so the
+    /// revert warns `vendor_lock_entry_removed` and finishes; reading it as
+    /// drift kept the tarball and ledger entry forever and looped
+    /// `vendor --check` → `scan --prune`.
+    #[tokio::test]
+    async fn revert_after_package_left_the_lock_is_not_drift() {
+        use base64::Engine as _;
+        let fx = flip_fixture().await;
+        let root = fx.root();
+        let VendorOutcome::Done {
+            result,
+            entry: Some(entry),
+            ..
+        } = flip_run(&fx, None).await
+        else {
+            panic!("vendoring must wire the binary lock");
+        };
+        assert!(result.success, "{result:?}");
+        let binary = entry.wiring.iter().find(|r| r.kind == KIND).unwrap();
+        let id: usize = binary.key.as_ref().unwrap().parse().unwrap();
+        let mut lock = BunLockb::parse(&std::fs::read(root.join(LOCK)).unwrap()).unwrap();
+        let integrity = format!(
+            "sha512-{}",
+            base64::engine::general_purpose::STANDARD.encode([7u8; 64])
+        );
+        lock.set_registry_package(
+            id,
+            "1.2.8",
+            "https://registry.npmjs.org/minimist/-/minimist-1.2.8.tgz",
+            &integrity,
+        )
+        .unwrap();
+        let relocked = lock.bytes();
+        assert!(!lock
+            .packages()
+            .unwrap()
+            .iter()
+            .any(|p| p.resolution.contains(UUID)));
+        std::fs::write(root.join(LOCK), &relocked).unwrap();
+
+        let outcome = revert(&entry, root, RevertOpts::new(false)).await;
+        assert!(outcome.success, "{outcome:?}");
+        assert!(!outcome.drift_skipped(), "{outcome:?}");
+        assert!(outcome.lock_entry_removed(), "{outcome:?}");
+        assert!(!outcome.kept_artifact, "{outcome:?}");
+        assert_eq!(std::fs::read(root.join(LOCK)).unwrap(), relocked);
+        assert!(!root.join(&entry.artifact.path).exists());
+    }
+
     /// With the canonical tarball GONE, an outage switches the same UUID
     /// from a prebuilt archive to a locally packed one. Different archive
     /// bytes must advance the integrity snapshot without losing the pristine

crates/socket-patch-core/src/vendor/pypi_pipenv.rs

@@ -597,6 +597,29 @@ pub(super) async fn revert_pipenv(
             warnings.push(drifted());
             continue;
         }
+        // A relock dropped the entry (`pipenv uninstall <pkg>`, a Pipfile
+        // edit + `pipenv lock`), or, on Pipenv 2022/2023, the whole category
+        // it emptied (#1142): the vendored reference is gone with it, so
+        // retire the record rather than keep the orphan forever. Only while
+        // no other entry still routes through this uuid dir — that one
+        // would need the artifact, so it stays drift.
+        let relocked_away = |lock: &Value| {
+            if rec.action == WiringAction::Rewritten
+                && rec.original.is_some()
+                && !to_canonical_json(lock).contains(&entry.uuid)
+            {
+                VendorWarning::new(
+                    "vendor_lock_entry_relocked",
+                    format!("{LOCK_FILE} entry for {:?} was removed by a relock; the vendored reference is already gone, so the record is retired", rec.key),
+                )
+            } else {
+                drifted()
+            }
+        };
+        if lock.get(section).is_none() {
+            warnings.push(relocked_away(&lock));
+            continue;
+        }
         let Some(map) = lock.get_mut(section).and_then(Value::as_object_mut) else {
             warnings.push(drifted());
             continue;
@@ -615,17 +638,7 @@ pub(super) async fn revert_pipenv(
             continue;
         };
         let Some(live) = map.get(name) else {
-            if rec.action == WiringAction::Rewritten && rec.original.is_some() {
-                // A relock dropped the entry (`pipenv uninstall <pkg>`, a Pipfile
-                // edit + `pipenv lock`): the vendored reference is gone with
-                // it — retire the record rather than keep the orphan forever.
-                warnings.push(VendorWarning::new(
-                    "vendor_lock_entry_relocked",
-                    format!("{LOCK_FILE} entry for {:?} was removed by a relock; the vendored reference is already gone, so the record is retired", rec.key),
-                ));
-            } else {
-                warnings.push(drifted());
-            }
+            warnings.push(relocked_away(&lock));
             continue;
         };
         // Still OUR reference (`file`/`path` string identical to what we
@@ -1801,7 +1814,7 @@ mod tests {
     /// successful revert with exactly one drift warning and the lock bytes
     /// UNCHANGED (changed=false must skip re-serialization entirely).
     #[tokio::test]
-    async fn revert_drift_skips_missing_section_entry_new_and_missing_original() {
+    async fn revert_retires_missing_section_entry_and_drift_skips_new_and_missing_original() {
         use serde_json::json;
         let mut no_develop: Value = serde_json::from_str(LOCK_DIRECT_REGISTRY).unwrap();
         no_develop.as_object_mut().unwrap().remove("develop");
@@ -1856,9 +1869,9 @@ mod tests {
             let outcome = revert_pipenv(&entry_for(vec![record], meta), tmp.path(), false).await;
             assert!(outcome.success, "{label}: {:?}", outcome.error);
             assert_eq!(outcome.warnings.len(), 1, "{label}: {:?}", outcome.warnings);
-            // An entry a relock REMOVED retires the record (the reference is
-            // gone with it); every other mismatch is drift.
-            let expected = if label.contains("entry removed") {
+            // An entry or category a relock REMOVED retires the record (the
+            // reference is gone with it, #1142); every other mismatch is drift.
+            let expected = if label.contains("entry removed") || label.contains("section deleted") {
                 "vendor_lock_entry_relocked"
             } else {
                 "vendor_lock_entry_drifted"
@@ -1877,6 +1890,61 @@ mod tests {
         }
     }
 
+    /// #1142: `pipenv uninstall six --categories docs` on Pipenv 2022/2023
+    /// drops the emptied `docs` key from Pipfile.lock altogether (2026 keeps
+    /// `"docs": {}`). The vendored reference left with the category, so the
+    /// record retires exactly like an entry a relock removed — reading it as
+    /// drift kept the wheel and ledger entry forever and looped
+    /// `vendor --check` → `scan --prune`. A lock that still names the uuid
+    /// anywhere (the entry moved to another category by hand) stays drift.
+    #[tokio::test]
+    async fn revert_retires_record_whose_category_a_relock_dropped() {
+        let registry_six =
+            serde_json::from_str::<Value>(LOCK_DIRECT_REGISTRY).unwrap()["default"]["six"].clone();
+        let vendored_six =
+            serde_json::from_str::<Value>(LOCK_DIRECT_VENDORED).unwrap()["default"]["six"].clone();
+        let record = WiringRecord {
+            file: LOCK_FILE.to_string(),
+            kind: KIND_LOCK_ENTRY.to_string(),
+            action: WiringAction::Rewritten,
+            key: Some("docs:six".to_string()),
+            original: Some(registry_six),
+            new: Some(vendored_six.clone()),
+        };
+        let meta = || PipenvMeta {
+            sections: vec!["docs".into()],
+        };
+
+        // The lock after the uninstall: no `docs` key, and nothing else
+        // mentions the vendored wheel.
+        let mut relocked: Value = serde_json::from_str(LOCK_DIRECT_REGISTRY).unwrap();
+        relocked["default"].as_object_mut().unwrap().remove("six");
+        let relocked_text = to_canonical_json(&relocked);
+        let tmp = write_lock(&relocked_text).await;
+        let outcome =
+            revert_pipenv(&entry_for(vec![record.clone()], meta()), tmp.path(), false).await;
+        assert!(outcome.success, "{:?}", outcome.error);
+        assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings);
+        assert_eq!(outcome.warnings.len(), 1, "{:?}", outcome.warnings);
+        assert_eq!(outcome.warnings[0].code, "vendor_lock_entry_relocked");
+        assert!(outcome.warnings[0].detail.contains("docs:six"));
+        assert_eq!(read_lock(tmp.path()).await, relocked_text);
+
+        // The same record while another category still routes through the
+        // wheel: deleting it would break that install, so it is drift.
+        let mut moved = relocked.clone();
+        moved["default"]
+            .as_object_mut()
+            .unwrap()
+            .insert("six".into(), vendored_six);
+        let moved_text = to_canonical_json(&moved);
+        let tmp = write_lock(&moved_text).await;
+        let outcome = revert_pipenv(&entry_for(vec![record], meta()), tmp.path(), false).await;
+        assert!(outcome.success, "{:?}", outcome.error);
+        assert!(outcome.drift_skipped(), "{:?}", outcome.warnings);
+        assert_eq!(read_lock(tmp.path()).await, moved_text);
+    }
+
     /// The Added arm removes a lock entry — a destructive edit driven by
     /// tamper-able state.json, gated ONLY by the live==rec.new deep-equality
     /// check. wire_pipenv never emits Added (forward-compat/defensive), so