test: move the recursive cp error race test to the shell cp builtin

[robobun]

Problem

• test/js/node/fs/cp.test.ts > "fs.promises.cp recursive does not free parent task while subtasks are in flight after an error" fails on every debug/ASAN run, alone or with the file: ^ this test timed out after 5000ms. (shell(cp): copy the file a symlink operand points at unless -R is given #37954 ran into the same timeout.)
• The child it spawns needs about 5s on a debug build: ~1.3s of startup plus 20 iterations of a recursive fs.promises.cp of 33 files at ~180ms each. On a release build the same child takes well under a second.
• The 20 iterations no longer buy anything. The test was written for fs.promises.cp: defer AsyncCpTask destruction until all subtasks finish #30162, a use-after-free between the native per-file copy tasks of a recursive copy. Since fs: port Node.js v26.3.0 fs tests and fix the gaps they surface — cp error semantics, watcher event delivery, watch ignore+AbortSignal, FileHandle pull/writer, glob port, opendir/Dir, mkdtempDisposable, rmdir-recursive end-of-life, mock.fn (+119 tests) #31830, fs.promises.cp of a directory whose destination already exists goes through the JS walker on every platform (src/js/node/fs.promises.ts:187-192 -> tryNativeFastPath in src/js/internal/fs/cp.ts, which only takes the native path for a directory on macOS when the destination is missing). The walker's copyDir (src/js/internal/fs/cp.ts:337) awaits one entry at a time, so there are no concurrent tasks for the loop to race, and every iteration just re-observes the same ERR_FS_CP_NON_DIR_TO_DIR rejection.
• The native fan-out is still live code: the shell's cp -R builtin drives the same NewAsyncCpTask / CpSingleTask (src/runtime/node/node_fs.rs:1359-1502) on every platform, and nothing exercised its "one file fails while its siblings are still copying" path.

Fix

• Test-only change; the defect is the test's time budget, so there is no src/ diff and no fail-before against src/.
• test/js/node/fs/cp.test.ts: the subprocess test is replaced by a case in the existing per-implementation loop (so it covers cpSync and promises.cp, on all platforms): a recursive copy where one entry's destination is a directory throws ERR_FS_CP_NON_DIR_TO_DIR with the entry's path and leaves the directory in place. This is what the removed test was actually checking; it now takes ~30ms on a debug build and runs in-process.
• test/js/bun/shell/commands/cp.test.ts: new test that runs cp -R src dst 20 times in a child bun with BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS=1, against 32 regular files plus one whose destination is a directory. Every run has to exit 1 with the builtin's own message (the system cp, which the shell falls back to without the env var, words it differently), the child has to exit 0 with an empty stderr, and all 32 siblings have to be in dst/src afterwards, which is what leaves them touching the shared parent task after the error was recorded. The child gets ASAN_OPTIONS=symbolize=0, as the other tests of this kind in the repo do, so an ASAN abort fails the assertion instead of timing the test out while llvm-symbolizer runs.
• Why this is the right place: the shell builtin is the one caller of the native recursive copy that runs on Linux, so it is the only way the ASAN lane can see this race. The test lives outside the file's describe.if(!builtinDisabled("cp")) block because that block is skipped on POSIX (CI does not set the env var), matching how the other recent shell cp tests are written.
• Verified:
  • bun bd test test/js/node/fs/cp.test.ts: 48 pass, 5 skip (before: 1 fail by timeout, every run).
  • bun bd test test/js/bun/shell/commands/cp.test.ts --rerun-each=10: 10/10 pass, 0.5s to 0.9s each on the debug/ASAN build.
  • To check that the moved test still guards what the old one was written for, I temporarily put the fs.promises.cp: defer AsyncCpTask destruction until all subtasks finish #30162 bug back in node_fs.rs (finish_concurrently posting the completion as soon as a result is recorded, and the failing CpSingleTask keeping its reference). The new shell test then fails in under a second with ERROR: AddressSanitizer: heap-use-after-free in 10 out of 10 runs, after 1 to 9 of the 20 iterations (report below). The removed fs.promises.cp test passes on that same broken build, since it never reaches this code.
  • Windows (the canary at the same commit, USE_SYSTEM_BUN=1): shell/commands/cp.test.ts 31 pass, fs/cp.test.ts 46 pass, 7 skip. The builtin is always on there and reports the failed file as Operation not permitted, which the test expects on that platform.

Background

• fs.cp / fs.promises.cp (src/js/node/fs.promises.ts) validate in JS and then either hand the copy to the native fs.cp binding or walk the tree with the node-ported walker in src/js/internal/fs/cp.ts. After fs: port Node.js v26.3.0 fs tests and fix the gaps they surface — cp error semantics, watcher event delivery, watch ignore+AbortSignal, FileHandle pull/writer, glob port, opendir/Dir, mkdtempDisposable, rmdir-recursive end-of-life, mock.fn (+119 tests) #31830 the native path is taken for single regular files and, on macOS only, for a directory tree with no existing destination (a single clonefile()); everything else, including any copy into an existing directory, is the JS walker.
• NewAsyncCpTask (src/runtime/node/node_fs.rs) is the native recursive copy: a work-pool task scans the directory and spawns one CpSingleTask per file, all holding a pointer to the parent. subtask_count is a refcount; the parent is destroyed on the JS thread once it reaches zero. fs.promises.cp: defer AsyncCpTask destruction until all subtasks finish #30162 fixed the version of this where a failing file copy destroyed the parent immediately, while sibling copies were still using it.
• The shell cp builtin (src/runtime/shell/builtin/cp.rs) resolves its operands and then starts the same NewAsyncCpTask (as ShellAsyncCpTask). On Linux and macOS the builtin is off by default and the shell runs the system cp instead; BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS=1 turns it on, and it is read once at startup, which is why the copies run in a child process. On Windows it is always on.
• ASAN_OPTIONS=symbolize=0: when ASAN aborts a debug bun it symbolizes the report first, which takes several seconds on this binary; with symbolization off the report header still lands in stderr and the abort is immediate.

What the new shell test reports with the #30162 bug put back (debug/ASAN build, symbolized run)

==32674==ERROR: AddressSanitizer: heap-use-after-free on address 0x7b97798e01ed at pc 0x00000e4b54aa bp 0x7a57247e8520 sp 0x7a57247e8518
READ of size 1 at 0x7b97798e01ed thread T10 (Bun Pool 7)
    #0 <bun_runtime::shell::builtins::cp::ShellCpTask>::cp_on_copy src/runtime/shell/builtin/cp.rs:459:13
    #1 <bun_runtime::node::fs::_async_tasks::NewAsyncCpTask<true>>::on_copy src/runtime/node/node_fs.rs:1534:18
    #2 <bun_runtime::node::fs::_async_tasks::CpSingleTask<true>>::run_owned src/runtime/node/node_fs.rs:1493:32
    #3 <bun_runtime::node::fs::_async_tasks::CpSingleTask<true> as bun_threading::work_pool::OwnedTask>::run src/threading/work_pool.rs:118:53
    ...
freed by thread T0 here:
    ...
    #9  <alloc::boxed::Box<bun_runtime::shell::builtins::cp::ShellCpTask> as core::ops::drop::Drop>::drop
    #11 <bun_runtime::shell::builtins::cp::Cp>::print_shell_cp_task src/runtime/shell/builtin/cp.rs:326:5
    #12 <bun_runtime::shell::builtins::cp::Cp>::on_shell_cp_task_done src/runtime/shell/builtin/cp.rs:304:9
    #16 <bun_runtime::shell::builtins::cp::ShellCpTask>::cp_on_finish src/runtime/shell/builtin/cp.rs:494:13
    #17 <bun_runtime::node::fs::_async_tasks::NewAsyncCpTask<true>>::run_from_js_thread src/runtime/node/node_fs.rs:1713:26

10 runs of the child loop on that build: ASAN fired in all 10, after 1, 3, 1, 2, 2, 1, 3, 9, 6 and 5 completed iterations.

Timing of the removed test's child on this machine

debug build:   files=32 iters=20 total=3572ms per-iter=178.6ms startup-to-main=1249ms
release build: files=32 iters=20 total=680ms  per-iter=34.0ms  startup-to-main=42ms

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 23 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: 57d689ab-9756-43e1-b323-062850acc43b

📥 Commits

Reviewing files that changed from the base of the PR and between b7a0431 and 76abe1c.

📒 Files selected for processing (2)

• test/js/bun/shell/commands/cp.test.ts
• test/js/node/fs/cp.test.ts

---

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

[robobun]

Status: reproduced and fixed.

• Reproduced on main (b7a0431) with bun bd test test/js/node/fs/cp.test.ts -t "does not free parent task": fails by the 5s timeout on the debug/ASAN build every run; the child it spawns takes ~5s there (1.3s startup, ~180ms per iteration) and well under a second on a release build.
• The loop was exercising the sequential JS walker (since fs: port Node.js v26.3.0 fs tests and fix the gaps they surface — cp error semantics, watcher event delivery, watch ignore+AbortSignal, FileHandle pull/writer, glob port, opendir/Dir, mkdtempDisposable, rmdir-recursive end-of-life, mock.fn (+119 tests) #31830), not the native race it was written for, so this PR replaces it with an in-process check and moves the race coverage to the shell cp -R builtin, which still uses the native per-file tasks. Details, including the check that the moved test still catches the fs.promises.cp: defer AsyncCpTask destruction until all subtasks finish #30162 bug when it is put back, are in the PR description.
• Test-only change: there is no src/ diff to prove fail-before against.

[claude]

LGTM — test-only relocation of a UAF regression guard to the code path that still reaches it.

What was reviewed:

• Confirmed fsCpNonDirToDirError in both cp.ts and cp-sync.ts sets err.path to the joined destination, so the new in-loop fs.cp test's e.path assertion holds for both impls and on Windows (copyDir uses path.join, which normalizes separators).
• Checked tempDir returns a realpath'd base, so the shell test's absolute-path stderr assertion won't mismatch on macOS's /var → /private/var symlink.
• Verified harness conventions: bunEnv spread, concurrent stdout/stderr/exited drain, using tempDir, subprocess isolation for the ASAN-abort case, and the outcomes Set makes any per-iteration variance visible in the diff rather than masked.

Extended reasoning...

Overview

Test-only PR touching two files. It removes a subprocess test from test/js/node/fs/cp.test.ts that was timing out on every debug/ASAN run (~5s child vs 5s default timeout) and, per the PR's tracing of tryNativeFastPath after #31830, no longer reached the native NewAsyncCpTask fan-out it was written to guard — fs.promises.cp into an existing directory now walks entries one at a time in JS. The guard is re-homed to test/js/bun/shell/commands/cp.test.ts against the shell cp -R builtin, which is the remaining caller of the concurrent native path on Linux (where ASAN runs). A simpler in-process case is added to the fs.cp/cpSync per-implementation loop covering what the removed test was actually observing: ERR_FS_CP_NON_DIR_TO_DIR with the entry's path, and the destination directory left intact.

Security risks

None. Test code only; no changes to runtime, parsing, or any user-reachable surface.

Level of scrutiny

Moderate — the review guidance flags "never silently weaken, skip, or delete an existing test or safety net" as merge-blocking. This PR clears that bar explicitly: the removed test's stated invariant (parent task survives a failing subtask while siblings run) is preserved by the new shell test, and the author verified it by temporarily reintroducing the #30162 bug and observing the new test fail with heap-use-after-free in 10/10 runs while the removed test passed on that same broken build. That is exactly the fail-before proof the review rules ask for.

Other factors

• I traced copyDir in src/js/internal/fs/cp.ts:337-345 and the fsCpNonDirToDirError throw sites in both cp.ts:60 and cp-sync.ts:190; both set path: dest where dest is the path.join-built entry path, so the new test's e.path === join(basename, "result", "b.txt") assertion is correct for both implementations and normalizes separators on Windows.
• tempDirWithFiles (harness.ts:458) realpaths os.tmpdir() before mkdtempSync, so the shell test's absolute-path stderr comparison is not at risk from the macOS /var symlink.
• Iteration-order independence: only b.txt (fs test) / 000-bad.txt (shell test) has a conflicting destination, so the error is deterministic regardless of opendir/readdir order.
• The shell test correctly sits outside describe.if(!builtinDisabled("cp")) (skipped on POSIX in CI) and forces the builtin via BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS=1 in a child, matching how the env var is read once at startup. The outcomes Set + toEqual([{...}]) pattern means any variance across the 20 iterations produces a readable diff rather than a silent pass.
• ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "symbolize=0"].filter(Boolean).join(":") correctly composes with any harness-set options and degrades to "symbolize=0" when none are set.
• Author ran --rerun-each=10 on debug/ASAN and the Windows canary; both files pass. No prior human or bot review comments to address.

[robobun]

^{Updated 5:05 AM PT - Aug 13th, 2026}

❌ @robobun, your commit 76abe1c has some failures in Build #94573 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38155

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

bun-38155 --bun