[robobun]

Repro

Dev server with a server-side framework (minimalFramework from the bake test harness is enough), lib/boom.ts throwing and routes/index.ts importing and calling it. The first request prints the stack correctly:

at boom2 (<root>/lib/boom.ts:1:8)
at routes_default (<root>/routes/index.ts:2:8)
at render (test/bake/minimal.server.ts:7:3)

After a hot update that only touches routes/index.ts (so lib/boom.ts and the framework entry are not re-bundled), the same request prints:

at boom2 (bake://server.patch.js:42:13)
at routes_default (<root>/routes/index.ts:5:8)
at render (<root>/routes/index.ts:4:1)

boom2 is printed raw and render, which lives in minimal.server.ts, is attributed to a file of the new patch. With framework: "react" the same thing happens to every react-server-dom frame after the first hot update (at react-stack-bottom-frame (bake://server.patch.js:3054:29)). The dev error page embeds the same wrong frames in its JSON payload, and error.stack goes through the same lookup.

Cause

BakeLoadServerHmrPatchWithSourceMap (src/runtime/bake/BakeSourceProvider.cpp) evaluated every server patch under the same URL, bake://server.patch.js. Each patch's DevServerSourceProvider registers itself in the VM's SavedSourceMap keyed by that URL, and put_source_provider replaces the existing entry, so after the second patch every frame with that URL, whichever patch it actually belongs to, is remapped through the newest patch's map. Modules that a hot update does not re-bundle keep running the code of the patch that loaded them (replaceModules only swaps the updated module ids), so that code is live for the whole session while its map is gone after the first unrelated update. Lines past the end of the newer map print raw; lines inside it get attributed to one of the newer patch's files.

Fix

Each patch gets its own URL, bake://server.patch.<n>.js, from a counter, so each patch keeps its own table entry. The entry is removed when the patch's provider is destroyed, which is what ~DevServerSourceProvider already does (remove_source_provider compares the provider identity, so with unique keys the removal now actually hits instead of being a no-op on the replaced entry). This mirrors the client side, where every hot update chunk already gets its own /_bun/client/<id>.js.map.

Why a counter and not the dev server's own bundle generation: the table is per VM and every dev server in a process shares it, so two dev servers would collide on the same generation number. A content hash (what the client uses for its ids) would reintroduce the collision for identical re-bundles, which is exactly the case where two live patches must stay distinct.

Memory: a patch's entry (and its lazily parsed mappings, once a frame from it has been looked up) now lives as long as the patch's code, rather than one bundle round. The source map JSON itself was already retained for that long by the provider; this only keeps it reachable through the table.

BakeLoadServerHmrPatch (the variant without a source map) is left as is. It is unreachable: receive_chunk pushes a current_chunk_source_maps entry (a real map or a line count) for every server chunk it adds to current_chunk_code, so whenever there is a patch to load there is also a map and the else branch in finalize_bundle never runs. It also uses the production Bake::SourceProvider, which registers under its URL too but never has a dev server map to offer and, unlike DevServerSourceProvider, has no destructor that unregisters it; giving that path unique URLs would only accumulate entries. Removing the dead variant and fixing that class's unregistration are separate changes.

Verification

test/bake/dev/server-sourcemap.test.ts:

• new case (minimalFramework, three requests): update only the route, then update only the lib, and check all three frames each time. The route frame has to move by the number of lines prepended to it, the lib frame has to stay put and then move once the lib itself is updated, and the framework frame has to keep pointing at minimal.server.ts. Lines are asserted relative to the first round because the absolute numbers are the subject of a separate dev server source map fix.
• the existing react HMR case additionally checks that the react-stack-bottom-frame frame still remaps after the update.

Without the fix, both cases fail (bake://server.patch.js:42 and routes/index.ts:4 show up in the first one's diff); with it the file passes, including the ASAN destructor case, as do test/bake/dev/sourcemap.test.ts, bundle.test.ts, react-response.test.ts and test/bake/deinitialization.test.ts on a debug build.

---

[review] gate passed · iteration 0 · 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/server-sourcemap.test.ts
bun test v1.4.0 (dbba65cbb)

test/bake/dev/server-sourcemap.test.ts:
Dev server testing directory: /tmp/bun-dev-test-jQ5tAf
bun add v1.4.0 (dbba65cbb)
Resolving dependencies
Resolved, downloaded and extracted [2]
Saved lockfile

installed react@0.0.0-experimental-603e6108-20241029
installed react-dom@0.0.0-experimental-603e6108-20241029
installed react-server-dom-bun@0.0.0-experimental-603e6108-20241029
installed react-refresh@0.0.0-experimental-603e6108-20241029

6 packages installed [439.00ms]
bun install v1.4.0 (dbba65cbb)

Checked 6 installs across 7 packages (no changes) [218.00ms]
�[0;30mdev|�[0m Started development server: http://localhost:39035
�[0;30mdev|�[0m �[32mBundled page in 3201ms�[0m�[2m:�[0m pages/[...slug].tsx �[2m+ 2 more�[0m
�[0;30mdev|�[0m �[0m�[1m1 |�[0m �[0m�[35mexport�[0m �[0m�[35mdefault�[0m �[0m�[35masync�[0m �[0m�[35mfunction�[0m MyPage(params) {
�[0;30mdev|�[0m �[0m�[1m2 |�[0m   myFunc()�[0m�[2m;�[0m
�[0;30mdev|�[0m �[0m�[1m3 |�[0m   �[0m�[35mreturn�[0m �[0m<�[0mh1>{JSO
... (truncated)

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

test/bake/dev/server-sourcemap.test.ts:
Dev server testing directory: /tmp/bun-dev-test-HUdORi
bun add v1.4.0-canary.1 (9008ae7ab)
Resolving dependencies
Resolved, downloaded and extracted [0]
Saved lockfile

installed react@0.0.0-experimental-603e6108-20241029
installed react-dom@0.0.0-experimental-603e6108-20241029
installed react-server-dom-bun@0.0.0-experimental-603e6108-20241029
installed react-refresh@0.0.0-experimental-603e6108-20241029

6 packages installed [28.00ms]
bun install v1.4.0-canary.1 (9008ae7ab)

Checked 6 installs across 7 packages (no changes) [6.00ms]
�[0;30mdev|�[0m Started development server: http://localhost:33149
�[0;30mdev|�[0m �[32mBundled page in 90ms�[0m�[2m:�[0m pages/[...slug].tsx �[2m+ 2 more�[0m
�[0;30mdev|�[0m �[0m�[1m1 |�[0m �[0m�[35mexport�[0m �[0m�[35mdefault�[0m �[0m�[35masync�[0m �[0m�[35mfunction�[0m MyPage(params) {
�[0;30mdev|�[0m �[0m�[1m2 |�[0m   myFunc()�[0m�[2m;�[0m
�[0;30mdev|�[0m �[0m�[1m3 |�[0m   �[0m�[35mreturn�[0m �[0m<�[0mh1>{JSON�[0m�[3m�[1m.stringify�[0m(params)}�[0m<�[0m/h1>�[0m�[2m;�[0m
�[0;30mdev|�[0m �[0m�[1m4 |�[0m }
�[0;30mdev|�[0m �[0m�[1m5 |�[0m 
�[0;30mdev|�[0m �[0m
... (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/server-sourcemap.test.ts
bun test v1.4.0 (dbba65cbb)

test/bake/dev/server-sourcemap.test.ts:
Dev server testing directory: /tmp/bun-dev-test-8BjQIB
bun add v1.4.0 (dbba65cbb)
Resolving dependencies
Resolved, downloaded and extracted [0]
Saved lockfile

installed react@0.0.0-experimental-603e6108-20241029
installed react-dom@0.0.0-experimental-603e6108-20241029
installed react-server-dom-bun@0.0.0-experimental-603e6108-20241029
installed react-refresh@0.0.0-experimental-603e6108-20241029

6 packages installed [134.00ms]
bun install v1.4.0 (dbba65cbb)

Checked 6 installs across 7 packages (no changes) [97.00ms]
�[0;30mdev|�[0m Started development server: http://localhost:46027
�[0;30mdev|�[0m �[32mBundled page in 3496ms�[0m�[2m:�[0m pages/[...slug].tsx �[2m+ 2 more�[0m
�[0;30mdev|�[0m �[0m�[1m1 |�[0m �[0m�[35mexport�[0m �[0m�[35mdefault�[0m �[0m�[35masync�[0m �[0m�[35mfunction�[0m MyPage(params) {
�[0;30mdev|�[0m �[0m�[1m2 |�[0m   myFunc()�[0m�[2m;�[0m
�[0;30mdev|�[0m �[0m�[1m3 |�[0m   �[0m�[35mreturn�[0m �[0m<�[0mh1>{JSON
... (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     dbba65cbba
  features     baseline

22 deps, 107 codegen, 1176 objects in 934ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1238] gen ErrorCode+*.h
[2/1238] gen .bind.ts → GeneratedBindings.cpp
[3/1238] install /workspace/bun
bun install v1.4.0-canary.1 (9008ae7ab)

Checked 107 installs across 153 packages (no changes) [23.00ms]
[4/1238] gen JSBuffer.lut.h
Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[5/1238] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (9008ae7ab)

Checked 1 install across 2 packages (no changes) [3.00ms]
[6/1238] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[7/1238] install /workspace/bun/src/node-fallbacks
bun install v1.4.0-canary.1 (9008ae7ab)

Checked 129 installs across 147 packages (no changes) [10.00ms]
[8/1238] fetch zlib
[zlib] up to date
[9/1238] gen ProcessBindingHTTPParser.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingHTTP
... (truncated)

diff hotspot

src/runtime/bake/BakeSourceProvider.cpp |  8 +++-
 test/bake/dev/server-sourcemap.test.ts  | 82 ++++++++++++++++++++++++++++++---
 2 files changed, 83 insertions(+), 7 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file

file                                     reads  edits  tests
src/runtime/bake/BakeSourceProvider.cpp      2      6      0
test/bake/dev/server-sourcemap.test.ts       3      7      0

self-review · no surviving concerns

33 concerns were raised and did not survive verification.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 17 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: a598c73b-81f4-4dd3-9624-13d33d58a40e

📥 Commits

Reviewing files that changed from the base of the PR and between b2fc659 and dbba65c.

📒 Files selected for processing (2)

• src/runtime/bake/BakeSourceProvider.cpp
• test/bake/dev/server-sourcemap.test.ts

---

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

[robobun]

Status

Reproduced on the released build and on main with test/bake/dev/server-sourcemap.test.ts (USE_SYSTEM_BUN=1 bun test test/bake/dev/server-sourcemap.test.ts): after one hot update that does not touch lib/boom.ts, its frame prints as bake://server.patch.js:42:13 and the framework frame is attributed to routes/index.ts:4. Both new assertions fail without the change in src/runtime/bake/BakeSourceProvider.cpp and pass with it (debug/ASAN build, whole file green).

Fix is in this PR (#37440). CI is green across the pipeline. Self-review turned up nothing in the code; it did catch an inaccurate sentence in the description about the no-source-map variant (Bake::SourceProvider does register itself), which is now corrected. Ready for a maintainer.

[claude]

LGTM — small, well-reasoned fix with a targeted regression test.

What was reviewed:

• Verified put_source_provider keys by URL and replaces, and remove_source_provider compares provider identity, so unique per-patch URLs both stop the map collision and make the destructor's removal land.
• Grepped for other consumers of bake://server.patch.js — only the no-source-map BakeLoadServerHmrPatch remains, which does not register a provider (as the PR notes).
• Checked the atomic counter: function-local static std::atomic<unsigned>, seq_cst ++, thread-safe init; process-wide scope is the conservative choice given vm.source_mappings is per-VM.
• New test uses the same __bunfallback JSON extraction as react-response.test.ts, asserts relative line deltas, and handles Windows path separators.

Extended reasoning...

Overview

The PR changes one function in src/runtime/bake/BakeSourceProvider.cpp (BakeLoadServerHmrPatchWithSourceMap) to give each server HMR patch a unique source URL (bake://server.patch.<n>.js) via a process-wide std::atomic<unsigned> counter, instead of reusing bake://server.patch.js for every patch. It adds a new minimalFramework test case in test/bake/dev/server-sourcemap.test.ts that updates the route and lib independently and asserts all three stack frames remap correctly after each patch, plus one extra assertion in the existing react HMR test that react-stack-bottom-frame still remaps after an update. A comment on the existing "repeated reloads" test is updated to reflect that patches now coexist rather than replace.

Security risks

None. This is dev-server-only source map bookkeeping — the URL is an internal key into SavedSourceMap and never exposed to untrusted input. No parsing, auth, crypto, or filesystem changes.

Level of scrutiny

Low-to-medium. The native change is 6 lines: a static atomic counter and a makeString call replacing a string literal. I traced the mechanism through DevServerSourceProvider::create → Bun__addDevServerSourceProvider → SavedSourceMap::put_source_provider (replaces by path key) and ~DevServerSourceProvider → remove_source_provider (matches on provider identity), which confirms the PR's cause analysis and that unique keys fix both the overwrite and the previously no-op removal. The atomic is correctly scoped: function-local static (thread-safe init in C++11+), seq_cst post-increment. unsigned wraparound at ~4.3B patches is not a practical concern for a dev session.

Other factors

• Grepped src/ for any code that pattern-matches on the old bake://server.patch.js string (special-casing in error display, source map lookup, etc.) — none. The only remaining occurrence is in BakeLoadServerHmrPatch (the no-source-map variant), which uses the plain SourceProvider and does not touch SavedSourceMap, so leaving it unchanged is correct as the PR states.
• The new test extracts frames from the dev error page's __bunfallback JSON payload — the same approach already used in test/bake/dev/react-response.test.ts and test/js/bun/http/serve.test.ts — rather than regex over ANSI-colored stdout, and asserts line numbers relative to the first round to avoid coupling to absolute positions. Path comparison splits on [\\/] for Windows.
• The PR description documents that both new assertions fail without the fix and pass with it, along with the neighboring bake test files and the ASAN destructor case.
• No CODEOWNERS entry covers src/runtime/bake/. No prior human review comments to address.