Repository navigation
install: replace a file at a command's name behind a symlinked node_modules - #45477
Merged
Merged
Conversation
…odules pnpm writes the commands in `node_modules/.bin` as shell script files. Since #45307, a file or directory that has a command's name is kept when `node_modules` is a symlink, so `bun install` in a project that pnpm installed there failed with "failed to link <name>: EEXIST" for every command. A file is replaced by the link again, as in 1.4.3. A directory is still kept and still fails the install: no package manager puts one there, and removing it would delete a tree that may lie outside the project. The global bin directory is unchanged.
Collaborator
|
Updated 3:24 AM PT - Oct 16th, 2026
✅ @Jarred-Sumner, your commit 9ece36989e9a77e71e10d31e72320e9d2f6d9960 passed in 🧪 To try this PR locally: bunx bun-pr 45477That installs a local version of the PR into your bun-45477 --bun |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe linker now attempts to unlink a regular-file destination behind a symlink after an ChangesLinked bin destination handling
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly and concisely describes the main change: replacing a file at a command name behind a symlinked node_modules. |
| Description check | |
The description includes both required sections. It explains the behavior change, preserved directory behavior, unchanged cases, and detailed verification results. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
- Commit to this branch
- Create a new PR
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Beyond the inline finding, I also checked the sibling arms: the global arm still returns EEXIST unchanged, the non-linked path still uses delete_tree_absolute, and the Windows shim path (foreign_exe_exists at src/install/bin.rs:1205) is untouched. The narrowed it.each in the test loses no coverage: the dropped .bin/dep file case is now asserted with its new expectation in the added test, including the untouched-sibling check.
Extended reasoning...
The change is confined to one match arm in Linker::create_symlink in src/install/bin.rs plus a test rework; it alters what bun deletes behind a symlinked node_modules, which is the file-deletion surface the inline finding concerns. Sibling arms and the Windows path were read and found unchanged, and the test matrix was confirmed to keep both linkers and both folder positions for both the directory and file cases.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Where the link fails for another reason, a file system without symlinks for one, it would fail again after the file is gone.
springmin
pushed a commit
to springmin/bun
that referenced
this pull request
Oct 17, 2026
37 upstream commits: SQL consolidation (oven-sh#45304), bun lint/check/fmt rewrite (oven-sh#45595), b.bytes + bindgen Bun.* functions (oven-sh#45590), WebSocket closing handshake rewrite (oven-sh#45223), install/yarn.lock migration (oven-sh#45616), YAML.parse (oven-sh#45593), bake fixes (oven-sh#45605), file-behind-symlink installs (oven-sh#45477), WebKit bump 87ee10da087b (oven-sh#45567). Conflicts (14): src/install/{PackageInstall,PackageInstaller}.rs, src/jsc/bindings/c-bindings.cpp, src/uws_sys/libuwsockets.cpp + 10 tests. Resolution: upstream structure + re-applied OHOS patches (is_linked_folder package.json identity, OHOS ELF signing, is_executable_file access(X_OK), loop post handler) and the serialization policy (bun-audit/lifecycle/ catalogs: describe.concurrent/test.concurrent -> serial).
Jarred-Sumner
added a commit
that referenced
this pull request
Oct 18, 2026
### What does this PR do? Follow-up fixes for `bun install` and related commands after #44968, #45307, #45460, #45477 and #45616. Every fix has a test that fails without it, except the performance fix, which is measured. #### Regressions on `main` (not in any release) - **`bun ci` no longer fails when `package.json` changed but the installed packages didn't.** Since #44968, `--frozen-lockfile` failed on any difference between `package.json` and `bun.lock`: changing `~11.2.0` to `^11.2.0`, moving a dependency to `devDependencies`, or adding a dependency that another workspace already has. 1.4.3 allowed all of these, because the same packages get installed. 2 of the 51 projects in this repo that have a `bun.lock` would have failed CI after upgrading. It still fails when an edit adds a package, removes one, or changes a version. - **`bun pm ls` works without `node_modules` again.** Since #44968 it exited 1 with "node_modules not found, nothing to list". It now lists what's in `bun.lock` and exits 0, like 1.4.3. When `node_modules` exists, it still lists what's on disk. - **Removed `warn: registry requests failed, retrying`** (added in #45460). A retry that succeeds doesn't need a message, and one that fails ends in an error that explains why. `--verbose` still prints every retry. Two messages stay: the same warning after a timeout, where the install has been silent for minutes, and the wait time when a registry sends `Retry-After`. - **Stale nested packages could survive an upgrade from 1.4.3 forever** (hoisted linker, since #45307). When an install skipped some dependencies (`--production`, `--omit`, `--filter`, or an optional dependency for another platform), 1.4.3 nested packages at paths that `bun.lock` has no entry for, like `n/node_modules/b`. #45307 cleans those up, but it skipped the cleanup in five cases: - the project has an install script and scripts are disabled, e.g. `"prepare": "husky"` with `bun install --production --ignore-scripts` - `n` isn't in the cache, e.g. a CI or Docker layer that keeps `node_modules` but not the cache - `n` is trusted and has lifecycle scripts - `n` is a git, `github:` or `file:` dependency - `n` is inside the `node_modules` of a workspace that has an install script In those cases `n` kept loading `b@2.0.0` after `bun.lock` had moved to `b@2.1.0`. See "Behavior change to review" below. - **The isolated linker could link a bin from the wrong package.** When two direct dependencies have the same bin (`typescript` and `typescript7`, or `esbuild` and `esbuild-wasm`), the first one by name is supposed to win. If another package that sorts earlier had the first one as a peer dependency, like `msw` has `typescript`, `node_modules/.bin/tsc` pointed at the second one. - **Renaming a workspace deleted the packages in its `node_modules`** (hoisted linker). After changing a workspace's `"name"`, `bun install` removed `packages/two/node_modules/y`, so the workspace loaded the hoisted version until the next install. The cleanup now identifies a workspace by its folder instead of its name. - **`bun --filter="*" publish` published every workspace.** `bun --filter "*" publish` runs each workspace's `publish` script, so the `=` form doing something completely different was a trap. `--filter` and `--recursive` now only apply to `bun publish` when they come after `publish`. Otherwise it stops with an error that shows both spellings. - **`bunx` failed when the install cache was under a path that isn't valid UTF-8**, like a home directory named `/home/jos\xE9`. It installed the package and then reported "could not determine executable to run for package". #### Bug fixes - **A peer dependency could resolve to an unrelated package's out-of-range version.** `elysia` has a peer dependency on `@sinclair/typebox@>=0.34.0 <1`. In a project that also uses jest, `@jest/schemas` depends on `@sinclair/typebox@^0.27.8`. If that was the only version installed, elysia got it and failed to load. 1.4.3 printed a warning, and since #45307 nothing is printed. Installing elysia first and jest second gave each its own version, so the same `package.json` could produce two different lockfiles. Now, when no installed version satisfies a peer range, Bun doesn't fall back to a version that's only there as a private dependency of unrelated packages. It resolves the peer from the registry instead. Everything else works as before: - `react@17` in the project, a workspace, or any package above the library is still used by a library that wants `^18`. - A version that satisfies some other package's peer range stays shared. `@octokit/rest` and its plugins share one `@octokit/core`, and `type-graphql` and its helpers share one `graphql`. Hoisting a second copy above it would split them up. - Two peer dependencies that nobody provides still share one version. Limitations: - The fix doesn't apply if any other package in the project has a peer range that the old version satisfies. - A lockfile that already has the wrong version keeps it until the package with the peer dependency changes version (`bun update`, `bun add`) or is removed and re-added. - Optional peer dependencies still aren't resolved. They use the nearest copy in `node_modules`. With the hoisted linker, elysia's dependency `exact-mirror` still loads the old typebox if that's the hoisted one, which requires `@jest/schemas` itself to be in `package.json`. - Bun now requests a manifest that it didn't need before. If that request fails, `bun add` exits 1 and leaves `package.json` and `bun.lock` untouched, but the new package's folder is already in `node_modules`. - **`bun update --latest` narrowed a peer range that already allowed the latest version.** A library with `"solid-js": "catalog:"` in `devDependencies` and `"solid-js": ">=1.9.0"` in `peerDependencies` had the peer range rewritten to `^1.9.15`. The docs say such a range is left alone. The check was reading the `devDependencies` entry from the lockfile, which is a catalog reference, not a range. - **`bun update --latest` downgraded catalog entries that were ahead of `latest`**, whether a pin (`3.0.1`), a range (`^3.0.1`) or a dist-tag (`canary`, `next`), in the default catalog and named ones. #45307 only fixed this for entries in `dependencies`. - **`bun run --filter <pkg> deploy` kept going after `predeploy` failed.** It ran `deploy` and `postdeploy` anyway. The exit code was right, but the deploy had already happened. A failing script now skips the rest of its package's `pre`/main/`post` chain, like `bun run` without `--filter`, and the skipped scripts are shown as `Skipped`. Other packages still run. - **`bun install --use-system-ca` accepted the flag and ignored it.** Only `NODE_USE_SYSTEM_CA=1` worked. The flag now works for every package manager command, before or after the command name, and in `BUN_OPTIONS`. `--use-openssl-ca` and `--use-bundled-ca` are handled too, and passing more than one is an error, same as `bun run`. - **A new workspace wasn't linked if the same name and version was already installed** (hoisted linker). After copying `dep@1.0.0` into `packages/dep` and depending on it with `"workspace:*"`, `bun install` reported "no changes" and `node_modules/dep` stayed the registry copy. - **`bun prune` deleted bundled dependencies that `bun.lock` doesn't mark as bundled** (hoisted linker, since 1.4.0). This repo's `test/bun.lock` is one example: `bun prune` removed all 34 folders in `node_modules/aws-cdk-lib/node_modules`. It now also checks `bundleDependencies` in the installed package's `package.json`. - **`bun pm cache rm` and `bun pm cache prune` printed "Cleared" without deleting anything** when `~/.bunfig.toml` sets the cache directory. They now read that file. A project's `.env`, `bunfig.toml` and `.npmrc` still can't choose what gets deleted, and neither can `cache` in `~/.npmrc`, because that's npm's cache too. - **`bun pm fetch` downloaded nothing in a monorepo's Docker layer.** With `COPY package.json bun.lock ./` and `RUN bun pm fetch`, workspaces matched by a glob aren't on disk yet, so they were dropped along with their dependencies: "No packages to fetch", exit 0, and the later `bun install --offline` failed. It now keeps them, like `--frozen-lockfile` does. - **`bun pm fetch` looked up a missing workspace in the registry** when the root depends on it as `"lib": "*"` or `"^1.0.0"`. It either failed with a 404 or cached a public package with the same name. It now stops with the same error as a frozen install. - **`packageExtensions` stopped working after migrating from pnpm.** Editing or removing an extension had no effect, and `--frozen-lockfile` didn't notice. `bun.lock` records which dependencies came from an extension, but `pnpm-lock.yaml` doesn't distinguish them. The migration already fetches every manifest, so it now records any extension dependency that the manifest doesn't declare. - **`node-gyp` in a lifecycle script re-ran itself hundreds of times when `TMPDIR` wasn't valid UTF-8.** Bun's `node-gyp` shim runs `bun x node-gyp`, which skips the shim by comparing an environment variable against `PATH`. One of the two was lossily formatted, so they never matched. - **Output from a failed lifecycle script was garbled if it wasn't UTF-8.** Each invalid byte became U+FFFD, which made Latin-1, Shift-JIS or GBK compiler errors unreadable. The bytes are now printed as-is, like in 1.3.14. - **The `--frozen-lockfile` error now says which dependency changed**, for example `dependencies in package.json changed since bun.lock was saved: basic-1 was added`, `a-dep was removed in packages/app`, or `no-deps changed from ^1.0.0 to ^2.0.0`. - **Docs:** fixed two wrong statements on the auto-install page. The `bunfig.toml` closest to the importing file wins, not the one where you started `bun`. And auto-install is disabled by a `node_modules` above the importing file, not above the working directory. #### Performance - **Hoisted installs were quadratic in the number of nested `node_modules` folders**, even when there was nothing to do (1.4.3 too). Every time a folder finished, Bun scanned every other folder for installs that were waiting on it. It now keeps a count of waiting installs and skips the scan when it's zero. User-mode instructions for a no-op `bun install`, on CI's release builds: | Project | Before | After | |---|---|---| | 2,000 packages, no nested folders | 77.2 M | 77.2 M | | 500 packages, each with a nested `node_modules` | 53.5 M | 29.8 M | | 2,000 packages, each with a nested `node_modules` | 491.6 M | 114.9 M | #### Behavior change to review The stale-nested-packages fix changes which folders an install deletes. As before, a folder is only considered if 1.4.3's layout for this install would have put a package at that exact path, the current layout doesn't, and the package that owns the `node_modules` is still installed in the same place. What changed is the decision after that: | Case | `main` | This PR | |---|---|---| | A script could have created the folder (an install script that won't run again, or a trusted owner) | kept | removed | | The owner is a registry package that isn't in the cache | kept | removed | | The owner is a git dependency or `file:` folder | kept | removed, because Bun never installs their `node_modules` | | The owner is a `github:` dependency | kept | kept only if the cached archive has the folder | | The owner's cached tarball has the folder, or the owner is a tarball dependency | kept | kept | | A parent package has bundled dependencies | kept | kept | I don't know of any script that creates a folder named after a dependency at exactly the path where the old layout nested that dependency. Two tests from #45307 that pinned "such a folder is kept" are removed. What this could get wrong: a script that does exactly that, or a tarball that ships `node_modules/b` without listing `b` in `bundledDependencies` while also depending on `b`, in a project with no cache. The owner would then load the `b` that `bun.lock` resolved for it, which is within its declared range. ### How did you verify your code works? **Tests.** New or updated tests in `hoist`, `bun-install-registry`, `bun-lock`, `frozen-lockfile-outdated`, `frozen-lockfile-pruned`, `frozen-lockfile-member-pins`, `bun-pm`, `bun-pm-fetch`, `bun-prune`, `bun-publish`, `bun-install-retry`, `bun-install-lifecycle-scripts`, `bun-update-lockfile-sync`, `bun-update-transitive`, `bun-workspaces`, `bun-add-filter`, `bunx`, `isolated-install`, `pnpm-lock-v9` and `filter-workspace`. The 23 `--frozen-lockfile` tests for edits that don't change any package also pass on 1.4.3's install code. **Peer dependency fix, with the real packages.** Every install order, both linkers: | Scenario | 1.4.3 and `main` | This PR | |---|---|---| | `elysia` with `@jest/schemas` or `jest-extended`: loads and serves a request | 4 of 18 | 18 of 18 | | A project with only `ajv-keywords@3.5.2` and `ajv-formats@3.0.1`: `ajv-keywords` works | 1 of 5 | 5 of 5 | | 15 monorepo layouts that differ in which package declares `react`, checked with a real render: lockfiles with two copies of `react` | 0 of 30 | 0 of 30 | | `type-graphql` without `graphql` in `package.json`, then `bun add` a package that wants an older `graphql`: `buildSchema` works | yes | yes | **Peer dependency fix, on the 51 projects in this repo that have a `bun.lock`,** compared with `main`: - A fresh resolve produces an identical lockfile in 47 of the 48 projects that resolve. The other one is `test/`, where elysia now gets a matching typebox. - `install`, `ci`, `add`, `remove` and `update` on the checked-in lockfiles produce identical lockfiles in 239 of 239 runs. - In 84 `bun add` runs chosen so the new package's peer range doesn't match what's installed, 67 produce identical lockfiles. In the other 17, the new package gets a version within its range. - In all of these, no peer dependency that was within range on `main` moved to a different version, and no regular dependency resolved differently. - 1.4.3 and `main` can read the lockfiles this PR writes for the two elysia projects without modifying them, and elysia works. **Upgrade from 1.4.3.** 996 random projects nested up to 7 levels deep, installed with 1.4.3's install code, then updated and reinstalled over the old tree. Number of projects where a package ends up loading a version outside its declared range: | Project | 1.4.3 (299 projects) | `main` | This PR | |---|---|---|---| | Has a `prepare` script, installed with `--ignore-scripts` | 0 | 50 | 0 | | No scripts | 0 | 0 | 0 | | Has a `prepare` script that runs | 0 | 0 | 0 | | No scripts, a quarter of the packages bundle a dependency | 0 | 5 | 5 | | `prepare` script, `--ignore-scripts`, and bundled dependencies | not run | 49 | 5 | The remaining 5 are inside packages with bundled dependencies, which the cleanup skips entirely, same as `main`. **CI.** Linux (including ASAN), Windows and macOS arm64 passed on `610f3ca95`. The commits after it merge `main` and reword comments, test names and three messages. The macOS x64 test jobs never got a machine. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
bun installworks again in a project that pnpm installed and whosenode_modulesis a symlink.pnpm writes the commands in
node_modules/.binas shell script files. Since #45307, a file or directory that has a command's name is kept whennode_modulesis a symlink, because what is behind the link need not be bun's. So every command of such a project failed the install:<name>.exeand<name>.bunxthere, and pnpm writes neither.There is no extra system call:
unlinkfails on a directory, which tells the two cases apart. The file is only replaced when the link failed because the name is taken. After any other failure, the file stays and the original error is reported.How did you verify your code works?
symlink-path-traversal.test.tsis split in two, each for both linkers and for thenode_modulesof the project and of a workspace. A directory in the way is kept and fails the install, as before. A file gives way to the link, and every other file behind the symlink is untouched, including another file in.bin.main, the 4 file cases fail with the error above and the 4 directory cases pass.9ece36989, all 8 cases pass.maintoo. 6 tests ofbun-pm-diff.test.tshit their 5 s timeout on the loaded machine; run alone, that file passes 75 of 75 on this build and onmain's.mordantcheck fails onJSPromiseStrong::from_valueinsrc/jsc/JSPromise.rs, which this PR doesn't touch: 4b1681a onmainremoved its only caller, and the check judges the PR merged withmain.