repl: interrupt a running evaluation with Ctrl+C

[robobun]

Repro

$ bun repl
> while (true) {}
^C ^C            # nothing. the process has to be killed from another terminal,
                 # and it leaves termios in raw mode

Node interrupts with ERR_SCRIPT_EXECUTION_INTERRUPTED and keeps going.

A second, quieter one: a single Ctrl+C during an await leaves the session
silently broken.

$ bun repl
> await new Promise(() => {})
^C
> await Promise.resolve(42)    # never prints 42, and never will again

Cause

The REPL keeps the terminal in raw mode while it evaluates. Raw mode clears
ISIG, so Ctrl+C is not a signal, it is byte 0x03 sitting in the tty input
queue with nobody reading it. Nothing can unwind the running script.

Node's REPL hands the terminal back to the line discipline for the duration of a
command (ISIG/ICANON/ECHO all back on), so Ctrl+C arrives as a real SIGINT,
and runInThisContext({ breakOnSigint: true }) turns that into a V8 termination.

The await case went through Repl.enable_signals_during_wait, whose handler
called JSC::VM::setExecutionForbidden(). That flag is one-way (JSC has no API
to clear it), and VM::drainMicrotasks then clears the microtask queue on every
tick, so the session never runs another microtask.

Fix

begin_interruptible_eval puts the terminal back in cooked mode and arms the
SigintWatcher already used by node:vm's breakOnSigint. The watcher thread
calls VM::notifyNeedTermination(), which raises a VMTraps::NeedTermination
trap at the next safepoint, so while (true) {} unwinds. The REPL then reports
ERR_SCRIPT_EXECUTION_INTERRUPTED and returns to the prompt, exactly as node
does. Windows already sets ENABLE_PROCESSED_INPUT, so only the watcher is
needed there.

One wrinkle worth recording: VM::executeEntryScopeServicesOnExit() clears
hasTerminationRequest() once the outermost entry scope unwinds, and the trap
bit is cleared by handleTraps() before that. Neither survives the return from
JSC::evaluate, so the durable record of the signal is the SigintReceiver
flag, which the watcher sets before raising the trap. That flag is written by
the watcher thread and read by the VM thread, so it is now std::atomic<bool>
(node:vm was reading it across threads too).

The setExecutionForbidden() path is gone. The promise wait now polls the
receiver flag and breaks out on it, leaving the VM able to run JavaScript.

evaluate_and_print and evaluate_and_copy had a duplicated copy of the
evaluate/await/unwrap sequence; they now share evaluate_transformed, which is
where the interrupt handling lives.

Who owns the SIGINT disposition

Arming the watcher around every evaluation also puts it in conflict with
BunProcess, which installs its own forwardSignal handler for
process.on("SIGINT"). Both sides refuse to uninstall a handler they did not
install, and that guard leaves the disposition wrong in both directions.

Adding a listener, sigaction(SIGINT, forwardSignal) displaces the watcher's
handler while it is armed, so Ctrl+C is downgraded to a JS event that the
loop it is meant to interrupt never drains:

> process.on("SIGINT", () => {}); while (true) {}
^C            # nothing, same as before this PR

Removing the last one, signal(SIGINT, SIG_DFL) finds the watcher's handler,
does not recognise it, and reinstates it. The disarm then sees its own handler
still installed and restores the snapshot it took at arm time, which is the
forwarder BunProcess just asked to drop, so an external kill -INT is
silently swallowed from then on.

While the watcher is armed it owns the disposition, so BunProcess hands it the
action to apply on disarm (SigintWatcher::deferSigintDisposition) instead of
installing over its handler. With nothing armed the method returns false on its
first line and BunProcess is unchanged, which is every path outside node:vm
and the REPL.

One more ordering bug fell out of the same audit. uninstall() cleared
m_installed while its own handler was still the disposition, so a signal in
that window ran the handler, posted the semaphore, and the watcher thread bailed
on if (!m_installed) return; before forwarding it. Nobody saw the signal, and
m_waiting stayed set, which deafens the watcher from then on. The disposition
now goes back first. With the window artificially widened to 50ms, 30 signals
land somewhere accounted for (fired + interrupted == 30) on every run; with the
old ordering two vanished outright in the run that finished and the other two
runs never reached the fixture's exit path.

Tests

The new REPL tests drive a real pty. They needed one harness change: handing
Bun.spawn an already-created Bun.Terminal skips the setsid() +
ioctl(TIOCSCTTY) the inline terminal: {...} option performs, so the child had
no controlling terminal (tty_nr: 0, tpgid: -1) and the line discipline had no
foreground process group to signal. withTerminalRepl now uses the inline form.
Its waitFor also gained the timeout its deadline always implied: it used to
sleep forever when the child went quiet, reporting a bare test timeout instead of
the captured output.

git checkout main -- src/ && bun bd test test/js/bun/repl/repl.test.ts \
  -t "Ctrl\+C interrupts|never-settling await"
  → 2 fail   "Timed out waiting for pattern: /interrupted/"

bun bd test test/js/bun/repl/repl.test.ts
  → 125 pass, 0 fail
bun bd test test/js/node/vm/vm.test.ts
  → 210 pass, 0 fail

Four of the REPL tests pin the SIGINT disposition, one per reachable state: no
listener (an external SIGINT terminates), a listener added (it does not), the
listener then removed (it terminates again), and a listener added in front of the
loop Ctrl+C has to break out of. The last two fail with deferSigintDisposition
stubbed to return false on the same build.

node:vm's sigint/timeout suites (test-vm-sigint.js,
test-vm-sigint-existing-handler.js, test-vm-timeout.js,
test-vm-timeout-escape-promise.js, test/js/node/vm/vm.test.ts) still pass,
as does test/js/bun/terminal/. bun run rust:check-all is clean on all 10
targets.

On Windows the watcher goes through SetConsoleCtrlHandler, and the REPL's raw
mode already sets ENABLE_PROCESSED_INPUT, so Ctrl+C was already arriving as a
console control event and only the watcher was missing. That path is untested
here: the terminal REPL suite is describe.todoIf(isWindows).

Bugs this surfaces in node:vm

Arming the watcher around every REPL evaluation makes the REPL an outer
SigintWatcher holder, which reaches code nothing reached before.

require('vm').runInThisContext('while(true){}') + Ctrl+C aborted the
process: the inner script's termination hit checkForTermination with neither
a SIGINT flag of its own nor a timeout and fell into
RELEASE_ASSERT_NOT_REACHED("vm.Script terminated due neither to SIGINT nor to timeout").

A nested script can't classify a termination it didn't request; it only knows
its own SIGINT flag and its own timeout. So when neither is set it now re-raises
the termination instead of converting it, and the enclosing scope picks the right
branch with its own limit and receiver flag. That matters beyond the REPL: an
outer runInThisContext({ timeout: N }) firing while a nested no-options script
is on the stack involves no signal at all, and node reports
ERR_SCRIPT_EXECUTION_TIMEOUT there.

const vm = require("vm");
globalThis.inner = () => vm.runInThisContext("while (true) {}");
vm.runInThisContext("inner()", { timeout: 100 });
// before this PR: abort (RELEASE_ASSERT)
// first attempt:  ERR_SCRIPT_EXECUTION_INTERRUPTED, "interrupted by `SIGINT`"
// now, and node:  ERR_SCRIPT_EXECUTION_TIMEOUT,     "timed out after 100ms"

