diff --git a/AGENTS.md b/AGENTS.md index 672c0086d6..a6e2e565e0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,6 +11,7 @@ In the codex-rs folder where the rust code lives: Before finalizing a change to `codex-rs`, run `just fmt` (in `codex-rs` directory) to format the code and `just fix -p ` (in `codex-rs` directory) to fix any linter issues in the code. Additionally, run the tests: 1. Run the test for the specific project that was changed. For example, if changes were made in `codex-rs/tui`, run `cargo test -p codex-tui`. 2. Once those pass, if any changes were made in common, core, or protocol, run the complete test suite with `cargo test --all-features`. +When running interactively, ask the user before running these commands to finalize. ## TUI style conventions diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index ecf1ae7125..2159eaf68a 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -1,6 +1,5 @@ use std::io::BufRead; use std::path::Path; -use std::sync::OnceLock; use std::time::Duration; use bytes::Bytes; @@ -8,7 +7,6 @@ use codex_login::AuthManager; use codex_login::AuthMode; use eventsource_stream::Eventsource; use futures::prelude::*; -use regex_lite::Regex; use reqwest::StatusCode; use serde::Deserialize; use serde::Serialize; @@ -54,8 +52,11 @@ struct ErrorResponse { #[derive(Debug, Deserialize)] struct Error { r#type: Option, - code: Option, message: Option, + + // Optional fields available on "usage_limit_reached" and "usage_not_included" errors + plan_type: Option, + resets_in_seconds: Option, } #[derive(Debug, Clone)] @@ -142,9 +143,12 @@ impl ModelClient { } let auth_manager = self.auth_manager.clone(); - let auth = auth_manager.as_ref().and_then(|m| m.auth()); - let auth_mode = auth.as_ref().map(|a| a.mode); + let auth_mode = auth_manager + .as_ref() + .and_then(|m| m.auth()) + .as_ref() + .map(|a| a.mode); let store = prompt.store && auth_mode != Some(AuthMode::ChatGPT); @@ -211,15 +215,18 @@ impl ModelClient { let mut attempt = 0; let max_retries = self.provider.request_max_retries(); - trace!( - "POST to {}: {}", - self.provider.get_full_url(&auth), - serde_json::to_string(&payload)? - ); - loop { attempt += 1; + // Always fetch the latest auth in case a prior attempt refreshed the token. + let auth = auth_manager.as_ref().and_then(|m| m.auth()); + + trace!( + "POST to {}: {}", + self.provider.get_full_url(&auth), + serde_json::to_string(&payload)? + ); + let mut req_builder = self .provider .create_request_builder(&self.client, &auth) @@ -303,19 +310,20 @@ impl ModelClient { if status == StatusCode::TOO_MANY_REQUESTS { let body = res.json::().await.ok(); - if let Some(ErrorResponse { - error: - Error { - r#type: Some(error_type), - .. - }, - }) = body - { - if error_type == "usage_limit_reached" { + if let Some(ErrorResponse { error }) = body { + if error.r#type.as_deref() == Some("usage_limit_reached") { + // Prefer the plan_type provided in the error message if present + // because it's more up to date than the one encoded in the auth + // token. + let plan_type = error + .plan_type + .or_else(|| auth.and_then(|a| a.get_plan_type())); + let resets_in_seconds = error.resets_in_seconds; return Err(CodexErr::UsageLimitReached(UsageLimitReachedError { - plan_type: auth.and_then(|a| a.get_plan_type()), + plan_type, + resets_in_seconds, })); - } else if error_type == "usage_not_included" { + } else if error.r#type.as_deref() == Some("usage_not_included") { return Err(CodexErr::UsageNotIncluded); } } @@ -563,9 +571,8 @@ async fn process_sse( if let Some(error) = error { match serde_json::from_value::(error.clone()) { Ok(error) => { - let delay = try_parse_retry_after(&error); let message = error.message.unwrap_or_default(); - response_error = Some(CodexErr::Stream(message, delay)); + response_error = Some(CodexErr::Stream(message, None)); } Err(e) => { debug!("failed to parse ErrorResponse: {e}"); @@ -653,40 +660,6 @@ async fn stream_from_fixture( Ok(ResponseStream { rx_event }) } -fn rate_limit_regex() -> &'static Regex { - static RE: OnceLock = OnceLock::new(); - - #[expect(clippy::unwrap_used)] - RE.get_or_init(|| Regex::new(r"Please try again in (\d+(?:\.\d+)?)(s|ms)").unwrap()) -} - -fn try_parse_retry_after(err: &Error) -> Option { - if err.code != Some("rate_limit_exceeded".to_string()) { - return None; - } - - // parse the Please try again in 1.898s format using regex - let re = rate_limit_regex(); - if let Some(message) = &err.message - && let Some(captures) = re.captures(message) - { - let seconds = captures.get(1); - let unit = captures.get(2); - - if let (Some(value), Some(unit)) = (seconds, unit) { - let value = value.as_str().parse::().ok()?; - let unit = unit.as_str(); - - if unit == "s" { - return Some(Duration::from_secs_f64(value)); - } else if unit == "ms" { - return Some(Duration::from_millis(value as u64)); - } - } - } - None -} - #[cfg(test)] mod tests { use super::*; @@ -907,7 +880,7 @@ mod tests { msg, "Rate limit reached for gpt-5 in organization org-AAA on tokens per min (TPM): Limit 30000, Used 22999, Requested 12528. Please try again in 11.054s. Visit https://platform.openai.com/account/rate-limits to learn more." ); - assert_eq!(*delay, Some(Duration::from_secs_f64(11.054))); + assert_eq!(*delay, None); } other => panic!("unexpected second event: {other:?}"), } @@ -1011,27 +984,4 @@ mod tests { ); } } - - #[test] - fn test_try_parse_retry_after() { - let err = Error { - r#type: None, - message: Some("Rate limit reached for gpt-5 in organization org- on tokens per min (TPM): Limit 1, Used 1, Requested 19304. Please try again in 28ms. Visit https://platform.openai.com/account/rate-limits to learn more.".to_string()), - code: Some("rate_limit_exceeded".to_string()), - }; - - let delay = try_parse_retry_after(&err); - assert_eq!(delay, Some(Duration::from_millis(28))); - } - - #[test] - fn test_try_parse_retry_after_no_delay() { - let err = Error { - r#type: None, - message: Some("Rate limit reached for gpt-5 in organization on tokens per min (TPM): Limit 30000, Used 6899, Requested 24050. Please try again in 1.898s. Visit https://platform.openai.com/account/rate-limits to learn more.".to_string()), - code: Some("rate_limit_exceeded".to_string()), - }; - let delay = try_parse_retry_after(&err); - assert_eq!(delay, Some(Duration::from_secs_f64(1.898))); - } } diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 356c23011c..b05ff1a581 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -128,27 +128,70 @@ pub enum CodexErr { #[derive(Debug)] pub struct UsageLimitReachedError { pub plan_type: Option, + pub resets_in_seconds: Option, } impl std::fmt::Display for UsageLimitReachedError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // Base message differs slightly for legacy ChatGPT Plus plan users. if let Some(plan_type) = &self.plan_type && plan_type == "plus" { write!( f, - "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing), or wait for limits to reset (every 5h and every week.)." + "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing) or try again" )?; + if let Some(secs) = self.resets_in_seconds { + let reset_duration = format_reset_duration(secs); + write!(f, " in {reset_duration}.")?; + } else { + write!(f, " later.")?; + } } else { - write!( - f, - "You've hit your usage limit. Limits reset every 5h and every week." - )?; + write!(f, "You've hit your usage limit.")?; + + if let Some(secs) = self.resets_in_seconds { + let reset_duration = format_reset_duration(secs); + write!(f, " Try again in {reset_duration}.")?; + } else { + write!(f, " Try again later.")?; + } } + Ok(()) } } +fn format_reset_duration(total_secs: u64) -> String { + let days = total_secs / 86_400; + let hours = (total_secs % 86_400) / 3_600; + let minutes = (total_secs % 3_600) / 60; + + let mut parts: Vec = Vec::new(); + if days > 0 { + let unit = if days == 1 { "day" } else { "days" }; + parts.push(format!("{} {}", days, unit)); + } + if hours > 0 { + let unit = if hours == 1 { "hour" } else { "hours" }; + parts.push(format!("{} {}", hours, unit)); + } + if minutes > 0 { + let unit = if minutes == 1 { "minute" } else { "minutes" }; + parts.push(format!("{} {}", minutes, unit)); + } + + if parts.is_empty() { + return "less than a minute".to_string(); + } + + match parts.len() { + 1 => parts[0].clone(), + 2 => format!("{} {}", parts[0], parts[1]), + _ => format!("{} {} {}", parts[0], parts[1], parts[2]), + } +} + #[derive(Debug)] pub struct EnvVarError { /// Name of the environment variable that is missing. @@ -181,6 +224,8 @@ impl CodexErr { pub fn get_error_message_ui(e: &CodexErr) -> String { match e { CodexErr::Sandbox(SandboxErr::Denied(_, _, stderr)) => stderr.to_string(), + // Timeouts are not sandbox errors from a UX perspective; present them plainly + CodexErr::Sandbox(SandboxErr::Timeout) => "error: command timed out".to_string(), _ => e.to_string(), } } @@ -193,19 +238,23 @@ mod tests { fn usage_limit_reached_error_formats_plus_plan() { let err = UsageLimitReachedError { plan_type: Some("plus".to_string()), + resets_in_seconds: None, }; assert_eq!( err.to_string(), - "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing), or wait for limits to reset (every 5h and every week.)." + "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing) or try again later." ); } #[test] fn usage_limit_reached_error_formats_default_when_none() { - let err = UsageLimitReachedError { plan_type: None }; + let err = UsageLimitReachedError { + plan_type: None, + resets_in_seconds: None, + }; assert_eq!( err.to_string(), - "You've hit your usage limit. Limits reset every 5h and every week." + "You've hit your usage limit. Try again later." ); } @@ -213,10 +262,59 @@ mod tests { fn usage_limit_reached_error_formats_default_for_other_plans() { let err = UsageLimitReachedError { plan_type: Some("pro".to_string()), + resets_in_seconds: None, }; assert_eq!( err.to_string(), - "You've hit your usage limit. Limits reset every 5h and every week." + "You've hit your usage limit. Try again later." + ); + } + + #[test] + fn usage_limit_reached_includes_minutes_when_available() { + let err = UsageLimitReachedError { + plan_type: None, + resets_in_seconds: Some(5 * 60), + }; + assert_eq!( + err.to_string(), + "You've hit your usage limit. Try again in 5 minutes." + ); + } + + #[test] + fn usage_limit_reached_includes_hours_and_minutes() { + let err = UsageLimitReachedError { + plan_type: Some("plus".to_string()), + resets_in_seconds: Some(3 * 3600 + 32 * 60), + }; + assert_eq!( + err.to_string(), + "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing) or try again in 3 hours 32 minutes." + ); + } + + #[test] + fn usage_limit_reached_includes_days_hours_minutes() { + let err = UsageLimitReachedError { + plan_type: None, + resets_in_seconds: Some(2 * 86_400 + 3 * 3600 + 5 * 60), + }; + assert_eq!( + err.to_string(), + "You've hit your usage limit. Try again in 2 days 3 hours 5 minutes." + ); + } + + #[test] + fn usage_limit_reached_less_than_minute() { + let err = UsageLimitReachedError { + plan_type: None, + resets_in_seconds: Some(30), + }; + assert_eq!( + err.to_string(), + "You've hit your usage limit. Try again in less than a minute." ); } } diff --git a/codex-rs/core/src/model_provider_info.rs b/codex-rs/core/src/model_provider_info.rs index 0102591d2c..1a3e5a2767 100644 --- a/codex-rs/core/src/model_provider_info.rs +++ b/codex-rs/core/src/model_provider_info.rs @@ -17,6 +17,10 @@ use crate::error::EnvVarError; const DEFAULT_STREAM_IDLE_TIMEOUT_MS: u64 = 300_000; const DEFAULT_STREAM_MAX_RETRIES: u64 = 5; const DEFAULT_REQUEST_MAX_RETRIES: u64 = 4; +/// Hard cap for user-configured `stream_max_retries`. +const MAX_STREAM_MAX_RETRIES: u64 = 100; +/// Hard cap for user-configured `request_max_retries`. +const MAX_REQUEST_MAX_RETRIES: u64 = 100; /// Wire protocol that the provider speaks. Most third-party services only /// implement the classic OpenAI Chat Completions JSON schema, whereas OpenAI @@ -207,12 +211,14 @@ impl ModelProviderInfo { pub fn request_max_retries(&self) -> u64 { self.request_max_retries .unwrap_or(DEFAULT_REQUEST_MAX_RETRIES) + .min(MAX_REQUEST_MAX_RETRIES) } /// Effective maximum number of stream reconnection attempts for this provider. pub fn stream_max_retries(&self) -> u64 { self.stream_max_retries .unwrap_or(DEFAULT_STREAM_MAX_RETRIES) + .min(MAX_STREAM_MAX_RETRIES) } /// Effective idle timeout for streaming responses. diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 9f33c25f99..d32b741c27 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -276,22 +276,6 @@ impl App { async fn handle_key_event(&mut self, tui: &mut tui::Tui, key_event: KeyEvent) { match key_event { - KeyEvent { - code: KeyCode::Char('c'), - modifiers: crossterm::event::KeyModifiers::CONTROL, - kind: KeyEventKind::Press, - .. - } => { - self.chat_widget.on_ctrl_c(); - } - KeyEvent { - code: KeyCode::Char('d'), - modifiers: crossterm::event::KeyModifiers::CONTROL, - kind: KeyEventKind::Press, - .. - } if self.chat_widget.composer_is_empty() => { - self.app_event_tx.send(AppEvent::ExitRequest); - } KeyEvent { code: KeyCode::Char('t'), modifiers: crossterm::event::KeyModifiers::CONTROL, diff --git a/codex-rs/tui/src/app_backtrack.rs b/codex-rs/tui/src/app_backtrack.rs index 6cfbe164b5..fcab4a83cc 100644 --- a/codex-rs/tui/src/app_backtrack.rs +++ b/codex-rs/tui/src/app_backtrack.rs @@ -207,9 +207,9 @@ impl App { overlay.handle_event(tui, event)?; if overlay.is_done { self.close_transcript_overlay(tui); + tui.frame_requester().schedule_frame(); } } - tui.frame_requester().schedule_frame(); Ok(()) } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index dc22dd939d..49ce112f47 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -1,6 +1,7 @@ use codex_core::protocol::TokenUsage; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; +use crossterm::event::KeyEventKind; use crossterm::event::KeyModifiers; use ratatui::buffer::Buffer; use ratatui::layout::Constraint; @@ -686,6 +687,15 @@ impl ChatComposer { /// Handle key event when no popup is visible. fn handle_key_event_without_popup(&mut self, key_event: KeyEvent) -> (InputResult, bool) { match key_event { + KeyEvent { + code: KeyCode::Char('d'), + modifiers: crossterm::event::KeyModifiers::CONTROL, + kind: KeyEventKind::Press, + .. + } if self.is_empty() => { + self.app_event_tx.send(AppEvent::ExitRequest); + (InputResult::None, true) + } // ------------------------------------------------------------- // History navigation (Up / Down) – only when the composer is not // empty or when the cursor is at the correct position, to avoid diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index cad1c59762..a8aef9d499 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -73,7 +73,7 @@ pub(crate) struct BottomPaneParams { } impl BottomPane { - const BOTTOM_PAD_LINES: u16 = 2; + const BOTTOM_PAD_LINES: u16 = 1; pub fn new(params: BottomPaneParams) -> Self { let enhanced_keys_supported = params.enhanced_keys_supported; Self { @@ -655,28 +655,15 @@ mod tests { "expected Working header on top row: {top:?}" ); - // Next row (spacer) is blank, and bottom two rows are blank padding - let mut spacer = String::new(); + // Last row should be blank padding; the row above should generally contain composer content. let mut r_last = String::new(); - let mut r_last2 = String::new(); for x in 0..area.width { - // Spacer row immediately below the status header lives at y=2. - spacer.push(buf[(x, 2)].symbol().chars().next().unwrap_or(' ')); r_last.push(buf[(x, height - 1)].symbol().chars().next().unwrap_or(' ')); - r_last2.push(buf[(x, height - 2)].symbol().chars().next().unwrap_or(' ')); } - assert!( - spacer.trim().is_empty(), - "expected spacer line blank: {spacer:?}" - ); assert!( r_last.trim().is_empty(), "expected last row blank: {r_last:?}" ); - assert!( - r_last2.trim().is_empty(), - "expected second-to-last row blank: {r_last2:?}" - ); } #[test] diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 3d7ee28faf..a82b8b5cba 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -247,6 +247,9 @@ impl ChatWidget { } fn on_error(&mut self, message: String) { + // Before emitting the error message, finalize the active exec as failed + // so spinners are replaced with a red ✗ marker. + self.finalize_active_exec_cell_as_failed(); self.add_to_history(history_cell::new_error_event(message)); self.bottom_pane.set_task_running(false); self.running_commands.clear(); @@ -537,17 +540,7 @@ impl ChatWidget { ev.result, )); } - fn interrupt_running_task(&mut self) { - if self.bottom_pane.is_task_running() { - self.active_exec_cell = None; - self.running_commands.clear(); - self.bottom_pane.clear_ctrl_c_quit_hint(); - self.submit_op(Op::Interrupt); - self.bottom_pane.set_task_running(false); - self.stream.clear_all(); - self.request_redraw(); - } - } + fn layout_areas(&self, area: Rect) -> [Rect; 2] { Layout::vertical([ Constraint::Max( @@ -660,48 +653,57 @@ impl ChatWidget { } pub(crate) fn handle_key_event(&mut self, key_event: KeyEvent) { - if key_event.kind == KeyEventKind::Press { - self.bottom_pane.clear_ctrl_c_quit_hint(); + match key_event { + KeyEvent { + code: KeyCode::Char('c'), + modifiers: crossterm::event::KeyModifiers::CONTROL, + kind: KeyEventKind::Press, + .. + } => { + self.on_ctrl_c(); + return; + } + other if other.kind == KeyEventKind::Press => { + self.bottom_pane.clear_ctrl_c_quit_hint(); + } + _ => {} } - // Alt+Up: Edit the most recent queued user message (if any). - if matches!( - key_event, + match key_event { KeyEvent { code: KeyCode::Up, modifiers: KeyModifiers::ALT, kind: KeyEventKind::Press, .. - } - ) && !self.queued_user_messages.is_empty() - { - // Prefer the most recently queued item. - if let Some(user_message) = self.queued_user_messages.pop_back() { - self.bottom_pane.set_composer_text(user_message.text); - self.refresh_queued_user_messages(); - self.request_redraw(); - } - return; - } - - match self.bottom_pane.handle_key_event(key_event) { - InputResult::Submitted(text) => { - // If a task is running, queue the user input to be sent after the turn completes. - let user_message = UserMessage { - text, - image_paths: self.bottom_pane.take_recent_submission_images(), - }; - if self.bottom_pane.is_task_running() { - self.queued_user_messages.push_back(user_message); + } if !self.queued_user_messages.is_empty() => { + // Prefer the most recently queued item. + if let Some(user_message) = self.queued_user_messages.pop_back() { + self.bottom_pane.set_composer_text(user_message.text); self.refresh_queued_user_messages(); - } else { - self.submit_user_message(user_message); + self.request_redraw(); } } - InputResult::Command(cmd) => { - self.dispatch_command(cmd); + _ => { + match self.bottom_pane.handle_key_event(key_event) { + InputResult::Submitted(text) => { + // If a task is running, queue the user input to be sent after the turn completes. + let user_message = UserMessage { + text, + image_paths: self.bottom_pane.take_recent_submission_images(), + }; + if self.bottom_pane.is_task_running() { + self.queued_user_messages.push_back(user_message); + self.refresh_queued_user_messages(); + } else { + self.submit_user_message(user_message); + } + } + InputResult::Command(cmd) => { + self.dispatch_command(cmd); + } + InputResult::None => {} + } } - InputResult::None => {} } } @@ -952,6 +954,16 @@ impl ChatWidget { self.frame_requester.schedule_frame(); } + /// Mark the active exec cell as failed (✗) and flush it into history. + fn finalize_active_exec_cell_as_failed(&mut self) { + if let Some(cell) = self.active_exec_cell.take() { + let cell = cell.into_failed(); + // Insert finalized exec into history and keep grouping consistent. + self.add_to_history(cell); + self.last_history_was_exec = true; + } + } + // If idle and there are queued inputs, submit exactly one to start the next turn. fn maybe_send_next_queued_input(&mut self) { if self.bottom_pane.is_task_running() { @@ -1106,22 +1118,15 @@ impl ChatWidget { } /// Handle Ctrl-C key press. - /// Returns CancellationEvent::Handled if the event was consumed by the UI, or - /// CancellationEvent::Ignored if the caller should handle it (e.g. exit). - pub(crate) fn on_ctrl_c(&mut self) -> CancellationEvent { - match self.bottom_pane.on_ctrl_c() { - CancellationEvent::Handled => return CancellationEvent::Handled, - CancellationEvent::Ignored => {} - } - if self.bottom_pane.is_task_running() { - self.interrupt_running_task(); - CancellationEvent::Ignored - } else if self.bottom_pane.ctrl_c_quit_hint_visible() { - self.submit_op(Op::Shutdown); - CancellationEvent::Handled - } else { - self.bottom_pane.show_ctrl_c_quit_hint(); - CancellationEvent::Ignored + fn on_ctrl_c(&mut self) { + if self.bottom_pane.on_ctrl_c() == CancellationEvent::Ignored { + if self.bottom_pane.is_task_running() { + self.submit_op(Op::Interrupt); + } else if self.bottom_pane.ctrl_c_quit_hint_visible() { + self.submit_op(Op::Shutdown); + } else { + self.bottom_pane.show_ctrl_c_quit_hint(); + } } } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap index 9a205be1d2..9173b72623 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap @@ -1,5 +1,6 @@ --- source: tui/src/chatwidget/tests.rs +assertion_line: 728 expression: terminal.backend() --- "? Codex wants to run echo hello world " @@ -10,4 +11,3 @@ expression: terminal.backend() "▌ Yes Always No No, provide feedback " "▌ Approve and run the command " " " -" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap index 2e79127faa..e449f0c576 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap @@ -1,6 +1,6 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 690 +assertion_line: 763 expression: terminal.backend() --- "The model wants to apply changes " @@ -11,4 +11,3 @@ expression: terminal.backend() "▌ Yes No No, provide feedback " "▌ Approve and apply the changes " " " -" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap index 48e6154eb2..bf8bd08ba1 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap @@ -1,5 +1,6 @@ --- source: tui/src/chatwidget/tests.rs +assertion_line: 779 expression: terminal.backend() --- "▌ Ask Codex to do anything " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap index 7ab4159ade..1104773679 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap @@ -1,7 +1,8 @@ --- source: tui/src/chatwidget/tests.rs +assertion_line: 779 expression: terminal.backend() --- +" " "▌ Ask Codex to do anything " " " -" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap index 48e6154eb2..7f84982aad 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap @@ -1,5 +1,6 @@ --- source: tui/src/chatwidget/tests.rs +assertion_line: 807 expression: terminal.backend() --- "▌ Ask Codex to do anything " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap index c77477a083..dbe9cbed21 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap @@ -1,5 +1,6 @@ --- source: tui/src/chatwidget/tests.rs +assertion_line: 807 expression: terminal.backend() --- " " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_exec_marks_failed.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_exec_marks_failed.snap new file mode 100644 index 0000000000..89ce848b73 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_exec_marks_failed.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: exec_blob +--- +>_ + ✗ ⌨  sleep 1 diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap index 79b55e2657..c598dbe19f 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap @@ -1,6 +1,6 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 806 +assertion_line: 878 expression: terminal.backend() --- " " @@ -9,4 +9,3 @@ expression: terminal.backend() "▌ Ask Codex to do anything " " ⏎ send Ctrl+J newline Ctrl+T transcript Ctrl+C quit " " " -" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap index 5cd475248e..3abc8c7bcc 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap @@ -1,5 +1,6 @@ --- source: tui/src/chatwidget/tests.rs +assertion_line: 851 expression: terminal.backend() --- "? Codex wants to run echo 'hello world' " @@ -10,4 +11,3 @@ expression: terminal.backend() "▌ Yes Always No No, provide feedback " "▌ Approve and run the command " " " -" " diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 519726d0fa..0ee87b0794 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -415,6 +415,48 @@ fn exec_history_cell_shows_working_then_failed() { ); } +// Snapshot test: interrupting a running exec finalizes the active cell with a red ✗ +// marker (replacing the spinner) and flushes it into history. +#[test] +fn interrupt_exec_marks_failed_snapshot() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + // Begin a long-running command so we have an active exec cell with a spinner. + chat.handle_codex_event(Event { + id: "call-int".into(), + msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { + call_id: "call-int".into(), + command: vec!["bash".into(), "-lc".into(), "sleep 1".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + parsed_cmd: vec![ + codex_core::parse_command::ParsedCommand::Unknown { + cmd: "sleep 1".into(), + } + .into(), + ], + }), + }); + + // Simulate the task being aborted (as if ESC was pressed), which should + // cause the active exec cell to be finalized as failed and flushed. + chat.handle_codex_event(Event { + id: "call-int".into(), + msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + reason: TurnAbortReason::Interrupted, + }), + }); + + let cells = drain_insert_history(&mut rx); + assert!( + !cells.is_empty(), + "expected finalized exec cell to be inserted into history" + ); + + // The first inserted cell should be the finalized exec; snapshot its text. + let exec_blob = lines_to_single_string(&cells[0]); + assert_snapshot!("interrupt_exec_marks_failed", exec_blob); +} + #[test] fn exec_history_extends_previous_when_consecutive() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); @@ -1210,7 +1252,7 @@ fn stream_error_is_rendered_to_history() { let cells = drain_insert_history(&mut rx); assert!(!cells.is_empty(), "expected a history cell for StreamError"); let blob = lines_to_single_string(cells.last().unwrap()); - assert!(blob.contains("⚠ ")); + assert!(blob.contains("⚠  ")); assert!(blob.contains("stream error:")); assert!(blob.contains("idle timeout waiting for SSE")); } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 32ecb2b71f..079c853bcf 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -175,6 +175,26 @@ impl WidgetRef for &ExecCell { } } +impl ExecCell { + /// Convert an active exec cell into a failed, completed exec cell. + /// Replaces the spinner with a red ✗ and sets a zero/elapsed duration. + pub(crate) fn into_failed(mut self) -> ExecCell { + let elapsed = self + .start_time + .map(|st| st.elapsed()) + .unwrap_or_else(|| Duration::from_millis(0)); + self.start_time = None; + self.duration = Some(elapsed); + self.output = Some(CommandOutput { + exit_code: 1, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }); + self + } +} + #[derive(Debug)] struct CompletedMcpToolCallWithImageOutput { _image: DynamicImage, @@ -210,6 +230,20 @@ fn pretty_provider_name(id: &str) -> String { title_case(id) } } +/// Return the emoji followed by a hair space (U+200A) and a normal space. +/// This creates a reasonable gap across different terminals, +/// in particular Terminal.app and iTerm, which render too tightly with just a single normal space. +/// +/// Improvements here could be to condition this behavior on terminal, +/// or possibly on emoji. +fn padded_emoji(emoji: &str) -> String { + format!("{emoji}\u{200A} ") +} + +/// Convenience function over `padded_emoji()`. +fn padded_emoji_with(emoji: &str, text: impl AsRef) -> String { + format!("{}{}", padded_emoji(emoji), text.as_ref()) +} pub(crate) fn new_session_info( config: &Config, @@ -348,22 +382,22 @@ fn new_parsed_command( for parsed in parsed_commands.iter() { let text = match parsed { - ParsedCommand::Read { name, .. } => format!("📖 {name}"), + ParsedCommand::Read { name, .. } => padded_emoji_with("📖", name), ParsedCommand::ListFiles { cmd, path } => match path { - Some(p) => format!("📂 {p}"), - None => format!("📂 {cmd}"), + Some(p) => padded_emoji_with("📂", p), + None => padded_emoji_with("📂", cmd), }, ParsedCommand::Search { query, path, cmd } => match (query, path) { - (Some(q), Some(p)) => format!("🔎 {q} in {p}"), - (Some(q), None) => format!("🔎 {q}"), - (None, Some(p)) => format!("🔎 {p}"), - (None, None) => format!("🔎 {cmd}"), + (Some(q), Some(p)) => padded_emoji_with("🔎", format!("{q} in {p}")), + (Some(q), None) => padded_emoji_with("🔎", q), + (None, Some(p)) => padded_emoji_with("🔎", p), + (None, None) => padded_emoji_with("🔎", cmd), }, - ParsedCommand::Format { .. } => "✨ Formatting".to_string(), - ParsedCommand::Test { cmd } => format!("🧪 {cmd}"), - ParsedCommand::Lint { cmd, .. } => format!("🧹 {cmd}"), - ParsedCommand::Unknown { cmd } => format!("⌨️ {cmd}"), - ParsedCommand::Noop { cmd } => format!("🔄 {cmd}"), + ParsedCommand::Format { .. } => padded_emoji_with("✨", "Formatting"), + ParsedCommand::Test { cmd } => padded_emoji_with("🧪", cmd), + ParsedCommand::Lint { cmd, .. } => padded_emoji_with("🧹", cmd), + ParsedCommand::Unknown { cmd } => padded_emoji_with("⌨", cmd), + ParsedCommand::Noop { cmd } => padded_emoji_with("🔄", cmd), }; // Prefix: two spaces, marker, space. Continuations align under the text block. for (j, line_text) in text.lines().enumerate() { @@ -449,8 +483,10 @@ pub(crate) fn new_active_mcp_tool_call(invocation: McpInvocation) -> PlainHistor } pub(crate) fn new_web_search_call(query: String) -> PlainHistoryCell { - let lines: Vec> = - vec![Line::from(""), Line::from(vec!["🌐 ".into(), query.into()])]; + let lines: Vec> = vec![ + Line::from(""), + Line::from(vec![padded_emoji("🌐").into(), query.into()]), + ]; PlainHistoryCell { lines } } @@ -594,7 +630,10 @@ pub(crate) fn new_status_output( }; // 📂 Workspace - lines.push(Line::from(vec!["📂 ".into(), "Workspace".bold()])); + lines.push(Line::from(vec![ + padded_emoji("📂").into(), + "Workspace".bold(), + ])); // Path (home-relative, e.g., ~/code/project) let cwd_str = match relativize_to_home(&config.cwd) { Some(rel) if !rel.as_os_str().is_empty() => { @@ -675,7 +714,10 @@ pub(crate) fn new_status_output( if let Ok(auth) = try_read_auth_json(&auth_file) && let Some(tokens) = auth.tokens.clone() { - lines.push(Line::from(vec!["👤 ".into(), "Account".bold()])); + lines.push(Line::from(vec![ + padded_emoji("👤").into(), + "Account".bold(), + ])); lines.push(Line::from(" • Signed in with ChatGPT")); let info = tokens.id_token; @@ -702,7 +744,7 @@ pub(crate) fn new_status_output( } // 🧠 Model - lines.push(Line::from(vec!["🧠 ".into(), "Model".bold()])); + lines.push(Line::from(vec![padded_emoji("🧠").into(), "Model".bold()])); lines.push(Line::from(vec![ " • Name: ".into(), config.model.clone().into(), @@ -853,13 +895,21 @@ pub(crate) fn new_mcp_tools_output( } pub(crate) fn new_error_event(message: String) -> PlainHistoryCell { - let lines: Vec> = vec!["".into(), vec!["🖐 ".red().bold(), message.into()].into()]; + // Use a hair space (U+200A) to create a subtle, near-invisible separation + // before the text. VS16 is intentionally omitted to keep spacing tighter + // in terminals like Ghostty. + let lines: Vec> = vec![ + "".into(), + vec![padded_emoji("🖐").red().bold(), message.into()].into(), + ]; PlainHistoryCell { lines } } pub(crate) fn new_stream_error_event(message: String) -> PlainHistoryCell { - let lines: Vec> = - vec![vec!["⚠ ".magenta().bold(), message.dim()].into(), "".into()]; + let lines: Vec> = vec![ + vec![padded_emoji("⚠").magenta().bold(), message.dim()].into(), + "".into(), + ]; PlainHistoryCell { lines } } diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index d7fe24db0e..a41b7c3528 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -123,10 +123,10 @@ impl WidgetRef for StatusIndicatorWidget { for (i, piece) in wrapped.iter().take(3).enumerate() { let prefix = if i == 0 { " ↳ " } else { " " }; let content = format!("{prefix}{piece}"); - lines.push(Line::from(content.dim())); + lines.push(Line::from(content.dim().italic())); } if wrapped.len() > 3 { - lines.push(Line::from(" …".dim())); + lines.push(Line::from(" …".dim().italic())); } } if !self.queued_messages.is_empty() { diff --git a/codex-rs/tui/tests/fixtures/binary-size-log.jsonl b/codex-rs/tui/tests/fixtures/binary-size-log.jsonl index 90144f300f..3c537d91c0 100644 --- a/codex-rs/tui/tests/fixtures/binary-size-log.jsonl +++ b/codex-rs/tui/tests/fixtures/binary-size-log.jsonl @@ -3698,7 +3698,7 @@ {"ts":"2025-08-09T15:53:04.318Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:53:04.320Z","dir":"to_tui","kind":"app_event","variant":"Redraw"} {"ts":"2025-08-09T15:53:04.353Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_command_output_delta","call_id":"call_KOxVodT3X5ci7LJmudvcovhW","stream":"stderr","chunk":[32,32,32,32,70,105,110,105,115,104,101,100,32,96,100,101,118,96,32,112,114,111,102,105,108,101,32,91,117,110,111,112,116,105,109,105,122,101,100,32,43,32,100,101,98,117,103,105,110,102,111,93,32,116,97,114,103,101,116,40,115,41,32,105,110,32,53,56,46,50,49,115,10]}}} -{"ts":"2025-08-09T15:53:04.389Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_command_end","call_id":"call_KOxVodT3X5ci7LJmudvcovhW","stdout":"","stderr":"sandbox error: command timed out","exit_code":-1,"duration":{"secs":0,"nanos":0}}}} +{"ts":"2025-08-09T15:53:04.389Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_command_end","call_id":"call_KOxVodT3X5ci7LJmudvcovhW","stdout":"","stderr":"error: command timed out","exit_code":-1,"duration":{"secs":0,"nanos":0}}}} {"ts":"2025-08-09T15:53:04.389Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"token_count","input_tokens":14859,"cached_input_tokens":14625,"output_tokens":117,"reasoning_output_tokens":64,"total_tokens":14976}}} {"ts":"2025-08-09T15:53:04.389Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"turn_diff","unified_diff":"diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs\nindex a0bb4e69e27ae82c5f70d2f4cd079c5cea3ae4f7..18bae310be9cfb81ca73e136be05148ba0510cc5\n--- a/codex-rs/core/tests/common/lib.rs\n+++ b/codex-rs/core/tests/common/lib.rs\n@@ -90,14 +90,11 @@\n where\n F: FnMut(&codex_core::protocol::EventMsg) -> bool,\n {\n+ use tokio::time::Duration;\n use tokio::time::timeout;\n loop {\n-<<<<<<< HEAD\n // Allow a bit more time to accommodate async startup work (e.g. config IO, tool discovery)\n- let ev = timeout(Duration::from_secs(5), codex.next_event())\n-=======\n- let ev = timeout(wait_time, codex.next_event())\n->>>>>>> origin/main\n+ let ev = timeout(wait_time.max(Duration::from_secs(5)), codex.next_event())\n .await\n .expect(\"timeout waiting for event\")\n .expect(\"stream ended unexpectedly\");\n"}}} {"ts":"2025-08-09T15:53:04.389Z","dir":"to_tui","kind":"insert_history","lines":3}