install: fail the isolated install when a link: dependency's target is missing

[robobun]

Problem

• With linker = "isolated", bun install of a link:<name> dependency whose bun link registration no longer exists (the package was bun unlinked, or its directory was deleted) creates a dangling node_modules/<name> symlink, prints Done! Checked N packages (no changes) and exits 0. The hoisted linker fails the same state with FileNotFound: failed linking dependency/workspace to node_modules for package <name> and exits 1.
• Only reproduces when the dependency is already in the lockfile (a fresh resolution reads the linked package.json and fails with Package "<name>" is not linked). That is the common case: a clone with bun.lock checked in installs green with a broken node_modules.
• Cause: src/install/isolated_install.rs marks ResolutionTag::Symlink store entries Done without touching the filesystem ("no installation required"). The node_modules/<name> link is created later by each dependent's SymlinkDependencies step (isolated_install/Installer.rs, append_store_path), which builds the target path and symlinks to it without checking that it exists. Nothing in the isolated path ever opens the link target, so there is nothing to fail.

Fix

• In the Symlink arm of the main-thread install loop, build the entry's target path with append_store_path (the same path the dependents will point their symlinks at) and openat(O_RDONLY | O_DIRECTORY) it. On success the entry completes as before; on failure it goes through on_task_fail(TaskError::LinkPackage(err)), which prints the error, counts the entry in summary.fail (Failed to install 1 package, exit 1) and lets the install continue for the other entries.
• Why this is correct:
  • openat(O_RDONLY | O_DIRECTORY) is the exact call PackageInstall::install_from_link makes for the hoisted linker, so the two linkers now fail on the same set of states (missing entry, dangling symlink or junction, entry that is a file, unreadable entry) with the same errno. It follows symlinks and junctions, so a registration left dangling by deleting the package directory fails too, not just a removed one.
  • Symlink entries have no store directory and no task; this arm is the only place in the isolated install that looks at a link: package, so it is the only layer that can fail the install for it. Checking in each dependent's SymlinkDependencies step instead would attribute the error to the dependent and, for the root entry, abort linking of all its other dependencies.
  • Using append_store_path from init_top_level_dir() checks exactly the path the dependents link to rather than a second hand-built one, so it stays correct if the Symlink target computation changes (for example path-form link: targets in install: support path-form link: dependencies #35461).
  • Cost is one open/close per link: package on the main thread, which already does an exists probe per npm entry in the same loop.
• What changes is the report and the exit code, not the node_modules end state: the dependents still create (or keep) their node_modules/<name> symlink after the failure, which is how every failed isolated entry is handled today (the install reports the failure and exits 1, the next install retries; hoisted differs here because install_from_link removes the old entry first). bun linking the package again makes the next install succeed without removing node_modules. Removing dependents' links on failure would be a change to on_task_fail/SymlinkDependencies for every failure kind, not something specific to link:.
• Verified with test/cli/install/isolated-install.test.ts (link: dependencies): two cases, a plain name removed with bun unlink and a scoped name whose directory was deleted. Each case first installs successfully, then asserts that reinstalling over the existing node_modules and installing into a fresh one both print ENOENT + failed to link package: <name>@link:..., Failed to install 1 package, and exit 1. The bun link registration is pointed at a per-test BUN_INSTALL so the tests do not touch the real global link dir.
  • Fails on the current release (USE_SYSTEM_BUN=1): both installs exit 0 with empty stderr.
  • Passes with bun bd test.

Background

• link:<name> dependency: bun link run inside a package registers it by creating <global dir>/node_modules/<name> (a symlink on POSIX, a junction on Windows) pointing at the package directory. A consumer declaring "<name>": "link:<name>" gets a node_modules/<name> symlink to that registration. The lockfile stores only the name (Resolution.symlink), so on later installs nothing re-reads the linked package.
• Isolated linker store entries: install_isolated_packages creates one store entry per package in the dependency graph and, on the main thread, either starts a worker task for it (npm, git, tarball, folder: materialize into node_modules/.bun/<store path>) or completes it immediately. Each entry's task then runs SymlinkDependencies, which symlinks its dependencies' store paths into its own node_modules; for the root entry that is the project's node_modules/<dep>.
• on_task_fail is the shared failure path for an entry: it prints the TaskError, increments the install summary's fail counter (which drives the Failed to install N packages line and the exit code) and releases the entry's pending-task slot so the install loop can finish.

---

no test proof · iteration 0 · 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]

