install: remove node_modules entries that left the lockfile

src/install/PackageInstall.rs

@@ -1,6 +1,6 @@
 use core::sync::atomic::{AtomicU8, Ordering};
 
-use bun_collections::{ArrayHashMap, DynamicBitSet};
+use bun_collections::{ArrayHashMap, DynamicBitSet, StringHashMap};
 use bun_core::Progress::Progress;
 use bun_core::{Global, Output};
 use bun_core::{MutableString, ZStr};
@@ -2562,4 +2562,130 @@ impl<'a> PackageInstall<'a> {
     }
 }
 
+/// Remove top-level entries from an already-open root `node_modules`
+/// directory that are not reachable from `lockfile`. Scoped packages are
+/// handled by descending one level into `@scope/` and removing the empty
+/// scope directory if nothing remains. Dot-prefixed entries (`.bin`, `.bun`,
+/// `.cache`, …) are never touched.
+///
+/// Called by both the hoisted and isolated install paths after
+/// `Lockfile::clean_with_logger` has rebuilt the lockfile, so a dependency
+/// removed from `package.json` is also removed from disk on the next
+/// `bun install` instead of remaining importable until the user deletes
+/// `node_modules` by hand.
+///
+/// The kept set is every dependency alias in the lockfile (the alias is the
+/// `node_modules/<name>` folder both installers write), not the subset this
+/// particular invocation places at the root. That keeps the prune keyed to
+/// the lockfile graph rather than to install flags, so `--production`,
+/// `--omit`, `--filter`, and `--cpu`/`--os` overrides never turn a reinstall
+/// into a delete of packages the lockfile still contains.
+pub(crate) fn prune_extraneous_node_modules(lockfile: &Lockfile, node_modules: Fd) {
+    // A valid project always has at least the root package after `clean`, so
+    // zero packages means this lockfile never resolved; refuse to prune
+    // against it. A root with zero dependencies is a legitimate state (the
+    // last dependency was just removed) and must still prune.
+    if lockfile.packages.is_empty() {
+        return;
+    }
+
+    let string_buf = lockfile.buffers.string_bytes.as_slice();
+    let deps = lockfile.buffers.dependencies.as_slice();
+    let mut expected = StringHashMap::<()>::with_capacity(deps.len());
+    for dep in deps {
+        let alias = dep.name.slice(string_buf);
+        if !alias.is_empty() {
+            let _ = expected.put(alias, ());
+        }
+    }
+    let expected = &expected;
+
+    let dir = Dir::borrow(&node_modules);
+
+    // Snapshot the listing before deleting anything. `delete_tree` mutates
+    // the directory, and on some filesystems a readdir refill after a delete
+    // can re-surface entries; iterating an owned list sidesteps that.
+    let mut names: Vec<Vec<u8>> = Vec::new();
+    let mut iter = sys::iterate_dir(node_modules);
+    loop {
+        let entry = match iter.next() {
+            Ok(Some(e)) => e,
+            Ok(None) => break,
+            Err(_) => return,
+        };
+        let name = entry.name.slice_u8();
+        if name.is_empty() || name[0] == b'.' {
+            continue;
+        }
+        names.push(name.to_vec());
+    }
+
+    for name in &names {
+        if name[0] == b'@' {
+            prune_extraneous_scope(node_modules, name, expected);
+            continue;
+        }
+        if expected.contains_key(name.as_slice()) {
+            continue;
+        }
+        let _ = dir.delete_tree(name);
+    }
+}
+
+fn prune_extraneous_scope(parent: Fd, scope: &[u8], expected: &StringHashMap<()>) {
+    let scope_fd = match sys::open_dir_for_iteration(parent, scope) {
+        Ok(fd) => fd,
+        Err(_) => return,
+    };
+    let scope_dir = Dir::from_fd(scope_fd);
+
+    let mut names: Vec<Vec<u8>> = Vec::new();
+    let mut has_remaining = false;
+    let mut iter = sys::iterate_dir(scope_dir.fd());
+    loop {
+        let entry = match iter.next() {
+            Ok(Some(e)) => e,
+            Ok(None) => break,
+            Err(_) => return,
+        };
+        let name = entry.name.slice_u8();
+        if name.is_empty() {
+            continue;
+        }
+        if name[0] == b'.' {
+            has_remaining = true;
+            continue;
+        }
+        names.push(name.to_vec());
+    }
+
+    for name in &names {
+        let mut full = Vec::with_capacity(scope.len() + 1 + name.len());
+        full.extend_from_slice(scope);
+        full.push(b'/');
+        full.extend_from_slice(name);
+
+        if expected.contains_key(full.as_slice()) {
+            has_remaining = true;
+            continue;
+        }
+
+        if scope_dir.delete_tree(name).is_err() {
+            has_remaining = true;
+        }
+    }
+
+    // `scope_dir` owns the fd and must be dropped before removing the
+    // directory it points at (Windows cannot remove a dir with open handles).
+    drop(scope_dir);
+
+    if !has_remaining {
+        let mut buf = Vec::with_capacity(scope.len() + 1);
+        buf.extend_from_slice(scope);
+        buf.push(0);
+        let z = ZStr::from_buf(&buf, scope.len());
+        let _ = sys::rmdirat(parent, z);
+    }
+}
+
 type Walker = walker_skippable::Walker;

