fix(runtime): real isatty/columns/rows/setRawMode + VT output on Windows - #6630
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughWindows runtime console handling is centralized in a new Windows-only module. TTY detection, window sizing, raw input, and VT output setup now use shared helpers across GC startup, TUI input, and tty operations, with tests for console and piped handles. ChangesWindows console integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant js_gc_init
participant termios_impl
participant win_console
participant WindowsConsole
js_gc_init->>win_console: enable_vt_output()
termios_impl->>win_console: set_stdin_raw(true)
termios_impl->>win_console: enable_vt_output()
win_console->>WindowsConsole: GetConsoleMode / SetConsoleMode
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Fixes the 2026-07-18 audit defects in
crates/perry-runtime/src/tty.rs: the#[cfg(not(unix))]arms hardwiredisattyto false andcolumns/rowstoNone(the TODO admitted it), andsetRawModewas a no-op returning false. Net effect on Windows:process.stdout.isTTYwas always false (every color library disabled colors),columns/rowswere alwaysundefined, and interactive stdin (inquirer-style prompts) was dead. The irony being fixed:builtins/console.rs:1414already used std'sis_terminal()— which works on Windows via the sameGetConsoleModeprobe — while tty.rs never got its Windows arms.What each piece does
New shared module
crates/perry-runtime/src/win_console.rs(#[cfg(windows)], windows-sys 0.61 — already a dependency withWin32_System_Console/Win32_Foundation, so no manifest change):is_console_fd(fd)—GetStdHandle(STD_{INPUT,OUTPUT,ERROR}_HANDLE)+GetConsoleModesuccess ⇒ console. Piped/redirected handles fail the probe ⇒ false, matching Node (libuv classifies a std handle asUV_TTYby the same criterion).window_size(fd)—GetConsoleScreenBufferInfosrWindowextents (Right-Left+1,Bottom-Top+1) for fd 1 (stdout) / fd 2 (stderr). Window, not scrollback buffer — matchesuv_tty_get_winsize.set_stdin_raw(enable)— clearENABLE_{LINE,ECHO,PROCESSED}_INPUT, setENABLE_VIRTUAL_TERMINAL_INPUT(arrow keys arrive as ANSI\x1b[A..D, matching the Unix parsers); cooked mode saved on first enable, restored on disable; "disable without prior enable" is a successful no-op — the exact contract of the Unix termios arm. The flag handling mirrors the working precedent inperry-stdlib/src/readline.rs:969-1047(tty.setRawMode + perry/tui input on Windows: SetConsoleMode plumbing #406) andperry-runtime/src/tui/input.rs.enable_vt_output()— setsENABLE_VIRTUAL_TERMINAL_PROCESSINGon console stdout/stderr. Deliberately does NOT setDISABLE_NEWLINE_AUTO_RETURN: the runtime writes bare\nvia Rust std I/O throughout, so it relies on the console's automatic NL→CRNL.tty.rs— real#[cfg(windows)]arms forisatty_impl/winsize_impl/set_fd_raw_modedelegating to the module above (setRawModeaccepts fd 0 only — the only fdGetStdHandlecan map to the console input handle). Non-unix non-windows targets keep the old stub behavior under#[cfg(not(any(unix, windows)))]. This flows through totty.isatty(fd),process.std{in,out,err}.isTTY(Node parity:trueorundefined, neverfalse),process.stdout.columns/.rows,getWindowSize, andReadStream.setRawMode.Startup VT enable —
js_gc_init(the once-per-program bootstrap every compiled binary calls first) now callswin_console::enable_vt_output()on Windows, so runtime-emitted escapes (console.clear, tty cursor ops, chalk-style color output unlocked byisTTYnow being true) render instead of printing literally. No-op for pipes/redirects (probe fails ⇒ untouched), a failingSetConsoleMode(legacy conhost) is ignored — it can never fail program startup. Idempotent.Dedup —
tui/input.rs's Windowstermios_impl(a copy of the readline.rs implementation) now delegates towin_console, so the flag math and save/restore live in one place inside perry-runtime.perry-stdlib/src/readline.rsis intentionally untouched: its enable() couples stdin raw mode with stdout VT in one saved pair, and consolidating it is a separate cleanup (no runtime→stdlib dependency was added; none was needed).Out of scope
resizeevents: Windows has no SIGWINCH — deliveringprocess.stdout.on('resize')needs event-loop polling of the console size (or a console input record reader), i.e. event-loop integration. The module doc now says so explicitly.columns/rowsreads are live (re-queried per access), so consumers that poll get fresh values.Tests + verification
win_console::tests::pipe_handle_is_not_a_console— a fresh anonymous pipe (std::io::pipe) must fail theGetConsoleModeprobe and have no screen-buffer info. Deterministic everywhere.win_console::tests::raw_input_mode_clears_cooked_flags_and_sets_vt_input— pure flag math, deterministic.win_console::tests::stdin_raw_mode_round_trip_when_console_available— save/restore round-trip on a real console handle, gated: whenGetConsoleModeon stdin fails (CI runners and piped invocations have no console) it asserts the enable-path failure and skips the round-trip with a message. Not flaky by construction.tty::tests::isatty_and_winsize_report_false_for_piped_std_handles— re-invokes the test binary with all three std streams piped and asserts in the child:isatty(0/1/2)false,isTTYundefined(notfalse— Node parity),columns/rowsundefined,setRawMode(true)fails. Deterministic whether or not the environment has a console.Ran on Windows 11 x64 / MSVC:
cargo check -p perry-runtime(warning set byte-identical to the origin/main baseline modulo line shifts — zero new warnings), the 11 tty+win_console tests (all pass; the console round-trip skipped in this headless session as designed), and the touched-module sweep (tty:: win_console:: tui:: gc:: process::— 538 passed; the 3 failures aregc::teststiming flakes that fail identically, with run-to-run set shifts, on an unmodified origin/main baseline). Caveats: the full perry-runtime Windows unit suite aborts indyn_eval::tests(non-unwinding panic) on unmodified origin/main too — pre-existing, unrelated; and interactive-console behavior (real colors/dimensions/raw prompts in a live terminal) was not exercisable from this headless session — the console-dependent paths are covered by the probe/flag tests above. Note origin/main currently doesn't compile on Windows at all (ExitProcessmissing-> !inprocess/env_misc.rs, pre-existing, fix belongs to PR #6609); verification used that one-line local overlay, which is NOT part of this PR.No version bump/changelog per maintainer instruction (external-contributor PR flow: maintainer folds in metadata at merge).
Summary by CodeRabbit
New Features
Bug Fixes