Warning

Review limit reached

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

Next review available in: 16 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: 7338ca2e-a7ea-408f-93d6-5f5e28121dad

📥 Commits

Reviewing files that changed from the base of the PR and between 05dd45e and 5dbf7e1.

📒 Files selected for processing (2)

• src/install/isolated_install.rs
• test/cli/install/isolated-install.test.ts

---

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

[robobun]

Status: ready for review.

Reproduced on the released build (1.4.0-canary da3851e): bun link a package, bun install it as link:<name> with linker = "isolated", bun unlink it (or delete its directory), rm -rf node_modules, bun install again prints Done! Checked 2 packages (no changes) and exits 0 with node_modules/<name> dangling. Hoisted fails the same state with FileNotFound and exit 1.

With this branch the same sequence prints ENOENT: No such file or directory: failed to link package: <name>@link:<name> (open) and Failed to install 1 package, exit 1; bun linking the package again makes the next install succeed.

Test: test/cli/install/isolated-install.test.ts (link: dependencies), fails with USE_SYSTEM_BUN=1, passes with bun bd test.

[claude]

LGTM — small, well-scoped parity fix bringing the isolated linker in line with hoisted for missing link: targets.

What was reviewed:

• Verified global_link_dir_path is ensured on the main thread (isolated_install.rs:2080-2087) before the new append_store_path call reaches the Symlink arm's debug_assert.
• Confirmed on_task_fail decrements pending tasks and handles ResolutionTag::Symlink as a no-op for store cleanup, so the install loop terminates and nothing spurious is deleted.
• Checked the test isolates BUN_INSTALL per case, drains stdout/stderr concurrently, and covers both bun unlink and directory-deleted (dangling registration) with plain and scoped names.

Extended reasoning...

Overview

This PR touches two files: a ~22-line change to the ResolutionTag::Symlink arm of install_isolated_packages in src/install/isolated_install.rs, and a new describe block in test/cli/install/isolated-install.test.ts. Previously, symlink (link:) entries were marked Done and completed as Skipped unconditionally. Now the target path is built via the existing append_store_path helper and probed with sys::openat(O_RDONLY | O_DIRECTORY) — the same call the hoisted linker's install_from_link uses. On success the fd is closed and the entry completes as before; on failure the entry is routed through on_task_fail(TaskError::LinkPackage(err)).

Security risks

None. The path being opened is derived from the lockfile's Resolution.symlink name joined onto the global link directory — the same path the isolated linker already symlinks dependents to today, and the same path the hoisted linker already opens. No new untrusted input is introduced; this is purely an existence probe that turns a silent success into a reported failure.

Level of scrutiny

Medium-low. The change is confined to one match arm on the main thread, reuses existing helpers (append_store_path, on_task_fail, TaskError::LinkPackage, AbsPath::init_top_level_dir, sys::openat), and follows the file's local conventions (use bun_sys::FdExt as _; matches lines 1719/1917; AbsPath::init_top_level_dir() matches line 2227). I traced the preconditions: global_link_dir_path is lazily ensured at lines 2080-2087 before this loop begins, so the debug_assert!(!symlink_dir_path.is_empty()) inside append_store_path's Symlink branch cannot trip. on_task_fail calls decrement_pending_tasks() + resume_unblocked_tasks(), so the pending-task accounting stays balanced whether the openat succeeds or fails. The step is stored as Done before the probe, so dependents' is_task_blocked checks are unaffected on the failure path (matching the PR description's stated intent that dependents still create their symlink and the next install retries).

Other factors

