From fafc0645500af5610dcc4c837b6985fb6ec2027b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 13:19:34 +0000 Subject: [PATCH 1/4] install: remove the node_modules links of a renamed or removed workspace Both linkers only create or repoint the node_modules/ links the current lockfile asks for. When a workspace package is renamed (or removed from the project), its old name leaves the lockfile and the install summary counts it as removed, but the link under the old name stayed behind in the root node_modules and, with the isolated linker, in the node_modules of every workspace that depended on it. After the lockfile is rebuilt and before the linker runs, unlink node_modules/ at the root and in every current workspace for each workspace name that was in the previous lockfile and is not in the new one. Only symlinks (junctions on Windows) are removed, and because this happens before linking, a name the new lockfile still places there is linked again. --- src/install/PackageManager.rs | 2 + .../PackageManager/install_with_manager.rs | 7 + .../remove_stale_workspace_links.rs | 93 +++++++++ test/cli/install/bun-workspaces.test.ts | 176 ++++++++++++++++++ 4 files changed, 278 insertions(+) create mode 100644 src/install/PackageManager/remove_stale_workspace_links.rs diff --git a/src/install/PackageManager.rs b/src/install/PackageManager.rs index 3aad03b5f197..cfb90a43ae97 100644 --- a/src/install/PackageManager.rs +++ b/src/install/PackageManager.rs @@ -73,6 +73,8 @@ pub mod populate_manifest_cache; pub mod process_dependency_list; #[path = "PackageManager/ProgressStrings.rs"] pub mod progress_strings; +#[path = "PackageManager/remove_stale_workspace_links.rs"] +pub(crate) mod remove_stale_workspace_links; #[path = "PackageManager/runTasks.rs"] pub mod run_tasks; #[path = "PackageManager/security_scanner.rs"] diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index 4cf780fb2d2c..d8923c58ccc9 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -34,6 +34,7 @@ use bun_install_types::NodeLinker::NodeLinker; // Free-function "methods" on `PackageManager` hosted in sibling modules // to avoid one giant `impl PackageManager` block. +use crate::package_manager_real::remove_stale_workspace_links::remove_stale_workspace_links; use crate::package_manager_real::run_tasks::{RunTasksCallbacks, run_tasks}; use crate::package_manager_real::{ UpdateRequest, enqueue_dependency_list, enqueue_dependency_with_main, enqueue_patch_task_pre, @@ -799,6 +800,12 @@ pub fn install_with_manager( break 'install_summary PackageInstallSummary::default(); } + // A renamed or removed workspace always shows up as a diff on the root + // package; without one the previous and current workspace sets match. + if had_any_diffs { + remove_stale_workspace_links(&lockfile_before_clean, &manager.lockfile); + } + let mut linker = manager.options.node_linker; loop { match linker { diff --git a/src/install/PackageManager/remove_stale_workspace_links.rs b/src/install/PackageManager/remove_stale_workspace_links.rs new file mode 100644 index 000000000000..441b22ff256a --- /dev/null +++ b/src/install/PackageManager/remove_stale_workspace_links.rs @@ -0,0 +1,93 @@ +//! A workspace package is linked into `node_modules/`: the root's, and +//! that of every workspace depending on it. Both linkers only create or +//! repoint the links the current lockfile asks for, so when a workspace is +//! renamed or removed from the project the link under its previous name +//! survives, still resolving to the workspace folder, even though the install +//! summary reports the package as removed. This runs after +//! `Lockfile::clean_with_logger` and before either linker: every workspace +//! name that was in the previous lockfile and is not in the new one has its +//! links unlinked, and anything the new lockfile still wants under that name +//! is recreated by the linker that follows. + +use bstr::BStr; +use bun_paths::AutoAbsPathChecked; + +use crate::lockfile::Lockfile; +use crate::lockfile::package::PackageColumns as _; +use crate::package_installer::alias_is_safe_install_target; +use crate::package_manager_real::PackageManager; +use crate::resolution::Tag as ResolutionTag; + +pub(crate) fn remove_stale_workspace_links(previous: &Lockfile, current: &Lockfile) { + let previous_string_buf = previous.buffers.string_bytes.as_slice(); + let previous_packages = previous.packages.slice(); + let resolutions = previous_packages.items_resolution(); + let name_hashes = previous_packages.items_name_hash(); + let names = previous_packages.items_name(); + + for pkg_id in 0..previous_packages.len() { + if resolutions[pkg_id].tag != ResolutionTag::Workspace + || current.workspace_paths.contains_key(&name_hashes[pkg_id]) + { + continue; + } + + let name = names[pkg_id].slice(previous_string_buf); + if !alias_is_safe_install_target(name) { + continue; + } + + remove_links_named(current, name); + } +} + +#[cold] +#[inline(never)] +fn remove_links_named(current: &Lockfile, name: &[u8]) { + let current_string_buf = current.buffers.string_bytes.as_slice(); + + let mut path = AutoAbsPathChecked::init_top_level_dir(); + let top_level_len = path.len(); + + remove_link(&mut path, b"", name); + for workspace_path in current.workspace_paths.values() { + path.set_length(top_level_len); + remove_link(&mut path, workspace_path.slice(current_string_buf), name); + } +} + +/// Unlinks `/node_modules/` if it is a symlink (or, on +/// Windows, a junction). A real directory or file at that path was not put +/// there as a workspace link and is left alone. +fn remove_link(path: &mut AutoAbsPathChecked, package_dir: &[u8], name: &[u8]) { + if path.append(package_dir).is_err() + || path.append(b"node_modules").is_err() + || path.append(name).is_err() + { + return; + } + + let mut link_target = bun_paths::path_buffer_pool::get(); + if bun_sys::readlink(path.slice_z(), &mut link_target).is_err() { + return; + } + + bun_output::scoped_log!( + PackageManager, + "removing stale workspace link {}", + BStr::new(path.slice()) + ); + + #[cfg(windows)] + { + // Directory symlinks and junctions are removed with rmdir; only a file + // symlink needs unlink. + if bun_sys::rmdir(path.slice_z()).is_err() { + let _ = bun_sys::unlink(path.slice_z()); + } + } + #[cfg(not(windows))] + { + let _ = bun_sys::unlink(path.slice_z()); + } +} diff --git a/test/cli/install/bun-workspaces.test.ts b/test/cli/install/bun-workspaces.test.ts index f6d72798485a..b24bf6921565 100644 --- a/test/cli/install/bun-workspaces.test.ts +++ b/test/cli/install/bun-workspaces.test.ts @@ -2324,3 +2324,179 @@ describe("packages whose version label is longer than 512 bytes", () => { ); }); }); + +// Both linkers only create or repoint the `node_modules/` links the current +// lockfile asks for. When a workspace is renamed or removed, its name leaves the +// lockfile and the links bun had created under that name (in the root's node_modules +// and, with the isolated linker, in the node_modules of the workspaces depending on +// it) used to stay behind, still resolving to the workspace folder. +describe("links of a renamed or removed workspace", () => { + type Linker = "hoisted" | "isolated"; + const linkers: Linker[] = ["hoisted", "isolated"]; + + async function install(ctx: TestCtx, linker: Linker, ...args: string[]): Promise { + await using proc = spawn({ + cmd: [bunExe(), "install", "--linker", linker, ...args], + cwd: ctx.packageDir, + stdout: "pipe", + stderr: "pipe", + env: ctx.env, + }); + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(err).not.toContain("error:"); + expect(exitCode).toBe(0); + return out; + } + + function writeProject( + packageDir: string, + rootDependencies: Record, + packages: Record>, + ) { + return Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ name: "foo", workspaces: ["packages/*"], devDependencies: rootDependencies }), + ), + ...Object.entries(packages).map(([dir, packageJson]) => + write(join(packageDir, "packages", dir, "package.json"), JSON.stringify({ version: "1.0.0", ...packageJson })), + ), + ]); + } + + // Package entries of a node_modules directory (no `.bun`/`.bin`), or [] when it does + // not exist. A listing (rather than `exists`) also sees links whose target is gone. + async function entries(nodeModules: string): Promise { + try { + return (await readdirSorted(nodeModules)).filter(name => !name.startsWith(".")); + } catch (err: any) { + if (err.code !== "ENOENT") throw err; + return []; + } + } + + for (const linker of linkers) { + test.concurrent(`${linker}: renaming a workspace removes the links under its old name`, async () => { + using ctx = await setupTest(); + const { packageDir } = ctx; + const rootNodeModules = join(packageDir, "node_modules"); + const bNodeModules = join(packageDir, "packages", "b", "node_modules"); + + await writeProject( + packageDir, + { a: "*", b: "*" }, + { a: { name: "a" }, b: { name: "b", dependencies: { a: "*" } } }, + ); + await install(ctx, linker); + expect(await entries(rootNodeModules)).toEqual(["a", "b"]); + expect(await entries(bNodeModules)).toEqual(linker === "isolated" ? ["a"] : []); + + await writeProject( + packageDir, + { "a-renamed": "*", b: "*" }, + { a: { name: "a-renamed" }, b: { name: "b", dependencies: { "a-renamed": "*" } } }, + ); + await install(ctx, linker); + expect(await entries(rootNodeModules)).toEqual(["a-renamed", "b"]); + expect(await entries(bNodeModules)).toEqual(linker === "isolated" ? ["a-renamed"] : []); + expect(await file(join(rootNodeModules, "a-renamed", "package.json")).json()).toMatchObject({ + name: "a-renamed", + }); + + expect(await install(ctx, linker)).toContain("(no changes)"); + expect(await entries(rootNodeModules)).toEqual(["a-renamed", "b"]); + expect(await entries(bNodeModules)).toEqual(linker === "isolated" ? ["a-renamed"] : []); + }); + + test.concurrent(`${linker}: renaming a scoped workspace removes the link under its old name`, async () => { + using ctx = await setupTest(); + const { packageDir } = ctx; + const scopeDir = join(packageDir, "node_modules", "@repo"); + + await writeProject( + packageDir, + { "@repo/a": "*", "@repo/b": "*" }, + { a: { name: "@repo/a" }, b: { name: "@repo/b" } }, + ); + await install(ctx, linker); + expect(await entries(scopeDir)).toEqual(["a", "b"]); + + await writeProject( + packageDir, + { "@repo/a-renamed": "*", "@repo/b": "*" }, + { a: { name: "@repo/a-renamed" }, b: { name: "@repo/b" } }, + ); + await install(ctx, linker); + expect(await entries(scopeDir)).toEqual(["a-renamed", "b"]); + expect(await file(join(scopeDir, "a-renamed", "package.json")).json()).toMatchObject({ name: "@repo/a-renamed" }); + }); + + test.concurrent(`${linker}: removing a workspace from the project removes its link`, async () => { + using ctx = await setupTest(); + const { packageDir } = ctx; + const rootNodeModules = join(packageDir, "node_modules"); + + await writeProject(packageDir, { a: "*", b: "*" }, { a: { name: "a" }, b: { name: "b" } }); + await install(ctx, linker); + expect(await entries(rootNodeModules)).toEqual(["a", "b"]); + + await rm(join(packageDir, "packages", "a"), { recursive: true }); + await writeProject(packageDir, { b: "*" }, { b: { name: "b" } }); + await install(ctx, linker); + expect(await entries(rootNodeModules)).toEqual(["b"]); + }); + + // The old name is unlinked before the linker runs, so a dependency that still + // wants that name (here an alias of the renamed workspace) is linked again. + test.concurrent(`${linker}: the old name is linked again when a dependency still uses it`, async () => { + using ctx = await setupTest(); + const { packageDir } = ctx; + const rootNodeModules = join(packageDir, "node_modules"); + + await writeProject(packageDir, { a: "*" }, { a: { name: "a" } }); + await install(ctx, linker); + expect(await entries(rootNodeModules)).toEqual(["a"]); + + await writeProject(packageDir, { a: "workspace:a-renamed@*" }, { a: { name: "a-renamed" } }); + await install(ctx, linker); + expect(await entries(rootNodeModules)).toEqual(linker === "isolated" ? ["a"] : ["a", "a-renamed"]); + expect(await file(join(rootNodeModules, "a", "package.json")).json()).toMatchObject({ name: "a-renamed" }); + }); + } + + // The isolated linker only links the workspaces the root depends on, so nothing is + // ever linked for `a` here and the directory under its name is not bun's to remove. + test.concurrent("a directory under the old name is left alone", async () => { + using ctx = await setupTest(); + const { packageDir } = ctx; + const rootNodeModules = join(packageDir, "node_modules"); + const marker = join(rootNodeModules, "a", "marker.txt"); + + await writeProject(packageDir, { b: "*" }, { a: { name: "a" }, b: { name: "b" } }); + await install(ctx, "isolated"); + expect(await entries(rootNodeModules)).toEqual(["b"]); + + await write(marker, "not a workspace link"); + await writeProject(packageDir, { b: "*" }, { a: { name: "a-renamed" }, b: { name: "b" } }); + await install(ctx, "isolated"); + expect(await entries(rootNodeModules)).toEqual(["a", "b"]); + expect(await file(marker).text()).toBe("not a workspace link"); + }); + + test.concurrent("--dry-run does not touch the links", async () => { + using ctx = await setupTest(); + const { packageDir } = ctx; + const rootNodeModules = join(packageDir, "node_modules"); + + await writeProject(packageDir, { a: "*", b: "*" }, { a: { name: "a" }, b: { name: "b" } }); + await install(ctx, "hoisted"); + expect(await entries(rootNodeModules)).toEqual(["a", "b"]); + + await writeProject(packageDir, { "a-renamed": "*", b: "*" }, { a: { name: "a-renamed" }, b: { name: "b" } }); + await install(ctx, "hoisted", "--dry-run"); + expect(await entries(rootNodeModules)).toEqual(["a", "b"]); + + await install(ctx, "hoisted"); + expect(await entries(rootNodeModules)).toEqual(["a-renamed", "b"]); + }); +}); From 644693cabf0122977d35ed6de9a138783ff5ea49 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:07:33 +0000 Subject: [PATCH 2/4] ci: retrigger From a02fc5dfd730a26f772719ace7f6d0dc43c49353 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:13:03 +0000 Subject: [PATCH 3/4] install: shorten the comments in remove_stale_workspace_links --- .../PackageManager/install_with_manager.rs | 4 ++-- .../remove_stale_workspace_links.rs | 23 +++++++------------ 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index d8923c58ccc9..e30a0f9b0503 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -800,8 +800,8 @@ pub fn install_with_manager( break 'install_summary PackageInstallSummary::default(); } - // A renamed or removed workspace always shows up as a diff on the root - // package; without one the previous and current workspace sets match. + // Every workspace is a dependency of the root package, so without a diff + // the workspace set is unchanged. if had_any_diffs { remove_stale_workspace_links(&lockfile_before_clean, &manager.lockfile); } diff --git a/src/install/PackageManager/remove_stale_workspace_links.rs b/src/install/PackageManager/remove_stale_workspace_links.rs index 441b22ff256a..e589485ce037 100644 --- a/src/install/PackageManager/remove_stale_workspace_links.rs +++ b/src/install/PackageManager/remove_stale_workspace_links.rs @@ -1,13 +1,8 @@ -//! A workspace package is linked into `node_modules/`: the root's, and -//! that of every workspace depending on it. Both linkers only create or -//! repoint the links the current lockfile asks for, so when a workspace is -//! renamed or removed from the project the link under its previous name -//! survives, still resolving to the workspace folder, even though the install -//! summary reports the package as removed. This runs after -//! `Lockfile::clean_with_logger` and before either linker: every workspace -//! name that was in the previous lockfile and is not in the new one has its -//! links unlinked, and anything the new lockfile still wants under that name -//! is recreated by the linker that follows. +//! Neither linker deletes anything from `node_modules`; each only creates or +//! repoints the links the current lockfile asks for. The links of a workspace +//! whose name left the lockfile (renamed, or removed from the project) are +//! unlinked here instead, before the linker runs, so a name the new lockfile +//! still places is simply linked again. use bstr::BStr; use bun_paths::AutoAbsPathChecked; @@ -56,9 +51,8 @@ fn remove_links_named(current: &Lockfile, name: &[u8]) { } } -/// Unlinks `/node_modules/` if it is a symlink (or, on -/// Windows, a junction). A real directory or file at that path was not put -/// there as a workspace link and is left alone. +/// Unlinks `/node_modules/`. `readlink` doubles as the +/// symlink/junction check: a real directory or file there is not ours to remove. fn remove_link(path: &mut AutoAbsPathChecked, package_dir: &[u8], name: &[u8]) { if path.append(package_dir).is_err() || path.append(b"node_modules").is_err() @@ -80,8 +74,7 @@ fn remove_link(path: &mut AutoAbsPathChecked, package_dir: &[u8], name: &[u8]) { #[cfg(windows)] { - // Directory symlinks and junctions are removed with rmdir; only a file - // symlink needs unlink. + // Directory symlinks and junctions need rmdir; a file symlink needs unlink. if bun_sys::rmdir(path.slice_z()).is_err() { let _ = bun_sys::unlink(path.slice_z()); } From 1d58928085cbf53b753fb8044979462abc0ae206 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:16:29 +0000 Subject: [PATCH 4/4] install: one-line comments in remove_stale_workspace_links --- src/install/PackageManager/install_with_manager.rs | 3 +-- .../PackageManager/remove_stale_workspace_links.rs | 10 ++-------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index e30a0f9b0503..51ec4cbd9037 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -800,8 +800,7 @@ pub fn install_with_manager( break 'install_summary PackageInstallSummary::default(); } - // Every workspace is a dependency of the root package, so without a diff - // the workspace set is unchanged. + // Every workspace is a root dependency, so without a diff the workspace set is unchanged. if had_any_diffs { remove_stale_workspace_links(&lockfile_before_clean, &manager.lockfile); } diff --git a/src/install/PackageManager/remove_stale_workspace_links.rs b/src/install/PackageManager/remove_stale_workspace_links.rs index e589485ce037..3540d66fc4f3 100644 --- a/src/install/PackageManager/remove_stale_workspace_links.rs +++ b/src/install/PackageManager/remove_stale_workspace_links.rs @@ -1,9 +1,3 @@ -//! Neither linker deletes anything from `node_modules`; each only creates or -//! repoints the links the current lockfile asks for. The links of a workspace -//! whose name left the lockfile (renamed, or removed from the project) are -//! unlinked here instead, before the linker runs, so a name the new lockfile -//! still places is simply linked again. - use bstr::BStr; use bun_paths::AutoAbsPathChecked; @@ -13,6 +7,7 @@ use crate::package_installer::alias_is_safe_install_target; use crate::package_manager_real::PackageManager; use crate::resolution::Tag as ResolutionTag; +/// Linkers only visit names in the new lockfile; a workspace name that left it is unlinked here. pub(crate) fn remove_stale_workspace_links(previous: &Lockfile, current: &Lockfile) { let previous_string_buf = previous.buffers.string_bytes.as_slice(); let previous_packages = previous.packages.slice(); @@ -51,8 +46,7 @@ fn remove_links_named(current: &Lockfile, name: &[u8]) { } } -/// Unlinks `/node_modules/`. `readlink` doubles as the -/// symlink/junction check: a real directory or file there is not ours to remove. +/// `readlink` is the symlink (or junction) check; a real directory or file there is left alone. fn remove_link(path: &mut AutoAbsPathChecked, package_dir: &[u8], name: &[u8]) { if path.append(package_dir).is_err() || path.append(b"node_modules").is_err()