From f549049385dd668cfc2de43bb97dbf4e2b2adfc5 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 27 Mar 2026 15:02:19 -0600 Subject: [PATCH] Surface MCP startup failures in tui_app_server --- codex-rs/tui_app_server/src/chatwidget.rs | 87 ++++++++++++++---- ...artup_failure_renders_warning_history.snap | 15 ++++ .../tui_app_server/src/chatwidget/tests.rs | 88 +++++++++++++++++++ 3 files changed, 175 insertions(+), 15 deletions(-) create mode 100644 codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__app_server_mcp_startup_failure_renders_warning_history.snap diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index 68380ec8ce..d2edbf7a9b 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -76,6 +76,8 @@ use codex_app_server_protocol::ErrorNotification; use codex_app_server_protocol::FileChangeRequestApprovalParams; use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::ItemStartedNotification; +use codex_app_server_protocol::McpServerStartupState; +use codex_app_server_protocol::McpServerStatusUpdatedNotification; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequest; use codex_app_server_protocol::ThreadItem; @@ -2780,15 +2782,41 @@ impl ChatWidget { self.request_redraw(); } - #[cfg(test)] - fn on_mcp_startup_update(&mut self, ev: McpStartupUpdateEvent) { - let mut status = self.mcp_startup_status.take().unwrap_or_default(); - if let McpStartupStatus::Failed { error } = &ev.status { + fn update_mcp_startup_status( + &mut self, + server: String, + status: McpStartupStatus, + complete_when_settled: bool, + ) { + let mut startup_status = self.mcp_startup_status.take().unwrap_or_default(); + if let McpStartupStatus::Failed { error } = &status { self.on_warning(error); } - status.insert(ev.server, ev.status); - self.mcp_startup_status = Some(status); + startup_status.insert(server, status); + self.mcp_startup_status = Some(startup_status); self.update_task_running_state(); + if complete_when_settled + && let Some(current) = &self.mcp_startup_status + && !current.is_empty() + && current + .values() + .all(|state| !matches!(state, McpStartupStatus::Starting)) + { + let mut failed = Vec::new(); + let mut cancelled = Vec::new(); + for (name, state) in current { + match state { + McpStartupStatus::Ready => {} + McpStartupStatus::Failed { .. } => failed.push(name.clone()), + McpStartupStatus::Cancelled => cancelled.push(name.clone()), + McpStartupStatus::Starting => {} + } + } + failed.sort(); + cancelled.sort(); + self.finish_mcp_startup(failed, cancelled); + return; + } if let Some(current) = &self.mcp_startup_status { let total = current.len(); let mut starting: Vec<_> = current @@ -2828,18 +2856,21 @@ impl ChatWidget { } #[cfg(test)] - fn on_mcp_startup_complete(&mut self, ev: McpStartupCompleteEvent) { - let mut parts = Vec::new(); - if !ev.failed.is_empty() { - let failed_servers: Vec<_> = ev.failed.iter().map(|f| f.server.clone()).collect(); - parts.push(format!("failed: {}", failed_servers.join(", "))); - } - if !ev.cancelled.is_empty() { + fn on_mcp_startup_update(&mut self, ev: McpStartupUpdateEvent) { + self.update_mcp_startup_status(ev.server, ev.status, /*complete_when_settled*/ false); + } + + 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: {}", - ev.cancelled.join(", ") + cancelled.join(", ") )); } + 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("; "))); } @@ -2850,6 +2881,30 @@ impl ChatWidget { self.request_redraw(); } + #[cfg(test)] + fn on_mcp_startup_complete(&mut self, ev: McpStartupCompleteEvent) { + let failed = ev.failed.into_iter().map(|f| f.server).collect(); + self.finish_mcp_startup(failed, ev.cancelled); + } + + fn on_mcp_server_status_updated(&mut self, notification: McpServerStatusUpdatedNotification) { + let status = match notification.status { + McpServerStartupState::Starting => McpStartupStatus::Starting, + McpServerStartupState::Ready => McpStartupStatus::Ready, + McpServerStartupState::Failed => McpStartupStatus::Failed { + error: notification.error.unwrap_or_else(|| { + format!("MCP client for `{}` failed to start", notification.name) + }), + }, + McpServerStartupState::Cancelled => McpStartupStatus::Cancelled, + }; + self.update_mcp_startup_status( + notification.name, + status, + /*complete_when_settled*/ true, + ); + } + /// Handle a turn aborted due to user interrupt (Esc). /// When there are queued user messages, restore them into the composer /// separated by newlines rather than auto‑submitting the next one. @@ -6318,6 +6373,9 @@ impl ChatWidget { .map(|details| format!("{}: {details}", notification.summary)) .unwrap_or(notification.summary), ), + ServerNotification::McpServerStatusUpdated(notification) => { + self.on_mcp_server_status_updated(notification) + } ServerNotification::ItemGuardianApprovalReviewStarted(notification) => { self.on_guardian_review_notification( notification.target_item_id, @@ -6401,7 +6459,6 @@ impl ChatWidget { | ServerNotification::RawResponseItemCompleted(_) | ServerNotification::CommandExecOutputDelta(_) | ServerNotification::McpToolCallProgress(_) - | ServerNotification::McpServerStatusUpdated(_) | ServerNotification::McpServerOauthLoginCompleted(_) | ServerNotification::AppListUpdated(_) | ServerNotification::FsChanged(_) diff --git a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__app_server_mcp_startup_failure_renders_warning_history.snap b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__app_server_mcp_startup_failure_renders_warning_history.snap new file mode 100644 index 0000000000..c3c5c4eee4 --- /dev/null +++ b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__app_server_mcp_startup_failure_renders_warning_history.snap @@ -0,0 +1,15 @@ +--- +source: tui_app_server/src/chatwidget/tests.rs +assertion_line: 11571 +expression: term.backend().vt100().screen().contents() +--- + + + +⚠ MCP client for `alpha` failed to start: handshake failed +⚠ MCP startup incomplete (failed: alpha) + + +› Ask Codex to do anything + + ? for shortcuts 100% context left diff --git a/codex-rs/tui_app_server/src/chatwidget/tests.rs b/codex-rs/tui_app_server/src/chatwidget/tests.rs index bcd8ff06be..c733316879 100644 --- a/codex-rs/tui_app_server/src/chatwidget/tests.rs +++ b/codex-rs/tui_app_server/src/chatwidget/tests.rs @@ -50,6 +50,8 @@ use codex_app_server_protocol::ItemGuardianApprovalReviewCompletedNotification; use codex_app_server_protocol::ItemGuardianApprovalReviewStartedNotification; use codex_app_server_protocol::ItemStartedNotification; use codex_app_server_protocol::MarketplaceInterface; +use codex_app_server_protocol::McpServerStartupState; +use codex_app_server_protocol::McpServerStatusUpdatedNotification; use codex_app_server_protocol::PatchApplyStatus as AppServerPatchApplyStatus; use codex_app_server_protocol::PatchChangeKind; use codex_app_server_protocol::PluginAuthPolicy; @@ -11518,6 +11520,92 @@ async fn mcp_startup_complete_does_not_clear_running_task() { assert!(chat.bottom_pane.status_indicator_visible()); } +#[tokio::test] +async fn app_server_mcp_startup_failure_renders_warning_history() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + chat.show_welcome_banner = false; + + for notification in [ + McpServerStatusUpdatedNotification { + name: "alpha".to_string(), + status: McpServerStartupState::Starting, + error: None, + }, + McpServerStatusUpdatedNotification { + name: "beta".to_string(), + status: McpServerStartupState::Starting, + error: None, + }, + ] { + chat.handle_server_notification( + ServerNotification::McpServerStatusUpdated(notification), + None, + ); + } + + assert!(drain_insert_history(&mut rx).is_empty()); + assert!(chat.bottom_pane.is_task_running()); + + chat.handle_server_notification( + ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification { + name: "alpha".to_string(), + status: McpServerStartupState::Failed, + error: Some("MCP client for `alpha` failed to start: handshake failed".to_string()), + }), + None, + ); + + let failure_cells = drain_insert_history(&mut rx); + let failure_text = failure_cells + .iter() + .map(|lines| lines_to_single_string(lines)) + .collect::(); + assert!(failure_text.contains("MCP client for `alpha` failed to start: handshake failed")); + assert!(!failure_text.contains("MCP startup incomplete")); + assert!(chat.bottom_pane.is_task_running()); + + chat.handle_server_notification( + ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification { + name: "beta".to_string(), + status: McpServerStartupState::Ready, + error: None, + }), + None, + ); + + let summary_cells = drain_insert_history(&mut rx); + let summary_text = summary_cells + .iter() + .map(|lines| lines_to_single_string(lines)) + .collect::(); + assert_eq!(summary_text, "⚠ MCP startup incomplete (failed: alpha)\n"); + assert!(!chat.bottom_pane.is_task_running()); + + let width: u16 = 120; + let ui_height: u16 = chat.desired_height(width); + let vt_height: u16 = 10; + let viewport = Rect::new(0, vt_height - ui_height - 1, width, ui_height); + + let backend = VT100Backend::new(width, vt_height); + let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal"); + term.set_viewport_area(viewport); + + for lines in failure_cells.into_iter().chain(summary_cells) { + crate::insert_history::insert_history_lines(&mut term, lines) + .expect("Failed to insert history lines in test"); + } + + term.draw(|f| { + chat.render(f.area(), f.buffer_mut()); + }) + .expect("draw MCP startup warning history"); + + assert_snapshot!( + "app_server_mcp_startup_failure_renders_warning_history", + term.backend().vt100().screen().contents() + ); +} + #[tokio::test] async fn background_event_updates_status_header() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;