Skip to content

Conversation

@Jarred-Sumner
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner commented Jan 29, 2026

Summary

Adds bun run --parallel and bun run --sequential — new flags for running multiple package.json scripts concurrently or sequentially with Foreman-style prefixed output. Includes full --filter/--workspaces integration for running scripts across workspace packages.

Usage

# Run "build" and "test" concurrently from the current package.json
bun run --parallel build test

# Run "build" and "test" sequentially with prefixed output
bun run --sequential build test

# Glob-matched script names
bun run --parallel "build:*"

# Run "build" in all workspace packages concurrently
bun run --parallel --filter '*' build

# Run "build" in all workspace packages sequentially
bun run --sequential --workspaces build

# Glob-matched scripts across all packages
bun run --parallel --filter '*' "build:*"

# Multiple scripts across all packages
bun run --parallel --filter '*' build lint test

# Continue running even if one package fails
bun run --parallel --no-exit-on-error --filter '*' test

# Skip packages missing the script
bun run --parallel --workspaces --if-present build

How it works

Output format

Each script's stdout/stderr is prefixed with a colored, padded label:

build | compiling...
test  | running suite...
lint  | checking files...

Label format

  • Without --filter/--workspaces: labels are just the script name → build | output
  • With --filter/--workspaces: labels are package:scriptpkg-a:build | output
  • Fallback: if a package.json has no name field, the relative path from the workspace root is used (e.g., packages/my-pkg:build)

Execution model

  • --parallel: all scripts start immediately, output is interleaved with prefixes
  • --sequential: scripts run one at a time in order, each waiting for the previous to finish
  • Pre/post scripts (prebuild/postbuild) are grouped with their main script and run in dependency order within each group
  • By default, a failure kills all remaining scripts. --no-exit-on-error lets all scripts finish.

Workspace integration

The workspace branch in multi_run.zig uses a two-pass approach for deterministic ordering:

  1. Collect: iterate workspace packages using FilterArg.PackageFilterIterator (same infrastructure as filter_run.zig), filtering with FilterArg.FilterSet, collecting matched packages with their scripts, PATH, and cwd.
  2. Sort: sort matched packages by name (tiebreak by directory path) for deterministic ordering — filesystem iteration order from the glob walker is nondeterministic.
  3. Build configs: for each sorted package, expand script names (including globs like build:*) against that package's scripts map, creating ScriptConfig entries with pkg:script labels and per-package cwd/PATH.

Behavioral consistency with filter_run.zig

Behavior filter_run.zig multi_run.zig (this PR)
--workspaces skips root package Yes Yes
--workspaces errors on missing script Yes Yes
--if-present silently skips missing Yes Yes
--filter without --workspaces includes root Yes (if matches) Yes (if matches)
Pre/post script chains Per-package Per-package
Per-package cwd Yes Yes
Per-package PATH (node_modules/.bin) Yes Yes

Key implementation details

  • Each workspace package script runs in its own package directory with its own node_modules/.bin PATH
  • dirpath from the glob walker is duped to avoid use-after-free when the iterator's arena is freed between patterns
  • addScriptConfigs takes an optional label_prefix parameter — null for single-package mode, package name for workspace mode
  • MultiRunProcessHandle is registered in the ProcessExitHandler tagged pointer union in process.zig

Files changed

File Change
src/cli/multi_run.zig New file: process management, output routing, workspace integration, dependency ordering
src/cli.zig Dispatch to MultiRun.run() for --parallel/--sequential, new context fields
src/cli/Arguments.zig Parse --parallel, --sequential, --no-exit-on-error flags
src/bun.js/api/bun/process.zig Register MultiRunProcessHandle in ProcessExitHandler tagged pointer union
test/cli/run/multi-run.test.ts 118 tests (102 core + 16 workspace integration)
docs/pm/filter.mdx Document --parallel/--sequential + --filter/--workspaces combination
docs/snippets/cli/run.mdx Add --parallel, --sequential, --no-exit-on-error parameter docs

Test plan

All 118 tests pass with debug build (bun bd test test/cli/run/multi-run.test.ts). The 16 new workspace tests all fail with system bun (USE_SYSTEM_BUN=1), confirming they test new functionality.

