Skip to content

install: deliver a manifest task's waiters on every pass of run_tasks - #44414

Merged
dylan-conway merged 5 commits into
mainfrom
robobun/7c49da34/update-peer-manifest-waiters
Oct 3, 2026
Merged

dylan-conway merged 5 commits into
mainfrom
robobun/7c49da34/update-peer-manifest-waiters

Conversation

@robobun

@robobun robobun commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • bun update <dep> <peer> exits 1 with error: util@^1.0.0 failed to resolve when <peer> is an auto-installed peer that nothing else depends on.
  • enqueue_peer_rows (src/install/update_transitive.rs:452) calls populate_manifest_cache while dependencies wait for manifests. Its wait runs run_tasks in manifests-only mode, which skips their waiter lists (runTasks.rs:662, :1070).
  • The opposite mismatch aborts bun install over a yarn.lock with an unusable scope registry: panic: infallible: task queued.

Fix

  • run_tasks takes the waiter list of a finished manifest request on every pass. A prefetch request has none.
  • print_log returns InstallFailed when the log it resets holds an error. populate_manifest_cache starts the progress bar before its wait (panic: downloads_node active).
  • Verified: 18 new cases in bun-update-transitive.test.ts, 1 in yarn-lock-migration.test.ts. All fail without the fix.
  • Self-reviewed: 2 concerns raised, 2 addressed, one in part: the bar after the --latest prefetch stays open.

Background

  • A waiter is a dependency that waits for a manifest request. task_queue maps request ids to waiters. Only run_tasks retires requests.
  • populate_manifest_cache fetches manifests ahead of use (bun outdated, bare bun update, five more entrances). Its requests have no waiters.
  • Considered a reorder of the named update: it covers 1 of 7 entrances and keeps both skips.

Downsides

  • A prefetch pass does one task_queue lookup per stored manifest: 15 more instructions, no insert, no allocation.
  • Queued dependencies now resolve inside the prefetch wait. Such a run can print no Resolving dependencies line.
  • After a failed download, bun update --latest <name> exits 1 and saves nothing. It exited 0 and saved bun.lock.
Notes

