mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
[codex] fix Windows ConPTY input handling (#29734)
## 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
This commit is contained in:
1 parent
8a6a34be75
commit
a781761eda
8 files changed
+226
-23
No files matched your search
@@ -6,6 +6,8 @@ pub mod pty;
|
|||||||
mod tests;
|
mod tests;
|
||||||
#[cfg(windows)]
|
#[cfg(windows)]
|
||||||
mod win;
|
mod win;
|
||||||
|
#[cfg(windows)]
|
||||||
|
mod windows_input;
|
||||||
|
|
||||||
pub const DEFAULT_OUTPUT_BYTES_CAP: usize = 1024 * 1024;
|
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;
|
pub use win::PsuedoCon;
|
||||||
#[cfg(windows)]
|
#[cfg(windows)]
|
||||||
pub use win::conpty::RawConPty;
|
pub use win::conpty::RawConPty;
|
||||||
|
#[cfg(windows)]
|
||||||
|
pub use windows_input::WindowsTtyInputNormalizer;
|
||||||
@@ -217,7 +217,11 @@ async fn spawn_process_portable(
|
|||||||
let writer_handle: JoinHandle<()> = tokio::spawn({
|
let writer_handle: JoinHandle<()> = tokio::spawn({
|
||||||
let writer = Arc::clone(&writer);
|
let writer = Arc::clone(&writer);
|
||||||
async move {
|
async move {
|
||||||
|
#[cfg(windows)]
|
||||||
|
let mut windows_input = crate::WindowsTtyInputNormalizer::default();
|
||||||
while let Some(bytes) = writer_rx.recv().await {
|
while let Some(bytes) = writer_rx.recv().await {
|
||||||
|
#[cfg(windows)]
|
||||||
|
let bytes = windows_input.normalize(&bytes);
|
||||||
let mut guard = writer.lock().await;
|
let mut guard = writer.lock().await;
|
||||||
use std::io::Write;
|
use std::io::Write;
|
||||||
let _ = guard.write_all(&bytes);
|
let _ = guard.write_all(&bytes);
|
||||||
|
|||||||
@@ -16,6 +16,10 @@ use crate::spawn_pipe_process;
|
|||||||
use crate::spawn_pipe_process_no_stdin;
|
use crate::spawn_pipe_process_no_stdin;
|
||||||
use crate::spawn_pty_process;
|
use crate::spawn_pty_process;
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
#[path = "windows_tests.rs"]
|
||||||
|
mod windows_tests;
|
||||||
|
|
||||||
fn find_python() -> Option<String> {
|
fn find_python() -> Option<String> {
|
||||||
for candidate in ["python3", "python"] {
|
for candidate in ["python3", "python"] {
|
||||||
if let Ok(output) = std::process::Command::new(candidate)
|
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(
|
async fn wait_for_output_contains(
|
||||||
output_rx: &mut tokio::sync::broadcast::Receiver<Vec<u8>>,
|
output_rx: &mut tokio::sync::broadcast::Receiver<Vec<u8>>,
|
||||||
needle: &str,
|
needle: &str,
|
||||||
|
|||||||
@@ -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<u8> {
|
||||||
|
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;
|
||||||
@@ -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);
|
||||||
|
}
|
||||||
@@ -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<String>,
|
||||||
|
child_command: String,
|
||||||
|
}
|
||||||
|
|
||||||
|
fn find_powershell() -> Option<String> {
|
||||||
|
["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::<Vec<_>>()
|
||||||
|
.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<String, String> = 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<String, String> = 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(())
|
||||||
|
}
|
||||||
@@ -1,5 +1,4 @@
|
|||||||
use super::windows_common::finish_driver_spawn;
|
use super::windows_common::finish_driver_spawn;
|
||||||
use super::windows_common::normalize_windows_tty_input;
|
|
||||||
use crate::conpty::ConptyInstance;
|
use crate::conpty::ConptyInstance;
|
||||||
use crate::conpty::spawn_conpty_process_as_user;
|
use crate::conpty::spawn_conpty_process_as_user;
|
||||||
use crate::desktop::LaunchDesktop;
|
use crate::desktop::LaunchDesktop;
|
||||||
@@ -22,6 +21,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
|||||||
use codex_utils_pty::ProcessDriver;
|
use codex_utils_pty::ProcessDriver;
|
||||||
use codex_utils_pty::SpawnedProcess;
|
use codex_utils_pty::SpawnedProcess;
|
||||||
use codex_utils_pty::TerminalSize;
|
use codex_utils_pty::TerminalSize;
|
||||||
|
use codex_utils_pty::WindowsTtyInputNormalizer;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
use std::ptr;
|
use std::ptr;
|
||||||
@@ -153,13 +153,13 @@ fn spawn_input_writer(
|
|||||||
normalize_newlines: bool,
|
normalize_newlines: bool,
|
||||||
) -> tokio::task::JoinHandle<()> {
|
) -> tokio::task::JoinHandle<()> {
|
||||||
tokio::task::spawn_blocking(move || {
|
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() {
|
while let Some(bytes) = writer_rx.blocking_recv() {
|
||||||
let Some(handle) = input_write else {
|
let Some(handle) = input_write else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
let bytes = if normalize_newlines {
|
let bytes = if normalize_newlines {
|
||||||
normalize_windows_tty_input(&bytes, &mut previous_was_cr)
|
windows_input.normalize(&bytes)
|
||||||
} else {
|
} else {
|
||||||
bytes
|
bytes
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ use anyhow::Result;
|
|||||||
use codex_utils_pty::ProcessDriver;
|
use codex_utils_pty::ProcessDriver;
|
||||||
use codex_utils_pty::SpawnedProcess;
|
use codex_utils_pty::SpawnedProcess;
|
||||||
use codex_utils_pty::TerminalSize;
|
use codex_utils_pty::TerminalSize;
|
||||||
|
use codex_utils_pty::WindowsTtyInputNormalizer;
|
||||||
use codex_utils_pty::spawn_from_driver;
|
use codex_utils_pty::spawn_from_driver;
|
||||||
use std::fs::File;
|
use std::fs::File;
|
||||||
use tokio::sync::broadcast;
|
use tokio::sync::broadcast;
|
||||||
@@ -25,23 +26,6 @@ pub(crate) fn finish_driver_spawn(driver: ProcessDriver, stdin_open: bool) -> Sp
|
|||||||
spawned
|
spawned
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn normalize_windows_tty_input(bytes: &[u8], previous_was_cr: &mut bool) -> Vec<u8> {
|
|
||||||
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(
|
pub(crate) fn start_runner_pipe_writer(
|
||||||
mut pipe_write: File,
|
mut pipe_write: File,
|
||||||
) -> std::sync::mpsc::Sender<FramedMessage> {
|
) -> std::sync::mpsc::Sender<FramedMessage> {
|
||||||
@@ -63,10 +47,10 @@ pub(crate) fn start_runner_stdin_writer(
|
|||||||
stdin_open: bool,
|
stdin_open: bool,
|
||||||
) -> tokio::task::JoinHandle<()> {
|
) -> tokio::task::JoinHandle<()> {
|
||||||
tokio::task::spawn_blocking(move || {
|
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() {
|
while let Some(bytes) = writer_rx.blocking_recv() {
|
||||||
let bytes = if normalize_newlines {
|
let bytes = if normalize_newlines {
|
||||||
normalize_windows_tty_input(&bytes, &mut previous_was_cr)
|
windows_input.normalize(&bytes)
|
||||||
} else {
|
} else {
|
||||||
bytes
|
bytes
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in new issue
Block a user