mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
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
This commit is contained in:
committed by
GitHub
Unverified
parent
3ba0e85edd
commit
989e513969
+42
-9
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user