diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index d7a605c73..11be48b2c 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -1312,6 +1312,7 @@ async fn run_ratatui_app( let mut tui = Tui::new( initialized_terminal.terminal, initialized_terminal.enhanced_keys_supported, + initialized_terminal.stderr_guard, ); let mut terminal_restore_guard = TerminalRestoreGuard::new(); diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 47a6aeb6e..49d838e86 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -56,6 +56,7 @@ mod frame_requester; #[cfg(unix)] mod job_control; mod keyboard_modes; +mod terminal_stderr; /// Target frame interval for UI redraw scheduling. pub(crate) const TARGET_FRAME_INTERVAL: Duration = frame_rate_limiter::MIN_FRAME_INTERVAL; @@ -66,6 +67,7 @@ pub type Terminal = CustomTerminal>; pub(crate) struct InitializedTerminal { pub(crate) terminal: Terminal, pub(crate) enhanced_keys_supported: bool, + pub(crate) stderr_guard: terminal_stderr::TerminalStderrGuard, } pub(crate) fn running_in_vscode_terminal() -> bool { @@ -281,7 +283,16 @@ pub fn restore() -> Result<()> { /// Uses a stronger keyboard reset than [`restore`] so the parent shell recovers even if a /// terminal missed the stack pop that normally pairs with [`set_modes`]. pub fn restore_after_exit() -> Result<()> { - restore_common(RawModeRestore::Disable, KeyboardRestore::ResetAfterExit) + let mut first_error = + restore_common(RawModeRestore::Disable, KeyboardRestore::ResetAfterExit).err(); + if let Err(err) = terminal_stderr::finish() { + first_error.get_or_insert(err); + } + + match first_error { + Some(err) => Err(err), + None => Ok(()), + } } /// Restore the terminal to its original state, but keep raw mode enabled. @@ -425,9 +436,11 @@ pub(crate) fn init() -> Result { !keyboard_modes::keyboard_enhancement_disabled() && detect_keyboard_enhancement_supported(); let tui = CustomTerminal::with_options_and_cursor_position(backend, cursor_pos)?; + let stderr_guard = terminal_stderr::TerminalStderrGuard::install()?; Ok(InitializedTerminal { terminal: tui, enhanced_keys_supported, + stderr_guard, }) } @@ -489,6 +502,8 @@ pub struct Tui { notification_condition: NotificationCondition, // When false, enter_alt_screen() becomes a no-op. alt_screen_enabled: bool, + // Keeps unmanaged process stderr writes out of the inline viewport. + _stderr_guard: terminal_stderr::TerminalStderrGuard, } struct PendingHistoryLines { @@ -509,7 +524,11 @@ where } impl Tui { - pub fn new(terminal: Terminal, enhanced_keys_supported: bool) -> Self { + pub(crate) fn new( + terminal: Terminal, + enhanced_keys_supported: bool, + stderr_guard: terminal_stderr::TerminalStderrGuard, + ) -> Self { let (draw_tx, _) = broadcast::channel(1); let frame_requester = FrameRequester::new(draw_tx.clone()); @@ -534,6 +553,7 @@ impl Tui { notification_backend: Some(detect_backend(NotificationMethod::default())), notification_condition: NotificationCondition::default(), alt_screen_enabled: true, + _stderr_guard: stderr_guard, } } @@ -577,8 +597,8 @@ impl Tui { /// Temporarily restore terminal state to run an external interactive program `f`. /// /// This pauses crossterm's stdin polling by dropping the underlying event stream, restores - /// terminal modes (optionally keeping raw mode enabled), then re-applies Codex TUI modes and - /// flushes pending stdin input before resuming events. + /// terminal modes and stderr (optionally keeping raw mode enabled), then re-applies Codex TUI + /// modes and stderr suppression before resuming events. pub async fn with_restored(&mut self, mode: RestoreMode, f: F) -> R where F: FnOnce() -> Fut, @@ -596,9 +616,15 @@ impl Tui { if let Err(err) = mode.restore() { tracing::warn!("failed to restore terminal modes before external program: {err}"); } + if let Err(err) = terminal_stderr::pause() { + tracing::warn!("failed to restore terminal stderr before external program: {err}"); + } let output = f().await; + if let Err(err) = terminal_stderr::resume() { + tracing::warn!("failed to suppress terminal stderr after external program: {err}"); + } if let Err(err) = set_modes() { tracing::warn!("failed to re-enable terminal modes after external program: {err}"); } diff --git a/codex-rs/tui/src/tui/job_control.rs b/codex-rs/tui/src/tui/job_control.rs index 368041948..a07d42840 100644 --- a/codex-rs/tui/src/tui/job_control.rs +++ b/codex-rs/tui/src/tui/job_control.rs @@ -175,8 +175,12 @@ impl PreparedResumeAction { /// Deliver SIGTSTP after restoring terminal state, then re-applies terminal modes once resumed. fn suspend_process() -> Result<()> { super::restore()?; - unsafe { libc::kill(0, libc::SIGTSTP) }; + super::terminal_stderr::pause()?; + unsafe { + libc::kill(/*pid*/ 0, libc::SIGTSTP) + }; // After the process resumes, reapply terminal modes so drawing can continue. + super::terminal_stderr::resume()?; super::set_modes()?; Ok(()) } diff --git a/codex-rs/tui/src/tui/terminal_stderr.rs b/codex-rs/tui/src/tui/terminal_stderr.rs new file mode 100644 index 000000000..e1e0c0cdb --- /dev/null +++ b/codex-rs/tui/src/tui/terminal_stderr.rs @@ -0,0 +1,275 @@ +//! Protect the inline viewport from unmanaged macOS writes to stderr. +//! +//! Some macOS frameworks and runtime diagnostics write directly to file +//! descriptor 2. While the inline TUI is active, those writes paint into the +//! same terminal region as the composer without going through the renderer. +//! Keep them off the terminal until the TUI releases terminal ownership. + +use std::io; + +#[cfg(target_os = "macos")] +use std::fs::OpenOptions; +#[cfg(target_os = "macos")] +use std::io::IsTerminal; +#[cfg(target_os = "macos")] +use std::mem::MaybeUninit; +#[cfg(target_os = "macos")] +use std::os::fd::AsRawFd; +#[cfg(target_os = "macos")] +use std::os::fd::FromRawFd; +#[cfg(target_os = "macos")] +use std::os::fd::OwnedFd; +#[cfg(target_os = "macos")] +use std::sync::Mutex; +#[cfg(target_os = "macos")] +use std::sync::MutexGuard; + +#[cfg(target_os = "macos")] +static STDERR_STATE: Mutex = Mutex::new(StderrState { + owner_active: false, + saved_stderr: None, +}); + +#[cfg(target_os = "macos")] +struct StderrState { + owner_active: bool, + saved_stderr: Option, +} + +/// Keeps unmanaged stderr output away from the terminal while the TUI owns it. +pub(crate) struct TerminalStderrGuard { + active: bool, +} + +impl TerminalStderrGuard { + pub(super) fn install() -> io::Result { + #[cfg(target_os = "macos")] + { + if stderr_targets_stdout_terminal() { + return Self::install_suppression(); + } + } + + Ok(Self { active: false }) + } + + #[cfg(target_os = "macos")] + fn install_suppression() -> io::Result { + let mut state = lock_state()?; + if state.owner_active { + return Err(io::Error::new( + io::ErrorKind::AlreadyExists, + "terminal stderr suppression is already active", + )); + } + suppress_locked(&mut state)?; + state.owner_active = true; + Ok(Self { active: true }) + } +} + +impl Drop for TerminalStderrGuard { + fn drop(&mut self) { + if self.active { + let _ = finish(); + self.active = false; + } + } +} + +/// Restores stderr while terminal ownership is temporarily released. +pub(super) fn pause() -> io::Result<()> { + #[cfg(target_os = "macos")] + { + let mut state = lock_state()?; + if state.owner_active { + restore_locked(&mut state)?; + } + } + + Ok(()) +} + +/// Suppresses stderr again when terminal ownership returns to the TUI. +pub(super) fn resume() -> io::Result<()> { + #[cfg(target_os = "macos")] + { + let mut state = lock_state()?; + if state.owner_active { + suppress_locked(&mut state)?; + } + } + + Ok(()) +} + +/// Restores stderr permanently when the TUI session ends. +pub(super) fn finish() -> io::Result<()> { + #[cfg(target_os = "macos")] + { + let mut state = lock_state()?; + if state.owner_active { + restore_locked(&mut state)?; + state.owner_active = false; + } + } + + Ok(()) +} + +#[cfg(target_os = "macos")] +fn lock_state() -> io::Result> { + STDERR_STATE + .lock() + .map_err(|_| io::Error::other("terminal stderr suppression lock poisoned")) +} + +#[cfg(target_os = "macos")] +fn stderr_targets_stdout_terminal() -> bool { + if !io::stdout().is_terminal() || !io::stderr().is_terminal() { + return false; + } + + let mut stdout_stat = MaybeUninit::::uninit(); + let mut stderr_stat = MaybeUninit::::uninit(); + // SAFETY: both output pointers reference valid storage for libc to initialize. + if unsafe { + libc::fstat(libc::STDOUT_FILENO, stdout_stat.as_mut_ptr()) != 0 + || libc::fstat(libc::STDERR_FILENO, stderr_stat.as_mut_ptr()) != 0 + } { + return false; + } + // SAFETY: both fstat calls above returned successfully. + let (stdout_stat, stderr_stat) = + unsafe { (stdout_stat.assume_init(), stderr_stat.assume_init()) }; + stdout_stat.st_dev == stderr_stat.st_dev && stdout_stat.st_ino == stderr_stat.st_ino +} + +#[cfg(target_os = "macos")] +fn suppress_locked(state: &mut StderrState) -> io::Result<()> { + if state.saved_stderr.is_some() { + return Ok(()); + } + + // SAFETY: dup returns a newly owned file descriptor on success. + let saved_stderr = unsafe { libc::dup(libc::STDERR_FILENO) }; + if saved_stderr == -1 { + return Err(io::Error::last_os_error()); + } + // SAFETY: saved_stderr is a fresh descriptor returned by dup above. + let saved_stderr = unsafe { OwnedFd::from_raw_fd(saved_stderr) }; + let devnull = OpenOptions::new().write(true).open("/dev/null")?; + // SAFETY: both descriptors are valid for the duration of this call. + if unsafe { libc::dup2(devnull.as_raw_fd(), libc::STDERR_FILENO) } == -1 { + return Err(io::Error::last_os_error()); + } + state.saved_stderr = Some(saved_stderr); + Ok(()) +} + +#[cfg(target_os = "macos")] +fn restore_locked(state: &mut StderrState) -> io::Result<()> { + let Some(saved_stderr) = state.saved_stderr.as_ref() else { + return Ok(()); + }; + + // SAFETY: saved_stderr was duplicated from stderr and remains owned here. + if unsafe { libc::dup2(saved_stderr.as_raw_fd(), libc::STDERR_FILENO) } == -1 { + return Err(io::Error::last_os_error()); + } + state.saved_stderr = None; + Ok(()) +} + +#[cfg(all(test, target_os = "macos"))] +mod tests { + use std::fs::File; + use std::io::Read; + use std::io::Seek; + use std::io::Write; + use std::os::fd::AsRawFd; + use std::os::fd::FromRawFd; + use std::os::fd::OwnedFd; + + use pretty_assertions::assert_eq; + use serial_test::serial; + + use super::TerminalStderrGuard; + use super::finish; + use super::pause; + use super::resume; + + struct CapturedStderr { + saved_stderr: OwnedFd, + } + + impl CapturedStderr { + fn start(file: &File) -> std::io::Result { + // SAFETY: dup returns a newly owned file descriptor on success. + let saved_stderr = unsafe { libc::dup(libc::STDERR_FILENO) }; + if saved_stderr == -1 { + return Err(std::io::Error::last_os_error()); + } + // SAFETY: saved_stderr is a fresh descriptor returned by dup above. + let saved_stderr = unsafe { OwnedFd::from_raw_fd(saved_stderr) }; + // SAFETY: both descriptors are valid for the duration of this call. + if unsafe { libc::dup2(file.as_raw_fd(), libc::STDERR_FILENO) } == -1 { + return Err(std::io::Error::last_os_error()); + } + Ok(Self { saved_stderr }) + } + } + + impl Drop for CapturedStderr { + fn drop(&mut self) { + // SAFETY: saved_stderr remains owned for the duration of this call. + let _ = unsafe { libc::dup2(self.saved_stderr.as_raw_fd(), libc::STDERR_FILENO) }; + } + } + + fn write_stderr(message: &str) -> std::io::Result<()> { + let mut stderr = std::io::stderr().lock(); + stderr.write_all(message.as_bytes())?; + stderr.flush() + } + + #[test] + #[serial] + fn suppresses_stderr_only_while_terminal_is_owned() -> std::io::Result<()> { + let mut output = tempfile::tempfile()?; + let capture = CapturedStderr::start(&output)?; + + let _guard = TerminalStderrGuard::install_suppression()?; + write_stderr("hidden while active\n")?; + pause()?; + write_stderr("visible while paused\n")?; + resume()?; + write_stderr("hidden after resume\n")?; + finish()?; + write_stderr("visible after finish\n")?; + + drop(capture); + output.rewind()?; + let mut captured = String::new(); + output.read_to_string(&mut captured)?; + assert_eq!(captured, "visible while paused\nvisible after finish\n"); + Ok(()) + } + + #[test] + #[serial] + fn preserves_stderr_when_already_redirected() -> std::io::Result<()> { + let mut output = tempfile::tempfile()?; + let capture = CapturedStderr::start(&output)?; + + let _guard = TerminalStderrGuard::install()?; + write_stderr("visible while redirected\n")?; + + drop(capture); + output.rewind()?; + let mut captured = String::new(); + output.read_to_string(&mut captured)?; + assert_eq!(captured, "visible while redirected\n"); + Ok(()) + } +}