From 32c303c197e437cb13d444389d651fbaae02a6ed Mon Sep 17 00:00:00 2001 From: Ian MacLeod Date: Thu, 3 Sep 2026 20:16:14 +0000 Subject: [PATCH] Condense TUI startup warnings (#42609) ## What changed - Coalesce configuration, skill, sandbox, and MCP startup diagnostics into a single summary below the session header, including MCP and sign-in counts. - Keep the complete warning text in the transcript and update the summary's transcript shortcut when key bindings change. - Continue rendering warnings that arrive after work begins as inline history entries. - Hide the MCP startup status row before a turn while preserving the configured interrupt shortcut. ## Testing - Add coverage for warning ordering and deduplication, transcript details, MCP sign-in counts, resumed sessions, runtime warnings, reflow, backtracking, and interruption without a visible status row. GitOrigin-RevId: 009657cf04f1d3a52270cf096eebf9db7513c040 --- codex-rs/tui/src/app.rs | 1 + codex-rs/tui/src/app/event_dispatch.rs | 9 +- codex-rs/tui/src/app/history_ui.rs | 11 + codex-rs/tui/src/app/session_lifecycle.rs | 12 +- codex-rs/tui/src/app/startup.rs | 6 +- codex-rs/tui/src/app/startup_prompts.rs | 44 ++-- codex-rs/tui/src/app/startup_warnings.rs | 68 ++++++ codex-rs/tui/src/app/tests.rs | 20 +- codex-rs/tui/src/app/tests/mcp_startup.rs | 4 +- ...p_warnings_tests__mcp_startup_details.snap | 9 + ...p_warnings_tests__mcp_startup_summary.snap | 7 + ...warnings_tests__mixed_startup_summary.snap | 7 + codex-rs/tui/src/app/tests/startup.rs | 17 +- .../src/app/tests/startup_warnings_tests.rs | 211 ++++++++++++++++++ codex-rs/tui/src/app/thread_routing.rs | 14 +- codex-rs/tui/src/app/working_directory.rs | 5 +- codex-rs/tui/src/bottom_pane/mod.rs | 30 ++- codex-rs/tui/src/chatwidget.rs | 3 + codex-rs/tui/src/chatwidget/mcp_startup.rs | 79 +++++-- codex-rs/tui/src/chatwidget/protocol.rs | 15 +- codex-rs/tui/src/chatwidget/replay.rs | 4 + ...et__tests__mcp_startup_header_booting.snap | 2 - .../tui/src/chatwidget/tests/app_server.rs | 25 ++- codex-rs/tui/src/chatwidget/tests/helpers.rs | 15 +- .../tui/src/chatwidget/tests/mcp_startup.rs | 63 +++++- ...s__app_server__runtime_config_warning.snap | 5 + ...sts__mcp_startup__runtime_mcp_warning.snap | 7 + codex-rs/tui/src/chatwidget/turn_runtime.rs | 7 + codex-rs/tui/src/chatwidget/warnings.rs | 2 + codex-rs/tui/src/history_cell/mod.rs | 2 + .../tui/src/history_cell/startup_warnings.rs | 93 ++++++++ 31 files changed, 728 insertions(+), 69 deletions(-) create mode 100644 codex-rs/tui/src/app/startup_warnings.rs create mode 100644 codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mcp_startup_details.snap create mode 100644 codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mcp_startup_summary.snap create mode 100644 codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mixed_startup_summary.snap create mode 100644 codex-rs/tui/src/app/tests/startup_warnings_tests.rs create mode 100644 codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__app_server__runtime_config_warning.snap create mode 100644 codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__mcp_startup__runtime_mcp_warning.snap create mode 100644 codex-rs/tui/src/history_cell/startup_warnings.rs diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 7b33c53c31..1959d1f6a5 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -242,6 +242,7 @@ mod session_picker; mod side; mod startup; mod startup_prompts; +mod startup_warnings; mod thread_event_buffer; mod thread_events; mod thread_goal_actions; diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index f0fd46fc2b..7b3aa66873 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -70,7 +70,11 @@ impl App { } AppEvent::CloseMisalignmentReview => self.chat_widget.show_misalignment_policy_precaution(), AppEvent::SkillsListLoaded { ref cwd, .. } - | AppEvent::PluginMentionsLoaded { ref cwd, .. } + if cwds_differ(cwd, self.config.cwd.as_path()) => + { + self.skill_load_warnings.startup_complete = true; + } + AppEvent::PluginMentionsLoaded { ref cwd, .. } if cwds_differ(cwd, self.config.cwd.as_path()) => {} AppEvent::NewSession { name } => { self.start_fresh_session_with_summary_hint( @@ -1147,6 +1151,7 @@ impl App { result.map_err(|err| color_eyre::eyre::eyre!(err)), "failed to load skills on startup", ); + self.skill_load_warnings.startup_complete = true; } AppEvent::StartFileSearch(query) => { self.file_search.on_user_query(query.clone()); @@ -2973,9 +2978,11 @@ impl App { } => { self.apply_keymap_capture(context, action, key, intent) .await; + self.merge_startup_warnings(tui, &history_cell::StartupWarningsCell::default()); } AppEvent::KeymapCleared { context, action } => { self.apply_keymap_clear(context, action).await; + self.merge_startup_warnings(tui, &history_cell::StartupWarningsCell::default()); } AppEvent::GenerateRecap { thread_id } => { if self.current_displayed_thread_id() == Some(thread_id) { diff --git a/codex-rs/tui/src/app/history_ui.rs b/codex-rs/tui/src/app/history_ui.rs index 024288de54..9d63c68a1c 100644 --- a/codex-rs/tui/src/app/history_ui.rs +++ b/codex-rs/tui/src/app/history_ui.rs @@ -22,6 +22,14 @@ pub(super) struct ThreadUsageStatusHistory { impl App { pub(super) fn insert_history_cell(&mut self, tui: &mut tui::Tui, cell: Box) { + if let Some(warnings) = cell + .as_any() + .downcast_ref::() + { + self.merge_startup_warnings(tui, warnings); + return; + } + let is_session_header = cell.as_any().is::(); let cell: Arc = cell.into(); if let Some(Overlay::Transcript(t)) = &mut self.overlay { t.insert_cell(cell.clone()); @@ -62,6 +70,9 @@ impl App { // A committed cell can unblock a settled /usage card that was waiting // behind a transient active cell or a provisional stream tail. self.chat_widget.request_pending_usage_output_insertion(); + if is_session_header { + self.merge_startup_warnings(tui, &history_cell::StartupWarningsCell::default()); + } } pub(super) fn finish_thread_usage_refresh( diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index e783ca17c0..7513d368af 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -801,8 +801,18 @@ impl App { } Err(err) if self.recover_transport_error(&err) => {} Err(err) => { + let warnings = self + .transcript_cells + .iter() + .find_map(|cell| { + cell.as_any() + .downcast_ref::() + }) + .filter(|cell| cell.pending_header) + .map(|cell| format!("\n\nStartup warnings:\n{}", cell.messages.join("\n"))) + .unwrap_or_default(); return Err(color_eyre::eyre::eyre!( - "Failed to start a fresh session through the app server: {err}" + "Failed to start a fresh session through the app server: {err}{warnings}" )); } } diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index 1434b908fd..9a9913e344 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -127,7 +127,11 @@ impl App { let startup_started_at = Instant::now(); let (app_event_tx, mut app_event_rx) = unbounded_channel(); let app_event_tx = AppEventSender::new(app_event_tx); - emit_project_config_warnings(&app_event_tx, &config); + if let Some(message) = project_config_warning(&config) { + app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( + history_cell::StartupWarningsCell::new(vec![message]), + ))); + } emit_system_bwrap_warning(&app_event_tx, &config); tui.set_notification_settings( local_settings.tui.notification_settings.method, diff --git a/codex-rs/tui/src/app/startup_prompts.rs b/codex-rs/tui/src/app/startup_prompts.rs index 3014bebade..fad4c82bab 100644 --- a/codex-rs/tui/src/app/startup_prompts.rs +++ b/codex-rs/tui/src/app/startup_prompts.rs @@ -17,6 +17,8 @@ struct SkillLoadWarningKey { #[derive(Debug, Default)] pub(super) struct SkillLoadWarningState { active: HashSet, + // Both session attachment and the background startup fetch can finish the initial load. + pub(super) startup_complete: bool, } impl SkillLoadWarningState { @@ -45,28 +47,24 @@ impl SkillLoadWarningState { } } -pub(super) fn emit_skill_load_warnings(app_event_tx: &AppEventSender, errors: &[SkillErrorInfo]) { +pub(super) fn skill_load_warning_messages(errors: &[SkillErrorInfo]) -> Vec { if errors.is_empty() { - return; + return Vec::new(); } let error_count = errors.len(); - app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( - crate::history_cell::new_warning_event(format!( - "Skipped loading {error_count} skill(s) due to invalid SKILL.md files." - )), - ))); - - for error in errors { - let path = error.path.display(); - let message = error.message.as_str(); - app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( - crate::history_cell::new_warning_event(format!("{path}: {message}")), - ))); - } + let mut messages = vec![format!( + "Skipped loading {error_count} skill(s) due to invalid SKILL.md files." + )]; + messages.extend( + errors + .iter() + .map(|error| format!("{}: {}", error.path.display(), error.message)), + ); + messages } -pub(super) fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config) { +pub(super) fn project_config_warning(config: &Config) -> Option { let mut disabled_folders = Vec::new(); for layer in config.config_layer_stack.all_layers_low_to_high() { @@ -83,7 +81,7 @@ pub(super) fn emit_project_config_warnings(app_event_tx: &AppEventSender, config } if disabled_folders.is_empty() { - return; + return None; } let mut message = concat!( @@ -97,9 +95,7 @@ pub(super) fn emit_project_config_warnings(app_event_tx: &AppEventSender, config message.push_str(&format!(" {reason}\n")); } - app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( - history_cell::new_warning_event(message), - ))); + Some(message) } pub(super) fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: &Config) { @@ -110,7 +106,7 @@ pub(super) fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: & }; app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( - history_cell::new_warning_event(message), + history_cell::StartupWarningsCell::new(vec![message]), ))); } @@ -424,7 +420,11 @@ mod tests { let (tx, mut rx) = unbounded_channel(); let app_event_tx = AppEventSender::new(tx); - emit_skill_load_warnings(&app_event_tx, errors); + for message in skill_load_warning_messages(errors) { + app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( + history_cell::new_warning_event(message), + ))); + } let mut rendered = Vec::new(); while let Ok(AppEvent::InsertHistoryCell(cell)) = rx.try_recv() { diff --git a/codex-rs/tui/src/app/startup_warnings.rs b/codex-rs/tui/src/app/startup_warnings.rs new file mode 100644 index 0000000000..70b902b820 --- /dev/null +++ b/codex-rs/tui/src/app/startup_warnings.rs @@ -0,0 +1,68 @@ +//! Coalesces startup diagnostics directly below the session splash. + +use super::*; +use crate::history_cell::StartupWarningsCell; + +impl App { + pub(super) fn merge_startup_warnings( + &mut self, + tui: &mut tui::Tui, + incoming: &StartupWarningsCell, + ) { + let existing = self + .transcript_cells + .iter() + .position(|cell| cell.as_any().is::()); + let mut warnings = existing + .map(|index| self.transcript_cells.remove(index)) + .and_then(|cell| cell.as_any().downcast_ref::().cloned()) + .unwrap_or_default(); + for message in &incoming.messages { + if !warnings.messages.contains(message) { + warnings.messages.push(message.clone()); + } + } + warnings + .other_sources + .extend(incoming.other_sources.iter().cloned()); + warnings + .mcp_servers + .extend(incoming.mcp_servers.iter().cloned()); + // The final MCP summary must retain the individual diagnostics' sign-in subset. + warnings + .sign_in_servers + .extend(incoming.sign_in_servers.iter().cloned()); + if warnings.messages.is_empty() { + return; + } + warnings.transcript_hint = crate::keymap::primary_binding(&self.keymap.app.open_transcript) + .map(|binding| binding.display_label()); + let header = self + .transcript_cells + .iter() + .rposition(|cell| cell.as_any().is::()); + warnings.pending_header = header.is_none() && self.chat_widget.thread_id().is_none(); + let ready_to_render = !warnings.pending_header; + self.transcript_cells + .insert(header.map_or(0, |index| index + 1), Arc::new(warnings)); + if let Some(Overlay::Transcript(overlay)) = &mut self.overlay { + overlay.replace_cells(self.transcript_cells.clone()); + } + if self.backtrack.overlay_preview_active { + self.apply_backtrack_selection_internal(self.backtrack.nth_user_message); + } + if ready_to_render { + if let Some(buffer) = self.initial_history_replay_buffer.as_mut() { + buffer.retained_lines.clear(); + buffer.render_from_transcript_tail = true; + return; + } + self.schedule_immediate_resize_reflow(tui); + if let Err(error) = + self.maybe_run_resize_reflow(tui, tui.terminal.last_known_screen_size) + { + tracing::warn!(%error, "failed to refresh startup warnings"); + } + } + } +} diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 795bc65af1..37efe5f453 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -40,6 +40,8 @@ mod safety_buffering; mod session_lifecycle_requests; mod session_summary; mod startup; +#[path = "tests/startup_warnings_tests.rs"] +mod startup_warnings_tests; #[path = "tests/stream_animation_tests.rs"] mod stream_animation_tests; #[path = "tests/thread_usage.rs"] @@ -4847,7 +4849,9 @@ async fn primary_thread_ignores_child_mcp_startup_notifications() { let mut rendered_cells = Vec::new(); while let Ok(event) = app_event_rx.try_recv() { if let AppEvent::InsertHistoryCell(cell) = event { - rendered_cells.push(lines_to_single_string(&cell.display_lines(/*width*/ 120))); + rendered_cells.push(lines_to_single_string( + &cell.transcript_lines(/*width*/ 120), + )); } } let rendered = rendered_cells.join("\n"); @@ -4964,12 +4968,24 @@ async fn active_side_thread_renders_live_mcp_startup_notifications() { app.handle_thread_event_now(event); } + let mut tui = crate::tui::test_support::make_test_tui().expect("test tui"); let mut rendered_cells = Vec::new(); while let Ok(event) = app_event_rx.try_recv() { if let AppEvent::InsertHistoryCell(cell) = event { - rendered_cells.push(lines_to_single_string(&cell.display_lines(/*width*/ 120))); + rendered_cells.push(lines_to_single_string( + &cell.transcript_lines(/*width*/ 120), + )); + app.insert_history_cell(&mut tui, cell); } } + let inline = app + .transcript_cells + .iter() + .flat_map(|cell| cell.display_lines(/*width*/ 120)) + .map(|line| line.to_string()) + .collect::>() + .join("\n"); + assert!(inline.contains("1 MCP startup issue")); let rendered = rendered_cells.join("\n"); assert!(app.chat_widget.side_conversation_active()); assert_eq!(rendered.matches("sentry is not logged in").count(), 1); diff --git a/codex-rs/tui/src/app/tests/mcp_startup.rs b/codex-rs/tui/src/app/tests/mcp_startup.rs index 40ac1c6688..40d0922099 100644 --- a/codex-rs/tui/src/app/tests/mcp_startup.rs +++ b/codex-rs/tui/src/app/tests/mcp_startup.rs @@ -86,9 +86,9 @@ async fn subagent_mcp_startup_settles_while_cached_servers_remain_deferred() { } insta::assert_snapshot!(visible_startup_states.join("\n"), @r" - eager: • Booting MCP server: eager eager: idle - deferred: • Booting MCP server: deferred + eager: idle + deferred: idle deferred: idle "); } diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mcp_startup_details.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mcp_startup_details.snap new file mode 100644 index 0000000000..88ee611162 --- /dev/null +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mcp_startup_details.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/app/tests/startup_warnings_tests.rs +expression: lines_to_single_string(&warnings.transcript_lines(40)) +--- +⚠ MCP alpha: connection unavailable +⚠ MCP beta: connection unavailable +⚠ MCP gamma: connection unavailable +⚠ MCP startup incomplete (failed: alpha, + beta, gamma) diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mcp_startup_summary.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mcp_startup_summary.snap new file mode 100644 index 0000000000..6b5d082fa8 --- /dev/null +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mcp_startup_summary.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/app/tests/startup_warnings_tests.rs +expression: warnings.display_lines(100) +--- +[ + Line::from(Span::from("⚠ 3 MCP startup issues (2 need sign-in) · ctrl + t for details").yellow().dim()), +] diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mixed_startup_summary.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mixed_startup_summary.snap new file mode 100644 index 0000000000..9576e9ffae --- /dev/null +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__mixed_startup_summary.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/app/tests/startup_warnings_tests.rs +expression: "app.transcript_cells[1].display_lines(100)" +--- +[ + Line::from(Span::from("⚠ 4 startup issues (3 MCP; 2 need sign-in) · ctrl + t for details").yellow().dim()), +] diff --git a/codex-rs/tui/src/app/tests/startup.rs b/codex-rs/tui/src/app/tests/startup.rs index ac42050976..1c1880da91 100644 --- a/codex-rs/tui/src/app/tests/startup.rs +++ b/codex-rs/tui/src/app/tests/startup.rs @@ -1240,6 +1240,13 @@ async fn owned_subagent_approval_before_thread_started_is_preserved() -> Result< async fn startup_thread_start_failure_returns_error() { let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await; app.pending_startup_thread_start = true; + let mut tui = crate::tui::test_support::make_test_tui().expect("test tui"); + app.insert_history_cell( + &mut tui, + Box::new(history_cell::StartupWarningsCell::new(vec![ + "Skill manifest is invalid.".to_string(), + ])), + ); let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker( app.chat_widget.config_ref(), @@ -1251,10 +1258,12 @@ async fn startup_thread_start_failure_returns_error() { .await .expect_err("startup thread failure should exit instead of leaving chat unconfigured"); - assert!( - err.to_string() - .contains("Failed to start a fresh session through the app server: boom") - ); + insta::assert_snapshot!(err.to_string(), @" + Failed to start a fresh session through the app server: boom + + Startup warnings: + Skill manifest is invalid. + "); assert!(!app.pending_startup_thread_start); assert_eq!(app.primary_thread_id, None); } diff --git a/codex-rs/tui/src/app/tests/startup_warnings_tests.rs b/codex-rs/tui/src/app/tests/startup_warnings_tests.rs new file mode 100644 index 0000000000..ab2bcaa976 --- /dev/null +++ b/codex-rs/tui/src/app/tests/startup_warnings_tests.rs @@ -0,0 +1,211 @@ +//! Startup diagnostic ordering, aggregation, and transcript coverage. + +use super::*; +use crate::history_cell::StartupWarningsCell; +use codex_app_server_protocol::McpServerStartupFailureReason; +use pretty_assertions::assert_eq; +use tokio::sync::mpsc::UnboundedReceiver; + +fn insert_warnings(app: &mut App, tui: &mut tui::Tui, messages: &[&str]) { + app.insert_history_cell( + tui, + Box::new(StartupWarningsCell::new( + messages.iter().map(|message| (*message).into()).collect(), + )), + ); +} + +fn insert_splash(app: &mut App, tui: &mut tui::Tui) { + let session = test_thread_session(ThreadId::new(), app.config.cwd.to_path_buf()); + app.insert_history_cell( + tui, + Box::new(history_cell::new_session_info( + &app.config, + &app.local_settings, + "gpt-test", + &session, + /*is_first_event*/ false, + /*tooltip_override*/ None, + /*auth_plan*/ None, + /*show_fast_status*/ false, + )), + ); +} + +fn drain_history(app: &mut App, tui: &mut tui::Tui, events: &mut UnboundedReceiver) { + while let Ok(event) = events.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = event { + app.insert_history_cell(tui, cell); + } + } +} + +#[tokio::test] +async fn startup_warnings_preserve_stream_repair_and_backtrack_selection() -> Result<()> { + let mut app = crate::app::test_support::make_test_app().await; + let mut tui = crate::tui::test_support::make_test_tui()?; + app.chat_widget.handle_thread_session(test_thread_session( + ThreadId::new(), + app.config.cwd.to_path_buf(), + )); + app.transcript_cells = vec![Arc::new(UserHistoryCell { + message: "Selected prompt".into(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + })]; + app.transcript_reflow.mark_resize_requested_during_stream(); + insert_warnings(&mut app, &mut tui, &["First warning"]); + assert!(app.transcript_reflow.take_stream_finish_reflow_needed()); + // Recreate the insertion case with a backtracking overlay already open. + app.transcript_cells.remove(0); + app.backtrack.overlay_preview_active = true; + app.open_transcript_overlay(&mut tui); + app.apply_backtrack_selection_internal(/*nth_user_message*/ 0); + insert_warnings(&mut app, &mut tui, &["Second warning"]); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 12, + ); + let mut buffer = Buffer::empty(area); + let Some(Overlay::Transcript(overlay)) = app.overlay.as_mut() else { + panic!("expected transcript") + }; + overlay.render(area, &mut buffer); + let highlighted = area + .positions() + .filter(|position| { + buffer[*position] + .style() + .add_modifier + .contains(ratatui::style::Modifier::REVERSED) + }) + .map(|position| buffer[position].symbol()) + .collect::(); + assert!(highlighted.contains("Selected prompt"), "{highlighted}"); + Ok(()) +} + +#[tokio::test] +async fn mcp_startup_summary_counts_servers_and_sign_in_subset() -> Result<()> { + let (mut app, mut events, _op_rx) = make_test_app_with_channels().await; + let mut tui = crate::tui::test_support::make_test_tui()?; + app.chat_widget + .set_mcp_startup_expected_servers(["alpha", "beta", "gamma"].map(str::to_owned)); + insert_splash(&mut app, &mut tui); + let sign_in = Some(McpServerStartupFailureReason::ReauthenticationRequired); + for (name, failure_reason) in [("alpha", sign_in), ("beta", sign_in), ("gamma", None)] { + app.chat_widget.handle_server_notification( + ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification { + thread_id: None, + name: name.into(), + status: McpServerStartupState::Failed, + error: Some(format!("MCP {name}: connection unavailable")), + failure_reason, + }), + /*replay_kind*/ None, + ); + } + drain_history(&mut app, &mut tui, &mut events); + assert_eq!(app.transcript_cells.len(), 2); + let warnings = &app.transcript_cells[1]; + insta::assert_debug_snapshot!("mcp_startup_summary", warnings.display_lines(/*width*/ 100)); + insta::assert_snapshot!( + "mcp_startup_details", + lines_to_single_string(&warnings.transcript_lines(/*width*/ 40)) + ); + insert_warnings(&mut app, &mut tui, &["Skill manifest is invalid."]); + insta::assert_debug_snapshot!( + "mixed_startup_summary", + app.transcript_cells[1].display_lines(/*width*/ 100) + ); + Ok(()) +} + +#[tokio::test] +async fn startup_warnings_wait_for_splash_and_coalesce_with_full_details() -> Result<()> { + let mut app = crate::app::test_support::make_test_app().await; + let mut tui = crate::tui::test_support::make_test_tui()?; + let first = "Skill manifest is invalid."; + insert_warnings(&mut app, &mut tui, &[first]); + let pending = &app.transcript_cells[0]; + assert!(pending.display_lines(/*width*/ 80).is_empty()); + + insert_splash(&mut app, &mut tui); + insert_warnings(&mut app, &mut tui, &[first, "MCP alpha failed to start."]); + + assert_eq!(app.transcript_cells.len(), 2); + let header = app.transcript_cells[0].as_any(); + assert!(header.is::()); + let display = app.transcript_cells[1].display_lines(/*width*/ 80); + let transcript = app.transcript_cells[1].transcript_lines(/*width*/ 80); + insta::assert_snapshot!(format!("display:\n{}\n\ntranscript:\n{}", lines_to_single_string(&display), lines_to_single_string(&transcript)), @" + display: + ⚠ 2 startup issues · ctrl + t for details + + transcript: + ⚠ Skill manifest is invalid. + ⚠ MCP alpha failed to start. + "); + app.keymap.app.open_transcript = vec![crate::key_hint::plain(crossterm::event::KeyCode::F(12))]; + app.merge_startup_warnings(&mut tui, &StartupWarningsCell::default()); + insta::assert_snapshot!(lines_to_single_string(&app.transcript_cells[1].display_lines(/*width*/ 80)), @"⚠ 2 startup issues · f12 for details"); + app.keymap.app.open_transcript.clear(); + app.merge_startup_warnings(&mut tui, &StartupWarningsCell::default()); + insta::assert_snapshot!(lines_to_single_string(&app.transcript_cells[1].display_lines(/*width*/ 80)), @"⚠ 2 startup issues"); + Ok(()) +} + +#[tokio::test] +async fn startup_skill_load_order_preserves_runtime_error_recurrence() -> Result<()> { + for background_first in [false, true] { + let (mut app, mut events, _op_rx) = make_test_app_with_channels().await; + let mut tui = crate::tui::test_support::make_test_tui()?; + let mut server = start_config_write_test_app_server(&app).await?; + let cwd = app.config.cwd.to_path_buf(); + let response = SkillsListResponse { + data: vec![codex_app_server_protocol::SkillsListEntry { + cwd: cwd.clone(), + skills: vec![], + errors: vec![SkillErrorInfo { + path: PathBuf::from("bad/SKILL.md"), + message: "invalid description".into(), + }], + }], + }; + // Session attachment and the background startup request may finish in either order. + for background in [background_first, !background_first] { + if background { + app.handle_event( + &mut tui, + &mut server, + AppEvent::SkillsListLoaded { + cwd: cwd.clone(), + result: Ok(response.clone()), + }, + ) + .await?; + } else { + app.handle_skills_list_result(Ok(response.clone()), "initial session refresh"); + } + } + drain_history(&mut app, &mut tui, &mut events); + assert_eq!(app.transcript_cells.len(), 1); + assert!(app.transcript_cells[0].as_any().is::()); + app.handle_skills_list_result(Ok(SkillsListResponse { data: vec![] }), "clean refresh"); + app.handle_skills_list_result(Ok(response), "runtime refresh"); + drain_history(&mut app, &mut tui, &mut events); + assert_eq!(app.transcript_cells.len(), 3); + let runtime = app.transcript_cells[1..] + .iter() + .flat_map(|cell| cell.display_lines(/*width*/ 120)) + .collect::>(); + insta::allow_duplicates! { + insta::assert_snapshot!(lines_to_single_string(&runtime), @" + ⚠ Skipped loading 1 skill(s) due to invalid SKILL.md files. + ⚠ bad/SKILL.md: invalid description + "); + } + server.shutdown().await?; + } + Ok(()) +} diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index a40fe6b273..c2b47d43a5 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -1686,7 +1686,19 @@ impl App { let cwd = self.chat_widget.config_ref().cwd.clone(); let errors = errors_for_cwd(&cwd, &response); let errors = self.skill_load_warnings.newly_active_errors(&errors); - emit_skill_load_warnings(&self.app_event_tx, &errors); + let warnings = skill_load_warning_messages(&errors); + if self.skill_load_warnings.startup_complete { + for warning in warnings { + self.chat_widget.add_warning_message(warning); + } + } else { + self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( + // The per-file diagnostics already identify every affected skill. + history_cell::StartupWarningsCell::new( + warnings.into_iter().skip(/*n*/ 1).collect(), + ), + ))); + } self.chat_widget.handle_skills_list_response(response); } diff --git a/codex-rs/tui/src/app/working_directory.rs b/codex-rs/tui/src/app/working_directory.rs index 5e6d7a97d6..66e4eacf59 100644 --- a/codex-rs/tui/src/app/working_directory.rs +++ b/codex-rs/tui/src/app/working_directory.rs @@ -209,9 +209,12 @@ impl App { } self.cancel_pending_key_chord(); self.keymap = keymap; + self.merge_startup_warnings(tui, &history_cell::StartupWarningsCell::default()); self.restore_runtime_theme_from_config(); self.runtime_working_directory_override = Some(cwd.to_path_buf()); - emit_project_config_warnings(&self.app_event_tx, &self.config); + if let Some(message) = project_config_warning(&self.config) { + self.chat_widget.add_warning_message(message); + } let message = format!("Working directory changed to: {}", cwd.display()); self.chat_widget.add_info_message(message, /*hint*/ None); if !self.config.bypass_hook_trust { diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 5921a0c833..21d13cf1eb 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -738,14 +738,15 @@ impl BottomPane { if self.handle_inline_banner_key(key_event) { return InputResult::None; } - // If a task is running and a status line is visible, allow the - // configured action to interrupt even while the composer has focus. + // Allow the configured interrupt while a task is running, even when its + // status row is hidden or the composer has focus. // When a popup is active, prefer dismissing it over interrupting the task. - if self.should_interrupt_running_task(key_event) - && let Some(status) = &self.status - { - // Send Op::Interrupt - status.interrupt(); + if self.should_interrupt_running_task(key_event) { + if let Some(status) = &self.status { + status.interrupt(); + } else { + self.app_event_tx.interrupt(); + } self.request_redraw(); return InputResult::None; } @@ -1499,7 +1500,6 @@ impl BottomPane { && !(is_agent_command && key_event.code == KeyCode::Esc) && self.no_modal_or_popup_active() && !self.composer_should_handle_vim_insert_escape(key_event) - && self.status_widget().is_some() } pub(crate) fn terminal_title_requires_action(&self) -> bool { @@ -3213,6 +3213,13 @@ mod tests { matches!(rx.try_recv(), Ok(AppEvent::CodexOp(Op::Interrupt))), "expected Esc to send Op::Interrupt while a task is running" ); + + pane.hide_status_indicator(); + pane.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + assert!(matches!( + rx.try_recv(), + Ok(AppEvent::CodexOp(Op::Interrupt)) + )); } #[test] @@ -3237,6 +3244,13 @@ mod tests { matches!(rx.try_recv(), Ok(AppEvent::CodexOp(Op::Interrupt))), "expected configured key to interrupt while `/subagents` is being edited" ); + + pane.hide_status_indicator(); + pane.handle_key_event(KeyEvent::new(KeyCode::F(12), KeyModifiers::NONE)); + assert!(matches!( + rx.try_recv(), + Ok(AppEvent::CodexOp(Op::Interrupt)) + )); } #[test] diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index eba19306ee..f8e9d78cc1 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1306,6 +1306,9 @@ impl ChatWidget { self.bottom_pane.set_task_running(/*running*/ true); } self.review.is_review_mode = true; + if !from_replay { + self.bottom_pane.ensure_status_indicator(); + } let banner = format!(">> Code review started: {hint} <<"); self.add_to_history(history_cell::new_review_status_line(banner)); self.request_redraw(); diff --git a/codex-rs/tui/src/chatwidget/mcp_startup.rs b/codex-rs/tui/src/chatwidget/mcp_startup.rs index 343a38a687..ad3724acfd 100644 --- a/codex-rs/tui/src/chatwidget/mcp_startup.rs +++ b/codex-rs/tui/src/chatwidget/mcp_startup.rs @@ -3,9 +3,12 @@ //! The app server reports MCP server startup as per-server status updates. This //! module keeps the TUI's buffered startup round state coherent and translates //! those updates into status headers, warnings, and queued-input release points. +//! Initial diagnostics coalesce beneath the splash; warnings after a turn starts +//! stay inline so failures during ongoing work remain visible. use std::collections::BTreeSet; +use codex_app_server_protocol::McpServerStartupFailureReason; use codex_app_server_protocol::McpServerStartupState; use codex_app_server_protocol::McpServerStatusUpdatedNotification; @@ -14,11 +17,14 @@ use super::ChatWidget; const MCP_STARTUP_SINGLE_HEADER_PREFIX: &str = "Booting MCP server:"; const MCP_STARTUP_MULTI_HEADER_PREFIX: &str = "Starting MCP servers"; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum McpStartupStatus { Starting, Ready, - Failed { error: String }, + Failed { + error: String, + failure_reason: Option, + }, Cancelled, } @@ -82,23 +88,34 @@ impl ChatWidget { // Normal path: fold the update into the active round and surface // per-server failures immediately. let mut startup_status = self.mcp_startup_status.take().unwrap_or_default(); - if let McpStartupStatus::Failed { error } = &status { - let already_reported = matches!( - startup_status.get(&server), - Some(McpStartupStatus::Failed { error: previous }) if previous == error + if let McpStartupStatus::Failed { + error, + failure_reason, + } = &status + && startup_status.get(&server) != Some(&status) + { + self.add_mcp_startup_warning( + vec![error.clone()], + [server.clone()], + *failure_reason, ); - if !already_reported { - self.on_warning(error); - } } startup_status.insert(server, status); startup_status }; if activated_pending_round { // A promoted buffered round may already contain terminal failures. - for state in startup_status.values() { - if let McpStartupStatus::Failed { error } = state { - self.on_warning(error); + for (server, state) in &startup_status { + if let McpStartupStatus::Failed { + error, + failure_reason, + } = state + { + self.add_mcp_startup_warning( + vec![error.clone()], + [server.clone()], + *failure_reason, + ); } } } @@ -184,17 +201,25 @@ impl ChatWidget { pub(super) fn finish_mcp_startup(&mut self, failed: Vec, cancelled: Vec) { if !cancelled.is_empty() { - self.on_warning(format!( - "MCP startup interrupted. The following servers were not initialized: {}", - cancelled.join(", ") - )); + self.add_mcp_startup_warning( + vec![format!( + "MCP startup interrupted. The following servers were not initialized: {}", + cancelled.join(", ") + )], + cancelled, + /*failure_reason*/ None, + ); } let mut parts = Vec::new(); if !failed.is_empty() { parts.push(format!("failed: {}", failed.join(", "))); } if !parts.is_empty() { - self.on_warning(format!("MCP startup incomplete ({})", parts.join("; "))); + self.add_mcp_startup_warning( + vec![format!("MCP startup incomplete ({})", parts.join("; "))], + failed, + /*failure_reason*/ None, + ); } let mcp_startup_owned_status = self.status_header_is_mcp_startup_owned(); @@ -262,6 +287,25 @@ impl ChatWidget { .starts_with(MCP_STARTUP_MULTI_HEADER_PREFIX) } + fn add_mcp_startup_warning( + &mut self, + messages: Vec, + servers: impl IntoIterator, + failure_reason: Option, + ) { + if self.warning_display_state.startup_complete { + for message in messages { + self.on_warning(message); + } + } else { + self.add_to_history(crate::history_cell::StartupWarningsCell::mcp( + messages, + servers, + failure_reason, + )); + } + } + /// Update startup state and retry installed-app discovery when its MCP server becomes ready. pub(super) fn on_mcp_server_status_updated( &mut self, @@ -273,6 +317,7 @@ impl ChatWidget { McpServerStartupState::Starting => McpStartupStatus::Starting, McpServerStartupState::Ready => McpStartupStatus::Ready, McpServerStartupState::Failed => McpStartupStatus::Failed { + failure_reason: notification.failure_reason, error: notification.error.unwrap_or_else(|| { format!("MCP client for `{}` failed to start", notification.name) }), diff --git a/codex-rs/tui/src/chatwidget/protocol.rs b/codex-rs/tui/src/chatwidget/protocol.rs index 14e70f8ffa..fcd6149226 100644 --- a/codex-rs/tui/src/chatwidget/protocol.rs +++ b/codex-rs/tui/src/chatwidget/protocol.rs @@ -66,6 +66,7 @@ impl ChatWidget { self.turn_lifecycle.last_turn_id = Some(notification.turn.id); self.last_non_retry_error = None; if !matches!(replay_kind, Some(ReplayKind::ResumeInitialMessages)) { + self.warning_display_state.startup_complete = true; self.on_task_started(); } } @@ -194,12 +195,18 @@ impl ChatWidget { ServerNotification::DeprecationNotice(notification) => { self.on_deprecation_notice(notification.summary, notification.details) } - ServerNotification::ConfigWarning(notification) => self.on_warning( - notification + ServerNotification::ConfigWarning(notification) => { + let message = notification .details .map(|details| format!("{}: {details}", notification.summary)) - .unwrap_or(notification.summary), - ), + .unwrap_or(notification.summary); + if self.warning_display_state.startup_complete { + self.on_warning(message); + } else if self.warning_display_state.should_display(&message) { + self.add_to_history(history_cell::StartupWarningsCell::new(vec![message])); + self.request_redraw(); + } + } ServerNotification::McpServerStatusUpdated(notification) => { self.on_mcp_server_status_updated(notification) } diff --git a/codex-rs/tui/src/chatwidget/replay.rs b/codex-rs/tui/src/chatwidget/replay.rs index 61d69d10cb..52b31fcbd6 100644 --- a/codex-rs/tui/src/chatwidget/replay.rs +++ b/codex-rs/tui/src/chatwidget/replay.rs @@ -30,6 +30,9 @@ impl ChatWidget { /// avoid triggering side effects. Event ids are passed as `None` to /// distinguish replayed events from live ones. pub(crate) fn replay_thread_turns(&mut self, turns: Vec, replay_kind: ReplayKind) { + if matches!(replay_kind, ReplayKind::ThreadSnapshot) && !turns.is_empty() { + self.warning_display_state.startup_complete = true; + } let latest_turn_id = turns.last().map(|turn| turn.id.clone()); let hidden_nested_review_turns = std::iter::once(/*value*/ false) .chain(turns.windows(/*size*/ 2).map(|turns| { @@ -48,6 +51,7 @@ impl ChatWidget { duration_ms, } = turn; if matches!(status, TurnStatus::InProgress) { + self.warning_display_state.startup_complete = true; self.turn_lifecycle.last_turn_id = Some(turn_id.clone()); self.last_non_retry_error = None; self.on_task_started(); diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap index 014c13cd11..32a728c406 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap @@ -3,8 +3,6 @@ source: tui/src/chatwidget/tests/mcp_startup.rs expression: normalized_backend_snapshot(terminal.backend()) --- " " -"• Booting MCP server: alpha (0s • esc to interrupt) " -" " " " "› Ask Codex to do anything " " " diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index 40a76f5466..38f379b205 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -935,6 +935,29 @@ async fn live_app_server_strict_review_required_notification_renders_message() { assert!(chat.bottom_pane.is_task_running()); } +#[tokio::test] +async fn config_warning_during_turn_remains_inline() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + handle_turn_started(&mut chat, "turn-1"); + chat.handle_server_notification( + ServerNotification::ConfigWarning(ConfigWarningNotification { + summary: "Invalid configuration; using defaults.".into(), + details: Some("Check the project configuration.".into()), + path: None, + range: None, + }), + /*replay_kind*/ None, + ); + let cells = drain_insert_history(&mut rx); + insta::assert_snapshot!( + "runtime_config_warning", + cells + .iter() + .map(|lines| lines_to_single_string(lines)) + .collect::() + ); +} + #[tokio::test] async fn live_app_server_config_warning_prefixes_summary() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; @@ -949,7 +972,7 @@ async fn live_app_server_config_warning_prefixes_summary() { /*replay_kind*/ None, ); - let cells = drain_insert_history(&mut rx); + let cells = drain_insert_history_transcript(&mut rx); assert_eq!(cells.len(), 1, "expected one warning history cell"); let rendered = lines_to_single_string(&cells[0]); assert!( diff --git a/codex-rs/tui/src/chatwidget/tests/helpers.rs b/codex-rs/tui/src/chatwidget/tests/helpers.rs index eae1884d95..fa2bafe3de 100644 --- a/codex-rs/tui/src/chatwidget/tests/helpers.rs +++ b/codex-rs/tui/src/chatwidget/tests/helpers.rs @@ -330,11 +330,24 @@ pub(crate) async fn make_chatwidget_manual_with_sender() -> ( pub(super) fn drain_insert_history( rx: &mut tokio::sync::mpsc::UnboundedReceiver, +) -> Vec>> { + drain_insert_history_with(rx, |cell| cell.display_lines(/*width*/ 80)) +} + +pub(super) fn drain_insert_history_transcript( + rx: &mut tokio::sync::mpsc::UnboundedReceiver, +) -> Vec>> { + drain_insert_history_with(rx, |cell| cell.transcript_lines(/*width*/ 80)) +} + +fn drain_insert_history_with( + rx: &mut tokio::sync::mpsc::UnboundedReceiver, + render: impl Fn(&dyn HistoryCell) -> Vec>, ) -> Vec>> { let mut out = Vec::new(); while let Ok(ev) = rx.try_recv() { if let AppEvent::InsertHistoryCell(cell) = ev { - let mut lines = cell.display_lines(/*width*/ 80); + let mut lines = render(cell.as_ref()); if !cell.is_stream_continuation() && !out.is_empty() && !lines.is_empty() { lines.insert(0, "".into()); } diff --git a/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs b/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs index e12a99d317..260edd4d13 100644 --- a/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs +++ b/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs @@ -1,3 +1,4 @@ +use super::helpers::drain_insert_history_transcript as drain_insert_history; use super::*; use pretty_assertions::assert_eq; @@ -134,6 +135,8 @@ async fn mcp_startup_header_booting_snapshot() { notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); + assert!(chat.bottom_pane.is_task_running()); + assert!(!chat.bottom_pane.status_indicator_visible()); let height = chat.desired_height(/*width*/ 80); let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height)) .expect("create terminal"); @@ -146,6 +149,55 @@ async fn mcp_startup_header_booting_snapshot() { ); } +#[tokio::test] +async fn mcp_startup_updates_preserve_streaming_status_suppression() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.set_mcp_startup_expected_servers(["alpha".into(), "beta".into()]); + handle_turn_started(&mut chat, "turn-1"); + chat.on_agent_message_delta("Partial response\n".into()); + chat.on_commit_tick(); + assert!(!chat.bottom_pane.status_indicator_visible()); + + notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Ready); + assert!(chat.bottom_pane.is_task_running()); + assert!(!chat.bottom_pane.status_indicator_visible()); +} + +#[tokio::test] +async fn mcp_startup_summary_distinguishes_initial_resume_from_task_switch() { + use AppServerTurnStatus::Completed; + use AppServerTurnStatus::InProgress; + use ReplayKind::ResumeInitialMessages; + use ReplayKind::ThreadSnapshot; + + for (replay_kind, status, compact) in [ + (ResumeInitialMessages, Completed, true), + (ResumeInitialMessages, InProgress, false), + (ThreadSnapshot, Completed, false), + ] { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.replay_thread_turns( + vec![app_server_turn( + "previous-turn", + status, + /*duration_ms*/ None, + /*error*/ None, + )], + replay_kind, + ); + while rx.try_recv().is_ok() {} + chat.set_mcp_startup_expected_servers(["alpha".into()]); + notify_mcp_status_error(&mut chat, "alpha", "connection failed"); + let mut compact_cells = Vec::new(); + while let Ok(event) = rx.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = event { + compact_cells.push(cell.as_any().is::()); + } + } + assert_eq!(compact_cells, vec![compact; 2]); + } +} + #[tokio::test] async fn mcp_startup_complete_does_not_clear_running_task() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; @@ -257,6 +309,7 @@ async fn pending_mcp_startup_does_not_unblock_external_review() { chat.thread_id = Some(ThreadId::new()); handle_entered_review_mode(&mut chat, "current changes"); + assert!(chat.bottom_pane.status_indicator_visible()); chat.queue_user_message("queued follow-up".into()); assert!(chat.review.is_review_mode); @@ -270,6 +323,7 @@ async fn pending_mcp_startup_does_not_unblock_external_review() { chat.finish_mcp_startup(Vec::new(), Vec::new()); assert!(chat.bottom_pane.is_task_running()); + assert!(chat.bottom_pane.status_indicator_visible()); assert_eq!(chat.input_queue.queued_user_messages.len(), 1); assert_no_submit_op(&mut op_rx); } @@ -450,7 +504,14 @@ async fn mcp_startup_failure_restores_running_status_header() { "MCP client for `alpha` failed to start: handshake failed", ); notify_mcp_status(&mut chat, "beta", McpServerStartupState::Ready); - let _ = drain_insert_history(&mut rx); + let warnings = super::helpers::drain_insert_history(&mut rx); + insta::assert_snapshot!( + "runtime_mcp_warning", + warnings + .iter() + .map(|lines| lines_to_single_string(lines)) + .collect::() + ); assert!(chat.bottom_pane.is_task_running()); assert!(chat.bottom_pane.status_indicator_visible()); diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__app_server__runtime_config_warning.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__app_server__runtime_config_warning.snap new file mode 100644 index 0000000000..2581da12b5 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__app_server__runtime_config_warning.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests/app_server.rs +expression: "cells.iter().map(|lines| lines_to_single_string(lines)).collect::()" +--- +⚠ Invalid configuration; using defaults.: Check the project configuration. diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__mcp_startup__runtime_mcp_warning.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__mcp_startup__runtime_mcp_warning.snap new file mode 100644 index 0000000000..8049e83193 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__mcp_startup__runtime_mcp_warning.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests/mcp_startup.rs +expression: "warnings.iter().map(|lines| lines_to_single_string(lines)).collect::()" +--- +⚠ MCP client for `alpha` failed to start: handshake failed + +⚠ MCP startup incomplete (failed: alpha) diff --git a/codex-rs/tui/src/chatwidget/turn_runtime.rs b/codex-rs/tui/src/chatwidget/turn_runtime.rs index d57546436e..bcc2d34329 100644 --- a/codex-rs/tui/src/chatwidget/turn_runtime.rs +++ b/codex-rs/tui/src/chatwidget/turn_runtime.rs @@ -36,6 +36,12 @@ impl ChatWidget { || self.review.is_review_mode || self.mcp_startup_status.is_some(), ); + if self.mcp_startup_status.is_some() + && !self.turn_lifecycle.agent_turn_running + && !self.review.is_review_mode + { + self.bottom_pane.hide_status_indicator(); + } self.refresh_status_surfaces(); } @@ -83,6 +89,7 @@ impl ChatWidget { self.quit_shortcut_expires_at = None; self.quit_shortcut_key = None; self.update_task_running_state(); + self.bottom_pane.ensure_status_indicator(); self.bottom_pane.reset_status_timer(Duration::ZERO); self.status_state.retry_status_header = None; self.clear_active_hook_cell(); diff --git a/codex-rs/tui/src/chatwidget/warnings.rs b/codex-rs/tui/src/chatwidget/warnings.rs index ef9660dcff..ea82a51eae 100644 --- a/codex-rs/tui/src/chatwidget/warnings.rs +++ b/codex-rs/tui/src/chatwidget/warnings.rs @@ -6,6 +6,8 @@ const FALLBACK_MODEL_METADATA_WARNING_SUFFIX: &str = #[derive(Default)] pub(super) struct WarningDisplayState { + /// Completed resume history does not end startup; active work does. + pub(super) startup_complete: bool, fallback_model_metadata_slugs: HashSet, } diff --git a/codex-rs/tui/src/history_cell/mod.rs b/codex-rs/tui/src/history_cell/mod.rs index 4e51d50f7c..5caf68623c 100644 --- a/codex-rs/tui/src/history_cell/mod.rs +++ b/codex-rs/tui/src/history_cell/mod.rs @@ -116,6 +116,7 @@ mod request_user_input; mod search; mod separators; mod session; +mod startup_warnings; pub(crate) use approvals::*; pub(crate) use base::*; @@ -132,6 +133,7 @@ pub(crate) use request_user_input::*; pub(crate) use search::*; pub(crate) use separators::*; pub(crate) use session::*; +pub(crate) use startup_warnings::StartupWarningsCell; #[cfg(test)] mod tests; diff --git a/codex-rs/tui/src/history_cell/startup_warnings.rs b/codex-rs/tui/src/history_cell/startup_warnings.rs new file mode 100644 index 0000000000..49393b03da --- /dev/null +++ b/codex-rs/tui/src/history_cell/startup_warnings.rs @@ -0,0 +1,93 @@ +//! Compact startup diagnostics with full details retained in the transcript. +//! Affected sources are unique; sign-in servers are a subset of MCP servers. + +use super::*; +use codex_app_server_protocol::McpServerStartupFailureReason; +use std::collections::BTreeSet; + +#[derive(Clone, Debug, Default)] +pub(crate) struct StartupWarningsCell { + pub(crate) messages: Vec, + pub(crate) other_sources: BTreeSet, + pub(crate) mcp_servers: BTreeSet, + pub(crate) sign_in_servers: BTreeSet, + pub(crate) pending_header: bool, + pub(crate) transcript_hint: Option, +} + +impl StartupWarningsCell { + pub(crate) fn new(messages: Vec) -> Self { + Self { + other_sources: messages.iter().cloned().collect(), + messages, + ..Self::default() + } + } + + pub(crate) fn mcp( + messages: Vec, + servers: impl IntoIterator, + failure_reason: Option, + ) -> Self { + let mcp_servers: BTreeSet<_> = servers.into_iter().collect(); + let sign_in_servers = match failure_reason { + Some(McpServerStartupFailureReason::ReauthenticationRequired) => mcp_servers.clone(), + None => BTreeSet::new(), + }; + Self { + messages, + mcp_servers, + sign_in_servers, + ..Self::default() + } + } +} + +impl HistoryCell for StartupWarningsCell { + #[allow(clippy::disallowed_methods)] + fn display_lines(&self, width: u16) -> Vec> { + if self.pending_header || self.messages.is_empty() || width == 0 { + return Vec::new(); + } + let mcp_count = self.mcp_servers.len(); + let count = mcp_count + self.other_sources.len(); + let sign_in_count = self.sign_in_servers.len(); + let plural = if count == 1 { "" } else { "s" }; + let source = if mcp_count == count { "MCP " } else { "" }; + let mut summary = format!("⚠ {count} {source}startup issue{plural}"); + let mut breakdown = Vec::new(); + if mcp_count > 0 && mcp_count < count { + breakdown.push(format!("{mcp_count} MCP")); + } + if sign_in_count > 0 { + let verb = if sign_in_count == 1 { "needs" } else { "need" }; + breakdown.push(format!("{sign_in_count} {verb} sign-in")); + } + if !breakdown.is_empty() { + summary.push_str(&format!(" ({})", breakdown.join("; "))); + } + if let Some(hint) = &self.transcript_hint { + summary.push_str(&format!(" · {hint} for details")); + } + vec![ + crate::line_truncation::truncate_line_with_ellipsis_if_overflow( + Line::from(summary.yellow().dim()), + width as usize, + ), + ] + } + + fn transcript_lines(&self, width: u16) -> Vec> { + self.messages + .iter() + .flat_map(|message| new_warning_event(message.clone()).transcript_lines(width)) + .collect() + } + + fn raw_lines(&self) -> Vec> { + self.messages + .iter() + .flat_map(|message| raw_lines_from_source(message)) + .collect() + } +}