install: add install.hoist to disable the isolated linker's hoisted fallback directory

[robobun]

Problem

With --linker=isolated, every install creates node_modules/.bun/node_modules/, a hidden directory containing a symlink to every installed package. Since it is an ancestor of every store entry, it sits on the upward module-resolution path of every package in the store: a package can require() a dependency it never declared and it still resolves, under Bun and Node alike.

mkdir proj && cd proj
echo '{"dependencies":{"dep-a":"file:../dep-a.tgz","dep-b":"file:../dep-b.tgz"}}' > package.json
# dep-b depends on dep-c; dep-a requires dep-c without declaring it
bun install --linker=isolated
bun -e 'require("dep-a")'   # works: dep-c resolves via node_modules/.bun/node_modules/dep-c

Strictness is the point of the isolated layout, and this fallback makes phantom imports dependent on what else happens to be installed: the same import resolves in one project's closure and fails in another's.

Fix

Add install.hoist = true | false (bunfig.toml) and hoist=false (.npmrc), matching pnpm's hoist setting. It applies to the isolated linker only; hoisted installs ignore it, and the docs say so explicitly:

• hoist = false skips creating node_modules/.bun/node_modules and nothing else. The project's declared-dependency symlinks, the store layout, bin links, and publicHoistPattern behavior are unchanged. Nothing in Bun's own resolution reads that directory; it is only created (lazily) for hoisted entries.
• When a previous install left the directory behind, a hoist = false install removes it, so flipping the setting actually restores strictness without rm -rf node_modules. The removal unlinks the fallback symlinks without following them.
• hoist = false takes precedence over install.hoistPattern, same as pnpm where hoist=false discards hoist-pattern.

One caveat, shared with pnpm: the project root node_modules also sits above the store (.bun nests inside it), so packages linked there (direct dependencies, publicHoistPattern matches, workspace packages) remain resolvable from any store package. The docs state this and a test pins it.

Note hoistPattern = "" (or any non-matching pattern) already suppresses the directory today; this adds the documented boolean switch for it, plus the stale-directory cleanup, which the pattern path does not do.

Tests

test/cli/install/isolated-install.test.ts, describe("hoist"):

• hoist = false leaves no node_modules/.bun/node_modules; a store package resolves its declared deps through its own store entry, fails with MODULE_NOT_FOUND on an undeclared package, and can still reach a root-linked direct dependency (the pnpm-parity caveat above); root symlinks unchanged
• a stale fallback directory from a previous (default) install is removed on the next hoist = false install, and the store entries the fallback symlinks pointed at survive
• hoist = false wins over hoistPattern = "*"
• hoist = false leaves publicHoistPattern hoisting intact
• .npmrc hoist=false works

All five fail on bun without this change (the fallback directory exists) and pass with it. Full isolated-install.test.ts and public-hoist-pattern.test.ts pass.

Docs updated: install.hoist in bunfig.mdx, hoist in npmrc.mdx, and a section in isolated-installs.mdx (the layout diagram now shows the fallback directory).

---

no test proof · iteration 3 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/isolated-install.test.ts

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 68213287-6f9a-4ae4-8770-7f2ecd87587a

📥 Commits

Reviewing files that changed from the base of the PR and between 0713a9a and e2cfb4f.

📒 Files selected for processing (1)

• src/install/isolated_install.rs

---

Walkthrough

Changes

Isolated install hoist control

Layer / File(s)	Summary
Configuration contract and loading src/options_types/schema.rs, src/bunfig/bunfig.rs, src/ini/lib.rs, src/install/PackageManager/PackageManagerOptions.rs	Adds the optional install.hoist setting. Bunfig and npmrc loaders parse it. Package manager options default it to true and apply configured values.
Fallback hoisting enforcement src/install/isolated_install.rs, docs/pm/isolated-installs.mdx, docs/pm/npmrc.mdx, docs/runtime/bunfig.mdx	Disables fallback hoisting when hoist is false and removes stale fallback directories. Documentation describes the fallback layout and configuration rules.
Hoist configuration test coverage test/harness.ts, test/cli/install/isolated-install.test.ts	Extends test configuration generation and validates resolution boundaries, cleanup, precedence, and npmrc support.

