node:module: make findSourceMap() honor process.setSourceMapsEnabled()

[robobun]

module.findSourceMap() always returns undefined, even after process.setSourceMapsEnabled(true), which is accepted and then ignored. Tooling that uses the API Node documents for stack remapping and coverage silently gets no mappings for every module.

import { findSourceMap } from "node:module";
process.setSourceMapsEnabled(true);
await import("./lib.ts");
const sm = findSourceMap(require.resolve("./lib.ts"));
console.log(sm?.constructor.name, sm?.findEntry(0, 13).originalSource);
// node: SourceMap file:///.../lib.ts
// bun:  undefined undefined

Cause

The registry lookup in find_source_map was gated on

/// TODO: when we implement --enable-source-map CLI flag, set this to true.
pub(crate) static ENABLE_SOURCE_MAPS: AtomicBool = AtomicBool::new(false);

and nothing ever stored true. process.setSourceMapsEnabled() wrote to Process::m_sourceMapsEnabled, which nothing read.

Flipping the gate alone is not enough: a SourceMap from findSourceMap() is built from Bun's already-parsed mappings, not from the JSON they came from, so payload was undefined and the only entry in sources was the generated file rather than the original source. That missing payload is why the gate was left off: Next.js reads sourceMap.payload on startup and logs Invalid source map. Only conformant source maps can be used to filter stack frames. Cause: TypeError: payload is not an Object.

Fix