Workspace integration tests (16 tests)

  1. --parallel --filter='*' runs script in all packages
  2. --parallel --filter='pkg-a' runs only in matching package
  3. --parallel --workspaces matches all workspace packages
  4. --parallel --filter='*' with glob expands per-package scripts
  5. --sequential --filter='*' runs in sequence (deterministic order)
  6. Workspace + failure aborts other scripts
  7. Workspace + --no-exit-on-error lets all finish
  8. --workspaces skips root package
  9. Each workspace script runs in its own package directory (cwd verification)
  10. Multiple script names across workspaces (build + test)
  11. Pre/post scripts work per workspace package
  12. --filter skips packages without the script (no error)
  13. --workspaces errors when a package is missing the script
  14. --workspaces --if-present skips missing scripts silently
  15. Labels are padded correctly across workspace packages
  16. Package without name field uses relative path as label

@Jarred-Sumner Jarred-Sumner changed the title feat(cli/run): add --parallel and --sequential multi-script execution feat(cli/run): add --filter/--workspaces support to --parallel/--sequential Jan 29, 2026
@robobun
Copy link
Collaborator

robobun commented Jan 29, 2026

Updated 3:49 AM PT - Jan 29th, 2026

@autofix-ci[bot], your commit 0abcd15 has 4 failures in Build #36098 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 26551

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

bun-26551 --bun

@Jarred-Sumner Jarred-Sumner force-pushed the claude/multi-run-filter-workspaces branch 2 times, most recently from cc1e723 to aec8b59 Compare January 29, 2026 10:51
@Jarred-Sumner Jarred-Sumner requested a review from alii as a code owner January 29, 2026 10:51
@Jarred-Sumner Jarred-Sumner changed the base branch from jarred/auto-accessor to main January 29, 2026 10:51
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 29, 2026

Walkthrough

Adds a new multi-script runner and CLI flags to run multiple scripts in parallel or sequentially, integrates it with workspace filtering and package script discovery, updates process exit handling, adds extensive tests, and expands documentation for the new options.

Changes

Cohort / File(s) Summary
Documentation
docs/pm/filter.mdx, docs/snippets/cli/run.mdx
Adds "Parallel and sequential mode" docs and documents new CLI flags --parallel, --sequential, and --no-exit-on-error, including examples, prefixing behavior, and --if-present usage.
Multi-run implementation
src/cli/multi_run.zig
New comprehensive multi-script runner: ScriptConfig, PipeReader, ProcessHandle, State, glob matching, workspace-aware discovery/filtering, parallel/sequential orchestration, prefixed/colorized output, dependency handling, signal abort handling, and public run entrypoint.
CLI wiring
src/cli.zig, src/cli/Arguments.zig
Introduces parallel, sequential, and no_exit_on_error ContextData fields, imports MultiRun, routes run/auto/run-command paths to MultiRun.run when flags set, and registers/reads new CLI flags.
Process exit handling
src/bun.js/api/bun/process.zig
Extends ProcessExitHandler.TaggedPointer to include MultiRunProcessHandle and dispatches onProcessExit to the new handle type.
Tests
test/cli/run/multi-run.test.ts
Adds a large test suite validating parallel/sequential behavior, output formatting/prefixing, error propagation and --no-exit-on-error, glob/workspace interactions, pre/post scripts, raw commands, and many edge cases.

Possibly related PRs

Suggested reviewers

  • nektro
  • dylan-conway
  • taylordotfish
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately summarizes the main feature: adding --parallel and --sequential flags for running multiple scripts with workspace support.
Description check ✅ Passed The description is comprehensive and well-structured, covering usage examples, implementation details, behavioral consistency, and test coverage.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

