install: keep the scope when a yarn.lock npm: alias points at a scoped package

[robobun]

Problem

• Migrating a yarn v1 lockfile whose npm: alias points at a scoped package records the unscoped tail of the name. For "my-node-types@npm:@types/node@20.11.5" resolved from https://registry.yarnpkg.com/@types/node/-/node-20.11.5.tgz#..., bun.lock gets "my-node-types": ["node@20.11.5", ...] instead of ["@types/node@20.11.5", ...], and the post-migration manifest pass asks the registry for node. The same migration runs inside bun install when only a yarn.lock exists, which is the GET https://registry.npmjs.org/monaco-editor-treemended/-/monaco-editor-treemended-1.83.16.tgz - 404 reported in yarn.lock migration strips scope from npm: aliased packages, causing 404s #27781. Reproduces on the released canary (b7a0431) and on main.
• Cause: Entry::get_package_name_from_resolved_url (src/install/yarn.rs:239), which names alias entries after their tarball URL, walks back from /-/ and checks the last path segment for a leading @. In a scoped tarball URL the last segment is the unscoped name and the scope is the segment before it, so the check never fires and only the last segment is returned. The Zig original had the same bug; fix(install): preserve scope in scoped packages during yarn.lock migration #27782 and fix: yarn.lock migration preserves @scope for npm: aliased packages #27889 fixed it there and were closed when the Zig sources went away.

Fix

• get_package_name_from_resolved_url now splits the path before /-/ into its last segment and the one before it, and returns both when the earlier one starts with @, otherwise just the last one. It returns None (callers then fall back to the spec name) when there is no name segment, where the old code returned an off-by-one slice or an empty name.
• Why this is correct: the registry tarball layout is <registry>/<name>/-/<basename>-<version>.tgz, and <name> is one segment for plain packages and exactly two (@scope/name) for scoped ones, so the segment before the name starting with @ is the only thing that distinguishes the two shapes; nothing else in the URL encodes it. Plain packages take the same path as before, so their output is unchanged.
• All three callers benefit: the package name written to the lockfile (yarn.rs:941), the (name, version) key used to give the alias and a plain entry of the same package one package id (yarn.rs:843), and the alias detection that registers an entry under its spec name only when it differs from the real name (yarn.rs:1603), which previously fired for every scoped package.
• Left alone on purpose: Entry::parse_npm_alias also mis-splits npm:@scope/name@range at the scope's @, but its only consumers are the first dependency pass, whose slices are overwritten by the second pass at yarn.rs:1695 onward, and a version field spelled npm:..., which yarn v1 never writes. Nothing observable depends on it.
• Test: test/cli/install/migration/yarn-lock-migration.test.ts, "yarn.lock with npm aliases of a scoped package keep the scope". One migration covers an unscoped alias, a scoped alias and a transitive alias of @types/node@20.11.5 (sharing one yarn entry, as yarn writes patterns that resolve to the same tarball) next to a plain @types/node@18.19.0, asserts the full packages object of the resulting bun.lock, and runs against a loopback registry that records which manifests the migration asks for (@types/node and the consumer, no node). Fails on the released build with the three alias entries reading node@20.11.5, passes with this change.
• The rest of yarn-lock-migration.test.ts (scoped-package and real-world fixtures, 14 snapshots), migrate.test.ts and lockfile-only.test.ts pass unchanged with the debug build; the yarn-cli-repo output is byte-identical to its snapshot. That case still brushes its 5s timeout under debug+ASAN, which is pre-existing and tracked by test(install): fix yarn-lock-migration yarn-cli-repo case under debug+ASAN #35377.
• No overlap with the other open yarn migration PRs: install: fix default-trusted lifecycle scripts being blocked after yarn.lock migration #38795 and install: fix yarn.lock migration panic on tarball URLs with "/-/" right after the host #38803 rework the name_to_use block for tarball entries and leave this function as is (after install: fix default-trusted lifecycle scripts being blocked after yarn.lock migration #38795 it becomes the only source of alias package names).

Fixes #27781

Background