Repro. A loopback registry has util@1.0.0 with peerDependencies: { core: "^1.0.0" } and core@1.0.0. The project depends on util ^1.0.0. bun install, then bun update util core: exit 1, error: util@^1.0.0 failed to resolve, nothing saved. The result is the same after util@1.1.0 and core@1.1.0 exist, and in either name order. The failure is in every release since 1.4.0 (#38333 added enqueue_peer_rows).

Why 1.4.2 passed one shape. When core also depends on util, 1.4.2 moved both packages. The re-resolved peer made a new core@1.1.0, its dependency started a tarball download of a new util, and the extract arm of run_tasks ran the waiter list of the util manifest. #43122 removed that download. No revert is needed: the waiter was already dropped, and shapes without that download fail on 1.4.2 too.

Mechanism. bun update does not read the manifest disk cache, so each queued dependency starts a manifest request and parks a TaskCallbackContext::Dependency in task_queue (PackageManagerEnqueue.rs:1331). populate_manifest_cache flushes and schedules every queued request and waits until the manager has no pending task. In that wait the old code stored the manifest and took continue before the task_queue take. network_dedupe_map keeps the request id, so nothing asks again. The dependency stays unresolved.

Forms that failed and now pass (each is a test). Both name orders. A glob next to the peer and *. --latest. A transitive dependency named next to the peer. The peer named from a workspace member. The peer itself added to package.json. A dependency, an optional dependency, an override or a file: folder added to package.json before bun update <peer>. --prefer-offline with only the peer's manifest cached. A patched dependency named next to the peer. Two forms were silent on main. With an optional dependency added to package.json, bun update <peer> exits 0 and saves a bun.lock without it. bun update <optional-dep> <peer> exits 0, writes "util": "" to package.json and saves a bun.lock with no packages.

print_log. It has two callers, enqueue_peer_rows and plan_edges. Both print and reset the log in the middle of an install, and install_with_manager reads log.has_errors() later to decide whether to save. With the waiters delivered inside the prefetch wait, an error of such a dependency reaches the first caller: a tarball 404 or an integrity failure for a dependency that the request does not name. The second caller has the same defect on main: bun update --latest <name> runs plan_edges after the first resolve wave, so a tarball 404 in that wave prints error: GET ... - 404, exits 0 and saves bun.lock and package.json, where bun update <name> exits 1 and saves nothing. Two tests pin both callers. Each fails without the guard.

Migration abort. Packages::All returns at the first manifest request it cannot start, after it scheduled earlier ones. fetch_necessary_package_metadata_after_yarn_or_pnpm_migration drops that error. The install wait then completed a request with no task_queue entry and hit .expect("infallible: task queued"). Release build of the merge base: exit 134. This branch: exit 0, bun.lock written.

Progress bar. start_manifest_task starts the bar only when it creates a request. When every name is cached or already requested by a queued dependency, the prefetch wait started with no bar, and the first pass of run_tasks names the bar if a manifest download has completed by then. Real timing did not hit the window: 0 panics in 200 pty runs on each build. With the main thread paused for 400 ms before that first pass (gdb), a release build of main panics with downloads_node active in 5 of 5 runs, for the peer added to package.json and for --prefer-offline with only the peer's manifest cached. This branch: 0 of 25 runs. No test covers it, because it needs a pty and that pause. One more wait has the same window and is not changed here: under --latest, the prefetch ends the bar that resolve_pending_tasks started, and the wait at install_with_manager.rs:2001 then runs without one (3 of 3 panics with the same pause on this head, 0 of 20 with real timing).

Measurements (release builds of the merge base 4b02e10 and of this branch, unless noted).

  • run_tasks_erased, pass of a normal install: parsed-manifest arm 35 -> 33 instructions from the manifest store to the process_dependency_list_for_ctx call, 304 arm 34 -> 33. The compare-and-branch on manifests_only is gone at both sites. Whole function: 6517 -> 6448 instructions.
  • run_tasks_erased: 33,102 -> 32,764 bytes. populate_manifest_cache: 3,724 -> 3,756 bytes. Release .text: 0 bytes delta (80,679,983 both). Stripped binary: 80,848,456 bytes both. These sizes are from the first push. The later print_log change adds one compare and one early return.
  • Prefetch pass with nothing queued (bun outdated, 30 dependencies, empty manifest cache): 30 task_queue lookups (0 on the merge base), 0 inserts, 0 waiter deliveries. After the manifest store the pass runs 20 instructions for each manifest, 12 of them in HashMap::get_index. The merge base runs 5.
  • Waiters (debug build, gdb): bun update util core 1 of 1 delivered once. With core depending on util: 2 of 2. bun update '*' over 40 direct dependencies and 1 peer: 40 of 40 delivered once, 41 manifest requests with no duplicate, 41 packages moved, bun install --frozen-lockfile passes. The take in the extract arm found 0 non-empty lists.
  • Requests of bun update util core with both packages newer: 1 GET /util, 1 GET /core, then the 2 tarballs. bun update core nope: 0 requests, same reject text.
  • stdout, stderr and requests of bun update util and of bun update core: 0 changed lines.
  • Event-loop waits entered (AnyEventLoop::tick_raw, debug builds, two runs each): bun update util 3 and 5 on both builds. bun update core 3 and 5 to 6 on the merge base, 3 and 6 on this branch.

Other suites run. The update, audit, yarn and pnpm migration, add, lock, workspaces, catalogs and update -i suites pass. bun-install.test.ts fails the same 13 cases with and without the change (they need the public network).

Left for later.

  • The bar of the wait after the --latest prefetch (see Progress bar). The place for it is the top of each run_tasks pass.
  • A waiter that is parked on a failed manifest request stays parked in both modes.
  • The prefetch pass still runs with install_peer = true. Only non-peer dependencies can be parked there today.
  • MANIFESTS_ONLY now only keeps the parsed-manifest arm from naming the progress bar.
  • A manifest that answers 200 with a body that does not parse: the exit code is 1 before and after (the parse task prints error: Unexpected registry and marks the run as failed). Before, bun.lock was still saved. Now nothing is saved.
  • Open PRs on the same lines. install: remove every unsafe from src/install #40284 rewrites the same two hunks and keeps both early exits. install: filter optionalDependencies by libc and record it in the lockfile #38797 carries both early exits, a third manifests_only gate and a helper that ends in .expect("infallible: task queued"), so a rebase of it over this change must drop them. install: re-resolve rows whose override or catalog value changed in the package.json write-back #43981 adds another populate_manifest_cache call inside an install, which this change makes safe in any call order.

@robobun

robobun commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on main and on 1.4.2 with a loopback registry:

  • The registry has util@1.0.0 with peerDependencies: { core: "^1.0.0" } and core@1.0.0. The project depends on util ^1.0.0.
  • bun install, then bun update util core: exit 1, error: util@^1.0.0 failed to resolve. bun update util alone and bun update core alone pass.
  • The new cases in test/cli/install/bun-update-transitive.test.ts and test/cli/install/migration/yarn-lock-migration.test.ts fail without the fix and pass with it.

Merged as 80de08a.

@robobun

robobun commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 4:51 PM PT - Oct 2nd, 2026

✅ @dylan-conway, your commit 3624e2a0d7930e7332bada6f8b98771b2b12f51d passed in Build #122991! 🎉


🧪   To try this PR locally:

bunx bun-pr 44414

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

bun-44414 --bun

@dylan-conway
dylan-conway marked this pull request as ready for review October 2, 2026 23:23
@coderabbitai

coderabbitai Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (2)
test/CLAUDE.md — configured
src/CLAUDE.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Essentials
  • Run ID: 2ea0a172-edc5-422c-8ef9-7b6cdd54e118
📥 Commits

Reviewing files that changed from the base of the PR and between e29a7ca and 3624e2a.

📒 Files selected for processing (5)
  • src/install/PackageManager/PopulateManifestCache.rs
  • src/install/PackageManager/runTasks.rs
  • src/install/update_transitive.rs
  • test/cli/install/bun-update-transitive.test.ts
  • test/cli/install/migration/yarn-lock-migration.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


Walkthrough

Manifest task processing and named dependency updates changed. Pending manifest tasks can show progress, missing task-queue entries are skipped, and update errors are returned. New tests cover auto-installed peers, download failures, and installation after Yarn lockfile migration.

Changes

Install and dependency update handling

Layer / File(s) Summary
Manifest task processing and migration coverage
src/install/PackageManager/PopulateManifestCache.rs, src/install/PackageManager/runTasks.rs, test/cli/install/migration/yarn-lock-migration.test.ts
Manifest processing starts progress before waiting on pending tasks. Cached and parsed manifests process dependency lists only when a task-queue entry exists. A migration test checks package installation and lockfile entries when a scoped registry uses FTP.
Named update resolution and error handling
src/install/update_transitive.rs, test/cli/install/bun-update-transitive.test.ts
print_log returns InstallFailed when the log contains errors. Tests cover named updates with auto-installed peers, offline preference, patched dependencies, and failed downloads that leave manifests and lockfiles unchanged.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 3624e

This change fixes named updates that leave an auto-installed peer unresolved, and it fixes a yarn.lock migration panic. It adds regression tests for both. No concrete merge-blocking risk was found in the supplied context. The author says the final build and measurements were still pending.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: delivering waiters for manifest tasks on every run_tasks pass.
Description check ✅ Passed The description explains the problem, implementation, verification, failure cases, tradeoffs, and limitations. It does not use the exact template headings, but it provides the required change summary …
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.
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

@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.

Comment thread src/install/update_transitive.rs Resolved
@dylan-conway
dylan-conway merged commit 80de08a into main Oct 3, 2026
10 checks passed
@dylan-conway
dylan-conway deleted the robobun/7c49da34/update-peer-manifest-waiters branch October 3, 2026 00:30
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.

3 participants