Possibly related PRs

• oven-sh/bun#36973: Both PRs update isolated-install fallback layout documentation and resolution behavior.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: adding install.hoist to disable the isolated linker's hoisted fallback directory.
Description check	✅ Passed	The description explains the problem, fix, behavior, configuration options, tests, documentation updates, and verification status.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

No bugs found. This adds a new user-facing config option (install.hoist in bunfig / hoist in .npmrc) and changes documented install.hoistPattern default wording, so it's worth a maintainer look at the API surface before landing.

What was reviewed:

• Traced hoisted → link_to_hidden_node_modules: gating on options.hoist correctly suppresses the fallback dir; public_hoist_pattern path is independent and unaffected.
• delete_tree on the stale fallback: entries are unlinked via unlinkat and subdirs opened with O_NOFOLLOW, so store-entry symlink targets are not followed; Fd::cwd() matches surrounding mkdirat usage.
• Option threading (schema → bunfig/npmrc → Options) mirrors the existing global_store/hoist_pattern pattern; default true preserves current behavior.

Extended reasoning...

Overview

Adds install.hoist = true|false (bunfig) and hoist=false (.npmrc), mirroring pnpm's hoist setting. When false, the isolated linker skips creating node_modules/.bun/node_modules (the hidden fallback that lets store packages resolve undeclared deps), and removes a stale one left by a previous install. Touches the schema type, bunfig/npmrc parsers, PackageManagerOptions, two spots in isolated_install.rs, three docs pages, the test harness (writeBunfig), and adds four tests.

Security risks

None identified. The only new filesystem mutation is a best-effort delete_tree of node_modules/.bun/node_modules relative to the install cwd. I checked bun_sys::Dir::delete_tree: entries are first unlinkat'd (removing symlinks themselves, not targets) and directory descent uses O_NOFOLLOW, so the store entries the fallback symlinks point at are not touched — the second test asserts exactly this. The path is a fixed literal, not user-controlled.

Level of scrutiny

Medium-high. The implementation itself is small and mechanical (a boolean gate before an existing labelled block, plus a cleanup call), and it precisely follows the plumbing pattern of the neighboring global_store and hoist_pattern options. However, it introduces new documented user-facing API surface, and it also rewrites the docs description of install.hoistPattern's default (from "Default []" to "every package is hoisted there, equivalent to ["*"]"). Both look correct to me — the code shows that with no hoist_pattern set, every dep is hoisted to the hidden dir — but per the repo's guidance API additions should get a maintainer's eyes.

Other factors

• The prior nit (bare .toThrow()) was addressed in 9c36fb2; the PR diff now asserts code: "MODULE_NOT_FOUND".
• publicHoistPattern is handled on a separate branch (root entry_dependencies[0]) and is not gated by options.hoist, matching the PR description.
• The let _ = on delete_tree swallows errors, which matches the surrounding best-effort renameat calls in the same function and is acceptable for cleanup of a directory that may not exist.
• The harness change also fixes a latent missing-newline bug in the existing publicHoistPattern bunfig writer (previously the last option written had no trailing \n).

[robobun]

Status: the change is complete and review feedback is addressed.

• 5 tests in test/cli/install/isolated-install.test.ts (describe("hoist")) fail without the change and pass with it, locally and in CI.
• The only hard CI failure across builds 89232/89240/89251 is test/js/node/async_hooks/AsyncLocalStorage-tracking.test.ts on the x64-asan lane: a pre-existing BoringSSL RSA keygen leak that also fails on main, unrelated to this diff (install-only). It has been reported separately.
• Remaining CI entries are flakes that passed on retry, on lanes this diff does not touch.

Ready for review.

[claude]

