bake: retract a client component's bundling failure when it is demoted

[robobun]

Problem

• Dev server with serverComponents.separateSSRGraph on: a "use client" file breaks (Could not resolve: "./missing"), then one edit removes both the directive and the bad import. The route serves again, but no errors packet retracting the failure is sent, so browser overlays keep showing it.
• The dev server keeps it too: the next unrelated "Build Failed" page lists it again, and the "any failures?" checks that gate hot updates keep taking the failure path.
• Cause: demotion deletes the client graph's node for the file instead of re-bundling it, and deletion released neither the node's recorded failure nor its old code and source map. Every other way a node stops failing does both. The Zig original had the same gap.

Fix

• Deleting a client node now frees its content and, if it was failing, moves its failure from the tracked set onto the current bundle's removed list, as a clean re-bundle would.
• That is enough because deletion only happens inside a bundle, and every bundle publishes its removed list in the errors packet it sends anyway; owner ids stay unambiguous because tombstoned slots are never reused.
• The rest of the tombstone is left alone, so a still-reachable one still hits the existing Server Incremental Graph is missing component for "" panic.
• Verification: a new dev-server test decodes the HMR errors packets. On the released build the demoting edit publishes nothing and a later "Build Failed" page lists both failures; with the change it publishes exactly one removal. Passes on a debug/ASAN build.

Background

• Bake is bun's dev server. It keeps an incremental graph per side (server, client) with one node per bundled file, and an edit re-bundles only the nodes it invalidates.
• A "use client" file is a boundary with a node in both graphs. Removing the directive (demotion) makes the server graph delete the client node; that node is an HMR root, so the edit never re-bundles it. separateSSRGraph is what makes the failure belong to the client node.
• Failures live in one set keyed by (side, node index). Each bundle publishes the set's added and removed entries as an errors packet on the HMR socket; the "Build Failed" page renders the whole set.
• Deleted nodes are tombstoned, not removed, because routes and failures hold indices into the graph. A tombstone is never bundled again, so whatever it owns must be released when it is deleted.

Original description

Repro

Dev server with a framework that sets serverComponents.separateSSRGraph = true (bun-framework-react does). routes/index.ts imports components/Comp.ts, which starts out as a "use client" file and bundles fine. Then, in two edits:

// 1. break it while it is still a client component
"use client";
import './missing';
export const marker = "initial";

// 2. drop the directive and the bad import in the same edit
export const marker = "plain";

Edit 1 produces a Could not resolve: "./missing" failure owned by the client graph's node for Comp.ts, and an errors packet adding it is published on the HMR socket. After edit 2 the route serves fine again, but no errors packet removing the failure is ever published, so browsers keep the entry in the error overlay, and dev.bundling_failures still holds it for the rest of the process: the next unrelated failure (say in routes/index.ts) renders a "Build Failed" page listing both the new error and the long fixed Comp.ts one, and the bundling_failures.is_empty() checks in finalize_bundle (hot-update route list, "Reloaded in" line) and in the route state machine keep taking the failure path.

Cause

Edit 2 re-bundles Comp.ts on the server only (the client node is an HMR root, so invalidate skips it). server_graph.receive_chunk sees a file that was a boundary and no longer is, and calls client_graph.disconnect_and_delete_file on the client node. That function disconnects the node's edges and tombstones its key, but leaves the node's failed entry in dev.bundling_failures and never pushes it to incremental_result.failures_removed. Every other place a node stops failing (receive_chunk on both sides, insert_failure replacing an entry) does both; this is the one path where a node goes away without being received again, so nothing later can clean it up. It also left the node's last bundled code and source map behind, a small leak per demotion. The Zig original had the same omissions.

Fix

disconnect_and_delete_file now releases what the node owns before tombstoning it: free_file_content, and if the node was failing, fetch_swap_remove its OwnerPacked(side, index) entry from bundling_failures onto failures_removed, the same sequence receive_chunk uses. The function only runs from receive_chunk, i.e. inside finalize_bundle pass 1, and index_failures drains failures_removed after pass 2 of the same bundle, so the removal is published in the same errors packet the bundle would send anyway, keyed by the owner id the client already has (tombstoned indices are never reused, so the id stays unambiguous).

