[robobun]

Problem

• When one test times out, bun test kills every child process the file spawned: killed 7 dangling processes where the test spawned 1.
• Execution::kill_dangling_processes_on_timeout (src/runtime/test_runner/Execution.rs:324) calls auto_killer.kill(), which drains the per-VM set. Rewrite test/describe, add test.concurrent #22534 (1.2.23) dropped the per-test clear.

Fix

• ProcessAutoKiller stamps each process with a scope number. The runner begins one per callback, and a timeout kills only that scope.
• A timed-out afterEach kills what its test started, a timed-out afterAll what its describe block or file started. --bail kills every tracked process.
• The step that declares the timeout kills: a returned callback passes, a leftover timer kills nothing. A wait in place (expect().resolves) keeps the wide kill, at its deadline or at file end.
• Verified: test/cli/test/test-timeout-behavior.test.ts (23 new cases, 18 fail on 1.4.3), two --bail cases in bun-test.test.ts. Self-review: see notes.

Background

• ProcessAutoKiller (src/jsc/) is the per-VM set of child processes that a test file spawned.
• After-hooks run after a timeout, so only the interrupted callback's children lack cleanup.
• One file timer serves every deadline. expect(promise).resolves waits in place: it turns the event loop until the promise settles.
• Alternative to bun test: kill only the timed-out test's processes, and a concurrent group's once it is done #39657 (open): there a beforeEach child died before afterEach ran. Builds on bun test: only kill the timed-out test's own subprocesses on timeout #38774.

Downsides

• A timed-out test body leaves its beforeEach children to afterEach, and a timed-out afterAll leaves outer children alone. main kills both.
• --bail also kills after a non-timeout failure. main does not.
• No-timeout cost: +2 instructions per callback, +3 per test, +8 per tracked spawn, +22 per exit, +4 B per child, .text +1,792 B.

Notes

Repro. One file: beforeAll starts 2 children (Bun.spawn and node:child_process), beforeEach starts 1 per test, the first test starts 2, the second test starts 1 and never settles (timeout 200 ms). A third test does a stdin/stdout round trip with every helper.

build	output at the timeout	helpers of beforeAll, beforeEach, earlier test
1.4.3, main 9634520	killed 7 dangling processes	dead (EPIPE)
this PR	killed 1 dangling process	all answer

The result is the same with --isolate, --parallel and --parallel --no-isolate.

Who is killed. "Tracked" means every process the hooks and tests started that is still running.

what times out, or stops the run	1.4.3 and main	this PR
test body	every tracked process	what the body started
beforeAll or beforeEach	every tracked process	what that hook started
afterEach, onTestFinished	every tracked process	what the test and its hooks started, in every attempt of a retry
afterAll of a describe block	every tracked process	what the hooks and tests of that block started
afterAll at the top level of a file	every tracked process	what the hooks and tests of that file started
afterAll of a preload	every tracked process	every tracked process
--bail after a timeout	every tracked process, at the timeout	what the callback started, then every tracked process at the exit
--bail after another failure	nothing	every tracked process at the exit
callback that returned before the timer ran	every tracked process	nothing
timer of an earlier entry fires during a test with timeout: 0	every tracked process	nothing
callback that waits in expect().resolves before its first await (the runner cannot step)	every tracked process	every tracked process
the same, after the timer of an earlier entry fired	nothing, the run waits for the whole life of the child	every tracked process, at the deadline
test that waits in expect().resolves after an await (the runner goes on)	every tracked process	what the body started
end of a file while a wait in place still lasts	nothing	what the timeouts of that file spared
callback that overran in spawnSync and then waits in place	every tracked process, when spawnSync returns	what the callback started, then every tracked process when the wait begins

History. A probe with one child each from beforeAll, beforeEach and the test body prints killed 1 on 1.1.30 and 1.2.22, and killed 3 on 1.2.23 and 1.4.3. The commit message of #22534 lists a per-test auto_killer layer as not done ("at the end of the test, we kill the test processes"). Commit 534ba47 records one timeout in bun-publish.test.ts that killed the Verdaccio registry started in beforeAll. I found no user issue for this.

Where the kill runs. On main the timer callback kills when now is past the deadline of the active entry, and then the step reads the clock again to declare the timeout. Two things follow from the two reads.

• A timer that fires a little before the deadline of the active entry passes the first read and fails the second: the entry times out and nothing is killed. Such a timer exists whenever an earlier entry with the same timeout finished, because the file timer only moves earlier. Probe: 150 tests that hang, each after a beforeEach that holds the thread for 100 µs under the same 50 ms timeout. main: 5 and 6 of the 150 timeouts print no kill in 2 of 3 runs (a later timeout then prints killed 2). With only the scoped kill those children stay: 5 of 150 in one run of an earlier state of this PR. This PR: 150 of 150 in 3 of 3 runs.
• A timer that fires while an entry with timeout: 0 is active kills on main (timespec 0 is before now). That is the bug of fix(test): preserve unlimited test children after stale deadlines #44613.

In this PR step_sequence_one calls the kill when evaluate_timeout reports the timeout, and handle_timeout wakes the runner. A timeout always has its kill, and nothing else has one. A result that is already queued is stepped before the wake-up, so a callback that returned late passes and keeps its children. An earlier state of this PR scanned the result queue for that case. That scan is gone. spawnSync keeps its own call, because its isolated loop does not run the file timer.

A wait in place. expect(promise).resolves, .rejects and an async toThrow wait in place: they turn the event loop under the script frame that called them, and that frame returns only when the promise settles. Nothing can interrupt that wait. On main the kill of every tracked process at a timeout is what ends it when the promise depends on a child, for example child.exited or a request to a helper. A promise that depends on no child is never ended, on main and here (a test that waits in expect(new Promise(() => {})).resolves hangs bun test on 1.4.3 too). Two cases keep a kill for it.

• The callback that waits is on the stack of run (it has not reached its first await). The runner cannot step until it returns, so handle_timeout decides itself: past the deadline of that callback it kills every tracked process. With only the scoped kill, a wait for a child of beforeAll lasted for the whole life of that child (8.1 s in a probe, 0.2 s on main and now). When the timer fired for an earlier entry, handle_timeout arms the timer again for the deadline of the callback, because run does that only after the callback returns. main loses the timer there: a test that waits for its own child after an earlier test waits for the whole life of the child (11.8 s on main, 0.3 s here). An entry with timeout: 0 gets nothing.
• The wait is in a continuation (after an await). The runner steps, so the timeout stays scoped and the helpers serve the next tests. The frame that waits is still under every later step, and the file cannot end before it returns. The event loop now counts its wait_for_promise calls, and ProcessAutoKiller records the scope of its last scoped kill. When a file ends while that count is not zero, advance kills the processes that a scoped kill of this file left alive (kill_spared): the ones that main killed at that timeout (9.0 s with only the scoped kill, 0.2 s on main and now). Without a wait in place, or without a scoped kill before it, the end of a file kills nothing. A test that fails through done(err) steps the runner under a script frame too, and a preload server must survive a file in which no timeout spared it.

spawnSync reports a deadline that passed in its own loop through bun_test_timeout_in_spawn_sync, with the callback still on the stack. The report arms the timer again for that deadline, which is past. When the callback then starts a wait in place, the timer fires under it and the first case applies. main killed every tracked process at the report.

Three narrow cases still wait where main does not. Each needs an earlier timeout that spared a helper, and a later wait in place that ends only when that helper dies. The wait is in a callback with timeout: 0 before its first await, or in a group of test.concurrent tests before its first await, or it starts after its file ended.

Open for a maintainer.

• A test body that times out leaves its beforeEach children to afterEach. That is the 1.2.22 behavior. The other choice is to kill them when the sequence of the test ends. That also takes a helper that beforeEach starts once for later tests, and it fails the retry case.
• fix(test): preserve unlimited test children after stale deadlines #44613 guards the kill in the timer callback against timeout: 0. This PR moves that kill to the step, and the one case that stays in the timer callback skips an entry with no timeout. The regression test of fix(test): preserve unlimited test children after stale deadlines #44613 passes here without its guard (it fails on main). The line that fix(test): preserve unlimited test children after stale deadlines #44613 changes is still there, and git merge-tree merges the two PRs in either order.
• bun test: kill only the timed-out test's processes, and a concurrent group's once it is done #39657 is still open. I did not open it in this work, so I leave it to a maintainer.
• A wait in place could end at the deadline itself: wait_for_promise returns, and the matcher throws a timeout error. That ends every such wait, also one that depends on no child, and both kills of "A wait in place" go away. It changes what expect().resolves does at a timeout, so it is not in this PR.

Also fixed here.

• A callback that resumes from the event loop, holds the thread past its deadline and then returns: the entry passes, but main kills every tracked child when the timer fires before the runner steps (killed 1 dangling process with one beforeAll server, 3 of 3 runs). 1.2.22 skipped the kill once a test had reported.
• The spawnSync wait loop calls the same function, so a test that times out inside Bun.spawnSync or child_process.spawnSync loses only that child (main: killed 2).
• A retry: the helper that beforeEach started for attempt 1 survives the timeout of attempt 1. If the afterEach of a later attempt times out, that helper is killed with the rest of the test.

Not changed.

• A group of two or more test.concurrent tests: the runner kills nothing at a timeout, as on main.
• A callback that overruns its deadline without yielding is marked timed out and gets no kill, as on main. bun test: interrupt synchronous infinite loops on --timeout #30598 adds that call and then gets the scoped kill.
• Attribution is a time window. A child that a late timer of an earlier test starts while the timed-out test is active is killed with it.
• A child that an earlier test leaked is no longer reaped as a side effect of a later test timeout. The afterAll of its block, of its file or of a preload, and --bail, still reap it.
• Concurrent tests on both sides of a describe boundary run in one group. The afterAll of that block then also kills what the other tests of that group started. The docs say so. Inside a group the callbacks of the sequences interleave, and a process gets the scope of the callback that began last, so a mark per sequence does not separate them either.
• node:test files: a node-level beforeEach runs inside the test's bun:test callback, so its child still dies with the test.
• A timeout while a callback waits in expect().resolves before its first await kills every tracked process, as on main. See "A wait in place".
• A file that ends while a wait in place lasts kills what the timeouts of that file spared. main killed those processes at the timeout.

Other designs. A per-group clear of the set fails under --isolate, where the set must hold the whole file. A mark per describe block in DescribeScope, written when the order is built, needs a field and a special case for concurrent tests that join the group before the block. The scan in scope_start_mark runs only when an afterAll times out.

Other open PRs. #44613, #41878 and #45792 merge cleanly with this branch. #43803, #30598 and #40235 already conflict with main. This PR changes regions of Execution.rs that #40235 rewrites.

Measurements. Release builds (ThinLTO) of main 9634520 and this PR, same build directory.

• callback start: +2 instructions per executed callback, +0 per entry without a callback, 0 allocations (objdump of execution::step_sequence, where step_sequence_one is inlined)
• group (one per serial test, one per beforeAll or afterAll hook, one per run of concurrent tests): +1 instruction when the order is built, +2 when the group starts (objdump of Order::generate_order_describe and BunTest::run)
• end of a file: +4 instructions (the check for a wait in place) (objdump of BunTest::run, where advance is inlined)
• timer fire: the timer callback no longer calls kill_dangling_processes_on_timeout and its clock read. It tests in_run_loop instead (objdump of BunTest::on_timeout)
• wait_for_promise: +2 instructions when the promise is already settled, about +6 when it waits (objdump, 123 -> 132 instructions in the function)
• ProcessAutoKiller: 96 B -> 104 B (one per VM), its map: 88 B -> 88 B, EventLoop: 448 B -> 448 B, ConcurrentGroup: 48 B -> 48 B, ExecutionSequence: 64 B -> 64 B, Execution: 80 B -> 80 B (array-length asserts with cargo check)
• per tracked child: +4 B, +1 heap block per VM that tracks a child (counting allocator at 1/8/64 children: +16/+32/+256 B)
• tracked spawn: +8 instructions (1 at the call site, ArrayHashMap::put 48 -> 55), first tracked spawn of a VM: +98 (put 230 -> 327), tracked exit: +22 (swap_remove 62 -> 84), spawn/exit with tracking off: +0 (objdump of the two sites in spawn_maybe_sync, gdb single-step, 4 of 4 calls identical)
• SIGTERMs at one timeout on the repro: 1 (main: 7), with --isolate: 1 (main: 7) (gdb catch syscall kill. strace is not installed in my container.)
• release .text: +1,792 B, the 28 functions that changed size: +1,510 B, stripped binary: 107,525,856 B -> 107,525,856 B (size -A and nm -S)
• Only on the timeout path: kill_dangling_processes_on_timeout grows from 403 B to 574 B, the timer callback from 232 B to 455 B.

Tests.

• test-timeout-behavior.test.ts: 25 cases, 23 of them new. USE_SYSTEM_BUN=1 (1.4.3) and a release build of main: 7 pass, 18 fail. Debug and release builds of this PR: 25 pass, 3 of 3 release runs. Release build with every core busy 3 times over: 25 of 25 runs, 8 times over: 12 of 12. It also passes with the leak-check environment of the ASAN lane.
• bun-test.test.ts: two new --bail cases. Both fail on 1.4.3 and main. The timeout case starts a child in the test that times out, so it expects one kill at the timeout and one at the exit. They are in that file with the other --bail tests. With the leak-check environment, a --bail run with both kills exits with code 1 and no leak report (checked on the debug build).
• The new cases that pass on main (retry with a hung afterEach, a wait in expect().resolves for its own child and for a child of beforeAll, a file that ends under a wait in place with no timeout before it, a last test that fails through done(err)) pin behavior that the narrower kill must keep. The file-end case fails when the end of the file kills every tracked process (checked with that change in a debug build). The retry case fails when the mark is read again per attempt (checked with that change in a debug build). The wait for its own child fails with the kill only at the step: no kill, and 12.3 s against 0.5 s (checked on a build of d20ac16). The wait for a child of beforeAll fails with the scoped kill there: 8.1 s against 0.2 s (checked on a build of 669f229).
• Liveness is a stdin/stdout echo round trip. A killed child never answers.
• No callback that has to finish runs under a short timeout. A callback that returns at once can still take longer than 100 ms on a loaded machine (seen once on Windows 11 aarch64). The hooks that hang are inside a describe block, the retry case times out on every attempt, and the late-return case (500 ms) runs alone after the concurrent cases.
• On a debug build the new cases run one at a time (test.concurrentIf(!isDebug)). A debug bun needs about 0.5 s of CPU to start, and all cases at once ran past the default 5 s timeout on a 12-core machine. A release build runs them together: 2.0 s for the file.
• The two existing fixtures no longer import harness (2.6 s per process in a debug build), and their runner gets cwd: import.meta.dir so the repo bunfig.toml preload is not loaded. Their slow test after test timeout loses its 200 ms timeout: with all cases at once its 100 ms sleep overran it once. The assertions are unchanged.
• Also run on the debug build: isolation (47 pass. Before the last merge of main, 2 of 6 debug runs each failed one case that passes alone: one uses a FinalizationRegistry, one a RedisClient), expect (424 pass), node-test (45 pass), spawnSync, spawnsync-isolated-event-loop, spawnsync-no-microtask-drain, bun_test, concurrent, concurrent_immediate, failure-skip, test-retry-repeats-basic, test-failing, jest-hooks, concurrent-test-glob, done-async, test-timers, test-test, test-on-test-finished, describe, nested-describes, bunfig/preload, regression/issue/23865. cargo clippy on bun_runtime.
• test/cli/test/bun-test.test.ts (it has the --bail cases): 109 pass on a release build of this PR. On the debug build 108 pass: instances of a class nested too deeply to print are a RangeError and the data of events nested too deeply to print are a RangeError fail there. Those cases print a value nested 100,000 deep and do not spawn or time out. They pass on release builds of main and this PR.

Review history.

