test(inspector): park the inspectee at a second debugger statement before it exits

[robobun]

Problem

• test/cli/inspect/bun-inspector-protocol.test.ts is the test most often flagged as flaky in CI right now: it failed once and passed on retry in 221 of the last 400 builds, on every Linux lane, almost always in the parallel batch. The failure is WebSocket closed (1006) (inspectee exit: 1) or inspectee exited (inspectee exit: 1) with the fixture's error: reported in the inspectee's stderr.
• The fixture calls reportError(). Bun counts that as an unhandled error (VirtualMachine::uncaught_exception), and is_event_loop_alive_excluding_immediates (src/jsc/VirtualMachine.rs:1206) returns false once unhandled_error_counter is set, so the inspectee exits with code 1 as soon as the module body finishes. The setInterval in the fixture did not keep it alive.
• The inspector writes its messages from the debugger thread (BunInspectorConnection::sendMessageToDebuggerThread, src/jsc/bindings/BunDebugger.cpp). After Debugger.resume the inspectee runs to the end of the module and exits. Under load it is gone before the response to Debugger.resume and the Debugger.resumed event reach the socket. An instrumented copy of the test confirmed this: in 58 failures out of 300 runs under load, every one was waiting for one of those two messages.

Fix

• The fixture has a second debugger statement. After the test resumes the first pause, the inspectee pauses again at once, so it is still alive while the debugger thread writes the resume messages. The test waits for that second Debugger.paused event, then closes the socket. Closing the last connection resumes the inspectee, which then exits as before.
• The setInterval line is gone. It had no effect, see above.
• Verified: bun bd test test/cli/inspect/bun-inspector-protocol.test.ts. With the release build and 12 copies of the file running in a loop: 123 failures in 480 runs before this change, 0 in 480 after. The debug build passed 30 of 30 loaded runs.

Background

• --inspect-wait makes bun start the script only after a client sends Inspector.initialized. The test enables every domain first, so the debugger statements pause the script.
• A pause runs BunInspectorConnection::runWhilePaused on the JS thread. It returns when a client resumes the script or when every connection is closed.
• Bun does not flush queued inspector messages when the process exits. LifecycleReporter.preventExit sets a flag that nothing reads. This change only removes the test's dependence on that, it does not change bun.

---

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

passes on PR (with fix)

Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/cli/inspect/bun-inspector-protocol.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/cli/inspect/bun-inspector-protocol.test.ts
bun test v1.4.0 (4c689909e)

test/cli/inspect/bun-inspector-protocol.test.ts:
(pass) the protocol snapshot in packages/bun-inspector-protocol matches what bun sends [965.22ms]

 1 pass
 0 fail
 4 expect() calls
Ran 1 test across 1 file. [3.28s]
Exit: 0

diff hotspot

test/cli/inspect/bun-inspector-protocol.test.ts | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file

file                                             reads  edits  tests
test/cli/inspect/bun-inspector-protocol.test.ts      1      2      0

[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: 99fc2e31-3b86-4d52-b568-8c0c6f69bc2f

📥 Commits

Reviewing files that changed from the base of the PR and between cfa9f8e and e20f531.

📒 Files selected for processing (1)

• test/cli/inspect/bun-inspector-protocol.test.ts

Included review availability: Your plan provides up to 5 included reviews per hour; 1 remains after this review.

---

Walkthrough

The inspector protocol test replaces the interval-based keepalive with a second debugger statement. After resuming, it waits for the second Debugger.paused event before closing the final WebSocket connection.

Changes

Inspector lifecycle validation

Layer / File(s)	Summary
Pause, resume, and connection cleanup test/cli/inspect/bun-inspector-protocol.test.ts	The fixture parks the inspectee at a second debugger statement. The test waits for the resulting Debugger.paused event and documents final-connection cleanup behavior.

Possibly related PRs

• oven-sh/bun#39110: Introduces the inspector conformance test updated by this pull request.

Suggested reviewers: alii

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly identifies the test change and explains that the second debugger statement prevents premature inspectee exit.
Description check	✅ Passed	The description explains the problem, fix, background, and verification results, although it does not use the template headings.

---

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

[claude]

LGTM — well-diagnosed de-flake that parks the inspectee at a second debugger statement so the debugger thread can finish writing the resume messages before the process exits.

What was reviewed:

• Confirmed pausedAgain is registered before Debugger.resume is sent, so the second Debugger.paused cannot race past the waiter.
• Checked that overwriting the Debugger.paused entry in eventWaiters is safe — the first waiter already resolved before the new one is set.
• Verified the post-close failWith from the close listener is harmless (swallowed by failed.catch), same as before this change.

Extended reasoning...

Overview

This is a test-only change to test/cli/inspect/bun-inspector-protocol.test.ts, currently the flakiest test in CI (221/400 builds). The fixture's setInterval is replaced with a second debugger statement, and the test now awaits the second Debugger.paused event before closing the socket. Two explanatory comments are added. No production code is touched.

Security risks

None. This modifies only a test fixture and the test's await sequence.

Level of scrutiny

Low-to-moderate. It's a de-flake of a single test file with a thoroughly documented root cause: reportError() sets unhandled_error_counter, which makes is_event_loop_alive_excluding_immediates return false, so the inspectee exits immediately after the module body finishes regardless of setInterval. The inspector writes from a separate debugger thread, so under load the process can be gone before the Debugger.resume response and Debugger.resumed event reach the socket. Parking at a second debugger statement guarantees the process is alive until the test has received those messages. The PR author verified 0/480 failures under load after the change vs 123/480 before.

Other factors

• The waitForEvent map is keyed by method name; I checked that registering pausedAgain for Debugger.paused correctly overwrites the (already-resolved) first waiter, and that both resumed and pausedAgain are registered synchronously before send("Debugger.resume"), eliminating any registration race.
• The removed setInterval was demonstrably not keeping the process alive (per the root-cause analysis), so removing it is correct cleanup rather than a behavior change.
• The finally block's ws.close() triggers the close listener → failWith → fail(...), but failed.catch(() => {}) already swallows that and all try-block awaits have completed — unchanged from before.
• The change follows the repo's de-flaking guidance: it awaits an actual observable condition (the second pause) rather than adding a sleep or raising a timeout, and the comment explains why no simpler signal exists.