test(install): normalize the registry URL, not the bare port, in the lockb print snapshot

[robobun]

Problem

• test/cli/install/bun-install-registry.test.ts, test duplicate dependency in optionalDependencies maintains sort order, flakes on main depending on which random port verdaccio was started on. Seen on the Windows x64 lane in main build 95598 (passed on retry); the port that run got was 7280 and the snapshot diff was:

  - # bun ./bun.lockb --hash: A1A17280329F8383-20d6659d9c0623de-94508CB3B7915517-4b22a59b37f2f4f6
  + # bun ./bun.lockb --hash: A1A14873329F8383-20d6659d9c0623de-94508CB3B7915517-4b22a59b37f2f4f6
  

• Cause: the test prints the binary lockfile with bun bun.lockb and normalizes the port with out.replaceAll(${port}, "4873") (bun-install-registry.test.ts:6252). The bare digits are replaced everywhere in the output, not only in the resolved URLs, so any port whose digits occur in the deterministic --hash: header line corrupts that line before it is compared with the snapshot.
• Enumerating every port randomPort() can return (1024 to 65534) against the committed snapshot: 15 ports break the old assertion (1551, 1728, 2803, 4508, 5517, 6659, 7280, 7915, 8032, 8383, 9155, 9450, 15517, 17280, 28032), all of them through the hash line. About 0.023% of runs of this file, every platform.
• This is the only bare-port normalization left in test/; every other lockfile snapshot in this file and its siblings already uses replaceAll(/localhost:\d+/g, "localhost:1234").

Fix

• Replace only the localhost:<port> URLs, using the same regex and localhost:1234 placeholder as the other lockfile snapshots in the file, and update the six resolved URLs in the snapshot entry accordingly. The hash line in the snapshot is unchanged (it never depended on the port, which is why the test passes for the other 64496 ports).
• Correct because the port can only legitimately appear in the output as part of a registry URL; anything else in the output that happens to contain the same digits (the content hash, integrity strings) must be compared as-is.
• Verified:
  • bun bd test test/cli/install/bun-install-registry.test.ts -t "duplicate dependency in optionalDependencies maintains sort order" passes (snapshot matched, not rewritten).
  • Fail-before: with registry.port forced to 7280 and the original assertion and snapshot, the same command fails with the exact diff above, on both the debug build and the release bun on PATH. With this change and the port forced to 7280, 8383 and 1728 it passes.
  • Port enumeration script (below) reports 15 failing ports for the old assertion and 0 for the new one.
• Test-only change, so there is no src/ diff to stash for a mechanical before/after; the forced-port runs above are the before/after.

Background

• VerdaccioRegistry (test/harness.ts) starts the fixture npm registry on randomPort(), so every resolved URL that bun install writes into the lockfile contains a different port on each run. Snapshot tests of lockfile contents therefore have to normalize the port before comparing.
• bun <path>.lockb prints a binary lockfile in yarn v1 lockfile format. Its header includes # bun ./bun.lockb --hash: <hex> (src/install/lockfile/printer/Yarn.rs:46), the lockfile's meta hash. generate_meta_hash (src/install/lockfile.rs:2924) hashes name@resolution lines plus lifecycle scripts, and for npm packages the resolution formats as the bare version (src/install/resolution.rs:767), so the hash never contains the registry port: it is stable across runs and belongs in the snapshot as-is, which is exactly what the old normalization broke.

Port enumeration

Takes the committed snapshot as the expected normalized output, substitutes each candidate port into its URLs to reconstruct what bun bun.lockb would print, then applies the old and new normalizations.

const snap = await Bun.file("test/cli/install/__snapshots__/bun-install-registry.test.ts.snap").text();
const key = "exports[`duplicate dependency in optionalDependencies maintains sort order 1`] = `\n";
const start = snap.indexOf(key) + key.length;
const expectedOld = snap.slice(start, snap.indexOf("\n`;", start)).replace(/^"/, "").replace(/"$/, "");
// snapshot as it was before this PR used localhost:4873; the new one uses localhost:1234
const expectedNew = expectedOld.replaceAll("localhost:4873", "localhost:1234");
const badOld: number[] = []; let badNew = 0;
for (let p = 1024; p <= 65534; p++) {
  const out = expectedOld.replaceAll("localhost:4873", `localhost:${p}`);
  if (out.replaceAll(`${p}`, "4873") !== expectedOld) badOld.push(p);
  if (out.replaceAll(/localhost:\d+/g, "localhost:1234") !== expectedNew) badNew++;
}
console.log(badOld.length, badOld.join(","), badNew);

Output against the pre-PR snapshot:

ports tried: 64511 (1024..65534)
old assertion fails for 15 ports (0.023%)
  1551, 1728, 2803, 4508, 5517, 6659, 7280, 7915, 8032, 8383, 9155, 9450, 15517, 17280, 28032
