[robobun]

test/js/bun/spawn/spawn-maxbuf.test.ts has been whole-file [ FLAKY ] in test/expectations.txt, hiding all 16 maxBuffer/timeout cases on every CI lane. Probed in build 87834 it passes every release and release-asan lane cleanly, so the quarantine is stale for CI. What it was hiding is a debug-only false positive: under bun bd the two yes is killed cases fail deterministically.

Repro

$ bun bd test test/js/bun/spawn/spawn-maxbuf.test.ts -t "yes is killed"
(fail) yes is killed > Bun.spawn
  error: expect(received).toBeLessThan(expected)
  Expected: < 100
  Received: 170
(fail) yes is killed > Bun.spawnSync
  error: expect(received).toBeLessThan(expected)
  Expected: < 100
  Received: 162

Cause

Both tests assert Date.now() from before spawn() to after exited is under 100ms, with the comment "make sure it's not waiting a full tick". The child is [bunExe(), "exec", "yes"], so the measured window includes the child bun process reaching main() before yes can write its first byte. On a debug+ASAN build that startup alone is ~150-250ms:

$ bun bd -e 'const t=Date.now(); Bun.spawnSync([process.execPath,"-e",""]); console.log(Date.now()-t)'
185

so the 100ms budget is exhausted before maxBuffer has anything to react to. Release and release-asan start in <15ms and stay well inside the bound.

Fix

Widen the wall-clock window to isDebug ? 1000 : 100, matching the pattern used elsewhere for timing budgets that include a debug-build spawn (e.g. fetch-tls-abortsignal-timeout.test.ts, buffer-indexof-worstcase.test.ts). Release keeps the original 100ms bound unchanged. Promptness of the kill itself is already asserted byte-precisely by the "caps the buffer while the child is still writing" tests (stdout.length <= maxBuffer + 64*1024 against an 8MB firehose), so this assertion is only the coarse "not deferred by seconds" sanity check.

Drop the whole-file quarantine line.

Verification

$ bun bd test test/js/bun/spawn/spawn-maxbuf.test.ts
 16 pass
 0 fail

Five consecutive bun bd runs: yes is killed at 128-157ms (< 1000). Release: 2-11ms (< 100).

Test-only change; no runtime behaviour is touched.

---

no test proof · iteration 0 · test-only change (timing threshold + unquarantine), no src/ change to stash

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d0ca4433-435c-4482-bff9-0e813eb04bfc

📥 Commits

Reviewing files that changed from the base of the PR and between fdbaf06 and c41ec15.

📒 Files selected for processing (2)

• test/expectations.txt
• test/js/bun/spawn/spawn-maxbuf.test.ts

💤 Files with no reviewable changes (1)

• test/expectations.txt

---

Walkthrough

The spawn max-buffer test now uses a longer termination window for debug builds and a shorter window for release builds. The test is also removed from the quarantine list.

Changes

Spawn max-buffer test

Layer / File(s)	Summary
Build-dependent termination timing test/js/bun/spawn/spawn-maxbuf.test.ts, test/expectations.txt	The test imports isDebug, defines a conditional killWindow, and applies it to asynchronous and synchronous timing assertions. The quarantine entry is removed.

Possibly related PRs

• oven-sh/bun#33622: Both PRs adjust spawn-maxbuf.test.ts timing for debug-build slowness.
• oven-sh/bun#34166: Both PRs adjust test timing for debug and CI startup races.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the debug timing-budget change and removal of the flaky-test quarantine.
Description check	✅ Passed	The description explains the change, cause, fix, and verification results, covering the required template information.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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

[claude]

LGTM — test-only de-flake that branches the timing budget on isDebug and restores 16 quarantined cases to CI.

What was reviewed:

• isDebug is exported from harness.ts and the branch-per-build-type pattern matches existing usage; release keeps the original 100ms bound.
• The property under test (kill isn't deferred by a full tick) is preserved; byte-level promptness is separately covered by the "caps the buffer" tests, so widening the coarse wall-clock check on debug loses no precision.
• Checked the other timing assertions in the file (timeout kills at <200ms, timeout Infinity at <1500ms) — those are parent-side timers not gated on child startup writing bytes, and the PR's bun bd run confirms all 16 pass on debug.

Extended reasoning...

Overview

Two-file test-only change: (1) test/js/bun/spawn/spawn-maxbuf.test.ts widens the yes is killed wall-clock assertion from a hardcoded 100ms to isDebug ? 1000 : 100, replacing a // TODO with a comment explaining why; (2) test/expectations.txt drops the whole-file [ FLAKY ] quarantine that was hiding all 16 maxBuffer/timeout cases on every CI lane.

Security risks

None. No runtime code is touched; this only adjusts a test timing threshold and removes a quarantine entry.

Level of scrutiny

Low. This is the canonical de-flake pattern the repo's review guidance calls for: "When de-flaking, keep asserting the property the original assertion protected — branch per-platform rather than dropping precision." The release bound is unchanged at 100ms, and the debug-only widening is well-justified (the measured window includes a debug+ASAN child bun reaching main(), ~150-250ms on its own). The added comment names the mechanism and points to the byte-precise sibling tests that guard actual promptness.

Other factors

• The PR description shows a probe build (87834) passing cleanly on release and release-asan, five consecutive bun bd runs at 128-157ms (well under 1000), and release at 2-11ms (well under 100), so the new bounds have headroom on both sides.
• Unquarantining is a net coverage gain — the file had been entirely skipped on all lanes.
• I spot-checked the remaining timing assertions in the file for the same debug-startup hazard: the timeout: 100 / < 200 cases fire a parent-side timer and send the signal regardless of child readiness (default SIGHUP disposition terminates during startup), and the bun bd 16-pass verification confirms they hold.
• No prior reviews or comments on the timeline.

[robobun]

Follow-up: #36792 fixes the remaining timeout kills the process > Bun.spawnSync boundary flake (toBeGreaterThan(100) receiving exactly 100) that surfaced once this file was unquarantined.