From a781761eda225ba85c2b3b64ba2b74c0c54fbecf Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Wed, 24 Jun 2026 11:27:44 -0700 Subject: [PATCH] [codex] fix Windows ConPTY input handling (#29734) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Windows unified-exec TTY input did not behave like the non-Windows PTY path. ConPTY sessions could receive the wrong line ending or mishandle backspace, especially when sending input to a foreground program through PowerShell or cmd. The local, legacy restricted, and elevated paths also handled this normalization separately. ## What changed - share one stateful Windows TTY input normalizer across local, legacy restricted, and elevated runner paths - translate LF and split CRLF into one Windows terminal Enter, encode backspace as DEL, and preserve UTF-8 and control bytes such as Ctrl-C - add Windows integration coverage for Unicode input, backspace, Enter, and PowerShell foreground-child Ctrl-C behavior ## Validation - `just test -p codex-utils-pty` (13 tests passed; the Unicode integration test retried once) - the Unicode integration test passed five consecutive runs with retries disabled - integration coverage sends `cafeé 漢字` through cmd and PowerShell and verifies that Ctrl-C interrupts a running PowerShell foreground child --- codex-rs/utils/pty/src/lib.rs | 4 + codex-rs/utils/pty/src/pty.rs | 4 + codex-rs/utils/pty/src/tests.rs | 5 +- codex-rs/utils/pty/src/windows_input.rs | 35 ++++ codex-rs/utils/pty/src/windows_input_tests.rs | 16 ++ codex-rs/utils/pty/src/windows_tests.rs | 157 ++++++++++++++++++ .../src/unified_exec/backends/legacy.rs | 6 +- .../unified_exec/backends/windows_common.rs | 22 +-- 8 files changed, 226 insertions(+), 23 deletions(-) create mode 100644 codex-rs/utils/pty/src/windows_input.rs create mode 100644 codex-rs/utils/pty/src/windows_input_tests.rs create mode 100644 codex-rs/utils/pty/src/windows_tests.rs diff --git a/codex-rs/utils/pty/src/lib.rs b/codex-rs/utils/pty/src/lib.rs index 9f0cb442a..ff0d16844 100644 --- a/codex-rs/utils/pty/src/lib.rs +++ b/codex-rs/utils/pty/src/lib.rs @@ -6,6 +6,8 @@ pub mod pty; mod tests; #[cfg(windows)] mod win; +#[cfg(windows)] +mod windows_input; pub const DEFAULT_OUTPUT_BYTES_CAP: usize = 1024 * 1024; @@ -39,3 +41,5 @@ pub use pty::spawn_process as spawn_pty_process; pub use win::PsuedoCon; #[cfg(windows)] pub use win::conpty::RawConPty; +#[cfg(windows)] +pub use windows_input::WindowsTtyInputNormalizer; diff --git a/codex-rs/utils/pty/src/pty.rs b/codex-rs/utils/pty/src/pty.rs index 951818a80..f2690c4de 100644 --- a/codex-rs/utils/pty/src/pty.rs +++ b/codex-rs/utils/pty/src/pty.rs @@ -217,7 +217,11 @@ async fn spawn_process_portable( let writer_handle: JoinHandle<()> = tokio::spawn({ let writer = Arc::clone(&writer); async move { + #[cfg(windows)] + let mut windows_input = crate::WindowsTtyInputNormalizer::default(); while let Some(bytes) = writer_rx.recv().await { + #[cfg(windows)] + let bytes = windows_input.normalize(&bytes); let mut guard = writer.lock().await; use std::io::Write; let _ = guard.write_all(&bytes); diff --git a/codex-rs/utils/pty/src/tests.rs b/codex-rs/utils/pty/src/tests.rs index 56fa486b3..f9255176f 100644 --- a/codex-rs/utils/pty/src/tests.rs +++ b/codex-rs/utils/pty/src/tests.rs @@ -16,6 +16,10 @@ use crate::spawn_pipe_process; use crate::spawn_pipe_process_no_stdin; use crate::spawn_pty_process; +#[cfg(windows)] +#[path = "windows_tests.rs"] +mod windows_tests; + fn find_python() -> Option { for candidate in ["python3", "python"] { if let Ok(output) = std::process::Command::new(candidate) @@ -140,7 +144,6 @@ async fn collect_output_until_exit( } } -#[cfg(unix)] async fn wait_for_output_contains( output_rx: &mut tokio::sync::broadcast::Receiver>, needle: &str, diff --git a/codex-rs/utils/pty/src/windows_input.rs b/codex-rs/utils/pty/src/windows_input.rs new file mode 100644 index 000000000..ea38e19e6 --- /dev/null +++ b/codex-rs/utils/pty/src/windows_input.rs @@ -0,0 +1,35 @@ +/// Stateful normalizer for bytes written to a Windows pseudoconsole. +/// +/// ConPTY accepts UTF-8 input, but an Enter key is represented by a carriage +/// return on Windows. This converts line feeds and collapses existing CRLF +/// sequences, including when the two bytes arrive in separate writes, so each +/// requested newline submits exactly one line. Backspace is encoded as DEL, +/// which ConPTY translates to `VK_BACK`. All other bytes, including UTF-8 and +/// terminal control characters, pass through unchanged. +#[derive(Default)] +pub struct WindowsTtyInputNormalizer { + previous_was_cr: bool, +} + +impl WindowsTtyInputNormalizer { + pub fn normalize(&mut self, bytes: &[u8]) -> Vec { + let mut normalized = Vec::with_capacity(bytes.len()); + for &byte in bytes { + match byte { + b'\x08' => normalized.push(b'\x7f'), + b'\n' => { + if !self.previous_was_cr { + normalized.push(b'\r'); + } + } + _ => normalized.push(byte), + } + self.previous_was_cr = byte == b'\r'; + } + normalized + } +} + +#[cfg(test)] +#[path = "windows_input_tests.rs"] +mod tests; diff --git a/codex-rs/utils/pty/src/windows_input_tests.rs b/codex-rs/utils/pty/src/windows_input_tests.rs new file mode 100644 index 000000000..5f0bbc4b4 --- /dev/null +++ b/codex-rs/utils/pty/src/windows_input_tests.rs @@ -0,0 +1,16 @@ +use super::WindowsTtyInputNormalizer; +use pretty_assertions::assert_eq; + +#[test] +fn normalizes_terminal_input_without_changing_text_or_ctrl_c() { + let mut normalizer = WindowsTtyInputNormalizer::default(); + assert_eq!(normalizer.normalize(b"first\n"), b"first\r"); + assert_eq!(normalizer.normalize(b"second\r"), b"second\r"); + assert_eq!(normalizer.normalize(b"\nthird\r\n"), b"third\r"); + + let mut input_with_controls = "cafeé 漢字".as_bytes().to_vec(); + input_with_controls.extend_from_slice(b"\x08\x03"); + let mut expected = "cafeé 漢字".as_bytes().to_vec(); + expected.extend_from_slice(b"\x7f\x03"); + assert_eq!(normalizer.normalize(&input_with_controls), expected); +} diff --git a/codex-rs/utils/pty/src/windows_tests.rs b/codex-rs/utils/pty/src/windows_tests.rs new file mode 100644 index 000000000..6923f266c --- /dev/null +++ b/codex-rs/utils/pty/src/windows_tests.rs @@ -0,0 +1,157 @@ +use super::collect_output_until_exit; +use super::combine_spawned_output; +use super::find_python; +use super::wait_for_output_contains; +use crate::TerminalSize; +use crate::spawn_pty_process; +use std::collections::HashMap; +use std::path::Path; + +const READY_MARKER: &str = "__CODEX_CHILD_READY__"; +const VALUE_MARKER: &str = "__CODEX_CHILD_VALUE__"; + +struct WindowsShell { + name: &'static str, + program: String, + args: Vec, + child_command: String, +} + +fn find_powershell() -> Option { + ["pwsh.exe", "powershell.exe"] + .into_iter() + .find_map(|candidate| { + std::process::Command::new(candidate) + .args(["-NoLogo", "-NoProfile", "-Command", "exit 0"]) + .status() + .ok() + .filter(std::process::ExitStatus::success) + .map(|_| candidate.to_string()) + }) +} + +fn utf8_hex(value: &str) -> String { + value + .as_bytes() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::>() + .join("") +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn conpty_delivers_input_to_foreground_children() -> anyhow::Result<()> { + let Some(python) = find_python() else { + eprintln!("python not found; skipping ConPTY input test"); + return Ok(()); + }; + let code = format!( + "print('__CODEX_CHILD_'+'READY__', flush=True); value=input(); print('{VALUE_MARKER}'+value.encode('utf-8').hex(), flush=True)" + ); + let expected = "cafeé 漢字"; + let expected_marker = format!("{VALUE_MARKER}{}", utf8_hex(expected)); + let mut shells = vec![WindowsShell { + name: "cmd", + program: std::env::var("COMSPEC").unwrap_or_else(|_| "cmd.exe".to_string()), + args: vec!["/D".to_string(), "/Q".to_string()], + child_command: format!("\"{}\" -u -c \"{code}\"", python.replace('"', "\"\"")), + }]; + if let Some(program) = find_powershell() { + shells.push(WindowsShell { + name: "PowerShell", + program, + args: vec!["-NoLogo".to_string(), "-NoProfile".to_string()], + child_command: format!("& '{}' -u -c \"{code}\"", python.replace('\'', "''")), + }); + } + let env: HashMap = std::env::vars().collect(); + + for shell in shells { + let spawned = spawn_pty_process( + &shell.program, + &shell.args, + Path::new("."), + &env, + /*arg0*/ &None, + TerminalSize::default(), + ) + .await?; + let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned); + let writer = session.writer_sender(); + writer + .send(format!("{}\n", shell.child_command).into_bytes()) + .await?; + wait_for_output_contains(&mut output_rx, READY_MARKER, /*timeout_ms*/ 10_000) + .await + .map_err(|err| anyhow::anyhow!("{} child did not become ready: {err}", shell.name))?; + + writer + .send(format!("{expected}X\u{8}\n").into_bytes()) + .await?; + let mut output = + wait_for_output_contains(&mut output_rx, &expected_marker, /*timeout_ms*/ 10_000) + .await + .map_err(|err| { + anyhow::anyhow!("{} child received incorrect input: {err}", shell.name) + })?; + + writer.send(b"exit 0\n".to_vec()).await?; + let (remaining, exit_code) = + collect_output_until_exit(output_rx, exit_rx, /*timeout_ms*/ 10_000).await; + output.extend_from_slice(&remaining); + + assert_eq!( + exit_code, + 0, + "{} did not exit cleanly: {:?}", + shell.name, + String::from_utf8_lossy(&output) + ); + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn conpty_ctrl_c_interrupts_powershell_foreground_child() -> anyhow::Result<()> { + let Some(program) = find_powershell() else { + return Ok(()); + }; + let args = vec!["-NoLogo".to_string(), "-NoProfile".to_string()]; + let env: HashMap = std::env::vars().collect(); + let spawned = spawn_pty_process( + &program, + &args, + Path::new("."), + &env, + /*arg0*/ &None, + TerminalSize::default(), + ) + .await?; + let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned); + let writer = session.writer_sender(); + writer.send(b"ping.exe -4 -t localhost\n".to_vec()).await?; + wait_for_output_contains(&mut output_rx, "127.0.0.1", /*timeout_ms*/ 10_000).await?; + + writer.send(vec![0x03]).await?; + tokio::time::sleep(tokio::time::Duration::from_millis(200)).await; + writer.send(b"cmd.exe /D /C ver\n".to_vec()).await?; + let mut output = wait_for_output_contains( + &mut output_rx, + "Microsoft Windows", + /*timeout_ms*/ 10_000, + ) + .await?; + + writer.send(b"exit 0\n".to_vec()).await?; + let (remaining, exit_code) = + collect_output_until_exit(output_rx, exit_rx, /*timeout_ms*/ 10_000).await; + output.extend_from_slice(&remaining); + assert_eq!( + exit_code, + 0, + "PowerShell did not resume after Ctrl-C: {:?}", + String::from_utf8_lossy(&output) + ); + Ok(()) +} diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs index abd0c020d..f3c8ada8c 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs @@ -1,5 +1,4 @@ use super::windows_common::finish_driver_spawn; -use super::windows_common::normalize_windows_tty_input; use crate::conpty::ConptyInstance; use crate::conpty::spawn_conpty_process_as_user; use crate::desktop::LaunchDesktop; @@ -22,6 +21,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_pty::ProcessDriver; use codex_utils_pty::SpawnedProcess; use codex_utils_pty::TerminalSize; +use codex_utils_pty::WindowsTtyInputNormalizer; use std::collections::HashMap; use std::path::Path; use std::ptr; @@ -153,13 +153,13 @@ fn spawn_input_writer( normalize_newlines: bool, ) -> tokio::task::JoinHandle<()> { tokio::task::spawn_blocking(move || { - let mut previous_was_cr = false; + let mut windows_input = WindowsTtyInputNormalizer::default(); while let Some(bytes) = writer_rx.blocking_recv() { let Some(handle) = input_write else { continue; }; let bytes = if normalize_newlines { - normalize_windows_tty_input(&bytes, &mut previous_was_cr) + windows_input.normalize(&bytes) } else { bytes }; diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/windows_common.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/windows_common.rs index f7be315ad..f37829e95 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/windows_common.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/windows_common.rs @@ -11,6 +11,7 @@ use anyhow::Result; use codex_utils_pty::ProcessDriver; use codex_utils_pty::SpawnedProcess; use codex_utils_pty::TerminalSize; +use codex_utils_pty::WindowsTtyInputNormalizer; use codex_utils_pty::spawn_from_driver; use std::fs::File; use tokio::sync::broadcast; @@ -25,23 +26,6 @@ pub(crate) fn finish_driver_spawn(driver: ProcessDriver, stdin_open: bool) -> Sp spawned } -pub(crate) fn normalize_windows_tty_input(bytes: &[u8], previous_was_cr: &mut bool) -> Vec { - let mut normalized = Vec::with_capacity(bytes.len()); - for &byte in bytes { - if byte == b'\n' { - if !*previous_was_cr { - normalized.push(b'\r'); - } - normalized.push(b'\n'); - *previous_was_cr = false; - } else { - normalized.push(byte); - *previous_was_cr = byte == b'\r'; - } - } - normalized -} - pub(crate) fn start_runner_pipe_writer( mut pipe_write: File, ) -> std::sync::mpsc::Sender { @@ -63,10 +47,10 @@ pub(crate) fn start_runner_stdin_writer( stdin_open: bool, ) -> tokio::task::JoinHandle<()> { tokio::task::spawn_blocking(move || { - let mut previous_was_cr = false; + let mut windows_input = WindowsTtyInputNormalizer::default(); while let Some(bytes) = writer_rx.blocking_recv() { let bytes = if normalize_newlines { - normalize_windows_tty_input(&bytes, &mut previous_was_cr) + windows_input.normalize(&bytes) } else { bytes };