new assertion fails for 0 ports
  15 ports hit line 3: # bun ./bun.lockb --hash: A1A17280329F8383-20d6659d9c0623de-94508CB3B7915517-4b22a59b37f2f4f6

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 37 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: aee879d8-5d30-44c5-8c15-f11dd910edbb

📥 Commits

Reviewing files that changed from the base of the PR and between 2f5c180 and 84eea36.

⛔ Files ignored due to path filters (1)

• test/cli/install/__snapshots__/bun-install-registry.test.ts.snap is excluded by !**/*.snap

📒 Files selected for processing (1)

• test/cli/install/bun-install-registry.test.ts

---

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

[robobun]

Status: fix pushed, waiting on CI.

Reproduced by forcing registry.port to 7280 (the port from main build 95598) with the original assertion: bun bd test test/cli/install/bun-install-registry.test.ts -t "duplicate dependency in optionalDependencies maintains sort order" fails with the hash-line diff in the description, on both the debug build and the release binary. With this change the same command passes with the port forced to 7280, 8383 and 1728, and with a random port. Enumerating all 64511 possible ports against the snapshot: 15 broke the old assertion, 0 break the new one (details in the description).

[claude]

LGTM — test-only de-flake that swaps a bare-port replaceAll for the file's established localhost:\d+ normalization.

What was reviewed:

• Confirmed the new regex matches the pattern already used ~20× across test/cli/install/ for lockfile snapshots.
• Snapshot diff only rewrites the six resolved URLs from :4873 to :1234; the --hash: header line is untouched, so the assertion still guards the deterministic hash.
• No src/ changes; port was a shared module-level var, so no dead local left behind.

Extended reasoning...

Overview

One-line change in test/cli/install/bun-install-registry.test.ts (the duplicate dependency in optionalDependencies maintains sort order test) plus the six matching resolved URL lines in its .snap entry. The old normalization did out.replaceAll(${port}, "4873"), which replaced the bare port digits anywhere in the yarn-lockfile output — including inside the deterministic --hash: header — and flaked on ~15 of the 64k possible verdaccio ports. The new normalization uses replaceAll(/localhost:\d+/g, "localhost:1234"), which is the exact idiom already used throughout bun-install-registry.test.ts, bun-lock.test.ts, bun-workspaces.test.ts, and others.

Security risks

None. Test-only; no runtime code, no network, no auth.

Level of scrutiny

Low. This is a mechanical test-normalization fix that converges on an existing convention. The snapshot update is a direct consequence of changing the placeholder from 4873 to 1234; the hash line — the part the flake was corrupting — is byte-identical before and after, so assertion strength is preserved (it still catches sort-order regressions and hash changes).

Other factors

• Grep confirms this was the only remaining bare-port replaceAll in test/; the PR description's "only one left" claim holds.
• port is a module-scope variable used elsewhere in the file, so removing this reference leaves no dead code.
• No prior reviewer comments to address; the bug-hunting pass found nothing.

[robobun]

Review status: the automated review above found nothing to change, and a separate self-review of the diff (checking the premise against the snapshot, the other lockfile snapshots in this file, and whether the --hash: header can ever contain the port) came back without open concerns. The one addition from it is in the description's Background: the header is the lockfile meta hash, built from name@version lines (src/install/lockfile.rs generate_meta_hash), so it is port-independent by construction and must be compared as-is.

CodeRabbit has not reviewed yet (rate limited); nothing pending on this PR from my side. The only red lane on the first CI run was test/js/node/test/parallel/test-cluster-shared-leak.js timing out on Windows aarch64, which this test-only install change does not touch and which also times out on main's current build; CI was re-run once.

[claude]

LGTM — test-only de-flake that swaps a bare-port replaceAll for the file's standard localhost:\d+ normalization.

What was reviewed:

• Confirmed the new regex matches the convention already used at 18+ other sites in this test file.
• Snapshot diff touches only the six resolved URLs; the port-independent --hash: header line is left intact, which is the point of the fix.
• Checked that assertion strength is preserved (still an exact snapshot; only the normalization is narrowed).

Extended reasoning...

Overview

One-line change in test/cli/install/bun-install-registry.test.ts replacing out.replaceAll(\${port}`, "4873")without.replaceAll(/localhost:\d+/g, "localhost:1234"), plus the corresponding six resolved` URL lines in the committed snapshot. No source code touched.

Security risks

None. Test-only, no runtime behavior, no external inputs.

Level of scrutiny

Low. This is a mechanical de-flake that brings the last remaining bare-port normalization in this file into line with the pattern used everywhere else in the same file (verified by grep). The root cause — bare digit substitution corrupting the deterministic hash header when the random port's digits happen to appear in it — is clearly explained and easy to verify against the snapshot content.

Other factors

The PR description demonstrates fail-before/pass-after with forced ports and enumerates the 15 colliding ports. The snapshot's hash line is unchanged, confirming the meta hash is port-independent and the assertion is not being weakened — it's being made more precise (integrity strings and the hash header are now compared verbatim instead of being subject to accidental digit substitution). No outstanding review comments; no prior claude[bot] reviews on this PR.