Skip to content

install: apply overrides to the range that bun add and bun update write - #46230

Merged
Jarred-Sumner merged 1 commit into
mainfrom
robobun/d8aee377/override-decides-written-range
Oct 22, 2026
Merged

Jarred-Sumner merged 1 commit into
mainfrom
robobun/d8aee377/override-decides-written-range

Conversation

@robobun

@robobun robobun commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • On main, bun update and bun add drop an override for the name of a workspace package. If that package satisfies the range they write, bun.lock binds it and not the version the rule chose. Exit 0, no message. 1.4.3 is not affected.
  • Cause: Lockfile::bind_workspace_link (src/install/lockfile.rs:2451, from install: fix regressions and older bugs found after the package manager consolidation #45307) binds every rewritten range that links a workspace package and does not ask the overrides.

Fix

  • bind_workspace_link binds only if linked_workspace_path links the workspace package for the version the override gives. settle_without_manifest decides the same way on a clean install.
  • sync_lockfile binds after it copies the edited overrides. An override value can be the range the command wrote ("$name").
  • Verified: test/cli/install/bun-workspaces.test.ts (48 of 54 new cases fail on main) and 34 other install suites. Self-reviewed: 8 concerns raised, 7 addressed.

Background

Downsides

  • edit_after_resolve_slow grows by 2,009 bytes. A command runs 20 more instructions per rewritten range and allocates one Vec. A plain install is unchanged.
  • Still wrong: two rules for one name, where the rule for the written range names another package. The next --frozen-lockfile install fails, as on 1.4.3.
Notes

Repro

Registry: the test registry of test/cli/install (no-deps has 1.0.0, 1.0.1, 1.1.0, 2.0.0).

  • package.json: {"name":"root","workspaces":["packages/*"],"overrides":{"no-deps":"1.0.0"}}
  • packages/m/package.json: {"name":"no-deps","version":"1.0.5"}
  • packages/bar/package.json: {"name":"bar","dependencies":{"no-deps":"^2.0.0"}}
  • bun install: bar gets the registry's no-deps@1.0.0.
  • cd packages/bar && bun update: writes "no-deps": "^1.0.0". On main bar now gets the workspace package 1.0.5, and bun.lock has no bar/no-deps entry. With this PR bar keeps no-deps@1.0.0.
  • Remove bun.lock and node_modules, then bun install: no-deps@1.0.0 on both builds.

Which build fails the new cases

  • The 48 cases are 4 rules ("no-deps" in overrides, "bar>no-deps", "no-deps" in resolutions, "no-deps": "latest"), 6 commands (update, update no-deps, update --latest, add no-deps in the workspace, update -r and update --filter bar in the root) and 2 linkers. Each case compares bun.lock with the one that a clean install writes.
  • They fail on a debug and on a release build of main (c1df64b). The regression is not in a release: on 1.4.3-canary.1, 44 of the 48 pass. The other 4 are update --filter bar with the hoisted linker.
  • The 6 other new cases pass on main. Two pin a rule whose version the workspace package satisfies. Four pin that the package links after a command when the rule for the written range accepts it, or when the rule is "$no-deps" and takes the written range. The first version of this fix kept every dependency that had a rule, and failed those four.

Cells

  • bun add and bun update in a workspace, 13 commands, 2 linkers, 3 combinations of member version, rule value and declared range, under a rule without a selector: 78 cells. With this PR every cell has the bun.lock of a clean install. On main 12 differ.
  • The same 78 cells under a rule with a version selector (no-deps@^1.0.0): equal to a clean install on both builds. A selector does not match a range that links a workspace package, so this PR changes nothing there.
  • --dev, --optional and --peer forms of bun add, and bun update with a peer range (18 cells): equal to a clean install. A peer gets no copy, and it already was bound to the workspace package.

What a project pays that does not hit the bug

Release builds without LTO of main (c1df64b) and of this change on that commit.

  • Size (llvm-size): text 101,521,860, then 101,523,860. Data and bss are equal. By symbol (llvm-nm -S): only package_json_write_back::edit_after_resolve_slow changes, 15,505 to 17,514 bytes. sync_lockfile and bind_workspace_link are inlined there.
  • Instructions (qemu-x86_64 -one-insn-per-tb -d exec,nochain -dfilter on the symbol ranges, two runs per build):
    • install --lockfile-only on 6 fixtures (no rules, a rule for another name, a rule with a selector and 8 dependents, a rule with a selector on a member name, a rule without a selector on a member name, no workspaces): lookup_selecting, bind_row, parse_with_tag, semver_query::parse_with and tag_workspace_links run the same number of instructions on both builds. edit_after_resolve_slow is not entered.
    • bun update that rewrites 3 ranges, none links a workspace package: edit_after_resolve_slow 1,148, then 1,208.
    • bun update --latest that writes a range that links a workspace package, no rule for the name: edit_after_resolve_slow 1,016, then 1,072. lookup_selecting 87, then 116: one rule lookup for each rewritten range that links.
  • Lockfiles: bun.lock and bun.lockb are byte-identical between the two builds on 6 of 6 fixtures, and each build installs the other's file with --frozen-lockfile.
  • valgrind and bloaty are not in the container. The allocation count is from the code: one Vec<DependencyID> for a command that rewrites at least one range.

The case that is still wrong

  • "overrides": {"no-deps": "npm:a-dep@1.0.1", "no-deps@^2": "1.0.0"}, workspace package 1.0.5, bar declares ^2.0.0. bun update in bar writes ^1.0.0. A clean install then gives a-dep@1.0.1.
  • main: the workspace package, and --frozen-lockfile passes. This PR and 1.4.3-canary.1: no-deps@1.0.0, and --frozen-lockfile fails once. A plain bun install corrects it.
  • With two rules for no-deps itself ("no-deps": "1.1.0" beside "no-deps@^2": "1.0.0") this PR gives 1.1.0, as a clean install does. main gives the workspace package.

Policy

Self-review

Addressed:

  • The guard decides by the version of the rule and not by its existence.
  • The overrides are synced before the rows are bound.
  • Parent-scoped rules, resolutions, bun add, update -r and update --filter are tested.
  • The failing baseline is named.
  • Comments are one line.
  • The test helper stops at the filesystem root.
  • The change for rules with a version selector is not in this PR.

Not taken: one decider through the resolver (see Background).

Suites run on a debug build of this branch

bun-workspaces (339), overrides, nested-overrides, catalogs, bun-install, bun-install-registry, bun-lock, bun-lockb, lockfile checks under frozen-lockfile-* (4 files), bun-update, bun-update-lockfile-sync, bun-update-transitive, bun-add, bun-add-filter, bun-add-catalog, bun-dedupe, bun-audit, bun-remove, bun-prune, isolated-install, hoist, bun-workspaces-self-contained, and the 10 files of migration/. 0 failures with a per-test timeout of 180 s. The default 5 s times out under the sanitizer build in this container.

`bun add` and `bun update` write the range of the version they
resolved to package.json, and `sync_lockfile` copies that range into
the lockfile. If a workspace member satisfies the new range,
`Lockfile::bind_workspace_link` bound the dependency to the member. It
did not ask if an override replaces the dependency, so the command
dropped a rule that a clean install of the same package.json applies.

`bind_workspace_link` now asks `linked_workspace_path` for the version
of the override, as the resolver does. The dependency keeps its package
if that version does not link the member. The rows are bound after the
overrides of the edited files are copied into the lockfile, because the
value of an override can be a range that the command wrote.
@coderabbitai

coderabbitai Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 334030ec-a8fb-4c90-917a-48f7bee77ce9
📥 Commits

Reviewing files that changed from the base of the PR and between ea6a2c7 and d4ad217.

📒 Files selected for processing: 3
  • src/install/PackageManager/package_json_write_back.rs
  • src/install/lockfile.rs
  • test/cli/install/bun-workspaces.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@robobun

robobun commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 4:02 PM PT - Oct 20th, 2026

✅ @robobun, your commit d4ad217f6f97bfe28c3bb06c9c7fdf5186374088 passed in Build #129608! 🎉


🧪   To try this PR locally:

bunx bun-pr 46230

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

bun-46230 --bun

@robobun

robobun commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on main (c1df64b) with the test registry of test/cli/install:

  • Root package.json: "workspaces": ["packages/*"], "overrides": {"no-deps": "1.0.0"}.
  • packages/m is the workspace package no-deps@1.0.5. packages/bar depends on no-deps@^2.0.0.
  • bun install: bar gets the registry's no-deps@1.0.0.
  • bun update in packages/bar: it writes "no-deps": "^1.0.0", and bar now gets the workspace package. bun.lock has no bar/no-deps entry.
  • A clean install of the same files gives no-deps@1.0.0. 1.4.3-canary.1 keeps no-deps@1.0.0 after the command.

The fix is this PR: #46230.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the bind_workspace_link signature change — sync_lockfile is its only caller, so no stale call site misses the new flag — and the deferred-binding reorder: the rows are bound only after both the catalog and override syncs, so the guard reads the overrides the command just wrote rather than the pre-edit ones.

Extended reasoning...

The change touches the lockfile workspace-link binding in src/install/lockfile.rs and the package.json write-back sync in src/install/PackageManager/package_json_write_back.rs, plus the bun-workspaces test suite; it touches no security-sensitive surface. Two confirmed findings are posted inline (the two-rules-for-one-name lockfile that frozen-lockfile rejects, and already-affected lockfiles not being repaired), so a human should weigh them.

Comment thread src/install/lockfile.rs Resolved
@Jarred-Sumner
Jarred-Sumner merged commit c6dd13d into main Oct 22, 2026
11 of 12 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the robobun/d8aee377/override-decides-written-range branch October 22, 2026 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants