mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
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
This commit is contained in:
@@ -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<DefaultColors> {
|
||||
crate::terminal_probe::default_colors(crate::terminal_probe::DEFAULT_TIMEOUT)
|
||||
.ok()
|
||||
@@ -293,8 +254,6 @@ mod imp {
|
||||
_colors: Option<crate::terminal_probe::DefaultColors>,
|
||||
) {
|
||||
}
|
||||
|
||||
pub(super) fn requery_default_colors() {}
|
||||
}
|
||||
|
||||
/// The subset of Xterm colors that are usually consistent across terminals.
|
||||
|
||||
@@ -271,7 +271,8 @@ impl<S: EventSource + Default + Unpin> TuiEventStream<S> {
|
||||
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();
|
||||
|
||||
@@ -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<u8>,
|
||||
cursor_answered: bool,
|
||||
palette_answered: bool,
|
||||
keyboard_answered: bool,
|
||||
_codex_home: TempDir,
|
||||
}
|
||||
|
||||
impl PtyCodex {
|
||||
fn start(repo_root: &Path, codex_home: TempDir) -> Result<Self> {
|
||||
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")
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user