Thanks for addressing the .toThrow() nit. I didn't find any further issues, but this adds a new user-facing config option (install.hoist in bunfig + hoist in .npmrc), so it's worth a maintainer confirming the API surface and pnpm-parity semantics.

Checked: the hoisted short-circuit leaves publicHoistPattern and root-symlink paths untouched; delete_tree on the stale fallback opens with O_NOFOLLOW and unlinks symlinks without following them into store entries (test pins this); the harness publicHoistPattern newline fix doesn't change any existing test's bunfig (it was previously last in the file).

Extended reasoning...

Overview

Adds install.hoist = true|false (bunfig) and hoist=false (.npmrc), mirroring pnpm's hoist setting. When false, the isolated linker skips creating node_modules/.bun/node_modules (the hidden fallback that lets store packages resolve undeclared deps), and removes a stale one left by a prior install. Touches config plumbing (schema.rs, bunfig.rs, ini/lib.rs, PackageManagerOptions.rs), a two-line short-circuit plus a cleanup call in isolated_install.rs, five new tests, harness bunfig-writer additions, and docs for three pages.

Security risks

None identified. The only filesystem mutation added is Fd::cwd().delete_tree("node_modules/.bun/node_modules"), a fixed literal path inside the project's own node_modules that Bun itself creates. delete_tree opens the target with O::NOFOLLOW and unlinks entries by kind, so the symlinks inside the fallback are removed without following them into the store — the "removes a stale fallback directory" test verifies the pointed-at store entries survive. No user-controlled input reaches the path.

Level of scrutiny

Medium. Default is true, so existing installs are unaffected; the implementation is ~30 lines of Rust and follows the exact pattern of the neighboring hoist_pattern/public_hoist_pattern/global_store options end-to-end. But it is new documented API surface for the package manager, and the PR also rewrites the install.hoistPattern doc default from "[]" to "equivalent to ["*"]" — accurate to the code (None → hoist every dep name), but a user-visible doc change a maintainer should sanity-check.

Other factors

• Test coverage is thorough: fallback absent, declared-dep resolution intact, undeclared-dep now MODULE_NOT_FOUND, root-linked deps still reachable (the pnpm caveat), stale-directory cleanup, precedence over hoistPattern, publicHoistPattern independence, and the .npmrc path. My prior review's bare-toThrow() nit was fixed in 9c36fb2.
• The harness change adds trailing \n to the existing publicHoistPattern bunfig line — previously it was the last line written so no existing test is affected, and the new hoistPattern/hoist keys are appended after it.
• let _ = ... delete_tree(...) swallows the cleanup error. That's consistent with the surrounding best-effort node_modules setup code and only affects the opt-in hoist=false path; a failure just leaves the stale directory in place.
• PR notes "no test proof" locally (deferred to CI), so CI green is the actual verification signal here.

[robobun]

^{Updated 12:43 PM PT - Aug 5th, 2026}

⏳ @robobun, your commit e2cfb4f is still building in Build #89303, but has 1 failures so far (All Failures):

• test/js/node/async_hooks/AsyncLocalStorage-tracking.test.ts - direct leak of 24b in run (src/runtime/node/node_crypto_binding.rs:85:21) +34 more on 🐧 13 x64-asan

[coderabbitai]

Actionable comments posted: 4

🤖 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 `@docs/runtime/bunfig.mdx`:
- Line 695: Update the documentation describing the isolated linker’s
node_modules/.bun/node_modules fallback to state that it contains symlinks only
for packages selected by install.hoistPattern, with all installed packages
selected by default when no pattern is configured. Preserve the existing hoist =
false behavior and root node_modules caveat.

In `@src/install/isolated_install.rs`:
- Line 1927: Update the stale-fallback cleanup around Fd::cwd().delete_tree to
construct the target through the established absolute-path helper before
deletion, replacing the relative
paths::path_literal!("node_modules/.bun/node_modules") value while preserving
the existing best-effort cleanup behavior.
- Around line 1925-1927: The stale-fallback cleanup in the hoist-disabled branch
must handle the result of Fd::cwd().delete_tree for
node_modules/.bun/node_modules instead of discarding it. Ignore only the
expected not-found error; for any other failure, report the exact cleanup path
and propagate the error to stop installation.

