bake production + FrameworkRouter: remove unsafe; BundleV2 holds its transpiler as a ParentRef

[Jarred-Sumner]

What

Same programme as #40055 … #40260. Stacked on #40255 (base branch claude/devserver-zero-unsafe → #40214; retarget as those land).

src/runtime/bake/production.rs 28 → 4 (all-safe fn extern blocks), FrameworkRouter.rs 10 → 0, bake_body.rs 3 → 0, and DevServer.rs's last real block (bundle_borrows) is gone.

• BundleV2 holds its primary transpiler as ParentRef<Transpiler, Mut>, the same non-exclusive back-reference it already used for client_transpiler/ssr_transpiler, with transpiler()/transpiler_mut() accessors; init/generate_from_cli/scan_module_graph_from_cli/generate_from_bake_production_cli take the ParentRef (callers write ParentRef::from_ref_mut(..)). init_with_owned_heap(.., Box<Arena>, ..) stores the arena in a private OwnedHeap declared as the last field, so the bundle's arena is freed after every field that points into it by construction — this is what lets the dev server build a bundle without lifetime laundering. bundle_v2.rs 124 → 127 (the accessors), not a target here.
• production.rs: PerThread/AttachedPerThread typed; Bake__* SSG callbacks are HOST_EXPORTs; BakeGetOnModuleNamespace takes EncodedJSValue + a by-value {ptr,len} and dynamicDowncasts.
• FrameworkRouter.rs: InsertionContext is a trait object; bun_resolver::fs::DirEntry::iter() (safe, asserts the entries mutex in debug) and Entry::kind_with(&self, fs, fd) replace the raw-pointer walks; EntryKindResolver::resolve_kind/RealFS::kind take &self.
• bake_body.rs: the duplicated option structs (Framework, FileSystemRouterType, BuildConfigSubset, SplitBundlerOptions, StringRefList, arena_erase/arena_dupe_z, …) are deleted in favour of the bake:: ones with owned fields (UserOptions.root: Box<[u8]>, env_prefix: Option<Box<[u8]>>); ServerInitContext.js_string_allocations removed.

Residual contract (documented, unchanged from main): the owner keeps using its Box<Transpiler> while a BundleV2 holds a ParentRef to it. Pre-existing bug fixed in passing (with a test): fileSystemRouterTypes[].ignoreDirs was parsed by iterating the outer fileSystemRouterTypes array, so user ignoreDirs never matched.

Testing

