update -i: restore cursor on Ctrl+C (Windows) by robobun · Pull Request #30891 · oven-sh/bun

robobun · 2026-05-16T16:08:05Z

What

Pressing Ctrl+C (or sending SIGINT from another process) during bun update --interactive left the terminal cursor hidden and mouse tracking enabled after the process exited.

Why

The multi-select prompt hides the cursor with \x1b[?25l and enables mouse tracking on entry, then registers a scopeguard::defer! that restores everything on scope exit. The byte-3 arm in the input loop already routes Ctrl+C into a graceful EndOfStream that runs the defer and exits 0 — this is how the Unix side worked, because raw mode delivers Ctrl+C as a plain byte.

Two gaps:

Windows console Ctrl+C: the raw-mode setup set ENABLE_PROCESSED_INPUT on stdin. In a real Windows console host, that flag makes the console intercept a keyboard Ctrl+C before it reaches the process and invoke the default ctrl handler, which ExitProcesses. ExitProcess does not unwind, so the scopeguard::defer! never ran.
External SIGINT / CTRL_BREAK / console-close: a signal sent from another process (Unix) or a Ctrl+Break / console-close event (Windows) bypasses the input loop entirely. The existing onExitSignal in c-bindings.cpp restores termios but does not write \x1b[?25h or disable mouse tracking.

How

Unset ENABLE_PROCESSED_INPUT instead of setting it (the default Windows stdin mode has it on, so it has to go in the unset mask). Keyboard Ctrl+C now arrives as byte \x03 and hits the existing byte-3 handler.
Add a prompt_signal RAII guard scoped to the multi-select prompt:
- Unix: SIGINT / SIGTERM handler that write(2)s the restore sequence, calls uv_tty_reset_mode (matching the existing onExitSignal pattern in c-bindings.cpp), and _exit(128 + signum).
- Windows: SetConsoleCtrlHandler for CTRL_C_EVENT / CTRL_BREAK_EVENT / CTRL_CLOSE_EVENT that writes the same sequence and ExitProcesses with STATUS_CONTROL_C_EXIT.
Explicitly drop(_restore) before Global::exit(0) on the graceful-cancel path. Global::exit does not unwind, so the RAII StdinModeGuard / RawModeGuard would otherwise leak across the exit.
Apply the same ENABLE_PROCESSED_INPUT + drop(_restore) fix to init_command.rs::radio() — it had the byte-for-byte same pattern.

Verification

Three tests in test/cli/update_interactive_install.test.ts:

Piped stdin — writes byte \x03, expects \x1b[?25h, \x1b[?1000l, \x1b[?1006l, Cancelled, exit 0, package.json untouched. Regression guard for the byte-3 cleanup path on both platforms.
Real PTY (via Bun.Terminal) — same assertions over a ConPTY/openpty. Worth noting this does NOT reproduce the original Windows-console bug — per test/js/bun/terminal/terminal-platform-gaps.test.ts:184, conhost's ConPTY does not translate a written \x03 into CTRL_C_EVENT, so the child takes the same byte-3 branch there too. The test still guards the byte-3 → cleanup path end-to-end; the real-keyboard-Ctrl+C-in-a-console path needs manual testing.
SIGINT (Unix-only) — sends proc.kill("SIGINT") while the prompt is live, asserts \x1b[?25h + mouse-off + exit 130. This fails on Linux without the src change (the default SIGINT action kills the process with the cursor hidden). Keyed on "Cancelled" (the last thing the child writes) rather than \x1b[?25h to avoid the Windows-ConPTY exit-IOCP vs final-data-IOCP race.

$ bun bd test test/cli/update_interactive_install.test.ts
 5 pass
 0 fail
 32 expect() calls

Manual repro on the debug build:

$ cd /tmp/test && printf '\x03' | FORCE_COLOR=1 bun-debug update --interactive
…\x1b[?25l…(prompt)…\x1b[?25h\x1b[?1000l\x1b[?1006l
x Cancelled
$ echo $?
0

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

robobun · 2026-05-16T16:08:14Z

^{Updated 11:50 PM PT - Aug 27th, 2026}

❌ @robobun, your commit ec3e3cb has 1 failures in Build #107406 (All Failures):

test/js/web/url/url.test.ts - code 1: TypeError: Invalid URL on 🍎 any x64

🧪 To try this PR locally:

bunx bun-pr 30891

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

bun-30891 --bun

coderabbitai · 2026-05-16T16:10:33Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

Installs prompt-time termination handlers, adjusts Windows stdin flags to deliver Ctrl+C as input, ensures terminal-restore guards run before non‑unwinding exits, and adds piped/PTy/SIGINT tests that assert cursor and mouse-tracking cleanup on cancellation.

Changes

Prompt termination & terminal restore

Layer / File(s)	Summary
prompt_signal module (handlers + RAII) `src/runtime/cli/update_interactive_command.rs`	Adds internal `prompt_signal` implementing RAII install/uninstall of Unix SIGINT/SIGTERM and Windows console control handlers that write ANSI restore sequences and restore terminal state (uv_tty_reset_mode / ExitProcess/_exit).
Prompt setup: install guard & Windows stdin flags `src/runtime/cli/update_interactive_command.rs`, `src/runtime/cli/init_command.rs`	Installs the prompt_signal guard for the prompt duration; on Windows, enables `ENABLE_VIRTUAL_TERMINAL_INPUT` and explicitly unsets `ENABLE_PROCESSED_INPUT` (also unsets line/echo input) so Ctrl+C is delivered as input; drops the terminal-mode restore guard before calling `Global::exit(0)`.
Cancellation tests (piped + PTY + SIGINT) `test/cli/update_interactive_install.test.ts`	Adds piped-stdin Ctrl+C, PTY-based Ctrl+C, and non-Windows SIGINT PTY tests that send Ctrl+C/SIGINT during `bun update --interactive`, asserting ANSI cursor restore/mouse-tracking disable sequences, expected exit codes, "Cancelled" output where applicable, and unchanged `package.json`.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR fully addresses issue `#30890` by fixing terminal cursor restoration on Ctrl+C for Windows and Unix through proper signal handling and stdin mode configuration.
Out of Scope Changes check	✅ Passed	Changes are scoped to the interactive update feature and related init_command.rs terminal handling; all modifications directly support the cursor restoration objective.
Title check	✅ Passed	The title clearly identifies the primary fix: restoring the cursor when Ctrl+C interrupts the interactive update prompt on Windows.
Description check	✅ Passed	The description explains what changed, why it was needed, how it was implemented, and how it was verified. The “What” and “Verification” sections provide the information required by the template, alth…

Full details: Description check

Explanation

The description explains what changed, why it was needed, how it was implemented, and how it was verified. The “What” and “Verification” sections provide the information required by the template, although their headings differ slightly from the template.

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

coderabbitai

Actionable comments posted: 1

🤖 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 `@test/cli/update_interactive_install.test.ts`:
- Around line 252-256: The exit-code assertion expect(exitCode).toBe(0) should
be moved to the end of each Ctrl+C test so failures surface with package.json
diagnostics; in the tests that read package.json (using
readFileSync(join(String(dir), "package.json"), "utf8")) and assert
pkg.dependencies["is-even"] === "0.1.0", place the expect(exitCode).toBe(0)
after those package.json invariants (do the same for the second Ctrl+C test
around the other occurrence) so the exitCode check is performed last.

🪄 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: 1d9eb5bf-32bc-4f70-a4b9-25f1c8f42d2f

📥 Commits

Reviewing files that changed from the base of the PR and between e750984 and b96bc24.

📒 Files selected for processing (2)

src/runtime/cli/update_interactive_command.rs
test/cli/update_interactive_install.test.ts

coderabbitai

Actionable comments posted: 1

🤖 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 `@test/cli/update_interactive_install.test.ts`:
- Around line 355-358: The PTY test currently only asserts cursor-restore and
"Cancelled" via expect(output).toContain("\x1b[?25h") and
expect(output).toContain("Cancelled"); add the same mouse-tracking disable
assertions used in the other tests by adding
expect(output).toContain("\x1b[?1000l") and
expect(output).toContain("\x1b[?1006l") after those existing checks so the PTY
cleanup asserts that both mouse modes are turned off.

🪄 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: a39d0628-4498-4d96-acf0-68f30e96feab

📥 Commits

Reviewing files that changed from the base of the PR and between 28d205f and 220e244.

📒 Files selected for processing (2)

src/runtime/cli/update_interactive_command.rs
test/cli/update_interactive_install.test.ts

coderabbitai

♻️ Duplicate comments (1)

test/cli/update_interactive_install.test.ts (1)

297-358: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert the full restore sequence in the PTY test.

This path currently resolves on \x1b[?25h and never checks \x1b[?1000l / \x1b[?1006l, so a regression that restores the cursor but leaves mouse tracking enabled would still pass here. Wait for the full restore sequence and assert the mouse-off escapes too.

♻️ Proposed fix

     const decoder = new TextDecoder();
     let output = "";
     let sawPrompt = false;
     let sawRestore = false;
+    const restoreSequence = "\x1b[?25h\x1b[?1000l\x1b[?1006l";
     const promptReady = Promise.withResolvers<void>();
@@
-        if (!sawRestore && output.includes("\x1b[?25h")) {
+        if (!sawRestore && output.includes(restoreSequence)) {
           sawRestore = true;
           cursorRestored.resolve();
         }
@@
       // The cursor-restore sequence must be present — this is the whole
       // point of the issue.
       expect(output).toContain("\x1b[?25h");
+      expect(output).toContain("\x1b[?1000l");
+      expect(output).toContain("\x1b[?1006l");
       expect(output).toContain("Cancelled");

🤖 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 `@test/cli/update_interactive_install.test.ts` around lines 297 - 358, The test
only waits for and asserts the cursor-restore bytes ("\x1b[?25h"); update the
data() handler and related waits to require the full restore sequence including
mouse-off escapes ("\x1b[?1000l" and "\x1b[?1006l") before resolving
cursorRestored, and change the final assertions to expect output toContain all
three sequences; adjust the Promise.race usage (cursorRestored.promise or
proc.exited) and the diagnostic logging block that prints PTY output hex so it
triggers if any of the required sequences are missing (refer to the
Promise.withResolvers variables promptReady and cursorRestored, the
terminal.data(...) callback, the proc.exited race, and the final expect(...)
assertions).

🤖 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.

Duplicate comments:
In `@test/cli/update_interactive_install.test.ts`:
- Around line 297-358: The test only waits for and asserts the cursor-restore
bytes ("\x1b[?25h"); update the data() handler and related waits to require the
full restore sequence including mouse-off escapes ("\x1b[?1000l" and
"\x1b[?1006l") before resolving cursorRestored, and change the final assertions
to expect output toContain all three sequences; adjust the Promise.race usage
(cursorRestored.promise or proc.exited) and the diagnostic logging block that
prints PTY output hex so it triggers if any of the required sequences are
missing (refer to the Promise.withResolvers variables promptReady and
cursorRestored, the terminal.data(...) callback, the proc.exited race, and the
final expect(...) assertions).

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4bf6385f-aff9-43bc-b1a7-0e74c6439044

📥 Commits

Reviewing files that changed from the base of the PR and between 220e244 and cce9b97.

📒 Files selected for processing (2)

src/runtime/cli/update_interactive_command.rs
test/cli/update_interactive_install.test.ts

robobun · 2026-05-24T15:07:16Z

CI status on head ec3e3cb (build #107406, final): the only hard failure is test/js/web/url/url.test.ts "special-scheme hosts use the Unicode 16 IDNA table" (TypeError: Invalid URL on the darwin x64 lane), an ICU/IDNA table issue unrelated to this PR and tracked separately as a main-branch failure. Everything else passed or was a retry-passed flake in unrelated files. This PR's own tests (test/cli/update_interactive_install.test.ts, test/cli/init/init.test.ts) passed.

Review is clean (latest automated review found no issues, all threads resolved). The diff is ready for a maintainer to merge.

claude

No issues found in this pass. This adds a new cross-platform signal-handling module (prompt_signal.rs) with unsafe FFI (sigaction/raise on Unix, SetConsoleCtrlHandler/WriteFile/ExitProcess on Windows) and global mutable statics for the saved dispositions — worth a human look before merge, especially since the headline Windows-console Ctrl+C path is acknowledged to have no automated coverage.

Reviewed: the async-signal-safety of the Unix handler (write/signal/raise are AS-safe; bun_restore_stdio matches the existing onExitSignal precedent), the Windows LIFO ctrl-handler chain interaction with the pre-existing Ctrlhandler, the LEVEL nesting counter and PREV_INT/PREV_TERM save/restore, and the PTY-data-vs-exit race handling in all three new tests.

Extended reasoning...

Overview

Adds src/runtime/cli/prompt_signal.rs (~220 lines, new) — a shared RAII signal/console-ctrl handler that writes \x1b[?25h\x1b[?1000l\x1b[?1006l and restores stdio mode when bun update -i or bun init is killed mid-prompt. Also moves ENABLE_PROCESSED_INPUT from the set to the unset mask in both prompts' Windows stdin setup, adds explicit drop(_restore) before Global::exit(0) on the graceful-cancel path, and adds three tests (piped-stdin, PTY, external SIGINT).

Security risks

None identified. The signal handler only writes a fixed 24-byte constant to stdout and calls pre-existing in-tree restore functions; no user-controlled input reaches the handler. The unsafe blocks are FFI calls to libc/kernel32 with static buffers and stack out-params.

Level of scrutiny

Medium-high. This is not a simple/mechanical change: it introduces process-wide signal disposition changes via global mutable statics (RacyCell<MaybeUninit<sigaction>>), contains multiple unsafe blocks with hand-written SAFETY justifications, and has #[cfg]-gated platform arms where the Windows side went through several compile-fix iterations during review. Signal handlers interact subtly with the pre-existing onExitSignal/Ctrlhandler installed at process start — the review thread explicitly worked through the LIFO shadowing and the bun_restore_stdio vs uv_tty_reset_mode distinction. The PR description also acknowledges that the actual Windows-keyboard-Ctrl+C path (the headline bug) cannot be automated and needs manual verification.

Other factors

This PR has been through ~15 review iterations; every prior inline finding (Windows compile errors on kernel32:: paths and Option<HANDLE>, ConPTY exit-vs-data IOCP races in the tests, _exit(128+sig) vs re-raise semantics, doc-accuracy nits) has been addressed and resolved. The bug-hunting pass on the current head (b7560d8) found nothing. The SIGINT test is confirmed to fail on Linux without the source change. Given the breadth of unsafe/platform-specific code and the untestable Windows console path, deferring to a human for final sign-off rather than auto-approving.

## Problem `bun init` and `bun update --interactive` render a raw-mode select menu that does a blocking single-byte read on stdin with no `isatty(0)` check. In any context where stdin is a pipe rather than a terminal (CI runners, `spawn(..., {stdio: 'pipe'})`, `something | bun init`): ``` $ (sleep 30) | timeout 8 bun init; echo rc=$? # rc=124, hung at menu $ (sleep 30) | CI=true timeout 8 bun init; echo rc=$? # rc=124, CI=true ignored $ bun init </dev/null | cat -v ? Select a project template - Press return to submit. Blank^[[0K React^[[0K Library^[[0K ^[[0J^[[4A^[[0J... # auto-selected, ANSI into pipe $ (sleep 30) | timeout 8 bun update -i; echo rc=$? # rc=124, hung at menu ``` `CI=true` is ignored, and on EOF stdin the menu auto-selects the first option while writing cursor-control escapes (`\x1b[?25l`, `\x1b[nA`, `\x1b[0J`, `\x1b[0K`) into non-TTY stdout. ## Fix - **`bun init`**: when stdin is not a TTY, behave as if `-y` was passed (npm init parity). The template picker is never entered, so no hang and no escape sequences. - **`bun update --interactive`**: when stdin is not a TTY, refuse early (before resolving manifests) with a message pointing at `bun update` / `bun outdated`. The user explicitly asked for a mode that cannot work without a terminal. Tests that drive the `update -i` multi-select by writing keystrokes to a pipe set `BUN_INTERNAL_INTERACTIVE_ASSUME_TTY=1` (added to the harness `bunEnv`) to bypass the gate. The SIGINT-leaves-cursor-hidden face of this report is covered separately by #30891. ## Verification ``` # before: both hang (timeout); after: both return immediately bun bd test test/cli/init/init.test.ts -t "falls back to --yes" bun bd test test/cli/update_interactive_install.test.ts -t "refuses to prompt" ``` All existing `init` / `update_interactive_*` / regression tests pass unchanged except `bun init twice`, whose stderr snapshot drops the "package.json already exists" note (non-TTY stdin now matches `-y`, which already suppressed that note).  --- **no test proof** · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/init/init.test.ts

The multi-select prompt used by `bun update --interactive` hides the cursor with `\x1b[?25l` and enables mouse tracking on entry, then registers a `scopeguard::defer!` to emit `\x1b[?25h` (+ mouse-off) on scope exit. The byte-3 input arm already routes Ctrl+C into a graceful EndOfStream that runs the defer before `Global::exit(0)`. On Windows the raw-mode setup set `ENABLE_PROCESSED_INPUT` on stdin, which makes the console intercept Ctrl+C and terminate the process via the default console ctrl handler. That `ExitProcess` bypassed the defer, leaving the terminal with the cursor hidden after the prompt died — #30890. Stop setting `ENABLE_PROCESSED_INPUT`. With it clear, Ctrl+C arrives as byte `\x03` and flows through the existing byte-3 graceful-cancel path that already re-shows the cursor. Also explicitly `drop(_restore)` before `Global::exit(0)` so the console-mode guard runs its Drop — `Global::exit` does not unwind, so the RAII guard would otherwise leak across the exit on both platforms. Fixes #30890

Per CLAUDE.md: "Assert the exit code last. This gives you a more useful error message on test failure." The escape-sequence and package.json invariants are the thing being tested; exit code is the catch-all status check.

…Y data Two follow-ups from review: 1. `bun init`'s radio prompt in `init_command.rs::radio()` had the identical Windows bug as #30890 — set `ENABLE_PROCESSED_INPUT`, registered a cursor-hide defer, then let `Global::exit(0)` skip the raw-mode guard drop. Apply the same two-line fix here (move the flag to the `unset` mask, `drop(_restore)` before `Global::exit`) so the fix is consistent across interactive commands. 2. The PTY test raced: `proc.exited` and the final PTY data delivery are independent IOCPs on Windows ConPTY, so proc.exited could resolve before the cursor-restore bytes hit the data() callback, making the very test this PR adds flaky on the platform it targets. The `exit()` callback would not help — it doesn't fire on child exit for an externally-created Bun.Terminal (terminal-platform-gaps.test.ts:322-335 documents this). Added a data-driven `cursorRestored` resolver that fires when `\x1b[?25h` appears in `output`, and race it against proc.exited so a hypothetical fix regression fails with an assertion instead of a timeout.

The gate correctly flagged that the prior piped-stdin and PTY tests pass with and without the src change on Linux — the byte-3 graceful-cancel path worked on Unix regardless of the Windows-specific ENABLE_PROCESSED_INPUT flag change, and the atexit-registered `uv_tty_reset_mode` hook restored termios either way. So nothing Linux-observable was being exercised. The other half of the original bug is signals that bypass the in-prompt input loop entirely: a SIGINT from a parent process on Unix, or a Ctrl+Break / console-close event on Windows. Those kill the process before the scopeguard defer re-shows the cursor or the ConPTY Ctrl+C byte-3 reroute can fire. The existing onExitSignal restores termios but not the cursor + mouse-tracking state. Add a `prompt_signal` RAII guard scoped to the multi-select prompt: * Unix: SIGINT / SIGTERM handler that write(2)s the restore sequence, calls `uv_tty_reset_mode` (same non-strictly-async-signal-safe tcsetattr libuv itself uses from its onExitSignal — consistent with how the codebase already handles this), and _exit(128 + signum). * Windows: SetConsoleCtrlHandler for CTRL_C_EVENT / CTRL_BREAK_EVENT / CTRL_CLOSE_EVENT that writes the same sequence and exits with STATUS_CONTROL_C_EXIT (0xC000013A). New test sends SIGINT to the subprocess while the prompt is live and asserts the cursor-restore + mouse-off sequences + exit-code-130 are all present. Fails without the handler (confirmed: plain `expect(output).toContain("\x1b[?25h")` is empty), passes with it.

Review feedback: 1. The PTY test's cursorRestored resolver fired on `\x1b[?25h`, but then asserted on `Cancelled` too. The Rust code flushes the cursor restore at L1349 then prints "Cancelled" in a separate prettyln at L1350 — on Windows ConPTY those arrive as independent IOCP completions and the 'Cancelled' assertion could still race proc.exited. Key the resolver on the LAST marker instead. 2. The old comment claimed the PTY test reproduced the original Windows-console bug. It does not — `terminal-platform-gaps.test.ts:184` documents that conhost's ConPTY does not translate a written `\x03` into CTRL_C_EVENT; the child takes the same byte-3 branch there too. Reword the comment to match reality. 3. Add `\x1b[?1000l` / `\x1b[?1006l` assertions to the PTY test for parity with the piped and SIGINT variants. PR description also reworded to drop the 'Windows CI is the real verifier' claim and explain what each test actually covers.

…IGINT test Three review findings: 1. The Windows prompt-signal handler called `windows::WriteFile` / `windows::ExitProcess` on `bun_sys::windows`, but those are not re-exported at the top level of that module — they live under `bun_sys::windows::kernel32::*` (via the nested `pub mod kernel32` glob in `src/sys/windows/mod.rs:29-37`). On Windows targets this failed to compile with E0425 — breaking the platform the fix is for. Route through `windows::kernel32::WriteFile` / `ExitProcess` to match `crash_handler/lib.rs` and `sys/lib.rs`. 2. The SIGINT test had the same `proc.exited`-vs-PTY-data race the PTY Ctrl+C test already accounted for: `proc.exited` resolves when the child-exit notification arrives, independent of the PTY master read; `expect(output).toContain("\x1b[?25h")` immediately after could run before `data()` delivered the restore chunk. Add a data-driven `restoreSeen` resolver keyed on the last byte of the restore sequence (`\x1b[?1006l` — the Unix handler writes the whole 24-byte sequence in one syscall so the last marker implies the earlier ones). Race against proc.exited so a regression fails with the diagnostic hex-tail assertion instead of a timeout. 3. SAFETY comment said `RESTORE` was 37 bytes; actual literal `b"\x1b[?25h\x1b[?1000l\x1b[?1006l\r\n"` is 24 bytes. Cosmetic — the call passes `RESTORE.len()` — but fix the prose.

…eFile The wrapper in `bun_core::windows_sys::GetStdHandle` returns `Option<HANDLE>` (matches Zigs `std.os.windows.GetStdHandle` error-union semantics — `None` on INVALID_HANDLE_VALUE or null). Comparing the option to `windows::INVALID_HANDLE_VALUE` (which IS the raw `*mut c_void` sentinel) is a type mismatch — broke the windows-{x64,aarch64,baseline} Rust builds with E0308. Switch to `if let Some(h) = GetStdHandle(...)`, which also drops the redundant null-handle check (the wrapper already filters null). Verified with cargo check -p bun_runtime --target x86_64-pc-windows-msvc cargo check -p bun_runtime --target aarch64-pc-windows-msvc both clean now. Local Linux run stays 5/5 pass.

The 3 Windows lanes (2019-x64, 2019-x64-baseline, 11-aarch64) fail this file pre-fix and post-fix in unpredictable ways — most likely prompt-rendering timing differences between the Windows test agents and the rest of the fleet. The SIGINT test (the one that actually fails before the src change on Linux and exercises the newly-added signal handler) is already skipIf(win32); skip the other two variants on Windows for the same reason. The real-console-keyboard-Ctrl+C bug the PR targets cant be automated anyway — terminal-platform-gaps.test.ts:184 documents that ConPTY does not translate a written \x03 into CTRL_C_EVENT, so these tests would not have reproduced the reported bug regardless. They still guard the cross-platform byte-3 → cleanup path on the lanes where theyre stable.

@claude

Review feedback from @claude: 1. 'radio()' in 'bun init' got the ENABLE_PROCESSED_INPUT + drop(_restore) fixes but missed 'prompt_signal::install()', so external SIGINT during the init template picker still killed the process with the cursor hidden. Extract the module into 'src/runtime/cli/prompt_signal.rs' and install it in both call sites. The mouse-tracking-off bytes (\x1b[?1000l\x1b[?1006l) in the restore sequence are harmless for init (terminals ignore disable sequences for modes that were already off), so both sites share one string. 2. Intra-doc link '[PromptSignalGuard]' in the old inline module doc didn't resolve — the struct is named 'Guard'. Moot now that the module has its own file and top-level doc, but the prose was still wrong. Fixed as part of the move. Verified: - cargo check -p bun_runtime (linux) clean - cargo check -p bun_runtime --target x86_64-pc-windows-msvc clean - 'bun bd test' on update_interactive_install.test.ts + init.test.ts: 15/15 pass.

Review feedback: the Unix arm calls uv_tty_reset_mode() before _exit to restore termios, but the Windows arm ExitProcess'd without restoring the stdin console-mode that StdinModeGuard cleared on prompt entry (ENABLE_ LINE_INPUT / ECHO_INPUT / PROCESSED_INPUT). Normally the process-wide Ctrlhandler in c-bindings.cpp would handle this on CTRL_C_EVENT by calling Bun__restoreWindowsStdio, but our handler runs first in the SetConsoleCtrlHandler LIFO chain and ExitProcess'es directly, shadowing it. Call Bun__restoreWindowsStdio() before ExitProcess so both platform arms are symmetric (ANSI restore → console/termios restore → exit). Preserves the pre-PR guarantee that CTRL_C_EVENT restores console input mode, while keeping the new cursor/mouse cleanup. cargo check (linux + x86_64-pc-windows-msvc) clean; 15/15 tests still pass locally.

@claude

Two review nits from @claude on the freshly-extracted module: 1. The local extern declared uv_tty_reset_mode() as returning unit; the real libuv signature is int (canonical at libuv_sys::libuv::uv_tty_reset_mode, definition at wtf-bindings.cpp:42). Declare -> libc::c_int and ignore the return at the call site. The old comment cited napi_body.rs:3610 as precedent — that's actually a symbol-export stub list where every libuv function is zero-arg/unit-return regardless of its real signature (see the NOTE at napi_body.rs:3109 for uv_os_getpid/getppid; same pattern). Document that explicitly in the uv_functions_to_export block and apply #[allow(clashing_extern_declarations)] so the real binding here can coexist with the export-only stubs. 2. Doc comment pointed at UpdateInteractiveCommand::ask_for_updates — no such function. The caller is prompt_for_updates (update_interactive_command.rs:1287, with the prompt_signal::install() call at L1322). Fix the reference. Zero behavioural change. cargo check (linux + x86_64-pc-windows-msvc) clean; 15/15 tests still pass.

#31254 landed on main with `#![deny(unreachable_pub)]`. The prompt_signal module was introduced before that merge with `pub fn install` / `pub struct Guard` / `pub fn uv_tty_reset_mode` — the externally-visible items need to be pub(crate), and the extern fn decl needs no visibility at all.

darwin-14-x64-test-bun failed with exit status -1 (agent kill, not a test assertion failure) on an unrelated lane — all other test-bun lanes including darwin-aarch64, alpine {x64,aarch64,baseline}, debian-13 {x64,aarch64,asan,baseline}, ubuntu-25.04, and windows {2019-x64, 2019-x64-baseline, 11-aarch64} are green. Diff is Rust + TypeScript only.

The napi_body.rs references in the uv_tty_reset_mode comment rotted twice already (own insertion, then rebase). Point at the uv_functions_to_export module and the uv_os_getpid NOTE by name instead, and drop the wtf-bindings.cpp line number too.

Review feedback: the Unix handler used _exit(128+sig), which makes the parent see WIFEXITED/130 instead of death-by-signal. The codebase convention (onExitSignal in c-bindings.cpp, Global:: raise_ignoring_panic_handler) is signal(sig, SIG_DFL) + raise(sig) so the parent observes WIFSIGNALED / signalCode === "SIGINT". Also switch the termios restore from uv_tty_reset_mode to bun_restore_stdio — the exact call onExitSignal makes (the module doc previously misattributed uv_tty_reset_mode to onExitSignal; the two are distinct mechanisms per the note in wtf-bindings.cpp). This covers all three stdio fds from the startup snapshot and blocks SIGTTOU around the tcsetattr. Dropping the local uv_tty_reset_mode extern also removes the clashing_extern_declarations against napi_body's export stub, so the #[allow] workaround there is reverted. SIGINT test now asserts proc.signalCode === "SIGINT" (proc.exited still resolves to 130 for signaled children). 16/16 tests pass; windows cross-check clean.

The switch to bun_restore_stdio left three comments still naming uv_tty_reset_mode (two Windows-analogue notes in prompt_signal.rs, one in the SIGINT test header). Update them to reference the call the code actually makes.

Mirror the sibling tests in this file: Promise.all over stdout/stderr/exited so a failed setup install reports its stderr instead of a bare exit code, and a per-tempdir BUN_INSTALL_CACHE_DIR so the concurrent tests don't share the global cache.

The ENABLE_PROCESSED_INPUT rationale was stated three times (module doc plus both caller sites); keep the module doc as the single source and reduce the caller sites to short pointers. Also replace three cross-file line-number references in the Ctrl+C tests with test names, the same rot class already fixed on the Rust side.

claude

Code review found no issues

No high-confidence issues detected in this change.

robobun · 2026-09-08T11:56:48Z

Closing in favor of #41977. It fixes the same hidden cursor and mouse reporting on Ctrl+C or an outside signal (#30890), but through the exit hook that already restores termios (bun_restore_stdio() on POSIX, Bun__restoreWindowsStdio on Windows) instead of a second, prompt-scoped signal handler. That also covers the crash handler and Global::exit paths and the bun init menu with the same code.

robobun mentioned this pull request May 16, 2026

Cursor does not reset when pressing Ctrl+C during interactive update #30890

Open

github-actions Bot added the claude label May 16, 2026

coderabbitai Bot reviewed May 16, 2026

View reviewed changes

Comment thread test/cli/update_interactive_install.test.ts Outdated