From 5784d727a0d473f69106b9f6ad2af253e6368709 Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 10 Jun 2026 07:49:34 +0200 Subject: [PATCH] fix(tui): fixed Windows terminal rendering after console codepage flips - Applied CREATE_NO_WINDOW and CREATE_NEW_PROCESS_GROUP when spawning Windows child processes to isolate console use. - Removed per-command Windows flag mutations so process-group/console flags are now applied uniformly in the central spawn path. - Added a win32-safe terminal write guard that restored UTF-8 input/output codepages before each write when drift was detected. --- .../src/sys/tokio_process.rs | 21 +++++ .../src/sys/windows/commands.rs | 27 ++++--- packages/tui/CHANGELOG.md | 1 + packages/tui/src/terminal.ts | 77 +++++++++++++++++++ .../tui/test/render-stable-prefix.test.ts | 3 +- 5 files changed, 117 insertions(+), 12 deletions(-) diff --git a/crates/brush-core-vendored/src/sys/tokio_process.rs b/crates/brush-core-vendored/src/sys/tokio_process.rs index 27a7409b5..6de639dbb 100644 --- a/crates/brush-core-vendored/src/sys/tokio_process.rs +++ b/crates/brush-core-vendored/src/sys/tokio_process.rs @@ -6,5 +6,26 @@ pub(crate) use tokio::process::Child; pub(crate) fn spawn(command: std::process::Command) -> std::io::Result { let mut command = tokio::process::Command::from(command); command.kill_on_drop(true); + // Isolate every external child from the host's console: + // + // - `CREATE_NO_WINDOW` gives the child its own *invisible* console instead + // of attaching it to ours. Console-sharing children can mutate shared + // console state behind the host's back — most notably the output + // codepage (PHP >=7.1 CLI issues the equivalent of `chcp` and skips the + // restore when killed; php.net request #73716), which degraded every + // non-ASCII glyph a hosting TUI painted into CP437 mojibake (`Γöé`). + // Inherited stdio handles are unaffected (handle-routed, not + // console-routed); interactive commands belong to the PTY path, which + // provisions a dedicated ConPTY anyway. + // - `CREATE_NEW_PROCESS_GROUP` makes the child a ctrl-event group root. + // Windows cannot join an existing group, so this is applied uniformly + // here rather than per-command (`creation_flags` replaces rather than + // ORs; the `sys::windows::commands` ext traits intentionally leave + // creation flags alone). + #[cfg(windows)] + { + use windows_sys::Win32::System::Threading::{CREATE_NEW_PROCESS_GROUP, CREATE_NO_WINDOW}; + command.creation_flags(CREATE_NEW_PROCESS_GROUP | CREATE_NO_WINDOW); + } command.spawn() } diff --git a/crates/brush-core-vendored/src/sys/windows/commands.rs b/crates/brush-core-vendored/src/sys/windows/commands.rs index f52b5bce6..c22d0a3a3 100644 --- a/crates/brush-core-vendored/src/sys/windows/commands.rs +++ b/crates/brush-core-vendored/src/sys/windows/commands.rs @@ -1,8 +1,12 @@ //! Command execution utilities. +//! +//! On Windows, process creation flags are applied uniformly in +//! `sys::process::spawn` (`CREATE_NEW_PROCESS_GROUP | CREATE_NO_WINDOW`); the +//! per-command extension methods below intentionally do not touch creation +//! flags, because `CommandExt::creation_flags` replaces rather than ORs and +//! two writers would silently clobber each other. -use std::{ffi::OsStr, os::windows::process::CommandExt as WindowsCommandExt}; - -use windows_sys::Win32::System::Threading::CREATE_NEW_PROCESS_GROUP; +use std::ffi::OsStr; use crate::{ShellFd, error, openfiles}; @@ -34,10 +38,9 @@ impl CommandExt for std::process::Command { self } - fn process_group(&mut self, pgroup: i32) -> &mut Self { - if pgroup == 0 { - self.creation_flags(CREATE_NEW_PROCESS_GROUP); - } + fn process_group(&mut self, _pgroup: i32) -> &mut Self { + // NOTE: Windows cannot join an existing process group, and new-group + // creation is handled uniformly by `sys::process::spawn`. self } } @@ -95,19 +98,21 @@ pub trait CommandFgControlExt { impl CommandFgControlExt for std::process::Command { fn take_foreground(&mut self) { - self.creation_flags(CREATE_NEW_PROCESS_GROUP); + // NOTE: no terminal foregrounding on Windows; group/console flags are + // applied uniformly by `sys::process::spawn`. } fn lead_session(&mut self) { - self.creation_flags(CREATE_NEW_PROCESS_GROUP); + // NOTE: no sessions on Windows; group/console flags are applied + // uniformly by `sys::process::spawn`. } } /// Extension trait for detaching a command from the parent's controlling terminal. pub trait CommandSessionExt { /// Arranges for the command to run in a new POSIX session with no controlling - /// terminal. On Windows this is a no-op; process-group behavior is handled - /// by `CommandFgControlExt` via `CREATE_NEW_PROCESS_GROUP`. + /// terminal. On Windows this is a no-op; process-group and console behavior + /// are handled uniformly by `sys::process::spawn`. fn detach_session(&mut self); } diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 40fada74d..5a1c6de23 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -24,6 +24,7 @@ ### Fixed +- Fixed Windows rendering degrading into CP437 mojibake (`Γöé`/`ΓöÇ` instead of box-drawing borders and Nerd Font glyphs) after a console-sharing child process changed the console codepage (e.g. PHP CLI's implicit `chcp`, php.net request #73716): the breakage stayed latent until the next full repaint such as ctrl+o expand. The terminal now re-asserts the UTF-8 codepage (output and input) before each stdout write - Fixed crash recovery leaving the shell unusable: `emergencyTerminalRestore` (and `terminal.stop()`) never left the alt screen nor disabled mouse tracking, so a crash during a fullscreen overlay stranded the user on the alternate buffer with any-motion mouse reporting spewing escape garbage until a manual `reset` - Fixed bracketed paste with a lost `ESC[201~` end marker (ssh/tmux truncation) silently eating all subsequent input forever while growing memory unboundedly — paste mode now has an inactivity watchdog (1s) and a byte cap (64 MiB) that exit paste mode and deliver the accumulated bytes through the paste event - Fixed vertical cursor movement using UTF-16 code units as visual columns: Up/Down over emoji/CJK lines could land the cursor mid-surrogate-pair, rendering a lone surrogate and permanently corrupting the buffer on the next insert; movement now walks graphemes and snaps the target offset to a cluster boundary, also fixing column drift across wide glyphs diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index 26f852f09..80c42db4d 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -125,6 +125,79 @@ let terminalEverStarted = false; const STD_INPUT_HANDLE = -10; const ENABLE_VIRTUAL_TERMINAL_INPUT = 0x0200; +/** UTF-8 codepage id for SetConsoleCP/SetConsoleOutputCP. */ +const CP_UTF8 = 65001; + +/** + * Lazily-initialized closure re-asserting the UTF-8 console codepage, or + * `null` when unavailable (non-win32, FFI failure, console detached). + */ +let consoleCodepageGuard: (() => void) | null | undefined; + +/** + * Re-assert the UTF-8 console codepage before writing (win32 only). + * + * Bun sets both console codepages to UTF-8 (65001) at startup, and + * `process.stdout.write(string)` hands UTF-8 bytes to `WriteFile`, which + * conhost translates using the *current* console output codepage. Child + * processes spawned by tools (bash commands, MCP/LSP servers, eval kernels) + * share this console, and some flip the codepage behind our back: PHP >=7.1 + * CLI issues the equivalent of `chcp` whenever `internal_encoding` mismatches + * the console codepage (php.net request #73716) and skips the restore when + * killed — and two PHP processes in a pipeline race their restores. Once the + * codepage falls back to an OEM page (437/850), every non-ASCII glyph the TUI + * paints is mis-translated: box-drawing borders degrade into `Γöé`/`ΓöÇ` + * mojibake on the next full repaint (most visibly ctrl+o expand, which + * rewrites every row). + * + * `GetConsoleOutputCP` is one cheap console call per `#safeWrite`; the setter + * only runs after a foreign flip. A reading of 0 means "no console" — leave + * that alone. Guarding the write chokepoint (rather than per-spawn cleanup) + * covers every console-sharing child and long-running processes that flip + * the codepage mid-session. + */ +function ensureWindowsConsoleUtf8(): void { + if (consoleCodepageGuard === undefined) consoleCodepageGuard = createConsoleCodepageGuard(); + consoleCodepageGuard?.(); +} + +let lastWarnedCodepage = 0; + +function createConsoleCodepageGuard(): (() => void) | null { + if (process.platform !== "win32") return null; + try { + const kernel32 = dlopen("kernel32.dll", { + GetConsoleOutputCP: { args: [], returns: FFIType.u32 }, + SetConsoleOutputCP: { args: [FFIType.u32], returns: FFIType.bool }, + GetConsoleCP: { args: [], returns: FFIType.u32 }, + SetConsoleCP: { args: [FFIType.u32], returns: FFIType.bool }, + }); + return () => { + try { + const outCp = kernel32.symbols.GetConsoleOutputCP(); + if (outCp !== 0 && outCp !== CP_UTF8) { + kernel32.symbols.SetConsoleOutputCP(CP_UTF8); + if (outCp !== lastWarnedCodepage) { + lastWarnedCodepage = outCp; + logger.warn("console output codepage changed by a child process; restoring UTF-8", { + codepage: outCp, + }); + } + } + const inCp = kernel32.symbols.GetConsoleCP(); + if (inCp !== 0 && inCp !== CP_UTF8) { + kernel32.symbols.SetConsoleCP(CP_UTF8); + } + } catch { + // Console APIs failed (console detached mid-session); disable the guard. + consoleCodepageGuard = null; + } + }; + } catch { + // bun:ffi unavailable; rendering proceeds without the guard. + return null; + } +} /** * Emergency terminal restore - call this from signal/crash handlers * Resets terminal state without requiring access to the ProcessTerminal instance @@ -1127,6 +1200,10 @@ export class ProcessTerminal implements Terminal { // Skip control sequences when stdout isn't a TTY (piped output, tests, log // files). They serve no purpose there and would surface as visible noise. if (!process.stdout.isTTY) return; + // A console-sharing child process may have flipped the console codepage + // away from UTF-8; repair it before any bytes hit WriteFile so no frame + // is ever translated through an OEM codepage. See ensureWindowsConsoleUtf8. + if (process.platform === "win32") ensureWindowsConsoleUtf8(); try { // Windows ConPTY drops viewport tracking when a single write exceeds // ~32-64 KB: the host UI's scroll position stays parked at wherever diff --git a/packages/tui/test/render-stable-prefix.test.ts b/packages/tui/test/render-stable-prefix.test.ts index cb842e13a..035092a61 100644 --- a/packages/tui/test/render-stable-prefix.test.ts +++ b/packages/tui/test/render-stable-prefix.test.ts @@ -99,7 +99,8 @@ describe("RenderStablePrefix engine contract", () => { expect(duplicated).toEqual([]); // And in original append order. - expect(buffer.match(/ROW-\d{3}/g) ?? []).toEqual(markers); + const observedMarkers = Array.from(buffer.matchAll(/ROW-\d{3}/g), match => match[0]); + expect(observedMarkers).toEqual(markers); } finally { tui.stop(); await term.flush();