install: don't mark a placed peer optional when saving a migrated bun.lock

[robobun]

Repro

bun install right after a package-lock.json migration is not a fixed point: the very next bun install --frozen-lockfile on an unchanged tree fails, and a plain re-install rewrites bun.lock.

mkdir -p vendor/gamma vendor/delta packages/ws-a
echo '{"name":"gamma","version":"1.0.0","peerDependencies":{"delta":"*"}}' > vendor/gamma/package.json
echo '{"name":"delta","version":"2.0.0"}' > vendor/delta/package.json
echo '{"name":"ws-a","version":"0.1.0","dependencies":{"delta":"file:../../vendor/delta"}}' > packages/ws-a/package.json
echo '{"name":"sandbox","version":"1.0.0","workspaces":["packages/ws-a"],"dependencies":{"gamma":"file:vendor/gamma"}}' > package.json
npm install --package-lock-only

bun install                    # migrates, writes bun.lock
bun install --frozen-lockfile  # error: lockfile had changes, but lockfile is frozen
bun install                    # silently removes the "gamma/delta" entry from bun.lock

Any project migrated from package-lock.json with this shape (a peer dependency of a file: package whose resolution is only placed nested, never hoisted to the root) fails its next --frozen-lockfile CI run.

Cause

When saving a lockfile loaded in the binary format (which includes the package-lock.json migration path), the writer checks every non-optional peer dependency for a resolution in the hoisted tree, and lists peers without one in optionalPeers so the saved file stays parseable. That lookup (PkgMap::find_resolution) walked up from the parent tree node's relative_path instead of from the package entry's own path. For the entry "gamma" it probed "delta" (root) instead of "gamma/delta" first, so a peer placed nested under the package itself was missed and marked optional, even though its resolution was written two lines below:

"gamma": ["gamma@file:vendor/gamma", { "peerDependencies": { "delta": "*" }, "optionalPeers": ["delta"] }],
"gamma/delta": ["delta@file:vendor/delta", {}],

