[robobun]

What does this PR do?

Fixes #5267
Fixes #27542

node:readline, bun repl, and anything else that calls setRawMode(true) clear ICANON, ECHO, ISIG and ICRNL on the tty. While the tty is in that state:

• Enter delivers a bare \r (no CR→LF translation)
• typed characters are not echoed
• Ctrl+C is not translated into SIGINT

alert(), confirm() and prompt() all block on a read that waits for \n, so calling any of them from inside a readline callback (or from bun repl) appeared to hang with no visible input and no way to interrupt.

Repro

const rl = require("node:readline").createInterface({ input: process.stdin, output: process.stdout });
rl.question("rl? ", () => {
  const name = prompt("Name?"); // hung: nothing echoed, Enter ignored, Ctrl+C ignored
  console.log(name);
  rl.close();
});

Fix

Wrap each dialog in an RAII CookedStdinGuard that, on POSIX,

1. saves the current termios with tcgetattr,
2. OR's in ICANON | ECHO | ISIG | IEXTEN and ICRNL,
3. applies with tcsetattr(TCSANOW), and
4. restores the saved termios on drop.

The guard is installed before the prompt text is written: the line discipline applies ICRNL at receive time, so a \r that arrives between the prompt becoming visible and tcsetattr running stays a \r in the input queue and the canonical read never sees a line.

tcgetattr fails on non-tty stdin, so piped input keeps its existing behaviour (the guard is inert). After the read, whatever mode the caller had installed (raw for readline/repl) is put back exactly as it was, so readline keeps working.

On Windows the same guard wraps StdinModeGuard with ENABLE_LINE_INPUT | ENABLE_ECHO_INPUT | ENABLE_PROCESSED_INPUT set and ENABLE_VIRTUAL_TERMINAL_INPUT cleared, applied to all three functions (previously only prompt() touched console mode, and only to clear the VT-input flag). The guard is inert when stdin is not a console.

Why this fix rather than accepting \r

