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,170 @@ impl<'a> PackageInstall<'a> {
     }
 }
 
+/// Remove top-level entries from an already-open root `node_modules`
+/// directory whose names are not in `expected`. 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. Finishes with a sweep of `bin_path` that
+/// unlinks any `.bin` entry left dangling by a deleted package.
+///
+/// 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.
+///
+/// Each linker builds `expected` as "every folder name the lockfile can
+/// legitimately place at the root", independent of install flags: the root
+/// package's declared dependency aliases (all behaviors, so `--production`
+/// or `--omit` never turn a reinstall into a delete) unioned with the
+/// linker's own root placements (the hoisted root tree, or the isolated root
+/// store entry's dependencies which include `publicHoistPattern` hoists).
+///
+/// The prune is best effort: a stale directory the filesystem refuses to
+/// delete (locked, read-only) is left behind rather than failing the
+/// install, matching the uninstall paths elsewhere in this file.
+pub(crate) fn prune_extraneous_node_modules(
+    expected: &StringHashMap<()>,
+    node_modules: Fd,
+    bin_path: &[u8],
+) {
+    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());
+    }
+
+    let mut removed_any = false;
+    for name in &names {
+        if name[0] == b'@' {
+            removed_any |= prune_extraneous_scope(node_modules, name, expected);
+            continue;
+        }
+        if expected.contains_key(name.as_slice()) {
+            continue;
+        }
+        if dir.delete_tree(name).is_ok() {
+            removed_any = true;
+        }
+    }
+
+    // A deleted package's `.bin` symlink now dangles; sweep it so scripts get
+    // "command not found" instead of an ENOENT on the link target.
+    if removed_any {
+        if let Ok(bin_dir) = sys::open_dir_for_iteration(Fd::cwd(), bin_path) {
+            prune_dangling_bin_links(bin_dir);
+        }
+    }
+}
+
+/// Unlink every dangling symlink in the already-open `.bin` directory and
+/// close `bin_dir`. Shared by the install-time prune above and the
+/// `bun remove` cleanup in `updatePackageJSONAndInstall.rs`.
+pub(crate) fn prune_dangling_bin_links(bin_dir: Fd) {
+    let mut name_buf = PathBuffer::uninit();
+    let mut iter = sys::iterate_dir(bin_dir);
+    'iterator: loop {
+        let Ok(Some(entry)) = iter.next() else { break };
+        match entry.kind {
+            EntryKind::SymLink => {
+                // Any symlink we cannot open is assumed dangling. `access`
+                // would not work here because it does not resolve symlinks.
+                let name = entry.name.slice_u8();
+                name_buf[..name.len()].copy_from_slice(name);
+                name_buf[name.len()] = 0;
+                let buf: &ZStr = ZStr::from_buf(&name_buf, name.len());
+
+                match sys::File::openat(bin_dir, buf, sys::O::RDONLY, 0) {
+                    Ok(file) => {
+                        let _ = file.close();
+                    }
+                    Err(_) => {
+                        let _ = sys::unlinkat(bin_dir, buf);
+                        continue 'iterator;
+                    }
+                }
+            }
+            _ => {}
+        }
+    }
+    let _ = sys::close(bin_dir);
+}
+
+/// Returns `true` if anything was deleted.
+fn prune_extraneous_scope(parent: Fd, scope: &[u8], expected: &StringHashMap<()>) -> bool {
+    let scope_fd = match sys::open_dir_for_iteration(parent, scope) {
+        Ok(fd) => fd,
+        Err(_) => return false,
+    };
+    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 false,
+        };
+        let name = entry.name.slice_u8();
+        if name.is_empty() {
+            continue;
+        }
+        if name[0] == b'.' {
+            has_remaining = true;
+            continue;
+        }
+        names.push(name.to_vec());
+    }
+
+    let mut removed_any = false;
+    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_ok() {
+            removed_any = true;
+        } else {
+            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);
+    }
+
+    removed_any
+}
+
 type Walker = walker_skippable::Walker;

src/install/PackageManager/updatePackageJSONAndInstall.rs

