From 58159383c4964b8bc196b533d121b70c04d3f0d1 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Mon, 20 Oct 2025 14:40:14 -0700 Subject: [PATCH] fix terminal corruption that could happen when onboarding and update banner (#5269) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of printing characters before booting the app, make the upgrade banner a history cell so it's well-behaved. Screenshot 2025-10-16 at 4 20 51 PM --------- Co-authored-by: Josh McKinney --- codex-rs/Cargo.lock | 10 +++ codex-rs/Cargo.toml | 1 + codex-rs/cli/src/main.rs | 2 +- codex-rs/tui/Cargo.toml | 1 + codex-rs/tui/src/app.rs | 20 ++++- codex-rs/tui/src/history_cell.rs | 56 ++++++++++++++ codex-rs/tui/src/lib.rs | 124 +----------------------------- codex-rs/tui/src/update_prompt.rs | 4 +- codex-rs/tui/src/updates.rs | 69 ++++++++++++++++- 9 files changed, 157 insertions(+), 130 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 5075d37f..f87f3af2 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1459,6 +1459,7 @@ dependencies = [ "pulldown-cmark", "rand 0.9.2", "ratatui", + "ratatui-macros", "regex-lite", "serde", "serde_json", @@ -4777,6 +4778,15 @@ dependencies = [ "unicode-width 0.2.1", ] +[[package]] +name = "ratatui-macros" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6fef540f80dbe8a0773266fa6077788ceb65ef624cdbf36e131aaf90b4a52df4" +dependencies = [ + "ratatui", +] + [[package]] name = "redox_syscall" version = "0.5.15" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 5b7732b2..67041c6f 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -150,6 +150,7 @@ pretty_assertions = "1.4.1" pulldown-cmark = "0.10" rand = "0.9" ratatui = "0.29.0" +ratatui-macros = "0.6.0" regex-lite = "0.1.7" reqwest = "0.12" rmcp = { version = "0.8.0", default-features = false } diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 39935034..940e1e53 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -19,7 +19,7 @@ use codex_exec::Cli as ExecCli; use codex_responses_api_proxy::Args as ResponsesApiProxyArgs; use codex_tui::AppExitInfo; use codex_tui::Cli as TuiCli; -use codex_tui::UpdateAction; +use codex_tui::updates::UpdateAction; use owo_colors::OwoColorize; use std::path::PathBuf; use supports_color::Stream; diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index e8cb0fa5..824e4fee 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -59,6 +59,7 @@ ratatui = { workspace = true, features = [ "unstable-rendered-line-info", "unstable-widget-ref", ] } +ratatui-macros = { workspace = true } regex-lite = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["preserve_order"] } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index e1c2d062..5c274121 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -1,4 +1,3 @@ -use crate::UpdateAction; use crate::app_backtrack::BacktrackState; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -13,6 +12,7 @@ use crate::render::highlight::highlight_bash_to_lines; use crate::resume_picker::ResumeSelection; use crate::tui; use crate::tui::TuiEvent; +use crate::updates::UpdateAction; use codex_ansi_escape::ansi_escape_line; use codex_core::AuthManager; use codex_core::ConversationManager; @@ -39,7 +39,9 @@ use std::thread; use std::time::Duration; use tokio::select; use tokio::sync::mpsc::unbounded_channel; -// use uuid::Uuid; + +#[cfg(not(debug_assertions))] +use crate::history_cell::UpdateAvailableHistoryCell; #[derive(Debug, Clone)] pub struct AppExitInfo { @@ -146,6 +148,8 @@ impl App { }; let file_search = FileSearchManager::new(config.cwd.clone(), app_event_tx.clone()); + #[cfg(not(debug_assertions))] + let upgrade_version = crate::updates::get_upgrade_version(&config); let mut app = Self { server: conversation_manager, @@ -166,6 +170,18 @@ impl App { pending_update_action: None, }; + #[cfg(not(debug_assertions))] + if let Some(latest_version) = upgrade_version { + app.handle_event( + tui, + AppEvent::InsertHistoryCell(Box::new(UpdateAvailableHistoryCell::new( + latest_version, + crate::updates::get_update_action(), + ))), + ) + .await?; + } + let tui_events = tui.event_stream(); tokio::pin!(tui_events); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 4cbf25ae..6bae1bd0 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -15,6 +15,8 @@ use crate::style::user_message_style; use crate::text_formatting::format_and_truncate_tool_result; use crate::text_formatting::truncate_text; use crate::ui_consts::LIVE_PREFIX_COLS; +use crate::updates::UpdateAction; +use crate::version::CODEX_CLI_VERSION; use crate::wrapping::RtOptions; use crate::wrapping::word_wrap_line; use crate::wrapping::word_wrap_lines; @@ -264,6 +266,60 @@ impl HistoryCell for PlainHistoryCell { } } +#[cfg_attr(debug_assertions, allow(dead_code))] +#[derive(Debug)] +pub(crate) struct UpdateAvailableHistoryCell { + latest_version: String, + update_action: Option, +} + +#[cfg_attr(debug_assertions, allow(dead_code))] +impl UpdateAvailableHistoryCell { + pub(crate) fn new(latest_version: String, update_action: Option) -> Self { + Self { + latest_version, + update_action, + } + } +} + +impl HistoryCell for UpdateAvailableHistoryCell { + fn display_lines(&self, width: u16) -> Vec> { + use ratatui_macros::line; + use ratatui_macros::text; + let update_instruction = if let Some(update_action) = self.update_action { + line!["Run ", update_action.command_str().cyan(), " to update."] + } else { + line![ + "See ", + "https://github.com/openai/codex".cyan().underlined(), + " for installation options." + ] + }; + + let content = text![ + line![ + padded_emoji("✨").bold().cyan(), + "Update available!".bold().cyan(), + " ", + format!("{CODEX_CLI_VERSION} -> {}", self.latest_version).bold(), + ], + update_instruction, + "", + "See full release notes:", + "https://github.com/openai/codex/releases/latest" + .cyan() + .underlined(), + ]; + + let inner_width = content + .width() + .min(usize::from(width.saturating_sub(4))) + .max(1); + with_border_with_inner_width(content.lines, inner_width) + } +} + #[derive(Debug)] pub(crate) struct PrefixedWrappedHistoryCell { text: Text<'static>, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 83a35f79..1d3cc4c0 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -71,45 +71,14 @@ mod text_formatting; mod tui; mod ui_consts; mod update_prompt; +pub mod updates; mod version; -/// Update action the CLI should perform after the TUI exits. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum UpdateAction { - /// Update via `npm install -g @openai/codex@latest`. - NpmGlobalLatest, - /// Update via `bun install -g @openai/codex@latest`. - BunGlobalLatest, - /// Update via `brew upgrade codex`. - BrewUpgrade, -} - -impl UpdateAction { - /// Returns the list of command-line arguments for invoking the update. - pub fn command_args(&self) -> (&'static str, &'static [&'static str]) { - match self { - UpdateAction::NpmGlobalLatest => ("npm", &["install", "-g", "@openai/codex@latest"]), - UpdateAction::BunGlobalLatest => ("bun", &["install", "-g", "@openai/codex@latest"]), - UpdateAction::BrewUpgrade => ("brew", &["upgrade", "codex"]), - } - } - - /// Returns string representation of the command-line arguments for invoking the update. - pub fn command_str(&self) -> String { - let (command, args) = self.command_args(); - let args_str = args.join(" "); - format!("{command} {args_str}") - } -} - mod wrapping; #[cfg(test)] pub mod test_backend; -#[cfg(not(debug_assertions))] -mod updates; - use crate::onboarding::TrustDirectorySelection; use crate::onboarding::WSL_INSTRUCTIONS; use crate::onboarding::onboarding_screen::OnboardingScreenArgs; @@ -343,56 +312,6 @@ async fn run_ratatui_app( } } - // Show update banner in terminal history (instead of stderr) so it is visible - // within the TUI scrollback. Building spans keeps styling consistent. - #[cfg(not(debug_assertions))] - if let Some(latest_version) = updates::get_upgrade_version(&initial_config) { - use crate::history_cell::padded_emoji; - use crate::history_cell::with_border_with_inner_width; - use ratatui::style::Stylize as _; - use ratatui::text::Line; - - let current_version = env!("CARGO_PKG_VERSION"); - - let mut content_lines: Vec> = vec![ - Line::from(vec![ - padded_emoji("✨").bold().cyan(), - "Update available!".bold().cyan(), - " ".into(), - format!("{current_version} -> {latest_version}.").bold(), - ]), - Line::from(""), - Line::from("See full release notes:"), - Line::from(""), - Line::from( - "https://github.com/openai/codex/releases/latest" - .cyan() - .underlined(), - ), - Line::from(""), - ]; - - if let Some(update_action) = get_update_action() { - content_lines.push(Line::from(vec![ - "Run ".into(), - update_action.command_str().cyan(), - " to update.".into(), - ])); - } else { - content_lines.push(Line::from(vec![ - "See ".into(), - "https://github.com/openai/codex".cyan().underlined(), - " for installation options.".into(), - ])); - } - - let viewport_width = tui.terminal.viewport_area.width as usize; - let inner_width = viewport_width.saturating_sub(4).max(1); - let mut lines = with_border_with_inner_width(content_lines, inner_width); - lines.push("".into()); - tui.insert_history_lines(lines); - } - // Initialize high-fidelity session event logging if enabled. session_log::maybe_init(&initial_config); @@ -525,47 +444,6 @@ async fn run_ratatui_app( app_result } -/// Get the update action from the environment. -/// Returns `None` if not managed by npm, bun, or brew. -#[cfg(not(debug_assertions))] -pub(crate) fn get_update_action() -> Option { - let exe = std::env::current_exe().unwrap_or_default(); - let managed_by_npm = std::env::var_os("CODEX_MANAGED_BY_NPM").is_some(); - let managed_by_bun = std::env::var_os("CODEX_MANAGED_BY_BUN").is_some(); - if managed_by_npm { - Some(UpdateAction::NpmGlobalLatest) - } else if managed_by_bun { - Some(UpdateAction::BunGlobalLatest) - } else if cfg!(target_os = "macos") - && (exe.starts_with("/opt/homebrew") || exe.starts_with("/usr/local")) - { - Some(UpdateAction::BrewUpgrade) - } else { - None - } -} - -#[test] -#[cfg(not(debug_assertions))] -fn test_get_update_action() { - let prev = std::env::var_os("CODEX_MANAGED_BY_NPM"); - - // First: no npm var -> expect None (we do not run from brew in CI) - unsafe { std::env::remove_var("CODEX_MANAGED_BY_NPM") }; - assert_eq!(get_update_action(), None); - - // Then: with npm var -> expect NpmGlobalLatest - unsafe { std::env::set_var("CODEX_MANAGED_BY_NPM", "1") }; - assert_eq!(get_update_action(), Some(UpdateAction::NpmGlobalLatest)); - - // Restore prior value to avoid leaking state - if let Some(v) = prev { - unsafe { std::env::set_var("CODEX_MANAGED_BY_NPM", v) }; - } else { - unsafe { std::env::remove_var("CODEX_MANAGED_BY_NPM") }; - } -} - #[expect( clippy::print_stderr, reason = "TUI should no longer be displayed, so we can write to stderr." diff --git a/codex-rs/tui/src/update_prompt.rs b/codex-rs/tui/src/update_prompt.rs index 5da84626..d505385b 100644 --- a/codex-rs/tui/src/update_prompt.rs +++ b/codex-rs/tui/src/update_prompt.rs @@ -1,6 +1,5 @@ #![cfg(not(debug_assertions))] -use crate::UpdateAction; use crate::history_cell::padded_emoji; use crate::key_hint; use crate::render::Insets; @@ -12,6 +11,7 @@ use crate::tui::FrameRequester; use crate::tui::Tui; use crate::tui::TuiEvent; use crate::updates; +use crate::updates::UpdateAction; use codex_core::config::Config; use color_eyre::Result; use crossterm::event::KeyCode; @@ -39,7 +39,7 @@ pub(crate) async fn run_update_prompt_if_needed( let Some(latest_version) = updates::get_upgrade_version_for_popup(config) else { return Ok(UpdatePromptOutcome::Continue); }; - let Some(update_action) = crate::get_update_action() else { + let Some(update_action) = crate::updates::get_update_action() else { return Ok(UpdatePromptOutcome::Continue); }; diff --git a/codex-rs/tui/src/updates.rs b/codex-rs/tui/src/updates.rs index ca004a36..fe859e15 100644 --- a/codex-rs/tui/src/updates.rs +++ b/codex-rs/tui/src/updates.rs @@ -1,5 +1,3 @@ -#![cfg(any(not(debug_assertions), test))] - use chrono::DateTime; use chrono::Duration; use chrono::Utc; @@ -142,6 +140,53 @@ fn parse_version(v: &str) -> Option<(u64, u64, u64)> { Some((maj, min, pat)) } +/// Update action the CLI should perform after the TUI exits. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum UpdateAction { + /// Update via `npm install -g @openai/codex@latest`. + NpmGlobalLatest, + /// Update via `bun install -g @openai/codex@latest`. + BunGlobalLatest, + /// Update via `brew upgrade codex`. + BrewUpgrade, +} + +#[cfg(any(not(debug_assertions), test))] +pub(crate) fn get_update_action() -> Option { + let exe = std::env::current_exe().unwrap_or_default(); + let managed_by_npm = std::env::var_os("CODEX_MANAGED_BY_NPM").is_some(); + let managed_by_bun = std::env::var_os("CODEX_MANAGED_BY_BUN").is_some(); + if managed_by_npm { + Some(UpdateAction::NpmGlobalLatest) + } else if managed_by_bun { + Some(UpdateAction::BunGlobalLatest) + } else if cfg!(target_os = "macos") + && (exe.starts_with("/opt/homebrew") || exe.starts_with("/usr/local")) + { + Some(UpdateAction::BrewUpgrade) + } else { + None + } +} + +impl UpdateAction { + /// Returns the list of command-line arguments for invoking the update. + pub fn command_args(self) -> (&'static str, &'static [&'static str]) { + match self { + UpdateAction::NpmGlobalLatest => ("npm", &["install", "-g", "@openai/codex@latest"]), + UpdateAction::BunGlobalLatest => ("bun", &["install", "-g", "@openai/codex@latest"]), + UpdateAction::BrewUpgrade => ("brew", &["upgrade", "codex"]), + } + } + + /// Returns string representation of the command-line arguments for invoking the update. + pub fn command_str(self) -> String { + let (command, args) = self.command_args(); + let args_str = args.join(" "); + format!("{command} {args_str}") + } +} + #[cfg(test)] mod tests { use super::*; @@ -165,4 +210,24 @@ mod tests { assert_eq!(parse_version(" 1.2.3 \n"), Some((1, 2, 3))); assert_eq!(is_newer(" 1.2.3 ", "1.2.2"), Some(true)); } + + #[test] + fn test_get_update_action() { + let prev = std::env::var_os("CODEX_MANAGED_BY_NPM"); + + // First: no npm var -> expect None (we do not run from brew in CI) + unsafe { std::env::remove_var("CODEX_MANAGED_BY_NPM") }; + assert_eq!(get_update_action(), None); + + // Then: with npm var -> expect NpmGlobalLatest + unsafe { std::env::set_var("CODEX_MANAGED_BY_NPM", "1") }; + assert_eq!(get_update_action(), Some(UpdateAction::NpmGlobalLatest)); + + // Restore prior value to avoid leaking state + if let Some(v) = prev { + unsafe { std::env::set_var("CODEX_MANAGED_BY_NPM", v) }; + } else { + unsafe { std::env::remove_var("CODEX_MANAGED_BY_NPM") }; + } + } }