• A self-review before I opened the PR returned "merge after changes". Its summary named seven things. Six are done: name fix(test): preserve unlimited test children after stale deadlines #44613, skip the kill once the callback has reported, correct the test.concurrent sentence in the docs, list the after-hook and --bail cases, state what needs a maintainer's decision, and cite the repro and the two commits instead of a user report. The seventh is about bun test: kill only the timed-out test's processes, and a concurrent group's once it is done #39657, and I leave that PR to a maintainer. That review also counted 15 smaller concerns about the implementation. That list was not available to me, so I do not claim them.
• The first review on this PR asked for three changes. All three are made: a timed-out afterEach or afterAll kills what it was going to stop, --bail kills the tracked processes before it exits, and the late-return test holds the thread with a loop.
• The second review found two things. Both are done: the mark for an afterEach timeout is taken once per test and not per attempt, and a timed-out afterAll kills what its own describe block started and not the children of outer scopes.
• I reviewed the commits after that by hand, line by line. That review moved the kill to the step: a load test of the new retry case failed 1 of 15 runs through the timeout without a kill that is described above.
• The third review had one optional point: concurrent tests on both sides of a describe boundary. It is now in the docs, and "Not changed" says why the code stays.
• A report from another work session: with the kill only at the step, a test that waits for its child in expect().resolves got no kill at its deadline. I confirmed it and fixed it in handle_timeout. The same place now also arms the timer again, which main does not.
• The fourth review found that the scoped kill there leaves a wait for a child of beforeAll without an end. Confirmed and changed: see "A wait in place". The same check found the wait in a continuation, which the end of the file now handles. Its optional point about the --bail timeout case is done too.
• The fifth review found three faults in that change, and all three held. The end of a file took any script frame for a wait in place, so done(err) killed every tracked child. It killed every tracked process and not only what a timeout spared, so a preload server died where main keeps it. A wait in place after a spawnSync timeout in the same callback had no timer. The count in the event loop, kill_spared and the timer that the spawnSync report arms are the answers. Each has a case in the test file.

---

no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/test/test-timeout-behavior.test.ts, test/cli/test/bun-test.test.ts

[robobun]

^{Updated 6:05 PM PT - Oct 21st, 2026}

❌ @robobun, your commit 497bcf1 has some failures in Build #130304 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 45081

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

bun-45081 --bun

[robobun]

Status: reproduced, fixed, waiting for CI and a maintainer.

How I reproduced it:

1. A test file whose beforeAll starts two long-lived children (Bun.spawn and node:child_process), whose beforeEach starts one per test, whose first test starts two more, and whose second test starts one and never settles (timeout 200 ms). A third test does a stdin/stdout round trip with every helper.
2. bun test on that file with bun 1.4.3 and with a build of main: the runner prints killed 7 dangling processes and every helper is dead. The timed-out test spawned 1 of the 7.
3. With this branch the runner prints killed 1 dangling process and every helper answers, also with --isolate and --parallel.

The same file shape is in test/cli/test/test-timeout-behavior.test.ts: 23 new cases, 18 of them fail on 1.4.3 and all pass on this branch. Two --bail cases are in test/cli/test/bun-test.test.ts.

After review, a timed-out afterEach kills what its test started in every attempt, and a timed-out afterAll kills what its own describe block or file started. The kill runs in the step that declares the timeout. A wait in place (expect().resolves) that a timeout cannot interrupt keeps the kill of main: every tracked process at the deadline when the runner cannot step, and otherwise what the timeouts of the file spared when the file ends under that wait. The table in the description lists who is killed in each case.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Essentials
• Run ID: 5b8e91ae-0de3-443e-8c66-c4558d32f66e

📥 Commits

Reviewing files that changed from the base of the PR and between 772f87f and a189309.

📒 Files selected for processing (6)

• docs/test/writing-tests.mdx
• src/jsc/ProcessAutoKiller.rs
• src/runtime/test_runner/Execution.rs
• test/cli/test/process-kill-fixture-sync.ts
• test/cli/test/process-kill-fixture.ts
• test/cli/test/test-timeout-behavior.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

Walkthrough

ProcessAutoKiller now tracks spawned processes by scope. The test runner starts a scope for each callback and uses scoped cleanup for eligible timeouts. Documentation and tests describe and check which processes are cleaned up.

Changes

Test timeout process cleanup

