install: read the package.json of a file: dependency declared by a local file: package

[robobun]

Problem

• A file: dependency declared by another local file: package is recorded in the lockfile as a bare row ("a/b": ["b@file:vendor/b", {}]): no bin, no dependencies. Both linkers link bins from the lockfile's bin field, so no .bin entry is created, and the package's own dependencies are never resolved or installed, while bun install exits 0. The same folder declared by the root gets a full row and works; npm links bins and installs dependencies of file: packages at any depth.
• Found by reading the code while working on another file: dependency bug (install: make bins of symlink-installed packages executable #38777), not from a user report. The inconsistency was already visible in the existing tests: a fresh resolve wrote stub rows for a/b, while reloading the same project from bun.lock produced full rows (installs file: dependencies that depend on each other vs its from a lockfile twin).
• Cause: the Tag::Folder arm of get_or_put_resolved_package (src/install/PackageManager/PackageManagerEnqueue.rs) only reads the folder's package.json (FolderResolution::get_or_put) when the dependency is declared by the root or a workspace. Every other declarer gets a stub Package holding a name and a resolution. The stub exists for registry packages that declare file: dependencies (fix(install): handle transitive folder dependencies #10445): their paths are relative to the declaring package, which is not on disk until it is installed. A local file: package is not in that situation: Package::parse already rewrote its file: paths relative to the top-level dir (the same base as the root's own file: dependencies, and the base the installer uses for them since install: link a transitive file: dependency of a local file: package #33159), and the target folder is on disk while resolving.

Repro (no network)

mkdir -p app/vendor/a app/vendor/b
echo '{"name":"a","version":"1.0.0","dependencies":{"b":"file:../b"}}' > app/vendor/a/package.json
echo '{"name":"b","version":"1.0.0","bin":{"bcli":"cli.js"}}' > app/vendor/b/package.json
printf '#!/usr/bin/env node\nconsole.log("hi")\n' > app/vendor/b/cli.js
echo '{"name":"app","dependencies":{"a":"file:./vendor/a"}}' > app/package.json
cd app && bun install && cat bun.lock && find node_modules -name .bin

Before (hoisted and isolated, bun 1.4.0 and main): "a/b": ["b@file:vendor/b", {}] and no .bin directory anywhere. A dependency added to b is not installed either.

After: "a/b": ["b@file:vendor/b", { "bin": { "bcli": "cli.js" } }] plus node_modules/a/node_modules/.bin/bcli (hoisted) or node_modules/.bun/a@file+vendor+a/node_modules/.bin/bcli (isolated).

Fix

• resolution::Tag::is_local_package() (root, workspace, folder) names the local/remote split that Tree.rs and verify_resolutions each spelled out as an inline match; both now call it. Lockfile::get_parent_pkg_of_dependency and Lockfile::is_dependency_of_local_package apply it to the package that declares a dependency. These are the helpers install: only constrain transitive file: targets of remote packages #33106 (open, the sibling change to this block) defines; they are added here in its shape so it reduces to consumers of them.
• The Folder arm reads the folder from disk through the existing get_or_put path when is_dependency_of_local_package holds, and falls back to the stub only for registry-declared dependencies. The .. escape check for dependencies not declared by the root or a workspace is unchanged and still runs first; whether local declarers should be exempt from it is install: only constrain transitive file: targets of remote packages #33106's question, and its "a folder declarer is always root-declared" argument becomes "a folder package with a dependency list is only ever reached through local packages" (registry-declared folders get no dependency list), which is now stated on the helper.
• Why this is correct: the only thing the stub works around is the path base, and a local package's dependency paths have the same base as the root's, so the dependency goes through exactly the code that resolves root file: dependencies today. That brings the same dedupe with it (get_or_put caches by absolute path and reuses a package with the same resolution, so a folder reached from both the root and a file: package is one package instead of a package plus a stub), and the folder cycle guard in Tree.rs (install: pnpm parity — dedupe, prune, pm licenses, audit fix, add --filter/--catalog, nested overrides, transitive update, and workspace fixes #38333, added for the lockfile reload and migration cases, where these rows already arrive as full packages) keeps a -> b -> a from nesting forever. has_install_script now comes from the package.json, so such a package's scripts are blocked and listed by bun pm untrusted like any dependency-declared git or tarball package's, instead of being dropped unrecorded; the trust rules are untouched.
• What a user sees change, on fresh resolution only (rows already in a lockfile are not re-resolved, so existing lockfiles do not change by themselves):
  • the package's bins are linked into the declaring package's .bin under both linkers. The isolated linker also makes them executable. The hoisted linker installs a folder package at this depth as per-file symlinks, and its bin chmod deliberately does not follow symlinks (lchmod), so there the link runs only if the vendored script is already executable; install: make bins of symlink-installed packages executable #38777 is the fix for that half (its installed_from logic covers these packages as is, since their resolutions are root-relative like the root-declared ones it was written for). The new test makes the scripts executable up front and runs both links under both linkers.
  • the package's dependencies are resolved. Under both linkers a registry dependency of such a package is installed and resolvable from its code (hoisted puts it in the root node_modules, which the package's real location under vendor/ can see). Under the hoisted linker only, a dependency that cannot be hoisted (for example the package's own file: dependency) ends up in the nested node_modules bun creates for the package, which its code, resolved from its real location, does not see: the hoisted layout never hoists folders. Previously nothing of the package's was installed at all; the isolated linker handles every case.
  • a missing target folder fails while resolving, with the same error as a missing root file: dependency, instead of writing the lockfile and failing during install. The install-time error from install: link a transitive file: dependency of a local file: package #33159 still covers a lockfile whose folder disappeared later.
  • a nested override pointing a folder package's dependency at a file: path records the normalized path (shared@file:vendor/shared), as a root dependency would, instead of the literal (shared@file:./vendor/shared).
  • a local tarball declared by such a package now fails the same way one declared by a root file: package already does (enqueue_local_tarball resolves it against the project root; tracked separately, it reproduces on the released build at depth one without this change). install: keep file: folder paths declared by git and tarball packages relative to the package #38816 covers file: folders declared by git and tarball packages, which this PR does not touch.
• Verified with:
  • test/cli/install/bun-install.test.ts: new hoisted:/isolated: nested file: dependencies are installed with their dependencies and bins (root -> lib -> tool, where tool has a bin, a file: dependency with a bin and a file: devDependency; asserts the lockfile, both .bin directories and the layout, runs both bins through their links on POSIX, then repeats everything from --frozen-lockfile and checks the lockfile is byte-identical), new fails to resolve when a transitive file: dependency's folder does not exist plus a lockfile variant keeping the old install-time assertion, and the two snapshot updates described above. All of the new and updated tests fail on the current build.
  • test/cli/install/bun-install-registry.test.ts: new hoisted:/isolated: a file: dependency of a local file: package is resolved like a root one: the nested package depends on a registry package; asserts the lockfile, the bin, and that require("lib") reaches the registry package at runtime, fresh and from the lockfile. Fails on the current build (2 packages installed, the registry package is never fetched).
  • No changes elsewhere in bun-install.test.ts (its remaining failures here are the bitbucket/gitlab/external-URL tests, which need network), bun-install-registry.test.ts (file|folder subset and the whole transitive file dependencies block, which pins that registry-declared file: dependencies still get stubs), isolated-install, bun-lock, bun-workspaces, bun-prune, bun-dedupe, overrides, nested-overrides, config-version, lockfile-only, lockfile-version-2 and the lockfile migration suites.

Background

• A file: dependency on a directory resolves to Resolution::Folder, whose payload is the directory as a path string. Two parsers produce these strings: Package::parse (used for the root, workspaces and local file: packages) stores the path relative to the project's top-level dir; Package::from_npm (registry manifests) stores the literal, which is relative to the declaring package.
• FolderResolution::get_or_put (src/install/resolvers/folder_resolver.rs) reads a folder's package.json into a full lockfile Package (name, bin, scripts, dependency list), caches the result per absolute path, and reports a new package so the resolver enqueues that package's dependency list.
• Both linkers decide whether a package has bins to link from the lockfile Package's bin field (PackageInstaller.rs for the hoisted layout, isolated_install/Installer.rs for the isolated one), so a package that reaches them as a stub has nothing to link. After linking, the bin linker sets the executable bit on the target with lchmod, which does not follow symlinks, so that a package cannot aim the chmod at an arbitrary file through a symlink it ships.
• Folder packages are never hoisted: the tree builder places them in the node_modules of the package that declared them, which is why the bins land in the declaring package's .bin and the lockfile keys look like a/b or lib/tool/helper. The hoisted linker materializes a folder package in a nested position as a real directory containing one symlink per file; in the root position it hardlinks or copies the files.

Earlier revisions

• The first push added a dedicated Lockfile::is_dependency_of_folder_package scan and combined it with is_workspace_dependency at the call site. Review pointed out that this was a fourth inline spelling of the local/remote declarer split and the same predicate install: only constrain transitive file: targets of remote packages #33106 introduces, so it was replaced with the helpers described above; behavior is identical.
• The second push only read the .bin links back. Review pointed out that under the hoisted linker a linked bin is not necessarily runnable (the lchmod point above), so the test now makes the scripts executable and runs the links, and the description states that limitation and install: make bins of symlink-installed packages executable #38777 explicitly.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 17 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9838bc35-4fc9-417b-960a-6ee60843a9b8

📥 Commits

Reviewing files that changed from the base of the PR and between 88a6398 and 113269f.

📒 Files selected for processing (7)

• src/install/PackageManager/PackageManagerEnqueue.rs
• src/install/PackageManager/PackageManagerResolution.rs
• src/install/lockfile.rs
• src/install/lockfile/Tree.rs
• src/install/resolution.rs
• test/cli/install/bun-install-registry.test.ts
• test/cli/install/bun-install.test.ts

---

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

[robobun]

^{Updated 11:05 AM PT - Aug 15th, 2026}

❌ @robobun, your commit 113269f has some failures in Build #98239 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38814

That installs a local version of the PR into your bun-38814 executable, so you can run:

bun-38814 --bun

[robobun]

Status: reproduced on bun 1.4.0 and main with both linkers using the repro in the description ("a/b": ["b@file:vendor/b", {}], no .bin entry, and a dependency added to b is not installed). Fix is in this PR, rebased on current main; the new tests in test/cli/install/bun-install.test.ts and test/cli/install/bun-install-registry.test.ts fail on the current build and pass with the change.

CI on 1ee9393: every lane that runs the install suites passed. The one red lane is Windows 2019 x64, where test/bake/deinitialization.test.ts segfaults at dev-server teardown; that test is not touched by this change and has been reported separately. The two macOS 14 aarch64 test lanes have been waiting for an agent since the build started.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes package-manager resolution behavior for transitive file: dependencies — with observable lockfile-format changes and interactions with cycle handling, overrides, and install-script trust — a human look would still be worthwhile.

Checked that the reordered bin_target_escapes_package_dir guard preserves the workspace bypass (!is_workspace_dependency prefix) and still runs before folder-declared deps reach get_or_put.
Checked that is_dependency_of_folder_package mirrors get_workspace_pkg_if_workspace_dep's scan shape and that a registry-declared file: dep (whose declarer's tag is not Folder) still falls through to the stub path.
Checked the two updated snapshots against the description's stated observable changes (normalized override path, cycle rows now carry deps).

Extended reasoning...

Overview

The PR extends get_or_put_resolved_package's Tag::Folder arm in src/install/PackageManager/PackageManagerEnqueue.rs so that a file: dependency declared by another local file: package is resolved via FolderResolution::get_or_put (reading its package.json for bin/scripts/dependencies) instead of being recorded as a bare stub. It adds Lockfile::is_dependency_of_folder_package in src/install/lockfile.rs (a linear scan over packages, structurally identical to get_workspace_pkg_if_workspace_dep), reorders the existing ..-escape check to run before the new branch while keeping the workspace bypass, and rewrites the stub-path comment to name what still reaches it (registry-declared file: deps). Tests add hoisted+isolated coverage in both bun-install.test.ts and bun-install-registry.test.ts, split the missing-folder test into resolve-time and lockfile-time variants, and update two snapshots.

Security risks

The change reads additional package.json files from disk during resolution and now records has_install_script for nested folder packages (surfacing them in bun pm untrusted rather than silently dropping their scripts). The bin_target_escapes_package_dir check is preserved for non-workspace declarers and runs before the new full-resolve branch, so the existing path-escape gate is not weakened. I don't see a new injection or escape surface, but the interaction between trust (install scripts becoming visible), overrides now taking the normalizing path, and the .. guard is subtle enough that a maintainer familiar with #33106/#10445 should confirm the intended trust boundary.

Level of scrutiny

High. Package-manager resolution is a critical, widely-exercised path, and this PR intentionally changes observable behavior in three documented ways: missing nested folders now fail at resolve time, nested-override folder paths are normalized in the lockfile, and the mutual-file:-dep lockfile rows now carry dependencies. The reasoning about path bases (Package::parse rewriting to top-level-relative vs. Package::from_npm keeping declarer-relative) is the load-bearing invariant, and correctness of the cycle case leans on the Tree.rs guard from #38333. These are design-level decisions a maintainer should sign off on.

Other factors

Test coverage is thorough: both linkers, fresh-resolve and --frozen-lockfile passes with byte-identical lockfile assertion, bin linking at two nesting depths, a runtime require reaching a registry dep of the nested package, and negative coverage for the missing-folder case in both resolve and install-from-lockfile forms. The description enumerates the unaffected suites. No prior human review on the thread; CI is still building. Given the scope, deferring rather than approving.

[robobun]

On the install-script trust point raised above, for whoever picks this up: the trust rules themselves are untouched. has_trusted_dependency still only trusts a non-registry package when the root or a workspace declares it (declared_by_root_or_workspace), so a file: package that is only reachable through another file: package now gets its scripts recorded and blocked (it shows up in bun pm untrusted, and bun pm trust runs them on demand) exactly as a git or tarball package declared by a dependency does. Before this change those scripts were dropped without being recorded at all. Root-declared file: packages behave as before, including with trustedDependencies; I checked both cases by hand on this branch and on the released build.

[robobun]

Pushed a revised version (1ee9393), rebased on current main:

• The call site now uses one named predicate instead of a second ad hoc scan: resolution::Tag::is_local_package() plus Lockfile::get_parent_pkg_of_dependency / is_dependency_of_local_package, in the shape install: only constrain transitive file: targets of remote packages #33106 defines them, and the two places that spelled the same root/workspace/folder split out inline (Tree.rs, verify_resolutions) call it too. The escape check is unchanged. Behavior is identical to the first push; the description is updated to match, and now also spells out the hoisted-linker limitation for dependencies of such a package that cannot be hoisted, and the related local-tarball path issue that is tracked separately.
• The inline bot comments were about doc comments; two were removed or shortened, the rest are the one to three line definitions the helpers need, replied to individually.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes package-manager resolution semantics for nested file: dependencies (what gets written to the lockfile, what gets installed, when a missing folder fails) and overlaps with the open #33106's design question around the escape check, a human look would still be worthwhile.

What was reviewed:

• The Folder arm reordering in get_or_put_resolved_package: the .. escape check still runs first and still gates on is_workspace_dependency, so folder-declared folders remain subject to it.
• is_local_package() refactor in Tree.rs and verify_resolutions — behavior-preserving (same three tags as before).
• get_parent_pkg_of_dependency uses ExternalSlice::contains (range check on off..off+len), matching how get_workspace_pkg_if_workspace_dep scans.
• Trust rules unchanged: has_trusted_dependency still keys on root/workspace declarers, so nested folder scripts are recorded and blocked, not auto-trusted.

Extended reasoning...

Overview

The PR changes the Tag::Folder arm of get_or_put_resolved_package so a file: dependency declared by a local file: package is read from disk via FolderResolution::get_or_put (like a root/workspace-declared one), instead of being recorded as a stub with no bin and no dependency list. It introduces resolution::Tag::is_local_package() and Lockfile::{get_parent_pkg_of_dependency, is_dependency_of_local_package}, and rewrites two existing inline Root|Workspace|Folder matches (Tree.rs, verify_resolutions) to call the new predicate. Tests cover both linkers, fresh resolve + --frozen-lockfile, the moved missing-folder error, and a registry dependency of a nested file: package resolved at runtime.

Security risks

The .. escape check is preserved and moved to run before the new branch; it still gates on is_workspace_dependency (root/workspace only), so folder-declared folders are not newly exempt from it. Install-script trust is unchanged (declared_by_root_or_workspace), so a nested folder package's scripts are now recorded and blocked rather than silently dropped — a strict improvement. I did not identify a way for this change to read or install from a path the previous code would have rejected.

Level of scrutiny

High. This is a semantic change to what bun install resolves and writes to bun.lock: nested file: packages now contribute dependencies, bins, and has_install_script; a missing target folder now fails during resolve instead of during install; a nested override's path is now normalized. The description also acknowledges a known hoisted-linker limitation (non-hoistable deps of such a package land in a nested node_modules the real folder cannot see) and defers a related local-tarball path issue and the escape-check scope question to #33106/#38816. Those are design trade-offs a maintainer should sign off on.

Other factors

Test coverage is thorough (variant matrix over linker, fresh vs. frozen, negative case with and without a lockfile, runtime require reaching a registry dep), the two updated snapshots are the expected consequence of the fix, and the folder cycle guard in Tree.rs is already in place to keep a → b → a from nesting forever now that both directions carry a dependency list. No prior claude[bot] review exists on this PR.

[robobun]

Pushed 113269f: same code as before; the nested-bins test now makes the two scripts executable and runs both .bin links under each linker, instead of only reading the links back. Reason: with the hoisted linker these packages are installed as per-file symlinks and the bin linker's lchmod stops at bun's own symlink, so a linked bin is only runnable if the vendored script already has the executable bit. That half is #38777; the description now says so and spells out what each linker does with the new rows.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes package-manager resolution behavior (when a nested file: folder is read from disk vs. stubbed), reorders the .. escape check, and shifts a failure from install-time to resolve-time, a human look would still be worthwhile.

What was reviewed:

• The reordered escape check in the Tag::Folder arm — it still runs before get_or_put and still exempts root/workspace declarers and overrides.
• is_dependency_of_local_package vs. the previous is_workspace_dependency gate — registry-declared folder deps still hit the stub path.
• The folder cycle guard in Tree.rs — it covers the new full-row cycle case (a/b and b/a now carry dependency lists).
• Install-script trust — has_trusted_dependency still gates on root/workspace declarers, so newly recorded scripts are blocked, not auto-run.

Extended reasoning...

Overview

This PR changes the Tag::Folder arm of get_or_put_resolved_package so that a file: dependency declared by another local file: package is resolved by reading the target folder's package.json (via FolderResolution::get_or_put), the same as one declared by the root or a workspace. Previously such a dependency was recorded as a stub row with no bin and no dependency list. Supporting changes: a new resolution::Tag::is_local_package() predicate (root/workspace/folder), new Lockfile::get_parent_pkg_of_dependency / is_dependency_of_local_package helpers, and two existing inline root/workspace/folder matches (Tree.rs, verify_resolutions) rewritten to call the predicate. The pre-existing .. escape check is moved above the read-from-disk branch so it runs first. Tests cover both linkers, fresh resolve and --frozen-lockfile, bin linking, a registry dependency of the nested package, and the missing-folder error at both resolve time and install time.

Security risks

The change touches two security-relevant paths. First, the bin_target_escapes_package_dir guard: the diff moves it earlier without weakening it — non-root/workspace declarers (which now includes folder-declared deps that are about to be read from disk) still hit the check before any filesystem read, and the override exemption is unchanged. Second, install-script trust: nested file: packages now record has_install_script from their package.json, but has_trusted_dependency still keys on declared_by_root_or_workspace, so those scripts are blocked and surfaced by bun pm untrusted rather than auto-run. Both look correct, but they are exactly the kind of subtle interaction a maintainer should confirm.

Level of scrutiny

High. This is a user-visible behavioral change to bun install's resolution logic and lockfile output, in production-critical code. It moves a failure from install-time to resolve-time, changes what gets written to bun.lock for nested file: rows, and interacts with an open sibling PR (#33106) whose helper shape it adopts. The description also names known limitations (hoisted-linker nested node_modules visibility, a local-tarball path issue tracked separately as #38816) that a maintainer should agree are acceptable to ship around.

Other factors

The test coverage is thorough (both linkers, lockfile round-trip, runtime require, missing-folder both fresh and from lockfile), and CI on the install suites passed on every lane that ran; the one Windows failure is an unrelated bake/deinitialization segfault. All comment-cop threads are resolved. No prior human review is on the thread. Given the scope and the security-adjacent surface, this should not land on an automated approval alone.