test(shell): run the pwd &> / &>> redirect tests in a temp dir and check the file contents

[robobun]

Problem

• bun test test/js/bun/shell/file-io.test.ts run from the repo root leaves pwd_output.txt and append_output.txt untracked in the checkout; the second grows a line per run.
• The two tests that write them redirect pwd to a relative path but never ask TestBuilder for a temp dir, so the shell runs in the test runner's cwd.
• The &>> test also never checked that anything was appended; a truncating &>> would have passed it.

Fix

• Both tests now assert the redirected file's contents. Asserting on a file is what makes TestBuilder run the command in a temp dir, so nothing lands in the checkout. The append test seeds the file first and checks the cwd was added after the existing line.
• Two cases added: a builtin's stderr goes into the &> file, and &>> still creates a missing file (seeding the append test would otherwise have removed the only test of that).
• TestBuilder: string fileEquals() expectations get the $TEMP_DIR substitution stdout() already has, and the non-zero-exit path now awaits its checks. Before, a fileEquals mismatch there surfaced as "Unhandled error between tests" while the test passed.
• Verification: test-only change, so nothing fails before the fix. The file passes and git status is clean afterwards; each new assertion was checked to fail against wrong contents, a truncating &>>, and a mismatch on the non-zero-exit path.

Background

• TestBuilder (test/js/bun/shell/test_builder.ts) is the shell test harness: a tagged command template, chained expectations such as exitCode() and fileEquals(), then runAsTest(name).
• It only creates a temp dir and runs the shell inside it when an expectation asks for one (file(), fileEquals(), ...); otherwise the command runs in the test runner's cwd.
• &> sends stdout and stderr to one file; &>> appends both instead of truncating. This describe block exists because a builtin with both redirected once closed the same fd twice (fix(shell): prevent double-close of fd when using &> redirect with builtins #25568).
• $TEMP_DIR in an expected string is replaced with the temp dir path at check time, so a test can assert on pwd output that differs per run.

---

[stamp-90s] gate passed · iteration 1 · 2 files touched

passes on PR (with fix)

Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/js/bun/shell/file-io.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/js/bun/shell/file-io.test.ts
bun test v1.4.0 (f02c937a9)

test/js/bun/shell/file-io.test.ts:
(pass) IOWriter file output redirection > basic file redirection > simple echo to file [62.65ms]
(pass) IOWriter file output redirection > basic file redirection > empty output to file [12.46ms]
(pass) IOWriter file output redirection > basic file redirection > zero-length write should trigger onIOWriterChunk callback [12.82ms]
(pass) IOWriter file output redirection > drainBufferedData edge cases > large single write [13.15ms]
(pass) IOWriter file output redirection > drainBufferedData edge cases > write to subdirectory [13.37ms]
bun: Not a directory: /dev/null/invalid/path(pass) IOWriter file output redirection > file system error conditions > write to invalid path should fail [24.34ms]
bun: No such file or directory: /nonexistent/file.txt(pass) IOWriter file output redirection > file system error conditions > write to non-existent directory should fail [10.16ms]
(pass) IOWriter file output redirection > special file types > write to /dev/null [7.40ms]
(pass) IOWriter file output redirection > special file types > redirect to a FIFO whose reader drains slowly applies backpressure [1153.62ms]
(pass) IOWriter file output redirection > writer queue and bump behavior > single writer completion and cleanup [8.58ms]
(pass) IOWriter file output redirection > writer queue and bump behavior > writer marked as dead during write [7.44ms]
(pass) IOWriter file output redirection > writer queue and bump behavior > bytelist capture during file write [7.72ms]
(pass) IOWriter file output redirection > error handling and unreachable paths > attempt to trigger partial write panic [7.17ms]
(pass) IOWriter file output redirection > error handling and unreachable paths > EAGAIN should never occur for files [6.51ms]
bun: No such file or directory: nonexistent_dir/file.txt(pass) IOWriter file output re
... (truncated)
Exit: 0

diff hotspot

test/js/bun/shell/file-io.test.ts | 22 ++++++++++++++++++++--
 test/js/bun/shell/test_builder.ts |  9 +++++++--
 2 files changed, 27 insertions(+), 4 deletions(-)

gate history · 1 passed · 0 rejected · iteration 1

evidence per changed file

file                               reads  edits  tests
test/js/bun/shell/file-io.test.ts      3      2      0
test/js/bun/shell/test_builder.ts      2      4      0

Original description

What

test/js/bun/shell/file-io.test.ts has two tests that redirect pwd into a relative file without setting up a TestBuilder temp dir:

TestBuilder.command`pwd &> pwd_output.txt`.exitCode(0).runAsTest("builtin pwd with &> redirect");
TestBuilder.command`pwd &>> append_output.txt`.exitCode(0).runAsTest("builtin pwd with &>> append redirect");

TestBuilder only sets the shell's cwd to a temp dir when something asks for one (ensureTempDir(), file(), fileEquals(), ...). These two never do, so the shell runs in the test runner's cwd and the files land there. From the repo root:

$ bun test test/js/bun/shell/file-io.test.ts >/dev/null; git status --short
?? append_output.txt
?? pwd_output.txt

append_output.txt also grows by one line per run, since that test appends to whatever the previous run left behind. Running the whole test/js/bun/shell/ directory, these two files are the only things left in the checkout.

Fix

Both tests now assert the redirected file's contents, which gives them a temp dir like every other redirect test in the file:

• pwd &> pwd_output.txt checks the file holds the cwd ($TEMP_DIR\n, the same convention the existing pwd test in bunshell.test.ts uses for stdout).
• pwd &>> append_output.txt seeds the file first and checks the cwd was appended after the existing line, so it now actually exercises the append half of &>> (before, a truncating &>> would have passed).
• Added case pwd extra-arg &> stderr_output.txt checks the builtin's stderr ends up in the &> file and not on the shell's stderr. The describe block is about stdout and stderr sharing one writer (fix(shell): prevent double-close of fd when using &> redirect with builtins #25568), and until now only the stdout half was checked.
• Added case pwd &>> new_append_output.txt keeps &>> against a missing file covered, since seeding append_output.txt would otherwise have removed the only test doing that.

Two small TestBuilder changes to support this, both in test_builder.ts:

• fileEquals() string expectations get the same $TEMP_DIR substitution stdout()/stderr() already have. No existing fileEquals() call uses that string, so nothing else changes.
• The non-zero-exit path of run() (where Bun.$ throws) called doChecks() without awaiting it. Assertions made after the first await inside doChecks() (fileEquals, doesNotExist) therefore surfaced as "Unhandled error between tests" while the test itself was reported as passing. The new exitCode(1) test goes through that path, so it is now awaited; a mismatch there fails the test it belongs to.

Verification

bun bd test test/js/bun/shell/file-io.test.ts passes (29 tests) and git status stays clean afterwards. The full test/js/bun/shell/ directory has the same results as before the change (the only failures are the two ls permission-denied tests, which fail when running as root, and the shell load process-limit test, both unrelated), and leaves nothing in the repo root.

Also checked that the new assertions bite: wrong expected contents, a truncating &>>, and a fileEquals mismatch on the non-zero-exit path each fail the corresponding test.

Test-only change; there is no runtime behavior to fail before the fix, the symptom is the stray files.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c38527cb-3d5a-4028-8d2a-6107eb7515f3

📥 Commits

Reviewing files that changed from the base of the PR and between 3e61ab3 and 00e28e7.

📒 Files selected for processing (2)

• test/js/bun/shell/file-io.test.ts
• test/js/bun/shell/test_builder.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: fixed and ready for review. Code is unchanged since f02c937; 00e28e7 is an empty commit that reran CI.

Reproduced from a clean checkout on main (fd1ea1b): bun test test/js/bun/shell/file-io.test.ts run from the repo root leaves pwd_output.txt and append_output.txt untracked in the root, and a second run adds another line to append_output.txt. With this branch the same run (and the whole test/js/bun/shell/ directory) leaves the checkout clean; bun bd test test/js/bun/shell/file-io.test.ts passes (29 tests).

f02c937 adds one more &>> case against a missing file, so that path stays covered now that the original append test seeds its file.

CI, for the same test diff across three builds:

• 92778 (d63c9b8): passed on every lane, including both macOS 26 aarch64 jobs.
• 93076 (f02c937): every job that ran passed; the two macOS 26 aarch64 jobs expired without getting an agent.
• 93340 (00e28e7): the only failed job is macOS 14 aarch64, where test/cli/install/migration/complex-workspace.test.ts fails in its install step (reported separately; this PR only touches shell tests). The two macOS 26 aarch64 jobs again expired without getting an agent.

The remaining annotations on all three builds are retried flakes on unrelated lanes. I am not going to push further reruns; the shell tests this PR changes have passed on every lane that ran them.

Related open PRs #34901 and #34904 add other tests to this file but do not touch these two, so there should be no overlap.

[claude]

LGTM — test-only hermeticity fix with strengthened assertions.

What was reviewed:

• Verified no existing fileEquals() call passes a string containing $TEMP_DIR, so the new substitution is inert for existing tests.
• Confirmed pwd extra-arg produces exactly pwd: too many arguments\n on stderr with exit 1 (src/runtime/shell/builtin/pwd.rs:31, and exec.test.ts already asserts this).
• Checked the $TEMP_DIR\n expectation matches the existing cross-platform pwd stdout convention in bunshell.test.ts.
• The await this.doChecks() fix in the catch path is a real correctness fix — doChecks is async and fileEquals/doesNotExist assertions after the first internal await were previously orphaned on non-zero-exit tests.

Extended reasoning...

Overview

Test-only change touching two files under test/js/bun/shell/. In file-io.test.ts, two existing pwd &> / pwd &>> tests are made hermetic by adding .fileEquals() (which triggers TestBuilder's temp-dir setup), and one new test is added to cover the stderr half of &>. In test_builder.ts, fileEquals() string expectations gain the same $TEMP_DIR substitution that stdout()/stderr() already have, and a missing await on doChecks() in the Bun.$-throws path of run() is added.

Security risks

None. No runtime code is touched; changes are confined to test fixtures and a test helper.

Level of scrutiny

Low. This is a hygiene fix for tests that were leaking files into the checkout, plus a small test-builder correctness fix. The diff is small, follows the exact conventions already used by neighbouring tests ($TEMP_DIR in bunshell.test.ts, .fileEquals() throughout file-io.test.ts), and the PR description documents that the assertions were verified to fail when they should.

Other factors

• Grepped all fileEquals( call sites across test/ — none pass a literal $TEMP_DIR string, so the new replaceAll cannot change behaviour of any existing test.
• The new stderr test's expected message matches the builtin exactly (src/runtime/shell/builtin/pwd.rs:31) and the exit-code/stderr contract is already asserted in exec.test.ts:53.
• The unawaited doChecks() on the error path was a genuine latent bug in the harness: any .exitCode(1).fileEquals(...) test would have reported passing while surfacing the mismatch as an unhandled rejection between tests. The fix is one await and mirrors the success path immediately above it.
• The .file() seed for the append test runs at builder time (before the shell), and .fileEquals() reads after — ordering is correct.

[robobun]

^{Updated 10:05 PM PT - Aug 11th, 2026}

❌ @robobun, your commit d63c9b8 has some failures in Build #92778 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 37725

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

bun-37725 --bun

[robobun]

^{Updated 2:05 PM PT - Aug 12th, 2026}

❌ @robobun, your commit 00e28e7 has 1 failures in Build #93340 (All Failures):

• test/cli/install/migration/complex-workspace.test.ts - code 1: error: Failed to install on 🍎 14 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 37725

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

bun-37725 --bun

[claude]

LGTM — test-only hermeticity fix that follows the established TestBuilder pattern.

What was reviewed:

• Confirmed fileEquals() triggers getTempDir() so the shell now runs in a temp cwd; verified no existing fileEquals() call passes a literal $TEMP_DIR string, so the new substitution can't change other tests.
• Checked the pwd builtin source: it writes cwd + "\n" on stdout and "pwd: too many arguments\n" + exit 1 on stderr for extra args, matching the new assertions; TestBuilder.tmpdir() already returns a realpathSync'd path so the $TEMP_DIR comparison holds.
• The added await on doChecks() in the error path is a real fix — the success path already awaited it, and fileEquals/doesNotExist assertions live after the first await inside doChecks.

Extended reasoning...

Overview

Test-only PR touching two files: test/js/bun/shell/file-io.test.ts (converts two non-hermetic pwd &> / pwd &>> tests to use fileEquals(), adds a stderr-redirect case and an append-creates-file case) and test/js/bun/shell/test_builder.ts (adds $TEMP_DIR substitution to fileEquals() string expectations, and awaits doChecks() in the non-zero-exit error path). No runtime code is touched.

Security risks

None. This is test harness and test assertion code only; no user-facing surface, no auth/crypto/permissions.

Level of scrutiny

Low. The change follows the exact pattern used by every other redirect test in the same file (.fileEquals() to get a temp dir). I cross-checked the source: src/runtime/shell/builtin/pwd.rs writes cwd + b'\n' on the success path and b"pwd: too many arguments\n" with exit 1 on the error path (via on_io_writer_chunk returning 1 for WaitKind::Stderr), which matches the new expectations. The $TEMP_DIR convention mirrors bunshell.test.ts:1676, and TestBuilder.tmpdir() returns a realpathSync'd path so pwd output will match. I grepped every fileEquals( call site in test/js/bun/shell/ — none pass a string containing $TEMP_DIR, so the new substitution cannot alter existing behavior.

The await this.doChecks(...) in the catch branch is a genuine harness bug fix: doChecks is async and its fileEquals/doesNotExist assertions run after the first await, so without the added await a mismatch on the non-zero-exit path would surface as an unhandled rejection between tests rather than failing the correct test. The success path already awaited it.

Other factors

The append test now seeds the file (.file("append_output.txt", "existing line\n")) and asserts both lines, so it actually distinguishes append from truncate — a strictly stronger assertion than before. The added &>> creates a missing file case preserves the create-if-missing coverage the original test happened to provide. No prior human review comments to address; the earlier CI status was on the previous commit and the description notes the full shell test directory results are unchanged.

[claude]

LGTM — test-only hermeticity fix that stops file-io.test.ts from leaving pwd_output.txt / append_output.txt in the repo root, and strengthens the assertions while it's at it.

What was reviewed:

• fileEquals $TEMP_DIR substitution mirrors the existing stdout()/stderr() behavior; grepped every existing .fileEquals() string call and none contain $TEMP_DIR, so no behavior change elsewhere.
• Verified against src/runtime/shell/builtin/pwd.rs that pwd extra-arg writes exactly pwd: too many arguments\n to stderr and exits 1, and that success writes cwd + "\n" (matches $TEMP_DIR\n since tempdir is realpath'd).
• The await this.doChecks(...) in the catch path is a real fix — doChecks awaits Bun.file().text(), so fileEquals failures on non-zero-exit tests were previously unhandled rejections after the test passed.

Extended reasoning...

Overview

Test-only change touching two files under test/js/bun/shell/. In file-io.test.ts, the two pwd &> / pwd &>> tests are given .fileEquals() assertions so TestBuilder allocates a temp dir (previously they wrote into the test runner's cwd and left stray files in the checkout). Two new cases are added: one that redirects builtin stderr with &>, and one that keeps &>>-creates-missing-file covered now that the append test seeds its target. In test_builder.ts, fileEquals() string expectations gain the same $TEMP_DIR substitution that stdout()/stderr() already have, and the catch-path doChecks() call is now awaited.

Security risks

None. This is test infrastructure only; no runtime code, no untrusted input handling, no auth/crypto/permissions surface.

Level of scrutiny

Low. The change is small, mechanical, and follows the exact conventions already used throughout this test file and in bunshell.test.ts (which already uses $TEMP_DIR for pwd stdout at line 1672). I cross-checked the new assertions against src/runtime/shell/builtin/pwd.rs: with args it writes b"pwd: too many arguments\n" to stderr and exits 1 (both the write_no_io fast path and the on_io_writer_chunk completion for WaitKind::Stderr); without args it writes cwd + '\n'. The tempdir passed to .cwd() is fs.realpathSync(fs.mkdtempSync(...)), so the shell's cwd string equals the substituted $TEMP_DIR exactly.

Other factors

• I grepped every .fileEquals( call in test/js/bun/shell/ — none of the existing string arguments contain $TEMP_DIR, so the new replaceAll cannot change any existing test's behavior.
• The missing await on doChecks in the catch branch was a genuine latent harness bug: doChecks is async and awaits Bun.file(...).text() before the fileEquals/doesNotExist assertions, so those failures previously surfaced as unhandled rejections after the test had already been marked passing. The new exitCode(1) test goes through that path, so the fix is exercised.
• CI on the earlier commits passed everywhere agents were available; the latest commit is an empty retrigger. No prior review comments to address.