Layer / File(s)	Summary
Track processes by scope src/jsc/ProcessAutoKiller.rs	ProcessAutoKiller associates each tracked process with a scope. It adds begin_scope and kill_scope; full cleanup still drains all tracked processes.
Apply scoped cleanup to timeouts src/runtime/test_runner/Execution.rs	Execution begins a scope before each callback and skips timeout cleanup when the active entry already has a queued result. For a timed-out active entry in a single-sequence group, cleanup targets the current scope.
Document and test cleanup boundaries test/cli/test/*process-kill-fixture*.ts, test/cli/test/test-timeout-behavior.test.ts, docs/test/writing-tests.mdx	Timeout tests cover process ownership across tests and hooks, retries, synchronous spawns, exited children, and callbacks that return after deadlines. Fixture commands use process.execPath without the harness environment. The documentation describes cleanup limits.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to a1893

The scoped timeout cleanup is ready to merge after normal checks; the examined synchronous-spawn path does not expose a process-ownership failure.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Linked Issues check		Direct issue [#39657] requires cleanup for a completed concurrent test group, while preserving a still-running sibling. Execution::kill_dangling_processes_on_timeout still skips cleanup for groups w…	Implement the remaining #39657 behavior: record concurrent-group timeouts and kill the shared scope after the group has no running sequences; invoke scoped cleanup when evaluate_timeout detects a synchronous overrun; and provide the requi…

Passed checks: 3 passed

Check name	Status	Explanation
Out of Scope Changes check		The changes stay within the timeout-cleanup objective. ProcessAutoKiller.rs adds scoped tracking, Execution.rs starts callback scopes and performs scoped cleanup, and the tests cover test and hook…
Title check		The title clearly and concisely states the main change: timeout cleanup now kills only processes spawned by the timed-out test or hook callback.
Description check		The description is complete and relevant. It clearly explains what the PR changes and provides extensive verification details, including tests, builds, regressions, and known limitations. It does not …

Full details: Out of Scope Changes check

Explanation

The changes stay within the timeout-cleanup objective. ProcessAutoKiller.rs adds scoped tracking, Execution.rs starts callback scopes and performs scoped cleanup, and the tests cover test and hook timeouts, retries, synchronous spawn waits, and --isolate. The fixture and documentation changes support those tests and document the behavior. No unrelated change is established.

Full details: Description check

Explanation

The description is complete and relevant. It clearly explains what the PR changes and provides extensive verification details, including tests, builds, regressions, and known limitations. It does not use the template headings exactly, but it covers both required sections.

Full details: Linked Issues check

Explanation

Direct issue [#39657] requires cleanup for a completed concurrent test group, while preserving a still-running sibling. Execution::kill_dangling_processes_on_timeout still skips cleanup for groups with multiple sequences, as the added tests and PR description state. The issue also requires cleanup for synchronous timeout overruns and stale-timer timeouts. This PR does not add the synchronous-return kill and leaves the stale-timer case to #44613. The individual callback scope cleanup in ProcessAutoKiller.rs and Execution.rs satisfies only the narrower part of the issue.

Resolution

Implement the remaining #39657 behavior: record concurrent-group timeouts and kill the shared scope after the group has no running sequences; invoke scoped cleanup when evaluate_timeout detects a synchronous overrun; and provide the required stale-timer behavior or include the required dependency and evidence. Add or retain automated tests for these cases.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[robobun]

The review found no actionable comments. One correction to the linked-issues check: #39657 is an alternative pull request for the same bug, not an issue whose list this PR must complete. The three behaviors it names are left out on purpose.

• Concurrent groups. bun test: kill only the timed-out test's processes, and a concurrent group's once it is done #39657 kills the shared scope of a test.concurrent group when the group completes. That also signals the children of sibling tests that passed. This PR leaves concurrent groups as they are on main.
• Synchronous overrun. bun test: interrupt synchronous infinite loops on --timeout #30598 adds the call at the synchronous return by name. After this PR, that call gets the scoped kill.
• Stale timer during a test with no timeout. fix(test): preserve unlimited test children after stale deadlines #44613 carries that guard. This PR does not touch that line, and the two merge in either order.

The notes in the description have the probe for each case.

[claude]

Beyond the inline findings (all about which children a timeout should reap), I also checked the memory-safety side of the refactor: kill_scope walks the map backwards so swap_remove_at cannot skip an entry, every removed key is deref'd exactly once in kill_and_release, and clear/Drop are unchanged, so the ref taken on insert stays balanced. The raw-pointer read of result_queue in active_entry_reported never materializes a &BunTest and the scan does not mutate the queue, so it does not alias &mut self.

Extended reasoning...

The change scopes bun test's timeout kill to the processes spawned by the timed-out callback (ProcessAutoKiller gains a per-entry scope counter and kill_scope; Execution.rs begins a scope per callback and skips the kill once the entry has reported) and adds ~350 lines of subprocess tests plus a docs paragraph. It touches no security-sensitive surface. The posted findings are a behavioral tradeoff (hook timeouts and --bail now leave earlier scopes' children running) that the PR itself flags as needing a maintainer decision, so a human must weigh it; the refcount and aliasing mechanics of the new unsafe code were examined and hold.

Findings marked 🟡 are optional suggestions and need no follow-up push.

[claude]

The new commits address the three points from my earlier review (afterEach/afterAll timeouts now reap via kill_since/kill, both --bail exit sites drain the tracked set, and the late-return test holds the thread with a loop instead of a wall-clock sleepSync). I found no bugs on this pass, but because this changes the runner's process-reaping policy and the description itself asks for a maintainer decision on the hook rules, a human look is still worthwhile.

What was reviewed:

• kill_since arithmetic and the backwards swap_remove_at walk: entries stamped == mark are kept, kill_scope reduces to span 1, and Process::kill is a plain send_signal so nothing re-enters the map mid-walk; refs are released once per removed entry on every path.
• active_entry_reported: result_queue is reached through the impl_field_parent! raw pointer only, peek_item copies, and the spawnSync wait loop cannot see a queued result for the callback still on the stack.
• mark_after_all is applied per describe, so an inner describe's afterAll timeout also kills children of the outer beforeAll (and module-scope children under --isolate) that the outer afterAll was still going to stop; this matches the new docs sentence but is a policy choice worth confirming.

Extended reasoning...

The diff scopes bun test's timeout process-killing to the timed-out callback's own spawns via a u32 scope counter in ProcessAutoKiller.rs, with wider kills for afterEach (kill_since sequence mark), afterAll (kill all), and --bail; it touches unsafe Rust in the test runner (raw parent-pointer read of result_queue, arena entry pointer walks) and the --bail exit paths. No security-sensitive surface is touched. The bug hunt ran dry and my own reading of the ref-count paths, the wrapping arithmetic, and the swap-remove walk found them balanced and correct, and the new tests cover each kill rule including the swap-remove ordering case. Deferring rather than approving because the change redefines user-visible reaping behavior for hook timeouts, including a nested-describe afterAll case that kills outer-scope children, which the author explicitly flags as needing a maintainer decision.

[claude]

The three points from the earlier rounds (after-hook timeouts sparing earlier scopes, --bail leaving beforeAll children, and the wall-clock-dependent sleepSync in the late-return case) are addressed by the new commits. Beyond the inline findings, I also checked kill_since in src/jsc/ProcessAutoKiller.rs for the swap-remove-while-iterating hazard and refcount balance: it walks backwards so the swapped-in entry was already visited, the wrapping scope - value < span test is correct for kill_scope (span 1) and for marks older than the current scope, and kill_and_release derefs on both the exited and killed paths.

Extended reasoning...

The change rescopes bun test's timeout process-kill from the whole per-VM set to per-callback scopes across ProcessAutoKiller.rs, Execution.rs, Order.rs, bun_test.rs and test_command.rs, with no security-sensitive surface. Two confirmed inline findings remain (retry restart re-reading auto_kill_mark, and nested-describe afterAll timeouts draining the whole set), so a human decision on the intended hook-timeout semantics is still needed.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I re-checked the two points from my last review against the latest push: the retry case is covered now that the mark lives on ConcurrentGroup (set once in step_group when executing flips, and reset_sequence does not touch it, so attempt-1 scopes stay above the mark), and a nested describe's afterAll timeout now goes through scope_start_mark instead of kill(). I also walked kill_since's wrapping arithmetic: it removes exactly the scopes in (mark, current], and kill_scope is the mark = current - 1 case, with the backward walk keeping swap_remove_at from skipping entries.

Extended reasoning...

The latest commits move the kill call from handle_timeout into step_sequence_one after evaluate_timeout, record the auto-kill mark per concurrent group, and add scope_start_mark/after_all_scope for afterAll timeouts; both earlier concerns I raised (retry attempts and nested-describe afterAll) are addressed in code. No security-sensitive surface is touched. One nit finding remains from this run, and the change is a non-trivial rewrite of process-reaping semantics in the test runner, so a human look is still appropriate.

[robobun]

A question for the maintainer about the signal that a timeout sends, before more is built on this PR.

This PR changes which processes a timeout signals. It keeps the one SIGTERM. On main, a child that ignores or handles SIGTERM keeps running after killed 1 dangling process is printed, and also after bun test exits (3 of 3 runs with sh -c "trap '' TERM; exec sleep N").

Which answer do you want for Bun.spawn children?

1. SIGKILL at once.
2. SIGTERM, then SIGKILL 500 ms later to what still runs.
3. Keep one SIGTERM, and say sent SIGTERM to N in the message and in the docs.

For 1 and 2 there is a second choice: the spawned pid, or its process tree.

The data and a worked version of answer 2 are in the Notes of #45792. That PR does only the spawnSync part, and it does not touch the files of this PR. The Bun.spawn part would be a small change on top of this PR, after the answer.

[claude]

The earlier points about the retry restart re-reading the mark and a nested describe's afterAll draining the whole set are addressed by the per-group auto_kill_mark and scope_start_mark. Beyond the inline findings, I also checked the refcount balance in kill/kill_since/kill_and_release (each removed entry derefs exactly once; clear and Drop unchanged) and the wrapping scope arithmetic (kill_scope hits only the current scope, kill_since(mark) hits exactly the scopes after mark), and found no issue there.

Extended reasoning...

The change scopes the test runner's dangling-process auto-killer in src/jsc/ProcessAutoKiller.rs and src/runtime/test_runner/Execution.rs, moves the timeout kill from the timer callback into the runner's step, and adds a whole-set kill at both --bail exit sites in src/runtime/cli/test_command.rs. It touches no security-sensitive surface. Two findings from this run are posted inline, including a possible run-level hang when a test waits in place on a child owned by an earlier scope, so a human should weigh those before merging.

Findings marked 🟡 are optional suggestions and need no follow-up push.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

[claude]

🟡 nit (optional): maintainers get two fixtures that spawn Bun without the harness bunExe/bunEnv the test guide requires; the base branch used them. test/cli/test/process-kill-fixture.ts:5 and process-kill-fixture-sync.ts:5 now spawn process.execPath with the inherited env. Fix: keep the harness convention for every Bun spawn in both fixtures (or state in a one-line comment why the harness import is avoided here, e.g. the hooks harness.ts registers would change the kill-scope accounting), while keeping the test's cwd: import.meta.dir so no bunfig preload runs. Same pattern at 2 sites (test/cli/test/process-kill-fixture.ts:5, test/cli/test/process-kill-fixture-sync.ts:5).

Why this was flagged

test/CLAUDE.md:28 says that when spawning Bun processes in tests, use bunExe and bunEnv from harness so the same build is used and debug logging is silenced. The diff removes import { bunEnv, bunExe } from "harness" and the env: bunEnv option from test/cli/test/process-kill-fixture.ts:5-10 and test/cli/test/process-kill-fixture-sync.ts:5-9, replacing bunExe() with process.execPath. The behaviour is equivalent today only because the outer spawn at test/cli/test/test-timeout-behavior.test.ts:20 passes env: bunEnv, which the fixture inherits, and because the fixture runs under the same binary; nothing in the fixture states that dependency. The base branch followed the convention. No user-visible failure; this is a convention slip flagged because CLAUDE.md violations are in scope for this review.

Verification: nit. Trigger: any maintainer reading or copying these fixtures; nothing breaks at runtime. The diff removes import { bunEnv, bunExe } from "harness" from both test/cli/test/process-kill-fixture.ts and test/cli/test/process-kill-fixture-sync.ts, replacing bunExe() with process.execPath. test/CLAUDE.md:28 requires bunExe and bunEnv from harness, and the base branch followed it.