Conversation
For `bun run <script>` and `bun <script>`, bunfig.toml is loaded after argument parsing (RunCommand::exec), so the [run] section overwrote values that --silent, --bun, --shell, and --elide-lines had already set, inverting the documented precedence. Record which of those flags were passed and have the bunfig parser skip the corresponding [run] keys.
|
Found 4 issues this PR may fix:
🤖 Generated with Claude Code |
|
None of these four are closed by this PR, so I'm not adding the
What this PR fixes (explicit |
|
Warning Review limit reached
Next review available in: 11 minutes 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 (1)
WalkthroughAdds "*_from_cli" boolean tracking fields to BundlerOptions and DebugOptions context structs, sets them during CLI argument parsing for --elide-lines, --silent, --bun, and --shell flags, and updates bunfig.toml [run] parsing to skip overwriting values already set via CLI. Adds tests validating this precedence. ChangesCLI flag precedence over bunfig run settings
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Arguments
participant Context as DebugOptions/BundlerOptions
participant BunfigParser
CLI->>Arguments: parse --silent/--bun/--shell/--elide-lines
Arguments->>Context: set option value + *_from_cli = true
Context->>BunfigParser: pass ctx with from_cli flags
BunfigParser->>BunfigParser: read [run] keys from bunfig.toml
alt from_cli flag is true
BunfigParser->>BunfigParser: skip overwrite, keep CLI value
else from_cli flag is false
BunfigParser->>Context: set value from bunfig
end
Estimated code review effort: 2 🔵🔵⚪⚪⚪ Related PRs: None identified. Suggested labels: bug, cli, bunfig Suggested reviewers: None identified. 🐰 A flag from the shell now outranks the file, 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/cli/install/bun-run-bunfig.test.ts`:
- Around line 255-328: Add a precedence test for `--elide-lines` in the
`describe.concurrent.each` block that already covers `--bun`, `--silent`, and
`--shell=bun`, using the same `bunExe()`/`Bun.spawn` pattern and a `bunfig.toml`
with `run.elide-lines`. Verify that the CLI flag overrides the bunfig setting by
asserting the expected output behavior from the relevant script command and that
the process exits successfully. Use the existing `runArg` setup and mirror the
style of the neighboring tests so the new case is easy to locate and maintain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e61b6b6d-23e7-4453-9cb5-630c8b21009f
📒 Files selected for processing (4)
src/bunfig/bunfig.rssrc/options_types/context.rssrc/runtime/cli/Arguments.rstest/cli/install/bun-run-bunfig.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward precedence fix following the existing path_ignore_patterns_from_cli pattern.
Extended reasoning...
Overview
This PR fixes a precedence bug where [run] keys in bunfig.toml (silent, bun, shell, elide-lines) would override their explicit CLI flag counterparts, because bun run loads bunfig.toml after CLI args are parsed. The fix adds four *_from_cli bool fields to DebugOptions/BundlerOptions, sets them in Arguments.rs when the flag is passed, and gates the bunfig assignment on them in bunfig.rs. Six new subprocess tests cover --bun, --silent, and --shell for both bun run and bun invocations.
Security risks
None. This only touches CLI-flag / config-file precedence for local run options; no auth, network, or untrusted-input parsing is involved.
Level of scrutiny
Low-to-moderate. The change follows the exact pattern already established in the same file for test_options.path_ignore_patterns_from_cli, so there's no new design surface. The shell block refactor is behavior-preserving: invalid values still error regardless of the _from_cli flag, and valid values map to the same use_system_shell state as before. Both --elide-lines handlers in Arguments.rs set the new flag. Struct Default impls are updated consistently.
Other factors
The one inline nit (stdout piped but not drained in the shell test) has no functional impact — the failing command writes nothing to stdout, so it can't deadlock — it's just a consistency cleanup with the sibling tests. --elide-lines precedence isn't covered by a new test, but the code path is identical to the three tested flags. CLI > config-file is the conventional precedence, so no design question here.
There was a problem hiding this comment.
LGTM — the stdout nit is addressed and the fix follows the existing path_ignore_patterns_from_cli pattern.
Extended reasoning...
Overview
Restores the documented CLI > bunfig precedence for four [run] keys (silent, bun, shell, elide-lines) by adding *_from_cli tracking booleans to DebugOptions/BundlerOptions, setting them in Arguments::parse, and guarding the corresponding assignments in the bunfig [run] parser. Six new tests in bun-run-bunfig.test.ts cover --bun/--silent/--shell for both bun run <script> and bun <script>.
Security risks
None. This is CLI option precedence plumbing — no auth, crypto, network, path validation, or untrusted input parsing is touched. Type validation in the bunfig parser is preserved (invalid values still error even when the CLI flag was set).
Level of scrutiny
Low. The change is mechanical and copies the exact pattern already used for test.path_ignore_patterns_from_cli in the same three files. New struct fields default to false, so behavior is unchanged when flags are absent. The shell branch refactor (value == b"bun" || value == b"system" → use_system_shell = value == b"system") is semantically identical to the two-branch original and still rejects unknown values. No CODEOWNER-protected paths are touched.
Other factors
- My previous inline comment (undrained
stdout: "pipe"in the--shelltest) was addressed in 4a37be2 by switching tostdout: "ignore"; the thread is resolved. - CodeRabbit's nitpick about a missing
--elide-linestest is already answered in the PR description: the only consumer is the--filterrunner, which re-applies the CLI value and is not observable without a PTY. That's a reasonable justification for a consistency-only guard. - The bug hunting system found no issues on the current revision.
- Tests follow harness conventions (
bunEnv,bunExe,tempDirWithFiles, concurrent pipe draining,await using), and the pre-existing 28 tests in the file — including the ones that assert bunfig[run]keys still apply without flags — remain in place.
|
CI status for build 67636 (head 4a37be2): every failing lane is a test unrelated to this change, and the same tests are failing on other PRs' concurrent builds (67634, 67626, 67611, 67605):
None of these exercise the CLI or bunfig code paths touched here, and |
…quential (#40621) Fixes #17918 Fixes #31479 Supersedes #31480 ### Problem - `bun run --filter <pkg> <script>`, `bun --filter`, `--workspaces`, `--parallel` and `--sequential` ignore an auto-discovered `bunfig.toml`. `[run] bun = true`, `elide-lines`, `shell`, `silent` and `noOrphans` are dead for these runners unless `--config=` is passed. Copying the file into each package directory does not help: the parent process decides these settings. - Cause: `exec_auto_or_run` (`src/runtime/cli/mod.rs:1430`) dispatches to `multi_run::run` and `filter_run::run_scripts_with_filter` right after argument parsing. The lazy `bunfig.toml` load for `bun run <script>` lives in `RunCommand::exec_with_cfg` (`src/runtime/cli/run_command.rs:2312`), which these paths never reach. `load_config` (`src/bunfig/arguments.rs:169`) only auto-loads for `bun test`, `bun <file>` and the install commands. ### Fix - Extend the auto-load condition in `load_config` with: run or auto command, and one of `parallel`, `sequential`, `workspaces`, or a non-empty `filters`. These fields are set before `load_config_with_cmd_args` runs. - The file now loads inside `Arguments::parse`, the same place `bun test` and `bun <file>` load it. `--bun`, `--elide-lines`, `--shell` and `--silent` are applied after that call, so a CLI flag still wins over its `[run]` key. - A malformed `bunfig.toml` now fails these runs with the parser error and exit code 1, the same as `bun test` and `bun run -c=bunfig.toml`. - Verified: `test/cli/run/filter-workspace.test.ts` (7 new tests, 5 fail on 1.4.1) and `test/cli/run/multi-run.test.ts` (5 new, 4 fail on 1.4.1). Also `test/cli/install/bun-run-bunfig.test.ts`, `test/config/bunfig/`, `test/cli/run/env.test.ts`, `no-envfile.test.ts`, `run-shell.test.ts`. ### Background - `bunfig.toml` is read from the working directory. `Arguments::parse` loads it for commands in `ALWAYS_LOADS_CONFIG`. `bun run <script>` is not in that table and loads it later, in `exec_with_cfg`. - `[run] bun = true` puts a `node` shim that points at bun on `PATH`. The tests use `node -e "console.log(typeof Bun)"` to see which binary ran. - Elision (`elide-lines`) only happens when stdout is a terminal. On POSIX, `FORCE_COLOR=1` turns the terminal renderer on for a pipe, which the existing elision tests rely on. <details><summary>Notes</summary> - Repro on 1.4.1: workspace root `bunfig.toml` with `[run]\nbun = true`, package script `node -e "console.log('Bun is', typeof Bun)"`. `bun run --filter a hello` prints `Bun is undefined`. `bun run -c=bunfig.toml --filter a hello` prints `Bun is object`. `bun run --parallel hello hello2` and `--sequential` print `Bun is undefined` too. Plain `bun run hello` prints `Bun is object`. - Two precedence tests (`--bun` over `bun = false`, `--elide-lines 15` over `elide-lines = 0`) pass before and after. They guard against the inverted precedence that the exec-time load has for plain `bun run` (#33198). - Top-level keys in the same file (`env = false`, `preload`, `define`) are parsed for the parent too. Only `env` changes what the parent does: it stops loading the root `.env` files that the child scripts otherwise inherit, which is what the key documents. - #31480 added the same load inside `run_scripts_with_filter` with a snapshot of the two CLI values. It covered `--filter` only and predates the `tempDirWithFiles` removal. Loading during argument parsing needs no snapshot and covers `--parallel` and `--sequential`. - Debug builds recreate `/tmp/bun-node-debug` on every `--bun` run (`src/install/lib.rs:564`), so concurrent `--bun` processes can race each other. The multi-run tests are serial because of that. Reported separately. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/run/multi-run.test.ts, test/cli/run/filter-workspace.test.ts <!-- robobun:evidence:end -->
|
Stale PR review: keep open, rework. The bug is still on main and the fix is wanted. Take an auto-discovered bunfig.toml with #40621 merged tests that assert this order for The current diff is not ready to merge, although GitHub reports no conflict:
The wanted shape is the same mechanism in one rebase:
|
This branch has not been deployed
1 participant
Problem
The
[run]section in bunfig.toml silently overrides explicit CLI flags forbun run <script>andbun <script>, inverting the documented precedence ("CLI flags overridebunfigsettings where applicable").bun run --bun where-node # runs the real node: the --bun flag is ignoredReproduced on 1.4.0 and current main with
--bunvsrun.bun,--silentvsrun.silent, and--shellvsrun.shell:Cause
Arguments::parseapplies--silent/--bun/--shell/--elide-linesto the context afterload_config, which is correct when bunfig.toml is loaded during argument parsing (-c=...,bun file.ts,bun test, ...). But forbun run <script>andbun <script-name>the local bunfig.toml is only loaded later, insideRunCommand::exec(#16664), and its[run]keys overwrote the values the CLI flags had already set. That is whybun run -c=bunfig.toml --bun xbehaved correctly whilebun run --bun xdid not.Fix
Record which of the four
[run]-related CLI flags were explicitly passed (*_from_cli, the same patterntest.pathIgnorePatternsalready uses) and have the bunfig[run]parser skip a key whose flag was passed. This keeps the deferred bunfig load forbun run(so[run]settings still apply when no flag is given) while restoring CLI > bunfig precedence in both load orders.run.elide-linesgets the same guard for consistency; its only consumer is the--filterrunner, which re-applies the CLI value today, so it is not observable in a test without a PTY.Verification
New tests in
test/cli/install/bun-run-bunfig.test.ts, for bothbun run <script>andbun <script>:--bunoverridesrun.bun = false(node resolves to thebun-node-*shim)--silentoverridesrun.silent = false(no$ echo 1echo)--shell=bunoverridesrun.shell = "system"(bun shell error message)All 6 fail on the unfixed build and pass with this change; the 28 pre-existing tests in the file (including
run.bun/run.silent/run.shellwithout flags, and the bunfig autoload tests) still pass, as dotest/cli/run/filter-workspace.test.ts,test/cli/run/run_command.test.ts, andtest/cli/bunfig-test-options.test.ts.