In `@test/harness.ts`:
- Around line 2025-2029: Update the hoistPattern serialization in the
opts.hoistPattern branch to check explicitly for undefined so an empty string is
preserved, and TOML-escape every string value before quoting it in both the
scalar and array forms. Keep array handling intact while ensuring each element
is serialized safely.

🪄 Autofix

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: 4d029d35-a9ac-4254-90fb-561f638a5229

📥 Commits

Reviewing files that changed from the base of the PR and between 1f606e6 and adc1c00.

📒 Files selected for processing (10)

• docs/pm/isolated-installs.mdx
• docs/pm/npmrc.mdx
• docs/runtime/bunfig.mdx
• src/bunfig/bunfig.rs
• src/ini/lib.rs
• src/install/PackageManager/PackageManagerOptions.rs
• src/install/isolated_install.rs
• src/options_types/schema.rs
• test/cli/install/isolated-install.test.ts
• test/harness.ts

[claude]

No issues found — the implementation is clean and well-tested, and all prior review feedback has been addressed. Deferring because this adds a new user-facing config option (install.hoist / npmrc hoist), which is an API-surface decision worth a maintainer's sign-off.

What was reviewed:

• The hoist gate in isolated_install.rs and the stale-fallback delete_tree cleanup — verified delete_tree returns Ok on ENOENT and unlinks symlinks without following them into store entries (the test pins this).
• Config plumbing through bunfig/npmrc/schema/Options — matches the existing hoist_pattern/node_linker pattern; default true preserves current behavior.
• Test harness writeBunfig changes — both hoistPattern and publicHoistPattern now serialize consistently.

Extended reasoning...

Overview

This PR adds install.hoist (bunfig.toml) and hoist (.npmrc), a boolean matching pnpm's hoist setting, that controls whether the isolated linker creates the node_modules/.bun/node_modules fallback directory. Files touched: src/install/isolated_install.rs (two small hunks — a gate on the hoisted computation and a stale-directory cleanup), src/install/PackageManager/PackageManagerOptions.rs + src/options_types/schema.rs + src/bunfig/bunfig.rs + src/ini/lib.rs (config plumbing), three docs pages, five new tests in isolated-install.test.ts, and test/harness.ts (writeBunfig gains hoist/hoistPattern and hardens publicHoistPattern serialization).

Security risks

None identified. The only new filesystem operation is a delete_tree of node_modules/.bun/node_modules relative to the project-root cwd, gated on !hoist && !is_new_bun_modules. The path is a fixed literal (no user-controlled components), delete_tree unlinks symlinks rather than following them (verified in src/sys/dir.rs, and the stale-cleanup test asserts the pointed-at store entries survive), and the surrounding function already performs all node_modules operations cwd-relative.

Level of scrutiny

Moderate-to-high. The runtime change is small and mechanically safe (default hoist: true preserves existing behavior; the new branch only fires when the user explicitly opts in). But it is a new user-facing configuration option in the package manager: the naming, the precedence over hoistPattern, the decision to make stale-cleanup failure fatal via Global::exit(1), and the pnpm-parity framing are API design choices a maintainer should confirm. REVIEW.md flags API-surface additions as warranting the situational .claude/docs/landing-prs.md API-design read.

Other factors

