install: bind an unsatisfiable peer to the copy bun.lock nests under its dependent - #38992
install: bind an unsatisfiable peer to the copy bun.lock nests under its dependent#38992robobun wants to merge 1 commit into
Conversation
…its dependent Loading bun.lock binds a ranged peer by version; when no version in the lockfile satisfies the range it took the highest one. 1.3.x bound every peer by path, so its lockfiles can nest a copy under such a dependent, and that is the only placement the hoister produces for a binding nothing enclosing satisfies. Rebinding it left the nested copy uninstalled and without a dependent, so the next save dropped it. In that case prefer the entry printed inside the dependent's own node_modules; a lockfile written by this version has the fallback candidate there or nothing, so it binds the same as before.
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. The fix is small and well-targeted, but because it changes which package a peer edge binds to when loading a bun.lock (and the description notes coordination with #38767/#38768), a human look would still be worthwhile.
Checked that all four callers of resolve_peer_dep_version_based pass the new argument, that get_nested matches find_resolution_impl's first probe, and that the nested() check sits strictly on the nothing-satisfies path (satisfied ranges, optional peers, * peers, and overridden names still return before it). Also confirmed the strict-peer-dep registry fixture declares no-deps@^2.0.0, so the test's hand-written lockfile is unsatisfiable as intended.
Extended reasoning...
Overview
Adds a nested closure parameter to resolve_peer_dep_version_based in src/install/lockfile/bun.lock.rs so that when no candidate satisfies a required peer's range, the entry printed at dependent/peer-name in the lockfile is bound before falling back to the highest-ordered candidate. A new PkgMap::get_nested helper builds that key (mirroring the first probe of find_resolution_impl). All four call sites are updated: the root loop and the pnpm migration pass || None, the workspace loop passes the already-built workspace_node_modules slice, and the per-package loop calls get_nested. A regression test in test/cli/install/bun-lock.test.ts hand-writes a 1.3-shaped lockfile with strict-peer-dep/no-deps nested under an unsatisfiable ^2.0.0 peer and asserts the nested copy is installed, kept by --lockfile-only, and accepted by --frozen-lockfile.
Security risks
None. This is lockfile-load resolution logic; no untrusted input parsing, no path traversal, no auth/crypto.
Level of scrutiny
Moderate. The change is narrow — a three-line insertion between the candidates loop and the existing fallback, gated behind deferred_peer_range (so optional/* peers and overridden names never reach it) and behind the candidates loop (so any satisfied range still wins). It only alters behavior on the exact path the PR targets: candidates exist, none satisfies, and the printed tree has a nested entry. That said, it does change which package ID a peer edge binds to when loading an existing bun.lock, which flows into what is installed on disk and what --frozen-lockfile accepts. Peer resolution here has been the subject of several recent fixes (#32182, #38853, #38870), and the description flags that #38767 and #38768 add callers that will need the extra argument.
Other factors
The get_nested bounds check degrades to None (falling through to the pre-existing highest-candidate fallback) rather than erroring, which is safe. Borrow patterns match the surrounding code (dep_name slices string_buf, the closure's &mut path_buf is released before find_resolution reuses it). The test uses existing registry fixtures and the file's makeInstallRunner helper, and asserts the actual installed tree plus lockfile round-trip. The PR description includes verification against the real-world activepieces repro. No CODEOWNERS entry covers src/install/.
|
Updated 10:16 AM PT - Aug 15th, 2026
❌ @robobun, your commit c6f3e57 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 38992That installs a local version of the PR into your bun-38992 --bun |
|
Closing in favor of #38892, which fixes the same bug (the activepieces entries Compared on local builds of both branches:
The one assertion this PR had that #38892 lacked, |
2 participants
Problem
bun.lockwritten by bun 1.3.x can nest a copy of a peer under a dependent whose peer range no version in the file satisfies. activepieces/activepieces (HEAD, also at ec27c2f2) commitsreact-json-view/react,react-json-view/react-dom(react@18.3.1,react-dom@18.3.1;react-json-viewdeclaresreact: ^17 || ^16.3 || ^15.5.4, the root holdsreact@19.2.5),flux/reactandreact-json-view/react-dom/scheduler. 1.3.14 installs that file as written and re-saves it byte-identically.react@19.2.5/react-dom@19.2.5:resolve_peer_dep_version_based(src/install/lockfile/bun.lock.rs, from install: fix stale global-store links on disable and peer edge drift across bun.lock loads #32182) binds a ranged peer by version and, when nothing satisfies the range, takes the highest candidate. The nested copies are then not installed (react-json-viewgets React 19),react-dom@18.3.1has no dependent left, and a plainbun installon the untouched checkout deletes the four entries (Clean lockfile: 4069 -> 4068). Until install: fix the package id boundary in Lockfile::eql #38870 this also failed--frozen-lockfile; now the frozen install succeeds but lays out a different tree than 1.3.14 does from the same file. Part of the 1.3 to 1.4 lockfile ledger (the other four repos there are the optional-peer case: install: keep optional-peer-held packages when the lockfile is frozen #38853 for frozen installs, install: keep the packages bun.lock holds only through optional peers when an install re-resolves #39002 for the rest).Fix
node_modules(PkgMap::get_nested, passed to the helper as a lazy closure) before falling back to the highest candidate. The hoister nests a peer under its dependent exactly when nothing enclosing satisfies it, so that entry is the binding the file records, and it is what node resolves from the dependent. Binding it makes the file a fixed point: the copy is placed nested again, installed, and written back.*peers and overridden names are untouched. The root loop passes|| None(root-level entries are hoisted ones, not bindings); the pnpm migration passes|| None(no printed tree yet). One hash lookup, only on the nothing-satisfies path.|| Nonefor install: rebind ranged peers whose target the saved tree drops #38768's placed-package scan; for install: bind peer edges in the hoister, the way loading bun.lock binds them #38767's pass over already bound edges the natural value is the edge's current target, so a recorded nothing-satisfies binding is kept rather than re-derived).test/cli/install/bun-lock.test.ts, "installs and keeps the copy an older lockfile nests under a dependent whose peer range nothing satisfies": a hand-written 1.3-shaped lockfile on the registry fixtures (strict-peer-depwantsno-deps@^2.0.0;one-dephoistsno-deps@1.0.1;strict-peer-dep/no-depsis1.0.0). Rebuilt with main'ssrc/installit fails (strict-peer-dep/node_modules/no-depsis not installed: receivednull, expected"1.0.0"); with this change it passes,--lockfile-onlykeeps the nested entry and--frozen-lockfileaccepts the re-saved file.--frozen-lockfileexits 0 and a plain install no longer touches the four entries (the remaining diff there is the optional-peer case above).across installs from bun.locktests (install: fix stale global-store links on disable and peer edge drift across bun.lock loads #32182) pass. Full isolated-install / pnpm-lock-v9 runs on this box hitConnectionRefusedfrom the test registry under load (load average above 200) on varying tests; the same tests pass when re-run. isolated-install's "ranged peer dependency resolution is stable" is intermittent on its fresh-install assertion (line 1202, before any lockfile is loaded; the arrival-order dependence install: bind peer edges in the hoister, the way loading bun.lock binds them #38767 describes), with or without this change.Background
bun.lockkeys each package by thenode_modulespath the hoister gave it and stores no target for peer edges. On load, ordinary edges are re-bound by walking up those paths; since install: fix stale global-store links on disable and peer edge drift across bun.lock loads #32182 required ranged peers are re-bound by version instead, because a peer whose range is satisfied by a version hoisted above it has no path of its own, and the walk rebound it to that version (re-keying isolated store entries on every warm install).Tree::hoist_dependencycannot dedupe it and places it under the dependent, which is thedependent/nameentry this change reads back. 1.3.x bound every peer by path, so it produced and preserved such entries whenever a project's peer ranges fell behind the versions it installed.clean_with_loggerrebuilds the lockfile from the packages the edges reach; a package no edge points at is dropped, which is how rebinding the peer deletes the nested copy on the next save.