agent/codex
1
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(tui): restore TUI after suspend (#28342)

Browse Source 
## Why

On Linux, suspending Codex with `Ctrl+Z` and returning with `fg` can
leave the composer misaligned or inject terminal response bytes such as
focus reports into the prompt. Shell job-control output moves the cursor
while Codex is suspended, and terminal input polling can race with the
responses used to restore the inline viewport.

Fixes #26564.

## What changed

- preserve and restore keyboard reporting without disturbing the parent
terminal stack
- pause terminal event polling while Codex is suspended and flush
buffered input before resuming it
- force crossterm's cached raw-mode state back in sync after the shell
completes its `fg` handoff
- probe the actual post-`fg` cursor position with the tolerant
terminal-response parser, then realign the inline viewport before
redrawing

## How to Test

1. On Linux, start the development TUI with `just c`.
2. Type text into the composer without submitting it.
3. Press `Ctrl+Z`, run any harmless shell command, then run `fg`.
4. Confirm the composer redraws below the shell output, the draft text
is preserved, and no raw escape sequences appear.
5. Repeat the suspend/resume cycle and confirm normal typing still
works.

Targeted tests:

- `cargo test -p codex-tui --lib parses_cursor_position_as_zero_based -j
1`
- `cargo test -p codex-tui --lib tui::event_stream::tests -j 1`
This commit is contained in:
Felipe Coury
2026-06-16 12:09:24 -04:00
committed by GitHub
Unverified
parent 7162030b37
commit 76135cbe7e
4 changed files with 82 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/terminal_probe.rs
+22 -11
View File
@@ -1,16 +1,16 @@
//! Short, best-effort terminal response probes for TUI startup.
//! Short, best-effort terminal response probes for TUI startup and resume.
//!
//! 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.
//! TUI startup and resume, where unsupported terminals should simply fall back to conservative
//! defaults.
//! This module sends the same kinds of optional terminal queries with a caller-provided deadline,
//! prefers duplicated stdio handles, falls back to the controlling terminal path when stdio is
//! unavailable, and reports `None` when a response is unavailable.
//!
//! The probes run before the crossterm event stream is created, so they do not share crossterm's
//! internal skipped-event queue. Bytes read while looking for probe responses are consumed from the
//! terminal; keeping the timeout short is part of the contract that makes this acceptable for
//! startup. A future input-preservation layer would need to replay unrelated bytes through the same
//! parser that normal TUI input uses.
//! Probes run only while the crossterm event stream is absent or paused, so they do not share
//! crossterm's internal skipped-event queue. Bytes read while looking for probe responses are
//! consumed from the terminal; callers must therefore own terminal input for the duration of the
//! short timeout and accept that unrelated buffered input will be discarded.
use std::time::Duration;
@@ -58,7 +58,7 @@ mod imp {
Skip,
}
/// Temporary terminal handle used while a startup probe owns terminal input.
/// Temporary terminal handle used while a probe owns terminal input.
///
/// The preferred path is duplicated stdin/stdout, because terminal replies are delivered to the
/// same input stream crossterm reads from. Some embedded or redirected environments expose a
@@ -72,7 +72,7 @@ mod imp {
}
impl Tty {
/// Opens an isolated reader and writer for startup probes.
/// Opens an isolated reader and writer for terminal probes.
///
/// The reader and writer must be separate file descriptions so switching the reader into
/// nonblocking mode does not also make writes fail with `WouldBlock` under terminal
@@ -232,6 +232,17 @@ mod imp {
Ok(Some(colors))
}
/// Queries the terminal cursor position while normal input polling is paused.
///
/// Resume can emit a focus report immediately before the cursor-position response. Reusing
/// the startup parser lets the probe find the response without leaking either sequence into
/// the composer.
pub(crate) fn cursor_position(timeout: Duration) -> io::Result<Option<Position>> {
let mut tty = Tty::open()?;
tty.write_all(b"\x1B[6n")?;
read_until(&mut tty, timeout, parse_cursor_position)
}
/// Runs the optional terminal queries needed during TUI startup under one shared deadline.
///
/// Keeping these queries batched avoids paying one timeout per unsupported capability before
@@ -255,8 +266,8 @@ mod imp {
/// Reads available terminal bytes until `parse` recognizes a probe response or time expires.
///
/// The accumulated buffer may include unrelated terminal input. This helper intentionally does
/// not try to replay those bytes, so it must stay limited to short startup probes that run
/// before normal crossterm input polling begins.
/// not try to replay those bytes, so callers must use it only during short, exclusive probe
/// windows before normal crossterm input polling begins or while that polling is paused.
fn read_until<T>(
tty: &mut Tty,
timeout: Duration,
codex-rs/tui/src/tui.rs
+14 -2
View File
@@ -281,6 +281,18 @@ pub fn restore() -> Result<()> {
restore_common(RawModeRestore::Disable, KeyboardRestore::PopStack)
}
/// Force crossterm's cached raw-mode state back in sync with the terminal after `fg`.
///
/// A shell may restore the job's saved termios after the process receives `SIGCONT`. When that
/// races with [`set_modes`], crossterm still believes raw mode is enabled even though the terminal
/// has returned to canonical, echoing mode. Clearing crossterm's saved state before enabling raw
/// mode again makes the kernel state authoritative once the shell has completed its handoff.
#[cfg(unix)]
pub(super) fn reapply_raw_mode_after_resume() -> Result<()> {
disable_raw_mode()?;
enable_raw_mode()
}
/// Restore the terminal after Codex is exiting.
///
/// Uses a stronger keyboard reset than [`restore`] so the parent shell recovers even if a
@@ -873,7 +885,7 @@ impl Tui {
#[cfg(unix)]
let mut prepared_resume = self
.suspend_context
.prepare_resume_action(&mut self.terminal, &mut self.alt_saved_viewport);
.prepare_resume_action(&mut self.alt_saved_viewport);
// Precompute any viewport updates that need a cursor-position query before entering
// the synchronized update, to avoid racing with the event reader.
@@ -1009,7 +1021,7 @@ impl Tui {
#[cfg(unix)]
let mut prepared_resume = self
.suspend_context
.prepare_resume_action(&mut self.terminal, &mut self.alt_saved_viewport);
.prepare_resume_action(&mut self.alt_saved_viewport);
ensure_virtual_terminal_processing()?;
codex-rs/tui/src/tui/event_stream.rs
+10 -1
View File
@@ -239,7 +239,16 @@ impl<S: EventSource + Default + Unpin> TuiEventStream<S> {
Event::Key(key_event) => {
#[cfg(unix)]
if crate::tui::job_control::SUSPEND_KEY.is_press(key_event) {
let _ = self.suspend_context.suspend(&self.alt_screen_active);
self.broker.pause_events();
let suspend_result = self.suspend_context.suspend(&self.alt_screen_active);
self.broker.resume_events();
if let Err(err) = suspend_result {
tracing::warn!(
event = "tui_suspend_failed",
error = %err,
"failed to suspend TUI process"
);
}
return Some(TuiEvent::Draw);
}
Some(TuiEvent::Key(key_event))
codex-rs/tui/src/tui/job_control.rs
+36 -11
View File
@@ -13,7 +13,6 @@ use crossterm::event::KeyCode;
use crossterm::terminal::EnterAlternateScreen;
use crossterm::terminal::LeaveAlternateScreen;
use ratatui::crossterm::execute;
use ratatui::layout::Position;
use ratatui::layout::Rect;
use crate::key_hint;
@@ -72,7 +71,30 @@ impl SuspendContext {
}
let y = self.suspend_cursor_y.load(Ordering::Relaxed);
let _ = execute!(stdout(), MoveTo(0, y), Show);
suspend_process()
suspend_process()?;
super::reapply_raw_mode_after_resume()?;
// The shell writes its job-control status and the resumed command after `fg`, so the
// cursor may no longer be on the row cached before suspending. The event stream remains
// paused until this method returns, which makes it safe for the probe to consume both an
// interleaved focus report and the cursor-position response without racing the background
// input reader.
match crate::terminal_probe::cursor_position(crate::terminal_probe::DEFAULT_TIMEOUT) {
Ok(Some(position)) => self.set_cursor_y(position.y),
Ok(None) => tracing::debug!("terminal cursor position unavailable after resume"),
Err(err) => tracing::debug!(
error = %err,
"failed to read terminal cursor position after resume"
),
}
super::flush_terminal_input_buffer();
tracing::trace!(
event = "tui_suspend_resumed",
cursor_y = self.cursor_y(),
"restored terminal state after resume"
);
Ok(())
}
/// Consume the pending resume intent and precompute any viewport changes needed post-resume.
@@ -81,23 +103,22 @@ impl SuspendContext {
/// resumes; returns `None` when there was no pending suspend intent.
pub(crate) fn prepare_resume_action(
&self,
terminal: &mut Terminal,
alt_saved_viewport: &mut Option<Rect>,
) -> Option<PreparedResumeAction> {
let action = self.take_resume_action()?;
match action {
ResumeAction::RealignInline => {
let cursor_pos = terminal
.get_cursor_position()
.unwrap_or(terminal.last_known_cursor_pos);
let viewport = Rect::new(0, cursor_pos.y, 0, 0);
let viewport = Rect::new(
/*x*/ 0,
self.cursor_y(),
/*width*/ 0,
/*height*/ 0,
);
Some(PreparedResumeAction::RealignViewport(viewport))
}
ResumeAction::RestoreAlt => {
if let Ok(Position { y, .. }) = terminal.get_cursor_position()
&& let Some(saved) = alt_saved_viewport.as_mut()
{
saved.y = y;
if let Some(saved) = alt_saved_viewport.as_mut() {
saved.y = self.cursor_y();
}
Some(PreparedResumeAction::RestoreAltScreen)
}
@@ -112,6 +133,10 @@ impl SuspendContext {
self.suspend_cursor_y.store(value, Ordering::Relaxed);
}
fn cursor_y(&self) -> u16 {
self.suspend_cursor_y.load(Ordering::Relaxed)
}
/// Record a pending resume action to apply after SIGTSTP returns control.
fn set_resume_action(&self, value: ResumeAction) {
*self