[robobun]

Repro

A three-package, all-local project where a file: dependency has its own file: dependency (the standard vendored-monorepo shape):

# package.json            depends on  file:./vendor/lib
# vendor/lib/package.json depends on  file:../util
mkdir -p vendor/lib vendor/util
echo '{"name":"app","dependencies":{"lib":"file:./vendor/lib"}}' > package.json
echo '{"name":"lib","version":"1.0.0","main":"index.js","dependencies":{"util2":"file:../util"}}' > vendor/lib/package.json
echo 'module.exports = require("util2")' > vendor/lib/index.js
echo '{"name":"util2","version":"1.0.0","main":"index.js"}' > vendor/util/package.json
echo 'module.exports = "ok"' > vendor/util/index.js

bun install                         # "2 packages installed", exit 0
node -e 'require("lib")'            # Cannot find module 'util2'

The lockfile records the resolution ("lib/util2": ["util2@file:vendor/util", {}]), but node_modules/lib/node_modules is left empty. The install reports success, and every downstream consumer (runtime require, --frozen-lockfile reinstalls, lifecycle scripts for that package) fails in confusing ways. The isolated linker handles the same project correctly; only the hoisted (default) linker is affected.

Cause

Resolution::Folder paths are produced by two parsers with different bases, and the hoisted installer only handles one of them:

• Lockfile::Package::parse (used for the root, workspace members, and local file: packages) normalizes each file: dependency relative to the top-level dir. A file:../util declared in vendor/lib/package.json is stored as vendor/util.
• Package::from_npm (registry manifests) stores the file: path verbatim, so it stays relative to the declaring package (for example ./the-files inside file-dep).

PackageInstaller (src/install/PackageInstaller.rs) sourced every transitive folder dependency from dirname(node_modules.path), the declaring package's install directory. That is only correct for the npm case. For a local file: parent it looked for node_modules/lib/vendor/util, which never exists, and the ENOENT -> InstallResult::Success rewrite on the next lines turned the failure into a successful, counted install.

Fix

• Lockfile::is_folder_tree_id(tree_id): the package whose node_modules a tree represents is resolved from a local file: folder.

• In the installer's transitive-folder branch, when the declaring package is a Folder resolution, install from Fd::cwd() (the top-level dir, which bun install chdirs to) instead of the parent's install dir, and do not swallow the error. A genuinely missing source folder now fails the install with

  error: Could not find folder "file:vendor/util" for dependency "util2"
  

  instead of exiting 0 with the package missing.

• npm-declared transitive folder dependencies keep the existing behavior, including treating a file: path absent from the published tarball as nothing to link. test/cli/install/bun-install-registry.test.ts (transitive file dependencies, the missing-file-dep / self-file-dep / file-dep fixtures) depends on both.

Related: #33106 lifts the ..-escape rejection for transitive file: dependencies declared by local packages. That PR and this one address different halves of the same user story (resolve-time rejection vs install-time materialization) and do not depend on each other; they conflict on one comment line.

Tests

test/cli/install/bun-install.test.ts:

• installs the transitive file: dependency of a file: dependency: asserts node_modules/lib/node_modules/nested/package.json exists and require("lib") works, for both a fresh resolve and a --frozen-lockfile install from the saved lockfile.
• fails when a transitive file: dependency's folder does not exist: the source folder is deleted; asserts the new error message and a nonzero exit instead of a silent success.

Both fail on the released build and pass with this change. Verified no regression in the npm path by diffing bun.lock and the node_modules tree produced by the released build vs this build for all seven *file-dep registry fixtures: byte-identical. bun-install.test.ts (file: and folder), the full bun-install-registry.test.ts, isolated-install.test.ts, and bun-workspaces.test.ts pass (the only failures are pre-existing tests that need network access).

[robobun]

^{Updated 9:31 PM PT - Jun 30th, 2026}

@robobun, your commit 16ca462 is building: #67475

[robobun]

Reproduced with a three-package local project: app -> file:./vendor/lib -> file:../util. On bun 1.4.0-canary.1 and current main, bun install records lib/util2 -> util2@file:vendor/util in bun.lock but leaves node_modules/lib/node_modules empty, prints "2 packages installed", and exits 0; require("lib") then fails with Cannot find module.

Status: fix and tests are pushed (c11d7af); the 4 bot review findings are addressed and resolved.

CI on c11d7af (build 67417, 282 passed / 4 failed) was green on every lane that runs install code, including Windows 2019. The 4 red jobs were unrelated to this diff, and I have re-run CI once for them:

• darwin 26 aarch64 - test-bun (x2): buildkite-agent artifact download timed out after 120s on the same agent; 0 tests ran.
• alpine 3.23 x64 / x64-baseline - test-bun: test/js/node/test/parallel/test-net-connect-memleak.js, the known musl flake tracked in CI: test-net-connect-memleak.js fails on half of PR builds on linux-x64-musl since June 28 ~23:00 UTC #33044.

