From 916a160ec4c55e3378f080dcaf250a392e350a43 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 27 Mar 2026 15:13:15 -0600 Subject: [PATCH] codex: address PR review feedback (#16041) --- codex-rs/tui_app_server/src/chatwidget.rs | 10 +++ .../tui_app_server/src/chatwidget/tests.rs | 79 +++++++++++++++---- 2 files changed, 75 insertions(+), 14 deletions(-) diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index d2edbf7a9b..cd9032b1d4 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -2798,6 +2798,7 @@ impl ChatWidget { if complete_when_settled && let Some(current) = &self.mcp_startup_status && !current.is_empty() + && current.len() >= self.enabled_mcp_server_count() && current .values() .all(|state| !matches!(state, McpStartupStatus::Starting)) @@ -2855,6 +2856,15 @@ impl ChatWidget { self.request_redraw(); } + fn enabled_mcp_server_count(&self) -> usize { + self.config + .mcp_servers + .get() + .values() + .filter(|cfg| cfg.enabled) + .count() + } + #[cfg(test)] fn on_mcp_startup_update(&mut self, ev: McpStartupUpdateEvent) { self.update_mcp_startup_status(ev.server, ev.status, /*complete_when_settled*/ false); diff --git a/codex-rs/tui_app_server/src/chatwidget/tests.rs b/codex-rs/tui_app_server/src/chatwidget/tests.rs index c733316879..0bf3f95cc0 100644 --- a/codex-rs/tui_app_server/src/chatwidget/tests.rs +++ b/codex-rs/tui_app_server/src/chatwidget/tests.rs @@ -79,6 +79,8 @@ use codex_core::config::Config; use codex_core::config::ConfigBuilder; use codex_core::config::Constrained; use codex_core::config::ConstraintError; +use codex_core::config::types::McpServerConfig; +use codex_core::config::types::McpServerTransportConfig; use codex_core::config::types::Notifications; #[cfg(target_os = "windows")] use codex_core::config::types::WindowsSandboxModeToml; @@ -11524,24 +11526,61 @@ async fn mcp_startup_complete_does_not_clear_running_task() { 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; + chat.config.mcp_servers = Constrained::allow_any(HashMap::from([ + ( + "alpha".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: "alpha".to_string(), + args: Vec::new(), + env: None, + env_vars: Vec::new(), + cwd: None, + }, + enabled: true, + required: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth_resource: None, + tools: HashMap::new(), + }, + ), + ( + "beta".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: "beta".to_string(), + args: Vec::new(), + env: None, + env_vars: Vec::new(), + cwd: None, + }, + enabled: true, + required: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth_resource: None, + tools: HashMap::new(), + }, + ), + ])); - for notification in [ - McpServerStatusUpdatedNotification { + chat.handle_server_notification( + ServerNotification::McpServerStatusUpdated(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, - ); - } + }), + None, + ); assert!(drain_insert_history(&mut rx).is_empty()); assert!(chat.bottom_pane.is_task_running()); @@ -11564,6 +11603,18 @@ async fn app_server_mcp_startup_failure_renders_warning_history() { 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::Starting, + error: None, + }), + None, + ); + + assert!(drain_insert_history(&mut rx).is_empty()); + assert!(chat.bottom_pane.is_task_running()); + chat.handle_server_notification( ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification { name: "beta".to_string(),