NodeVMModule had the identical shape.

require('vm').runInThisContext('1', {breakOnSigint:true}); while(true){} +
Ctrl+C hung: registerGlobalObject used appendIfNotContains while
unregisterGlobalObject removes one entry unconditionally, so the inner holder
carried the outer holder's registration out with it. Appending unconditionally
balances the two; signalAll tolerates duplicates.

A third one, pre-existing rather than surfaced, shares a line with the above.
drainMicrotasksForGlobalObject clears rather than drains, so a terminated
script may only clear the queue of the context it ran in. runInThisContext has
no context of its own and checkForTermination was handing it the caller's
global, discarding everything the caller had parked:

const { promise, resolve } = Promise.withResolvers();
const chain = promise.then(() => "survived");
resolve();
try { vm.runInThisContext("while (true) {}", { timeout: 100 }); } catch {}
chain.then(v => console.log(v));   // never prints; node prints "survived"

The contextified global is now a parameter of its own: runInContext passes its
NodeVMGlobalObject, runInThisContext passes null. clearForGlobalObject
early-returns on null, so that path clears nothing.

Known gap: nested vm timeout classification

The re-raise guard asks whether this scope configured a timeout, not whether its
watchdog actually fired. So an inner runInThisContext(src, { timeout: N }) with
no breakOnSigint, terminated by an enclosing SIGINT before N elapses, still
reports its own ERR_SCRIPT_EXECUTION_TIMEOUT.

Pre-existing: the old else if (timeout) was byte-identical, and the non-REPL
shape (outer breakOnSigint + inner timeout) already behaved this way. The REPL
prints the right thing regardless, because take_interrupt_error supersedes off
the sticky receiver flag; it only leaks to a caller that wraps the nested call in
its own try/catch.

Not fixed here because the obvious discriminator is unsafe: comparing a monotonic
deadline would, on a false "didn't fire" at the outermost scope, re-raise an
uncatchable TerminationException into user JS instead of the catchable
ERR_SCRIPT_EXECUTION_TIMEOUT that test-vm-timeout.js asserts on. Doing it right
means tracking fired-state the way node's Watchdog carries timed_out_, and
setupWatchdog's enteredVM() already resets an enclosing deadline, and that
bookkeeping wants its own change.

Known gap: a parked event loop

wait_for_promise_interruptible is only prompt because an idle loop polls
without blocking. Once something keeps the loop alive it parks in
epoll_pwait2/kevent64, and every path in us_loop_run_bun_tick re-enters
the poll on EINTR, so a signal alone never returns from it. A Ctrl+C landing
after the park waits for whatever wakes the loop next.

This is not a regression (the old sigint_handler → wait_for_promise →
auto_tick path had the same gap), and the while (true) {} case in #27558 is
unaffected because that thread is running JS, not parked. Closing it properly
means waking the loop from the watcher thread the way
WebWorker__notifyNeedTermination does: notify_need_termination() followed by
event_loop().wakeup(). That can't be done blindly in signalAll(), because
NodeVMGlobalObject derives from Bun::GlobalScope, not Zig::GlobalObject, so
it has no bunVM() to reach a loop through. Left for a follow-up rather than
bolted on here. The never-settling-await test is written against the idle loop so
it is deterministic rather than racing the park.

Not in scope

The related complaint that the JS event loop does not run while the REPL waits
at the prompt (timers, servers and fetches frozen between commands) is already
covered by #30560, so this PR leaves read_byte alone.

#33237 (Bun.spawn({ terminal }) gives the child no controlling terminal) is a
real bug that this PR ran into, but does not fix: the spawn path is untouched,
only the REPL's own test harness moves to the inline terminal: {...} option,
which already does the setsid() + ioctl(TIOCSCTTY). Confirmed while
debugging, for whoever picks it up: with a pre-created Bun.Terminal the child
comes up tty_nr: 0 / tpgid: -1, with the inline option it gets the pty.

Fixes #27558

---

no test proof · iteration 8 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/repl/repl.test.ts

[coderabbitai]

Walkthrough

This PR replaces the REPL’s Unix SIGINT path with per-evaluation SigintWatcher-based interruption, updates VM and module termination handling, removes the unused execution-forbidden binding, and adds atomic SIGINT state plus expanded REPL and VM coverage.

Changes

REPL SIGINT interruption and termination classification

Layer / File(s)	Summary
Remove setExecutionForbidden binding src/jsc/VM.rs, src/jsc/bindings/bindings.cpp, src/jsc/bindings/headers.h	Removes the JSC__VM__setExecutionForbidden FFI declaration, Rust method, exported binding, and header declaration.
SigintReceiver and SigintWatcher state handling src/jsc/bindings/vm/SigintReceiver.h, src/jsc/bindings/vm/SigintWatcher.h, src/jsc/bindings/vm/SigintWatcher.cpp, src/jsc/bindings/BunProcess.cpp	Makes SIGINT receipt atomic, stores and restores the previous SIGINT handler, defers SIGINT disposition during signal-handler updates, and allows duplicate receiver/object registration entries.
VM module and script termination classification src/jsc/bindings/NodeVMModule.cpp, src/jsc/bindings/NodeVMScript.cpp	Reclassifies termination so enclosing-scope requests rethrow the singleton termination exception, while owned SIGINT and timeout paths clear termination state and throw the matching execution error.
REPL SIGINT bindings src/jsc/bindings/bindings.cpp	Adds REPL SIGINT arm/disarm/poll/error-taking bindings, the termination-clearing helper, and the termination-exception short-circuit in Bun__REPL__evaluate.
Interruptible REPL evaluation flow src/runtime/cli/repl.rs	Replaces Unix signal handling with per-evaluation SIGINT scope management, interruptible promise waiting, transformed-evaluation outcome tracking, and updated print/copy behavior for value, error, and pending outcomes.
REPL and VM interruption tests test/js/bun/repl/repl.test.ts, test/js/node/vm/vm.test.ts	Refactors the terminal REPL harness, adds Ctrl+C interruption coverage for multiple REPL cases, and adds VM tests for nested timeout propagation and errored module re-evaluation.

Possibly related PRs

• oven-sh/bun#32018: Both PRs modify NodeVMScript.cpp and NodeVMModule.cpp termination handling, including microtask drainage and ERR_SCRIPT_EXECUTION_* error behavior.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes implement REPL Ctrl+C interruption and keep the session alive after the first interrupt, matching #27558.
Out of Scope Changes check	✅ Passed	The extra VM and SigintWatcher changes support the REPL interrupt path and regression fixes, so no unrelated scope stands out.
Title check	✅ Passed	The title is concise, specific, and accurately summarizes the main change: adding Ctrl+C interruption to the REPL evaluation flow.
Description check	✅ Passed	It covers the PR's purpose, fix, and verification, though the exact template headings are not used.

---

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

[robobun]

^{Updated 11:17 PM PT - Jul 15th, 2026}

❌ @robobun, your commit 92c7b6f has 4 failures in Build #73614 (All Failures):

