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,194 @@
     }
 }
 
+/// 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 every lockfile
+/// alias matching `publicHoistPattern` for the isolated store).
+///
+/// `keep_symlinks` is set by the hoisted linker: its extraneous entries are
+/// always real extracted directories, while a root symlink there is a
+/// workspace link, a `file:` folder dependency, or a `bun link <pkg>`
+/// registration. The last of those is recorded in no manifest (`bun link` is
+/// `--no-save` by default), so it can never be in `expected` and must not be
+/// deleted. The isolated linker's own root entries are symlinks into
+/// `node_modules/.bun`, so it cannot skip them.
+///
+/// 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],
+    keep_symlinks: bool,
+) {
+    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;
+        }
+        if keep_symlinks && matches!(entry.kind, EntryKind::SymLink) {
+            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, keep_symlinks);
+            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 => {
+                // A symlink whose target no longer exists cannot be opened.
+                // `access` would not work here because it does not resolve
+                // symlinks. Only a missing target makes a link dangling;
+                // other open failures (EACCES, EMFILE, transient I/O) can
+                // happen to a perfectly good shim and must not delete it.
+                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(err)
+                        if err.get_errno() == sys::E::ENOENT
+                            || err.get_errno() == sys::E::ENOTDIR =>
+                    {
+                        let _ = sys::unlinkat(bin_dir, buf);
+                        continue 'iterator;
+                    }
+                    Err(_) => {}
+                }
+            }
+            _ => {}
+        }
+    }
+    let _ = sys::close(bin_dir);
+}
+
+/// Returns `true` if anything was deleted.
+fn prune_extraneous_scope(
+    parent: Fd,
+    scope: &[u8],
+    expected: &StringHashMap<()>,
+    keep_symlinks: bool,
+) -> 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'.' || (keep_symlinks && matches!(entry.kind, EntryKind::SymLink)) {
+            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,56 @@ pub(crate) fn install_hoisted_packages(
     // 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).
+    //
+    // `None` (no prune) for:
+    // - global installs: the global `node_modules` is also the `bun link`
+    //   registry, whose entries are bare symlinks recorded in no manifest
+    //   and must survive `bun add -g`.
+    // - the security scanner's narrowed pre-install pass; the full install
+    //   that follows performs the prune.
+    let expected_root_entries: Option<StringHashMap<()>> = 'expected: {
+        if this.options.global || 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 +265,24 @@ pub(crate) fn install_hoisted_packages(
         }
     };
 
+    // 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() {
+            // `keep_symlinks`: a root symlink here is a workspace link, a
+            // `file:` folder dependency, or an unsaved `bun link <pkg>`
+            // registration; the last is recorded in no manifest and must
+            // survive `bun install`.
+            package_install::prune_extraneous_node_modules(
+                expected,
+                node_modules_folder.fd(),
+                this.options.bin_path.as_bytes(),
+                true,
+            );
+        }
+    }
+
     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,61 @@ 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
+        // places there, so a dependency dropped from `package.json` is also
+        // removed from disk by `bun install`. The kept set is everything the
+        // isolated linker can legitimately place at the root, independent of
+        // install flags: the root package's declared dependency aliases (all
+        // behaviors, so `--production`/`--omit` never turn a reinstall into a
+        // delete) unioned with every lockfile alias that matches
+        // `publicHoistPattern` (a superset of the hoists any one install
+        // performs; the store's own root entry cannot be used here because it
+        // is narrowed by `--filter`). A dependency that became transitive
+        // only is in neither set and is pruned, so the root symlink for it
+        // stops being importable.
+        //
+        // Skipped for global installs (the global `node_modules` is also the
+        // `bun link` registry, whose entries are recorded in no manifest) and
+        // for the security scanner's narrowed pre-install pass; the full
+        // install that follows performs it.
+        if !is_new_bun_modules
+            && !manager.options.global
+            && packages_to_install.is_none()
+            && !lockfile_ro.packages.is_empty()
+        {
+            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_pkg_deps.len as usize);
+            for dep_id in root_pkg_deps.begin()..root_pkg_deps.end() {
+                let alias = deps[dep_id as usize].name.slice(string_buf);
+                if !alias.is_empty() {
+                    let _ = expected.put(alias, ());
+                }
+            }
+            if let Some(public_hoist_pattern) = &manager.options.public_hoist_pattern {
+                for dep in deps {
+                    let alias = dep.name.slice(string_buf);
+                    if !alias.is_empty() && public_hoist_pattern.is_match(alias) {
+                        let _ = expected.put(alias, ());
+                    }
+                }
+            }
+            if let Ok(fd) = sys::open_dir_for_iteration(Fd::cwd(), b"node_modules") {
+                // `keep_symlinks: false`: the isolated linker's own root
+                // entries are symlinks into `node_modules/.bun`, so they must
+                // be prunable.
+                crate::package_install::prune_extraneous_node_modules(
+                    &expected,
+                    fd,
+                    manager.options.bin_path.as_bytes(),
+                    false,
+                );
+                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());