The design question from @dylan-conway above is still open; this diff is the targeted installer fix either way, and I am happy to follow up with the structural FolderResolution route if that is preferred.

[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: 13f25c6b-5e74-4011-938a-f54c77aa27cc

📥 Commits

Reviewing files that changed from the base of the PR and between ad3d55d and c11d7af.

📒 Files selected for processing (2)

• src/install/PackageInstaller.rs
• test/cli/install/bun-install.test.ts

---

Walkthrough

This PR changes how Bun's installer handles transitive file: dependencies that resolve to local folders. Folder-tree installs now take a direct install path, Lockfile::is_folder_tree_id detects folder trees, and missing transitive folders produce a targeted error. Two integration tests cover success and failure.

Changes

Transitive folder dependency install handling

Layer / File(s)	Summary
Lockfile folder-tree detection src/install/lockfile.rs	Adds is_folder_tree_id, which maps a tree id through dependency and package ids and returns whether the package resolution is Folder, handling id == 0 and invalid package ids safely.
Direct install path for transitive folder dependencies src/install/PackageInstaller.rs	Updates transitive Resolution::Folder handling to call installer.install(...) directly for folder trees, adjusts related comments, and adds a missing-cache branch that logs a specific folder-not-found error and increments the failure count.
Integration tests for transitive file: dependencies test/cli/install/bun-install.test.ts	Adds one test for successful nested transitive file: installs across repeated runs and one test for the missing-folder failure path with the expected error message and non-zero exit.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing transitive file: dependency linking for local file: packages.
Description check	✅ Passed	The description covers what changed and how it was verified, though it uses custom headings instead of the template's exact sections.
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.}