The test coverage is thorough: test.concurrent.each over two removal modes (bun unlink removing the registration entry; rm -rf leaving it dangling) × plain and scoped names, each asserting the success-then-fail sequence for both reinstall-over-existing and fresh-install-from-lockfile. Each case gets its own tempDir and per-test BUN_INSTALL, so concurrent runs don't touch the real global link dir or each other. The runBun helper drains stdout/stderr/exited concurrently. Assertions check for the specific error text (ENOENT, failed to link package: <name>@link:, Failed to install 1 package) and exit code 1. The PR description states USE_SYSTEM_BUN=1 fails and bun bd test passes. No outstanding reviewer comments; the only timeline entry is a CodeRabbit rate-limit notice.

[robobun]

^{Updated 5:05 AM PT - Aug 13th, 2026}

❌ @robobun, your commit 5dbf7e1 has some failures in Build #94594 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38045

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

bun-38045 --bun

[claude]

LGTM — focused parity fix bringing the isolated linker in line with the hoisted linker for missing link: targets.

Reviewed the ResolutionTag::Symlink arm change: append_store_path clears the buffer and builds the absolute global-link-dir path, global_link_dir_path is ensured on the main thread before this loop runs, and on_task_fail correctly no-ops the store-deletion step for Symlink entries while still decrementing pending tasks (so the install loop terminates). The openat(O_RDONLY | O_DIRECTORY) matches PackageInstall::install_from_link exactly. Tests are hermetic (per-test BUN_INSTALL), drain stdout/stderr concurrently, and cover both unlink and dangling-directory cases with scoped and unscoped names.

Extended reasoning...

Overview

This PR fixes a silent-success bug in the isolated linker: when a link:<name> dependency in the lockfile no longer has a valid bun link registration (unlinked or directory deleted), bun install with linker = "isolated" would exit 0 and leave a dangling node_modules/<name> symlink. The hoisted linker already fails this case with exit 1.

The fix adds ~19 lines to the ResolutionTag::Symlink arm of the main-thread install loop in src/install/isolated_install.rs: before marking the entry done, it builds the same target path the dependents will symlink to (via append_store_path) and opens it with O_RDONLY | O_DIRECTORY. On failure it routes through the existing on_task_fail(TaskError::LinkPackage(err)) handler. A new describe("link: dependencies") block in test/cli/install/isolated-install.test.ts covers both removal modes.

Security risks

None. This adds a read-only openat on a path derived from the lockfile and the global link directory, both of which are already trusted inputs. No new attack surface.

Level of scrutiny

Medium. The package manager install path is production-critical, but this change is narrow: it only affects link: dependencies under the isolated linker, and only converts a silent-success into the same failure the hoisted linker already produces. I traced the surrounding invariants:

• global_link_dir_path is lazily ensured earlier in install_isolated_packages (line ~2084) before any entry is processed, so the debug_assert!(!symlink_dir_path.is_empty()) in append_store_path holds.
• append_store_path for Symlink calls buf.clear() first, so the init_top_level_dir() seed is irrelevant — the result is the absolute global-link path.
• on_task_fail explicitly handles ResolutionTag::Symlink as a no-op in the store-cleanup match, increments summary.fail, and calls decrement_pending_tasks() + resume_unblocked_tasks(). Since pending tasks are incremented upfront by store.entries.len(), the loop still terminates on the failure branch.
• The openat flags match PackageInstall::install_from_link at src/install/PackageInstall.rs:2104 exactly, so both linkers fail on the same set of states (missing, dangling symlink/junction, not-a-directory).

Other factors

• Tests follow REVIEW.md conventions: test.concurrent.each, per-test BUN_INSTALL for hermeticity, Promise.all on stdout/stderr/exited, stderr assertions before exit-code assertions, tempDir from harness. The PR states both cases fail with USE_SYSTEM_BUN=1 and pass with bun bd test.
• No CODEOWNERS entry covers src/install/.
• The comment-cop bot flagged comment length twice; both threads are resolved and the final comment is one line pointing at the hoisted counterpart.
• No design decision here — this is bringing one linker into parity with the other on an unambiguous failure case.