install: remove node_modules entries that left the lockfile - #32974
install: remove node_modules entries that left the lockfile#32974robobun wants to merge 13 commits into
Conversation
A dependency removed from package.json and reinstalled was pruned from bun.lock but never from node_modules, so the package stayed importable on every machine that had installed it (and broke on fresh clones). Both the hoisted and the isolated linker only added or updated entries. Add a shared prune that runs after the lockfile is cleaned: any top level node_modules entry (including one level into @scope/) whose folder name is not a dependency alias in the lockfile is removed. Keying the kept set to the lockfile graph rather than to what a single invocation places at the root means --production, --omit, --filter, and --cpu/--os overrides never turn a reinstall into a delete of packages the lockfile still has. Dot-prefixed entries (.bin, .bun, .cache) are never touched. Also bind the test Verdaccio registry to 127.0.0.1 explicitly; with only a port it listens on whatever getaddrinfo(localhost) returns first, and on hosts where that is ::1 every registry-backed test fails with ConnectionRefused.
The 'add trusted, delete, then add again' test's own comment says to update it once bun install starts deleting removed dependencies from node_modules. The manual-edit (withRm: false) branch now converges onto the bun rm (withRm: true) branch: uses-what-bin is gone from disk on both paths, and re-adding it installs both it and its hoisted transitive dependency what-bin.
WalkthroughAdds pruning helpers for stale ChangesExtraneous node_modules pruning
Verdaccio IPv4 loopback binding
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 4:49 AM PT - Jun 28th, 2026
❌ @robobun, your commit c4a6dc4 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 32974That installs a local version of the PR into your bun-32974 --bun |
|
Found 4 issues this PR may fix:
🤖 Generated with Claude Code |
|
Went through all four. One is fixed by this PR, three are not; the three that are not each live in a location this prune deliberately does not walk, so I have not added them to the #8662 - fixed. #26305 - not fixed. The stale copy there is #21216 - not fixed. The leftover old versions live in #22702 - not fixed. The spurious entries described there are nested, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/PackageInstall.rs`:
- Around line 2592-2600: The keep-set in the installation cleanup path is being
built from every lockfile dependency instead of only the aliases actually linked
at the project root, which leaves stale root-level entries behind after direct
dependencies are removed. Update the helper around the expected set construction
and the cleanup logic to use the root-visible alias set for the active linker,
or thread that root-linked alias set into this function, so the removal check
only preserves entries that are still linked at the root. Keep the fix localized
to the code that builds expected and the subsequent node_modules cleanup in the
install flow.
- Around line 2609-2632: The prune logic is swallowing filesystem errors, so
update the cleanup path around iterate_dir, prune_extraneous_scope, and
dir.delete_tree to return the first encountered error instead of continuing
silently. Thread that error back through the caller in PackageInstall.rs so bun
install fails when pruning cannot complete, and make sure any recursive
scope/package deletion also propagates failures rather than discarding them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 38ce182e-1142-488e-abf9-4577de64578f
📒 Files selected for processing (6)
src/install/PackageInstall.rssrc/install/hoisted_install.rssrc/install/isolated_install.rstest/cli/install/bun-install-lifecycle-scripts.test.tstest/cli/install/bun-install-registry.test.tstest/harness.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…n links Two issues from review. The keep-set was every dependency alias in the lockfile, which over-keeps at the root: a package demoted from a direct dependency to transitive-only (something else still depends on it) stayed as a root node_modules symlink under the isolated linker, exactly the phantom dependency that linker is meant to prevent. Build a per-linker set instead, each 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 the linker's own root placements, the unfiltered root tree for hoisted (transitives legitimately live there) and the root store entry's dependencies for isolated (which include publicHoistPattern hoists). A lockfile with no dependencies has zero trees; that contributes an empty tree set rather than skipping the prune. Pruning a package also left its node_modules/.bin symlink dangling, so scripts got ENOENT on the link target instead of command not found. Sweep dangling .bin symlinks after the prune, reusing the loop bun remove already ran; it is extracted into prune_dangling_bin_links and the bun remove path now calls the shared helper.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/PackageInstall.rs`:
- Around line 2653-2659: The cleanup in the `File::openat` error branch is too
broad and currently unlinks `.bin` entries for any failure, not just
missing-target cases. Update the `match sys::File::openat(...)` handling in the
bin-shim iterator so `sys::unlinkat` is only called for missing-target errors
like `ENOENT` or `ENOTDIR`, and leave other errors untouched so valid shims are
preserved.
In `@test/cli/install/bun-install-registry.test.ts`:
- Around line 9366-9373: The binEntries helper is swallowing every readdir
failure and returning an empty array, which can mask real filesystem or
permission issues. Update the catch in binEntries to only convert ENOENT from
readdirSorted(join(packageDir, "node_modules", ".bin")) into [] and rethrow any
other error so the final assertion in this test still fails on unexpected .bin
read problems.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 97084a23-e3d2-4a91-afb4-6c7c5fbac95b
📒 Files selected for processing (5)
src/install/PackageInstall.rssrc/install/PackageManager/updatePackageJSONAndInstall.rssrc/install/hoisted_install.rssrc/install/isolated_install.rstest/cli/install/bun-install-registry.test.ts
The dangling .bin sweep removed an entry on any open failure, so an EACCES, EMFILE, or transient I/O error on a perfectly good shim would delete it. Only ENOENT and ENOTDIR mean the target is gone; everything else leaves the entry alone. This also applies to the bun remove path, which shares the helper. Tighten the test's .bin readdir helper the same way so only a missing directory reads as empty.
… hoists Three holes from review, each reproduced by a new regression test that fails on the previous commit. Global installs chdir into the global install dir, so the prune ran on <global>/node_modules. That directory is also the bun link registry: bun link records a package there as a bare symlink with no entry in the global package.json or lockfile, so bun add -g deleted every link registration and then its now dangling global bin shim. The global directory is a tool registry, not a project; never prune it. bun link <pkg> in a consumer is --no-save by default, so the local node_modules/<pkg> symlink is recorded in no manifest either and the next bun install deleted it. The hoisted prune now never deletes a symlink: hoisted extraneous entries are always real extracted directories, and a root symlink there is a workspace link, a file: folder dependency, or a bun link, none of which it should touch. The isolated linker keeps pruning symlinks because they are its own root entries. The isolated keep-set used the root store entry's dependencies, which are narrowed by --filter, so a publicHoistPattern hoist originating in a filtered-out workspace was deleted by a filtered install. The set is now the root package's declared aliases unioned with every lockfile alias matching publicHoistPattern, both independent of install flags.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/isolated_install.rs`:
- Around line 1998-2011: In the isolated install flow, the `expected.put(...)`
calls are currently ignoring `AllocError`, which can leave the keep-set
incomplete before `prune_extraneous_node_modules()` runs. Update this section to
propagate failures from `StringHashMap::put` instead of discarding them, and
thread the `Result` through the surrounding install routine so the prune step
only proceeds when both inserts succeed. Use the existing `expected`
map-building logic and the caller path around `prune_extraneous_node_modules()`
to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e77817b-113f-40e6-882c-1081368ac306
📒 Files selected for processing (4)
src/install/PackageInstall.rssrc/install/hoisted_install.rssrc/install/isolated_install.rstest/cli/install/bun-install-registry.test.ts
…ilures bun patch materializes the package being patched at the root node_modules/<name> even when no tree places it there (under the isolated linker it is a symlink into the store), so the prune deleted it and bun patch --commit lost the folder it had just told the user to edit. Both keep-sets now include the name of every package with a patchedDependencies entry, matched the same way the installer matches patches. Caught by the existing bun patch workspace test. readdir does not fill d_type on some filesystems (NFS, FUSE, bind mounts), so a bun link symlink could be reported as Unknown and deleted despite keep_symlinks. Resolve Unknown entries with an lstat, mirroring walker_skippable. Insertion failures while building a keep-set now propagate instead of being ignored; an incomplete set must never reach the prune.
Same d_type gap as the prune's symlink guard: on filesystems that do not populate d_type, every .bin entry reported Unknown and the sweep was a no-op. Reuse entry_is_symlink, which falls back to lstat only for Unknown entries. This also covers the bun remove path that shares the helper.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/install/PackageInstall.rs (1)
2628-2629: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDon’t descend into top-level
@scopesymlinks.A top-level
node_modules/@scopesymlink is routed intoprune_extraneous_scope(), which opens it as a directory before deleting child entries. In the isolated flow (keep_symlinks = false), that can follow a symlink outsidenode_modulesand prune the target directory’s contents. Snapshot the symlink bit and unlink/skip the top-level symlink itself instead of passing it to the scope walker. As per coding guidelines, “lexical containment is defeated by symlinks — re-verify after realpath, prefer O_NOFOLLOW-style atomic flags over check-then-act.”Also applies to: 2756-2764
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/install/PackageInstall.rs` around lines 2628 - 2629, The top-level `@scope` handling in PackageInstall::prune_extraneous and prune_extraneous_scope is following symlinks into a scope directory before deletion, which can escape node_modules; detect when the entry is a symlink and do not descend into it. Snapshot the symlink state for the top-level `@scope` entry, then unlink or skip that symlink directly instead of passing it into prune_extraneous_scope, and apply the same fix in the later `@scope` branch that uses the same walker.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/hoisted_install.rs`:
- Around line 151-170: The new keep-set build in the hoisted install flow can
exit early before the restore guard is armed, which may leave the lockfile
buffers and hoisted dependencies filtered instead of restored. In the
`hoisted_install` logic, wrap the `keep(dep_id)` loop and
`extend_expected_with_patched_packages` call in a temporary closure or result
block, then create `_restore_buffers` before applying the fallible result with
`?`. Keep the fix centered on the `keep` helper and
`package_install::extend_expected_with_patched_packages` path so the RAII guard
always exists before any fallible operation.
In `@src/install/PackageInstall.rs`:
- Around line 2667-2678: The resolution handling in PackageInstall::prune is
dropping entries when resolution.fmt(...) exceeds the fixed 512-byte buffer,
which can cause patched root packages to be omitted from expected. Update the
loop in the package resolution formatting path to use the growable
name_and_version buffer directly for formatting name@version instead of relying
on resolution_buf, so long patched-package resolutions are always preserved and
included in expected.
---
Outside diff comments:
In `@src/install/PackageInstall.rs`:
- Around line 2628-2629: The top-level `@scope` handling in
PackageInstall::prune_extraneous and prune_extraneous_scope is following
symlinks into a scope directory before deletion, which can escape node_modules;
detect when the entry is a symlink and do not descend into it. Snapshot the
symlink state for the top-level `@scope` entry, then unlink or skip that symlink
directly instead of passing it into prune_extraneous_scope, and apply the same
fix in the later `@scope` branch that uses the same walker.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a106d841-6c7f-43fd-b47b-883e8d52424d
📒 Files selected for processing (3)
src/install/PackageInstall.rssrc/install/hoisted_install.rssrc/install/isolated_install.rs
… the fixed version buffer A top-level node_modules/@scope entry that is a symlink was passed to the scope walker, which opens through the link, so an isolated install could delete the contents of a directory outside node_modules. Symlinked scope entries are now never descended into; deleting a symlink itself is safe because delete_tree unlinks it without following. The keep-set build is fallible and ran before the scopeguard that restores the pre-filter tree buffers was armed, so an early return would have left the filtered buffers in place. It now runs after the guard and reads the snapshot through it. Formatting a patched package's resolution through a fixed 512 byte buffer silently dropped entries with longer resolutions (git or tarball URLs) from the keep-set; format straight into the growable buffer.
There was a problem hiding this comment.
All earlier findings look addressed and I didn't spot anything new, but this introduces destructive node_modules pruning on every bun install with a fair amount of edge-case surface (global/link registry, symlinks, --filter, publicHoistPattern, patched packages) — worth a human sign-off on the keep-set semantics before it ships.
Extended reasoning...
Overview
Adds prune_extraneous_node_modules and supporting helpers (extend_expected_with_patched_packages, entry_is_symlink, prune_dangling_bin_links, prune_extraneous_scope) in src/install/PackageInstall.rs (~250 new lines), wires them into both hoisted_install.rs and isolated_install.rs, replaces the inline .bin sweep in updatePackageJSONAndInstall.rs with the shared helper, updates lifecycle-script test expectations, adds ~400 lines of new registry tests across both linkers, and binds Verdaccio to 127.0.0.1 in the test harness.
Security risks
The new code performs delete_tree on entries inside the project's root node_modules based on a computed keep-set. The blast radius is bounded to that directory (dot-prefixed entries are skipped, @scope symlinks are never followed, global installs are gated out, hoisted symlinks are preserved), and there is no path-traversal vector since names come from readdir and are passed to *at syscalls relative to the open dir fd. No auth/crypto/permissions code is touched. The real risk is correctness rather than security: an incomplete keep-set silently deletes packages the lockfile still places at the root.
Level of scrutiny
High. This is a behavioral change to bun install itself — every install on an existing tree now walks node_modules and may delete_tree entries. The PR went through several review rounds where each uncovered a case that would have destroyed user state (global bun link registry, unsaved consumer bun link, --filter-narrowed publicHoistPattern hoists, DT_UNKNOWN filesystems, long patched-dep resolutions). All of those are now addressed and pinned by tests, but the number of iterations is itself a signal that the keep-set semantics are subtle enough to merit a human reviewer confirming the final design — particularly the linker-specific union rules and the best-effort-on-delete-failure decision.
Other factors
Test coverage is thorough (12 parameterized tests × 2 linkers plus 2 standalone link/global tests, plus the updated lifecycle test). All inline review threads from coderabbit and from me are resolved. The harness change (Verdaccio 127.0.0.1 bind) is a low-risk test-infra fix. CI build #66161 is in progress on the latest commit. Given the destructive nature and the volume of edge-case handling that accumulated during review, I'm deferring rather than approving.
There was a problem hiding this comment.
All earlier findings look addressed and I didn't spot anything new, but this introduces a destructive prune on every bun install's hot path with a lot of edge-case guards (global, bun link, --filter, patched deps, symlinks, DT_UNKNOWN), so it's worth a maintainer's eyes before merging.
Extended reasoning...
Overview
This PR adds prune_extraneous_node_modules to src/install/PackageInstall.rs and wires it into both hoisted_install.rs and isolated_install.rs, so that bun install deletes top-level node_modules entries that are no longer in the lockfile. It also extracts the .bin dangling-symlink sweep into a shared prune_dangling_bin_links helper (now called by both bun remove and the new prune), adds an extend_expected_with_patched_packages helper, and ships ~400 lines of new test coverage in bun-install-registry.test.ts plus updates to the lifecycle-scripts test. A small unrelated fix binds Verdaccio to 127.0.0.1 in the test harness.
Security risks
No injection/auth/crypto surface. The risk is correctness rather than security: a destructive delete_tree now runs on every non-fresh bun install against the user's node_modules. The failure mode of an under-populated keep-set is silently deleting legitimate user state (workspace symlinks, bun link registrations, patched packages, publicHoistPattern hoists). The PR went through eight+ review rounds, each of which found a real case where the original implementation would have deleted something it shouldn't (global link registry, unsaved consumer-side links, --filter-narrowed hoists, DT_UNKNOWN filesystems, long patched-resolution strings, RAII-guard ordering). All have been addressed and pinned by tests, but the volume of edge cases is itself a signal that this surface deserves maintainer sign-off.
Level of scrutiny
High. This is not a mechanical change — it changes bun install from purely additive to actively pruning, on the most-executed code path in the package manager, across both linkers. There are design decisions a maintainer should ratify: pruning on every install vs. an explicit bun prune, the hoisted linker's "never delete symlinks" heuristic, the acknowledged remaining gap for unsaved bun link under the isolated linker (#30289), and best-effort vs. hard-fail on deletion errors.
Other factors
The bug-hunting system found nothing on the current revision; all my prior inline findings and all CodeRabbit findings are resolved. Test coverage is thorough (12 new parametrized tests across both linkers plus two standalone link-survival tests), and the PR description is unusually detailed. CI build #66190 is in progress. Given the blast radius and the number of correctness fixes already applied during review, I'm deferring rather than approving.
|
CI status for the reviewers: the diff's own lanes are green and the remaining red is unrelated to this change. Build #66190 (current head) finished with 282 passing jobs, including every install test lane;
None of these involve |
…om bun.lock (#35681) ### Repro ```sh bun add -D optional-peer-deps # has an optional peer on no-deps; no-deps is NOT in bun.lock bun add no-deps # no-deps now in bun.lock as a direct dep bun remove no-deps ``` After the remove, `bun.lock` still contains a `packages` entry for `no-deps`: ```json "packages": { "no-deps": ["no-deps@1.0.0", "...", {}, "sha512-..."], "optional-peer-deps": ["optional-peer-deps@1.0.0", "...", { "peerDependencies": { "no-deps": "*" }, "optionalPeers": ["no-deps"] }, "sha512-..."] } ``` Same thing happens when `no-deps` is removed from `package.json` by hand and `bun install` is re-run. npm and yarn 1.x both return the lockfile to its pre-`add` state. Reported as the second repro on #8662 (#8662 (comment)); distinct from the `node_modules` pruning part of that issue, which is #32974. ### Cause `Lockfile::clean_with_logger` rebuilds the package list by a reachability walk from the root (`Package::clone` over each resolution slot). Once `hoist` has filled an optional-peer slot with a valid `PackageID`, `Package::clone` treats it exactly like a hard dependency edge: it is enqueued and the target survives the clean. During fresh resolve the slot is never populated independently (`enqueue_dependency_with_main_and_success_fn` returns early for optional peers) and is only filled by `hoist` when the target is already in the tree via some other edge; the clean step did not honour that invariant. ### Fix In `Package::clone`, write `invalid_package_id` to an optional-peer resolution slot instead of enqueuing the old target. `Cloner::flush` finishes with `resolve()`/`hoist()`, which already walks the freshly cloned tree and fills an optional-peer slot only when the target package was kept alive by some non-optional-peer edge, so a still-referenced peer target is re-bound and an unreferenced one is dropped together with its transitives. Because the slot is now always re-derived from the post-clean tree instead of carried over, a lockfile that contains a history-dependent nested optional-peer placement (the old carried value diverged from what a fresh install would place) may be rewritten once on the first `bun install` after upgrading. The rewritten value is the fresh-install value, so this is a one-time convergence rather than churn. ### Verification Four tests in `test/cli/install/bun-lock.test.ts`: - `bun remove` of a package that also satisfied an optional peer returns `bun.lock` to its pre-`add` state and round-trips `--frozen-lockfile` - `bun install` after editing the same removal into `package.json` does the same - control: when a hard dependency elsewhere still pulls the package in, `bun.lock` is byte-identical before and after the add/remove pair - idempotence guard: a non-wildcard optional peer (`no-deps@^1.0.0`) with two versions of `no-deps` in the tree (1.0.1 and 2.0.0) produces a byte-identical lockfile across two consecutive installs and passes `--frozen-lockfile` The first two fail on the released build with the `no-deps` entry quoted above and pass with this change; the control and idempotence tests pass on both. Also ran the peer/optional filters of `bun-install-registry.test.ts`, `isolated-install.test.ts`, `hoist.test.ts`, `bun-remove.test.ts` and `bun-lockb.test.ts` with no new failures. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-lock.test.ts <!-- robobun:evidence:end -->
…ilter/--catalog, nested overrides, transitive update, and workspace fixes (#38333) Brings Bun's package manager to parity with pnpm for monorepo workflows, and fixes the bugs found while checking every command against pnpm's implementation, pnpm's test suites, pnpm's open issue tracker, pnpm's docs, npm's arborist fixtures, and — for `bun update` — running real pnpm and Bun side by side on the same projects. ### What does this PR do? #### New commands - **`bun dedupe [--check]`** — collapses duplicate versions in `bun.lock` onto the smallest set that still satisfies every dependent's range, using only versions already in the lockfile, then installs. Never downgrades a direct dependency unless that is the only way to drop a version; keeps patched versions (and anything needed to reach them, and says so); refuses to run on a lockfile that is behind `package.json`. `--check` exits 1 without writing. - **`bun prune [--production | --omit=…] [--dry-run] [--filter <ws>]`** — removes everything in `node_modules` that the lockfile does not put there; `--production` leaves exactly what `bun install --production` would. Hoisted and isolated layouts, Windows junctions and shims, workspace links, bundled deps; refuses when `package.json` and `bun.lock` disagree or when `node_modules` was laid out by a different linker; understands turbo-pruned checkouts. - **`bun pm licenses [--json] [--prod|--dev] [--long] [--filter <ws>]`** — installed packages grouped by license, with a `(dev)` marker and `paths`/`license`/`description` in `--json`. - **`bun audit fix [--latest] [--dry-run] [--json]`** — moves each vulnerable package to the lowest safe version its dependents accept, per installed instance; rewrites exact pins when that is the only way; `--latest` also rewrites your own declared ranges (root, workspace, catalog) so a semver-major fix can be taken, and every blocked or unfixable item is followed by the command that resolves it (`bun audit fix --latest`, `bun audit --ignore GHSA-…`); re-audits the tree it actually installed and reports/exits from that second response (npm's `_submitQuickAudit`), so an advisory that starts at the version it moved to is not missed; works across registries; security fixes bypass `minimumReleaseAge` with an annotation. `bun audit --json` honors `--audit-level`/`--ignore` for its exit code; `--omit` is honored by `audit` and `licenses`. #### `bun update` semantics (pnpm's model) - A bare `bun update` re-resolves **transitive** packages too — every edge moves to the newest version its own range (or dist-tag) allows, per dependent, so `bun.lock` no longer stays stale after an update; overrides/catalogs changed since the last install are honored. From a workspace member or `--filter`, only what the selected workspaces reach is re-resolved; from the root, everything. - `bun update <name>` reaches any depth, matches `npm:` aliases by real name, updates in place, never adds to `package.json`, and errors on a name nothing selected depends on. `-r`/`--filter` fan a named update out across workspaces. `--latest` never downgrades a locked version that is ahead of the tag, and `update <name> --latest` also refreshes that package's own dependencies. - Plain updates keep dist-tag literals and non-caret ranges (`*`, `1.x`, `^1 || ^2`) exactly as written and only move the lockfile; `--latest` rewrites them to the resolved version as before. `bun update -i` applies only what you selected. New: positional patterns (`bun update '@types/*'`), `--dev`/`--prod`/`--no-optional`, `-L`, `bun up`. - `package.json` is written after resolution and `bun.lock`'s declared ranges, overrides and catalogs are re-derived from the final `package.json`, replacing the per-command literal rewriting; a no-op update leaves the file byte-identical. #### Overrides - **Nested overrides** (#6608): npm's nested objects, yarn's `a/b` paths and pnpm's `a>b` selectors, applied to the direct parent→child edge; **version-scoped targets** (`"lodash@<4.17.21": "4.17.21"`, the shape `pnpm audit --fix` writes), matched against the dependent's declared range as pnpm does. Rules persist inside the `overrides` section and the file is stamped `lockfileVersion: 3` **only when such rules exist** — existing lockfiles are byte-identical. Flat overrides additionally fix `$ref` to workspace-member deps, catalog-valued rules going stale, and warn on pnpm's `-` / `pkg@` forms. #### Workspaces and filters - `bun add|remove|update … --filter <ws>` (also `-F`, also `bun install <pkg> --filter`) edits the selected workspaces' `package.json` files and **links only those workspaces**, like `bun install --filter`. Filters gain pnpm's relation selectors (`foo...`, `...foo`, `foo^...`, `...^foo`) and `{dir}` subtrees, for the install family **and** `bun run --filter`; `--filter` may precede the subcommand; every command warns about patterns that match nothing; `add`/`remove` no longer select the root implicitly. - `bun add <pkg> --catalog[=name]` reuses an existing catalog entry, keeps a range an explicit version fits, catalogs the range a package already declares, decides per target, and refuses workspace names and local paths; a plain `bun add` uses a default-catalog entry when one exists. A package defined in both `catalog` and `catalogs.default` is an error. `catalog:` peers of registry packages bind to the importer's copy instead of the root catalog. - `--frozen-lockfile` / `bun ci` on turbo-pruned monorepos: pruned-away workspaces are tolerated, a survivor depending on a pruned workspace is an error, catalog subsets are accepted, and an overrides/catalogs change is a frozen failure. #### One output vocabulary Every command here prints the install family's shapes: header, glyph rows (`+`/`-`/`↑`, dedupe's `↳ name old → new`), exactly one noun-first summary line with counts and a duration (`2 duplicate versions removed, 3 packages installed (checked 5 packages) [12ms]`, `N packages removed (checked C) [t]`), no-ops that say what was checked, remedies printed as copy-pasteable command lines, warnings as `warn:`, `--silent` printing nothing, and errors with their remedy together on stderr. Transitive and named updates render as the summary's `↑` rows (once per package; `--dry-run` prints the same rows plus `N packages would be updated`); dedupe reports after the install it triggers, so lifecycle-script output never splits it. A lockfile whose bytes did not change is no longer rewritten (`Saved lockfile` only prints on a real write; `--lockfile-only` no-ops print `Done! Checked N packages (no changes)`). This came out of running every command against fixtures and comparing with `install`/`add`/`remove` (95 findings, all fixed). #### Config precedence A project's `bunfig.toml` now beats any `.npmrc` (project or user-level) for the same key (npmrc files → bunfig's set fields → CLI); npmrc-only settings such as `//host/:_authToken` still attach to bunfig-declared registries, matched by host and path regardless of how either file spells the trailing slash. #### Lockfile migration - `package-lock.json`: rebuilt around a reachability walk that derives each resolution from the entry itself. Fixes `git+https://github.com/…` resolutions being written unparseably (the next install threw the lockfile away), root `bundleDependencies` migrating to an **empty** lockfile, lockfileVersion 1 (and npm's upcoming 4) making `bun install` exit 1 instead of resolving fresh, dependency-level bundles, `dependencies`+`optionalDependencies` double edges, unreferenced entries aborting the migration, duplicate packages for identical `name@version` at nested paths, lost `optionalPeers`, lost integrity when a bundled copy was seen first, and `overrides` not being carried over. All 57 of arborist's v2/v3 fixture projects are vendored and migrated under snapshot. - `pnpm-lock.yaml` v9: bare-hash `patchedDependencies`, snapshot aliases, `catalog:default`, recorded tarball URLs, git `path:`, multi-document files, `runtime:` entries, named registries, peer-suffixed keys chosen per importer, injected workspaces, manifest-only importer deps. #### Isolated linker - An existing store entry whose dependencies re-resolved (override, dedupe, update) now has its links refreshed on the next install (measured cost below). `bun prune` builds the same store the installer builds, so stale `name@version+<peerhash>` variants left by peer bumps are removed and a kept package's real entry never is (whether it was installed with full or `--production` features); on the hoisted linker, dedupe / audit fix / update delete the nested copies whose rows they collapsed instead of leaving the old copy loadable. - Blocked entries resume through per-entry intrusive waiter lists instead of a scan of every store entry after each completion (robobun's #25983/#28425 attempted this). Measured on the reporter's repro from #25799 (2,259 store entries) and a synthetic 6,425-entry monorepo, PR build vs merge-base build: main-thread CPU in the link phase drops 0.85 → 0.33 s and 5.5 → 0.85 s (the removed work grows quadratically); wall time is unchanged with spare cores and 16% / 26% faster pinned to one CPU, the CI/Docker shape in those reports. (The minute-long installs originally reported were peer resolution, fixed before this PR's base.) - Two pre-existing leaks surfaced by the new LSan-enabled tests are fixed: the header buffer of every authenticated registry request, and the per-entry lifecycle-script lists. #### Other bug fixes `bun add x@npm:pkg` writes a range; `bun add --trust a b` no longer drops `b` when `a` was already trusted; `bun add x --dev` no longer rewrote every group in `bun.lock`; `catalog:` peer hoisting; `dependency::Version::eql` treated all `catalog:` specifiers as equal; a `file:` package whose dependencies reach itself (its own name, an `npm:` alias under its own name, two link targets depending on each other — the shapes a `package-lock.json` migration produces, and #25202's `workspace:.` self-reference) hung `bun install` forever in the hoisting tree — the migration shapes now install, and #25202's literal shape now terminates with `Workspace dependency "foo" not found` rather than installing as npm does; a peer of a `file:` package that was only placed nested was also written to `optionalPeers` in a migrated bun.lock, so the next `--frozen-lockfile` failed and a plain install rewrote the lockfile; `catalog:` literals in `bun update`; alias output in the install summary; help/completions for everything above. #### Behavior changes to note in the release notes - `bun update` moves transitive packages; `bun update <name>` no longer adds an undeclared package (exit 1); `--production`/`--prod` on update means "only update `dependencies` and `optionalDependencies`" (a group filter like `--dev`, not the install flag) and `-r`+names with no match is an error; `-i` updates only the selection. - Project `bunfig.toml` overrides any `.npmrc`. - `bun install <pkg> --filter x` edits `x` (not the root); `bun add y --filter x` no longer installs a package named `x`; `add`/`remove --filter '*'` no longer includes the root. - A plain `bun add x` in a workspace whose default catalog lists `x` writes `catalog:`; `audit fix` may rewrite exact pins; `--frozen-lockfile --lockfile-only` writes nothing; overrides/catalog changes fail frozen installs. - One-time lockfile churn after upgrading for projects with `catalog:` peers or dead `pkg@range` override rows; lockfiles that use nested/scoped overrides are v3 and unreadable by older Bun (only when opted in). Turborepo, Nx and Dependabot have been checked; the needed upstream changes are open (nrwl/nx#36666 covers v2 and v3; vercel/turborepo#13740 accepts v3 and preserves the object rows through prune — turborepo main today parses v2 and rejects v3; dependabot needs nothing). Note v2 itself only exists on the 1.4 line. - `bun audit --json` keeps npm's contract: `--audit-level`/`--ignore` decide the exit code, the JSON document is the full registry report (closes #31013 as won't-change). Automatic removal of stale `node_modules` entries on plain `bun install` (#32974) is separate from this PR: `bun prune` is the manual form, and dedupe / audit fix / update now clean up the nested copies they collapse on the hoisted linker; #32974 should reuse prune's planner, and #29512 (sbom) is sequenced after this so it can build on `reachable.rs` instead of carrying its own walk. - Deliberately kept where we differ from pnpm: root `bun update` covers the whole workspace; dependents whose ranges allow follow a moved version (one copy, not two); `--latest` works on transitive names; `--no-save` touches neither file; prune deletion failures exit 1; audit requests stay per-registry. #### Performance Measured on a 1,113-package Next/Prisma/MUI app (PR build vs a PR build of the merge base, interleaved, plus canary and 1.3.14): every hoisted cell is within noise except no-op install, +0.8 ms (+2%, identical syscalls); isolated no-op is +1.9 ms (+3.9%) — the deliberate cost of re-checking existing entries' links every install rather than persisting a stamp file. Everything added is otherwise off the plain-install path (gated on the feature being used or on a diff), and id-indexed sets are bitsets. ### How did you verify your code works? ~1,100 new or ported test cases across the install suites (designed behavior, cases ported from pnpm's suites, pinning tests for pnpm bugs this implementation is immune to, arborist's fixtures, and the CI review findings), all `toStrictEqual`; the whole `test/cli/install` directory passes locally and the existing suites are unchanged except where a pre-existing expectation was deliberately changed (each listed above). `bun update` was additionally verified with a rerunnable differential harness that runs pnpm 11 and this branch on 26 scenario families against one registry and diffs the resulting resolutions edge by edge — after this PR only the deliberate differences above remain. Ecosystem: Turborepo, Nx and Dependabot were checked against the new lockfile output. Co-authored work absorbed with credit: @kjanat's #38190 (alias handling, co-author on the commit), @charpeni's #31143 and @crystalin's #34407 (both superseded), and the tests of the earlier `bun update` PRs (#31752 by @zlotnika, #33127, #36381, #36729, #38224). robobun's #34688 (folder-dependency cycles; its tests are lifted, co-author on the commit) and #37289 (migrated optionalPeers; its test is lifted, co-author on the commit) were fixed independently here and are closed by this PR. #28422's quadratic scan is fixed here as well (already closed). Related but not closed — `bun prune` gives these a manual fix while the automatic-cleanup asks stay open: #8662, #26305, #29793, #21216, #16176. Also related: #10930, #26970, #26751. Fixes #1343 Fixes #3605 Fixes #14719 Fixes #24122 Fixes #18612 Fixes #20238 Fixes #25826 Fixes #23615 Fixes #26973 Fixes #20593 Closes #31013 Fixes #28959 Fixes #28402 Fixes #27897 Fixes #26675 Fixes #10949 Fixes #18504 Fixes #13388 Fixes #24523 Fixes #6608 Fixes #19059 Fixes #16569 Fixes #8262 Fixes #11901 Fixes #13469 Fixes #25202 Closes #29664 Closes #31143 Closes #34407 Closes #34688 Closes #37289 Closes #38190 --------- Co-authored-by: Kaj Kowalski <info@kajkowalski.nl> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
1 participant
Fixes #16176. Fixes #8662.
What does this PR do?
bun installnow removes rootnode_modulesentries that are no longer reachable from the lockfile, for both the hoisted and the isolated linker, so a dependency removed frompackage.jsonstops being importable instead of lingering on disk forever. Details below.Problem
Removing a dependency from
package.jsonand runningbun installprunes it frombun.lockbut never fromnode_modules. The removed package stays importable on every machine that had installed it (a phantom dependency) and breaks on a fresh clone or in CI. Repeatedbun installkeeps reporting "no changes" and never cleans it up. Both the hoisted and the isolated linker are affected, and the same thing happens when an upgrade shrinks a package's transitive closure (the #16176 repro).npm,pnpm, andyarnall removenode_modules/gammahere.Cause
Both linkers drive purely off the lockfile tree and only add or update the entries it contains. Nothing ever walks
node_moduleson disk to compare against the new graph. The lockfile diff even notices (src/install/lockfile/Package.rscountssummary.removewith a comment saying "It will be cleaned up later") but nothing does the cleanup. The only place a stale top-level directory is deleted is thebun remove <pkg>subcommand's hardcoded cleanup, which a plainbun installnever reaches.Fix
Add
prune_extraneous_node_modules(src/install/PackageInstall.rs), called by both linkers afterLockfile::clean_with_loggerhas rebuilt the lockfile. It walks the top level of the rootnode_modules(descending one level into@scope/directories, and removing a scope directory that ends up empty) and deletes any entry whose folder name is not a dependency alias in the lockfile. Dot-prefixed entries (.bin,.bun,.cache, ...) are never touched, and the pass is skipped for the security scanner's narrowed pre-install (packages_to_install); the full install that follows performs it.Each linker builds its own kept set 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,--omit,--filter, and--cpu/--osoverrides never turn a reinstall into a delete of packages the lockfile still contains) unioned with the linker's own root placements and with the names of packages that have apatchedDependenciesentry. For the hoisted linker the root placements are the unfiltered root tree (snapshotted beforefilter()runs), because hoisted transitives legitimately live at the root; for the isolated linker they are every lockfile alias matchingpublicHoistPattern(the store's own root entry cannot be used because it is narrowed by--filter). Patched package names are included becausebun patchmaterializes the package being patched at the rootnode_modules/<name>even when no tree places it there; without them,bun patch --commitremoved the folder it had just told the user about (caught by the existingbun patchworkspace test inbun-patch.test.ts, which passes again). A direct dependency that is removed but survives as someone else's transitive is therefore kept by the hoisted linker (it is still correctly hoisted there) and pruned by the isolated linker, whose root must only contain direct dependencies.Two places the prune deliberately never runs or reaches:
bun add -g/bun install -goperate inside the global install directory, which is also thebun linkregistry:bun linkrecords a package there as a bare symlink with no entry in the global package.json or lockfile, so pruning it would destroy every link registration. The global directory is a tool registry, not a project.bun link <pkg>in a consumer is--no-saveby default, so the localnode_modules/<pkg>symlink is also recorded in no manifest. Hoisted extraneous entries are always real extracted directories; a root symlink there is a workspace link, afile:folder dependency, or abun link, none of which the prune should touch (declining to delete them is exactly main's behavior today). The isolated linker's own root entries are symlinks intonode_modules/.bun, so it keeps pruning them; isolated plus an unsavedbun linkis a pre-existing gap (install: isolated linker honors active bun link #30289).Two details worth calling out:
Dependency.name), because that is thenode_modules/<name>folder both installers write.Dependency.name_hashis the hash of the resolved package name, so for"moo": "npm:@barn/moo"it would missnode_modules/mooand delete it.node_modules/.bin/<cmd>symlink would otherwise be left dangling (turning a stale-but-working bin into anENOENT), so the prune finishes with the dangling-symlink sweepbun removealready performs. That sweep is extracted into a sharedprune_dangling_bin_linksand thebun removepath now calls it. The prune itself is best effort: a stale directory the filesystem refuses to delete (locked, read-only) is left behind rather than failing the install, matchingbun removeand npm.The existing
add trusted, delete, then add againtest inbun-install-lifecycle-scripts.test.tswas written with a comment saying to update it "when we change bun install to delete dependencies from node_modules for both cases"; updated as instructed (the manual-edit branch now matches thebun rmbranch, and the re-add installs the pruned transitive dependency as well).Unrelated to the prune but required to run these suites at all: the test harness now starts Verdaccio bound to
127.0.0.1explicitly. With only a port it listens on whatevergetaddrinfo("localhost")returns first, and on hosts where that is::1every registry-backed install test fails withConnectionRefused.How did you verify your code works?
12 new tests in
test/cli/install/bun-install-registry.test.tsunderprunes stale node_modules entries (<linker>), run for both the hoisted and the isolated linker:node_modules@scopedirectorybun installdoes not delete extraneous dependencies #16176 shape;bun remove <package>should remove the dependencies that thepackagedepended on if they are no longer used #8662 is the same mechanism viabun remove, covered by the updatedadd trusted, delete, then add againlifecycle test, whose hoisted transitivewhat-binis now pruned too)file:folder dependency survives the prune.binsymlinkpublicHoistPatternhoist from a--filtered-out workspace survives a filtered installplus two standalone tests: a
bun linkregistration in the global directory survivesbun add -g, and an unsavedbun link <pkg>in a consumer survivesbun install.16 of them fail on the current release build and pass with this change, as do the 4 updated lifecycle-script tests. The
--filter, global-install, and unsaved-link tests additionally fail on intermediate revisions of this branch that lacked their respective guards, which is how those three cases were pinned.Beyond the suites, both linkers were checked against a workspace symlink at the root (preserved),
--productionwith dev dependencies on disk (preserved), annpm:aliased scoped package (preserved, while an unrelated removed package alongside it is pruned), and apublicHoistPattern-hoisted transitive at the isolated root (preserved).The per-workspace
node_modulesdirectories are out of scope here; the hoisted side of that is #29793 / #29794.