Conversation
`bun -e ""`, `bun --print ""` (and the `--eval=` / `--print=` / `-pe ""` spellings) printed the CLI help text, `node -e ""` printed "Missing script", `bun repl -e ""` started the interactive REPL, and `bun -` with empty stdin failed with "Module not found '<cwd>/[stdin]'". node runs an empty program in all of these cases and exits 0 (`-p` prints undefined). The script was stored as a plain byte slice, so an explicitly empty script was indistinguishable from no script, and every dispatch site tested `is_empty()`. Command::start additionally short-circuited the exact `bun -e ""` argv shapes straight to the help text. Make `Eval::script` an `Option<Box<[u8]>>`: `None` means no script was given, `Some` means run it even when empty. Dispatch sites test `is_some()`, the help shortcut is removed, and `process._eval` reports `undefined` for an empty script, as node does. The child_process double-close test spawns 20 `bun -e ""` children that now boot the runtime; give it an explicit timeout for debug builds.
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
Comment |
|
Status: ready for review. Reproduced on the released binary (1.4.0-canary.1+b7a043103, linux-x64): The new tests in The rate-limit notice above needs no action; the review can be re-requested once the limit resets. |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. It's a clean, well-tested Node-compat fix, but since it touches CLI dispatch / startup and the author notes overlap with #32622 / #38556 / #38566, a maintainer should confirm the landing order.
What was reviewed:
- Grepped every
eval.script/eval_and_printconsumer — all sites updated;eval_and_print = trueis only ever set alongsidescript = Some(...), so dropping the|| eval_and_printdisjunct inReplRunneris safe. - Checked
from_raw_partson the emptyBox<[u8]>inboot— dangling-but-aligned pointer with len 0 is valid. - The
process._evalempty-check preserves the existing non-empty stdin_evaltest in run-eval.test.ts. - The removed fast-path in
Command::startexisted only to reproduce the help-text behavior;--version/bun <path>shortcuts remain.
Extended reasoning...
Overview
This PR changes Eval::script from Box<[u8]> to Option<Box<[u8]>> so that an explicitly empty -e/-p/stdin script is distinguishable from no script at all. It updates the three writers (Arguments.rs --print/--eval/--port, exec_stdin, exec_node_repl) to store Some, and the six readers (cli/mod.rs AutoCommand dispatch, run_command.rs boot/add_conditional_globals/exec_as_if_node, repl_command.rs ReplRunner) to test is_some(). The empty-eval fast-path in Command::start is deleted, and process._eval returns undefined for an empty source. Tests: 13 new cases in run-eval.test.ts, 3 in as-node.test.ts, 2 in repl.test.ts, plus a 30s timeout on one child_process test whose 20 children now boot the runtime.
Security risks
None. The change only affects how CLI argv is classified before dispatch; no untrusted-input parsing, no auth/crypto/permissions.
Level of scrutiny
Medium-high. The type change itself is mechanical and compiler-enforced, but it touches Command::start (the startup-critical dispatch root) and changes the semantics of bun -e "", which is used pervasively across the test suite as a trivial-child idiom. The PR author audited the affected suites (spawn, child_process, terminal, serve/fetch memory tests, emptyProcessMaxRSS()), but a maintainer should confirm the CI-wide impact and the emptyProcessMaxRSS() baseline shift doesn't destabilize leak tests on other platforms.
Other factors
- I verified via grep that every
eval.scriptsite is updated and thateval_and_printis never set withoutscript, so the removed|| eval_and_printdisjunct inReplRunner::startcannot change behavior. - The
repl_command.rsrefactor replaces a raw-pointer reborrow withtake()+Box::leak, which is strictly safer (drops anunsafeblock) and correct becausehold_api_locknever returns. - The PR explicitly notes overlap with #32622 (which fixes the same
bun -e ""case via a separate flag) and one-line conflicts with #38556 / #38566. A maintainer should decide landing order rather than an automated approval.
|
On landing order, for whoever merges: this PR does not depend on #38556, #38566 or #32622 and they do not depend on it.
|
|
#39687 rewrites the |
Problem
bun -e ""andbun --print ""(also--eval=,--print=,-e=,-p=,-pe "") print the 37-line CLI help text to stdout and exit 0.node -e ""runs an empty program and exits 0 silently;node -p ""printsundefined.node -e ""(bun running asnode) fails witherror: Missing script to execute,bun repl -e ""starts the interactive REPL, andbun -/bun run -with empty stdin fail witherror: Module not found '<cwd>/[stdin]'.Eval::script(src/options_types/context.rs) is a plainBox<[u8]>, so an explicitly empty script is indistinguishable from no script, and every dispatch site testsis_empty(): cli/mod.rs (AutoCommand dispatch), run_command.rs (boot,add_conditional_globals,exec_as_if_node) and repl_command.rs. On top of that,Command::startin cli/mod.rs short-circuited the exactbun -e ""argv shapes straight toHelpCommand::exec().len > 0gate dates back to when-ewas added; the Rust port preserved it and added the help shortcut.Fix
Eval::scriptbecomesOption<Box<[u8]>>:Nonemeans no script was given,Somemeans run it, empty or not. Argument parsing, the stdin reader and the--interactivebootstrap storeSome; the dispatch sites testis_some(); the exact-shape help shortcut inCommand::startis deleted (the--version/bun <path>shortcuts stay).ReplRunnertakesOption<&[u8]>and runs the script whenever one was given, sobun repl -e ""evaluates and exits likebun repl -p ""already did. The script istake()n out of the process-global context and leaked instead of reborrowed through a raw pointer, which drops anunsafeblock.process._evalreturnsundefinedwhen the eval source is empty (node_process.rs). Node only definesprocess._evalfor a truthy value, and the--interactivebranch right above already did this.-e ""silent,-p ""/-pe ""/-p "" xprintundefined, empty stdin silent,process._evalundefined under-e "",-i -e ""enters the REPL withprocess._evalundefined). Absent vs. empty is the distinction node itself tracks (has_eval_string), and theOptionputs that distinction in the type instead of in a convention about emptiness. An empty eval source goes through the sameParseResult::empty_withpath as an empty file, and--printfalls back toundefinedwhen the module produced no completion value, so nothing downstream needed to learn about empty scripts.bunstill prints help (positionalsempty, no script);bun -e ""with no positionals now boots the runtime likebun -e ";"does.process._eval, empty stdin forbun -andbun run -); all 13 fail on the released binary, 50/50 pass with this build.node -e ""/-p ""/-pe ""(3 fail before, 14/14 pass after).bun repl -e ""exits without the REPL (fails before),--interactive -e ""keeps entering the REPL withprocess._evalundefined (unchanged behavior, pinned); 150/150 pass.bun -e ""as a trivial child still pass: spawn, spawn-signal, spawn-stdin-pipe-fd-leak, spawn-stdin-readable-stream, child_process, child-process-stdio, terminal, terminal-platform-gaps, cli/bun, run_command, and thebounds memorytests in serve/fetch.emptyProcessMaxRSS()in test/harness.ts spawnsbun -e ""as the "empty bun process" baseline. It now measures a booted runtime (debug/ASAN: ~318 MiB instead of ~216 MiB for the help text; release: 27.4 vs 26.4 MiB), which is what the helper intends; every consumer assertsfixture - baseline < N, so their deltas only get smaller. Eachbun -e ""child also costs a runtime boot now (~6 ms release, ~265 ms instead of ~100 ms debug/ASAN); the one test that spawns 20 of them sequentially with full GCs between (extra stdio pipes are not double-closed on GC, child_process.test.ts) takes ~11.5 s on a debug build here, so it gets an explicit 30 s timeout; prettier re-indents that test body because of the added argument.bun -e ""case among many other things with a separateprovidedflag, but not the stdin orbun replcases; this PR is the small standalone version and would let that PR drop its flag on rebase. node emulation: keep the first positional in process.argv for node -e / -p #38556 and cli: read the script from stdin fornode -in node emulation #38566 touch neighboring lines ofexec_as_if_node/exec_stdin; whichever lands second has a one-line conflict to resolve.Background
-e/-p(andbun -for stdin) do not run a file.RunCommand::bootstores the script bytes asvm.module_loader.eval_source, aSourcekeyed at the synthetic path<cwd>/[eval](or[stdin]), and the module loader serves that source instead of reading the disk when the entry specifier ends in/[eval]or/[stdin]. Wheneval_sourceis not set, as happened for the empty-stdin case, loading the synthetic path fails with "Module not found".--interactive:exec_node_replswaps the user's-ebytes intoEval::interactive_scriptand puts the node:repl bootstrap inEval::script, soprocess._evalreads the user's bytes frominteractive_script(alreadyundefinedwhen empty); the non-interactive branch ofprocess._evalnow does the same.Command::startfast path: a few exact argv shapes (bun --version,bun <path>) are dispatched before the subcommand classifier runs, purely to keep startup small; the empty-eval shapes were in that list only to reproduce the help-text behavior, so removing them changes nothing else.Repro on the released binary vs node v26.3.0
With this branch, every bun invocation above matches the node output (
-pvariants printundefined, the rest exit 0 silently), andbun -r ./preload.cjs -e "" a bprintspreload _eval: undefined argv: ["a","b"].