From 976129f0973b3323e2200016df43e83f88270543 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Mon, 27 Jul 2026 17:54:28 +0000 Subject: [PATCH] Preserve TUI input when terminal focus returns (#35649) ## Why Refreshing the terminal palette on a focus event can block the input loop and discard keystrokes entered while focus is returning. ## What changed - Keep the palette cached by the startup probe when handling `FocusGained`. - Continue updating focus state and requesting a redraw without issuing new foreground or background color queries. ## Testing - Verify a queued key is delivered after `FocusGained`. - Exercise focus regain in a pseudo-terminal with immediate and delayed input, and verify it emits no additional palette queries. GitOrigin-RevId: 16e26f81c1343d01f6d0baeff3f204bcc91dd6f3 --- codex-rs/tui/src/terminal_palette.rs | 41 ---- codex-rs/tui/src/tui/event_stream.rs | 33 ++- codex-rs/tui/tests/suite/focus_palette.rs | 261 ++++++++++++++++++++++ codex-rs/tui/tests/suite/mod.rs | 2 + 4 files changed, 295 insertions(+), 42 deletions(-) create mode 100644 codex-rs/tui/tests/suite/focus_palette.rs diff --git a/codex-rs/tui/src/terminal_palette.rs b/codex-rs/tui/src/terminal_palette.rs index 3c288488e4..2c2a3bf03b 100644 --- a/codex-rs/tui/src/terminal_palette.rs +++ b/codex-rs/tui/src/terminal_palette.rs @@ -83,10 +83,6 @@ fn best_color_for_color_level(target: (u8, u8, u8), color_level: StdoutColorLeve } } -pub fn requery_default_colors() { - imp::requery_default_colors(); -} - #[derive(Clone, Copy)] pub struct DefaultColors { fg: (u8, u8, u8), @@ -115,9 +111,6 @@ pub(crate) fn set_default_colors_from_startup_probe( #[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; @@ -168,29 +161,6 @@ mod imp { } } - pub(super) fn requery_default_colors() { - if let Ok(mut cache) = default_colors_cache().lock() { - // Don't try to refresh if the cache is already attempted and failed. - if cache.attempted && cache.value.is_none() { - return; - } - - // Focus events arrive after crossterm's event stream is active. Requery through - // crossterm here so unrelated input stays in crossterm's skipped-event queue instead - // of being consumed by the bounded startup probe's direct tty reads. - let fg = query_foreground_color() - .ok() - .flatten() - .and_then(color_to_tuple); - let bg = query_background_color() - .ok() - .flatten() - .and_then(color_to_tuple); - cache.value = fg.zip(bg).map(|(fg, bg)| DefaultColors { fg, bg }); - cache.attempted = true; - } - } - /// Queries terminal default colors through the bounded startup probe path. /// /// The palette cache treats `None` as an attempted-but-unavailable result, so this function @@ -205,13 +175,6 @@ mod imp { bg: colors.bg, }) } - - fn color_to_tuple(color: CrosstermColor) -> Option<(u8, u8, u8)> { - match color { - CrosstermColor::Rgb { r, g, b } => Some((r, g, b)), - _ => None, - } - } } #[cfg(windows)] @@ -267,8 +230,6 @@ mod imp { } } - pub(super) fn requery_default_colors() {} - fn query_default_colors() -> Option { crate::terminal_probe::default_colors(crate::terminal_probe::DEFAULT_TIMEOUT) .ok() @@ -293,8 +254,6 @@ mod imp { _colors: Option, ) { } - - pub(super) fn requery_default_colors() {} } /// The subset of Xterm colors that are usually consistent across terminals. diff --git a/codex-rs/tui/src/tui/event_stream.rs b/codex-rs/tui/src/tui/event_stream.rs index b805176756..da719463f3 100644 --- a/codex-rs/tui/src/tui/event_stream.rs +++ b/codex-rs/tui/src/tui/event_stream.rs @@ -271,7 +271,8 @@ impl TuiEventStream { Event::Paste(pasted) => Some(TuiEvent::Paste(pasted)), Event::FocusGained => { self.terminal_focused.store(true, Ordering::Relaxed); - crate::terminal_palette::requery_default_colors(); + // Keep the startup-cached palette: querying terminal colors here blocks the + // input loop, and a direct probe would discard keys typed during the refresh. Some(TuiEvent::Draw) } Event::FocusLost => { @@ -431,6 +432,36 @@ mod tests { } } + #[tokio::test(flavor = "current_thread")] + async fn focus_gained_preserves_already_queued_key() { + let (broker, handle, _draw_tx, draw_rx, terminal_focused) = setup(); + terminal_focused.store(false, Ordering::Relaxed); + let mut stream = make_stream(broker.clone(), draw_rx, terminal_focused.clone()); + let expected_key = KeyEvent::new(KeyCode::Char('f'), KeyModifiers::NONE); + + handle.send(Ok(Event::FocusGained)); + handle.send(Ok(Event::Key(expected_key))); + + assert!(matches!(stream.next().await, Some(TuiEvent::Draw))); + assert!(terminal_focused.load(Ordering::Relaxed)); + assert!(matches!( + &*broker + .state + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner), + EventBrokerState::Running(_) + )); + + let next = timeout(Duration::from_millis(/*millis*/ 100), stream.next()) + .await + .expect("focus handling discarded an already queued key"); + + match next { + Some(TuiEvent::Key(key)) => assert_eq!(key, expected_key), + other => panic!("expected queued key event, got {other:?}"), + } + } + #[tokio::test(flavor = "current_thread")] async fn draw_and_key_events_yield_both() { let (broker, handle, draw_tx, draw_rx, terminal_focused) = setup(); diff --git a/codex-rs/tui/tests/suite/focus_palette.rs b/codex-rs/tui/tests/suite/focus_palette.rs new file mode 100644 index 0000000000..f8f12d1341 --- /dev/null +++ b/codex-rs/tui/tests/suite/focus_palette.rs @@ -0,0 +1,261 @@ +use std::fs::File; +use std::io::Read; +use std::io::Write; +use std::os::fd::AsRawFd; +use std::os::fd::FromRawFd; +use std::os::fd::OwnedFd; +use std::path::Path; +use std::process::Child; +use std::process::Command; +use std::time::Duration; +use std::time::Instant; + +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use anyhow::ensure; +use tempfile::TempDir; + +const STARTUP_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 15); +const FOCUS_INPUT_TIMEOUT: Duration = Duration::from_millis(/*millis*/ 500); +const FOCUS_PROBE_INPUT: &str = "focus-palette-24527"; + +#[test] +fn focus_gained_with_unanswered_palette_queries_preserves_immediate_input() -> Result<()> { + let repo_root = codex_utils_cargo_bin::repo_root()?; + let codex_home = tempfile::tempdir()?; + write_test_config(codex_home.path(), &repo_root)?; + + let mut terminal = PtyCodex::start(&repo_root, codex_home)?; + terminal.wait_for_startup()?; + + let startup_output_len = terminal.output.len(); + let focus_started = Instant::now(); + terminal.write_input(format!("\u{1b}[I{FOCUS_PROBE_INPUT}").as_bytes())?; + terminal.wait_for_focus_input(FOCUS_PROBE_INPUT, focus_started)?; + + let delayed_input = format!("{FOCUS_PROBE_INPUT}-delayed"); + let delayed_focus_started = Instant::now(); + terminal.write_input(b"\x1b[I")?; + terminal.read_output(Duration::from_millis(/*millis*/ 20))?; + terminal.write_input(delayed_input.as_bytes())?; + terminal.wait_for_focus_input(&delayed_input, delayed_focus_started)?; + + ensure!( + !contains_bytes(&terminal.output[startup_output_len..], b"\x1b]10;?") + && !contains_bytes(&terminal.output[startup_output_len..], b"\x1b]11;?"), + "focus regain queried terminal colors after the startup palette was cached", + ); + + Ok(()) +} + +struct PtyCodex { + master: File, + child: Child, + parser: vt100::Parser, + output: Vec, + cursor_answered: bool, + palette_answered: bool, + keyboard_answered: bool, + _codex_home: TempDir, +} + +impl PtyCodex { + fn start(repo_root: &Path, codex_home: TempDir) -> Result { + let mut master_fd = -1; + let mut slave_fd = -1; + let mut window_size = libc::winsize { + ws_row: 32, + ws_col: 120, + ws_xpixel: 0, + ws_ypixel: 0, + }; + + // SAFETY: `openpty` initializes both file descriptors on success, and the supplied window + // size remains valid for the duration of the call. + let result = unsafe { + libc::openpty( + &mut master_fd, + &mut slave_fd, + /*name*/ std::ptr::null_mut(), + /*termp*/ std::ptr::null_mut(), + &raw mut window_size, + ) + }; + if result == -1 { + return Err(std::io::Error::last_os_error()).context("open focus-test pseudo-terminal"); + } + + // SAFETY: a successful `openpty` transfers ownership of both unique file descriptors. + let master = File::from(unsafe { OwnedFd::from_raw_fd(master_fd) }); + // SAFETY: `slave_fd` is the second unique descriptor initialized by `openpty`. + let slave = File::from(unsafe { OwnedFd::from_raw_fd(slave_fd) }); + let stdin = slave.try_clone().context("clone pseudo-terminal stdin")?; + let stdout = slave.try_clone().context("clone pseudo-terminal stdout")?; + + let codex = codex_utils_cargo_bin::cargo_bin("codex") + .or_else(|_| codex_utils_cargo_bin::cargo_bin("codex-tui"))?; + let child = Command::new(codex) + .arg("--no-alt-screen") + .arg("-C") + .arg(repo_root) + .arg("-c") + .arg("analytics.enabled=false") + .env("TERM", "xterm-256color") + .env("OPENAI_API_KEY", "focus-palette-test") + .env("CODEX_HOME", codex_home.path()) + .stdin(stdin) + .stdout(stdout) + .stderr(slave) + .spawn() + .context("start Codex in focus-test pseudo-terminal")?; + + Ok(Self { + master, + child, + parser: vt100::Parser::new( + /*rows*/ 32, /*cols*/ 120, /*scrollback_len*/ 0, + ), + output: Vec::new(), + cursor_answered: false, + palette_answered: false, + keyboard_answered: false, + _codex_home: codex_home, + }) + } + + fn wait_for_startup(&mut self) -> Result<()> { + let deadline = Instant::now() + STARTUP_TIMEOUT; + while Instant::now() < deadline { + self.read_output(Duration::from_millis(/*millis*/ 50))?; + self.answer_startup_queries()?; + + if self.palette_answered && self.screen_contains("OpenAI Codex") { + return Ok(()); + } + + if let Some(status) = self.child.try_wait()? { + bail!( + "Codex exited before the focus test started ({status}); screen:\n{}", + self.screen_contents(), + ); + } + } + + bail!( + "Codex did not initialize within {:?}; screen:\n{}", + STARTUP_TIMEOUT, + self.screen_contents(), + ); + } + + fn wait_for_focus_input(&mut self, input: &str, focus_started: Instant) -> Result<()> { + while focus_started.elapsed() < FOCUS_INPUT_TIMEOUT { + self.read_output(Duration::from_millis(/*millis*/ 20))?; + if self.screen_contains(input) { + return Ok(()); + } + } + + bail!( + "focus-time palette refresh blocked or discarded {input:?} for more than {:?}; \ + screen:\n{}", + FOCUS_INPUT_TIMEOUT, + self.screen_contents(), + ); + } + + fn answer_startup_queries(&mut self) -> Result<()> { + if !self.cursor_answered && contains_bytes(&self.output, b"\x1b[6n") { + self.write_input(b"\x1b[1;1R")?; + self.cursor_answered = true; + } + + if !self.keyboard_answered && contains_bytes(&self.output, b"\x1b[?u") { + self.write_input(b"\x1b[?0u\x1b[?1;2c")?; + self.keyboard_answered = true; + } + + if !self.palette_answered + && contains_bytes(&self.output, b"\x1b]10;?") + && contains_bytes(&self.output, b"\x1b]11;?") + { + self.write_input(b"\x1b]10;rgb:ffff/ffff/ffff\x1b\\\x1b]11;rgb:0000/0000/0000\x1b\\")?; + self.palette_answered = true; + } + + Ok(()) + } + + fn read_output(&mut self, timeout: Duration) -> Result<()> { + let timeout_ms = timeout.as_millis().min(libc::c_int::MAX as u128) as libc::c_int; + let mut descriptor = libc::pollfd { + fd: self.master.as_raw_fd(), + events: libc::POLLIN, + revents: 0, + }; + + // SAFETY: `descriptor` points to one initialized poll descriptor. + let ready = unsafe { + libc::poll(&mut descriptor, /*nfds*/ 1, timeout_ms) + }; + if ready == -1 { + let error = std::io::Error::last_os_error(); + if error.kind() == std::io::ErrorKind::Interrupted { + return Ok(()); + } + return Err(error).context("poll focus-test pseudo-terminal"); + } + if ready == 0 || descriptor.revents & libc::POLLIN == 0 { + return Ok(()); + } + + let mut chunk = [0_u8; 8192]; + let count = self.master.read(&mut chunk)?; + self.output.extend_from_slice(&chunk[..count]); + self.parser.process(&chunk[..count]); + Ok(()) + } + + fn write_input(&mut self, bytes: &[u8]) -> Result<()> { + self.master.write_all(bytes)?; + self.master.flush()?; + Ok(()) + } + + fn screen_contains(&self, text: &str) -> bool { + self.parser.screen().contents().contains(text) + } + + fn screen_contents(&self) -> String { + self.parser.screen().contents() + } +} + +impl Drop for PtyCodex { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +fn contains_bytes(buffer: &[u8], needle: &[u8]) -> bool { + buffer.windows(needle.len()).any(|window| window == needle) +} + +fn write_test_config(codex_home: &Path, repo_root: &Path) -> Result<()> { + let repo_root = repo_root.display(); + let config = format!( + "model = \"gpt-5.6-terra\"\nmodel_provider = \"openai\"\n\ + suppress_unstable_features_warning = true\n\n\ + [projects.\"{repo_root}\"]\ntrust_level = \"trusted\"\n" + ); + std::fs::write(codex_home.join("config.toml"), config) + .context("write focus-test Codex configuration")?; + std::fs::write( + codex_home.join("auth.json"), + r#"{"OPENAI_API_KEY":"focus-palette-test","tokens":null,"last_refresh":null}"#, + ) + .context("write focus-test API-key authentication") +} diff --git a/codex-rs/tui/tests/suite/mod.rs b/codex-rs/tui/tests/suite/mod.rs index 7e19b10f2b..9565ec853f 100644 --- a/codex-rs/tui/tests/suite/mod.rs +++ b/codex-rs/tui/tests/suite/mod.rs @@ -1,4 +1,6 @@ // Aggregates all former standalone integration tests as modules. +#[cfg(unix)] +mod focus_palette; mod resize_reflow; mod status_indicator; mod vt100_history;