install: stop order-dependent peer-dep early match in resolution by dylan-conway · Pull Request #29804 · oven-sh/bun

dylan-conway · 2026-04-28T04:31:09Z

What

Removes the if (install_peer and behavior.isPeer()) early-return at the top of getOrPutResolvedPackage (PackageManagerEnqueue.zig), along with the now-dead resolutionSatisfiesDependency helper.
Restores the "incorrect peer dependency" warning from Tree.hoistDependency instead — the spot where a peer collapses into a non-satisfying workspace-root dep already had a TODO for this.

Why

The removed block scanned lockfile.package_index and bound the peer to whichever already-resolved version of the same package it found first. package_index is populated as manifest download/parse tasks complete (runTasks → processDependencyList), so its contents at peer-resolve time depend on network and thread-pool completion order — the same install can produce different lockfiles run-to-run.

With the block gone, peers go through the normal findBestVersion path. Dedup still happens in hoistDependency (line 721-735): if the peer's range satisfies the existing same-name dep at that tree level it hoists silently; if it doesn't but the existing dep is a workspace-root dep, it hoists with the warning. Hoist walks the tree in a fixed order, so both the placement and the warning are deterministic.

This is the piece that was missing from dylan/fix-flaky-install-test — that branch removed the analogous satisfies() check from getPackageID and moved dedup to hoist, but left this peer block in place.

Behavioural changes