[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/PackageInstaller.rs`:
- Around line 1986-1994: The folder-resolution error in
PackageInstaller::install is using the resolved package name from
self.names[package_id as usize] instead of the user-declared dependency alias.
Update the error path in the resolution::Tag::Folder and
cause.is_package_missing_from_cache() branch to use the original dependency
key/specifier that appears in package.json (the alias like "foo"), so the
message points users to the entry they need to fix.

In `@test/cli/install/bun-install.test.ts`:
- Line 9593: The install tests are over-specifying the physical
`node_modules/lib/node_modules/nested` layout for transitive local `file:`
dependencies. Update the affected assertions in
`test/cli/install/bun-install.test.ts` to verify the behavior that matters
instead: successful install and resolution for `require("lib")` on the happy
path, and the existing missing-folder failure path, without checking for nested
folder materialization. Use the nearby install/resolve test cases and
`require("lib")` expectations to keep the tests stable across different package
layout strategies.

🪄 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: 7197fd85-b262-4a73-8c10-41ffa12d676c

📥 Commits

Reviewing files that changed from the base of the PR and between 52a1ddf and ad3d55d.

📒 Files selected for processing (3)

• src/install/PackageInstaller.rs
• src/install/lockfile.rs
• test/cli/install/bun-install.test.ts

[claude]

Both test-portability nits from my earlier pass are addressed and I didn't find further issues, but this changes install-path resolution and error semantics in the hoisted linker based on a two-parser path-normalization invariant, so it's worth a human look from someone familiar with src/install/.

Extended reasoning...

Overview

This PR fixes transitive file: dependencies of local file: packages in the hoisted linker. It touches src/install/PackageInstaller.rs (adds a branch in the transitive-folder install path that sources from Fd::cwd() instead of dirname(node_modules.path) when the parent tree is itself a Folder resolution, and adds a new hard-fail error branch for missing local folders), src/install/lockfile.rs (new is_folder_tree_id helper mirroring is_workspace_tree_id), and adds two integration tests to bun-install.test.ts.

Security risks

None identified. No untrusted-input parsing, no path-traversal surface introduced (the folder path was already being used as an install source; only the base directory changes). The related ..-escape concern is explicitly deferred to #33106.

Level of scrutiny

Medium-high. The change is small and well-tested, but it lives in the package manager's hoisted linker — a critical path where subtle regressions silently corrupt node_modules layouts. The correctness of the fix hinges on two invariants that a maintainer should confirm: (1) Package::parse always normalizes Resolution::Folder paths relative to the top-level dir while Package::from_npm leaves them verbatim, including when the resolution is loaded from a lockfile rather than freshly parsed; and (2) Fd::cwd() is reliably the top-level dir at this point for every install entry point (bun install, bun add, bun update, filtered installs). The PR also changes user-visible behavior — a previously-silent success now hard-fails with exit 1 — which is almost certainly the right call but is a UX decision.

Other factors

I previously flagged two test-portability issues (Windows path separator in the error assertion, and expect(stderr).toBe("") under ASAN); both were addressed in c11d7af and the diff reflects the fixes. The bug-hunting system found no issues this round. The PR description is thorough, cites the relevant registry-fixture regression tests as verified byte-identical, and notes the one-line conflict with #33106. CI on the latest commit shows musl build failures and earlier Windows test failures on the pre-fix commit; the Windows failures should be resolved by c11d7af but a green run would confirm.

[robobun]

To save the human reviewer the two invariant checks:

(1) The two formats survive the lockfile round trip, so a lockfile-loaded install behaves the same as a fresh resolve. The text lockfile stores the Resolution::Folder string as is. With a local file: parent it reads "lib/util2": ["util2@file:vendor/util", {}] (top-level-dir-relative, produced by Lockfile::Package::parse), and with an npm parent it reads "file-dep/files": ["files@file:./the-files", {}] (verbatim, produced by Package::from_npm). Both came from real lockfiles dumped with the released build. The discriminator the installer branches on, the declaring package's resolution tag, is also stored (lib@file:vendor/lib vs file-dep@1.0.0), so lockfiles written by current releases work unchanged and there is no migration. Coverage for the loaded path: the second iteration of the new happy-path test wipes node_modules and reruns bun install --frozen-lockfile, and the existing transitive file dependencies registry tests exercise the npm side (present and genuinely missing file: paths) from a saved lockfile.

(2) Fd::cwd() is the top-level dir for every entry point that reaches PackageInstaller. All install subcommands (install, add, update, remove, and the filtered variants) go through the shared PackageManager::init, which sets top_level_dir and then calls bun_sys::chdir(&top_level_dir_z) unconditionally before any resolution or linking runs (src/install/PackageManager.rs, right after set_top_level_dir). The should_chdir_to_root() check a few lines above only decides whether to walk up to a workspace root before top_level_dir is set; the chdir is not gated on it. The node_modules.path the installer iterates is rooted at the same top_level_dir (hoisted_install.rs). This is also not a new assumption: the workspace-tree branch of the same Tag::Folder arm, and the Workspace and Root arms, already install from Fd::cwd() with a top-level-dir-relative subpath.

[dylan-conway]

@robobun is this happening because we need folder_dependencies hash map on the lockfile because there's some code path where we can register the same local package more than once?

[robobun]

Not for the symptom in this PR, but you have put your finger on the real design gap right next to it, and the duplication you are describing does happen.

Why this bug is not caused by duplicate registration. In the minimal repro (app -> file:./vendor/lib -> file:../util) the transitive util2 is registered exactly once. The failure is entirely in the installer: for a transitive Resolution::Folder it opened the source relative to dirname(node_modules.path) (the declaring package's install dir) and rewrote the resulting ENOENT into Success. That base is only right for the file:./the-files shape inside an npm tarball; a local file: parent's dependency list goes through Lockfile::Package::parse, which stores the path relative to the top-level dir, so node_modules/lib/vendor/util never exists. The lockfile entry is correct either way; it just never got linked.

But yes, the same local folder is registered more than once today. PackageManager.folders (the FolderResolutionMap keyed by a hash of the posix-normalized absolute path, maintained by FolderResolution::get_or_put) is only consulted on the is_workspace_dependency side of the Tag::Folder arm in PackageManagerEnqueue.rs. The transitive side never looks at it; it appends a fresh Package::default() stub per dependency edge (// these are always new), with no dependencies and scripts.filled = true. Two visible consequences, both with the released build:

The same folder as a root dependency and as a transitive one becomes two distinct packages, one fully resolved and one an empty stub:

# app depends on file:./vendor/lib and file:./vendor/util; lib depends on file:../util;
# util2 depends on file:../deep
"util2":     ["util2@file:vendor/util", { "dependencies": { "deep": "file:../deep" } }],
"lib/util2": ["util2@file:vendor/util", {}],

And because the stub never reads the folder's package.json, the third level of a file: chain is dropped entirely (nothing pointing at deep exists in the lockfile):

# app depends on file:./vendor/lib; lib depends on file:../util; util2 depends on file:../deep
"lib":       ["lib@file:vendor/lib", { "dependencies": { "util2": "file:../util" } }],
"lib/util2": ["util2@file:vendor/util", {}],

What routing the transitive side through the map would buy. For a dependency declared by a local file: package, get_or_put already has everything it needs: version.folder is top-level-dir-relative, so joining it onto the top-level dir gives the absolute key. That would collapse the duplicate into one fully resolved package and stop dropping the sub-dependencies, and this bug becomes much harder to hit (an un-conflicted transitive folder would then hoist into a workspace tree, where the installer already uses the right base). The one case it cannot cover is a file: path declared by an npm registry manifest, like file-dep@1.0.0's "files": "file:./the-files" in the test registry: Package::from_npm has no on-disk location to resolve against, so that string stays relative to the declaring package's install location, and the dirname(node_modules.path) base, plus treating a missing folder as nothing-to-link (which missing-file-dep pins), has to remain for it regardless.

I kept this PR at the installer because the enqueue change also changes the lockfile shape (the nested lib/util2 entry collapses into util2) and overlaps with the escape-check rework in #33106. Happy to do the get_or_put route here instead if you would rather take the structural fix, or follow up with it separately; let me know which you prefer.