• test/cli/install/bun-upgrade.test.ts - code 1 on 🍎 14 aarch64
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64-baseline
• test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan
• test/js/web/timers/timer-heap-race.test.ts - pid 9931 sigabrt on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 33411

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

bun-33411 --bun

[robobun]

Review didn't start (rate limit), so flagging that this diff hasn't had a bot pass yet.

@coderabbitai review

[coderabbitai]

@robobun, got it — kicking off the review now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[github-actions]

Found 2 issues this PR may fix:

1. bun repl: way to break infinite loop #27558 - Requests a way to break infinite loops in the REPL, which this PR directly fixes via Ctrl+C using VMTraps/SigintWatcher
2. Bun.Terminal: spawned child's controlling terminal is not the PTY (no setsid/TIOCSCTTY) — ^C never signals, /dev/tty points at the parent's tty #33237 - Reports that Bun.Terminal spawned children lack a controlling terminal (no setsid/TIOCSCTTY), so Ctrl+C never signals — this PR fixes the pty harness to use the inline terminal: spawn option which calls setsid/TIOCSCTTY

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #27558
Fixes #33237

🤖 Generated with Claude Code

[robobun]

Took the first suggestion, not the second.

#27558 — yes, added Fixes #27558. That issue asks for Ctrl+C to break out of while (1) {} instead of quitting the REPL, which is exactly this change. It was filed against Windows; the Windows path rides SigintWatcher's SetConsoleCtrlHandler and the REPL's raw mode already sets ENABLE_PROCESSED_INPUT, so only the watcher was missing there. Flagging honestly that it is untested on Windows, since the terminal REPL suite is describe.todoIf(isWindows).

#33237 — this PR does not fix it, so I left it out of the body. That issue is about Bun.spawn({ terminal }) not giving the child a controlling terminal; the spawn path here is untouched. What changed is the REPL's own test harness, which now spawns through the inline terminal: {...} option — that one already performs the setsid() + ioctl(TIOCSCTTY), so it sidesteps the gap rather than closing it.

Corroboration for whoever picks #33237 up, since I hit it head-on: with a pre-created Bun.Terminal handed to Bun.spawn, the child comes up tty_nr: 0 / tpgid: -1 and a \x03 written into the pty is only echoed. With the inline option on the same binary it comes up tty_nr: 34816 / tpgid: <child pid> and the \x03 delivers SIGINT. The fork is pty_slave_fd in js_bun_spawn_bindings.rs, which is only populated for terminal_info (newly created) and not for existing_terminal.

[robobun]

Four review findings, all landed in 07b57fe (the threads auto-resolved, so collecting the answers here).

🔴 Nested vm.runInThisContext(code) + Ctrl+C aborted the process. Confirmed under a pty before touching anything — SIGABRT, ASSERTION FAILED: vm.Script terminated due neither to SIGINT nor to timeout. The else branch in checkForTermination now throws ERR_SCRIPT_EXECUTION_INTERRUPTED instead of asserting, on the grounds that an enclosing scope's SIGINT is still a SIGINT. Same change in NodeVMModule. Re-verified: process stays alive, reports the interrupt, session keeps working.

🟡 Nested breakOnSigint disarmed the REPL's holder. Also reproduced (hang, ^C echo only). registerGlobalObject now appends unconditionally so it balances unregisterGlobalObject's single-entry removal. signalAll tolerates duplicates — notifyNeedTermination only re-sets an already-set trap bit. Both cases are now regression tests.

🟡 Dead set_execution_forbidden. Removed at all four sites (VM.rs wrapper + FFI import, the C++ shim, the headers.h decl). Getter kept, it still has the event_loop.rs:929 caller.

🟡 The auto_tick() EINTR comment was wrong. You're right, and the measurements agree. I instrumented the child's CPU ticks while it waits:

scenario	state while awaiting	Ctrl+C latency
nothing keeping the loop alive	spinning (201 ticks/2s)	101 ms
setTimeout(…, 600000)	parked (4 ticks/2s)	703 ms
Bun.serve (1s date-header timer)	parked (3 ticks/2s)	603 ms

So the parked cases only recover when something unrelated wakes the loop, exactly as you described. Two consequences: the comment now says what actually happens, and the never-settling-await test drops the 600 s timer it used to carry — that timer was what made it park and race. Against an idle loop auto_tick() doesn't block, so the flag is seen on the next iteration and the test is deterministic.

I did not close the underlying gap here. Doing it right means the notify_need_termination() + event_loop().wakeup() pairing that WebWorker__notifyNeedTermination already uses, but signalAll() can't do that blindly: NodeVMGlobalObject derives from Bun::GlobalScope, not Zig::GlobalObject, so reaching a loop through bunVM() isn't sound for every registered global. It's pre-existing (the old sigint_handler → wait_for_promise → auto_tick path had the identical swallow) and while (true) {} is unaffected, so it's written up in the PR body as a follow-up rather than bolted on here.

Unrelated find while writing the first version of the nested test: the REPL mangles a \n escape inside a single-quoted string that also contains ", e.g. require('vm').runInThisContext('process.stdout.write("a"+"b\n")') → SyntaxError: Unexpected EOF. Reproduces on release 1.4.0, so it predates this PR; the test just routes around it.

Also checked: test-vm-sigint.js, test-vm-sigint-existing-handler.js, test-vm-timeout{,-escape-promise{,-2,-module}}.js all still pass, and the 3 failures in test/js/node/vm/ reproduce identically with src/ at main.

[robobun]

Good catch on the error-code classification — fixed in 380d9522. You were right that the branch can't distinguish the two trigger classes, and I reproduced the one I had wrong:

const vm = require("vm");
globalThis.inner = () => vm.runInThisContext("while (true) {}");
vm.runInThisContext("inner()", { timeout: 100 });

	result
before this PR	abort (RELEASE_ASSERT_NOT_REACHED)
my first attempt	ERR_SCRIPT_EXECUTION_INTERRUPTED / "interrupted by `SIGINT`"
now, and node	ERR_SCRIPT_EXECUTION_TIMEOUT / "timed out after 100ms"

Took the first option you suggested. A nested script only knows its own SIGINT flag and its own timeout, so when neither is set it re-raises the termination with JSC::throwException(globalObject, scope, vm.ensureTerminationException()) and leaves hasTerminationRequest() intact. The enclosing scope's checkForTermination then classifies it with its own newLimit / receiver flag. NodeVMModule got the same treatment — the unconditional clearHasTerminationRequest() there moved below the re-raise too.

The REPL path is unaffected, exactly as you predicted: Bun__REPL__evaluate still sees a termination exception, hits its isTerminationException check, and Bun__REPL__takeSigintError reports the interrupt off the sticky receiver flag. Re-verified all three REPL shapes under a pty — plain while (true) {}, nested no-options, nested breakOnSigint — all stay alive, all report the interrupt, none abort.

Regression test added in vm.test.ts; it fails against the previous commit with precisely the wrong code:

Expected value: ObjectContaining { code: "ERR_SCRIPT_EXECUTION_TIMEOUT", ... }
Received value: error: Script execution was interrupted by `SIGINT`
 code: "ERR_SCRIPT_EXECUTION_INTERRUPTED"

test-vm-sigint{,-existing-handler}.js and test-vm-timeout{,-escape-promise{,-2,-module{.js,.mjs}}}.js all still pass; repl + vm suites are 328/328.

