Conversation
…bun remove -g` A global command links only the packages it names into the global bin dir. Every other top-level package links into the node_modules/.bin of the global dir. `bun remove` swept dangling links only in `options.bin_path`, which is the global bin dir in a global install, so the link in node_modules/.bin stayed behind. In a global install, also run `prune::prune_bins` on the node_modules of the global dir. It removes only the dangling entries of .bin, symlinks on POSIX and .bunx/.exe shim pairs on Windows.
|
Status: the fix and two tests are pushed. Waiting for CI. Reproduction (1.4.3-canary 26e7a4b, Linux x64 and Windows x64, with a private export BUN_INSTALL=$(mktemp -d)
bun add -g /path/to/what-bin # a folder package with the bin `what-bin`
bun add -g /path/to/other-bin # a second folder package with the bin `other-bin`
bun remove -g what-bin
ls -la $BUN_INSTALL/install/global/node_modules/.bin
# what-bin -> ../what-bin/cli.js (dangling, the folder is gone)
# other-bin -> ../other-bin/cli.jsWith this PR the listing holds only Test: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: oven-sh/bun/.coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. WalkthroughGlobal package removal now prunes dangling bins in the global ChangesGlobal removal cleanup
Priority: ⬇️ Low 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/install/PackageManager/updatePackageJSONAndInstall.rs— Windows users who runbun remove -g <pkg>still get a stale command on PATH after this merges; the PR title promises the dangling links are gone. The global bin dir sweep at updatePackageJSONAndInstall.rs:747 matches onlyEntryKind::SymLink, so the removed package's<name>.bunxand<name>.exepair stays inbin_path. The new block at :781 sweeps onlynode_modules/.bin, the internal dir. Fix: sweepbin_pathwith the same shim-aware logic as the Windowsprune_binsarm (read the.bunxtarget, resolve it relative to the parent of the bin dir, remove the pair when dangling) so both dirs are covered on every platform.Extended reasoning...
On Windows,
bun add -g what-binwriteswhat-bin.bunxandwhat-bin.exeinto$BUN_INSTALL/bin(bin.rs:1133-1190), the directory on PATH.bun remove -g what-bindeletesnode_modules/what-binat updatePackageJSONAndInstall.rs:737. Thebin_pathsweep at :741-767 iterates entries and only acts onEntryKind::SymLink(:747);.bunx/.exeare regular files and hit the_ => {}arm at :764. The new global block at :781-788 callsprune_binsonnode_modules, whose.binis not on PATH. So the user typeswhat-binand the shim fails on a missing..\install\global\node_modules\what-bin\cli.js. Population: every Windowsbun remove -guser, once per removal. The PR ships a Windows shim sweep (prune.rs:2018-2051) that reads each.bunxtarget withis_dangling(dir, target); applying it withdir= parent ofbin_pathcloses the same class in the same PR. The dismissal relied on the PR description pointing to #35585, which is a claim, not a merged fix.Verification: pre-existing. Trigger:
bun remove -g <pkg>on Windows, for a package whose shims were written into the global bin dir ($BUN_INSTALL/bin). Mechanism verified: thebin_pathsweep at /home/claude/bun/src/install/PackageManager/updatePackageJSONAndInstall.rs:741-779 only acts onbun_sys::EntryKind::SymLink(line 747) and falls into_ => {}(line 764) for everything else; on Windows the…
|
Updated 1:03 AM PT - Sep 19th, 2026
✅ @robobun, your commit efaa1e90f1f7b24226e0b6dea62fb29c10c030b8 passed in 🧪 To try this PR locally: bunx bun-pr 43403That installs a local version of the PR into your bun-43403 --bun |
An inherited BUN_INSTALL_GLOBAL_DIR or BUN_INSTALL_BIN, or a globalDir in the home bunfig, takes precedence over BUN_INSTALL. Set all three so the test cannot reach the real global folder of the machine that runs it.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
The test already asserts that a `bun link` registration survives `bun remove -g`, so the comment does not need to carry that rule.
|
On the additional finding about the Windows shims in the global bin dir: that is #11970, and #35585 is the open fix for it. It extends the The title and the body limit this PR to the |
There was a problem hiding this comment.
LGTM — the env-pinning commit resolves the earlier concern, and the remaining change is a small, best-effort reuse of the existing prune_bins sweep.
What was reviewed:
remove_leftover_node_modulestail block:cwdis the global dir (global modefchdirs into it inPackageManager.rs:1502), andprune_binsonly unlinks.binentries whose target failsfstatatwith ENOENT/ENOTDIR (prune.rs:1953), sobun linkregistrations and live bins are untouched.- Test isolation:
BUN_INSTALL_GLOBAL_DIRandBUN_INSTALL_BINare read before the bunfiginstall.globalDirandBUN_INSTALL(PackageManagerOptions.rs:327-341), so pinning them keeps the test off a developer's real global dir. - Swallowed
open_aterror: only skips an auxiliary cleanup whennode_modulesis absent/unopenable; no new exit path, matching the "no ENOTDIR crash" property the PR describes.
Extended reasoning...
Overview
The PR adds a 7-line block to remove_leftover_node_modules in /home/claude/bun/src/install/PackageManager/updatePackageJSONAndInstall.rs that, for global installs, opens node_modules under the current directory and calls crate::prune::prune_bins on it. prune_bins in /home/claude/bun/src/install/prune.rs changes visibility from private to pub(crate) on both the POSIX and Windows variants with no body edits. The test file /home/claude/bun/test/cli/install/bun-remove.test.ts gains a runWithEnv helper, a binFiles helper for Windows shim pairs, and one new concurrent test per linker that exercises add/add/link/remove against a temp global dir.
Security risks
None material. The new code deletes only entries in node_modules/.bin that are already dangling (POSIX: symlinks whose fstatat fails with ENOENT/ENOTDIR; Windows: .bunx/.exe pairs whose embedded relative target no longer resolves). open_real_subdir uses O_NOFOLLOW so a .bin symlink is not traversed. No user-controlled string becomes a path beyond the directory-entry names already present on disk, which is the same surface bun pm prune has had since #38333.
Level of scrutiny
Moderate. The behavioral change is small and reuses an existing, already-tested helper rather than adding new sweep logic. I confirmed the two facts the fix rests on: global mode fchdirs into the global dir before this runs (PackageManager.rs:1502), so Dir::cwd() is the global project; and prune_bins only removes dangling entries, so bun link registrations and the other package's live bin survive. The prior-run finding (test could touch a developer's real global dir when BUN_INSTALL_GLOBAL_DIR/BUN_INSTALL_BIN were inherited) was addressed in commit 9b02db6: open_global_dir checks BUN_INSTALL_GLOBAL_DIR first and open_global_bin_dir checks BUN_INSTALL_BIN first, both ahead of the bunfig and BUN_INSTALL fallbacks, so pinning them is sufficient.
Other factors
The test follows the file's existing conventions (it.concurrent, tempDir, expect(stderr).not.toContain("error:"), exit code asserted last) and branches on isWindows for the shim layout rather than skipping. A debug build was not available in this environment, so I did not execute the test locally; the PR states it fails on the base commit and passes with the fix on Linux and Windows, and CI covers both. The silent if let Ok around open_at is acceptable for a best-effort auxiliary cleanup and avoids the ENOTDIR exit-1 path the PR notes was present in an earlier shape. No CODEOWNERS entry covers the changed files. The bug-hunt exit reason was dry_streak with no findings.
This branch has not been deployed
1 participant
Problem
bun remove -g <pkg>leaves a dangling bin link in thenode_modules/.binof the global dir. Repro:bun add -g a,bun add -g b,bun remove -g a.node_modules/.bin/athen points at the deleted../a/cli.js(on Windows, thea.bunx/a.exeshims stay).remove_leftover_node_modules(src/install/PackageManager/updatePackageJSONAndInstall.rs:741). It sweeps dangling links only inoptions.bin_path. In a global install that is the global bin dir.Fix
bun removealso callsprune::prune_bins(from install: pnpm parity — dedupe, prune, pm licenses, audit fix, add --filter/--catalog, nested overrides, transitive update, and workspace fixes #38333, nowpub(crate)) on the globalnode_modules. Nothing else changes.prune_binsremoves only the dangling entries of.bin: symlinks on POSIX,.bunx/.exepairs on Windows. It never exits, sobun remove -ggets no new failure path.bun linkregistrations stay.test/cli/install/bun-remove.test.ts(hoisted, isolated) fail on 1.4.3-canary 26e7a4b and pass with the fix, on Linux x64 and Windows x64. Self-reviewed: 9 concerns raised, 8 addressed (see Notes).Background
$BUN_INSTALL/install/global) is a normal project with apackage.jsonand anode_modules. It is also thebun linkregistry. The global bin dir ($BUN_INSTALL/bin) is on$PATH.link_tree_bins,src/install/PackageInstaller.rs:646). Every other top-level package links into the globalnode_modules/.bin. So the secondbun add -glinks the first package there.bun removedeletes the folder of the removed package. Then it unlinks each dangling symlink inoptions.bin_path.Notes
bun remove#35585), and this PR does not touch that sweep.bin_pathsweep into a helper and ran it on both directories. The self-review found a new failure in that shape. If thenode_modules/.binof the global dir is not a directory,bun remove -gremoved the package and then exited 1 withENOTDIR: while reading node_modules/.bin.prune_binsopens.binonly when it is a real directory, so it has no such path. Checked by hand: with.binas a regular file,bun remove -gexits 0, the same as main.node_modules/.bintoday, so eachbun remove -gleft a dangling link there. install(isolated): link bins of -g update requests into $BUN_INSTALL/bin #30451 changes where the named packages link. The test assertions hold before and after it.bun add -g parent(depends onmytool),bun add -g mytool,bun remove -g parent,bun remove -g mytool. Before:.bin/mytooldangles. After: no dangling link.remove_leftover_node_modules(itsif global { bin_path } else { .bin }keeps this gap, and the new tests catch it), install(windows): remove .bunx/.exe shims onbun remove#35585 adds the Windows shim sweep forbin_path, Resolve readdir entries of unknown type in the remaining consumers (create, bin links, bunx, fs.watch, cp, ls, init) #38961 resolvesDT_UNKNOWNentries there. This PR adds one block after that code and leaves the code itself as on main.bun remove -gdoes not remove binaries #11970 (Windows shims stay in the global bin dir, see install(windows): remove .bunx/.exe shims onbun remove#35585) and bun install -g leaves an orphaned bin shim when an update drops a bin entry #36388 (a stale bin after an update drops a bin entry).bun update -g#43384 (barebun update -g) and install: only replace or remove a package's own bins in the global bin dir #39745 (bin ownership in the global bin dir).BUN_INSTALL,BUN_INSTALL_GLOBAL_DIRandBUN_INSTALL_BIN, likeglobalEnvinbun-prune.test.ts. An inherited variable or aglobalDirin the home bunfig cannot point them at a real global folder. Checked by hand for both cases.bun linkbefore the remove and assert that the registration survives. This pins the rule that only.binof the globalnode_modulesis swept.removetests oftest/cli/install/bun-add-filter.test.ts, and theglobaltests oftest/cli/install/bun-update.test.tsandtest/cli/install/bun-prune.test.ts.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/bun-remove.test.ts