install: prune stale workspace node_modules in hoisted installs - #29794
Open
robobun wants to merge 8 commits into
Open
install: prune stale workspace node_modules in hoisted installs#29794robobun wants to merge 8 commits into
robobun wants to merge 8 commits into
Claude / Claude Code Review
completed
Aug 14, 2026 in 41m 54s
Code review found 1 important issue
Found 3 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | test/cli/install/bun-update.test.ts:570-579 |
Rewritten '--filter pkg-a' test likely fails on HEAD (735d5c5) |
| 🟡 Nit | src/install/hoisted_install.rs:122-127 |
Prune uses Resolvable-mode original_trees, not the Filter-mode trees actually installed |
Annotations
Check failure on line 579 in test/cli/install/bun-update.test.ts
claude / Claude Code Review
Rewritten '--filter pkg-a' test likely fails on HEAD (735d5c51)
The rewritten assertion at line 575 will fail on HEAD (735d5c51): after `nestedBazRepo("0.0.5", "0.0.3", { pkgA: "~0.0.3" })`, npm's sticky rule (Package.rs:1559) keeps pkg-a's baz locked at 0.0.3, so its tree row stays and this PR's prune keeps `packages/pkg-a/node_modules/baz` on disk. Revert to the pre-735d5c51 shape — `expect(await pkgABazVersion()).toBe("0.0.3")`, drop the re-plant at 578-579, and restore the helper comment at line 550.
Check warning on line 127 in src/install/hoisted_install.rs
claude / Claude Code Review
Prune uses Resolvable-mode original_trees, not the Filter-mode trees actually installed
The prune's expected set is built from `original_trees` (Resolvable-mode, from `Lockfile::resolve()`), but the install loop lays out packages using the post-`filter()` Filter-mode trees, which hoist more aggressively (they drop os/cpu-disabled, bundled, and `--omit=dev/peer/optional` deps per Tree.rs:706). So under `--production`/`--omit` a dep that Filter hoists to root can still appear in the workspace's Resolvable expected set, and a stale nested copy survives — #29793 remains for those layou
Loading