mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Bound WSL terminal detection and handle inconclusive probes safely (#45811)
## Why WSL interop can block while launching the Windows terminal probe, stalling TUI startup. Inconclusive detection can also enable keyboard enhancements that break dead-key composition in VS Code on WSL. ## What changed - Give the Windows `TERM_PROGRAM` probe a one-second deadline, with launch and cleanup handled on a worker thread. Cache the result, including timeouts, and kill and reap probes that outlive the deadline once launch returns. - Distinguish VS Code, other terminals, and unknown detection results. Disable keyboard enhancements on WSL for VS Code or unknown results, while preserving the `CODEX_TUI_DISABLE_KEYBOARD_ENHANCEMENT` override. ## Testing Add regression tests for probe output and exit status, failed launches, timeouts during launch and execution, and child cleanup. Extend detection and override tests to cover unknown results. GitOrigin-RevId: 39e980b0d7cf3d6716ee61597d6002b0fd59651e
This commit is contained in:
@@ -3,6 +3,8 @@
|
||||
//! The TUI uses crossterm's keyboard enhancement stack while it owns the terminal, but
|
||||
//! process exit gets a stronger reset so the parent shell does not inherit enhanced key
|
||||
//! reporting if a terminal misses the normal stack pop.
|
||||
//! Windows terminal detection has a deadline, including WSL interop process launch.
|
||||
//! Inconclusive detection keeps keyboard enhancements disabled on WSL.
|
||||
|
||||
use std::fmt;
|
||||
use std::io::stdout;
|
||||
@@ -17,26 +19,41 @@ use ratatui::crossterm::execute;
|
||||
|
||||
const DISABLE_KEYBOARD_ENHANCEMENT_ENV_VAR: &str = "CODEX_TUI_DISABLE_KEYBOARD_ENHANCEMENT";
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
enum VscodeDetection {
|
||||
VsCode,
|
||||
Other,
|
||||
Unknown,
|
||||
}
|
||||
|
||||
pub(super) fn keyboard_enhancement_disabled() -> bool {
|
||||
let disable_env = std::env::var(DISABLE_KEYBOARD_ENHANCEMENT_ENV_VAR).ok();
|
||||
let is_wsl = running_in_wsl();
|
||||
let is_vscode_terminal = is_wsl && running_in_vscode_terminal();
|
||||
keyboard_enhancement_disabled_for(disable_env.as_deref(), is_wsl, is_vscode_terminal)
|
||||
let vscode_detection = if is_wsl {
|
||||
detect_vscode_terminal()
|
||||
} else {
|
||||
VscodeDetection::Other
|
||||
};
|
||||
keyboard_enhancement_disabled_for(disable_env.as_deref(), is_wsl, vscode_detection)
|
||||
}
|
||||
|
||||
fn keyboard_enhancement_disabled_for(
|
||||
disable_env: Option<&str>,
|
||||
is_wsl: bool,
|
||||
is_vscode_terminal: bool,
|
||||
vscode_detection: VscodeDetection,
|
||||
) -> bool {
|
||||
if let Some(disabled) = parse_bool_env(disable_env) {
|
||||
return disabled;
|
||||
}
|
||||
|
||||
// VS Code running a WSL shell can hide TERM_PROGRAM from the Linux process
|
||||
// environment, so `running_in_vscode_terminal` also probes the Windows-side
|
||||
// environment through WSL interop.
|
||||
is_wsl && is_vscode_terminal
|
||||
// environment. If the Windows-side probe is inconclusive, avoid enabling the
|
||||
// keyboard mode that can break dead-key composition in VS Code on WSL.
|
||||
is_wsl
|
||||
&& matches!(
|
||||
vscode_detection,
|
||||
VscodeDetection::VsCode | VscodeDetection::Unknown
|
||||
)
|
||||
}
|
||||
|
||||
fn parse_bool_env(value: Option<&str>) -> Option<bool> {
|
||||
@@ -64,69 +81,131 @@ fn running_in_wsl() -> bool {
|
||||
}
|
||||
|
||||
pub(super) fn running_in_vscode_terminal() -> bool {
|
||||
detect_vscode_terminal() == VscodeDetection::VsCode
|
||||
}
|
||||
|
||||
fn detect_vscode_terminal() -> VscodeDetection {
|
||||
if term_program_is_vscode(std::env::var("TERM_PROGRAM").ok().as_deref()) {
|
||||
return true;
|
||||
return VscodeDetection::VsCode;
|
||||
}
|
||||
vscode_terminal_detected(
|
||||
std::env::var("TERM_PROGRAM").ok().as_deref(),
|
||||
windows_term_program().as_deref(),
|
||||
windows_vscode_detection(),
|
||||
)
|
||||
}
|
||||
|
||||
fn vscode_terminal_detected(
|
||||
linux_term_program: Option<&str>,
|
||||
windows_term_program: Option<&str>,
|
||||
) -> bool {
|
||||
term_program_is_vscode(linux_term_program) || term_program_is_vscode(windows_term_program)
|
||||
windows_detection: VscodeDetection,
|
||||
) -> VscodeDetection {
|
||||
if term_program_is_vscode(linux_term_program) {
|
||||
VscodeDetection::VsCode
|
||||
} else {
|
||||
windows_detection
|
||||
}
|
||||
}
|
||||
|
||||
fn term_program_is_vscode(value: Option<&str>) -> bool {
|
||||
value.is_some_and(|value| value.eq_ignore_ascii_case("vscode"))
|
||||
}
|
||||
|
||||
fn windows_term_program() -> Option<String> {
|
||||
fn windows_vscode_detection() -> VscodeDetection {
|
||||
#[cfg(target_os = "linux")]
|
||||
{
|
||||
static WINDOWS_TERM_PROGRAM: std::sync::OnceLock<Option<String>> =
|
||||
static WINDOWS_VSCODE_DETECTION: std::sync::OnceLock<VscodeDetection> =
|
||||
std::sync::OnceLock::new();
|
||||
WINDOWS_TERM_PROGRAM
|
||||
.get_or_init(read_windows_term_program)
|
||||
.clone()
|
||||
*WINDOWS_VSCODE_DETECTION.get_or_init(read_windows_vscode_detection)
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
{
|
||||
None
|
||||
VscodeDetection::Unknown
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn read_windows_term_program() -> Option<String> {
|
||||
fn read_windows_vscode_detection() -> VscodeDetection {
|
||||
if !running_in_wsl() {
|
||||
return None;
|
||||
return VscodeDetection::Other;
|
||||
}
|
||||
let executable = codex_utils_path::system_executable("cmd.exe")?;
|
||||
let output = std::process::Command::new(executable)
|
||||
.args(["/d", "/s", "/c", "set TERM_PROGRAM"])
|
||||
.stdin(std::process::Stdio::null())
|
||||
.stderr(std::process::Stdio::null())
|
||||
.output()
|
||||
.ok()?;
|
||||
|
||||
if !output.status.success() {
|
||||
return None;
|
||||
}
|
||||
|
||||
String::from_utf8_lossy(&output.stdout)
|
||||
.lines()
|
||||
.find_map(|line| {
|
||||
line.trim_end_matches('\r')
|
||||
.strip_prefix("TERM_PROGRAM=")
|
||||
.map(str::to_string)
|
||||
})
|
||||
.filter(|value| !value.trim().is_empty())
|
||||
read_windows_vscode_detection_with_timeout(
|
||||
|| {
|
||||
let executable = codex_utils_path::system_executable("cmd.exe")?;
|
||||
std::process::Command::new(executable)
|
||||
.args(["/d", "/s", "/c", "set TERM_PROGRAM"])
|
||||
.stdin(std::process::Stdio::null())
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.stderr(std::process::Stdio::null())
|
||||
.spawn()
|
||||
.ok()
|
||||
},
|
||||
std::time::Duration::from_secs(/*secs*/ 1),
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(any(target_os = "linux", all(test, unix)))]
|
||||
fn read_windows_vscode_detection_with_timeout(
|
||||
spawn: impl FnOnce() -> Option<std::process::Child> + Send + 'static,
|
||||
timeout: std::time::Duration,
|
||||
) -> VscodeDetection {
|
||||
let deadline = std::time::Instant::now() + timeout;
|
||||
let (sender, receiver) = std::sync::mpsc::channel();
|
||||
// WSL interop can block inside spawn itself. Keep launch and cleanup off the
|
||||
// startup thread; the OnceLock caches a timeout so this worker is not retried.
|
||||
let worker = std::thread::Builder::new()
|
||||
.name("windows-term-program".into())
|
||||
.spawn(move || {
|
||||
let output = (|| {
|
||||
let mut child = spawn()?;
|
||||
while std::time::Instant::now() < deadline {
|
||||
match child.try_wait() {
|
||||
Ok(Some(_)) => return child.wait_with_output().ok(),
|
||||
Ok(None) => {
|
||||
std::thread::sleep(std::time::Duration::from_millis(/*millis*/ 10))
|
||||
}
|
||||
Err(_) => break,
|
||||
}
|
||||
}
|
||||
let _ = child.kill();
|
||||
let _ = child.wait();
|
||||
None
|
||||
})();
|
||||
let _ = sender.send(output);
|
||||
});
|
||||
if worker.is_err() {
|
||||
return VscodeDetection::Unknown;
|
||||
}
|
||||
|
||||
let Ok(Some(output)) =
|
||||
receiver.recv_timeout(deadline.saturating_duration_since(std::time::Instant::now()))
|
||||
else {
|
||||
return VscodeDetection::Unknown;
|
||||
};
|
||||
if !output.status.success() {
|
||||
// `cmd.exe /c set TERM_PROGRAM` exits with 1 when the variable is absent.
|
||||
return if output.status.code() == Some(1) {
|
||||
VscodeDetection::Other
|
||||
} else {
|
||||
VscodeDetection::Unknown
|
||||
};
|
||||
}
|
||||
|
||||
let output = String::from_utf8_lossy(&output.stdout);
|
||||
let term_program = output
|
||||
.lines()
|
||||
.find_map(|line| line.trim_end_matches('\r').strip_prefix("TERM_PROGRAM="))
|
||||
.filter(|value| !value.trim().is_empty());
|
||||
if term_program_is_vscode(term_program) {
|
||||
VscodeDetection::VsCode
|
||||
} else {
|
||||
VscodeDetection::Other
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, unix))]
|
||||
#[path = "windows_term_program_tests.rs"]
|
||||
mod windows_term_program_tests;
|
||||
|
||||
pub(super) fn enable_keyboard_enhancement() {
|
||||
if keyboard_enhancement_disabled() {
|
||||
return;
|
||||
@@ -318,6 +397,7 @@ mod tests {
|
||||
use super::DisableModifyOtherKeys;
|
||||
use super::EnableModifyOtherKeys;
|
||||
use super::ResetKeyboardEnhancementFlags;
|
||||
use super::VscodeDetection;
|
||||
use super::keyboard_enhancement_disabled_for;
|
||||
use super::keyboard_enhancement_flags;
|
||||
use super::parse_bool_env;
|
||||
@@ -432,53 +512,60 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keyboard_enhancement_auto_disables_for_vscode_in_wsl() {
|
||||
assert!(keyboard_enhancement_disabled_for(
|
||||
/*disable_env*/ None, /*is_wsl*/ true, /*is_vscode_terminal*/ true
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keyboard_enhancement_auto_disable_requires_wsl_and_vscode() {
|
||||
assert!(!keyboard_enhancement_disabled_for(
|
||||
/*disable_env*/ None, /*is_wsl*/ true, /*is_vscode_terminal*/ false
|
||||
));
|
||||
assert!(!keyboard_enhancement_disabled_for(
|
||||
/*disable_env*/ None, /*is_wsl*/ false, /*is_vscode_terminal*/ true
|
||||
));
|
||||
fn keyboard_enhancement_auto_disables_for_vscode_or_unknown_in_wsl() {
|
||||
for detection in [
|
||||
VscodeDetection::VsCode,
|
||||
VscodeDetection::Other,
|
||||
VscodeDetection::Unknown,
|
||||
] {
|
||||
assert_eq!(
|
||||
keyboard_enhancement_disabled_for(
|
||||
/*disable_env*/ None, /*is_wsl*/ true, detection
|
||||
),
|
||||
detection != VscodeDetection::Other,
|
||||
);
|
||||
assert!(!keyboard_enhancement_disabled_for(
|
||||
/*disable_env*/ None, /*is_wsl*/ false, detection
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keyboard_enhancement_env_flag_overrides_auto_detection() {
|
||||
assert!(!keyboard_enhancement_disabled_for(
|
||||
Some("0"),
|
||||
/*is_wsl*/ true,
|
||||
/*is_vscode_terminal*/ true
|
||||
));
|
||||
assert!(keyboard_enhancement_disabled_for(
|
||||
Some("1"),
|
||||
/*is_wsl*/ false,
|
||||
/*is_vscode_terminal*/ false
|
||||
));
|
||||
for detection in [
|
||||
VscodeDetection::VsCode,
|
||||
VscodeDetection::Other,
|
||||
VscodeDetection::Unknown,
|
||||
] {
|
||||
assert!(!keyboard_enhancement_disabled_for(
|
||||
Some("0"),
|
||||
/*is_wsl*/ true,
|
||||
detection
|
||||
));
|
||||
assert!(keyboard_enhancement_disabled_for(
|
||||
Some("1"),
|
||||
/*is_wsl*/ false,
|
||||
detection
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn vscode_terminal_detection_uses_linux_and_windows_term_program() {
|
||||
assert!(vscode_terminal_detected(
|
||||
Some("vscode"),
|
||||
/*windows_term_program*/ None
|
||||
));
|
||||
assert!(vscode_terminal_detected(
|
||||
/*linux_term_program*/ None,
|
||||
Some("vscode")
|
||||
));
|
||||
assert!(!vscode_terminal_detected(
|
||||
/*linux_term_program*/ None,
|
||||
Some("WindowsTerminal")
|
||||
));
|
||||
assert!(!vscode_terminal_detected(
|
||||
/*linux_term_program*/ None, /*windows_term_program*/ None
|
||||
));
|
||||
for detection in [
|
||||
VscodeDetection::VsCode,
|
||||
VscodeDetection::Other,
|
||||
VscodeDetection::Unknown,
|
||||
] {
|
||||
assert_eq!(
|
||||
vscode_terminal_detected(Some("vscode"), detection),
|
||||
VscodeDetection::VsCode,
|
||||
);
|
||||
assert_eq!(
|
||||
vscode_terminal_detected(/*linux_term_program*/ None, detection),
|
||||
detection,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
//! Regression coverage for bounded Windows terminal detection and child cleanup.
|
||||
|
||||
use super::VscodeDetection;
|
||||
use super::keyboard_enhancement_disabled_for;
|
||||
use super::read_windows_vscode_detection_with_timeout;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::process::Command;
|
||||
use std::process::Stdio;
|
||||
use std::sync::mpsc;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
#[test]
|
||||
fn reads_term_program_only_from_successful_probe() {
|
||||
for (script, expected) in [
|
||||
("printf 'TERM_PROGRAM=vscode\r\n'", VscodeDetection::VsCode),
|
||||
(
|
||||
"printf 'TERM_PROGRAM=vscode\r\n'; exit 1",
|
||||
VscodeDetection::Other,
|
||||
),
|
||||
(
|
||||
"printf 'TERM_PROGRAM=WindowsTerminal'",
|
||||
VscodeDetection::Other,
|
||||
),
|
||||
("exit 1", VscodeDetection::Other),
|
||||
("exit 2", VscodeDetection::Unknown),
|
||||
] {
|
||||
assert_eq!(
|
||||
read_windows_vscode_detection_with_timeout(
|
||||
move || {
|
||||
Command::new("/bin/sh")
|
||||
.args(["-c", script])
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()
|
||||
.ok()
|
||||
},
|
||||
Duration::from_secs(/*secs*/ 5),
|
||||
),
|
||||
expected,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn failed_launch_is_unknown() {
|
||||
assert_eq!(
|
||||
read_windows_vscode_detection_with_timeout(|| None, Duration::from_secs(/*secs*/ 5)),
|
||||
VscodeDetection::Unknown,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn times_out_and_reaps_running_probe() {
|
||||
let (sender, receiver) = mpsc::channel();
|
||||
let started = Instant::now();
|
||||
let result = read_windows_vscode_detection_with_timeout(
|
||||
move || {
|
||||
let child = Command::new("sleep").arg("30").spawn().unwrap();
|
||||
sender.send(child.id()).unwrap();
|
||||
Some(child)
|
||||
},
|
||||
Duration::from_millis(/*millis*/ 100),
|
||||
);
|
||||
assert_eq!(result, VscodeDetection::Unknown);
|
||||
assert!(keyboard_enhancement_disabled_for(
|
||||
/*disable_env*/ None, /*is_wsl*/ true, result
|
||||
));
|
||||
assert!(started.elapsed() < Duration::from_secs(/*secs*/ 2));
|
||||
assert_child_reaped(
|
||||
receiver
|
||||
.recv_timeout(Duration::from_secs(/*secs*/ 5))
|
||||
.unwrap(),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn times_out_during_launch_and_reaps_child_when_launch_finishes() {
|
||||
let (release, blocked) = mpsc::channel();
|
||||
let (sender, receiver) = mpsc::channel();
|
||||
let started = Instant::now();
|
||||
let result = read_windows_vscode_detection_with_timeout(
|
||||
move || {
|
||||
// Model a WSL launch that cannot return a Child until interop recovers.
|
||||
blocked
|
||||
.recv_timeout(Duration::from_secs(/*secs*/ 5))
|
||||
.unwrap();
|
||||
let child = Command::new("sleep").arg("30").spawn().unwrap();
|
||||
sender.send(child.id()).unwrap();
|
||||
Some(child)
|
||||
},
|
||||
Duration::from_millis(/*millis*/ 100),
|
||||
);
|
||||
release.send(()).unwrap();
|
||||
assert_eq!(result, VscodeDetection::Unknown);
|
||||
assert!(keyboard_enhancement_disabled_for(
|
||||
/*disable_env*/ None, /*is_wsl*/ true, result
|
||||
));
|
||||
assert!(started.elapsed() < Duration::from_secs(/*secs*/ 2));
|
||||
assert_child_reaped(
|
||||
receiver
|
||||
.recv_timeout(Duration::from_secs(/*secs*/ 5))
|
||||
.unwrap(),
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_child_reaped(pid: u32) {
|
||||
let started = Instant::now();
|
||||
loop {
|
||||
// SAFETY: signal 0 only checks whether this process still exists.
|
||||
if unsafe {
|
||||
libc::kill(pid as libc::pid_t, /*sig*/ 0)
|
||||
} == -1
|
||||
{
|
||||
assert_eq!(
|
||||
std::io::Error::last_os_error().raw_os_error(),
|
||||
Some(libc::ESRCH)
|
||||
);
|
||||
return;
|
||||
}
|
||||
assert!(
|
||||
started.elapsed() < Duration::from_secs(/*secs*/ 5),
|
||||
"timed-out terminal probe was not reaped"
|
||||
);
|
||||
std::thread::sleep(Duration::from_millis(/*millis*/ 10));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user