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.
This commit is contained in:
@@ -6,5 +6,26 @@ pub(crate) use tokio::process::Child;
|
||||
pub(crate) fn spawn(command: std::process::Command) -> std::io::Result<Child> {
|
||||
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()
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user