Debug+ASAN, --timeout 60000, all exit 0: test/bake/{framework-router, deinitialization, dev-and-prod, serve-plugins-dev-server}, test/bake/dev/* (production, ssg-pages-router, bundle, css, esm, hot, html, plugins, react-spa, sourcemap, server-sourcemap, incremental-graph-edge-deletion, vfile, import-meta-inline ±, request-cookies, response-to-bake-response, react-response, harness, ecosystem, stress), bun-serve-html*, test/cli/hot/*, test/bundler/{bundler_edgecase, bundler_html, bundler_html_server, bundler_plugin, bundler_splitting, bun-build-api, esbuild/default}. Hand-driven: bun build --splitting --minify --sourcemap=linked and --target bun outdirs byte-identical to system bun; Bun.build with an onResolve/onLoad plugin; bun build --app on the react SSG fixture (dist tree identical to system bun); HTML dev server edit → HMR → rebundled content, clean SIGTERM — no ASAN output. clippy clean on bun_bundler/bun_resolver/bun_jsc/bun_runtime; rust-check-all windows-msvc + apple-darwin pass.

[robobun]

^{Updated 9:05 AM PT - Sep 8th, 2026}

@Jarred-Sumner, your commit 519f047 is building: #112994

[claude]

Thanks for landing the ignoreDirs fix + test in db4e272 — verified the new bundle.test.ts case exercises the corrected iterator. This run found no further issues. Given the scope (BundleV2's transpiler moving to ParentRef + the new OwnedHeap drop-order contract, the BakeGetOnModuleNamespace FFI signature change, and the wholesale collapse of the duplicate bake_body option structs into the Cow-backed bake:: types), a human pass is still worthwhile.

Checked: init_with_owned_heap error-path drop order (local OwnedHeap outlives the failed init box via LIFO); owned_heap is declared last so field-drop order frees the arena after every borrowing field; EntryKindResolver::resolve_kind moving to &self — RealFS::kind only does syscalls + interning under its own locks, no &mut self state touched; AttachedPerThread detaches before the box frees on all paths; StoredPart payload provenance matches the old to_owned_part invariant (both InsertPattern arms allocate into pattern_string_arena).

Extended reasoning...

Overview

Part of the ongoing unsafe-removal series (stacked on #40255). Twenty files: BundleV2 now holds its primary transpiler as ParentRef<Transpiler, Mut> with transpiler()/transpiler_mut() accessors and a new init_with_owned_heap that takes ownership of the bundle arena via a private OwnedHeap last-field; DevServer drops its bundle_borrows lifetime-laundering helper and the separate CurrentBundle.heap box; production.rs replaces the raw-ptr VM/PerThread plumbing with VirtualMachine::get_mut() + a typed AttachedPerThread that detaches on Drop; FrameworkRouter replaces Part<'static> lifetime erasure with a StoredPart{tag, RawSlice} and swaps raw Entry/RealFS pointer walks for new safe DirEntry::iter()/Entry::kind_with(); bake_body.rs deletes ~450 lines of duplicated option structs (Framework, FileSystemRouterType, BuildConfigSubset, StringRefList, arena_erase/arena_dupe_z) in favour of the canonical Cow-backed bake:: types; BakeGetOnModuleNamespace C++ signature now takes EncodedJSValue + a by-value {ptr,len} struct and dynamicDowncasts. Plus the ride-along ignoreDirs fix (with test) that my prior review flagged.

Security risks

None identified. No auth, crypto, or untrusted-input parsing surface changed. The FFI signature change (BakeGetOnModuleNamespace) is strictly safer — dynamicDowncast + null-return replaces an unchecked pointer cast.

Level of scrutiny

High. This is a large refactor of lifetime/ownership contracts in the bundler core: BundleV2's primary transpiler moves from &'a mut to a non-exclusive back-reference (with the aliasing contract now documented rather than type-enforced), and the dev server's bundle-heap ownership moves inside BundleV2 via a hand-rolled OwnedHeap(NonNull<Arena>) whose correctness depends on struct field drop order and LIFO local drop on the error path. The resolve_kind &mut self → &self change touches the resolver FS layer used process-wide. These are exactly the shapes where a subtle drop-order or aliasing regression would only surface under ASAN on specific paths — the description's test list is extensive and covers those paths, but a maintainer should confirm the ParentRef aliasing contract and OwnedHeap last-field placement match their mental model.

Other factors

The prior review's only finding (ignoreDirs iterating the wrong array) was fixed in db4e272 with a regression test that asserts /ignored is 404. This run's bug-hunting pass found nothing new. The PR is stacked on #40255, so the base branch is not main — a human should confirm the stack is landing in order. No CODEOWNERS file in the repo. Testing coverage described is thorough (Debug+ASAN across the full bake/bundler/hot suites, byte-identical outdir checks, cross-target rust-check-all).

[claude]

I reviewed this PR and didn't find any bugs (the earlier ignoreDirs nit is now fixed in db4e272 with a test). Because it restructures ownership in BundleV2 — the ParentRef<Transpiler> back-reference contract, the new OwnedHeap drop-order invariant, and the collapse of the duplicated bake_body option structs — a human look at the lifetime/aliasing story would still be worthwhile.

What was reviewed:

• OwnedHeap field ordering and init_with_owned_heap error path — heap drops after bv2 on both success and ?-return.
• BakeOptions.ssr_transpiler: Option<NonNull<_>> — None correctly falls through to the ssr_alias = transpiler.as_mut_ptr() default in init.
• AttachedPerThread drop order vs. _api_lock/_vm_guard — detach runs under the API lock, before VM destroy.
• BakeGetOnModuleNamespace FFI reshaping — dynamicDowncast + {} on mismatch maps to the Rust-side is_empty() guard.

Extended reasoning...

Overview

This PR is the next step in the ongoing unsafe-removal series for src/runtime/bake/. It touches 20 files across the bundler core (bundle_v2.rs, LinkerContext.rs), the bake dev server / production build (DevServer.rs, production.rs, FrameworkRouter.rs, bake_body.rs, mod.rs), the resolver (fs.rs, lib.rs), server config, and one C++ FFI shim (BakeSourceProvider.cpp). The headline change is that BundleV2.transpiler becomes a ParentRef<Transpiler, Mut> (matching the existing client_transpiler/ssr_transpiler pattern) with transpiler()/transpiler_mut() accessors, and a new init_with_owned_heap constructor lets the dev server hand the bundle its arena as an owned last-field so drop order is correct by construction. Separately, the duplicated bake_body::{Framework, FileSystemRouterType, BuildConfigSubset, …} structs and the StringRefList/arena_erase lifetime-erasure machinery are deleted in favour of the canonical bake:: types with owned Cow/Box fields. A pre-existing ignoreDirs parsing bug is fixed with a regression test.

Security risks

None identified. No auth/crypto/permissions surface is touched. The FFI signature change to BakeGetOnModuleNamespace (now takes EncodedJSValue + a by-value {ptr,len} struct and dynamicDowncasts) is strictly more defensive than the old uncheckedDowncast on a raw pointer.

Level of scrutiny

High. This is a memory-ownership refactor at the heart of the bundler and dev server:

• The OwnedHeap last-field invariant is what guarantees the bundle's arena outlives every 'a borrow into it; getting this wrong is UAF.
• The ParentRef<Transpiler, Mut> contract ("owner keeps using its Box<Transpiler> while a BundleV2 holds a ParentRef to it") is documented but not compiler-enforced — transpiler_mut() calls assume_mut() on that promise.
• AttachedPerThread's detach-in-Drop must run before the VM guard destroys the VM and while the API lock is held; the declaration order in build_command establishes that but is easy to perturb.
• Collapsing bake_body's duplicated structs into the keystone types changes every string field from arena-borrowed &'static [u8] to owned Cow/Box — behaviour-preserving in principle but a wide surface.

None of this is mechanical enough for auto-approval.

Other factors

• CI is green (Build #105149).
• The PR is stacked on #40255 → #40214, so the base branch is not main; a human should confirm the retarget plan.
• Debug+ASAN test coverage listed in the description is extensive (all of test/bake/*, bundler suites, hand-driven bun build --app byte-identical outdirs), and rust-check-all passed for windows-msvc + apple-darwin.
• The one prior review comment (mine, on ignoreDirs) was addressed with both the fix and a test; that thread is resolved.
• The bug-hunting system found nothing this run.

[Jarred-Sumner]

CI's release lanes failed to compile (-D unused-mut on watcher_acquire_event's guard in dev_server/mod.rs, which is only written under debug_assertions). Fixed on the parent #40255 (5b23aae) and restacked this branch onto it (3fc0276); no other change.

[claude]

I re-reviewed after the ignoreDirs fix + test landed and the restack; no new issues found. Given the scope — 21 files, BundleV2's transpiler ownership model changing to ParentRef, the new OwnedHeap drop-order contract, and the collapse of the duplicated bake_body/bake struct families — a human look is still warranted.

Checked: OwnedHeap is the last field of BundleV2 so arena drops after all borrowers; init_with_owned_heap's error path drops heap after this; AttachedPerThread detaches before the box frees; BakeGetOnModuleNamespace now dynamicDowncasts and the Rust side handles the empty return; BakeOptions.ssr_transpiler: None correctly falls back to the primary-transpiler alias.

Extended reasoning...

Overview

Continuation of the unsafe-removal series. The load-bearing changes are: (1) BundleV2.transpiler becomes ParentRef<Transpiler, Mut> with transpiler()/transpiler_mut() accessors, and all four init/generate_* entry points plus every caller (build_command, ChangedFilesFilter, JSBundleCompletionTask, DevServer, bake production) are updated; (2) a new init_with_owned_heap stores a boxed arena in a private OwnedHeap last-field so DevServer can drop bundle_borrows and its lifetime-detach unsafe; (3) bake_body.rs deletes the parallel &'static [u8]-backed Framework/FileSystemRouterType/BuildConfigSubset/SplitBundlerOptions/StringRefList/arena_erase machinery in favour of the single Cow-backed bake:: types, and ServerConfig/server_body/server/mod.rs drop the bridge conversions; (4) production.rs replaces the MaybeUninit<Transpiler> triple, the DotenvSingleton UnsafeCell, and the raw-pointer PerThread attach/detach with a boxed AttachedPerThread whose Drop detaches; (5) FrameworkRouter.rs replaces to_owned_part's lifetime-detach with a StoredPart{tag, RawSlice} and the raw-pointer Entry/RealFS walks with a safe DirEntry::iter() + Entry::kind_with(&self, &fs); (6) the BakeGetOnModuleNamespace C++ FFI takes EncodedJSValue + a by-value {ptr,len} and dynamicDowncasts. The ride-along ignoreDirs fix (my prior nit) is applied with a dev-server test.

Security risks

None in the auth/crypto sense. The whole PR is memory-safety surface: aliasing contracts around ParentRef<_, Mut> (documented as non-exclusive, matching the pre-existing client_transpiler/ssr_transpiler pattern), arena drop order via struct field ordering, and FFI pointer registration lifetime for PerThread. These are exactly the kind of invariants the repo's review guidelines call out as most-blocked.

Level of scrutiny

High. bundle_v2.rs is the bundler's core struct and its transpiler-holding model is being changed; OwnedHeap relies on Rust's field-drop-order guarantee to keep arena borrows valid through drop; DevServer's async bundle lifecycle now depends on that instead of explicit ordering in CurrentBundle. The struct-family collapse in bake means every consumer of the old bake_body:: shapes had to be re-plumbed. This is well beyond "mechanical" even though each individual site is small.

Other factors

• Stacked on two open PRs (#40255 → #40214), so the base isn't main.
• Test coverage is broad per the description (bake/bundler/hot suites under debug+ASAN, byte-identical outdir checks, cross-target rust-check-all), and the ride-along fix has its own test.
• My previous inline nit was addressed and resolved; the only change since is a #[cfg_attr(.., allow(unused_mut))] for release lanes.
• I spot-checked the OwnedHeap error-path ordering, ssr_transpiler: None falling through to the pre-set ssr_alias, AttachedPerThread::Drop running before the box frees, and the C++/Rust signature match for BakeGetOnModuleNamespace — all look correct. But the aliasing contract on transpiler_mut() ("not borrowed elsewhere while the bundler runs") vs the doc comment saying the dev server resolves through it between turns is subtle enough that a maintainer should sign off on the design.

[claude]

Re-ran on the latest push and didn't find any new issues — the ignoreDirs fix now iterates the correct array and has a dev-server test covering it. Given the scope (ParentRef-held transpiler, OwnedHeap drop ordering, arena-erased 'static → owned field migration across bake/production/FrameworkRouter), a human pass on the ownership and drop-order changes is still worthwhile.

What was reviewed:

• bake_body.rs ignoreDirs parsing — now reads exts_js with its own length; test in bundle.test.ts exercises a non-default ignored dir.
• FrameworkRouter StoredPart/RawSlice and Entry::kind_with(&self) rewrite — checked that the &self relaxation on EntryKindResolver doesn't drop a needed &mut (the cache write goes through interior mutability).
• BundleV2::init_with_owned_heap field ordering — OwnedHeap is declared last so the arena outlives fields borrowing from it.

Extended reasoning...

Overview

This PR is a large (~1430 deletions / ~700 additions) unsafe-elimination refactor across the bake dev-server and production pipeline. It replaces arena-backed lifetime-erased &'static [u8] fields with owned Box<[u8]>/Cow<'static,[u8]>, deletes the StringRefList/arena_erase/arena_dupe_z machinery and the duplicate option structs in ServerConfig.rs, changes BundleV2 to hold its primary transpiler as a ParentRef<_, Mut> back-reference with a new init_with_owned_heap/OwnedHeap entry so the dev server can own the bundle arena across event-loop turns, rewrites FrameworkRouter's directory scan to use safe DirEntry::iter()/Entry::kind_with(&self) instead of raw-pointer walks, and unifies Framework::init_transpiler into mod.rs. Call sites across build_command.rs, hot_reloader.rs, LinkerContext.rs, ChangedFilesFilter.rs, server_body.rs, and others are updated to the new accessors. A separate one-token bug fix makes fileSystemRouterTypes[n].ignoreDirs iterate its own array instead of the outer router-types array, with a new devTest in test/bake/dev/bundle.test.ts.

Security risks

No new attack surface — this is an internal ownership/lifetime restructuring with no changes to input parsing beyond the ignoreDirs fix (which tightens behavior to match the documented contract). The BakeGetOnModuleNamespace FFI signature change to EncodedJSValue + by-value slice and dynamicDowncast is a hardening move. No auth, crypto, or permission paths touched.

Level of scrutiny

High. The change deliberately restructures who owns what and when it drops: OwnedHeap relies on struct-field drop order to keep the arena alive past every field that points into it; ParentRef<Transpiler, Mut> is a non-exclusive back-reference whose contract (owner keeps using its Box<Transpiler> while the bundle holds the ref) is documented but not compiler-enforced; BakeOptions.ssr_transpiler becoming Option<NonNull<_>> with implicit aliasing to the primary transpiler when None is a subtle invariant. These are exactly the memory-safety patterns REVIEW.md flags for careful human review even when the net unsafe count drops.

Other factors

The PR description lists a broad debug+ASAN test matrix and byte-identical bun build outdirs vs. system bun, plus clippy and cross-target checks. My earlier inline finding (the ignoreDirs array/length mix-up) was addressed in commit 357bfa0 with the exact fix suggested and a regression test that would fail on the old code path (/ignored would have been routed). The bug hunt on this push exited on dry_streak with no new findings and nothing ruled out. This is stacked on another open branch, so a maintainer will need to sequence the merge regardless.

[claude]

Re-reviewed after the force-push and didn't find any bugs. The ignoreDirs iteration I flagged earlier is now fixed (iterates exts_js with its own length) and covered by the new devTest in bundle.test.ts with a positive control. Given the size of the ownership/aliasing refactor — ParentRef<Transpiler, Mut> coexisting with the owner's Box<Transpiler>, the owned_heap field-order-dependent drop, and the BakeGetOnModuleNamespace ABI change — a human pass is still worthwhile.

What was reviewed:

• bake_body.rs ignoreDirs parsing — now reads the correct array and sizes from it; test asserts both the ignored route 404s and a sibling route still resolves.
• BundleV2::init_with_owned_heap / OwnedHeap — field is declared last so the arena drops after borrowers; Drop ordering matches the stated intent.
• BakeSourceProvider.cpp — dynamicDowncast result is null-checked before use; new FfiSlice param is passed by value on both sides.
• FrameworkRouter StoredPart — RawSlice is only re-borrowed against the same Utf8Bytes it was sliced from; no lifetime widening beyond what the old to_owned_part() did.

Extended reasoning...

Overview

This PR is a ~2,100-line refactor (net −729) that removes unsafe blocks from src/runtime/bake/{production.rs, FrameworkRouter.rs, bake_body.rs, DevServer.rs} and reworks how BundleV2 holds its primary transpiler (now ParentRef<Transpiler, Mut> instead of &'a mut, with new accessors and an init_with_owned_heap constructor that lets the bundle own its arena). It also changes an FFI signature (BakeGetOnModuleNamespace), replaces a lifetime-erasing helper in FrameworkRouter with a tagged StoredPart, adjusts EntryKindResolver::resolve_kind to &self, and deletes ~700 lines of duplicated option structs from bake_body.rs. Along the way it fixes a pre-existing bug where fileSystemRouterTypes[n].ignoreDirs was iterated from the wrong array, with a regression test added to test/bake/dev/bundle.test.ts. The remaining files are call-site updates.

Security risks

No new attack surface. The change is internal ownership plumbing in the bundler/dev-server; no user-input parsing paths are loosened. The ignoreDirs fix tightens routing (previously-ignored directories now actually skip route registration), which if anything reduces accidental exposure. The C++ dynamicDowncast guard in BakeSourceProvider.cpp adds a null check where a jsCast would previously have UB'd on a wrong-typed value.

Level of scrutiny

High. This is exactly the category REVIEW.md flags as most-blocked: memory ownership refactors across Rust/C++ FFI, arena lifetime management where drop order is load-bearing, and a ParentRef<_, Mut> back-reference that intentionally aliases a Box the owner keeps using. The PR description documents this as a residual contract carried over from main, and the pattern matches what client_transpiler/ssr_transpiler already do, but a maintainer familiar with the bun_ptr aliasing rules should confirm the transpiler_mut() call sites don't overlap with direct Box<Transpiler> access in the same frame. The OwnedHeap last-field trick is correct Rust drop-order semantics but is subtle enough to warrant eyes.

Other factors

Since my previous inline comment, the branch was fully re-pushed as four commits; the one concrete issue I raised (ignoreDirs iterating the outer array) is now fixed with a test that has both a positive and negative assertion. The bug-hunting run on this push exited on dry_streak with no findings and no ruled-out candidates. The PR description lists extensive local test coverage under debug+ASAN plus byte-identical output checks against system bun, and clippy/rust-check-all on non-host targets. There are earlier COMMENTED reviews under this app's identity whose content I cannot see, and no outstanding CHANGES_REQUESTED from a human — but the scope alone keeps this out of auto-approve territory.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

Code review found no issues

No high-confidence issues detected in this change.