[robobun]

Both findings investigated. One landed, one I'm declining with evidence — and it's the interesting one.

NodeVMModule.cpp — the return {} is load-bearing, not an oversight

The predicted symptom doesn't reproduce: the module ends up errored, not stuck in evaluating. reconcileEvaluationState isn't as narrow as it looks — JSC marks a terminated CyclicModuleRecord as Evaluated with an evaluationError set, so the lazy reconcile fires and settles the status.

More importantly, I applied the suggested fall-through and it aborts:

ASSERTION FAILED: !isTerminationException(exception) || hasTerminationRequest()
vendor/WebKit/Source/JavaScriptCore/runtime/VM.cpp(1072) : void JSC::VM::setException(Exception *)

VM_RETURN_IF_EXCEPTION stores scope.exception() — which here is the VM's singleton TerminationException — into m_evaluationException. Re-evaluating the errored module re-throws it once hasTerminationRequest() has been cleared, and VM::setException asserts. reconcileEvaluationState avoids this by doing Exception::create(vm, error), wrapping the error value in a fresh object.

So the early return stays, with a comment saying why, plus a regression test that re-evaluates the errored module (the step where the re-throw happens). It exits 134 against the fall-through shape and 0 against the current one — I verified both directions rather than trusting the first version of the test, which passed either way because it never re-evaluated.

Baseline for the record: this whole input dumps core on release 1.4.0 (the RELEASE_ASSERT_NOT_REACHED), so everything here is strictly downhill from an abort.

NodeVMScript.cpp — real, pre-existing, documented as a residual gap

Agreed that the guard tests whether a timeout was configured, not whether the watchdog fired, and that an inner {timeout: N} + an outer SIGINT misclassifies. Confirming your read that it's pre-existing: the old else if (timeout) was byte-identical, so the non-REPL shape (outer breakOnSigint + inner timeout) already behaved this way.

Not fixing it here, for a concrete reason beyond scope. The obvious discriminator — comparing a monotonic deadline — is dangerous at the outermost scope: a false "didn't fire" would re-raise an uncatchable TerminationException into user JS instead of a catchable ERR_SCRIPT_EXECUTION_TIMEOUT, which is exactly what test-vm-timeout.js asserts against. Doing it properly means tracking fired-state the way node's Watchdog carries timed_out_, and setupWatchdog's enteredVM() already resets an enclosing deadline, so that bookkeeping wants its own change. Written up in the PR body as a known gap.

Worth noting the REPL's printed output is already correct for this case: the inner's bogus ERR_SCRIPT_EXECUTION_TIMEOUT is superseded by take_interrupt_error off the sticky receiver flag, so the user sees the interrupt. It only leaks to a caller that wraps the nested call in its own try/catch.

repl + vm suites are 329/329; test-vm-sigint{,-existing-handler}.js and test-vm-timeout{,-escape-promise{,-module}} all pass.