• process.setSourceMapsEnabled() stores the flag on the VirtualMachine (per-VM, like Node's per-Environment flag), and find_source_map reads it. Default stays off, matching Node.
• sources resolve against the generated file's URL, the same as Node's sourcesToAbsolute(), so findEntry().originalSource points at the original file.
• payload is re-encoded from the parsed map into a conformant v3 document on first access (version, sources, names, mappings). It round-trips through new SourceMap(payload).

While in JSSourceMap: sources/names held bun.String, which is Copy and has no Drop, so the +1 each element carried was never released. They now hold OwnedString, which also covers the ? unwind paths out of the constructor.

Known gaps, not addressed here

Both predate this PR and are independent of it. Listing them so the tradeoffs are visible:

• Bun does not compose a file's own //# sourceMappingURL with the map it generates while transpiling that file, so for a module Bun transpiles, findSourceMap() returns Bun's map rather than chaining to the file's. An already-bundled (// @bun) file uses its own map, and that case matches Node.
• Maps in SavedSourceMap are parsed with include_names: false (the only ParseUrlResultHint::All construction site, SavedSourceMap.rs:460), so findEntry().name is already undefined on this path no matter what the map declared. payload.names is therefore [], which matches the 4-field mappings write_vlqs emits. Node returns the names. Making them work means parsing names into the cached map, teaching write_vlqs the optional 5th field, and accepting the extra memory on the hot stack-remapping path, so it belongs in its own change.
• payload omits file, sourceRoot, and sourcesContent. All three are optional in the v3 spec, and Bun deliberately does not keep source contents resident (see the comment on ParsedSourceMap::underlying_provider). Bun's parser has no references to sourceRoot anywhere, so stack remapping already ignores it.

Verification

$ bun bd test test/js/node/module/sourcemap.test.js
 18 pass, 0 fail

Three of the four new tests fail against the released binary; bun bd test test/js/node/module/ test/js/bun/sourcemap/ is 120 pass / 0 fail.

After the fix

$ bun-debug -e '...' # TS file, transpiled by Bun
entry:   {"generatedLine":0,"generatedColumn":13,"originalLine":0,"originalColumn":13,"originalSource":"file:///tmp/fsm/ts/lib.ts"}
origin:  {"line":0,"column":13,"fileName":"file:///tmp/fsm/ts/lib.ts"}
payload: {"version":3,"sources":["file:///tmp/fsm/ts/lib.ts"],"names":[],"mappings":"AAAO,aAAM,IAAY;..."}

A GC/ASAN stress over 3000 new SourceMap() + findSourceMap() + payload iterations (including the constructor's throwing path) is clean.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 11 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: 3a4f7fce-cea8-4e26-9455-51d388283928

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff9eb and 796e175.

📒 Files selected for processing (6)

• src/jsc/VirtualMachine.rs
• src/jsc/bindings/BunProcess.cpp
• src/jsc/bindings/BunProcess.h
• src/jsc/virtual_machine_exports.rs
• src/sourcemap_jsc/JSSourceMap.rs
• test/js/node/module/sourcemap.test.js

---

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

[robobun]

^{Updated 11:24 AM PT - Jul 6th, 2026}

❌ @robobun, your commit 796e175 has some failures in Build #68967 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 33454

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

bun-33454 --bun

[claude]

I didn't find any bugs, but this changes user-visible Node compat behavior that was deliberately gated off (the Next.js payload issue), reworks refcount ownership in JSSourceMap, and makes a design call to re-encode payload from parsed mappings rather than the original JSON — worth a human look.

Extended reasoning...

Overview

This PR wires process.setSourceMapsEnabled() to a new per-VM source_maps_enabled flag on VirtualMachine, replacing a dead process-global AtomicBool and the unread Process::m_sourceMapsEnabled field. module.findSourceMap() now reads that flag and, when enabled, returns a SourceMap whose sources are resolved against the generated file's URL (matching Node's sourcesToAbsolute()) and whose payload getter re-encodes a conformant v3 document from Bun's already-parsed mappings. It also switches JSSourceMap::{sources,names} from bun.String (Copy, no Drop) to OwnedString to release the +1 each element carries on both GC finalize and ?-unwind paths. A new Bun__VM__setSourceMapsEnabled FFI export bridges the C++ process object to the Rust VM. Four new subprocess tests cover the gate, transpiled-file mapping, inline sourceMappingURL resolution, and miss cases.

Security risks

None identified. No auth, crypto, or untrusted-input parsing is touched; the sourcemap parser itself is pre-existing and the new code only re-encodes already-parsed data and does URL joining via existing helpers.

Level of scrutiny

This warrants maintainer review rather than auto-approval. The prior code contained an explicit comment explaining that findSourceMap() was left disabled because enabling it without a real payload broke Next.js on startup — so flipping this on is a deliberate behavior change for real-world tooling, and the claim that the new get_payload() resolves that concern should be validated by someone who knows the history. The refcount/ownership rework (String → OwnedString, dupe_ref vs get in resolve_sources, the file:// path replacement) is exactly the memory-safety category the repo's review guidelines flag as most-blocked.

Other factors

• Design tradeoff: payload is reconstructed from decoded mappings, so it drops sourcesContent, sourceRoot, file, and original names from maps that had them. The PR argues it round-trips through new SourceMap(payload), but a maintainer should confirm this shape is sufficient for the tooling ecosystem (Next.js, coverage tools) the description cites.
• Acknowledged divergence: the PR notes Bun still doesn't chain a transpiled file's own //# sourceMappingURL through its generated map, so findSourceMap() on a Bun-transpiled file returns Bun's map rather than the file's — a known Node-compat gap left for later.
• VirtualMachine field addition and a new C++→Rust FFI export are small but touch core runtime plumbing.
• Test coverage is solid (per-process isolation, both path and file:// specifiers, payload round-trip, negative cases) and the description reports ASAN/GC stress is clean, but CI was still building at the time of review.

[robobun]

Answering the three things the review flagged, with the evidence behind each.

The gate and Next.js

The default is unchanged: findSourceMap() still returns undefined until something calls process.setSourceMapsEnabled(true). Node behaves the same way, so the two agree on the default. Next.js never calls it, so nothing changes for a default next dev / next start under Bun:

# next@15.5.20
$ grep -rl 'setSourceMapsEnabled' node_modules/next/dist | wc -l
0
$ grep -rl 'findSourceMap' node_modules/next/dist | wc -l
92

And the failure the old comment described, TypeError: payload is not an Object out of "sections" in payload, cannot fire anymore regardless, because payload is now an object. That was the whole reason the gate existed.

Refcount ownership

The old code was leaking, which is what pushed me to change the field types. bun.String is Copy with no Drop, so:

• JSSourceMap::{sources,names} were never released. The default finalize drops the Box<[...]>, but a bun.String element has no destructor, so every new SourceMap({ sources: [...] }) leaked one WTF ref per source, and a ? unwind in the constructor leaked the partially built Vec.
• URL::path_from_file_url(s) and friends take String by value, but because String: Copy that is a borrow, not a transfer (URL__pathFromFileURL only calls toWTFString() on it). The old source_url_string.dupe_ref() argument leaked a ref on every call.

OwnedString is #[repr(transparent)] over String with a Drop that calls deref(), so the balance is now mechanical. Every value stored is a fresh +1 (to_bun_string, clone_utf8, URL::join, URL::file_url_from_string, dupe_ref), released exactly once at finalize or on unwind. .get() is the non-bumping accessor, used only for the FFI calls that borrow.

3000 iterations of new SourceMap() + findSourceMap() + payload + the constructor's throwing path, with Bun.gc(true) interleaved, is clean under the ASAN debug build.

The payload shape

Bun deliberately does not keep the source map JSON or its sourcesContent resident (see the comment on ParsedSourceMap::underlying_provider), so reconstructing is the only option that does not change how maps are stored. Side by side with Node, for the same inline map:

field	Node	this PR
version	3	3
sources	absolute URLs	absolute URLs
mappings	verbatim	re-encoded from the parsed map
names	verbatim	[]
file	verbatim	absent
sourceRoot	""	absent
sourcesContent	verbatim	absent

file, sourceRoot and sourcesContent are all optional in the v3 spec. names: [] is not lost fidelity but a consequence of write_vlqs never emitting the optional name index, so the payload stays internally consistent: its mappings carry no name references. A consumer built from it resolves the same entries, which is what the round-trip assertion in the new test covers.

Worth knowing: Bun's source map parser has no references to sourceRoot anywhere in src/sourcemap/, so stack remapping already ignores it. I kept that behavior rather than honoring it only on this one path, since making it consistent means touching ParsedSourceMap, parse_json and Mapping::Lookup. Happy to do that separately if you want it.

CI

The Windows failures on the first build were cookie-map.test.ts, already fixed on main by #33425. Rebased past it. Format, Lint JavaScript and cargo clippy are green.

[claude]

I didn't find any bugs, but this changes user-visible Node compat behavior (findSourceMap() now returns real maps, which was previously gated off specifically because of Next.js breakage) and touches refcount handling and FFI wiring, so it's worth a human look at the semantics and the OwnedString lifetime changes.

Extended reasoning...

Overview

This PR wires process.setSourceMapsEnabled() through to a new per-VM source_maps_enabled flag and makes module.findSourceMap() honor it. It touches VirtualMachine.rs (new field), BunProcess.cpp/.h (removes dead m_sourceMapsEnabled, calls a new FFI export), virtual_machine_exports.rs (new Bun__VM__setSourceMapsEnabled), and JSSourceMap.rs (replaces the always-false ENABLE_SOURCE_MAPS static, adds resolve_sources() for URL-relative source resolution, implements get_payload() to re-encode a v3 sourcemap document, and switches sources/names from bun.String to OwnedString to fix a refcount leak). Four new subprocess tests are added.

Security risks

None identified. The flag defaults off (matching Node), so there's no new surface exposed by default. No auth/crypto/permissions code is touched. Inputs to the new resolve_sources/get_payload paths are already-parsed sourcemap data from Bun's own registry.

Level of scrutiny

Medium-high. The prior code had an explicit comment stating the gate was left off because enabling it broke Next.js (payload is not an Object). This PR addresses that by synthesizing a conformant payload, but that's a design decision (re-encode from parsed mappings rather than retain the original JSON) that a maintainer should sign off on — especially since the re-encoded payload drops names, sourcesContent, and any other fields the original map carried. The bun.String → OwnedString refcount change and the dupe_ref()/get() handling in resolve_sources also deserve a careful eye from someone familiar with the string ownership conventions.

Other factors

The PR is well-tested (four new subprocess tests covering the gate toggle, transpiled files, inline // @bun maps, and miss cases; payload round-trips through new SourceMap()), the description is thorough, and the bug-hunting system found no issues. It also cleanly deletes the dead m_sourceMapsEnabled field and ENABLE_SOURCE_MAPS static. But it's a ~200-line behavior change across the Rust/C++ FFI boundary with memory-management implications, not a mechanical fix, so I'm deferring rather than approving.

[robobun]

Same review re-run against the rebased commit, so I won't repeat the previous answer. One point is worth sharpening, though, because I checked it and my earlier framing undersold it.

payload.names

I said names: [] was "not lost fidelity." That's true of the payload in isolation, but it is a Node compat gap, and it is worth being precise about where it comes from. It is not introduced by reconstructing the payload: maps in SavedSourceMap are parsed with include_names: false at the only ParseUrlResultHint::All construction site (SavedSourceMap.rs:460), so the names never reach ParsedSourceMap in the first place.

Given a map that does declare names and references them in its mappings:

{ version: 3, sources: ["orig.ts"], names: ["myFn", "myVar"], mappings: "AAAAA,CAACC" }

bun (this PR)                         node v26.3.0
findEntry(0,0).name: undefined        findEntry(0,0).name: myFn
findEntry(0,1).name: undefined        findEntry(0,1).name: myVar
payload.names:       []               payload.names:       ["myFn","myVar"]
payload.mappings:    "AAAA,CAAC"      payload.mappings:    "AAAAA,CAACC"

So findEntry().name is already undefined on this path regardless of this PR, and payload.names: [] faithfully matches the 4-field mappings write_vlqs emits. Fixing it properly means parsing names into the cached map, teaching write_vlqs the optional 5th field, and paying the extra memory on the hot stack-remapping path, which is three interacting changes in a subsystem this PR otherwise only reads from. I've listed it in the PR description alongside the other known gaps rather than bolt it on here, but say the word if you'd rather it land together.

Worth noting the SourceMap constructor path is unaffected: new SourceMap(payload) parses names and findEntry().name works there today, and still does.

CI

Green so far on the rebased head (build #68869 running). The earlier Windows red was cookie-map.test.ts, fixed on main by #33425.

[claude]

I didn't find any bugs, but this touches refcount ownership (bun.String → OwnedString), FFI plumbing, and Node-compat semantics in an area with a documented Next.js footgun, plus several explicitly-flagged design tradeoffs (reconstructed payload, deferred names support) — worth a maintainer's eyes.

Extended reasoning...

Overview

This PR wires process.setSourceMapsEnabled() to actually gate module.findSourceMap(), replacing a dead AtomicBool and an unread C++ field with a per-VM flag. It also substantially reworks JSSourceMap.rs: changes the sources/names field types from bun.String to OwnedString to fix a refcount leak, adds resolve_sources() to resolve relative source URLs against the generated file (matching Node's sourcesToAbsolute()), and implements get_payload() to re-encode a conformant v3 sourcemap document from the parsed mappings. Six files changed, ~200 lines of native code plus four new subprocess tests.

Security risks

None apparent. The change reads user-provided module specifiers but only routes them through existing URL helpers and the existing SavedSourceMap lookup; no new parsing of untrusted bytes, no filesystem/network effects.

Level of scrutiny

High. This is not a mechanical change:

• The String → OwnedString refactor is a memory-safety fix touching refcount ownership across FFI. The author's analysis of which calls hand out a +1 vs. borrow (dupe_ref, URL::join, .get()) is careful and looks correct, and they report a clean 3000-iteration ASAN/GC stress, but this is exactly the class of change CLAUDE.md flags as most-blocked and warrants a second pair of eyes.
• The old gate carried an explicit comment about breaking Next.js. The author has argued convincingly that the default is unchanged and payload is now an object so the original failure can't recur, but a maintainer should confirm they're comfortable with that reasoning.
• The reconstructed payload deliberately omits names, file, sourceRoot, and sourcesContent, and re-encodes mappings rather than returning them verbatim. The author has documented these as known gaps and explained why each is deferred, but whether to ship the partial implementation vs. land it together is a maintainer call the author has explicitly asked about.

Other factors

Test coverage is good: four new subprocess tests covering the gate toggle, transpiled-file mapping with payload round-trip, inline sourceMappingURL resolution, and miss cases. The PR description and follow-up comments are unusually thorough. No prior human review on the thread; the coderabbit review was rate-limited and never ran.

[robobun]

CI status: the red is infrastructure, not this diff

Two consecutive builds have exactly one hard failure, :darwin: 26 aarch64 - test-bun, and it never runs a test. It dies during setup in scripts/runner.node.mjs:2182:

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'.
Refusing to continue with a partial download (would silently fall back to the wrong binary).

The artifact it cannot fetch is the bun binary itself, so zero tests execute on that shard.

Why this is not the diff:

• :darwin: aarch64 - build-bun passed with exit 0 on both builds, so the artifact exists and is uploaded.
• On build #68967, one :darwin: 26 aarch64 - test-bun shard passed while its sibling shard timed out downloading the same artifact from the same build. Same binary, same commit, opposite outcomes.
• Both failures landed on the same agent, darwin-aarch64-26-5-1-1.
• Neither build's annotations mention source maps at all. The only annotation on either is the flaky retry bucket (update_interactive_install, napi, serve, a @verdaccio/ui-theme npm integrity check, and friends), none of which this PR touches.
• Nothing in this change can affect buildkite-agent's S3 artifact download.

Build #68967 has now finished. Final tally: 186 passed, 1 hard failure (that darwin shard), plus 94 jobs left in waiting_failed, blocked behind it rather than failing on their own. Every test lane that got its artifact passed, including 20 alpine 3.23 x64-baseline - test-bun shards, debian 13 x64 - test-bun, and the sibling darwin 26 aarch64 - test-bun shard.

I have already spent my one re-roll (796e175a05, an empty ci: retrigger) and it reproduced identically, so I am not going to push again. Restarting that single job should clear it.

The diff itself

Unchanged since it was verified, and green locally:

$ bun bd test test/js/node/module/ test/js/bun/sourcemap/
 120 pass, 0 fail

Three of the four new tests fail against the released binary, which is the fail-before proof that they exercise the fix. A 3000-iteration GC stress over new SourceMap() + findSourceMap() + payload, including the constructor's throwing path, is clean under ASAN.

This is ready for a human. The open questions are the two design calls I flagged in the description rather than anything CI found: whether reconstructing payload from the parsed mappings is the right approach, and whether the deferred gaps (names, sourceRoot, //# sourceMappingURL chaining) should land separately or together. Happy to fold any of them in.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-06, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.