The rest of the tombstoned File is intentionally left as it was. In particular it keeps is_hmr_root, so a tombstone that is still reachable keeps hitting the existing Server Incremental Graph is missing component for "" panic instead of being silently traced as a dead end; this change is only about the failure and content the slot owned.

Verification

Three tests added to test/bake/dev/bundle.test.ts:

• "removing 'use client' from a failing component retracts its bundling failure": subscribes the harness socket to the errors topic and decodes the packets. Edit 1 publishes exactly one added failure for components/Comp.ts, edit 2 publishes exactly { removed: [that owner], added: [] } and the route then serves the demoted module, and a later unrelated failure's "Build Failed" page lists only that failure. On the released build the second step receives no packet at all and the page lists both failures.
• "removing 'use client' from a failing component clears the error overlay": the same edits with a browser (harness client) sitting on the Build Failed page; after edit 2 the overlay is empty and the page reloads onto the working route. On the released build the overlay still shows the Comp.ts error and no reload happens.
• "removing 'use client' from a working component": the non-failing demotion, which now frees the node's code and source map under ASAN and must publish nothing.

Runs:

• released build (USE_SYSTEM_BUN=1): the first two fail as described, the third passes
• debug/ASAN build with the change: the whole file passes (24 tests, including the existing demotion test, which now also goes through the new code)
• cherry-picked onto bake: keep a route failed when the "use client" file it imports fails to bundle #37850, which makes a "use client" file that never bundled take this same demotion path: demoting such a file publishes { removed, added: [] } and later error pages no longer list it, so the two changes compose (on main alone that scenario never reaches disconnect_and_delete_file, and this PR leaves it unchanged)

---

[review] gate passed · iteration 1 · 2 files touched

fails on main (without fix)

ASAN without fix: 2 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bake/dev/bundle.test.ts
bun test v1.4.0 (502492326)