When root declares both dependencies.X and peerDependencies.X at different versions, the peer now resolves and downloads its own tarball before hoist discards it. The on-disk install result is unchanged (still 1 package, root's dependencies version), but there's one extra tarball request. Two tests in bun-install.test.ts updated for the request count.
A peer whose closest ancestor is a non-root hoisted dep with a non-satisfying version now gets nested with its own version instead of being force-collapsed at resolve time. No tests cover this; closer to yarn v1's resolvePeerModules.

Not in this PR

getPackageID's own satisfies() dedup (lockfile.zig:1398/1410) is the other half of the non-determinism and is what flakes hoisting > text lockfile is hoisted — confirmed to flake on main in 1/3 runs locally. That's the change dylan/fix-flaky-install-test already had; keeping this PR to just the missing piece so we can see its CI effect in isolation.

Test plan

bun bd test test/cli/install/bun-install-registry.test.ts — 214 pass, 6 fail (matches main baseline exactly: 4× ASAN-stderr, 2× pre-existing debug warn:/snapshot flake)
bun bd test test/cli/install/bun-install-registry.test.ts -t peer — 25 pass, 4 pre-existing ASAN fails; both toContain("incorrect peer dependency") tests and all six not.toContain tests pass
bun bd test test/cli/install/bun-install.test.ts -t peer — 8 pass, 0 fail
bun bd test test/cli/install/bun-lock.test.ts — 9 pass, 0 fail
bun bd test test/cli/install/hoist.test.ts test/cli/install/isolated-install.test.ts test/cli/install/test-dev-peer-dependency-priority.test.ts — 47 pass, 1 pre-existing timeout

robobun · 2026-04-28T04:31:21Z

^{Updated 3:30 AM PT - May 4th, 2026}

@dylan-conway, your commit 9467193 is building: #51061

coderabbitai · 2026-04-28T04:33:19Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

Removed the peer-specific resolution reuse and its version-matching helper from package enqueue logic; adjusted hoisting to emit conditional peer-dependency warnings in resolvable mode. Updated tests to expect additional registry tarball fetches and added a regression test for one-time warning emission and lockfile stability.

Changes

Cohort / File(s)	Summary
Package enqueue `src/install/PackageManager/PackageManagerEnqueue.zig`	Removed the peer-specific resolution reuse path and deleted the `resolutionSatisfiesDependency` helper, eliminating tag/version satisfaction checks for peers during enqueue.
Lockfile hoisting & warnings `src/install/lockfile/Tree.zig`	Adjusted hoist logic to use a symmetric peer-side condition and added conditional warning emission for incorrect peer dependency resolution when `method` is `.resolvable`; warning write errors are swallowed.
Tests (unit & regression) `test/cli/install/bun-install.test.ts`, `test/cli/install/bun-install-registry.test.ts`	Updated expectations to include additional registry tarball downloads and increased `ctx.requested` counts; added a regression test verifying the peer-dependency warning appears only once and that `bun.lock` remains byte-identical across installs.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: removing an order-dependent early-match block for peer dependency resolution, which is the core focus of the changeset across multiple files.
Description check	✅ Passed	The PR description comprehensively covers both template sections: 'What' details the specific code removals and warnings, and 'How did you verify' is addressed through the extensive test plan with specific test commands and pass/fail counts.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

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

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

test/cli/install/bun-install-registry.test.ts (1)

7649-7676: ⚠️ Potential issue | 🟡 Minor

Rename this test to match the new expected behavior.

The test now asserts no "incorrect peer dependency" warning, but the title still says it should warn. Please rename it to avoid semantic drift.

Suggested diff

-  test("it should warn when the peer dependency resolution is incompatible", async () => {
+  test("it should not warn when the peer dependency resolution is incompatible", async () => {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/cli/install/bun-install-registry.test.ts` around lines 7649 - 7676,
Rename the test title string used in the test(...) call that currently reads "it
should warn when the peer dependency resolution is incompatible" to reflect the
new assertion (no warning); update it to something like "it should not warn when
the peer dependency resolution is incompatible" or "it should not emit incorrect
peer dependency warning" so the test description matches the expectations
asserted by stdout/stderr checks in the test body.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@test/cli/install/bun-install-registry.test.ts`:
- Around line 7649-7676: Rename the test title string used in the test(...) call
that currently reads "it should warn when the peer dependency resolution is
incompatible" to reflect the new assertion (no warning); update it to something
like "it should not warn when the peer dependency resolution is incompatible" or
"it should not emit incorrect peer dependency warning" so the test description
matches the expectations asserted by stdout/stderr checks in the test body.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7f4e6520-4ce9-4edc-ab73-79512865b2f1

📥 Commits

Reviewing files that changed from the base of the PR and between 68a2c3d and feef462.

📒 Files selected for processing (3)

src/install/PackageManager/PackageManagerEnqueue.zig
test/cli/install/bun-install-registry.test.ts
test/cli/install/bun-install.test.ts

💤 Files with no reviewable changes (1)

src/install/PackageManager/PackageManagerEnqueue.zig

github-actions · 2026-04-28T04:35:27Z

Found 6 issues this PR may fix:

Bun not able to be used in place of npm for install due to peerDependency resolution strategy #15711 - PR removes the order-dependent early-match logic that caused bun to pick the wrong peer dependency version (e.g., eslint v9 instead of v8) when multiple constraints exist
Inconsistent bun.lock when doing lockfile maintenance #16527 - PR eliminates the non-deterministic package_index scan that caused repeated bun install to produce different lockfiles depending on async task completion order
Bun incorrectly resolves optional peerDependencies，Different from npm #22150 - PR fixes order-dependent optional peer dep resolution where install order determined which version was matched (e.g., ajv@6 vs ajv@8)
bun installs multiple non-specified version dependencies #6678 - PR removes the early-match peer-dep logic that caused non-deterministic installation of wrong dependency versions
Bun has installed duplicate dependencies #7750 - PR addresses duplicate dependency installation by routing peer deps through findBestVersion instead of grabbing whatever resolved first
bun install warns on valid prereleases in peer deps #29444 - PR removes the resolutionSatisfiesDependency helper and "incorrect peer dependency" warning that produced false positives on valid prerelease versions

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #15711
Fixes #16527
Fixes #22150
Fixes #6678
Fixes #7750
Fixes #29444

🤖 Generated with Claude Code

claude

I didn't find any bugs, but this reworks core peer-dep resolution and hoist semantics (new .dedupe result that rewrites builder.resolutions, dropping the range-satisfies path from getPackageID, and removing the should_update gate in getOrPutResolvedPackageWithFindResult), so it's worth a human look — note the "Not in this PR" section of the description appears outdated since 290d5aa now includes the getPackageID satisfies-dedup removal.

Extended reasoning...

Overview

This PR removes the order-dependent peer-dependency early-match block from getOrPutResolvedPackage and the resolutionSatisfiesDependency helper, drops the version/range-satisfies dedup path from Lockfile.getPackageID (with all callers updated in migration.zig and folder_resolver.zig), bounds the retry_from_manifests_ptr loop to one iteration, and moves peer dedup into Tree.hoistDependency via a new .dedupe hoist result that rewrites builder.resolutions[dep_id] and emits the "incorrect peer dependency" warning when collapsing into a non-satisfying workspace-root dep. Tests/snapshots are updated for the extra tarball requests and new install layouts, plus three regression tests.

Security risks

None identified — this is package-resolution/hoisting algorithm work with no auth, network, or untrusted-input parsing changes.

Level of scrutiny

High. This is the core resolution path for bun install; the changes intentionally alter behavior (extra tarball fetch when root has both dependencies.X and peerDependencies.X, different nesting for non-root peers, lockfile hash changes in the snapshot). The new .dedupe arm mutates builder.resolutions during hoist, and getOrPutResolvedPackageWithFindResult no longer consults should_update/updating_packages before reusing an existing package — both deserve a maintainer's eye on bun update/bun add flows and the isolated linker.

Other factors

The PR is well-described and adds targeted regression tests (lockfile stability across reinstalls, no-loop on missing version, bun add with a root peer). However, the description's "Not in this PR" section says the getPackageID satisfies-dedup removal is excluded, yet commit 290d5aa includes it — worth confirming the intended scope. Given the breadth of behavioral change to install semantics, deferring to a human reviewer.

claude

I didn't find bugs, but this reworks core peer-dep resolution and hoisting semantics with user-visible behavioral changes (different package counts/nesting in several tests, and the lockfile.zig getPackageID satisfies-dedup removal that the description says is not in this PR appears to actually be included), so it warrants a human look.

Extended reasoning...

Overview

This PR removes the order-dependent peer-dependency early-match block from getOrPutResolvedPackage and the resolutionSatisfiesDependency helper in PackageManagerEnqueue.zig, drops the version parameter and npm-range satisfies() fallback from Lockfile.getPackageID (with callsite updates in migration.zig, folder_resolver.zig, and the enqueue path), and adds a new .dedupe hoist result in Tree.zig that rewrites builder.resolutions[dep_id] plus a symmetric peer-collapse check and the relocated "incorrect peer dependency" warning. It also adds a did_retry_from_manifests guard around the manifest-cache retry loop and removes the should_update/updating_packages gate in getOrPutResolvedPackageWithFindResult. Tests and snapshots are updated to reflect the new behavior (extra tarball downloads, additional nested installs, changed package counts).

Security risks

None apparent — this is internal resolution/hoisting logic with no auth, crypto, or untrusted-input parsing changes.

Level of scrutiny

High. This is the core dependency-resolution and hoisting algorithm for bun install. The changes intentionally alter user-visible behavior: dependency from root satisfies range from dependency now installs 3 packages instead of 2 with a nested no-deps@1.1.0, dragon test 2 goes from 3→4 packages with a nested no-deps@2.0.0, and the yarn-lock snapshot now splits no-deps into two entries. The new .dedupe arm mutates builder.resolutions during hoist, which feeds both the text lockfile and the isolated linker. These are exactly the kind of semantic shifts a maintainer should sign off on.

Other factors

The PR description's "Not in this PR" section says the getPackageID satisfies() removal (lockfile.zig:1398/1410) is being kept out for isolation, but the diff includes it — worth confirming the intended scope.
The should_update computation in getOrPutResolvedPackageWithFindResult is dropped entirely (_ = version; _ = dependency;); previously it gated whether range-satisfaction dedup applied during bun update. With getPackageID no longer doing range matching this may be moot, but it's a subtle interaction with the update flow.
Good regression coverage was added (lockfile stability across installs, no-loop on missing version, bun add with root peer), and the bunx test tweak (5.0.0→5.0.4) looks incidental.

claude

I didn't find any bugs, but this changes core peer-dependency resolution semantics in bun install (removing the early-match path, making the hoist peer-collapse symmetric, adding the did_retry_from_manifests loop guard) with acknowledged behavioural changes — worth a human look.

Extended reasoning...

Overview

This PR removes the ~80-line install_peer and behavior.isPeer() early-return block from getOrPutResolvedPackage in PackageManagerEnqueue.zig (plus the dead resolutionSatisfiesDependency helper), adds a did_retry_from_manifests guard to the retry_from_manifests_ptr loop, and reworks the peer-collapse condition in Tree.hoistDependency to be symmetric (dependency.behavior.isPeer() or dep.behavior.isPeer()) while restoring the "incorrect peer dependency" warning at the hoist site. Tests are updated for the extra tarball fetch and three new regression tests are added.

Security risks

None apparent — this is internal resolution/hoisting logic with no auth, crypto, or untrusted-input parsing changes.

Level of scrutiny

High. This is production-critical bun install dependency-resolution code where the PR description itself documents two intentional behavioural changes: (1) an extra tarball download when root has both dependencies.X and peerDependencies.X, and (2) non-root peers with non-satisfying ancestors now nest instead of force-collapsing. The symmetric peer check in hoistDependency is a real semantic change to hoisting, and the new did_retry_from_manifests guard implies the removal exposed a potential infinite retry that the author had to fence — both deserve a maintainer's eye.

Other factors

The PR is well-reasoned, has a thorough test plan, and is explicitly scoped as one half of a two-part determinism fix (the getPackageID half is deferred). The bug-hunting system found nothing. But given the blast radius (lockfile shape for every install with peer deps) and the subtle interactions with the manifest-cache retry path, this is not a mechanical change I can approve without human review.

Removes the early-return path in `getOrPutResolvedPackage` where a peer dependency would scan `lockfile.package_index` and adopt whichever already-resolved version of the same name happened to be there. That index is populated as manifests finish downloading/parsing, so the contents at peer-resolve time depend on network/thread completion order — different runs could pick different versions. Peers now resolve through the same `findBestVersion` path as regular dependencies; deduplication still happens later during hoisting, which walks the tree in a deterministic order. Side effects of dropping the block: - the resolve-time "incorrect peer dependency" warning is gone (the fallback that emitted it forced the peer onto a non-satisfying version anyway, which was itself order-dependent) - when root has both `dependencies.X` and `peerDependencies.X` at different versions, the peer's tarball is now downloaded before hoist collapses it; install output (versions placed on disk) is unchanged `resolutionSatisfiesDependency` is removed as it was only used here. Tests updated for the warning removal and the extra tarball request.

Previous commit removed the resolve-time peer match, which also dropped the "incorrect peer dependency" warning. The warning is still useful and matches npm/pnpm/yarn behavior, so restore it from `hoistDependency` — the spot where a peer is collapsed into a workspace-root dep whose version does not satisfy the peer's range. This already had a TODO for exactly this warning. Hoist runs after all resolution is complete and walks the tree in a fixed order, so the warning is now deterministic. Reverts the test changes that flipped the warning assertions to `not.toContain`; the original assertions are correct again.

`hoist` runs twice per fresh install (`.resolvable` from `cleanWithLogger` then `.filter` from `installHoistedPackages`), so the warning would double-fire. Gate it to `.resolvable` only. Net behavior: the warning fires exactly once on the install that first resolves the conflict, then not again on no-op or reinstall-from-lockfile runs (the cleaned lockfile no longer carries a separate resolution for the hoisted peer, so there is nothing to re-check). Adds a test asserting the count is exactly 1 on first install, 0 on a no-op second install, 0 on reinstall after wiping node_modules, and that the text lockfile is byte-identical across all three.

`hoistDependency` previously only ran the range-vs-resolution satisfies check when the dependency *being hoisted* was a peer. With the resolve-time peer match removed, peers now reach hoist with their own package_id, so a regular dependency hoisting past an already-placed peer (or vice versa) needs the same treatment. Apply the satisfies check when *either* side is a peer. Regular-into- regular is intentionally excluded — collapsing those would override locked resolutions, which `package added after install` correctly guards against (root adds `no-deps@1.0.0`, transitive's locked `no-deps@1.1.0` must stay nested even though the range would accept 1.0.0). Also gate the check on both resolutions being `.npm`, so a workspace- resolved npm range never collapses into a registry version.

`getOrPutResolvedPackage` returns null in two distinct cases: the manifest is not loaded yet (transient — retry helps), or the manifest is loaded but `findBestVersion` failed and the dep is a peer (permanent — retry never helps). The caller cannot distinguish them, and the retry at `retry_from_manifests_ptr` assumed the first case, so a peer with a version not present in a fresh disk-cached manifest spun forever. This was previously masked by the resolve-time peer block (removed in an earlier commit on this branch), which returned an existing `package_index` entry before `findBestVersion` ran. Reproducer: `bun init -y` (root gets `peerDependencies.typescript: "^5"`), then `bun add typescript@5.0.0` — 5.0.0 has never existed on npm (5.0.2 is the first stable 5.0.x), so the loop never exits. This is what timed out `bunx.test.ts` on Windows CI; that test does exactly init+add and is `skipIf(!isWindows)`. Fix: cap the retry to one pass. After one retry the manifest is guaranteed in `this.manifests`, so a second null is a permanent not-found; fall through to the network-task / waiting-list path and let `verifyResolutions` report it (or silently leave it unresolved for pure peers, matching prior behavior). Also: the bunx test asks for `typescript@5.0.0` which never existed — it only "passed" on main because the peer block silently substituted 5.9.3. Switch to 5.0.4. Adds two registry tests covering the loop and the `bun add` exact-version-with-existing-peer case.

`getPackageID` previously took a `version` range and, in addition to matching exact resolutions, returned any already-appended package whose version satisfied that range. Which packages were "already appended" depends on manifest network/thread-completion order, so two installs could produce different lockfiles. This is the cause of the `hoisting > text lockfile is hoisted` flake (now passes 3/3 in full-suite runs; previously 2/3 on main). Now `getPackageID` matches only on exact `resolution.eql()`. Each dependency resolves to its own `findBestVersion` result; two deps with the same range still share a package_id (same best version → exact match), but overlapping-different ranges (`*` vs `^1.0.1`) get distinct versions and nest. This is yarn-v1 / npm semantics. Hoist still works via the existing `res_id == package_id` path for regular deps. Peers collapse via the symmetric satisfies-check from an earlier commit on this branch — extended here to a new `.dedupe` variant that also rewrites `buffers.resolutions[dep_id]` to the existing package, so the lockfile and the isolated linker agree with the hoisted tree. Tests updated where the old order-dependent dedup was the asserted behavior (one-range-dep now keeps its own `no-deps@1.1.0`, dragon-2, optionalDependencies sort-order snapshot).

This reverts commit 290d5aa.

The removed resolve-time peer block had a fallback: when an existing package_index entry did not satisfy the peer's range but resolved from the same source (npm/git/github), the peer was force-collapsed into it with an "incorrect peer dependency" warning. Without that, a peer wanting `react@^18` whose nearest ancestor is a hoisted `react@17` (not a workspace-root dep) was getting `react@18` nested instead. yarn v1 and pnpm both collapse + warn in this scenario; only npm nests. Restore the collapse from `hoistDependency` so the result matches main (and yarn/pnpm) but is computed in deterministic tree order rather than network-completion order. The existing root-dep collapse remains as a secondary fallback for tag mismatches.

…fying version The previous commit collapsed any peer into a non-satisfying same-tag ancestor, matching yarn/pnpm for the case where nothing in the tree satisfies. But it also collapsed when a satisfying version *is* available via a regular dep — e.g. `ajv-keywords` peers on `ajv@^8`, `schema-utils` regular-deps on `ajv@^8`, root has `ajv@6` hoisted: the peer was being forced onto `ajv@6` instead of nesting with the `ajv@8` that `schema-utils` brings. That broke `expo export` in the integration test (`Cannot find module 'ajv/dist/compile/codegen'`). Restrict the force-collapse to the case where no non-peer dependency anywhere resolved to a version satisfying the peer's range. The check scans `buffers.dependencies`/`resolutions`, but only on the rare path where a peer hits a non-satisfying ancestor; its outcome (does *any* non-peer satisfy?) is determined by the dependency graph, not resolve order.

claude

I didn't find any bugs, but this reworks core peer-dep resolution/hoisting semantics with acknowledged behavioral changes (extra tarball fetches, different nesting for non-root peers — the latter not covered by tests), so it warrants a human look.

Extended reasoning...

Overview

This PR removes the order-dependent peer-dep early-match block from getOrPutResolvedPackage in PackageManagerEnqueue.zig (~80 lines + the resolutionSatisfiesDependency helper), adds a did_retry_from_manifests guard to bound the manifest-retry loop, and replaces the removed logic with new collapse/warning semantics in Tree.hoistDependency — including a symmetric peer check and a full-lockfile scan to decide whether a non-satisfying peer should force-collapse or nest. Tests are updated for the extra tarball request and three new regression tests are added.

Security risks

None identified — no auth, network surface, or untrusted-input parsing changes. The change is internal resolution/hoist algorithm behavior.

Level of scrutiny

High. This is the core bun install dependency resolution path that runs on every install. The PR explicitly changes user-visible semantics: (1) peers now download their own tarball before hoist discards them, and (2) peers whose closest non-root ancestor doesn't satisfy now nest instead of force-collapsing — the description notes "No tests cover this." The new has_satisfying_non_peer scan in hoistDependency is an O(all-deps) loop per peer collision, which is a non-trivial design choice worth a maintainer's eye. The commit history (revert → drop → bound-retry → force-collapse refinements) shows this is an actively iterated algorithmic change, and the PR is positioned as one half of a two-part fix split for CI isolation.

Other factors

The bug-hunting pass found no issues, the test plan is thorough, and the rationale (eliminating network/threadpool-order non-determinism in lockfiles) is well-argued. But the combination of untested behavioral changes, a new scan in the hoist hot path, and the explicit "part of a larger fix" framing make this unsuitable for auto-approval — a maintainer familiar with the install/hoist invariants should sign off on the new collapse-vs-nest semantics.

… versions Removes the install_peer && behavior.is_peer() block from get_or_put_resolved_package (PackageManagerEnqueue.rs) and the resolution_satisfies_dependency helper that only that block called. Peers now flow through findBestVersion; dedup and the 'incorrect peer dependency' warning live in Tree::hoist_dependency where placement is deterministic. Adds satisfies_include_prerelease on Group and List in SemverQuery.rs — the strict-semver siblings of satisfies_pre without the pre_matched gate. Wired into the peer paths in Tree::hoist_dependency so prerelease resolutions like 5.0.0-alpha.150 are accepted against ranges like >=1.0.0 (node-semver includePrerelease / yarn v1 satisfiesWithPrereleases semantics). The strict resolver path is untouched; Bun.semver.satisfies() keeps current behavior. Combines oven-sh#29804 (Dylan Conway, Zig-era refactor) with oven-sh#27085 (prerelease semver fix). Both were stuck pre-rewrite and don't apply to the Rust tree. Fixes oven-sh#29444.

… versions Removes the install_peer && behavior.is_peer() block from get_or_put_resolved_package (PackageManagerEnqueue.rs) and the resolution_satisfies_dependency helper that only that block called. Peers now flow through findBestVersion. Dedup and the "incorrect peer dependency" warning live in Tree::hoist_dependency where placement is deterministic. Adds satisfies_include_prerelease on Group and List in SemverQuery.rs, the strict-semver siblings of satisfies_pre without the pre_matched gate. Wired into the peer paths in Tree::hoist_dependency so prerelease resolutions like 5.0.0-alpha.150 are accepted against ranges like >=1.0.0 (node-semver includePrerelease, yarn v1 satisfiesWithPrereleases semantics). The strict resolver path is untouched. Bun.semver.satisfies() keeps current behavior. Combines oven-sh#29804 (Dylan Conway, Zig-era refactor) with oven-sh#27085 (prerelease semver fix). Both were stuck pre-rewrite and don't apply to the Rust tree. Fixes oven-sh#29444.

Removes the install_peer && behavior.is_peer() block from get_or_put_resolved_package (PackageManagerEnqueue.rs) and the resolution_satisfies_dependency helper that only that block called. Peers now flow through findBestVersion. Dedup and the "incorrect peer dependency" warning live in Tree::hoist_dependency where placement is deterministic. The deleted block scanned lockfile.package_index and bound peers to whichever already-resolved version of the same name appeared first. package_index is populated as manifest download tasks complete, which is whatever order the thread pool finishes them, so the same install produced different bun.lock contents run to run. Also bounds the 'retry_from_manifests_ptr cache-control retry to a single trip so a stale-but-fresh manifest paired with a missing resolution can't ping-pong forever after the early-match removal. Tree::hoist_dependency gains a symmetric peer-collapse check (either side may be a peer) and a has_satisfying_non_peer scan that prevents force-collapsing a peer onto a wrong-major resolution when a satisfying non-peer dep exists elsewhere in the lockfile. Rust port of Dylan Conway's oven-sh#29804 (Zig, stuck pre-rewrite).

robobun · 2026-06-26T08:52:56Z

Closing as stale: this PR predates the Rust rewrite. Every src/ file it modifies has since been removed or relocated on main (Zig sources deleted; src/bun.js/ reorganized into src/jsc/), so it can no longer merge.

If the underlying change is still wanted, it will need to be redone against the current Rust/C++ tree. Apologies for the churn, and thank you for the contribution.

coderabbitai Bot reviewed Apr 28, 2026

View reviewed changes

This was referenced Apr 28, 2026

inspect: handle throwing Proxy getPrototypeOf in forEachProperty #29814

Closed

webview: add mouse.down/up/move primitives for drag automation #29817

Open

claude Bot reviewed Apr 28, 2026

View reviewed changes

claude Bot reviewed Apr 29, 2026

View reviewed changes

claude Bot reviewed May 3, 2026

View reviewed changes

dylan-conway and others added 10 commits May 4, 2026 10:30

[autofix.ci] apply automated fixes

3c06230

Revert "install: drop range-satisfies dedup from getPackageID"

996fca8

This reverts commit 290d5aa.

Jarred-Sumner force-pushed the claude/quirky-curie-f8e60c branch from b9954a0 to 9467193 Compare May 4, 2026 10:30

claude Bot reviewed May 4, 2026

View reviewed changes

ramonclaudio mentioned this pull request May 15, 2026

install: fix bun add ignoring explicit version when name is a peerDep #30855

Open

ramonclaudio mentioned this pull request May 15, 2026

fix(install): use includePrerelease semantics for peer dep validation #27085

Closed

robobun closed this Jun 26, 2026

robobun mentioned this pull request Jun 27, 2026

url: follow the spec for the host/port setters, origin, and URLPattern groups #32826

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

install: stop order-dependent peer-dep early match in resolution#29804

install: stop order-dependent peer-dep early match in resolution#29804
dylan-conway wants to merge 10 commits into
mainfrom
claude/quirky-curie-f8e60c

dylan-conway commented Apr 28, 2026 •

edited

Loading

Uh oh!

robobun commented Apr 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Apr 28, 2026 •

edited

Loading

Reviews paused

Uh oh!

coderabbitai Bot left a comment

Uh oh!

github-actions Bot commented Apr 28, 2026

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

dylan-conway commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Behavioural changes

Not in this PR

Test plan

Uh oh!

robobun commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 28, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dylan-conway commented Apr 28, 2026 •

edited

Loading

robobun commented Apr 28, 2026 •

edited

Loading

coderabbitai Bot commented Apr 28, 2026 •

edited

Loading