[coderabbitai]

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/bindings/bindings.cpp`:
- Around line 6273-6276: Condense the new block comments in bindings.cpp so each
stays within the repository’s 3-line maximum. Update the two REPL/termination
comments near the watcher-thread and termination-trap logic in the affected
bindings code to preserve the same meaning but fit into three lines or fewer,
including the matching comment block in the later section referenced by the
review.

In `@src/jsc/bindings/NodeVMModule.cpp`:
- Around line 248-259: Trim the explanatory comment in
NodeVMModule::reconcileEvaluationState to fit the repo’s 3-line limit by keeping
only the essential invariant and removing the extra rationale/history; preserve
the key reason for returning early instead of falling through to
VM_RETURN_IF_EXCEPTION, but move the longer explanation into the PR discussion
or surrounding docs.
- Line 264: The microtask drain in NodeVMModule should use the normalized global
object instead of nodeVmGlobalObject, since nodeVmGlobalObject may be null on
this termination path. Update the drain call in the NodeVMModule flow to use
globalObject, which already resolves to the NodeVM global when available and is
otherwise guaranteed non-null.

In `@src/jsc/bindings/NodeVMScript.cpp`:
- Around line 289-292: The comment in NodeVMScript::checkForTermination exceeds
the 3-line limit; shorten it by removing one line while preserving the invariant
and the re-raise explanation. Keep the remaining text focused on the distinction
between the script’s own SIGINT/timeout and an enclosing scope, and retain
references to checkForTermination and Bun__REPL__evaluate only if still
necessary.

In `@src/jsc/bindings/vm/SigintWatcher.cpp`:
- Around line 122-126: Shorten the explanatory comment in SigintWatcher’s append
logic to fit the 3-line limit while keeping the core rationale. Keep the key
points near the existing `unregisterGlobalObject`, `signalAll`, and
`notifyNeedTermination` behavior, but compress the nested-holder/duplicate-entry
explanation into a single concise sentence or two. Remove any extra wording so
the comment stays within the repo’s maximum comment length without changing its
meaning.

In `@src/runtime/cli/repl.rs`:
- Around line 1331-1344: The promise wait logic in `repl.rs` is holding a raw
`*mut JSPromise` across `wait_for_promise_interruptible`, which can tick the VM
and run GC, so the promise must be rooted or copied before the wait. Update the
`result.as_promise()` handling path and the related
`wait_for_promise_interruptible` / `auto_tick` flow so the `JSPromise` stays
protected for the full wait, then use the rooted handle again when checking
`status()` and `result()`. Apply the same fix to the other promise-wait block
noted in the diff so no JSValue survives beyond the current call without
rooting.
- Around line 1631-1645: The `.copy` error path in `evaluate_and_print` updates
`last_error` but leaves the REPL `_error` special variable stale, unlike the
`EvalOutcome::Error` branch. Update the `EvalOutcome::Value` branch in
`evaluate_and_print` (around `copy_value_to_clipboard`/`print_js_error`) so the
caught exception is also written to `_error` using the same mechanism as the
normal error path, keeping `_error` in sync after copy failures or rejected
awaits.
- Around line 1384-1393: The `Repl::eval` promise-wait loop is busy-spinning
when `auto_tick()` has no live event-loop work and the promise remains pending.
Update the `while pending(promise) && !Self::sigint_requested(sigint)` block so
that after `vm.as_mut().tick()`/`auto_tick()` it exits once the promise is still
pending and the loop is no longer alive, instead of continuing forever. Use the
existing `pending(promise)`, `Self::sigint_requested`, and
`vm.as_mut().auto_tick()` flow to locate and adjust the return condition.

In `@test/js/node/vm/vm.test.ts`:
- Around line 1000-1004: The crash-regression test is recording stderr too late,
so a native abort can throw before diagnostics are captured. Move the
capture(stderr) call to occur before the combined expect assertion, so the
vm.test.ts case preserves stderr alongside stdout and exitCode when the
subprocess fails. Use the existing capture(stderr) helper and the
expect({...}).toEqual(...) check in this test block as the anchors for the
change.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c432f96c-dd87-495f-9769-06724abe738e

📥 Commits

Reviewing files that changed from the base of the PR and between 6a35e49 and 1d24dce.

📒 Files selected for processing (10)

• src/jsc/VM.rs
• src/jsc/bindings/NodeVMModule.cpp
• src/jsc/bindings/NodeVMScript.cpp
• src/jsc/bindings/bindings.cpp
• src/jsc/bindings/headers.h
• src/jsc/bindings/vm/SigintReceiver.h
• src/jsc/bindings/vm/SigintWatcher.cpp
• src/runtime/cli/repl.rs
• test/js/bun/repl/repl.test.ts
• test/js/node/vm/vm.test.ts

💤 Files with no reviewable changes (2)

• src/jsc/bindings/headers.h
• src/jsc/VM.rs

[robobun]

All eleven findings handled in 435b9fb. Two of them were real bugs I'd introduced, and one of those is the most serious thing anyone has caught on this PR.

🔴 The one that mattered: an external SIGINT was being swallowed

SigintWatcher::uninstall() hardcodes Bun__onPosixSignal + SA_RESTART rather than restoring what install() displaced. Arming the watcher around every REPL evaluation made that path hot, so after the first command the disposition was permanently clobbered. Measured under a pty:

	kill -INT at the prompt	terminal afterwards
release 1.4.0	dies	restored
this PR, before the fix	swallowed, stays alive	left raw
this PR, after	dies	restored

Worth stressing that the displaced handler isn't SIG_DFL — with a TTY, c-bindings.cpp installs onExitSignal, which runs bun_restore_stdio() then re-raises. So clobbering it didn't just ignore the signal, it also took out the thing that un-wrecks your terminal. install() now saves via sigaction's third argument and uninstall() puts it back, which incidentally stops a plain runInThisContext(x, {breakOnSigint:true}) from leaking the same change process-wide. Safe to capture there: Bun__ensureSignalHandler() only allocates the signal ring, it never touches the disposition, and ref() reaches install() only on the 0→1 transition. Regression test added; it reports "still running" against the old uninstall().

🟡 registerReceiver — thanks, that's exactly the sibling site

Same appendIfNotContains-vs-unconditional-remove asymmetry as registerGlobalObject, thirty lines down, and I missed it. Fixed the same way; setSigintReceived is an idempotent atomic store so duplicates cost nothing.

The CodeRabbit set

• Root the promise across the wait — taken. tick()/auto_tick() can collect and I was holding a bare *mut JSPromise. eval_script already had the protected() precedent.
• Busy-spin on an unsettleable await — taken, and measured: await new Promise(() => {}) went from 201 CPU ticks/2s to 0. eval_script already uses is_event_loop_alive() for exactly this. Knock-on: the never-settling-await test no longer leans on that spin — it now keeps the loop alive with a 10ms interval, so the wait parks but wakes often enough to be deterministic, with no EINTR dependency. Strictly better than both the spin and the 600s-timer version it replaced.
• drainMicrotasksForGlobalObject(nodeVmGlobalObject) — taken. Every other use in the file guards for null, and line 160 already does if (nodeVmGlobalObject) globalObject = nodeVmGlobalObject, so globalObject is identical when non-null and safe when not.
• _error on .copy's failure paths — taken.
• Four over-long comments + stderr capture order — taken. The 3-line rule is in CLAUDE.md and I broke it.

repl + vm are 330/330, test-vm-sigint{,-existing-handler} and the four test-vm-timeout* all pass (they register a SIGINT listener, so the restored disposition is what they already had), rust:check-all clean on 10 targets.

[robobun]

Both landed in 15ea4c3. The first one caught a regression that my previous fix introduced, which is a nice illustration of why this area is tricky.

The m_previousAction restore was clobbering process.on('SIGINT')

Exactly as described, and measured under a pty:

	process.on('SIGINT', h) then kill -INT
release 1.4.0	REPL alive, listener honored
435b9fb	REPL dies, listener discarded
now	REPL alive, listener honored

So fixing the swallowed SIGINT by restoring a snapshot traded one bug for another: anything installed during the armed window got thrown away, and since signalToContextIdsMap keeps its entry, BunProcess never reinstalls. Stranded for the session.

Took your suggested shape — uninstall() reads the current disposition and restores m_previousAction only when the handler still installed is the watcher's own, otherwise leaves it alone. The inline lambda became a named sigintWatcherHandler so it can be compared, with an SA_SIGINFO guard since that would mean it isn't ours anyway.

What I like about this is that the two tests form a vise, so neither fix can regress the other:

	no listener → must terminate	listener → must survive
unconditional restore (435b9fb)	pass	fail ("exited" vs "alive")
hardcoded Bun__onPosixSignal (pre-435b9fb)	fail ("still running")	pass
conditional restore (now)	pass	pass

Left alone, as you noted: a SIGINT listener registered before a spin still displaces the watcher's handler for the rest of that evaluation, so process.on('SIGINT', () => {}); while (true) {} stays uninterruptible. Not a regression, and fixing it needs BunProcess and the watcher to agree on who owns the disposition.

Spawned fixture for the nested-timeout test

Taken — Bun.spawn + {stdout, exitCode}, matching the sibling test. You're right that a regression back to RELEASE_ASSERT_NOT_REACHED would otherwise take the whole vm.test.ts run down with a bare SIGABRT instead of naming the test.

On the CI red

Triaged, none of it mine:

• test-vm-module-basic.js times out on main too (hangs locally with src/ stashed, exit 124). Pre-existing; CI only surfaced it on the Windows lanes.
• test-worker-message-port-transfer-terminate.js SIGABRT on x64-asan: passes 3/3 locally under the same ASAN build.
• hot.test.ts (Windows) and complex-workspace.test.ts are both annotated ctx=flaky and passed on retry.

331/331 on repl + vm, test-vm-sigint{,-existing-handler}, the test-vm-timeout* set, and test-signal-handler.js all pass.

[robobun]

Status on the red X, since it has now recurred across several builds and none of it comes from this diff.

Both failed jobs in #68816 are the same infra hiccup, on two shards of darwin 26 aarch64 - test-bun:

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'.
Refusing to continue with a partial download (would silently fall back to the wrong binary).

Zero tests ran on either — the runner never got the binary. The same timeout took a darwin shard in #68720 and #68749; in #68720 the sibling darwin shards passed and the build reached 283 passed / 1 failed before one of my own pushes cancelled it.

The two annotations are likewise not mine:

• test/js/node/test/parallel/test-vm-module-basic.js — hangs on main too. Verified by stashing src/, rebuilding, and running it: exit 124 (timeout). Pre-existing; CI happens to surface it on the Windows lanes.
• test/js/bun/spawn/spawn-pipe-leak.test.ts — annotated ctx=flaky, passed on retry.

Earlier rounds were the same story: test-worker-message-port-transfer-terminate.js's x64-asan SIGABRT passes 3/3 locally under the same ASAN build, and hot.test.ts / complex-workspace.test.ts are both ctx=flaky.

Not pushing a retrigger: the build is still running with ~185 jobs queued, so a push would cancel a build that is passing everywhere it has reported, to re-roll the same artifact-download dice. Happy to re-roll if a maintainer would rather see a clean board.

Locally on 15ea4c3: 331/331 across repl + vm, plus test-vm-sigint{,-existing-handler}.js, the four test-vm-timeout* files, and test-signal-handler.js. bun run rust:check-all is clean on all 10 targets, clang-format and rustfmt clean.

[robobun]

Correction to my last comment: test-vm-module-basic.js was not pre-existing. It was mine. Fixed in 607092f9.

I called it pre-existing off a local repro that hung on main as well. That repro was broken — the test re-spawns itself as a child process and that respawn doesn't work in my container, so both sides hung for a reason that had nothing to do with the diff. The tell I should not have ignored: 14 lanes across every platform, while three other PRs' builds (#68811, #68814, #68818) each have 17 failures and none of them is this test.

The bug

I took the drainMicrotasksForGlobalObject(nodeVmGlobalObject) → globalObject suggestion on the reasoning that they're identical when non-null and globalObject is safe when null. The name is a trap:

void VM::drainMicrotasksForGlobalObject(JSGlobalObject* globalObject)
{
    m_defaultMicrotaskQueue->clearForGlobalObject(globalObject);
}

It doesn't drain, it clears. So the argument isn't "which global do we pump", it's "whose pending microtasks do we throw away". nodeVmGlobalObject is null on purpose when the module has no context — nothing to clear. Passing the caller's global instead discarded the main thread's microtask queue, so every parked await continuation vanished and the process wedged.

new SourceTextModule("while (true) {}") + evaluate({ timeout: 500 }) with no context is exactly that shape, and it's block 3 of test-vm-module-basic.js.

Controlled A/B, same tree, one word, rebuilt each side:

drain target	test-vm-module-basic.js
nodeVmGlobalObject (main's)	exit 0 ×3
globalObject	exit 124 ×3 — hangs

Reverted, with a comment naming the trap so nobody re-applies it, and a regression test that parks a continuation in the main queue across a terminated context-less evaluation — it fails against the globalObject target.

On the rest of the board

The remaining reds are what I described before, and those I did verify:

• 3 darwin shards: buildkite-agent artifact download timed out (zero tests ran).
• spawn-pipe-leak.test.ts: annotated ctx=flaky, passed on retry.

Now green locally: test-vm-module-basic.js 3/3, 332/332 on repl + vm, and test-vm-sigint{,-existing-handler}, the test-vm-timeout* set, and test-signal-handler.js all pass.

Lesson recorded for the next reviewer of this PR: a "Quick win" one-word suggestion on a function whose name lies is exactly the kind I should have traced before taking.

[robobun]

Three findings, all three landed. Two were real bugs, and one of them is a node differential that has nothing to do with the REPL.

runInThisContext discards the caller's microtask queue (0298906). Exactly the sibling of what the last commit documented for modules: drainMicrotasksForGlobalObject clears rather than drains, and runInThisContext has no context of its own, so checkForTermination was handing it the caller's global.

const { promise, resolve } = Promise.withResolvers();
const chain = promise.then(() => "survived");
resolve();
try { vm.runInThisContext("while (true) {}", { timeout: 100 }); } catch {}
chain.then(v => console.log(v));

$ node r.js
survived
$ bun r.js          # prints nothing

The contextified global is now its own parameter: runInContext passes its NodeVMGlobalObject, runInThisContext passes null. Pre-existing, byte-identical to main, but the PR restructured that function and fixed the same shape next door, so it belongs here.

The SIGINT disposition has two owners (485045e). BunProcess installs forwardSignal for process.on("SIGINT"), the watcher installs its own handler while armed, and each refuses to uninstall a handler it didn't install. That guard leaves the disposition wrong in both directions:

• adding a listener displaces the watcher's handler mid-evaluation, so process.on("SIGINT", () => {}); while (true) {} is still unkillable by Ctrl+C, which is the bug this PR is about;
• removing the last one has signal(SIGINT, SIG_DFL) find the watcher's handler and reinstate it, after which the disarm restores the forwarder BunProcess just asked to drop, and an external kill -INT is swallowed. That one is a regression from 15ea4c3 in this PR.

Fixed where the conflict lives: while armed the watcher owns the disposition, so BunProcess hands it the action to apply on disarm rather than installing over it. With nothing armed it returns false on the first line and BunProcess is byte-for-byte unchanged, which covers every path outside node:vm and the REPL.

The SIGINT vise is now four legs, one per reachable state, and the two new ones fail with the fix stubbed out on the same build. The signal and node:vm parallel suites still pass (test-vm-sigint-existing-handler.js, test-signal-unregister.js, test-process-remove-all-signal-listeners.js, test-signal-handler-remove-on-exit.js, test-vm-timeout.js, test-vm-module-basic.js).

Comment length (0298906). My own rule, broken twice more. Both trimmed to three lines with the load-bearing clauses intact.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/jsc/bindings/vm/SigintWatcher.cpp (1)

89-110: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Restore SIGINT before marking the watcher uninstalled.

Line 91 clears m_installed while sigintWatcherHandler is still installed. A SIGINT in that window wakes the watcher, then Line 69 returns before forwarding, swallowing the interrupt.

Proposed ordering fix

 void SigintWatcher::uninstall()
 {
-    if (m_installed.exchange(false)) {
+    if (m_installed.load()) {
         WTF::Thread* currentThread = WTF::Thread::currentMayBeNull();
         ASSERT(!currentThread || m_thread->uid() != currentThread->uid());
 
 `#if` OS(WINDOWS)
         SetConsoleCtrlHandler(WindowsCtrlHandler, false);