@@ -692,38 +692,7 @@ fn update_package_json_and_install_with_manager_with_updates(
             // This could be slow if there are a lot of symlinks
             match bun_sys::open_dir_for_iteration(cwd.fd(), manager.options.bin_path.as_bytes()) {
                 Ok(node_modules_bin) => {
-                    // `defer node_modules_bin.close()` — explicit close below (Fd is Copy, no Drop).
-                    let mut iter = bun_sys::iterate_dir(node_modules_bin);
-                    'iterator: loop {
-                        let Ok(Some(entry)) = iter.next() else { break };
-                        match entry.kind {
-                            bun_sys::EntryKind::SymLink => {
-                                // any symlinks which we are unable to open are assumed to be dangling
-                                // note that using access won't work here, because access doesn't resolve symlinks
-                                let name = entry.name.slice_u8();
-                                node_modules_buf[..name.len()].copy_from_slice(name);
-                                node_modules_buf[name.len()] = 0;
-                                let buf: &ZStr = ZStr::from_buf(&node_modules_buf, name.len());
-
-                                match bun_sys::File::openat(
-                                    node_modules_bin,
-                                    buf,
-                                    bun_sys::O::RDONLY,
-                                    0,
-                                ) {
-                                    Ok(file) => {
-                                        let _ = file.close();
-                                    }
-                                    Err(_) => {
-                                        let _ = bun_sys::unlinkat(node_modules_bin, buf);
-                                        continue 'iterator;
-                                    }
-                                }
-                            }
-                            _ => {}
-                        }
-                    }
-                    let _ = bun_sys::close(node_modules_bin);
+                    crate::package_install::prune_dangling_bin_links(node_modules_bin);
                 }
                 Err(err) => {
                     if err.get_errno() != bun_sys::E::ENOENT {

src/install/hoisted_install.rs

@@ -115,6 +115,50 @@
     // block above, so no other borrow of `*mgr_ptr` is live here.
     let this = unsafe { &mut *mgr_ptr };
 
+    // Every folder name the lockfile can legitimately place at the root
+    // `node_modules`, so stale entries on disk can be pruned once the
+    // directory is opened below: the root package's declared dependency
+    // aliases (all behaviors, so `--production`/`--omit` never turn a
+    // reinstall into a delete) unioned with the unfiltered root tree (the
+    // hoisted transitives that belong there; `original_trees` was snapshotted
+    // before `filter()`, so `--filter` cannot shrink it either). Skipped for
+    // the security scanner's narrowed pre-install pass; the full install
+    // that follows performs the prune.
+    let expected_root_entries: Option<StringHashMap<()>> = 'expected: {
+        if packages_to_install.is_some() || this.lockfile.packages.is_empty() {
+            break 'expected None;
+        }
+        let deps = this.lockfile.buffers.dependencies.as_slice();
+        let string_buf = this.lockfile.buffers.string_bytes.as_slice();
+        // A lockfile with no dependencies at all has zero trees; that is a
+        // legitimate state (the last dependency was just removed) in which
+        // nothing belongs at the root, not a reason to skip the prune.
+        let root_tree_dep_ids: &[DependencyID] = if original_trees.is_empty() {
+            &[]
+        } else {
+            original_trees[0].dependencies.get(&original_tree_dep_ids)
+        };
+        let root_pkg_deps = this.lockfile.packages.slice().items_dependencies()[0];
+        let mut set = StringHashMap::<()>::with_capacity(
+            root_tree_dep_ids.len() + root_pkg_deps.len as usize,
+        );
+        let mut keep = |dep_id: DependencyID| {
+            if let Some(dep) = deps.get(dep_id as usize) {
+                let alias = dep.name.slice(string_buf);
+                if !alias.is_empty() {
+                    let _ = set.put(alias, ());
+                }
+            }
+        };
+        for &dep_id in root_tree_dep_ids {
+            keep(dep_id);
+        }
+        for dep_id in root_pkg_deps.begin()..root_pkg_deps.end() {
+            keep(dep_id);
+        }
+        break 'expected Some(set);
+    };
+
     let _restore_buffers = scopeguard::guard(
         (original_trees, original_tree_dep_ids),
         move |(trees, dep_ids)| {
@@ -215,6 +259,19 @@
         }
     };
 
+    // Remove entries the cleaned lockfile no longer places at the root, so a
+    // dependency dropped from `package.json` is also removed from disk by
+    // `bun install`.
+    if !new_node_modules {
+        if let Some(expected) = expected_root_entries.as_ref() {
+            package_install::prune_extraneous_node_modules(
+                expected,
+                node_modules_folder.fd(),
+                this.options.bin_path.as_bytes(),
+            );
+        }
+    }
+
     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,50 @@
         let pkg_name_hashes = pkgs.items_name_hash();
         let pkg_resolutions = pkgs.items_resolution();
 
+        // Remove root `node_modules` entries the cleaned lockfile no longer
+        // places there, so a dependency dropped from `package.json` is also
+        // removed from disk by `bun install`. The kept set is the root
+        // package's declared dependency aliases (all behaviors, so
+        // `--production`/`--omit` never turn a reinstall into a delete)
+        // unioned with the root store entry's dependencies (which include
+        // `publicHoistPattern` hoists). A dependency that became transitive
+        // only is in neither set and is pruned, so the root symlink for it
+        // stops being importable. 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() && !lockfile_ro.packages.is_empty()
+        {
+            let root_entry_deps =
+                entry_dependencies[store::entry::Id::ROOT.get() as usize].slice();
+            let root_pkg_deps = pkgs.items_dependencies()[0];
+            let deps = lockfile_ro.buffers.dependencies.as_slice();
+            let mut expected = bun_collections::StringHashMap::<()>::with_capacity(
+                root_entry_deps.len() + root_pkg_deps.len as usize,
+            );
+            let mut keep = |dep_id: DependencyID| {
+                if let Some(dep) = deps.get(dep_id as usize) {
+                    let alias = dep.name.slice(string_buf);
+                    if !alias.is_empty() {
+                        let _ = expected.put(alias, ());
+                    }
+                }
+            };
+            for dep in root_entry_deps {
+                keep(dep.dep_id);
+            }
+            for dep_id in root_pkg_deps.begin()..root_pkg_deps.end() {
+                keep(dep_id);
+            }
+            if let Ok(fd) = sys::open_dir_for_iteration(Fd::cwd(), b"node_modules") {
+                crate::package_install::prune_extraneous_node_modules(
+                    &expected,
+                    fd,
+                    manager.options.bin_path.as_bytes(),
+                );
+                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({