On reload the parser adds Behavior::OPTIONAL to anything in optionalPeers, and since bbe3f6a (#35681) optional-peer resolution slots are re-derived from the rebuilt tree during cleaning instead of carried over. A nested-only placement is not visible from the dependent's position, so the re-derive leaves the slot empty and the "gamma/delta" entry is dropped: the frozen check fails, and a plain install rewrites the lockfile. Before #35681 the stale optional marking round-tripped unnoticed; #35681 exposed it.

Fix

Pass the package entry's own tree path (its packages key) to write_package_info_object, the exact string the parser passes to find_resolution when binding that entry's dependencies, so writer and parser agree on which placements a peer can see. The new probe set is a superset of the old one (it checks one level deeper first), so a peer can only stop being marked optional, never start.

A non-optional peer with genuinely no placement anywhere is still marked optionalPeers, which keeps such lockfiles loadable, and that state round-trips stably.

Verification

New test in test/cli/install/migration/migrate.test.ts covering the repro above: migration output must not mark the placed peer optional, --frozen-lockfile must pass on the unchanged tree, and a re-install must leave bun.lock byte-identical. Fails before this change (the lockfile contains "optionalPeers": ["delta"] and the frozen install exits 1), passes after.

Also ran migrate.test.ts, bun-lock.test.ts (including the #35681 optional-peer tests), bun-lockb.test.ts, migrate-bun-lockb-v2.test.ts, pnpm-lock-migration.test.ts, yarn-lock-migration.test.ts, hoist.test.ts, bun-workspaces.test.ts, isolated-install.test.ts, overrides.test.ts, lockfile-only.test.ts, and the peer/optional filters of bun-install-registry.test.ts: no failures.

There is a second, independent way to break the same install && install --frozen-lockfile invariant, via the optionalDependencies + peerDependenciesMeta idiom on a file: package (a duplicate same-name tree entry from the folder-dependency hoist shortcut). That one is already fixed by the Tree.rs dedupe hunk in #33156, so it is intentionally not duplicated here; details posted on that PR.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/migration/migrate.test.ts

[robobun]

^{Updated 2:34 PM PT - Aug 9th, 2026}

❌ @robobun, your commit 60cb27f has 1 failures in Build #91071 (All Failures):

• test/napi/napi.test.ts - code 1 on 🍎 14 x64

---

🧪   To try this PR locally:

bunx bun-pr 37289

That installs a local version of the PR into your bun-37289 executable, so you can run:

bun-37289 --bun

[coderabbitai]

Walkthrough

Package serialization now writes complete package tree paths and passes them to package metadata and peer dependency resolution. A migration regression test covers nested file dependency peers, frozen installs, and repeat-install lockfile stability.

Changes

Lockfile package path resolution

Layer / File(s)	Summary
Serialize complete package paths src/install/lockfile/bun.lock.rs	Package keys now combine the full tree path with the dependency name. Package metadata serializers receive this complete path.
Resolve peers from package paths src/install/lockfile/bun.lock.rs, test/cli/install/migration/migrate.test.ts	Package and workspace peer lookups use the package’s own path. The migration test verifies nested peer placement, frozen installs, and unchanged repeated installs.

Possibly related PRs

• oven-sh/bun#35681: Both PRs modify lockfile dependency resolution and optional-peer handling.
• oven-sh/bun#37142: Both PRs modify peer and package resolution paths in src/install/lockfile/bun.lock.rs.
• oven-sh/bun#37245: Both PRs update package-path and lockfile-key resolution in src/install/lockfile/bun.lock.rs.

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 main fix: preventing placed peers from being marked optional in migrated bun.lock files.
Description check	✅ Passed	The description explains the problem, cause, fix, regression test, and verification results in sufficient detail.

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 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/migration/migrate.test.ts`:
- Around line 488-494: Shorten the comment above the gamma/delta fixture to two
concise lines that preserve only the invariant: delta is a regular peer
dependency with nested-only placement and must not appear in gamma’s
optionalPeers. Remove the detailed reload, resolution-slot, and frozen-lockfile
explanation.

🪄 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: ed1a95f3-7517-40ce-afa7-44881af39ddf

📥 Commits

Reviewing files that changed from the base of the PR and between cbc209f and 72795f0.

📒 Files selected for processing (2)

• src/install/lockfile/bun.lock.rs
• test/cli/install/migration/migrate.test.ts

[claude]

LGTM — targeted fix aligning the writer's peer-resolution probe with the parser's.

What was reviewed:

• The entry-key write is byte-identical to before (both halves already went through format_json_string_utf8 with quote: false, and / is not JSON-escaped; relative_path already carried interior slashes through the same escaper for depth>1).
• find_resolution now starts from the entry's own key instead of its parent's — the probe set is a strict superset (adds entry_path/dep_name first, then walks up as before), so peers can only stop being marked optional, never start.
• write_workspace_deps change is a parameter rename only; both callers already passed the workspace's own tree path (b"" for root, pkg_names[id] otherwise).
• New test is hermetic (all file: deps, no network), uses the shared install() helper which drains stderr concurrently, and asserts the fixed-point invariant directly.

Extended reasoning...

Overview

The PR fixes a lockfile round-trip bug in src/install/lockfile/bun.lock.rs: when serializing a binary/migrated lockfile to bun.lock, the writer checks each non-optional peer for a resolution via PkgMap::find_resolution and marks it optionalPeers if none is found. That lookup was seeded with the parent tree node's relative_path instead of the package entry's own key, so a peer placed only nested under the package (e.g. "gamma/delta") was invisible and wrongly marked optional. On reload, #35681's re-derive logic then dropped the entry, breaking --frozen-lockfile on an unchanged tree. The fix builds entry_path = relative_path[/]dep_name once per entry (also used as the JSON key, replacing three writes with one) and passes it to write_package_info_object. The write_workspace_deps hunk is a parameter rename with no caller changes. A new concurrent test in migrate.test.ts reproduces the migration → frozen-lockfile → reinstall sequence.

Security risks

None. This is lockfile-writer path-resolution logic operating on already-parsed internal data structures; no untrusted input handling, no filesystem writes beyond the lockfile, no network. The entry-key output is byte-identical to before.

Level of scrutiny

Medium. Lockfile serialization is user-visible and affects --frozen-lockfile in CI, so a regression would be widely felt. However, the change is narrow and provably monotone: find_resolution walks up the tree from its seed path, and the new seed (entry_path) is exactly one level deeper than the old seed (relative_path), so every probe the old code did is still done — the fix only adds the entry_path/dep_name probe first. A peer previously found is still found; only previously-missed nested placements are now seen. The written key is unchanged (verified that / passes through format_json_string_utf8 unescaped, and relative_path already contained slashes for depth>1 entries going through the same escaper).

Other factors

The test is well-constructed: hermetic (tempDir, all file: dependencies, no registry), uses the file's existing install() helper (which drains stderr and awaits exit concurrently), runs as test.concurrent, and asserts the strongest invariant — byte-identical lockfile after re-install plus --frozen-lockfile exit 0. The PR description reports passing runs across the migration, bun-lock, hoist, workspace, isolated, and peer/optional test suites. All comment-cop and CodeRabbit feedback (comment length) has been addressed and resolved. entry_path_buf is declared once outside the loop and clear()ed per iteration, avoiding per-entry allocation.

[robobun]

CI on 60cb27f: the only hard failure is test/napi/napi.test.ts on macOS 14 x64, which is pre-existing on main and reported separately. The remaining red entries are retry-passed flakes on lanes this diff does not touch (install tests on Windows aarch64 and macOS, http2 and fetch-backpressure on macOS, and parallel-batch timeouts that passed when rerun alone). The lockfile change and the new migration round-trip test are green on every lane.

[robobun]

Closing in favor of #38333, which carries the same find_resolution path change in bun.lock.rs (the writer now probes from the package entry's own path) and lifts this PR's migrate.test.ts case.

[Jarred-Sumner]

Thanks for this PR, @robobun — closing it in favor of #38333, which reworks the whole package-lock.json migration and carries this fix inside it.

Your repro: right after migrating, bun.lock marked gamma's peer delta as optionalPeers even though "gamma/delta" was placed two lines below. The next plain install then dropped that entry.

$ bun install                    # migrates package-lock.json -> bun.lock, no optionalPeers
$ bun install --frozen-lockfile  # exits 0 (was: lockfile had changes, but lockfile is frozen)
$ bun install                    # bun.lock is byte-identical

• Same fix as yours: the writer's peer lookup walks up from each packages entry's own key (e.g. "gamma"), so writer and parser agree on visible placements.
• It applies to every lockfile saved from the binary format, not just migrated ones.
• Your regression test was lifted as-is into test/cli/install/migration/migrate.test.ts.
• Migration output for all 57 arborist fixture projects is snapshotted alongside it, and install: pnpm parity — dedupe, prune, pm licenses, audit fix, add --filter/--catalog, nested overrides, transitive update, and workspace fixes #38333 credits this PR.

It'll be in the next canary (bun upgrade --canary) and in Bun v1.4 — if you still see the frozen-lockfile error right after a migration, comment here with the package-lock.json.

(This comment was written by Claude, on behalf of the Bun team.)

[robobun]

Sounds good, thanks. Confirmed #38333 is merged with the same writer-side lookup (walking up from each entry's own key) and the round-trip test, so this is covered. One note for anyone landing here from the frozen-lockfile symptom: the second shape of this bug (a file: package using optionalDependencies plus peerDependenciesMeta, no foreign lockfile involved) is a different mechanism, a duplicate same-name tree entry from the folder-dependency hoist shortcut, and is tracked in #33156.