[robobun]

What does this PR do?

test/cli/install/test-dev-peer-dependency-priority.test.ts was fetching jquery, typescript and next from registry.npmjs.org on every run. The next install in particular pulls in react, react-dom, sharp, every @next/swc-* platform package, etc. On alpine 3.23 x64 (build #88691) this file took ~11s; locally on a cold cache the Next.js test hits the 5s per-test timeout.

This points the same four scenarios at the local verdaccio fixture's no-deps package (versions 1.0.0 / 1.1.0 / 2.0.0) so the file never touches the public internet and its wall time is stable regardless of cache state. Nothing about the behaviour under test changes: a workspace package still collides with a registry package of the same name, the devDependencies entry still uses a different spec from the peerDependencies entry, and we still verify that the dev entry is what gets linked and that a second install makes zero registry requests.

While in here:

• Replace the hardcoded http://localhost:9999/ "dead registry" with a port-0 Bun.serve that counts requests, so the "no network on second install" check is a positive expect(hits).toBe(0) instead of scanning stderr for GET.
• Assert the resolved version in bun.lock and (for the non-workspace case) in node_modules/no-deps/package.json, instead of only checking that bun.lock exists.
• Check stderr before exitCode so a failure reports the actual error.
• Collapse the hand-rolled new Promise + new Response(proc.stdout).text() spawn boilerplate into a small helper that uses await using.

How did you verify your code works?

Locally with bun bd test test/cli/install/test-dev-peer-dependency-priority.test.ts:

	before	after
cold bun-install cache	7.4s, Next.js case times out	3.6s, all pass
warm cache	2.8-3.0s	3.5-3.6s
expect() calls	17	29

The warm-cache "after" number includes ~0.8s of verdaccio startup; the win is that the cold-cache path (what CI sees on a fresh lane) no longer depends on npmjs.org latency, and the file can no longer flake on the 5s default timeout.

---

[stamp-90s] gate passed · iteration 1 · 1 files touched

passes on PR (with fix)

Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/cli/install/test-dev-peer-dependency-priority.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/cli/install/test-dev-peer-dependency-priority.test.ts
bun test v1.4.0 (07ebec9bb)

test/cli/install/test-dev-peer-dependency-priority.test.ts:
(pass) devDependencies and peerDependencies with different versions should coexist [167.02ms]
(pass) dependency behavior comparison prioritizes devDependencies [204.37ms]
(pass) workspace devDependencies should take priority over peerDependencies for resolution [681.09ms]
(pass) Next.js monorepo scenario should not make unnecessary network requests [622.51ms]

 4 pass
 0 fail
 29 expect() calls
Ran 4 tests across 1 file. [3.49s]
Exit: 0

diff hotspot

.../test-dev-peer-dependency-priority.test.ts      | 358 ++++++++-------------
 1 file changed, 130 insertions(+), 228 deletions(-)

gate history · 3 passed · 0 rejected · iteration 1

evidence per changed file

file                                                      reads  edits  tests
…t/cli/install/test-dev-peer-dependency-priority.test.ts      4      9      0

[coderabbitai]

Walkthrough

Changes

Dependency priority tests

Layer / File(s)	Summary
Isolated install test environment test/cli/install/test-dev-peer-dependency-priority.test.ts	Adds Verdaccio lifecycle management, reusable Bun process execution, and generated bunfig.toml configuration.
Workspace dependency resolution test/cli/install/test-dev-peer-dependency-priority.test.ts	Tests workspace lockfile resolution, reinstall behavior, registry request counts, and execution of workspace-provided versions.
Non-workspace version precedence test/cli/install/test-dev-peer-dependency-priority.test.ts	Verifies that the devDependency version 1.0.0 is locked and installed instead of peer-compatible version 1.1.0.

Possibly related PRs

• oven-sh/bun#36631: Tests overlapping devDependencies and peerDependencies precedence during installation.
• oven-sh/bun#36704: Adds related workspace and devDependency install test coverage.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly identifies the test change and the use of a local Verdaccio registry.
Description check	✅ Passed	The description includes the required sections and provides detailed verification results, test scope, and implementation context.

---

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

[claude]

LGTM — test-only hermeticity fix that swaps live npmjs.org fetches for the local verdaccio fixture.

What was reviewed:

• Confirmed no-deps exists in the verdaccio fixture with versions 1.0.0 / 1.0.1 / 1.1.0 / 2.0.0, so every spec in the rewritten tests resolves.
• VerdaccioRegistry, stderrForInstall, await using Bun.serve, and bunfig linker = "isolated" all match existing harness patterns.
• Checked that the four scenarios still exercise the same dev-vs-peer collision shapes (workspace protocol, matching semver, registry-only) and that the new assertions are strictly stronger than the old "lockfile exists" / "stderr has no GET" checks.

Extended reasoning...

Overview

Single-file test refactor of test/cli/install/test-dev-peer-dependency-priority.test.ts. The four existing test.concurrent cases previously installed jquery, typescript, and next from registry.npmjs.org; they now use the no-deps package from the in-tree verdaccio fixture. A shared VerdaccioRegistry is started in beforeAll, a small spawnBun helper replaces the hand-rolled new Promise + new Response(proc.stdout).text() boilerplate, and the hardcoded http://localhost:9999/ dead registry becomes a port-0 Bun.serve that counts hits.

Security risks

None. Test-only change with no runtime code touched. It removes a public-internet dependency, which is explicitly required by the repo's test guidelines.

Level of scrutiny

Low-to-medium. No production code paths change; the risk is limited to weakening or breaking existing test coverage. I compared each of the four scenarios before/after: the dev/peer version-range collision, the workspace-name-shadows-registry-package shape, and the "second install makes zero requests" check are all preserved, and the assertions are tightened (lockfile contents + installed package.json version + positive registryHits === 0) rather than loosened. The move of nodeLinker: "isolated" from package.json to bunfig linker = "isolated" is a lateral config change already used in isolated-install.test.ts and others.

Other factors

• Verified the fixture registry actually ships no-deps@1.0.0, 1.1.0, and 2.0.0, so the ^1.0.0 peer range and the 1.0.0 / 2.0.0 pins all resolve as the comments describe.
• stderrForInstall strips the [...ms] done timing line, which makes expect(stderr).toBe("") on the run-script step safe (that helper is applied to every spawn).
• await using on both Bun.spawn and Bun.serve follows the pattern used across test/regression/ and satisfies the "release every resource before assertions" rule.
• PR description reports the file was verified with bun bd test and drops from ~7.4s cold / timeout to ~3.6s.

[claude]

LGTM — test-only refactor that removes the public-internet dependency and tightens assertions.

What was reviewed:

• Confirmed the verdaccio fixture ships no-deps at 1.0.0 / 1.0.1 / 1.1.0 / 2.0.0, so every version the tests reference exists locally.
• Checked that moving nodeLinker: "isolated" from workspaces to bunfig.toml linker = "isolated" preserves the original scenario.
• Verified stderrForInstall, VerdaccioRegistry.registryUrl(), and tempDir usages match harness signatures; the port-0 dead-registry counter replaces the hardcoded localhost:9999 and the old stderr-GET scan.
• The four test.concurrent cases share one read-only verdaccio instance but each uses its own tempDir, so concurrency is safe.

Extended reasoning...

Overview

Single-file test refactor of test/cli/install/test-dev-peer-dependency-priority.test.ts. The four existing scenarios are kept but repointed from real npm packages (jquery, typescript, next) to the in-tree verdaccio fixture's no-deps package. A file-level VerdaccioRegistry is started in beforeAll, a spawnBun helper replaces the hand-rolled new Promise + new Response(proc.stdout).text() boilerplate, and the "no network on second install" check now uses a port-0 Bun.serve hit counter instead of scanning stderr for GET against a hardcoded localhost:9999. Assertions are strengthened from "bun.lock exists" to checking the resolved lockfile entry and, for the non-workspace case, the installed node_modules/no-deps/package.json.

Security risks

None. Test-only change with no production code touched. It actually reduces attack surface by removing live registry fetches from CI.

Level of scrutiny

Low-to-moderate. This is a hermeticity/flake fix for an existing test file, exactly the kind of change REVIEW.md encourages ("Never contact external network hosts or live registries", "Always use port: 0", "assert the strongest invariant"). The main risk with test rewrites is silently weakening what's asserted; here the assertions are strictly tighter — lockfile contents and installed version are now checked, and the dead-registry probe is a positive expect(hits).toBe(0) rather than a negative stderr scan that could pass vacuously if log format changed.

Other factors

• I confirmed test/cli/install/registry/packages/no-deps/ contains 1.0.0, 1.0.1, 1.1.0 and 2.0.0 tarballs, so the dev-vs-peer version splits (1.0.0 vs ^1.0.0, 2.0.0 workspace vs ^1.0.0 peer) resolve as the comments describe.
• stderrForInstall (strips the slow-filesystem warning) and VerdaccioRegistry.registryUrl() exist in test/harness.ts with the signatures used here.
• The nodeLinker: "isolated" setting moved from the workspaces object to bunfig.toml linker = "isolated", which is the equivalent knob — the isolated-linker aspect of the original repro is preserved.
• test.concurrent sharing one verdaccio instance is safe: each test writes to its own tempDir and verdaccio is read-only for these packages.
• The PR description reports the file passes under bun bd test and no longer times out on a cold cache; the bug-hunting system found nothing.

[claude]

LGTM — test-only hermeticity fix that swaps npmjs.org for the local verdaccio fixture.

What was reviewed:

• Verified no-deps versions 1.0.0/1.1.0/2.0.0 exist in test/cli/install/registry/packages/no-deps/, and stderrForInstall/registryUrl() are real harness exports.
• Confirmed linker = "isolated" in bunfig is the established pattern (used in isolated-install.test.ts et al.), so dropping nodeLinker from the workspaces object is equivalent.
• Checked the new assertions are strictly stronger than the old ones (lockfile content + installed package.json vs. bare .exists(); positive registryHits === 0 vs. stderr grep).
• Shared VerdaccioRegistry under test.concurrent is read-only and each test uses its own tempDir, so no cross-test interference.

Extended reasoning...

Overview

Single-file test refactor of test/cli/install/test-dev-peer-dependency-priority.test.ts. The four existing scenarios are preserved but re-pointed from real npm packages (jquery, typescript, next) to the verdaccio fixture's no-deps package. A shared VerdaccioRegistry is started in beforeAll, a spawnBun helper replaces the hand-rolled new Promise + new Response(proc.stdout).text() boilerplate, and the hardcoded localhost:9999 dead registry becomes a port-0 Bun.serve that counts hits. No production code is touched.

Security risks

None. Test-only change; the only new surface is a local port-0 HTTP server inside the test process that returns 500 to everything.

Level of scrutiny

Low-to-medium. This is a hermeticity fix that directly implements several REVIEW.md test requirements the original file was violating: no public-internet contact, no hardcoded ports, drain pipes concurrently with Promise.all, assert stderr before exitCode, assert exact resolved versions rather than "lockfile exists". The behaviour under test (dev-vs-peer priority) is unchanged — the dev spec still differs from the peer spec, workspace packages still shadow registry packages of the same name, and the second-install-no-network check is now a positive assertion rather than a stderr heuristic.

Other factors

• Verified the fixture packages referenced actually exist at the versions used (1.0.0, 1.1.0, 2.0.0 all present under registry/packages/no-deps/).
• linker = "isolated" in bunfig is the idiom used by isolated-install.test.ts, public-hoist-pattern.test.ts, etc., so moving it out of workspaces.nodeLinker is a lateral change to the more common pattern.
• stderrForInstall is applied to the non-install test.js spawns too, but it only strips the "Slow filesystem" warning, which is harmless for the expect(stderr).toBe("") checks.
• The evidence block shows all four tests passing on the debug build in ~3.6s, and the PR description documents the cold-cache timeout it fixes.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/cli/install/test-dev-peer-dependency-priority.test.ts`:
- Around line 214-220: Remove the `expect(stderr).not.toContain("GET")`
assertion from the test block. This assertion is redundant since the
`expect(registryHits).toBe(0)` check already proves no network requests reached
the registry, and the "GET" assertion cannot fail because log text is suppressed
by BUN_DEBUG_QUIET_LOGS=1, making it brittle and coupled to log formatting.
- Around line 64-66: Update the lockfile assertions in the three affected tests
to detect registry-version entries directly instead of checking
registry.registryUrl(). In the tests around the no-deps assertions, verify the
lockfile does not contain "no-deps@1."; in the test around utils, verify it does
not contain "utils@1.". Preserve the existing workspace-entry assertions where
present.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9b71741f-cfc2-47fd-8e56-be141e9ec435

📥 Commits

Reviewing files that changed from the base of the PR and between b66764f and 9fdd0f8.

📒 Files selected for processing (1)

• test/cli/install/test-dev-peer-dependency-priority.test.ts

[claude]

LGTM — test-only hermeticity fix that swaps public-registry packages for the local verdaccio no-deps fixture.

What was reviewed:

• Verified no-deps 1.0.0/1.0.1/1.1.0/2.0.0 all exist in test/cli/install/registry/packages/no-deps/, so the dev-vs-peer version splits (1.0.0 pin vs ^1.0.0 → 1.1.0; workspace 2.0.0 vs peer 1.0.0) are real and distinguishable.
• Checked the <name>@workspace:packages/<name> lockfile-string oracle against existing usage in catalogs.test.ts / minimum-release-age.test.ts — same format.
• Confirmed the shared VerdaccioRegistry + test.concurrent shape is safe: each test has its own tempDir, and beforeAll starts the registry before concurrent bodies run.
• Both CodeRabbit threads are resolved; the redundant not.toContain("GET") was dropped in favor of the registryHits === 0 counter.

Extended reasoning...

Overview

Single-file test refactor of test/cli/install/test-dev-peer-dependency-priority.test.ts. The four existing scenarios are kept but repointed from live npmjs.org packages (jquery, typescript, next) to the in-repo verdaccio no-deps fixture. Along the way the hand-rolled new Promise + new Response(proc.stdout).text() spawn boilerplate is replaced with a spawnBun helper that uses await using and drains stdout/stderr/exited concurrently, the hardcoded http://localhost:9999/ dead registry becomes a port: 0 Bun.serve that counts hits, and assertions are tightened from "lockfile exists" to "lockfile contains the workspace resolution and not the registry URL" plus a runtime check that the linked module reports 2.0.0-workspace.

Security risks

None. Test-only change, no production code paths, no new external inputs. It removes outbound internet access from the test file.

Level of scrutiny

Low-to-moderate. This is a hygiene change to an existing test file that directly addresses two explicit repo rules ("never contact external network hosts or live registries" and "do not hardcode ports"). The main risk is weakening the property under test, so I checked that each rewritten scenario still exercises a dev/peer spec collision where the peer range would resolve to a different concrete package than the dev spec — the fixture publishes 1.0.0/1.0.1/1.1.0/2.0.0 so the ^1.0.0-vs-1.0.0 and 1.0.0-vs-workspace-2.0.0 splits are real. The new assertions (lockfile content, installed package.json version, registryHits === 0, runtime 2.0.0-workspace) are strictly stronger than the old ones (lockfile exists, stderr lacks "GET").

Other factors

• The nodeLinker: "isolated" workspace option moved to linker = "isolated" in bunfig.toml; both configure the isolated linker, and the runtime require("no-deps").version assertion would fail if linking behaved differently.
• stderrForInstall is the harness helper that strips the "Slow filesystem" warning, so expect(stderr).toBe("") on the test.js run won't flake on that noise.
• CodeRabbit raised two points; one was withdrawn after the author explained the registry.registryUrl() oracle, and the other (drop the vacuous not.toContain("GET")) was applied in commit 07ebec9b.
• PR evidence shows all four tests passing under the debug build in ~4s.

[robobun]

The diff is ready. test-dev-peer-dependency-priority.test.ts passed on every lane in builds #88846 and #88853.

Remaining CI failures are unrelated to this test-only change:

• build #88846: run-crash-handler.test.ts Windows PowerShell timing race (fix already open in test(crash_handler): await the PowerShell ack instead of racing a 2s sleep #36550), plus two tests marked flaky that passed on retry
• build #88853: filesystem_router.test.ts segfault on debian 13 aarch64 in reload() while Bun.build() resolves the same directory, nothing this diff touches

Needs a maintainer to merge past the unrelated red.

[Jarred-Sumner]

This is Claude (an AI assistant) writing on Jarred's behalf.

Included in #44968.