From 359328c66acd19184405daec4e7d87a483db1cae Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Fri, 1 May 2026 00:53:12 -0300 Subject: [PATCH] Add TUI terminal probe diagnostics --- codex-rs/tui/src/terminal_palette.rs | 78 ++++++++++++-- codex-rs/tui/src/terminal_probe.rs | 136 ++++++++++++++++++++++- codex-rs/tui/src/tui.rs | 155 +++++++++++++++++++++------ 3 files changed, 326 insertions(+), 43 deletions(-) diff --git a/codex-rs/tui/src/terminal_palette.rs b/codex-rs/tui/src/terminal_palette.rs index f89bead178..e9ef2cb676 100644 --- a/codex-rs/tui/src/terminal_palette.rs +++ b/codex-rs/tui/src/terminal_palette.rs @@ -71,8 +71,12 @@ pub fn default_bg() -> Option<(u8, u8, u8)> { #[cfg(all(unix, not(test)))] mod imp { use super::DefaultColors; + use crossterm::style::Color as CrosstermColor; + use crossterm::style::query_background_color; + use crossterm::style::query_foreground_color; use std::sync::Mutex; use std::sync::OnceLock; + use std::time::Instant; struct Cache { attempted: bool, @@ -126,13 +130,73 @@ mod imp { } fn query_default_colors() -> Option { - crate::terminal_probe::default_colors(crate::terminal_probe::DEFAULT_TIMEOUT) - .ok() - .flatten() - .map(|colors| DefaultColors { - fg: colors.fg, - bg: colors.bg, - }) + match crate::terminal_probe::selected_probe_mode() { + crate::terminal_probe::ProbeMode::Bounded => query_default_colors_bounded(), + crate::terminal_probe::ProbeMode::Crossterm => query_default_colors_crossterm(), + } + } + + fn query_default_colors_bounded() -> Option { + let start = Instant::now(); + let result = crate::terminal_probe::default_colors(crate::terminal_probe::DEFAULT_TIMEOUT); + let elapsed = start.elapsed(); + let outcome = match &result { + Ok(Some(_)) => "ok", + Ok(None) => "no_response", + Err(_) => "error", + }; + crate::terminal_probe::record_probe_timing( + "default_colors", + crate::terminal_probe::ProbeMode::Bounded, + elapsed, + outcome, + result + .as_ref() + .err() + .map(|err| err as &dyn std::fmt::Display), + ); + result.ok().flatten().map(|colors| DefaultColors { + fg: colors.fg, + bg: colors.bg, + }) + } + + fn query_default_colors_crossterm() -> Option { + let fg = query_default_color_crossterm("default_foreground", query_foreground_color); + let bg = query_default_color_crossterm("default_background", query_background_color); + fg.zip(bg).map(|(fg, bg)| DefaultColors { fg, bg }) + } + + fn query_default_color_crossterm( + probe: &'static str, + query: fn() -> std::io::Result>, + ) -> Option<(u8, u8, u8)> { + let start = Instant::now(); + let result = query(); + let elapsed = start.elapsed(); + let outcome = match &result { + Ok(Some(_)) => "ok", + Ok(None) => "no_response", + Err(_) => "error", + }; + crate::terminal_probe::record_probe_timing( + probe, + crate::terminal_probe::ProbeMode::Crossterm, + elapsed, + outcome, + result + .as_ref() + .err() + .map(|err| err as &dyn std::fmt::Display), + ); + result.ok().flatten().and_then(color_to_tuple) + } + + fn color_to_tuple(color: CrosstermColor) -> Option<(u8, u8, u8)> { + match color { + CrosstermColor::Rgb { r, g, b } => Some((r, g, b)), + _ => None, + } } } diff --git a/codex-rs/tui/src/terminal_probe.rs b/codex-rs/tui/src/terminal_probe.rs index 6d90f63ad1..0e5dad5574 100644 --- a/codex-rs/tui/src/terminal_probe.rs +++ b/codex-rs/tui/src/terminal_probe.rs @@ -3,6 +3,111 @@ //! Crossterm's public helpers wait up to two seconds for terminal responses. That is too long for //! TUI startup, where unsupported terminals should simply fall back to conservative defaults. +use std::fmt; +use std::time::Duration; + +pub(crate) const DEFAULT_TIMEOUT: Duration = Duration::from_millis(100); + +const PROBE_MODE_ENV_VAR: &str = "CODEX_TUI_TERMINAL_PROBE_MODE"; +const TRACE_PROBES_ENV_VAR: &str = "CODEX_TUI_TRACE_TERMINAL_PROBES"; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum ProbeMode { + Bounded, + Crossterm, +} + +impl ProbeMode { + pub(crate) fn as_str(self) -> &'static str { + match self { + Self::Bounded => "bounded", + Self::Crossterm => "crossterm", + } + } +} + +impl fmt::Display for ProbeMode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +pub(crate) fn selected_probe_mode() -> ProbeMode { + probe_mode_for(std::env::var(PROBE_MODE_ENV_VAR).ok().as_deref()) +} + +fn probe_mode_for(value: Option<&str>) -> ProbeMode { + match value.map(str::trim) { + Some(value) + if value.eq_ignore_ascii_case("crossterm") + || value.eq_ignore_ascii_case("legacy") + || value.eq_ignore_ascii_case("blocking") => + { + ProbeMode::Crossterm + } + Some(value) + if value.eq_ignore_ascii_case("bounded") + || value.eq_ignore_ascii_case("fast") + || value.eq_ignore_ascii_case("opportunistic") => + { + ProbeMode::Bounded + } + _ => ProbeMode::Bounded, + } +} + +pub(crate) fn trace_probes_enabled() -> bool { + parse_bool_env(std::env::var(TRACE_PROBES_ENV_VAR).ok().as_deref()) + .unwrap_or(/*default*/ false) +} + +fn parse_bool_env(value: Option<&str>) -> Option { + match value.map(str::trim) { + Some("1") => Some(true), + Some(value) if value.eq_ignore_ascii_case("true") => Some(true), + Some(value) if value.eq_ignore_ascii_case("yes") => Some(true), + Some(value) if value.eq_ignore_ascii_case("on") => Some(true), + Some("0") => Some(false), + Some(value) if value.eq_ignore_ascii_case("false") => Some(false), + Some(value) if value.eq_ignore_ascii_case("no") => Some(false), + Some(value) if value.eq_ignore_ascii_case("off") => Some(false), + _ => None, + } +} + +pub(crate) fn record_probe_timing( + probe: &'static str, + mode: ProbeMode, + elapsed: Duration, + outcome: &'static str, + error: Option<&dyn fmt::Display>, +) { + if !trace_probes_enabled() { + return; + } + + let elapsed_ms = elapsed.as_secs_f64() * 1000.0; + match error { + Some(error) => tracing::info!( + target: "codex_tui::terminal_probe", + probe, + mode = %mode, + elapsed_ms, + outcome, + error = %error, + "terminal capability probe" + ), + None => tracing::info!( + target: "codex_tui::terminal_probe", + probe, + mode = %mode, + elapsed_ms, + outcome, + "terminal capability probe" + ), + } +} + #[cfg(unix)] #[cfg_attr(test, allow(dead_code))] mod imp { @@ -17,8 +122,6 @@ mod imp { use crossterm::event::KeyboardEnhancementFlags; use ratatui::layout::Position; - pub(crate) const DEFAULT_TIMEOUT: Duration = Duration::from_millis(100); - #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub(crate) struct DefaultColors { pub(crate) fg: (u8, u8, u8), @@ -341,5 +444,30 @@ mod imp { #[cfg(unix)] pub(crate) use imp::*; -#[cfg(not(unix))] -pub(crate) const DEFAULT_TIMEOUT: std::time::Duration = std::time::Duration::from_millis(100); +#[cfg(test)] +mod env_tests { + use super::*; + + #[test] + fn probe_mode_parses_known_values() { + assert_eq!(probe_mode_for(Some("crossterm")), ProbeMode::Crossterm); + assert_eq!(probe_mode_for(Some("legacy")), ProbeMode::Crossterm); + assert_eq!(probe_mode_for(Some("blocking")), ProbeMode::Crossterm); + assert_eq!(probe_mode_for(Some("bounded")), ProbeMode::Bounded); + assert_eq!(probe_mode_for(Some("fast")), ProbeMode::Bounded); + assert_eq!(probe_mode_for(/*value*/ None), ProbeMode::Bounded); + } + + #[test] + fn bool_env_parses_common_values() { + assert_eq!(parse_bool_env(Some("1")), Some(true)); + assert_eq!(parse_bool_env(Some("true")), Some(true)); + assert_eq!(parse_bool_env(Some("yes")), Some(true)); + assert_eq!(parse_bool_env(Some("on")), Some(true)); + assert_eq!(parse_bool_env(Some("0")), Some(false)); + assert_eq!(parse_bool_env(Some("false")), Some(false)); + assert_eq!(parse_bool_env(Some("no")), Some(false)); + assert_eq!(parse_bool_env(Some("off")), Some(false)); + assert_eq!(parse_bool_env(Some("bogus")), None); + } +} diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 4d0a97340a..37091d8425 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -11,6 +11,7 @@ use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; use std::time::Duration; +use std::time::Instant; use crossterm::Command; use crossterm::SynchronizedUpdate; @@ -21,7 +22,6 @@ use crossterm::event::EnableFocusChange; use crossterm::event::KeyEvent; use crossterm::terminal::EnterAlternateScreen; use crossterm::terminal::LeaveAlternateScreen; -#[cfg(not(unix))] use crossterm::terminal::supports_keyboard_enhancement; use ratatui::backend::Backend; use ratatui::backend::CrosstermBackend; @@ -283,38 +283,138 @@ pub fn init() -> Result { set_panic_hook(); - #[cfg(unix)] - let backend = CrosstermBackend::new(stdout()); - - #[cfg(not(unix))] let mut backend = CrosstermBackend::new(stdout()); #[cfg(unix)] - let cursor_pos = - match crate::terminal_probe::cursor_position(crate::terminal_probe::DEFAULT_TIMEOUT) { - Ok(Some(pos)) => pos, - Ok(None) => { - tracing::warn!("initial cursor position probe timed out; defaulting to origin"); - Position { x: 0, y: 0 } + let cursor_pos = match crate::terminal_probe::selected_probe_mode() { + crate::terminal_probe::ProbeMode::Bounded => { + let start = Instant::now(); + let result = + crate::terminal_probe::cursor_position(crate::terminal_probe::DEFAULT_TIMEOUT); + let elapsed = start.elapsed(); + let outcome = match &result { + Ok(Some(_)) => "ok", + Ok(None) => "no_response", + Err(_) => "error", + }; + crate::terminal_probe::record_probe_timing( + "cursor_position", + crate::terminal_probe::ProbeMode::Bounded, + elapsed, + outcome, + result + .as_ref() + .err() + .map(|err| err as &dyn std::fmt::Display), + ); + match result { + Ok(Some(pos)) => pos, + Ok(None) => { + tracing::warn!("initial cursor position probe timed out; defaulting to origin"); + Position { x: 0, y: 0 } + } + Err(err) => { + tracing::warn!( + "failed to read initial cursor position; defaulting to origin: {err}" + ); + Position { x: 0, y: 0 } + } } - Err(err) => { - tracing::warn!( - "failed to read initial cursor position; defaulting to origin: {err}" - ); - Position { x: 0, y: 0 } - } - }; + } + crate::terminal_probe::ProbeMode::Crossterm => { + cursor_position_with_crossterm_timing(&mut backend) + } + }; #[cfg(not(unix))] - let cursor_pos = backend.get_cursor_position().unwrap_or_else(|err| { - tracing::warn!("failed to read initial cursor position; defaulting to origin: {err}"); - Position { x: 0, y: 0 } - }); + let cursor_pos = cursor_position_with_crossterm_timing(&mut backend); let tui = CustomTerminal::with_options_and_cursor_position(backend, cursor_pos)?; Ok(tui) } +fn cursor_position_with_crossterm_timing(backend: &mut CrosstermBackend) -> Position { + let start = Instant::now(); + let result = backend.get_cursor_position(); + let elapsed = start.elapsed(); + crate::terminal_probe::record_probe_timing( + "cursor_position", + crate::terminal_probe::ProbeMode::Crossterm, + elapsed, + if result.is_ok() { "ok" } else { "error" }, + result + .as_ref() + .err() + .map(|err| err as &dyn std::fmt::Display), + ); + result.unwrap_or_else(|err| { + tracing::warn!("failed to read initial cursor position; defaulting to origin: {err}"); + Position { x: 0, y: 0 } + }) +} + +#[cfg(unix)] +fn keyboard_enhancement_supported_with_timing() -> bool { + match crate::terminal_probe::selected_probe_mode() { + crate::terminal_probe::ProbeMode::Bounded => { + let start = Instant::now(); + let result = crate::terminal_probe::keyboard_enhancement_supported( + crate::terminal_probe::DEFAULT_TIMEOUT, + ); + let elapsed = start.elapsed(); + let outcome = match &result { + Ok(Some(true)) => "supported", + Ok(Some(false)) => "unsupported", + Ok(None) => "no_response", + Err(_) => "error", + }; + crate::terminal_probe::record_probe_timing( + "keyboard_enhancement", + crate::terminal_probe::ProbeMode::Bounded, + elapsed, + outcome, + result + .as_ref() + .err() + .map(|err| err as &dyn std::fmt::Display), + ); + result + .unwrap_or(/*default*/ None) + .unwrap_or(/*default*/ false) + } + crate::terminal_probe::ProbeMode::Crossterm => { + keyboard_enhancement_supported_with_crossterm_timing() + } + } +} + +#[cfg(not(unix))] +fn keyboard_enhancement_supported_with_timing() -> bool { + keyboard_enhancement_supported_with_crossterm_timing() +} + +fn keyboard_enhancement_supported_with_crossterm_timing() -> bool { + let start = Instant::now(); + let result = supports_keyboard_enhancement(); + let elapsed = start.elapsed(); + let outcome = match &result { + Ok(true) => "supported", + Ok(false) => "unsupported", + Err(_) => "error", + }; + crate::terminal_probe::record_probe_timing( + "keyboard_enhancement", + crate::terminal_probe::ProbeMode::Crossterm, + elapsed, + outcome, + result + .as_ref() + .err() + .map(|err| err as &dyn std::fmt::Display), + ); + result.unwrap_or(/*default*/ false) +} + fn set_panic_hook() { let hook = panic::take_hook(); panic::set_hook(Box::new(move |panic_info| { @@ -366,17 +466,8 @@ impl Tui { // Detect keyboard enhancement support before any EventStream is created so the // crossterm poller can acquire its lock without contention. - #[cfg(unix)] let enhanced_keys_supported = !keyboard_modes::keyboard_enhancement_disabled() - && crate::terminal_probe::keyboard_enhancement_supported( - crate::terminal_probe::DEFAULT_TIMEOUT, - ) - .unwrap_or(/*default*/ None) - .unwrap_or(/*default*/ false); - - #[cfg(not(unix))] - let enhanced_keys_supported = !keyboard_modes::keyboard_enhancement_disabled() - && supports_keyboard_enhancement().unwrap_or(false); + && keyboard_enhancement_supported_with_timing(); // Cache this to avoid contention with the event reader. supports_color::on_cached(supports_color::Stream::Stdout); let _ = crate::terminal_palette::default_colors();