@@
             sigaction(SIGINT, &m_previousAction, nullptr);
         }
 `#endif`
 
+        m_installed.store(false);
         m_semaphore.signal();
         m_thread->waitForCompletion();
     }
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/vm/SigintWatcher.cpp` around lines 89 - 110, The uninstall
flow in SigintWatcher::uninstall clears m_installed before restoring the SIGINT
handler, leaving a window where sigintWatcherHandler can still intercept and
drop an interrupt. Reorder the logic so the existing handler is restored first
(using the current sigaction/SetConsoleCtrlHandler path), then mark the watcher
uninstalled and proceed with the semaphore signal and waitForCompletion. Keep
the fix localized to SigintWatcher::uninstall and preserve the current
platform-specific behavior.

src/runtime/cli/repl.rs (1)

1017-1053: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Close the final SIGINT race before disarming.

take_interrupt_error() runs while the terminal is still cooked. A Ctrl+C arriving after Line 1354 but before Line 1357 can be caught by the armed watcher after the final take, then end_interruptible_eval() disarms without converting/clearing that interrupt for this evaluation. Restore raw mode first, take the interrupt while the scope is still armed, then disarm.

Proposed fix

-    fn end_interruptible_eval(&mut self, scope: Option<SigintScope>) {
+    fn end_interruptible_eval(&mut self, scope: Option<SigintScope>) -> Option<JSValue> {
         let Some(scope) = scope else {
-            return;
+            return None;
         };

         // Before anything fallible: leaving the prompt in cooked mode is worse
         // than any error we could hit below.
         #[cfg(unix)]
         let _ = tty::set_mode(0, tty::Mode::Raw);

@@
-        // SAFETY: `scope.scope` came from `Bun__REPL__armSigint` and is consumed
-        // once; `global` is a live opaque `JSGlobalObject` handle.
+        // SAFETY: `global` is live and `scope.scope` remains armed until disarm.
+        let error = unsafe { Bun__REPL__takeSigintError(global, scope.scope) };
+        let error = (!error.is_empty()).then_some(error);
+
+        // SAFETY: `scope.scope` came from `Bun__REPL__armSigint` and is consumed
+        // once; `global` is a live opaque `JSGlobalObject` handle.
         unsafe { Bun__REPL__disarmSigint(global, scope.scope) };
+        error
     }
@@
-        if let Some(error) = self.take_interrupt_error(sigint.as_ref()) {
+        if let Some(error) = self.end_interruptible_eval(sigint) {
             outcome = EvalOutcome::Error(error);
         }
-        self.end_interruptible_eval(sigint);

Also applies to: 1352-1357

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/runtime/cli/repl.rs` around lines 1017 - 1053, The SIGINT cleanup in
end_interruptible_eval is ordered too late: take_interrupt_error can miss a
Ctrl+C that arrives after the final check but before disarm, leaving the
interrupt uncleared for this evaluation. Update end_interruptible_eval to
restore raw mode first, then call take_interrupt_error while the SigintScope is
still armed, and only after that call Bun__REPL__disarmSigint; keep the existing
scope/global handling and use sigint_requested/take_interrupt_error as the key
helpers to preserve the final race-free cleanup.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/bindings/NodeVMScript.cpp`:
- Around line 284-300: `checkForTermination()` in `NodeVMScript.cpp` is only
checking `vm.hasTerminationRequest()` before the early rethrow path, so a
pending termination exception can bypass the SIGINT/timeout classification.
Update the initial guard in `checkForTermination(JSC::VM&, JSC::JSGlobalObject*,
NodeVMGlobalObject*, JSC::ThrowScope&, NodeVMScript*, std::optional<double>)` to
also account for `hasPendingTerminationException()`, matching the logic used in
`NodeVMModule.cpp`, so pending termination is handled in the same branch as
termination requests.

In `@test/js/bun/repl/repl.test.ts`:
- Around line 1024-1029: The REPL SIGINT test in repl.test.ts only proves the
process stays alive, so it can pass even if the JS listener never runs; update
the test around the existing process.on('SIGINT', ...) setup to emit a clear
marker from the handler and wait for that marker before asserting the outcome.
Use the same REPL send/waitFor flow and the proc.kill("SIGINT") step, but
strengthen the assertion by verifying the listener callback actually executed
rather than relying on the "alive" race alone.

In `@test/js/node/vm/vm.test.ts`:
- Around line 981-984: Remove the fixture watchdog timer from the vm test and
rely on the existing stdout/exitCode assertions instead. In the vm.test.ts
scenario around m.evaluate and chain, eliminate the setTimeout/process.exit
guard and keep the continuation log plus the current promise/exit assertions so
the test still fails if the expected output is missing. Use the existing
m.evaluate and chain flow to preserve the invariant without any embedded timer
watchdog.

---

Outside diff comments:
In `@src/jsc/bindings/vm/SigintWatcher.cpp`:
- Around line 89-110: The uninstall flow in SigintWatcher::uninstall clears
m_installed before restoring the SIGINT handler, leaving a window where
sigintWatcherHandler can still intercept and drop an interrupt. Reorder the
logic so the existing handler is restored first (using the current
sigaction/SetConsoleCtrlHandler path), then mark the watcher uninstalled and
proceed with the semaphore signal and waitForCompletion. Keep the fix localized
to SigintWatcher::uninstall and preserve the current platform-specific behavior.

In `@src/runtime/cli/repl.rs`:
- Around line 1017-1053: The SIGINT cleanup in end_interruptible_eval is ordered
too late: take_interrupt_error can miss a Ctrl+C that arrives after the final
check but before disarm, leaving the interrupt uncleared for this evaluation.
Update end_interruptible_eval to restore raw mode first, then call
take_interrupt_error while the SigintScope is still armed, and only after that
call Bun__REPL__disarmSigint; keep the existing scope/global handling and use
sigint_requested/take_interrupt_error as the key helpers to preserve the final
race-free cleanup.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 611d0156-ba72-4f66-9f0f-b41aae45e302

📥 Commits

Reviewing files that changed from the base of the PR and between 1d24dce and 485045e.

📒 Files selected for processing (9)

• src/jsc/bindings/BunProcess.cpp
• src/jsc/bindings/NodeVMModule.cpp
• src/jsc/bindings/NodeVMScript.cpp
• src/jsc/bindings/bindings.cpp
• src/jsc/bindings/vm/SigintWatcher.cpp
• src/jsc/bindings/vm/SigintWatcher.h
• src/runtime/cli/repl.rs
• test/js/bun/repl/repl.test.ts
• test/js/node/vm/vm.test.ts

[claude]

The last two commits address all three of my remaining points — the deferSigintDisposition handshake closes the add/remove-listener vise cleanly, and threading contextGlobalObject through checkForTermination fixes the runInThisContext sibling with a matching test. Nothing further from me; deferring to a human for the final call given the new BunProcess ↔ SigintWatcher disposition-ownership protocol is an architectural coupling worth a maintainer's eyes.

Extended reasoning...

Overview

This PR makes Ctrl+C interrupt a running REPL evaluation instead of hanging the process. It touches 12 files across the REPL (repl.rs), JSC VM termination handling (bindings.cpp, NodeVMScript.cpp, NodeVMModule.cpp), the process-wide SIGINT watcher (SigintWatcher.{h,cpp}, SigintReceiver.h), and — as of the latest commit — BunProcess.cpp's process.on('SIGINT') path. Since my last review at 607092f, two new commits landed: 0298906 threads a separate contextGlobalObject through checkForTermination so runInThisContext no longer clears the caller's microtask queue on timeout, and 485045e introduces SigintWatcher::deferSigintDisposition, a handshake that lets onDidChangeListeners hand its intended SIGINT action to the armed watcher rather than installing over it. Both come with regression tests, and the 4-line comments I flagged are now 3.

Security risks

None identified. Signal-handler installation is confined to SIGINT and uses the existing async-signal-safe pattern (semaphore post → watcher thread). The new deferSigintDisposition path is guarded by m_refCountMutex and only mutates m_previousAction; no new user-controlled data reaches native code.

Level of scrutiny

High. This is not a mechanical change: it rewires SIGINT disposition ownership across three subsystems (the REPL, node:vm's breakOnSigint, and process.on('SIGINT')), restructures node:vm termination classification to re-raise across nested scopes, and introduces a new coordination protocol (deferSigintDisposition) between BunProcess.cpp and SigintWatcher. The PR has gone through ~10 review rounds, each surfacing a real interaction bug (RELEASE_ASSERT abort, swallowed external SIGINT, clobbered listener, cleared main-thread microtask queue), which is itself evidence that the state space here is subtle. The Windows path is explicitly untested (describe.todoIf(isWindows)), and two known gaps are documented in the PR body (parked event loop, nested-timeout classification).

Other factors

Test coverage is thorough — 7 new pty-driven REPL tests forming a three-leg vise on SIGINT disposition, plus 4 spawned vm.test.ts regressions for the abort/microtask-clear cases. The author has been rigorous about A/B verification and honest about what's deferred. That said, the deferSigintDisposition mechanism in 485045e is a new architectural coupling introduced in the final commit specifically to close the third leg of the vise; a maintainer should confirm that having BunProcess reach into the SIGINT watcher's saved-action slot is the right layering, versus e.g. having the watcher chain to forwardSignal while armed. I'm not approving on that basis, not because I've found a defect.

[robobun]

Five findings this round, three landed in c7c5071, two declined with evidence. The two outside-diff ones didn't get inline threads, so they're answered here.

Taken

SigintWatcher::uninstall restored the disposition too late. Real, and worse than the comment suggested. m_installed.exchange(false) ran while sigintWatcherHandler was still the disposition, so a signal in that window runs the handler, posts the semaphore, and the watcher thread bails on if (!m_installed) return; before forwarding it. It also leaves m_waiting set, which deafens the watcher for the rest of the process.

The window is nanoseconds, so I widened it with a temporary 50ms sleep and counted where 30 signals went. Every signal should either interrupt the armed breakOnSigint script or reach the JS listener:

	run 1	run 2	run 3
restore first (now)	7 + 23 = 30	7 + 23 = 30	9 + 21 = 30
clear m_installed first (before)	2 + 26 = 28	process died mid-loop	process died mid-loop

Two signals vanished outright in the run that finished; the other two runs never reached the fixture's own exit path. The restore is already guarded on the handler still being ours, so moving it above the exchange is idempotent and the exchange still gates the thread teardown to one caller.

Assert the SIGINT listener actually runs. The "alive" race couldn't distinguish forwardSignal from a leftover sigintWatcherHandler (watcher thread joined, signal dropped, process alive). The exact suggestion hangs, though: the REPL's loop doesn't run while it's parked at the prompt, so the marker only appears on the next evaluation. The test now nudges with 3 + 4\n and waits for the marker, which is deterministic because the signal is already pending when the keystrokes are written, and a handler runs before the read() they satisfy returns to userspace.

Drop the fixture watchdog timer. Switching await chain to chain.then(...) means the cleared-queue case has nothing left to do and just exits, so the missing line is the failure. Verified against the broken clear: fails in 1.1s on - "chain=survived" rather than on an 8s timer.

Declined

repl.rs: take the interrupt before disarming. The race it describes is already handled one layer down. Bun__REPL__disarmSigint clears a trap bit left by a signal that raced the disarm, with that exact rationale:

// A signal that raced the disarm leaves a trap bit nobody will service;
// the next evaluation would terminate the instant it entered the VM.
if (vm.traps().hasTrapBit(JSC::VMTraps::NeedTermination)) [[unlikely]]
    replClearTermination(vm);

Reordering narrows the window but doesn't close it (a signal can still land between the take and the disarm), and the evaluation is already over by then, so there is nothing left to report as interrupted. The proposal also converts a dropped keystroke into a false ERR_SCRIPT_EXECUTION_INTERRUPTED on an evaluation that succeeded.

NodeVMScript: add hasPendingTerminationException() to the guard. The state is unreachable from both vm.Script entry points, so the clause would be dead. Full argument and the instrumented run (210 + 97 tests, probe never fired) are in the thread.

---

@claude, noted on the layering. The alternative you name, having the watcher chain to forwardSignal while armed, was the other shape I considered and rejected: it would make Ctrl+C during a breakOnSigint script run the user's JS listener and terminate the script, which is not what breakOnSigint promises. The handshake keeps the two effects mutually exclusive and leaves BunProcess byte-for-byte unchanged whenever nothing is armed. Happy to reshape it if a maintainer prefers the other tradeoff.

[robobun]

Rebased onto main (force-push, c7c5071 → 28383655). The diff is byte-identical; no code changed.

The red X on the previous build was two failures, neither from this diff:

• test/js/bun/util/v8-heap-snapshot.test.ts SIGKILLed (OOM) on 25.04 x64. The branch was 17 commits behind, so it was still running the pre-Take each v8 heap snapshot in its own process #33415 version of that test. Take each v8 heap snapshot in its own process #33415 is test-only (two files under test/js/bun/util/, zero src/) and exists precisely to stop that runner being OOM-killed, so the rebase is the fix rather than a retrigger.
• typescript@5.9.2 tarball download failed on 2019 x64-baseline. Registry flake.

Re-verified on the new base: repl.test.ts 125 pass, vm.test.ts 278 pass, and the 13 signal/node:vm files in test/js/node/test/parallel/ all exit 0.

Zero file overlap between this diff and the 62 files main touched in those 17 commits, and the rebase applied without conflicts.

[robobun]

Status: diff is green, CI is red on four unrelated failures

Build 73614 (the rebased 92c7b6f) finished with 281 jobs passed and four failing tests, none of which touch any file in this diff:

test	lane	also red on (last 30 builds)	class
test/js/node/test/parallel/test-net-connect-memleak.js	alpine 3.23 x64 + x64-baseline	22 other branches	break on main
test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js	debian 13 x64-asan	13 other branches	break on main
test/js/web/timers/timer-heap-race.test.ts	debian 13 x64-asan	1 other branch	break on main
test/cli/install/bun-upgrade.test.ts	darwin 14 aarch64	0	external dependency ("Canary builds are not available for this platform yet")

None of the crash stacks (ConcurrentTask / JSCScheduler / DeferredWorkTimer / WaiterListManager / Worker.cpp / web_worker.rs / JSObject::getOwnPropertyDescriptor / net / bun upgrade network download) touch any file this PR changes. All four are being handled separately.

repl.test.ts and vm.test.ts (the tests this PR adds and modifies) are green on every lane that ran them.

Local, on the current head:

• test/js/bun/repl/repl.test.ts: 125 pass, 0 fail
• test/js/node/vm/vm.test.ts: 279 pass, 0 fail
• 13 signal / node:vm files in test/js/node/test/parallel/: all exit 0

The one CI retrigger allowed per iteration is spent (build 69189 → 69274), so I'm leaving this for a maintainer rather than pushing another. The change is ready.

[robobun]

Rebased onto main (80513b5 → 92c7b6f, 202 commits). One conflict, in repl.rs: #33527 turned the module-level tty::set_mode(fd, mode) into a per-handle self.tty_state.set_mode(fd, mode), and begin_interruptible_eval/end_interruptible_eval now use that API. Everything else auto-merged. The stale ci: retrigger commit was dropped.

Re-verified on the new base: repl.test.ts 125 pass, vm.test.ts 279 pass, all 13 signal/node:vm parallel tests exit 0.

[robobun]

Independent confirmation of the await half of this PR, in case it helps prioritize it.

The "one Ctrl+C during an await leaves the session silently broken" case was re-found while reading the wait_for_promise / teardown code on current main: JSC__VM__setExecutionForbidden (src/jsc/bindings/bindings.cpp) ignores its bool and JSC::VM has no API to clear m_executionForbidden, so the set_execution_forbidden(false) recovery in repl.rs is a no-op and VM::drainMicrotasks discards every microtask for the rest of the session. Reproduced on release 1.4.0 under a pty: after await new Promise(() => {}) + Ctrl+C, both await Promise.resolve(1) and a plain .then() callback never run; the same steps without the Ctrl+C work. That matches the "never-settling await" test here, so no separate PR is being opened for it.

Note that the branch is conflicting with main again (#37075 touched the wait_for_promise call sites in repl.rs after the last rebase).