Skip to content

repl: clone the parsed options into the VM instead of moving them out of the CLI context - #46264

Open
robobun wants to merge 1 commit into
mainfrom
robobun/424c3b24/repl-serve-static-bunfig
Open

robobun wants to merge 1 commit into
mainfrom
robobun/424c3b24/repl-serve-static-bunfig

Conversation

@robobun

@robobun robobun commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • In bun repl, a Bun.serve HTML route built without the HMR dev server ignores minify, define and sourcemap from bunfig [serve.static]. bun file.ts honors them.
  • The REPL boot empties args in the process-global CLI context: core::mem::take(&mut ctx.args) (src/runtime/cli/repl_command.rs:80). Route::on_plugins_resolved (src/runtime/server/HTMLBundle.rs:424) reads those keys there and finds None.
  • Regression from the Rust port (Rewrite Bun in Rust #30412): the Zig REPL copied the struct. 1.3.13 honors the table, 1.4.2 does not. No issue reports it.

Fix

  • The REPL clones the options (ctx.args.clone()), like bun run, bun test, standalone executables and the bake build. The global keeps its values.
  • Verified: test/js/bun/http/bun-serve-html.test.ts (6 new cases, 4 fail on 1.4.3 and on main). Also ran repl.test.ts, bake/dev-and-prod.test.ts.
  • Self-reviewed: 14 concerns raised, 12 addressed. Kept both test modes (each flips different defaults). No issue opened for the --smol gap in Notes.

Background

  • The CLI parses flags and bunfig.toml into a process-global ContextData. A command's main VM boots with a copy of its args.
  • An HTML route reads publicPath and env from the VM copy and the three keys above from the global, so the REPL honored only the first two.
  • Considered moving those reads to the VM copy. The global would stay empty for the next reader (Apply CLI and bunfig define to HTML bundles served by Bun.serve #40485 adds one).

Downsides

  • One more options clone per bun repl process: +3 allocations (56 B) and +635 instructions in an empty directory. With the table from Notes: +8 allocations (136 B) and +984 instructions.
  • No other command pays: bun -e 1 makes 4436 allocator calls before and after.
Notes

Trigger. All of these at once: bun repl, an HTML import as a Bun.serve route, a mode without the HMR dev server (development: false, development: { hmr: false } or NODE_ENV=production), and minify, define or sourcemap in [serve.static]. The default mode is not affected: the HMR dev server reads serve_define from the VM copy (src/runtime/server/ServerConfig.rs:956). bun file.ts and bun -e are not affected. A read of the port found this. No user reported it.

Repro. bunfig.toml:

[serve.static]
minify = false
sourcemap = "linked"

[serve.static.define]
BUILD_FLAG = '"from-bunfig"'

index.html loads app.ts (function describeTheFlag(flag: string) { return flag === undefined ? "none" : "MARKER " + flag; } console.log(describeTheFlag(BUILD_FLAG));). serve.ts serves the page with Bun.serve({ port: 0, development: false, routes: { "/": index } }), fetches the script chunk and its .map, and prints both.

  • bun serve.ts and bun -e "$(cat serve.ts)": the chunk keeps describeTheFlag, holds "from-bunfig" and a sourceMappingURL comment, and the .map request answers 200.
  • bun repl -e "$(cat serve.ts)" on 1.4.3: function t(e){return e===void 0?"none":"MARKER "+e}console.log(t(BUILD_FLAG));, and the .map request answers 404.
  • With development: { hmr: false }, minify = true and sourcemap = "none" the REPL does the opposite: an unminified chunk, a bare BUILD_FLAG, and a sourcemap that the table turned off.

History. git show d4514457e8^:src/runtime/cli/repl_command.zig line 44 is .args = ctx.args, a struct copy. src/runtime/cli/test_command.rs:1952 has the same Clone (not take) comment for the same reason: a later reader of ctx.args.

Tests. The new describe runs one script in three ways: bun serve.ts, bun repl -e, and bun repl with the script typed on stdin. It has two modes: development: false with minify = false and sourcemap = "linked", and development: { hmr: false } with minify = true and sourcemap = "none". The 2 bun serve.ts cases pass before and after. They pin the reference that the REPL must match, and they are the first test of [serve.static] minify and define outside the REPL. The 4 REPL cases fail without the fix. The cases are serial because six debug ASAN processes at once take about 4 s on a 12-core machine, close to the 5 s limit. The script has no quote inside a regex literal, because the REPL line reader takes it for an open string (#37326, fix in #37327).

The interactive prompt. At an idle prompt the REPL does not run the event loop (#30559). A server that a prompt line starts answers only while a line awaits a promise, or from a Worker. The stdin cases await each request, so they use the first path. This PR does not change #30559.

Also checked by hand on the debug build. bun repl -p, a Bun.serve inside a Worker started from bun repl -e, and a route added with server.reload() in the REPL. All three ignore the table on 1.4.3 and honor it with the fix. They read the same context.

Measurements. Release builds (ThinLTO) of ea6a2c7b7c, three commits before the base of this branch, with and without the one line. Counts come from gdb breakpoints and single-steps on bun-profile, because valgrind, perf, strace and bloaty are not installed in the build container. Each allocator total was identical across 3 runs.

bun repl -e 1 without the fix with the fix
allocator calls, empty directory 4274 4277
allocator calls, table above 4295 4303
TransformOptions::clone calls 1 2
instructions from Arena::new() to the call of VirtualMachine::init, empty directory 190 825
the same, table above 190 1174
  • The added clone asks for 38 B in 3 allocations (56 B usable) in an empty directory, and 109 B in 8 allocations (136 B usable) with the table above. The copy lives as long as the process.
  • A whole bun repl -e 1 runs about 4.07 million user instructions (perf_event_open, 21 interleaved pairs). The count varies by about 30,000 between identical runs, so it cannot resolve the change. That is why the hand-off was counted by single-step.
  • bun -e 1: 4436 allocator calls and 2 TransformOptions::clone calls, before and after.
  • size: text 107,094,878 B to 107,094,622 B. data and bss do not change. Of 139,012 symbols only ReplCommand::exec changes size (5686 B to 5366 B). RunCommand::boot_main, create_context_data, init_runtime_state and Route::on_plugins_resolved disassemble to the same instructions.

Not changed here.

  • 13 other moves out of the context at a VM boot: runtime_options.debugger, preloads, passthrough and eval.interactive_script in repl_command.rs, run_command.rs and test_command.rs, and args.serve_plugins and args.bunfig_path in standalone executables (run_command.rs:1292). Nothing reads those through the global context after the boot. To clone them would add allocations to every bun run for no change in behavior.
  • on_plugins_resolved still reads from two places. HTMLBundle.rs is not touched, so Apply CLI and bunfig define to HTML bundles served by Bun.serve #40485 does not need a rebase, and its new read of the context works in the REPL.
  • cli: remove unsafe from bun run / bun test / --filter and the other CLI commands #40242 rewrites the lines around repl_command.rs:80 and still has the mem::take. A rebase of either PR must keep the clone. The new test fails if it does not. repl: run --preload / -r / --require / --import before the first evaluation #46160 edits the same function below that line and does not conflict.
  • A separate defect in the same options literal: the REPL passes smol without mini_mode (repl_command.rs:85, compare run_command.rs:1044), so --smol does not select the small JSC heap in bun repl. A loop that makes garbage peaks at about 41 MB under bun --smol -e and at about 50 MB under bun repl --smol -e, the same as without the flag. test_command.rs:1961 omits mini_mode too. It needs its own fix and test.

… of the CLI context

`bun repl` moved `ctx.args` out of the process-global CLI context when it
booted its VM. `Bun.serve` HTML routes read `serve_minify_*`,
`serve_define` and `serve_sourcemap` from that context when they build,
so inside the REPL the `[serve.static]` table of bunfig.toml had no
minify, define or sourcemap keys.

The REPL now clones the options, as `bun run`, `bun test` and the bake
production build do.
@robobun

robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:31 PM PT - Oct 20th, 2026

✅ @robobun, your commit 9949e3aac5893674f4e996461bea673afe7c5314 passed in Build #129746! 🎉


🧪   To try this PR locally:

bunx bun-pr 46264

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

bun-46264 --bun

@robobun

robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on 1.4.2, on 1.4.3-canary.1 (367d939) and on a debug build of main (ea6a2c7). Bun 1.3.13 does not have the bug.

cat > bunfig.toml <<'TOML'
[serve.static]
minify = false
sourcemap = "linked"

[serve.static.define]
BUILD_FLAG = '"from-bunfig"'
TOML
echo '<!doctype html><html><body><script type="module" src="./app.ts"></script></body></html>' > index.html
echo 'function describeTheFlag(flag: string) { return flag === undefined ? "none" : "MARKER " + flag; } console.log(describeTheFlag(BUILD_FLAG));' > app.ts
cat > serve.ts <<'TS'
import index from "./index.html";
const server = Bun.serve({ port: 0, development: false, routes: { "/": index } });
const html = await (await fetch(server.url)).text();
const jsPath = html.split('src="')[1].split('"')[0];
console.log(await (await fetch(new URL(jsPath, server.url))).text());
console.log((await fetch(new URL(jsPath + ".map", server.url))).status);
await server.stop(true);
TS
bun serve.ts                      # unminified, "from-bunfig", sourceMappingURL comment, 200
bun repl -e "$(cat serve.ts)"     # function t(e){...}console.log(t(BUILD_FLAG)); 404

With this PR the second command prints the same chunk and status as the first.

CI. Buildkite build #129746 passed: 183 of 183 jobs. test/js/bun/http/bun-serve-html.test.ts ran with 46 pass and 0 fail on the debian x64-asan, windows 2019 x64 and darwin aarch64 lanes. The one red check, claude-find-issues, comes from a workflow that failed on its 12 most recent runs that day and is now disabled. It does not build or test this PR, and it is not a required check.

@coderabbitai

coderabbitai Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: Advanced
  • Run ID: e063681b-1c53-45de-bede-a8318673d911
📥 Commits

Reviewing files that changed from the base of the PR and between 9b4284f and 9949e3a.

📒 Files selected for processing: 2
  • src/runtime/cli/repl_command.rs
  • test/js/bun/http/bun-serve-html.test.ts

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

Walkthrough

REPL VM initialization now clones command arguments before passing them to transform_options. Tests cover static-serving minify, define, and sourcemap settings in Bun serve and REPL modes.

Changes

REPL argument preservation and static-serving configuration

Layer / File(s) Summary
Preserve REPL arguments and test static-serving settings
src/runtime/cli/repl_command.rs, test/js/bun/http/bun-serve-html.test.ts
REPL initialization clones ctx.args for transform_options. Tests cover static-serving minify, define, and sourcemap settings across Bun serve and both REPL modes.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 9949e

The change is ready to merge after normal checks; the new tests cover the reported REPL configuration regression.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly summarizes the main change: bun repl now clones parsed options instead of moving them from the CLI context.
Description check Passed The description explains the problem, the fix, and how the author verified it. It does not use the template’s exact headings, but it provides the required information.
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.
✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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.

LGTM — a one-line change that brings bun repl in line with how bun run and bun test already hand ctx.args to the VM, with a test matrix that covers the REPL entry points alongside the bun <file> reference. Beyond the inline note, I checked that no other core::mem::take(&mut ctx.args) site remains in src/ and that HTMLBundle.rs:424-476 is the consumer reading Command::get().args after boot, so the comment is accurate.

Extended reasoning...

The diff replaces core::mem::take(&mut ctx.args) with ctx.args.clone() in ReplCommand::boot_repl_vm (src/runtime/cli/repl_command.rs:81) plus a one-line why-comment, and adds a serial describe block to test/js/bun/http/bun-serve-html.test.ts with six cases (two bunfig modes times three entry points) that assert exact minify/define/sourcemap outcomes and drain stdout/stderr concurrently. It touches no security-sensitive surface. The pattern already exists at run_command.rs:1040 and test_command.rs:1957, no mem::take(&mut ctx.args) sibling remains, the consumer at HTMLBundle.rs:424 is real, and no CODEOWNERS entry covers the changed files. The single inline finding is a pre-existing --smol/mini_mode omission the author explicitly scoped out, not something this PR introduces.

Comment thread src/runtime/cli/repl_command.rs

This branch has not been deployed

No deployments
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.

1 participant