Repository navigation
Conversation
…ot bun.lock's When packing a workspace package that depends on another workspace package via workspace:*, workspace:^ or workspace:~, bun pm pack resolved the substituted version from lockfile.workspace_versions. That value is whatever was in the dependency's package.json at the last bun install, so bumping a workspace package's version and packing a dependent without regenerating the lockfile produced a tarball pinned to the old version. Read the dependency's package.json from disk (located via lockfile.workspace_paths) and use its version field. Fall back to lockfile.workspace_versions only if the file is missing or unparseable so the existing behavior is preserved when the package.json cannot be read. Fixes #20477 Fixes #20829
|
Updated 11:20 PM PT - Jul 28th, 2026
❌ @robobun, your commit 199974e has 1 failures in
🧪 To try this PR locally: bunx bun-pr 36279That installs a local version of the PR into your bun-36279 --bun |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
WalkthroughChangesThe packing command now prefers current on-disk workspace versions when rewriting Workspace packaging
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@src/runtime/cli/pack_command.rs`:
- Around line 3300-3329: Update read_workspace_version_from_package_json to trim
the parsed version using the existing whitespace utilities before validating or
copying it. Reject the value when the trimmed version is empty, and return the
trimmed version so workspace versions cannot produce malformed specifications.
In `@test/cli/install/bun-pack.test.ts`:
- Around line 694-745: Add fallback coverage alongside
staleLockfileWorkspaceProtocolTests by deleting or corrupting
pkgs/pkg-a/package.json after bun install for each workspace protocol. Assert
pack still succeeds and the generated package.json dependency uses the
lockfile-cached workspace version, exercising
read_workspace_version_from_package_json’s lockfile.workspace_versions fallback
through the existing pack flow.
🪄 Autofix (Beta)
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: 215e882d-aa2d-4ff2-959a-3c97f89e3950
📒 Files selected for processing (2)
src/runtime/cli/pack_command.rstest/cli/install/bun-pack.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — the comment-trim from the earlier review landed in 199974e, and the fallback-path test was added in b5d2644.
What was reviewed:
read_workspace_version_from_package_jsonpath resolution:workspace_pathsentries are relative toFileSystem::instance().top_level_dir(), whichPackageManager::initsets to the workspace root even when packing from a subdirectory — the test atpkgs/pkg-bexercises this.- Lifetime of the returned slice: bytes are copied into the
pack_bump()thread-local arena beforesourcedrops, so nothing borrows freed memory. - Sibling sites:
edit_root_package_jsonis the onlyworkspace_versionsconsumer undersrc/runtime/cli/;bun publishreuses pack, so no parallel fix needed. - Fallback ordering preserves the prior error path (
break 'failed_to_resolve false) when both disk and lockfile miss.
Extended reasoning...
Overview
The PR changes bun pm pack's workspace-protocol rewriting so workspace:* / workspace:^ / workspace:~ resolve to the dependency's current on-disk package.json version rather than the version cached in bun.lock at last install. A new helper read_workspace_version_from_package_json reads and parses the dependency's manifest via the path already stored in lockfile.workspace_paths; on any I/O or parse failure it returns None and the caller falls back to the pre-existing lockfile.workspace_versions lookup, and from there to the pre-existing hard error. Four tests are added alongside the existing workspace-protocol coverage in test/cli/install/bun-pack.test.ts.
Security risks
None identified. The only new input surface is reading a workspace member's package.json — a file the user already controls and that bun install already parses. Path construction uses resolve_path::join_abs_string_buf_z against the workspace root with a lockfile-recorded relative path; no user-controlled traversal is introduced beyond what the lockfile already encodes.
Level of scrutiny
Low-to-moderate. This is a narrow behavioral fix inside a single CLI command, ~30 net lines of Rust plus tests, with a graceful fallback to the previous behavior. It touches no memory-lifetime-sensitive machinery beyond the existing pack_bump() arena pattern already used throughout the file, and the helper's owned Source outlives the borrowed version slice until it is copied into the arena. It fixes three linked user-reported issues and matches pnpm's documented semantics.
Other factors
All prior review threads on this PR are resolved: my earlier comment-style nit was addressed in 199974e (the src/ comment now reads // on-disk version is authoritative; the lockfile's cached version may be stale), CodeRabbit's fallback-coverage request was addressed in b5d2644, and the comment-cop flags are resolved. CodeRabbit's remaining whitespace-trim suggestion is a degenerate-input edge case (a "version": " " field) that is not a regression relative to the old lockfile path and was marked resolved. The bug-hunting system found nothing this run. CI build #84560 is running against the head commit.
There was a problem hiding this comment.
LGTM — the comment nits and fallback-coverage feedback from the earlier pass are all addressed.
What was reviewed:
read_workspace_version_from_package_jsonreuses the file's existing patterns (pack_bump()arena,File::read_from+parse_package_json_utf8as at lines 65–67/1065); the arena-copied return is lifetime-correct.- Checked that the on-disk read is keyed off
lockfile.workspace_pathsjoined againstFileSystem::instance().top_level_dir(), which the sub-workspace tests exercise. - Tests cover
workspace:*/^/~with a stale lockfile plus the no-version fallback branch; PR evidence shows they fail on main and pass here. - CodeRabbit's whitespace-only-
versionnote is a pre-existing edge case (lockfile path had the same behavior) and not a regression.
Extended reasoning...
Overview
Changes bun pm pack's edit_root_package_json so that when rewriting workspace:* / workspace:^ / workspace:~ dependency specifiers, it reads the dependency's current on-disk package.json version (via lockfile.workspace_paths) instead of the potentially-stale lockfile.workspace_versions. Adds a small helper read_workspace_version_from_package_json and falls back to the old lockfile-cached value on any I/O/parse failure. Four new tests in bun-pack.test.ts cover the three protocol variants with a stale lockfile plus the fallback-when-version-missing case.
Security risks
None. The only new I/O is reading a package.json at a path the lockfile already records, from within a CLI tool the user explicitly invoked. No untrusted input crosses a boundary that wasn't already crossed by the existing lockfile-loading code.
Level of scrutiny
Moderate. This is a focused ~40-line bugfix in a CLI command (not a hot runtime path), using established file-local idioms — the same File::read_from → Source::init_path_string_owned → JSON::parse_package_json_utf8 chain and pack_bump() arena already appear at lines 65–67 and 1065 of the same file. The returned &'static [u8] is arena-copied via alloc_slice_copy, so nothing borrows the dropped source/bytes. The else-chain preserves the pre-existing break 'failed_to_resolve false behavior when neither source yields a version.
Other factors
- All prior review threads are resolved: the comment-cop / claude comment-style nit was addressed in 199974e (comment now reads
// on-disk version is authoritative; the lockfile's cached version may be stale), and CodeRabbit's request for fallback-path coverage was addressed in b5d2644 with the "falls back to lockfile version when workspace package.json has no version" test. - PR evidence shows the new tests fail on main (3 FAILED under ASAN, 5 under release) and pass with the fix, satisfying the "prove the test fails for the right reason" bar.
- CodeRabbit's remaining minor note about trimming whitespace-only
versionstrings is a pre-existing edge case shared with the lockfile path and doesn't warrant blocking; theis_empty()guard already handles the common empty-string case. - Fixes three linked user-reported issues (#20477, #20829, #28935) and matches pnpm's documented behavior.
|
CI builds #84560 and #84681 both failed on unrelated infrastructure:
The new |
|
#38813 takes a different route to the same bug and also covers |
|
Closing in favor of #38813, which fixes #20477 by not reading Checked against a build of #38813: the three stale-lockfile tests from this PR ( |
2 participants
What does this PR do?
When packing a workspace package that depends on another workspace package via
workspace:*,workspace:^orworkspace:~,bun pm packresolved the substituted version fromlockfile.workspace_versions. That map holds whatever was in the dependency'spackage.jsonat the lastbun install, so bumping a workspace package's version and packing a dependent without regenerating the lockfile produced a tarball pinned to the stale version. This is the common flow for changesets/lerna/release-please style release tooling, and it contradicts the docs ("replaced by the package'spackage.jsonversion").Repro
Fix
edit_root_package_jsonnow reads the dependency'spackage.jsonfrom disk (its path is already inlockfile.workspace_paths) and uses itsversionfield. It falls back tolockfile.workspace_versionsonly if the file cannot be read or parsed, so behavior is unchanged when the package.json is missing.This matches pnpm's behavior.
How did you verify your code works?
Added parameterized tests to
test/cli/install/bun-pack.test.tscoveringworkspace:*,workspace:^,workspace:~with a lockfile that is stale relative to the on-diskpackage.json, plus a fallback case where the dependency'spackage.jsonhas no version field. The first three fail on main and pass with this change; the fallback case passes in both. Existing workspace-protocol tests still pass.Fixes #20477
Fixes #20829
Fixes #28935
[review] gate passed · iteration 1 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 1
evidence per changed file