An earlier attempt (#27544) taught the readers to accept a bare \r. That unblocks Enter but leaves the user typing blind (no echo, no line editing) and unable to Ctrl+C. Restoring cooked mode for the duration of the read fixes all three at once and is exactly what a user expects from a dialog-style prompt.

How did you verify your code works?

• test/js/web/web-globals.test.js: a PTY-backed test drives readline → prompt() → confirm() → alert() → readline again and asserts that each call returns the typed value, the typed characters are echoed, and raw mode is observably restored afterwards (readline's echo of the second answer appears exactly once; if the guard left stdin cooked the kernel would echo it too).
• test/js/bun/repl/repl.test.ts: a withTerminalRepl test types prompt(...), confirm(...) and alert(...) at the REPL prompt and asserts each returns and the REPL keeps evaluating.
• Both tests time out on an unpatched build and pass with the fix. The existing piped-stdin confirm tests in web-globals.test.js continue to pass on Linux and Windows.

The PTY tests are POSIX-only; the Windows REPL terminal suite is already todoIf(isWindows), and the existing piped-stdin tests verify the Windows guard is inert on non-console stdin.

---

no test proof · iteration 3 · 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 test/js/web/web-globals.test.js

[coderabbitai]

Walkthrough

Changes

Blocking dialog stdin handling

Layer / File(s)	Summary
Cooked stdin guard src/runtime/webcore/prompt.rs	Adds a platform-specific RAII guard that switches stdin to cooked mode for blocking dialogs and restores the previous mode when dropped.
Dialog integration and regression coverage src/runtime/webcore/prompt.rs, test/js/bun/repl/repl.test.ts, test/js/web/web-globals.test.js	Applies the guard to alert, confirm, and prompt, with tests covering REPL dialogs and readline raw-mode interactions.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address #5267 and #27542 by restoring cooked terminal mode for prompt, confirm, and alert, with matching regression tests.
Out of Scope Changes check	✅ Passed	The added code and tests stay focused on the stdin-mode fix and its regressions, with no clear unrelated changes.
Title check	✅ Passed	The title clearly matches the main change: forcing cooked stdin for prompt, alert, and confirm during blocking reads.
Description check	✅ Passed	The description includes the required purpose and verification sections, with concrete fix details and test coverage.

---

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

[robobun]

^{Updated 3:24 AM PT - Jul 25th, 2026}

❌ @robobun, your commit ac2daf5 has some failures in Build #80253 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 35464

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

bun-35464 --bun

[coderabbitai]

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)

test/js/bun/repl/repl.test.ts (1)

1-1: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add explicit timeouts to the new multi-round-trip dialog tests. Both newly added tests drive several sequential terminal write/wait cycles on top of process-spawn overhead without an explicit per-test timeout, the same test shape for which this repo already has a documented mitigation (explicit ~15000ms timeouts on comparable PTY tests in repl.test.ts).

• test/js/bun/repl/repl.test.ts#L1183-1203: add an explicit timeout (e.g., 15000) to the test(...) call, matching the file's existing convention for withTerminalRepl-based tests.
• test/js/web/web-globals.test.js#L360-440: add an explicit timeout (e.g., 15000) to the test.skipIf(isWindows)(...) call given its 5 sequential write/wait round trips plus child-process spawn overhead.

🤖 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/js/bun/repl/repl.test.ts` at line 1, Add explicit 15000ms per-test
timeouts to the newly added multi-round-trip dialog tests: the
withTerminalRepl-based test in repl.test.ts and the test.skipIf(isWindows) case
in web-globals.test.js. Preserve their existing bodies and assertions while
matching the repository’s timeout convention.

Source: Learnings

🤖 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/js/bun/repl/repl.test.ts`:
- Around line 1183-1203: Update the “alert/confirm/prompt return when called
from the REPL” test to pass an explicit approximately 15000ms timeout, following
the established timeout convention used by comparable PTY tests in this file.
Keep the existing dialog round-trips and final evaluation unchanged.

In `@test/js/web/web-globals.test.js`:
- Around line 360-440: Add an explicit extended timeout to the multi-round-trip
test registered by “alert/confirm/prompt work while readline has stdin in raw
mode,” using the test framework’s existing timeout option or convention. Keep
the five terminal interaction steps and assertions unchanged.

---

Outside diff comments:
In `@test/js/bun/repl/repl.test.ts`:
- Line 1: Add explicit 15000ms per-test timeouts to the newly added
multi-round-trip dialog tests: the withTerminalRepl-based test in repl.test.ts
and the test.skipIf(isWindows) case in web-globals.test.js. Preserve their
existing bodies and assertions while matching the repository’s timeout
convention.

🪄 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: 9219ca1c-aa8b-48ae-b849-66200552c03a

📥 Commits

Reviewing files that changed from the base of the PR and between 028f7a3 and 98744fa.

📒 Files selected for processing (3)

• src/runtime/webcore/prompt.rs
• test/js/bun/repl/repl.test.ts
• test/js/web/web-globals.test.js

[claude]

No further findings — all four earlier comments (Windows no-op guard, per-test timeout, dead exit() reject arm, clean-exit race) are addressed in the current diff. Deferring to a human for the final look since this changes TTY-mode handling for three user-facing globals and the Windows raw-mode path has no automated coverage (both new tests skip on Windows).

What was reviewed:

• CookedStdinGuard on both platforms: POSIX termios save/OR-in ICANON|ECHO|ISIG|IEXTEN+ICRNL/restore via TCSADRAIN; Windows wraps the existing StdinModeGuard with ENABLE_LINE_INPUT|ECHO|PROCESSED set and VT-input cleared — verified all referenced bun_sys items exist and the guard is inert on non-TTY stdin.
• Guard placement in alert/confirm/prompt: created after Output::flush() and lives across the whole blocking read; prompt()'s previous Windows-only StdinModeGuard is subsumed.
• New PTY test: proc.exited now rejects pending waitFors only on non-zero exit, avoiding the final-read race; asserts echo was on during prompt() and raw mode was restored for readline afterwards.

Extended reasoning...

Overview

Adds an RAII CookedStdinGuard in src/runtime/webcore/prompt.rs and inserts it into alert(), confirm(), and prompt() immediately before their blocking stdin reads. On Unix the guard tcgetattrs fd 0, ORs in ICANON|ECHO|ISIG|IEXTEN and ICRNL, applies with TCSADRAIN, and restores the saved termios on drop; on Windows it wraps the existing bun_sys::windows::StdinModeGuard with line/echo/processed input set and VT-input cleared. Two new PTY-backed tests cover the readline and bun repl paths.

Prior review history

I left four inline findings on earlier revisions, all now resolved:

• The Windows guard was a no-op stub → fixed in 191190c (now wraps StdinModeGuard for all three functions).
• Explicit 20_000 per-test timeout → dropped in b6ae9bc.
• Terminal exit() callback doesn't fire for a pre-constructed Bun.Terminal on child exit → d829957 keys off proc.exited instead.
• That handler raced the final PTY read on clean exit → d7604c2 skips rejection when code === 0.

I cross-checked bun_sys::posix::{tcgetattr, tcsetattr, TCSA} and bun_sys::windows::{StdinModeGuard, UpdateStdioModeFlagsOpts, ENABLE_*} — all exist with the shapes the PR uses. UpdateStdioModeFlagsOpts has exactly set/unset so the struct literal without ..Default::default() is complete.

Security risks

None. No untrusted input is parsed; the guard only toggles well-known termios/console-mode bits on fd 0 and restores them exactly. On non-TTY stdin tcgetattr/GetConsoleMode fail and the guard is inert, so piped-input behavior is unchanged (verified by the existing confirm (yes/no) tests that still run on all platforms).

Level of scrutiny

Medium. It's a targeted bugfix at the right layer (the blocking-read call sites own the invariant that stdin be in line mode for the duration), but it changes observable behavior of three Web-API globals whenever stdin is a TTY, and the Windows raw-mode path is untested — the new PTY test is skipIf(isWindows) and the REPL terminal suite is already todoIf(isWindows). The Windows code path reuses an existing in-tree guard and the flag values match the Win32 constants, so risk is low, but a human should sign off on shipping an untested platform branch.

Other factors

The tests are well-constructed per REVIEW.md: event-driven waitFor, every failure path (non-zero child exit) rejects the awaited promise with a diagnostic including accumulated PTY output, and the web-globals test additionally asserts the negative contract (raw mode is restored — kernel does not double-echo the second readline answer). CodeRabbit's contradictory "add a timeout" nits were correctly declined per test/CLAUDE.md. Given four rounds of iteration were needed to reach this state, it doesn't meet the "simple/mechanical/obvious" bar for auto-approval.

[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/js/web/web-globals.test.js`:
- Around line 398-410: Update the child-exit handling around proc.exited and
waitFor so a non-zero exit is stored when no waiters are pending. In waitFor,
immediately reject new callers with the persisted failure instead of enqueueing
and pumping; preserve current waiter rejection behavior when waiters exist and
successful exits remain unaffected.

🪄 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: af418323-8ed3-4f17-904a-67a4f4ec7e02

📥 Commits

Reviewing files that changed from the base of the PR and between 98744fa and 44966d0.

📒 Files selected for processing (2)

• src/runtime/webcore/prompt.rs
• test/js/web/web-globals.test.js

[claude]

No issues found, but this is ~90 lines of new platform-gated termios/console-mode manipulation applied to three call sites, and the Windows branch has no positive PTY coverage (both new tests are skipIf(isWindows) / inside describe.todoIf(isWindows)), so worth a human look.

What was reviewed:

• POSIX guard: tcgetattr → OR in ICRNL + ICANON|ECHO|ISIG|IEXTEN → tcsetattr(TCSADRAIN), restore on Drop; inert on non-tty — matches bun_sys::posix signatures.
• Windows guard: wraps the existing StdinModeGuard with set/unset fields that exist on UpdateStdioModeFlagsOpts; replaces the old prompt-only VT-input clear.
• Idempotence when stdin is already cooked (flags OR'd in are already set; exact original restored), and existing piped-stdin confirm tests still exercise the inert path on both platforms.
• The two PTY tests' waiter/exit plumbing after the four rounds of fixes (clean-exit early-return, persisted childExitError).

Extended reasoning...

Overview

The PR fixes #5267 / #27542: alert(), confirm() and prompt() hang when called while stdin is in raw mode (inside a node:readline callback or from bun repl). It introduces an RAII CookedStdinGuard in src/runtime/webcore/prompt.rs (~80 lines across #[cfg(unix)] / #[cfg(windows)] / fallback) and drops one let _cooked = CookedStdinGuard::new(); into each of the three functions, replacing the old prompt-only Windows StdinModeGuard block. Two PTY-driven tests are added: one in test/js/web/web-globals.test.js driving readline → prompt/confirm/alert → readline, and one in test/js/bun/repl/repl.test.ts driving the same three dialogs from the REPL.

Security risks

None identified. The guard reads and writes termios / console mode on fd 0 via existing bun_sys wrappers, restores exactly what it saved, and is inert when stdin is not a tty/console. No user-controlled data reaches the termios path.

Level of scrutiny

Medium. This is native code that mutates process-global terminal state on every alert/confirm/prompt call, with three cfg-gated implementations. The POSIX side is textbook and well-covered by the new PTY tests. The Windows side reuses the pre-existing StdinModeGuard helper (already inert on non-console stdin per its doc comment) but adds three new set flags and applies it to two functions (alert, confirm) that previously touched nothing on Windows — and neither new test runs there. The existing piped-stdin confirm tests do run on Windows and verify the guard is inert on non-console input, but there is no positive Windows test that raw-mode → dialog → raw-mode actually works. A maintainer should at minimum sanity-check the Windows flag choice (ENABLE_LINE_INPUT | ENABLE_ECHO_INPUT | ENABLE_PROCESSED_INPUT set, ENABLE_VIRTUAL_TERMINAL_INPUT cleared).

Other factors

• The PR explicitly chooses this approach over the earlier #27544 (accept bare \\r) and documents why; that's a reasonable design call but one a maintainer may want to acknowledge.
• Four earlier rounds of bot review feedback (Windows guard was a no-op stub; per-test timeout; dead exit() reject arm; success-path exit race; persist exit error for later waiters) have all been addressed in follow-up commits and every thread is resolved.
• No human reviewer has looked at this PR yet.

[claude]

No issues found in this pass, but this changes process-global tty/console state around three user-facing globals (alert/confirm/prompt) on both POSIX and Windows, so it's worth a human look — particularly the Windows branch, whose expanded ENABLE_LINE_INPUT|ECHO_INPUT|PROCESSED_INPUT behaviour has no direct test (both new PTY tests are POSIX-only).

What was reviewed:

• RAII shape of CookedStdinGuard: tcgetattr failure → inert; Drop restores saved termios on every early-return path including the to_slice(global)? throw.
• Guard ordering vs Output::flush() and the retained comment justifying it.
• Windows UpdateStdioModeFlagsOpts has exactly set/unset, so the literal without ..Default::default() is complete; StdinModeGuard is inert on non-console stdin.
• Test-side waitFor/proc.exited wiring after the d7604c2/20fe4828 fixes — the clean-exit race and the between-calls exit gap are both handled.

Extended reasoning...

Overview

The PR adds a CookedStdinGuard RAII struct in src/runtime/webcore/prompt.rs that forces stdin into canonical mode (POSIX: OR in ICANON|ECHO|ISIG|IEXTEN on c_lflag and ICRNL on c_iflag; Windows: wrap the existing StdinModeGuard with ENABLE_LINE_INPUT|ECHO_INPUT|PROCESSED_INPUT set and ENABLE_VIRTUAL_TERMINAL_INPUT unset) for the duration of the blocking line read inside alert(), confirm() and prompt(), restoring the caller's mode on drop. It removes the old prompt-only Windows VT-input unset (now subsumed) and adds two PTY-driven regression tests: a readline→prompt/confirm/alert→readline round-trip in web-globals.test.js and a REPL interaction in repl.test.ts.

Security risks

None identified. The guard reads and restores termios/console mode on fd 0 via the in-tree bun_sys wrappers; no untrusted input reaches a size/index computation, no new allocation, no privilege change. The only process-global side effect is the tty mode toggle, which is scoped by RAII and inert on non-tty stdin.

Level of scrutiny

Medium-high. This is a small diff (~90 native LOC) but it changes user-visible behaviour of three global Web APIs and mutates process-global terminal state on every call. A restore-path bug would leave the user's terminal in the wrong mode after prompt() returns. The POSIX path is well-covered by the new PTY test (which explicitly asserts raw mode is put back — the single-echo check on rl2), but the Windows branch's expanded flag set is exercised only indirectly by the pre-existing piped-stdin confirm tests, which verify the guard is inert, not that it does the right thing on a real console. The describe.todoIf(isWindows) REPL suite and test.skipIf(isWindows) PTY test mean the Windows raw-mode path ships without a direct regression test.

Other factors

• All four earlier review findings (Windows coverage of alert/confirm, per-test timeout, dead exit() reject arm, clean-exit race with the final PTY read) were addressed in follow-up commits and every thread is resolved.
• The guard uses the in-tree bun_sys::posix::tcgetattr/tcsetattr and bun_sys::windows::StdinModeGuard helpers rather than raw libc, per REVIEW.md.
• The guard is installed before arguments[0].to_slice(global)?, so a throwing toString on the message argument still hits Drop and restores the saved termios — checked.
• Minor: the PR description says TCSADRAIN but the code uses TCSA::Now. Since the guard runs before any output is written this is equivalent; noting only so a reviewer isn't surprised by the mismatch.

Deferring rather than approving because this is a cross-platform runtime behaviour change with one platform branch effectively untested, not because anything looks wrong.

[claude]

No issues found, but I'd like a human to sign off: this is cross-platform #[cfg]-gated native code that manipulates process-global tty/console state, and the Windows path broadens behavior (all three dialogs now set ENABLE_LINE_INPUT|ECHO_INPUT|PROCESSED_INPUT, vs. only prompt() clearing VT-input before) with no new Windows test coverage — both new tests are POSIX-only.

What was reviewed:

• CookedStdinGuard RAII: tcgetattr failure on non-tty makes it inert; ? early-returns after guard construction correctly restore termios via Drop.
• Verified bun_sys::posix::{tcgetattr,tcsetattr,TCSA}, UpdateStdioModeFlagsOpts field set, and the ENABLE_* constants all exist as used.
• Confirmed all prior inline threads (exit-race in waitFor, persisted exit error, comment-cop) are addressed in the current diff.

Extended reasoning...

Overview

The PR wraps alert(), confirm() and prompt() in a new CookedStdinGuard RAII type that forces stdin into canonical/cooked mode for the duration of the blocking line read, then restores the caller's termios/console mode. On POSIX it uses bun_sys::posix::tcgetattr/tcsetattr to OR in ICANON|ECHO|ISIG|IEXTEN and ICRNL; on Windows it wraps the existing bun_sys::windows::StdinModeGuard with ENABLE_LINE_INPUT|ENABLE_ECHO_INPUT|ENABLE_PROCESSED_INPUT set and ENABLE_VIRTUAL_TERMINAL_INPUT unset. The old Windows-only VT-input clear inside prompt() is removed as subsumed. Two PTY-driven tests are added (readline→dialogs in web-globals.test.js, and a REPL test in repl.test.ts); both are POSIX-only.

Security risks

None identified. The change reads/writes termios on fd 0 only, via existing in-tree wrappers, and is inert when stdin is not a tty/console. No untrusted input reaches the termios path.

Level of scrutiny

Medium. This is not large, but it is platform-gated native runtime code that mutates process-global tty state on a user-facing Web API. REVIEW.md calls out cross-platform/#[cfg]-gated changes as needing extra care, and the Windows arm broadens behavior relative to the previous code (previously only prompt() touched console mode, and only to clear VT-input; now all three dialogs also set line/echo/processed input) without any new Windows test — the REPL terminal suite is todoIf(isWindows) and the new web-globals test is skipIf(isWindows). The existing piped-stdin confirm tests only verify the guard is inert on non-console stdin, not the new cooked-mode behavior on a Windows console.

Other factors

• All referenced helpers exist and match the signatures used: bun_sys::posix::{tcgetattr,tcsetattr,TCSA::Now} (with Termios = libc::termios), UpdateStdioModeFlagsOpts { set, unset } (exactly two fields, so no ..Default::default() needed), StdinModeGuard::set, and the ENABLE_* constants.
• The guard is armed before arguments[0].to_slice(global)?, so a throwing toString on the message correctly restores termios via Drop on ? propagation.
• All prior review threads (mine on the waitFor exit wiring and the code-0 race, CodeRabbit's persisted-exit-error, comment-cop) are resolved and reflected in the current diff.
• The one retained doc comment on CookedStdinGuard was justified by the author as documenting a load-bearing ordering invariant (ICRNL applies at receive time), which fits REVIEW.md's "comments carry only durable non-obvious content" rule.

Given the untested Windows behavior expansion and the process-global-state nature of the change, deferring to a human reviewer rather than approving.

[robobun]

The diff is green: both new tests (test/js/web/web-globals.test.js and test/js/bun/repl/repl.test.ts) pass on every lane in build 80253, and the existing piped-stdin confirm tests continue to pass on Linux and Windows.

The remaining failures in that build are pre-existing flaky tests unrelated to this change (all passed on retry): bun-security-scanner-matrix-*.test.ts, test-fastutf8stream-reopen.js, in-process-cron.test.ts, webview-chrome.test.ts, test-repl-close.js, no-orphans.test.ts. None of them touch prompt.rs, readline, or terminal mode.

Ready for review.

[github-actions]

Closing this PR because it has been inactive for more than 90 days.

[robobun]

Superseded by #46894. It keeps the behaviour of this PR (line mode before the question is written, the owner's mode written back after) and moves the guard to bun_core::tty beside RawModeGuard. It also leaves a terminal that is already in line mode alone, so stty -echo stays hidden and a dialog costs one tcgetattr there.