From 989e513969424117baabd1090914d1ad29a05d0b Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Tue, 24 Mar 2026 14:29:29 -0600 Subject: [PATCH] tui: always restore the terminal on early exit (#15671) ## Summary Fixes early TUI exit paths that could leave the terminal in a dirty state and cause a stray `%` prompt marker after the app quit. ## Root cause Both `tui` and `tui_app_server` had early returns after `tui::init()` that did not guarantee terminal restore. When that happened, shells like `zsh` inherited the altered terminal state. ## Changes - Add a restore guard around `run_ratatui_app()` in both `tui` and `tui_app_server` - Route early exits through the guard instead of relying on scattered manual restore calls - Ensure terminal restore still happens on normal shutdown --- codex-rs/tui/src/lib.rs | 51 ++++++++++++++++++++++++------ codex-rs/tui_app_server/src/lib.rs | 49 +++++++++++++++++++++++----- 2 files changed, 83 insertions(+), 17 deletions(-) diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 541620069..039148a71 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -609,6 +609,7 @@ async fn run_ratatui_app( terminal.clear()?; let mut tui = Tui::new(terminal); + let mut terminal_restore_guard = TerminalRestoreGuard::new(); #[cfg(not(debug_assertions))] { @@ -619,7 +620,7 @@ async fn run_ratatui_app( match update_prompt::run_update_prompt_if_needed(&mut tui, &initial_config).await? { UpdatePromptOutcome::Continue => {} UpdatePromptOutcome::RunUpdate(action) => { - crate::tui::restore()?; + terminal_restore_guard.restore()?; return Ok(AppExitInfo { token_usage: codex_protocol::protocol::TokenUsage::default(), thread_id: None, @@ -660,7 +661,7 @@ async fn run_ratatui_app( ) .await?; if onboarding_result.should_exit { - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); let _ = tui.terminal.clear(); return Ok(AppExitInfo { @@ -701,7 +702,7 @@ async fn run_ratatui_app( let mut missing_session_exit = |id_str: &str, action: &str| { error!("Error finding conversation path: {id_str}"); - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); let _ = tui.terminal.clear(); Ok(AppExitInfo { @@ -773,7 +774,7 @@ async fn run_ratatui_app( error!( "Error reading session metadata from latest rollout: {rollout_path}" ); - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); let _ = tui.terminal.clear(); return Ok(AppExitInfo { @@ -795,7 +796,7 @@ async fn run_ratatui_app( } else if cli.fork_picker { match resume_picker::run_fork_picker(&mut tui, &config, cli.fork_show_all).await? { resume_picker::SessionSelection::Exit => { - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); return Ok(AppExitInfo { token_usage: codex_protocol::protocol::TokenUsage::default(), @@ -867,7 +868,7 @@ async fn run_ratatui_app( error!( "Error reading session metadata from latest rollout: {rollout_path}" ); - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); let _ = tui.terminal.clear(); return Ok(AppExitInfo { @@ -887,7 +888,7 @@ async fn run_ratatui_app( } else if cli.resume_picker { match resume_picker::run_resume_picker(&mut tui, &config, cli.resume_show_all).await? { resume_picker::SessionSelection::Exit => { - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); return Ok(AppExitInfo { token_usage: codex_protocol::protocol::TokenUsage::default(), @@ -929,7 +930,7 @@ async fn run_ratatui_app( { ResolveCwdOutcome::Continue(cwd) => cwd, ResolveCwdOutcome::Exit => { - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); return Ok(AppExitInfo { token_usage: codex_protocol::protocol::TokenUsage::default(), @@ -1003,7 +1004,7 @@ async fn run_ratatui_app( ) .await; - restore(); + terminal_restore_guard.restore_silently(); // Mark the end of the recorded session. session_log::log_session_end(); // ignore error when collecting usage – report underlying error instead @@ -1128,6 +1129,38 @@ fn restore() { } } +struct TerminalRestoreGuard { + active: bool, +} + +impl TerminalRestoreGuard { + fn new() -> Self { + Self { active: true } + } + + #[cfg_attr(debug_assertions, allow(dead_code))] + fn restore(&mut self) -> color_eyre::Result<()> { + if self.active { + crate::tui::restore()?; + self.active = false; + } + Ok(()) + } + + fn restore_silently(&mut self) { + if self.active { + restore(); + self.active = false; + } + } +} + +impl Drop for TerminalRestoreGuard { + fn drop(&mut self) { + self.restore_silently(); + } +} + /// Determine whether to use the terminal's alternate screen buffer. /// /// The alternate screen buffer provides a cleaner fullscreen experience without polluting diff --git a/codex-rs/tui_app_server/src/lib.rs b/codex-rs/tui_app_server/src/lib.rs index 17e309d5f..790a7c6f5 100644 --- a/codex-rs/tui_app_server/src/lib.rs +++ b/codex-rs/tui_app_server/src/lib.rs @@ -938,6 +938,7 @@ async fn run_ratatui_app( terminal.clear()?; let mut tui = Tui::new(terminal); + let mut terminal_restore_guard = TerminalRestoreGuard::new(); #[cfg(not(debug_assertions))] { @@ -948,7 +949,7 @@ async fn run_ratatui_app( match update_prompt::run_update_prompt_if_needed(&mut tui, &initial_config).await? { UpdatePromptOutcome::Continue => {} UpdatePromptOutcome::RunUpdate(action) => { - crate::tui::restore()?; + terminal_restore_guard.restore()?; return Ok(AppExitInfo { token_usage: codex_protocol::protocol::TokenUsage::default(), thread_id: None, @@ -1016,7 +1017,7 @@ async fn run_ratatui_app( ) .await?; if onboarding_result.should_exit { - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); let _ = tui.terminal.clear(); return Ok(AppExitInfo { @@ -1062,7 +1063,7 @@ async fn run_ratatui_app( let mut missing_session_exit = |id_str: &str, action: &str| { error!("Error finding conversation path: {id_str}"); - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); let _ = tui.terminal.clear(); Ok(AppExitInfo { @@ -1137,7 +1138,7 @@ async fn run_ratatui_app( .await? { resume_picker::SessionSelection::Exit => { - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); return Ok(AppExitInfo { token_usage: codex_protocol::protocol::TokenUsage::default(), @@ -1189,7 +1190,7 @@ async fn run_ratatui_app( .await? { resume_picker::SessionSelection::Exit => { - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); return Ok(AppExitInfo { token_usage: codex_protocol::protocol::TokenUsage::default(), @@ -1235,7 +1236,7 @@ async fn run_ratatui_app( { ResolveCwdOutcome::Continue(cwd) => cwd, ResolveCwdOutcome::Exit => { - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); return Ok(AppExitInfo { token_usage: codex_protocol::protocol::TokenUsage::default(), @@ -1303,7 +1304,7 @@ async fn run_ratatui_app( { Ok(app_server) => app_server, Err(err) => { - restore(); + terminal_restore_guard.restore_silently(); session_log::log_session_end(); return Err(err); } @@ -1326,7 +1327,7 @@ async fn run_ratatui_app( ) .await; - restore(); + terminal_restore_guard.restore_silently(); // Mark the end of the recorded session. session_log::log_session_end(); // ignore error when collecting usage – report underlying error instead @@ -1468,6 +1469,38 @@ fn restore() { } } +struct TerminalRestoreGuard { + active: bool, +} + +impl TerminalRestoreGuard { + fn new() -> Self { + Self { active: true } + } + + #[cfg_attr(debug_assertions, allow(dead_code))] + fn restore(&mut self) -> color_eyre::Result<()> { + if self.active { + crate::tui::restore()?; + self.active = false; + } + Ok(()) + } + + fn restore_silently(&mut self) { + if self.active { + restore(); + self.active = false; + } + } +} + +impl Drop for TerminalRestoreGuard { + fn drop(&mut self) { + self.restore_silently(); + } +} + /// Determine whether to use the terminal's alternate screen buffer. /// /// The alternate screen buffer provides a cleaner fullscreen experience without polluting