• A yarn v1 npm: alias entry is keyed by the alias spec (alias@npm:real-name@range) and carries the real package's version, resolved tarball URL and integrity. The migrator stores the package under its real name and hangs it in the tree under the alias, so it needs the real name, and it currently recovers it from the tarball URL.
• bun.lock packages entries are ["<real name>@<version>", "<registry, empty for the default>", {meta}, "<integrity>"] keyed by the node_modules path. The first element is what bun install later downloads, which is why a wrong name there turns into a 404 or a different package.
• After migrating, fetch_necessary_package_metadata_after_yarn_or_pnpm_migration fetches the manifest of every migrated npm package by the name recorded in the lockfile to fill in bin/os/cpu. The test points that pass at a loopback server and uses the requested names as a second, external view of which package each alias was resolved to.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 20 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: 7b1677e9-d512-4155-8832-c83865e86312

📥 Commits

Reviewing files that changed from the base of the PR and between a5c86ae and e76ecc0.

📒 Files selected for processing (2)

• src/install/yarn.rs
• test/cli/install/migration/yarn-lock-migration.test.ts

---

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

[robobun]

^{Updated 8:05 PM PT - Aug 14th, 2026}

@robobun, your commit e76ecc0 is building: #97086

[robobun]

Status: merged (e9b5e63), #27781 closed.

Reproduced on the released canary (b7a0431) with a two-line project: package.json depending on "my-node-types": "npm:@types/node@20.11.5" plus the matching yarn v1 entry, then bun pm migrate -f. The migrated bun.lock read "my-node-types": ["node@20.11.5", ...] and the manifest pass requested /node from the registry; with this change it reads ["@types/node@20.11.5", ...] and requests @types/node.

Self-review surfaced a broader follow-up that this PR does not cover and whose regression guard it provides: alias entries are still identified by the alias name or the tarball URL rather than the alias@npm:<real>@<range> spec, so aliases on registries without the <name>/-/ tarball layout still get the alias name recorded, and two different targets sharing one alias name and version are consolidated into a single package by consolidate_and_append_entry. That is being tracked separately.

[claude]

LGTM — focused fix to get_package_name_from_resolved_url so scoped tarball URLs yield @scope/name instead of the unscoped tail.

What was reviewed:

• Traced the new segment-split logic against scoped (@types/node/-/…), unscoped (lodash/-/…), and degenerate (foo/-/…, /-/…) URLs; scoped now returns @types/node, unscoped is unchanged, and malformed inputs return None where the old loop returned an off-by-one slice — all three callers already fall back to the spec name on None.
• Confirmed strings::rsplit_once_char / last_index_of_char exist in bun_core::string with the expected semantics.
• Test is hermetic (loopback Bun.serve registry, temp cache dir), drains pipes concurrently, asserts the full packages object plus the requested-manifest set, and follows the file's existing tempDir/bunEnv conventions.

Extended reasoning...

Overview

Rewrites Entry::get_package_name_from_resolved_url in src/install/yarn.rs (~12 lines net) to correctly extract the package name from a registry tarball URL when the package is scoped, and adds one test to test/cli/install/migration/yarn-lock-migration.test.ts covering direct, scoped-alias, and transitive npm:@scope/name aliases plus a co-existing plain @types/node entry.

Security risks

None. The function is a pure byte-slice parser over a URL string already read from a yarn.lock file; it only affects which package name is written into the migrated bun.lock. No new I/O, allocation, or unsafe code; the ? on index_of/rsplit_once_char guards every slice.

Level of scrutiny

Low-to-medium. The change is confined to one 10-line helper in the yarn v1 lockfile migrator (a one-shot CLI path, not runtime hot code) and a purely additive test. The three call sites are enumerated in the PR description; I checked that the behavioural change on None (previously an off-by-one slice) is handled by each caller's existing .unwrap_or_else(|| get_name_from_spec(...)) fallback, and that the third caller's alias-detection loop simply stops firing spuriously for non-aliased scoped packages.

Other factors

• The new implementation reuses the in-tree bun_core::strings helpers (rsplit_once_char, last_index_of_char) rather than the hand-rolled backward loop, which is idiomatic per the repo's "grep for the in-tree helper" rule.
• The test uses a loopback registry and the recorded manifest names as an external witness of the fix, so it fails observably on the released build (aliases would resolve to node) — matching the REVIEW.md "prove the test fails for the right reason" requirement.
• No CODEOWNERS entry covers src/install/ or this test file.
• PR description notes the existing 14 snapshots (including the large yarn-cli-repo fixture with many scoped packages) pass byte-identically, so the unscoped path is regression-covered.