• All prior review feedback has been addressed across five follow-up commits: my two nits (bare .toThrow() → MODULE_NOT_FOUND check; publicHoistPattern serialization consistency), CodeRabbit's four points (docs wording on hoistPattern, fatal error on cleanup failure, harness JSON-escaping — accepted; absolute-path suggestion — correctly declined as inconsistent with the file idiom), and the comment-cop length trims.
• Test coverage is thorough: five tests covering the base behavior, publicHoistPattern interaction, stale-directory cleanup, precedence over hoistPattern, and the .npmrc path. The PR body reports they fail on main and pass with the change; CI status per robobun's summary is green modulo an unrelated pre-existing ASAN failure.
• I checked delete_tree semantics in src/sys/dir.rs: delete_tree_open_initial_subpath returns Ok(None) on ENOENT, so the "succeeds on a missing tree" comment is accurate and no not-found carve-out is needed.

Deferring rather than approving solely because new package-manager config surface is the kind of thing a maintainer should nod at, not because anything looks wrong.

[coderabbitai]

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/isolated_install.rs`:
- Around line 1930-1935: Update the cleanup error handling around Output::err to
preserve err while stating that install.hoist = false cannot be enforced for
./node_modules/.bun/node_modules, and provide the concrete remedy of removing
the directory or fixing its permissions.

In `@test/harness.ts`:
- Around line 2018-2029: Update the publicHoistPattern and hoistPattern
serialization branches to reject strings containing lone surrogates before
constructing bunfig, covering both scalar values and array entries at each
JSON.stringify call. Prefer the existing TOML serializer if available; otherwise
validate each pattern and fail before emitting invalid TOML.

🪄 Autofix

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: 9c51be2d-a854-49d2-b97c-9544aba22401

📥 Commits

Reviewing files that changed from the base of the PR and between adc1c00 and 0713a9a.

📒 Files selected for processing (3)

• docs/runtime/bunfig.mdx
• src/install/isolated_install.rs
• test/harness.ts

[claude]

No issues found, but this adds new user-facing API surface (install.hoist in bunfig and hoist in .npmrc) and changes the isolated-install layout, so a maintainer should sign off on the API choice.

What was reviewed:

• The hoist gate in the store-entry loop and the stale-fallback delete_tree cleanup — placement, error handling, and that delete_tree succeeds on absent trees so no ENOENT carve-out is needed.
• Config plumbing through bunfig, .npmrc, schema, and Options — default true preserves current behavior; precedence over hoistPattern matches pnpm.
• Test coverage: fallback skipped, stale cleanup, precedence over hoistPattern, publicHoistPattern unaffected, .npmrc path — all assert observable layout/resolution.
• The earlier harness nit (parallel publicHoistPattern serialization) was addressed in 0713a9a.

Extended reasoning...

Overview

This PR adds install.hoist = true | false (bunfig) and hoist=false (.npmrc), mirroring pnpm's hoist setting, to control whether the isolated linker creates the node_modules/.bun/node_modules fallback directory. It touches config parsing (bunfig.rs, ini/lib.rs, schema.rs, PackageManagerOptions.rs), the isolated installer (isolated_install.rs), three docs pages, the test harness, and adds five tests to isolated-install.test.ts.

Security risks

None identified. The only new filesystem operation is a delete_tree on node_modules/.bun/node_modules relative to the project root, which is already the working directory for every other operation in that function. No user-controlled paths are introduced.

Level of scrutiny

Medium-high. This is new documented API surface that will be maintained indefinitely and changes what the isolated linker writes to disk. The implementation is small (~30 lines of runtime code) and well-tested, but the API choice — naming, semantics, precedence over hoistPattern, scope to isolated-only — is a maintainer decision. It follows pnpm's precedent, which is a strong argument, but per the repo's landing guidance new API surface warrants human sign-off.

Other factors

All prior review threads (CodeRabbit, comment-cop, and my own harness-consistency nit) are resolved. The stale-cleanup error handling was iterated on and now fails the install with a clear message naming the setting and path, matching the fatal mkdir failures in the same block. Test coverage is thorough: it verifies both the negative (fallback absent, undeclared import fails with MODULE_NOT_FOUND) and positive (declared deps and root-linked packages still resolve, store entries survive cleanup) contracts, plus config-source and precedence variants. The bug-hunting system found no issues this run.