…er`/`--workspaces` support

Add `bun run --parallel` and `bun run --sequential` for running multiple
scripts with Foreman-style prefixed output, including workspace support via
`--filter` and `--workspaces`.

Features:
- Run multiple scripts concurrently (`--parallel`) or sequentially (`--sequential`)
- Run scripts across workspace packages with `--filter`/`--workspaces`
- Glob-matched script names (e.g. `build:*`)
- Multiple script arguments (e.g. `build lint test`)
- Pre/post script chaining per package
- Per-package cwd and PATH (`node_modules/.bin`)
- `--no-exit-on-error` to continue after failures
- `--if-present` to skip packages missing the script
- Colored, padded label prefixes (`pkg-a:build | ...`)
- Deterministic package ordering (sorted by name, tiebreak by path)
- Fallback to relative path when package.json has no name field

New files:
- src/cli/multi_run.zig: process management, output routing, dependency ordering
- test/cli/run/multi-run.test.ts: 118 tests (102 core + 16 workspace integration)
@Jarred-Sumner Jarred-Sumner force-pushed the claude/multi-run-filter-workspaces branch from 3c13c08 to a21ce1a Compare January 29, 2026 11:01
@Jarred-Sumner Jarred-Sumner changed the title feat(cli/run): add --filter/--workspaces support to --parallel/--sequential feat(cli/run): add --parallel and --sequential for running multiple scripts with workspace support Jan 29, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Fix all issues with AI agents
In `@src/cli.zig`:
- Around line 894-899: The branch that handles concurrent execution (check of
ctx.parallel or ctx.sequential calling MultiRun.run) must short-circuit the
command flow: after calling MultiRun.run (both success and the catch/error path
where you call Output.prettyErrorln and Global.exit), ensure you return from the
enclosing function so execution does not fall through into FilterRun, RunCommand
or the auto-fallback and re-run scripts; update the block around MultiRun.run to
perform an early return on success and keep the existing error handling, and
apply the same early-return fix to the other identical branch that also handles
parallel/sequential invocation so both places stop further processing after
MultiRun.run.

In `@src/cli/Arguments.zig`:
- Around line 459-461: Add validation after flags are parsed in Arguments.zig to
enforce that ctx.parallel and ctx.sequential are mutually exclusive and that
ctx.no_exit_on_error is only allowed when one of those modes is set;
specifically, check if ctx.parallel && ctx.sequential and return/emit a
user-facing error indicating the conflict (referencing ctx.parallel and
ctx.sequential), and check if ctx.no_exit_on_error && !(ctx.parallel ||
ctx.sequential) and return/emit an error requiring a mode when
--no-exit-on-error is used (referencing ctx.no_exit_on_error and the mode
flags). Ensure these checks run where args.flags are handled (the same scope
that sets ctx.parallel/ctx.sequential/ctx.no_exit_on_error) and produce a clear
exit or error code consistent with the surrounding argument-parsing error
handling.

In `@src/cli/multi_run.zig`:
- Around line 351-366: Change the AbortHandler.should_abort from a plain bool to
an atomic boolean (e.g., std.atomic.AtomicBool) and update its initializer; in
posixSignalHandler and windowsCtrlHandler perform an atomic store (set true)
using an appropriate memory ordering; then update the event-loop read of
should_abort to use an atomic load with a matching memory ordering (replace
direct reads/writes of AbortHandler.should_abort with atomic load/store calls).
Reference AbortHandler, should_abort, posixSignalHandler, windowsCtrlHandler and
the location where the flag is read in the event loop to ensure all accesses use
atomic operations.

In `@test/cli/run/multi-run.test.ts`:
- Around line 1393-1404: The test "empty script string runs without crashing"
currently checks only stderr but not the spawned process exit code; update the
test to assert the process exited successfully by checking the runMulti result
(r) for a zero exit code (e.g., assert r.exitCode === 0) after calling
runMulti(["run", "--parallel", "empty"], String(dir)) to ensure failures are
caught; locate the test by its name and the use of runMulti and add the exit
code assertion alongside the existing stderr expectation.
- Around line 1022-1023: Replace the shell call Bun.$`realpath ${String(dir)}`
with Node/Bun fs.realpathSync to avoid invoking a shell: import fs (and path if
needed), compute const realDir = fs.realpathSync(String(dir)) (or
fs.realpathSync(path.join(...)) for composed parts) and pass that value to
expectPrefixed(r.stdout, "pwd", realDir) instead of using the Bun.$ command;
update references to realDir/dir and remove the Bun.$ usage.

Comment on lines +894 to +899
if (ctx.parallel or ctx.sequential) {
MultiRun.run(ctx) catch |err| {
Output.prettyErrorln("<r><red>error<r>: {s}", .{@errorName(err)});
Global.exit(1);
};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Short‑circuit after MultiRun.run to avoid duplicate execution.
Without an early return, the flow continues into FilterRun / RunCommand / auto fallback, which can re-run scripts when --parallel/--sequential is used.

🛠️ Proposed fix
                 if (ctx.parallel or ctx.sequential) {
                     MultiRun.run(ctx) catch |err| {
                         Output.prettyErrorln("<r><red>error<r>: {s}", .{`@errorName`(err)});
                         Global.exit(1);
                     };
+                    return;
                 }
                 if (ctx.parallel or ctx.sequential) {
                     MultiRun.run(ctx) catch |err| {
                         Output.prettyErrorln("<r><red>error<r>: {s}", .{`@errorName`(err)});
                         Global.exit(1);
                     };
+                    return;
                 }

Also applies to: 940-945

🤖 Prompt for AI Agents
In `@src/cli.zig` around lines 894 - 899, The branch that handles concurrent
execution (check of ctx.parallel or ctx.sequential calling MultiRun.run) must
short-circuit the command flow: after calling MultiRun.run (both success and the
catch/error path where you call Output.prettyErrorln and Global.exit), ensure
you return from the enclosing function so execution does not fall through into
FilterRun, RunCommand or the auto-fallback and re-run scripts; update the block
around MultiRun.run to perform an early return on success and keep the existing
error handling, and apply the same early-return fix to the other identical
branch that also handles parallel/sequential invocation so both places stop
further processing after MultiRun.run.

Comment on lines +459 to +461
ctx.parallel = args.flag("--parallel");
ctx.sequential = args.flag("--sequential");
ctx.no_exit_on_error = args.flag("--no-exit-on-error");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add mutual-exclusion validation for --parallel/--sequential (and stray --no-exit-on-error).
Right now both modes can be set at once, and --no-exit-on-error can be used without either mode. That makes the effective behavior ambiguous.

💡 Suggested guard
         ctx.parallel = args.flag("--parallel");
         ctx.sequential = args.flag("--sequential");
         ctx.no_exit_on_error = args.flag("--no-exit-on-error");

+        if (ctx.parallel and ctx.sequential) {
+            Output.prettyErrorln("<r><red>error<r>: --parallel and --sequential are mutually exclusive", .{});
+            Global.exit(1);
+        }
+        if (!ctx.parallel and !ctx.sequential and ctx.no_exit_on_error) {
+            Output.prettyErrorln("<r><red>error<r>: --no-exit-on-error requires --parallel or --sequential", .{});
+            Global.exit(1);
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ctx.parallel = args.flag("--parallel");
ctx.sequential = args.flag("--sequential");
ctx.no_exit_on_error = args.flag("--no-exit-on-error");
ctx.parallel = args.flag("--parallel");
ctx.sequential = args.flag("--sequential");
ctx.no_exit_on_error = args.flag("--no-exit-on-error");
if (ctx.parallel and ctx.sequential) {
Output.prettyErrorln("<r><red>error<r>: --parallel and --sequential are mutually exclusive", .{});
Global.exit(1);
}
if (!ctx.parallel and !ctx.sequential and ctx.no_exit_on_error) {
Output.prettyErrorln("<r><red>error<r>: --no-exit-on-error requires --parallel or --sequential", .{});
Global.exit(1);
}
🤖 Prompt for AI Agents
In `@src/cli/Arguments.zig` around lines 459 - 461, Add validation after flags are
parsed in Arguments.zig to enforce that ctx.parallel and ctx.sequential are
mutually exclusive and that ctx.no_exit_on_error is only allowed when one of
those modes is set; specifically, check if ctx.parallel && ctx.sequential and
return/emit a user-facing error indicating the conflict (referencing
ctx.parallel and ctx.sequential), and check if ctx.no_exit_on_error &&
!(ctx.parallel || ctx.sequential) and return/emit an error requiring a mode when
--no-exit-on-error is used (referencing ctx.no_exit_on_error and the mode
flags). Ensure these checks run where args.flags are handled (the same scope
that sets ctx.parallel/ctx.sequential/ctx.no_exit_on_error) and produce a clear
exit or error code consistent with the surrounding argument-parsing error
handling.

Comment on lines +351 to +366
const AbortHandler = struct {
var should_abort = false;

fn posixSignalHandler(sig: i32, info: *const std.posix.siginfo_t, _: ?*const anyopaque) callconv(.c) void {
_ = sig;
_ = info;
should_abort = true;
}

fn windowsCtrlHandler(dwCtrlType: std.os.windows.DWORD) callconv(.winapi) std.os.windows.BOOL {
if (dwCtrlType == std.os.windows.CTRL_C_EVENT) {
should_abort = true;
return std.os.windows.TRUE;
}
return std.os.windows.FALSE;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the AbortHandler struct and related code in multi_run.zig
cat -n src/cli/multi_run.zig | sed -n '340,380p'

Repository: oven-sh/bun

Length of output: 1813


🏁 Script executed:

# Now let's check lines around 808-810 where should_abort is read
cat -n src/cli/multi_run.zig | sed -n '800,820p'

Repository: oven-sh/bun

Length of output: 705


🏁 Script executed:

# Let's search for all occurrences of should_abort and AbortHandler
rg -n "should_abort" src/cli/multi_run.zig

Repository: oven-sh/bun

Length of output: 222


🏁 Script executed:

# Check the complete AbortHandler structure and its usage
rg -n "AbortHandler" src/cli/multi_run.zig -A 5

Repository: oven-sh/bun

Length of output: 915


🏁 Script executed:

# Check if there are other signal handlers or async patterns in the codebase for comparison
rg -n "sigaction|sig_atomic|@atomicStore|@atomicLoad" src/cli/multi_run.zig

Repository: oven-sh/bun

Length of output: 207


🏁 Script executed:

# Look at other files with signal handlers to see if atomics are used
fd "\.zig$" src/ | xargs rg -l "sigaction|Sigaction" | head -5

Repository: oven-sh/bun

Length of output: 167


🏁 Script executed:

# Check one of those files to see the pattern
rg -n "sigaction" src/main.zig -B 3 -A 3 | head -40

Repository: oven-sh/bun

Length of output: 318


🏁 Script executed:

# Check crash_handler.zig for signal handler patterns
rg -n "sigaction|@atomic|sig_atomic" src/crash_handler.zig -B 2 -A 2

Repository: oven-sh/bun

Length of output: 767


🏁 Script executed:

# Check filter_run.zig for signal handler patterns
rg -n "sigaction|@atomic|sig_atomic" src/cli/filter_run.zig -B 2 -A 2

Repository: oven-sh/bun

Length of output: 588


🏁 Script executed:

# Check process.zig for any signal handler patterns with atomics
rg -n "sigaction|@atomic|sig_atomic" src/bun.js/api/bun/process.zig -B 2 -A 2

Repository: oven-sh/bun

Length of output: 506


Make the abort flag atomic to prevent undefined behavior from signal/thread handlers.

should_abort is written from the POSIX signal handler (line 357) and Windows console control handler (line 362), then read in the event loop (line 809). This requires atomic operations to be async-signal-safe and thread-safe.

Suggested fix
 const AbortHandler = struct {
     var should_abort = false;

     fn posixSignalHandler(sig: i32, info: *const std.posix.siginfo_t, _: ?*const anyopaque) callconv(.c) void {
         _ = sig;
         _ = info;
-        should_abort = true;
+        `@atomicStore`(bool, &should_abort, true, .seq_cst);
     }

     fn windowsCtrlHandler(dwCtrlType: std.os.windows.DWORD) callconv(.winapi) std.os.windows.BOOL {
         if (dwCtrlType == std.os.windows.CTRL_C_EVENT) {
-            should_abort = true;
+            `@atomicStore`(bool, &should_abort, true, .seq_cst);
             return std.os.windows.TRUE;
         }
         return std.os.windows.FALSE;
     }

And update the read at line 809:

-        if (AbortHandler.should_abort and !state.aborted) {
+        if (`@atomicLoad`(bool, &AbortHandler.should_abort, .seq_cst) and !state.aborted) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const AbortHandler = struct {
var should_abort = false;
fn posixSignalHandler(sig: i32, info: *const std.posix.siginfo_t, _: ?*const anyopaque) callconv(.c) void {
_ = sig;
_ = info;
should_abort = true;
}
fn windowsCtrlHandler(dwCtrlType: std.os.windows.DWORD) callconv(.winapi) std.os.windows.BOOL {
if (dwCtrlType == std.os.windows.CTRL_C_EVENT) {
should_abort = true;
return std.os.windows.TRUE;
}
return std.os.windows.FALSE;
}
const AbortHandler = struct {
var should_abort = false;
fn posixSignalHandler(sig: i32, info: *const std.posix.siginfo_t, _: ?*const anyopaque) callconv(.c) void {
_ = sig;
_ = info;
`@atomicStore`(bool, &should_abort, true, .seq_cst);
}
fn windowsCtrlHandler(dwCtrlType: std.os.windows.DWORD) callconv(.winapi) std.os.windows.BOOL {
if (dwCtrlType == std.os.windows.CTRL_C_EVENT) {
`@atomicStore`(bool, &should_abort, true, .seq_cst);
return std.os.windows.TRUE;
}
return std.os.windows.FALSE;
}
};
🤖 Prompt for AI Agents
In `@src/cli/multi_run.zig` around lines 351 - 366, Change the
AbortHandler.should_abort from a plain bool to an atomic boolean (e.g.,
std.atomic.AtomicBool) and update its initializer; in posixSignalHandler and
windowsCtrlHandler perform an atomic store (set true) using an appropriate
memory ordering; then update the event-loop read of should_abort to use an
atomic load with a matching memory ordering (replace direct reads/writes of
AbortHandler.should_abort with atomic load/store calls). Reference AbortHandler,
should_abort, posixSignalHandler, windowsCtrlHandler and the location where the
flag is read in the event loop to ensure all accesses use atomic operations.

Comment on lines +1022 to +1023
const realDir = await Bun.$`realpath ${String(dir)}`.text();
expectPrefixed(r.stdout, "pwd", realDir.trim());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and get its line count
wc -l test/cli/run/multi-run.test.ts

Repository: oven-sh/bun

Length of output: 90


🏁 Script executed:

# Now look at the specific lines mentioned in the review
sed -n '1020,1025p' test/cli/run/multi-run.test.ts

Repository: oven-sh/bun

Length of output: 291


🏁 Script executed:

# Check lines 1899-1907 as well
sed -n '1899,1907p' test/cli/run/multi-run.test.ts

Repository: oven-sh/bun

Length of output: 657


🏁 Script executed:

# Check the test file imports and structure
head -30 test/cli/run/multi-run.test.ts

Repository: oven-sh/bun

Length of output: 1123


🏁 Script executed:

# Check for all uses of Bun.$ in the file to understand the pattern
rg -n 'Bun\.\$' test/cli/run/multi-run.test.ts

Repository: oven-sh/bun

Length of output: 203


🏁 Script executed:

# Check if fs or path are already imported in the file
rg -n "import.*from ['\"](?:fs|path)['\"]" test/cli/run/multi-run.test.ts

Repository: oven-sh/bun

Length of output: 37


🏁 Script executed:

# Also check all imports at the top of the file
sed -n '1,50p' test/cli/run/multi-run.test.ts | rg "^import"

Repository: oven-sh/bun

Length of output: 156


🏁 Script executed:

# Check if there are other Bun.$ usages with shell commands (not just realpath)
sed -n '1020,1025p' test/cli/run/multi-run.test.ts && echo "---" && sed -n '1025,1030p' test/cli/run/multi-run.test.ts

Repository: oven-sh/bun

Length of output: 428


🏁 Script executed:

# Also verify the context around line 1393-1404 that was mentioned in scratchpad
sed -n '1390,1410p' test/cli/run/multi-run.test.ts

Repository: oven-sh/bun

Length of output: 798


🏁 Script executed:

# Verify context around both realpath usages to ensure the diff is complete
echo "=== Line 1022-1023 context ===" && sed -n '1018,1027p' test/cli/run/multi-run.test.ts && echo -e "\n=== Line 1900 context ===" && sed -n '1896,1910p' test/cli/run/multi-run.test.ts

Repository: oven-sh/bun

Length of output: 1186


🏁 Script executed:

# Verify that fs.realpathSync and path.join are available in Node.js/Bun standard library
node -e "const fs = require('fs'); const path = require('path'); console.log('fs.realpathSync:', typeof fs.realpathSync); console.log('path.join:', typeof path.join)"

Repository: oven-sh/bun

Length of output: 100


🏁 Script executed:

# Double-check the exitCode handling at line 1393-1404 mentioned in scratchpad
sed -n '1390,1415p' test/cli/run/multi-run.test.ts

Repository: oven-sh/bun

Length of output: 942


Replace Bun.$\realpath`withfs.realpathSync` for cross-platform compatibility and to avoid shell commands in tests.

Using realpath via Bun.$ violates test guidelines and fails on Windows. Replace with fs.realpathSync and path.join for portable paths.

🔧 Suggested fix
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";
+import { realpathSync } from "fs";
+import { join } from "path";

At line 1022:

-    const realDir = await Bun.$`realpath ${String(dir)}`.text();
-    expectPrefixed(r.stdout, "pwd", realDir.trim());
+    const realDir = realpathSync(String(dir));
+    expectPrefixed(r.stdout, "pwd", realDir);

At line 1900:

-    const realDir = (await Bun.$`realpath ${String(dir)}`.text()).trim();
+    const realDir = realpathSync(String(dir));

At lines 1906–1907:

-    expect(pkgALines.some(l => l.includes(`${realDir}/packages/pkg-a`))).toBe(true);
-    expect(pkgBLines.some(l => l.includes(`${realDir}/packages/pkg-b`))).toBe(true);
+    expect(pkgALines.some(l => l.includes(join(realDir, "packages", "pkg-a")))).toBe(true);
+    expect(pkgBLines.some(l => l.includes(join(realDir, "packages", "pkg-b")))).toBe(true);

Per coding guidelines: Avoid shell commands in code; do not use find or grep in tests; use Bun's Glob and built-in tools instead.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const realDir = await Bun.$`realpath ${String(dir)}`.text();
expectPrefixed(r.stdout, "pwd", realDir.trim());
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";
import { realpathSync } from "fs";
import { join } from "path";
const realDir = realpathSync(String(dir));
expectPrefixed(r.stdout, "pwd", realDir);
🤖 Prompt for AI Agents
In `@test/cli/run/multi-run.test.ts` around lines 1022 - 1023, Replace the shell
call Bun.$`realpath ${String(dir)}` with Node/Bun fs.realpathSync to avoid
invoking a shell: import fs (and path if needed), compute const realDir =
fs.realpathSync(String(dir)) (or fs.realpathSync(path.join(...)) for composed
parts) and pass that value to expectPrefixed(r.stdout, "pwd", realDir) instead
of using the Bun.$ command; update references to realDir/dir and remove the
Bun.$ usage.

Comment on lines +1393 to +1404
test("empty script string runs without crashing", async () => {
using dir = tempDir("mr-empty-script", {
"package.json": JSON.stringify({
scripts: {
empty: "",
},
}),
});
const r = await runMulti(["run", "--parallel", "empty"], String(dir));
// Multi-run prefix must appear in stderr (status line)
expect(r.stderr).toMatch(/empty\s+\|/);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add an exit‑code assertion for the empty‑script case.

This test spawns a process but never asserts exitCode, which can hide failures.

✅ Suggested fix
     const r = await runMulti(["run", "--parallel", "empty"], String(dir));
     // Multi-run prefix must appear in stderr (status line)
     expect(r.stderr).toMatch(/empty\s+\|/);
+    expect(r.exitCode).toBe(0);
Based on learnings: Always check exit codes and test error scenarios when spawning processes in tests.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test("empty script string runs without crashing", async () => {
using dir = tempDir("mr-empty-script", {
"package.json": JSON.stringify({
scripts: {
empty: "",
},
}),
});
const r = await runMulti(["run", "--parallel", "empty"], String(dir));
// Multi-run prefix must appear in stderr (status line)
expect(r.stderr).toMatch(/empty\s+\|/);
});
test("empty script string runs without crashing", async () => {
using dir = tempDir("mr-empty-script", {
"package.json": JSON.stringify({
scripts: {
empty: "",
},
}),
});
const r = await runMulti(["run", "--parallel", "empty"], String(dir));
// Multi-run prefix must appear in stderr (status line)
expect(r.stderr).toMatch(/empty\s+\|/);
expect(r.exitCode).toBe(0);
});
🤖 Prompt for AI Agents
In `@test/cli/run/multi-run.test.ts` around lines 1393 - 1404, The test "empty
script string runs without crashing" currently checks only stderr but not the
spawned process exit code; update the test to assert the process exited
successfully by checking the runMulti result (r) for a zero exit code (e.g.,
assert r.exitCode === 0) after calling runMulti(["run", "--parallel", "empty"],
String(dir)) to ensure failures are caught; locate the test by its name and the
use of runMulti and add the exit code assertion alongside the existing stderr
expectation.

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.

3 participants