Skip to content

fix(lockfile): skip preprocessUpdateRequests for global installs - #28252

Closed
BillionClaw wants to merge 1 commit into
oven-sh:mainfrom
BillionClaw:clawoss/fix-28247-global-update-assertion
Closed

BillionClaw wants to merge 1 commit into
oven-sh:mainfrom
BillionClaw:clawoss/fix-28247-global-update-assertion

Conversation

@BillionClaw

Copy link
Copy Markdown

Summary

Fixes #28247

When running bun update -g --latest, the preprocessUpdateRequests function was being called for global installs. This caused an assertion failure when global packages had workspace dependencies that couldn't be resolved in the global context.

Changes

Modified cleanWithLogger in src/install/lockfile.zig to skip preprocessUpdateRequests for global installs since they don't have a proper workspace context.

Testing

This fix prevents the assertion failure by not attempting workspace-specific dependency preprocessing on global installs. The global update flow will still work correctly without this preprocessing step.

When running 'bun update -g --latest', the preprocessUpdateRequests
function was being called for global installs. This caused an assertion
failure when global packages had workspace dependencies that couldn't
be resolved in the global context.

This fix skips preprocessUpdateRequests for global installs since
global installs don't have a proper workspace context and don't need
the same dependency version preprocessing as local workspace installs.

Fixes oven-sh#28247
@coderabbitai

coderabbitai Bot commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a13a5d49-93e1-4d73-9544-5fe8336f505c

📥 Commits

Reviewing files that changed from the base of the PR and between a7e9abe and 336b5bf.

📒 Files selected for processing (1)
  • src/install/lockfile.zig

Walkthrough

Modified the condition in Lockfile.cleanWithLogger to include an additional check for global install mode. The preprocessing of update requests now only occurs when updates exist and the global flag is not set, preventing execution during global installs.

Changes

Cohort / File(s) Summary
Global Install Handling
src/install/lockfile.zig
Tightened the update preprocessing condition in Lockfile.cleanWithLogger from if updates.len > 0 to if updates.len > 0 and !manager.options.global to prevent preprocessing during global package installation.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: skipping preprocessUpdateRequests for global installs to fix an assertion failure.
Description check ✅ Passed The PR description provides context about the issue, explains the fix, and describes testing approach, though it deviates from the template structure by adding a 'Summary' and 'Changes' section instead of following the template's 'What does this PR do?' and 'How did you verify?' format.
Linked Issues check ✅ Passed The PR directly addresses issue #28247 by preventing preprocessUpdateRequests from being called during global installs, which resolves the assertion failure when global packages contain workspace dependencies.
Out of Scope Changes check ✅ Passed The change modifies only the lockfile.zig file's cleanWithLogger function to add a global install check, which is directly scoped to fixing the reported global update assertion failure.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

@CMLKevin

Copy link
Copy Markdown

This is BillionClaw. Happy to discuss the approach or make adjustments to the fix.

@CMLKevin

Copy link
Copy Markdown

I'll get the CLA signed — will follow up once it's done.

1 similar comment
@CMLKevin

Copy link
Copy Markdown

I'll get the CLA signed — will follow up once it's done.

@CMLKevin

Copy link
Copy Markdown

I’ll get the CLA signed — will follow up once it’s done.

@BillionClaw

Copy link
Copy Markdown
Author

Thank you for the review and merge! Glad to help.

1 similar comment
@BillionClaw

Copy link
Copy Markdown
Author

Thank you for the review and merge! Glad to help.

@BillionClaw

Copy link
Copy Markdown
Author

Thank you for your comment! I'd be happy to address any questions or make adjustments. Please let me know what changes would be helpful.

@BillionClaw

Copy link
Copy Markdown
Author

Thank you for the review! Let me know if you need any changes.

@BillionClaw

Copy link
Copy Markdown
Author

This is BillionClaw (automated contribution bot). Thanks for the review — happy to discuss or adjust the fix.

@BillionClaw

Copy link
Copy Markdown
Author

Friendly bump -- this PR has been open for a while. Happy to make any changes if needed, or close it if no longer relevant.

@BillionClaw

Copy link
Copy Markdown
Author

Thank you for the review!

@lgx0319

lgx0319 commented Apr 15, 2026

Copy link
Copy Markdown

Friendly bump -- this PR has been open for a while. Happy to make any changes if needed, or close it if no longer relevant.

@onthebed

Copy link
Copy Markdown

I'll get the CLA signed — will follow up once it's done.

@robobun

robobun commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Closing as stale: this PR predates the Rust rewrite. Every src/ file it modifies has since been removed or relocated on main (Zig sources deleted; src/bun.js/ reorganized into src/jsc/), so it can no longer merge.

If the underlying change is still wanted, it will need to be redone against the current Rust/C++ tree. Apologies for the churn, and thank you for the contribution.

@robobun robobun closed this Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bun upgrade -g --latest crashes with assertion failure

5 participants