src/install/hoisted_install.rs

@@ -215,6 +215,14 @@ pub(crate) fn install_hoisted_packages(
         }
     };
 
+    // Remove entries the cleaned lockfile no longer reaches, so a dependency
+    // dropped from `package.json` is also removed from disk by `bun install`.
+    // Skipped for the security scanner's narrowed pre-install pass; the full
+    // install that follows performs it.
+    if !new_node_modules && packages_to_install.is_none() {
+        package_install::prune_extraneous_node_modules(&this.lockfile, node_modules_folder.fd());
+    }
+
     let mut skip_delete = new_node_modules;
     let mut skip_verify_installed_version_number = new_node_modules;
 

src/install/isolated_install.rs

@@ -1971,6 +1971,18 @@ pub(crate) fn install_isolated_packages(
         let pkg_name_hashes = pkgs.items_name_hash();
         let pkg_resolutions = pkgs.items_resolution();
 
+        // Remove root `node_modules` entries the cleaned lockfile no longer
+        // reaches, so a dependency dropped from `package.json` is also removed
+        // from disk by `bun install`. Skipped for the security scanner's
+        // narrowed pre-install pass; the full install that follows performs it.
+        if !is_new_bun_modules && packages_to_install.is_none() {
+            if let Ok(fd) = sys::open_dir_for_iteration(Fd::cwd(), b"node_modules") {
+                crate::package_install::prune_extraneous_node_modules(lockfile_ro, fd);
+                use bun_sys::FdExt as _;
+                fd.close();
+            }
+        }
+
         let mut seen_entry_ids: HashMap<store::entry::Id, ()> = HashMap::default();
         seen_entry_ids.reserve(store.entries.len());
 

test/cli/install/bun-install-lifecycle-scripts.test.ts

@@ -3068,8 +3068,9 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) {
     });
 
     describe("add trusted, delete, then add again", async () => {
-      // when we change bun install to delete dependencies from node_modules
-      // for both cases, we need to update this test
+      // `bun install` prunes node_modules entries removed from the lockfile,
+      // so a dependency deleted from package.json by hand (withRm: false) is
+      // removed from disk the same way `bun rm` (withRm: true) removes it.
       for (const withRm of [true, false]) {
         test(withRm ? "withRm" : "withoutRm", async () => {
           using ctx = await setupTest();
@@ -3193,7 +3194,9 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) {
           expected = [expect.stringContaining("bun install v1."), ...expected];
           expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual(expected);
           expect(await exited).toBe(0);
-          expect(await exists(join(packageDir, "node_modules", "uses-what-bin"))).toBe(!withRm);
+          // pruned from node_modules on both paths now that the lockfile
+          // no longer reaches it
+          expect(await exists(join(packageDir, "node_modules", "uses-what-bin"))).toBe(false);
 
           // add again, bun pm untrusted should report it as untrusted
 
@@ -3222,18 +3225,19 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) {
           expect(err).not.toContain("error:");
           expect(err).not.toContain("warn:");
           out = await stdout.text();
-          expected = withRm
-            ? [
-                "",
-                expect.stringContaining("+ uses-what-bin@1.0.0"),
-                "",
-                "1 package installed",
-                "",
-                "Blocked 1 postinstall. Run `bun pm untrusted` for details.",
-                "",
-              ]
-            : ["", expect.stringContaining("Checked 3 installs across 4 packages (no changes)"), ""];
-          expected = [expect.stringContaining("bun install v1."), ...expected];
+          // The prune removed node_modules/uses-what-bin and its hoisted
+          // transitive dependency what-bin on both paths, so re-adding it
+          // always performs a fresh, untrusted install of both.
+          expected = [
+            expect.stringContaining("bun install v1."),
+            "",
+            expect.stringContaining("+ uses-what-bin@1.0.0"),
+            "",
+            "2 packages installed",
+            "",
+            "Blocked 1 postinstall. Run `bun pm untrusted` for details.",
+            "",
+          ];
           expect(out.replace(/\s*\[[0-9\.]+m?s\]$/m, "").split(/\r?\n/)).toEqual(expected);
 
           ({ stdout, stderr, exited } = spawn({