test/bake/dev/bundle.test.ts:
Dev server testing directory: /tmp/bun-dev-test-ek3jdh
�[0;30mdev|�[0m Started development server: http://localhost:35047
�[0;30mdev|�[0m �[32mBundled page in 254ms�[0m�[2m:�[0m routes/index.ts �[2m+ 1 more�[0m
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "db.ts" is not accepted by routes/index.ts,
�[0;30mdev|�[0m �[32mReloaded in 67ms�[0m�[2m:�[0m db.ts
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "routes/index.ts" is a root module that does not self-accept.
�[0;30mdev|�[0m �[36m[x2]�[0m �[32mReloaded in 112ms�
... (truncated)

release without fix: 2 FAILED
bun test v1.4.0-canary.1 (da3851e57)

test/bake/dev/bundle.test.ts:
Dev server testing directory: /tmp/bun-dev-test-vyB62A
�[0;30mdev|�[0m Started development server: http://localhost:45335
�[0;30mdev|�[0m �[32mBundled page in 4ms�[0m�[2m:�[0m routes/index.ts �[2m+ 1 more�[0m
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "db.ts" is not accepted by routes/index.ts,
�[0;30mdev|�[0m �[32mReloaded in 8ms�[0m�[2m:�[0m db.ts
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "routes/index.ts" is a root module that does not self-accept.
�[0;30mdev|�[0m �[36m[x2]�[0m �[32mReloaded in 2ms�[0m�[2m:�[0m routes/index.ts
(pass)  DEV:bundle-1: import identifier doesnt get renamed [251.97ms]
�[0;30mdev|�[0m Started development server: http://localhos
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bake/dev/bundle.test.ts
bun test v1.4.0 (502492326)

test/bake/dev/bundle.test.ts:
Dev server testing directory: /tmp/bun-dev-test-MGJ8ub
�[0;30mdev|�[0m Started development server: http://localhost:43633
�[0;30mdev|�[0m �[32mBundled page in 209ms�[0m�[2m:�[0m routes/index.ts �[2m+ 1 more�[0m
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "db.ts" is not accepted by routes/index.ts,
�[0;30mdev|�[0m �[32mReloaded in 55ms�[0m�[2m:�[0m db.ts
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "routes/index.ts" is a root module that does not self-accept.
�[0;30mdev|�[0m �[36m[x2]�[0m �[32mReloaded in 68ms�[
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     502492326c
  features     baseline

22 deps, 107 codegen, 1176 objects in 3576ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1238] gen ErrorCode+*.h
[2/1238] gen bindgenv2
[3/1238] gen .bind.ts → GeneratedBindings.cpp
[4/1238] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[5/1238] fetch tinycc
[tinycc] up to date
[6/1237] gen JSBuffer.lut.h
Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[7/1237] gen ProcessBindingBuffer.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingBuffer.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingBuffer.cpp
[8/1237] fetch zlib
[zlib] up to date
[9/1237] gen ProcessBindingConstants.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cpp
[10/1237] gen ProcessBindingHTTPParser.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingHTTPParser.lut.h 
... (truncated)

diff hotspot

src/runtime/bake/dev_server/incremental_graph.rs |  19 ++
 test/bake/dev/bundle.test.ts                     | 239 ++++++++++++++++++++++-
 2 files changed, 257 insertions(+), 1 deletion(-)

gate history · 1 passed · 0 rejected · iteration 1

evidence per changed file

file                                              reads  edits  tests
src/runtime/bake/dev_server/incremental_graph.rs     10      4      0
test/bake/dev/bundle.test.ts                          3      5      0

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d4904fac-8936-4cd8-b8e1-0e127d3d9e91

📥 Commits

Reviewing files that changed from the base of the PR and between 9a543cc and 5024923.

📒 Files selected for processing (2)

• src/runtime/bake/dev_server/incremental_graph.rs
• test/bake/dev/bundle.test.ts

---

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

[robobun]

^{Updated 8:05 PM PT - Aug 12th, 2026}

❌ @robobun, your commit 5024923 has 1 failures in Build #93690 (All Failures):

• scripts/build/ci.ts - build failed on 🐧 aarch64-android - build-bun
• scripts/build/ci.ts - build failed on 🐧 aarch64 - build-bun

---

🧪   To try this PR locally:

bunx bun-pr 37862

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

bun-37862 --bun

[robobun]

Status: ready for review. Reproduced on the released build with the new tests in test/bake/dev/bundle.test.ts (demoting a failing "use client" component publishes no errors packet, the overlay keeps the error, and the failure stays in bundling_failures; with this change the packet { removed: [owner], added: [] } is published, the error page reloads, and later error pages no longer list it). Fix is in disconnect_and_delete_file; self-review done, nothing outstanding.

CI: across the last three builds every test lane that ran (including the x64 ASAN lanes, which run the bake tests) passed; the red is confined to build lanes that could not download the mimalloc / lolhtml / WebKit tarballs from github.com (and the jobs depending on them), plus already-flagged flaky tests unrelated to bake. I am not going to keep re-triggering; the diff itself is green.

[claude]

I reviewed this PR and didn't find any bugs. The fix reuses the same failure-removal sequence already present at three other sites in incremental_graph.rs, but since it touches the bake dev server's cross-graph state machine (tombstoned slots, unsafe sibling-field projection) a human look would still be worthwhile.

What was reviewed:

• Confirmed disconnect_and_delete_file has a single caller on client_graph, so the debug_assert!(SIDE == Client) in free_file_content holds and the server graph is never affected.
• Checked that dev_bundling_failures() / dev_incremental_result() project to fields disjoint from both graphs and that the server-side caller holds no borrow on them at the call site.
• Verified FreeCssMode::UnrefCss matches the mode used by the other node-goes-away paths (insert_stale_extra, insert_failure) rather than the replace path in receive_chunk.
• The test's synchronization relies on dev.write resolving after a later socket message, so error packets from the same rebuild are already recorded before the assertion; the harness's onmessage → emit("hmr") ordering supports that.

Extended reasoning...

Overview

The PR adds ~20 lines to disconnect_and_delete_file in src/runtime/bake/dev_server/incremental_graph.rs so that when a client-graph node is tombstoned (only happens when a "use client" boundary is demoted from the server side), it releases the node's bundled content/source map and, if the node was failing, removes its entry from dev.bundling_failures and pushes it onto incremental_result.failures_removed — the exact sequence already used at three other places in the same file. A ~150-line test in test/bake/dev/bundle.test.ts subscribes the harness HMR socket to the errors topic, decodes the binary SerializedFailure / errors-packet wire format, and asserts the retraction is published and no longer appears on later "Build Failed" pages.

Security risks

None. This is dev-server-only error-tracking bookkeeping; no user input parsing, auth, crypto, or network-facing surface changes.

Level of scrutiny

Medium-high. The Rust change is small and mechanically mirrors an established pattern, but it lives inside the bake dev server's incremental graph, which coordinates two const-generic graph instances via unsafe owner()-based sibling-field projection and maintains subtle invariants about tombstoned slots (indices never reused, is_hmr_root intentionally preserved so a still-reachable tombstone keeps panicking rather than silently tracing). The PR description explains these invariants clearly and I verified the single caller and field-disjointness, but a maintainer familiar with bake should confirm the reasoning about slot state and the UnrefCss choice.

Other factors

The test is strong: it asserts the exact wire-format packet at each step (added → removed → later failure page lists only the new error), and the PR description confirms it fails on USE_SYSTEM_BUN=1 and passes on the debug/ASAN build along with the rest of bundle.test.ts (including the pre-existing demotion test that now also exercises the new code). No new unsafe blocks are introduced; the change only calls the pre-existing safe projection helpers. The core::mem::take / mutate / write-back pattern matches the one used in insert_stale_extra and insert_failure for the same borrow-checker reason. Given the subsystem's complexity I'm deferring rather than approving outright.

[claude]

I reviewed this PR and didn't find any bugs. The retraction logic in disconnect_and_delete_file reuses the exact take/free_file_content/put-back and bundling_failures.fetch_swap_remove → failures_removed sequence already established in receive_chunk and insert_stale_extra, and the sole call site is on the client graph so the Side::Client assert in free_file_content holds. That said, this touches unsafe sibling-projected state in the incremental graph and adds ~240 lines of test infrastructure that hard-decodes the HMR errors wire format, so a human look is still worthwhile.

What was reviewed:

• Confirmed disconnect_and_delete_file has exactly one caller (client_graph from server-side receive_chunk), so free_file_content's client-only assert and the new owner()-backed sibling accesses (bundling_failures, incremental_result, assets) stay disjoint from the server graph's live borrows.
• Checked that is_hmr_root and other flags survive the mem::take/write-back, matching the PR's stated intent for the tombstone.
• Traced the three new tests against serialized_failure.rs's layout and the harness's dev.write sequencing; the overlay test uses expectReload rather than a sleep.

Extended reasoning...

Overview

The PR modifies disconnect_and_delete_file in src/runtime/bake/dev_server/incremental_graph.rs (+19 lines) so that when a client-component boundary is demoted and its client-graph node is deleted, the node's cached content/source-map are freed and any recorded bundling failure is removed from dev.bundling_failures and pushed onto incremental_result.failures_removed. It also adds ~240 lines to test/bake/dev/bundle.test.ts: two binary decoders for the SerializedFailure / MessageId.errors wire format, a recordErrorsPackets helper, and three devTests covering the working-component demotion, the failing-component demotion (packet-level), and the same demotion observed through the error overlay.

Security risks

None. This is dev-server bookkeeping for the HMR error overlay; no network input parsing, auth, or user-controlled data reaches the changed code.

Level of scrutiny

Medium-high. The Rust change is small and mechanically identical to existing patterns in the same file, but it lives inside the incremental graph's owner()-based unsafe sibling-projection scheme: free_file_content reaches (*dev).assets, and the failure retraction reaches (*dev).bundling_failures / (*dev).incremental_result, all while the server graph's receive_chunk already holds raw-pointer-derived borrows of client_graph and directory_watchers. I verified these are all disjoint DevServer fields, but this is exactly the class of code where a maintainer's eye is valuable. The test additions also hard-code the HMR errors wire format in the harness, which a bake maintainer should confirm they're comfortable maintaining.

Other factors

• The failure-removal block is byte-for-byte the same as the client and server arms of receive_chunk, and the take/free/put-back matches insert_stale_extra and insert_failure, so there's no new pattern to reason about.
• disconnect_and_delete_file is private with a single call site on client_graph, so the debug_assert!(matches!(SIDE, Side::Client)) inside free_file_content is satisfied and dead-code elimination handles the server monomorphization.
• CI on the latest commit shows unrelated infra failures (aarch64 build-bun on macOS/FreeBSD, verify-baseline exit 56, an install-migration flake) with no test/bake failures, but the build is still in progress.
• The comment-cop bot's note about a long comment was already addressed in eaba299.

[claude]

I reviewed this PR and didn't find any bugs. The Rust change is small and mirrors the same cleanup sequence already used in receive_chunk and insert_stale_extra, but since it touches bake's incremental-graph node lifecycle and the tests hand-decode the HMR errors wire format, a human look would still be worthwhile.

What was reviewed:

• disconnect_and_delete_file: verified the take → free_file_content → write-back preserves is_hmr_root and other flags, and the RawSlice key stays valid until the later tombstone step.
• Confirmed the only caller is client_graph.disconnect_and_delete_file(...), so the client-only debug_assert in free_file_content holds.
• Checked the failure-removal sequence matches the existing pattern in receive_chunk (client) byte-for-byte.
• Tests: the errors-packet decoder layout was cross-checked against the field order described for SerializedFailure; the harness's emit("hmr", ...) and dev.socket are established patterns.

Extended reasoning...

Overview

The PR adds ~19 lines to IncrementalGraph::disconnect_and_delete_file in src/runtime/bake/dev_server/incremental_graph.rs so that when a client-graph node is tombstoned (a "use client" file loses its directive), its bundled content/source-map is freed and any recorded bundling failure is moved from dev.bundling_failures onto incremental_result.failures_removed. It also adds three devTest cases and two decoder helpers (~240 lines) to test/bake/dev/bundle.test.ts.

Security risks

None. This is dev-server error-overlay bookkeeping; no auth, no untrusted input parsing on the server side, no new unsafe blocks (the sibling-projection helpers dev_bundling_failures()/dev_incremental_result() are pre-existing).

Level of scrutiny

Medium-high. The Rust change itself is mechanical — it lifts the exact core::mem::take → free_file_content → fetch_swap_remove → failures_removed.push sequence already used in receive_chunk (client branch) and insert_stale_extra. But bake's incremental graph mixes const-generic side dispatch, RawSlice borrows of map keys, and container_of-style sibling access; a mistake here would be a UAF or stuck error state. I checked that free_file_content's debug_assert!(SIDE == Client) is satisfied (the sole caller is on client_graph), that the key RawSlice is not read after the slot's key is freed, and that non-content flags like is_hmr_root survive the take/write-back so the existing "missing component" panic still fires on a reachable tombstone.

Other factors

The tests are thorough (both HMR-socket-level and browser-overlay-level, plus the non-failing demotion under ASAN), and the PR description documents USE_SYSTEM_BUN=1 failing / debug+ASAN passing. The one bot comment (comment-cop about a long code comment) was addressed in eaba299. What gives me pause for auto-approval is that the tests introduce a hand-rolled decoder for the SerializedFailure / MessageId.errors binary format and a new "sre" subscription pattern — a maintainer should confirm they're happy taking that as test surface. That plus the general subtlety of the incremental-graph lifecycle keeps this out of the "obviously safe" bucket, so I'm deferring rather than approving.