diff --git a/codex-rs/tui/src/terminal_probe.rs b/codex-rs/tui/src/terminal_probe.rs index bea195083..9e88dda93 100644 --- a/codex-rs/tui/src/terminal_probe.rs +++ b/codex-rs/tui/src/terminal_probe.rs @@ -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> { + 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( tty: &mut Tty, timeout: Duration, diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 681e0cb39..a0f7ade04 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -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()?; diff --git a/codex-rs/tui/src/tui/event_stream.rs b/codex-rs/tui/src/tui/event_stream.rs index dcc6e17e0..9a5e416fd 100644 --- a/codex-rs/tui/src/tui/event_stream.rs +++ b/codex-rs/tui/src/tui/event_stream.rs @@ -239,7 +239,16 @@ impl TuiEventStream { 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)) diff --git a/codex-rs/tui/src/tui/job_control.rs b/codex-rs/tui/src/tui/job_control.rs index a07d42840..bc46a0a89 100644 --- a/codex-rs/tui/src/tui/job_control.rs +++ b/codex-rs/tui/src/tui/job_control.rs @@ -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, ) -> Option { 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