From bfec6c6d928243ae6a3c72ffd10c404ce9ae4998 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 17 Mar 2026 10:45:57 -0300 Subject: [PATCH 1/7] fix(tui): load app-server mcp inventory asynchronously MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route `/mcp` through app-server inventory RPCs in `tui_app_server` and render the returned server status data in the history view. This removes the app-server TUI fallback stub without changing `app-server` itself. Fetch MCP inventory on a background request handle and post the result back through `AppEvent` so the TUI can repaint immediately. This adds a transient `Loading MCP inventory…` row and avoids the silent delay caused by awaiting the RPC in the main event loop. --- codex-rs/tui_app_server/src/app.rs | 167 ++++++++++ codex-rs/tui_app_server/src/app_event.rs | 9 + codex-rs/tui_app_server/src/chatwidget.rs | 29 +- .../tui_app_server/src/chatwidget/tests.rs | 11 + codex-rs/tui_app_server/src/history_cell.rs | 289 ++++++++++++++++++ ...tests__mcp_inventory_loading_snapshot.snap | 6 + ..._statuses_renders_status_only_servers.snap | 18 ++ 7 files changed, 520 insertions(+), 9 deletions(-) create mode 100644 codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_inventory_loading_snapshot.snap create mode 100644 codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap diff --git a/codex-rs/tui_app_server/src/app.rs b/codex-rs/tui_app_server/src/app.rs index 9fd3f1bd3dd..056f6c65c6e 100644 --- a/codex-rs/tui_app_server/src/app.rs +++ b/codex-rs/tui_app_server/src/app.rs @@ -44,7 +44,13 @@ use crate::tui::TuiEvent; use crate::update_action::UpdateAction; use crate::version::CODEX_CLI_VERSION; use codex_ansi_escape::ansi_escape_line; +use codex_app_server_client::AppServerRequestHandle; +use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::ConfigLayerSource; +use codex_app_server_protocol::ListMcpServerStatusParams; +use codex_app_server_protocol::ListMcpServerStatusResponse; +use codex_app_server_protocol::McpServerStatus; +use codex_app_server_protocol::RequestId; use codex_core::config::Config; use codex_core::config::ConfigBuilder; use codex_core::config::ConfigOverrides; @@ -75,6 +81,8 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::FinalOutput; use codex_protocol::protocol::ListSkillsResponseEvent; #[cfg(test)] +use codex_protocol::protocol::McpAuthStatus; +#[cfg(test)] use codex_protocol::protocol::Op; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionConfiguredEvent; @@ -111,6 +119,7 @@ use tokio::sync::mpsc::error::TrySendError; use tokio::sync::mpsc::unbounded_channel; use tokio::task::JoinHandle; use toml::Value as TomlValue; +use uuid::Uuid; mod agent_navigation; mod app_server_adapter; mod app_server_requests; @@ -1535,6 +1544,42 @@ impl App { Ok(()) } + fn fetch_mcp_inventory(&mut self, app_server: &AppServerSession) { + let request_handle = app_server.request_handle(); + let app_event_tx = self.app_event_tx.clone(); + tokio::spawn(async move { + let result = fetch_all_mcp_server_statuses(request_handle) + .await + .map_err(|err| err.to_string()); + app_event_tx.send(AppEvent::McpInventoryLoaded { result }); + }); + } + + fn handle_mcp_inventory_result(&mut self, result: Result, String>) { + let config = self.chat_widget.config_ref().clone(); + self.chat_widget.clear_mcp_inventory_loading(); + + let statuses = match result { + Ok(statuses) => statuses, + Err(err) => { + self.chat_widget + .add_error_message(format!("Failed to load MCP inventory: {err}")); + return; + } + }; + + if config.mcp_servers.get().is_empty() && statuses.is_empty() { + self.chat_widget + .add_to_history(history_cell::empty_mcp_output()); + return; + } + + self.chat_widget + .add_to_history(history_cell::new_mcp_tools_output_from_statuses( + &config, &statuses, + )); + } + async fn try_submit_active_thread_op_via_app_server( &mut self, app_server: &mut AppServerSession, @@ -2999,6 +3044,12 @@ impl App { AppEvent::RefreshConnectors { force_refetch } => { self.chat_widget.refresh_connectors(force_refetch); } + AppEvent::FetchMcpInventory => { + self.fetch_mcp_inventory(app_server); + } + AppEvent::McpInventoryLoaded { result } => { + self.handle_mcp_inventory_result(result); + } AppEvent::StartFileSearch(query) => { self.file_search.on_user_query(query); } @@ -4421,6 +4472,70 @@ impl App { } } +async fn fetch_all_mcp_server_statuses( + request_handle: AppServerRequestHandle, +) -> Result> { + let mut cursor = None; + let mut statuses = Vec::new(); + + loop { + let request_id = RequestId::String(format!("mcp-inventory-{}", Uuid::new_v4())); + let response: ListMcpServerStatusResponse = request_handle + .request_typed(ClientRequest::McpServerStatusList { + request_id, + params: ListMcpServerStatusParams { + cursor: cursor.clone(), + limit: Some(100), + }, + }) + .await + .wrap_err("mcpServerStatus/list failed in app-server TUI")?; + statuses.extend(response.data); + if let Some(next_cursor) = response.next_cursor { + cursor = Some(next_cursor); + } else { + break; + } + } + + Ok(statuses) +} + +#[cfg(test)] +fn mcp_inventory_maps_from_statuses( + statuses: Vec, +) -> ( + HashMap, + HashMap>, + HashMap>, + HashMap, +) { + let mut tools = HashMap::new(); + let mut resources = HashMap::new(); + let mut resource_templates = HashMap::new(); + let mut auth_statuses = HashMap::new(); + + for status in statuses { + let server_name = status.name; + auth_statuses.insert( + server_name.clone(), + match status.auth_status { + codex_app_server_protocol::McpAuthStatus::Unsupported => McpAuthStatus::Unsupported, + codex_app_server_protocol::McpAuthStatus::NotLoggedIn => McpAuthStatus::NotLoggedIn, + codex_app_server_protocol::McpAuthStatus::BearerToken => McpAuthStatus::BearerToken, + codex_app_server_protocol::McpAuthStatus::OAuth => McpAuthStatus::OAuth, + }, + ); + resources.insert(server_name.clone(), status.resources); + resource_templates.insert(server_name.clone(), status.resource_templates); + for (tool_name, tool) in status.tools { + tools.insert(format!("mcp__{server_name}__{tool_name}"), tool); + } + } + + (tools, resources, resource_templates, auth_statuses) +} + #[cfg(test)] mod tests { use super::*; @@ -4446,11 +4561,13 @@ mod tests { use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; + use codex_protocol::mcp::Tool; use codex_protocol::openai_models::ModelAvailabilityNux; use codex_protocol::protocol::AgentMessageDeltaEvent; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; + use codex_protocol::protocol::McpAuthStatus; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionConfiguredEvent; use codex_protocol::protocol::SessionSource; @@ -4491,6 +4608,56 @@ mod tests { Ok(()) } + #[test] + fn mcp_inventory_maps_prefix_tool_names_by_server() { + let statuses = vec![ + McpServerStatus { + name: "docs".to_string(), + tools: HashMap::from([( + "list".to_string(), + Tool { + description: None, + name: "list".to_string(), + title: None, + input_schema: serde_json::json!({"type": "object"}), + output_schema: None, + annotations: None, + icons: None, + meta: None, + }, + )]), + resources: Vec::new(), + resource_templates: Vec::new(), + auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, + }, + McpServerStatus { + name: "disabled".to_string(), + tools: HashMap::new(), + resources: Vec::new(), + resource_templates: Vec::new(), + auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, + }, + ]; + + let (tools, resources, resource_templates, auth_statuses) = + mcp_inventory_maps_from_statuses(statuses); + let mut resource_names = resources.keys().cloned().collect::>(); + resource_names.sort(); + let mut template_names = resource_templates.keys().cloned().collect::>(); + template_names.sort(); + + assert_eq!( + tools.keys().cloned().collect::>(), + vec!["mcp__docs__list".to_string()] + ); + assert_eq!(resource_names, vec!["disabled", "docs"]); + assert_eq!(template_names, vec!["disabled", "docs"]); + assert_eq!( + auth_statuses.get("disabled"), + Some(&McpAuthStatus::Unsupported) + ); + } + #[test] fn startup_waiting_gate_is_only_for_fresh_or_exit_session_selection() { assert_eq!( diff --git a/codex-rs/tui_app_server/src/app_event.rs b/codex-rs/tui_app_server/src/app_event.rs index 0582538bd93..8b6513d24bd 100644 --- a/codex-rs/tui_app_server/src/app_event.rs +++ b/codex-rs/tui_app_server/src/app_event.rs @@ -10,6 +10,7 @@ use std::path::PathBuf; +use codex_app_server_protocol::McpServerStatus; use codex_chatgpt::connectors::AppInfo; use codex_file_search::FileMatch; use codex_protocol::ThreadId; @@ -165,6 +166,14 @@ pub(crate) enum AppEvent { force_refetch: bool, }, + /// Fetch MCP inventory via app-server RPCs and render it into history. + FetchMcpInventory, + + /// Result of fetching MCP inventory via app-server RPCs. + McpInventoryLoaded { + result: Result, String>, + }, + InsertHistoryCell(Box), /// Apply rollback semantics to local transcript cells. diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index 0b4fb7c184a..10db040c0c4 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -67,7 +67,6 @@ use codex_core::find_thread_name_by_id; use codex_core::git_info::current_branch_name; use codex_core::git_info::get_git_repo_root; use codex_core::git_info::local_git_branches; -use codex_core::mcp::McpManager; use codex_core::plugins::PluginsManager; use codex_core::project_doc::DEFAULT_PROJECT_DOC_FILENAME; use codex_core::skills::model::SkillMetadata; @@ -8219,17 +8218,29 @@ impl ChatWidget { } pub(crate) fn add_mcp_output(&mut self) { - let mcp_manager = McpManager::new(Arc::new(PluginsManager::new( - self.config.codex_home.clone(), + self.flush_answer_stream_with_separator(); + self.flush_active_cell(); + self.active_cell = Some(Box::new(history_cell::new_mcp_inventory_loading( + self.config.animations, ))); - if mcp_manager - .effective_servers(&self.config, /*auth*/ None) - .is_empty() + self.bump_active_cell_revision(); + self.request_redraw(); + self.app_event_tx.send(AppEvent::FetchMcpInventory); + } + + pub(crate) fn clear_mcp_inventory_loading(&mut self) { + let Some(active) = self.active_cell.as_ref() else { + return; + }; + if !active + .as_any() + .is::() { - self.add_to_history(history_cell::empty_mcp_output()); - } else { - self.add_app_server_stub_message("MCP tool inventory"); + return; } + self.active_cell = None; + self.bump_active_cell_revision(); + self.request_redraw(); } pub(crate) fn add_connectors_output(&mut self) { diff --git a/codex-rs/tui_app_server/src/chatwidget/tests.rs b/codex-rs/tui_app_server/src/chatwidget/tests.rs index 07770182b22..3e2c33de7ba 100644 --- a/codex-rs/tui_app_server/src/chatwidget/tests.rs +++ b/codex-rs/tui_app_server/src/chatwidget/tests.rs @@ -6041,6 +6041,17 @@ async fn slash_memory_drop_reports_stubbed_feature() { ); } +#[tokio::test] +async fn slash_mcp_requests_inventory_via_app_server() { + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await; + + chat.dispatch_command(SlashCommand::Mcp); + + assert!(active_blob(&chat).contains("Loading MCP inventory")); + assert_matches!(rx.try_recv(), Ok(AppEvent::FetchMcpInventory)); + assert!(op_rx.try_recv().is_err(), "expected no core op to be sent"); +} + #[tokio::test] async fn slash_memory_update_reports_stubbed_feature() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await; diff --git a/codex-rs/tui_app_server/src/history_cell.rs b/codex-rs/tui_app_server/src/history_cell.rs index bba63c77ab5..bb2d9e6989e 100644 --- a/codex-rs/tui_app_server/src/history_cell.rs +++ b/codex-rs/tui_app_server/src/history_cell.rs @@ -37,6 +37,7 @@ use crate::wrapping::RtOptions; use crate::wrapping::adaptive_wrap_line; use crate::wrapping::adaptive_wrap_lines; use base64::Engine; +use codex_app_server_protocol::McpServerStatus; use codex_core::config::Config; use codex_core::config::types::McpServerTransportConfig; use codex_core::mcp::McpManager; @@ -1963,6 +1964,190 @@ pub(crate) fn new_mcp_tools_output( PlainHistoryCell { lines } } + +pub(crate) fn new_mcp_tools_output_from_statuses( + config: &Config, + statuses: &[McpServerStatus], +) -> PlainHistoryCell { + let mut lines: Vec> = vec![ + "/mcp".magenta().into(), + "".into(), + vec!["🔌 ".into(), "MCP Tools".bold()].into(), + "".into(), + ]; + + let mut statuses_by_name = HashMap::new(); + for status in statuses { + statuses_by_name.insert(status.name.as_str(), status); + } + + let mut server_names: Vec = config.mcp_servers.keys().cloned().collect(); + for status in statuses { + if !server_names.iter().any(|name| name == &status.name) { + server_names.push(status.name.clone()); + } + } + server_names.sort(); + + let has_any_tools = statuses.iter().any(|status| !status.tools.is_empty()); + if !has_any_tools { + lines.push(" • No MCP tools available.".italic().into()); + lines.push("".into()); + } + + for server in server_names { + let cfg = config.mcp_servers.get().get(server.as_str()); + let status = statuses_by_name.get(server.as_str()).copied(); + let mut header: Vec> = vec![" • ".into(), server.clone().into()]; + + if let Some(cfg) = cfg + && !cfg.enabled + { + header.push(" ".into()); + header.push("(disabled)".red()); + lines.push(header.into()); + if let Some(reason) = cfg.disabled_reason.as_ref().map(ToString::to_string) { + lines.push(vec![" • Reason: ".into(), reason.dim()].into()); + } + lines.push(Line::from("")); + continue; + } + + lines.push(header.into()); + lines.push(vec![" • Status: ".into(), "enabled".green()].into()); + let auth_status = status + .map(|status| match status.auth_status { + codex_app_server_protocol::McpAuthStatus::Unsupported => McpAuthStatus::Unsupported, + codex_app_server_protocol::McpAuthStatus::NotLoggedIn => McpAuthStatus::NotLoggedIn, + codex_app_server_protocol::McpAuthStatus::BearerToken => McpAuthStatus::BearerToken, + codex_app_server_protocol::McpAuthStatus::OAuth => McpAuthStatus::OAuth, + }) + .unwrap_or(McpAuthStatus::Unsupported); + lines.push(vec![" • Auth: ".into(), auth_status.to_string().into()].into()); + + if let Some(cfg) = cfg { + match &cfg.transport { + McpServerTransportConfig::Stdio { + command, + args, + env, + env_vars, + cwd, + } => { + let args_suffix = if args.is_empty() { + String::new() + } else { + format!(" {}", args.join(" ")) + }; + let cmd_display = format!("{command}{args_suffix}"); + lines.push(vec![" • Command: ".into(), cmd_display.into()].into()); + + if let Some(cwd) = cwd.as_ref() { + lines.push( + vec![" • Cwd: ".into(), cwd.display().to_string().into()].into(), + ); + } + + let env_display = format_env_display(env.as_ref(), env_vars.as_slice()); + if env_display != "-" { + lines.push(vec![" • Env: ".into(), env_display.into()].into()); + } + } + McpServerTransportConfig::StreamableHttp { + url, + http_headers, + env_http_headers, + .. + } => { + lines.push(vec![" • URL: ".into(), url.clone().into()].into()); + if let Some(headers) = http_headers.as_ref() + && !headers.is_empty() + { + let mut pairs: Vec<_> = headers.iter().collect(); + pairs.sort_by(|(a, _), (b, _)| a.cmp(b)); + let display = pairs + .into_iter() + .map(|(name, _)| format!("{name}=*****")) + .collect::>() + .join(", "); + lines.push(vec![" • HTTP headers: ".into(), display.into()].into()); + } + if let Some(headers) = env_http_headers.as_ref() + && !headers.is_empty() + { + let mut pairs: Vec<_> = headers.iter().collect(); + pairs.sort_by(|(a, _), (b, _)| a.cmp(b)); + let display = pairs + .into_iter() + .map(|(name, var)| format!("{name}={var}")) + .collect::>() + .join(", "); + lines.push(vec![" • Env HTTP headers: ".into(), display.into()].into()); + } + } + } + } + + let mut names = status + .map(|status| status.tools.keys().cloned().collect::>()) + .unwrap_or_default(); + names.sort(); + if names.is_empty() { + lines.push(" • Tools: (none)".into()); + } else { + lines.push(vec![" • Tools: ".into(), names.join(", ").into()].into()); + } + + let server_resources = status + .map(|status| status.resources.clone()) + .unwrap_or_default(); + if server_resources.is_empty() { + lines.push(" • Resources: (none)".into()); + } else { + let mut spans: Vec> = vec![" • Resources: ".into()]; + + for (idx, resource) in server_resources.iter().enumerate() { + if idx > 0 { + spans.push(", ".into()); + } + + let label = resource.title.as_ref().unwrap_or(&resource.name); + spans.push(label.clone().into()); + spans.push(" ".into()); + spans.push(format!("({})", resource.uri).dim()); + } + + lines.push(spans.into()); + } + + let server_templates = status + .map(|status| status.resource_templates.clone()) + .unwrap_or_default(); + if server_templates.is_empty() { + lines.push(" • Resource templates: (none)".into()); + } else { + let mut spans: Vec> = vec![" • Resource templates: ".into()]; + + for (idx, template) in server_templates.iter().enumerate() { + if idx > 0 { + spans.push(", ".into()); + } + + let label = template.title.as_ref().unwrap_or(&template.name); + spans.push(label.clone().into()); + spans.push(" ".into()); + spans.push(format!("({})", template.uri_template).dim()); + } + + lines.push(spans.into()); + } + + lines.push(Line::from("")); + } + + PlainHistoryCell { lines } +} + pub(crate) fn new_info_event(message: String, hint: Option) -> PlainHistoryCell { let mut line = vec!["• ".dim(), message.into()]; if let Some(hint) = hint { @@ -1981,6 +2166,46 @@ pub(crate) fn new_error_event(message: String) -> PlainHistoryCell { PlainHistoryCell { lines } } +#[derive(Debug)] +pub(crate) struct McpInventoryLoadingCell { + start_time: Instant, + animations_enabled: bool, +} + +impl McpInventoryLoadingCell { + pub(crate) fn new(animations_enabled: bool) -> Self { + Self { + start_time: Instant::now(), + animations_enabled, + } + } +} + +impl HistoryCell for McpInventoryLoadingCell { + fn display_lines(&self, _width: u16) -> Vec> { + vec![ + vec![ + spinner(Some(self.start_time), self.animations_enabled), + " ".into(), + "Loading MCP inventory".bold(), + "…".dim(), + ] + .into(), + ] + } + + fn transcript_animation_tick(&self) -> Option { + if !self.animations_enabled { + return None; + } + Some((self.start_time.elapsed().as_millis() / 50) as u64) + } +} + +pub(crate) fn new_mcp_inventory_loading(animations_enabled: bool) -> McpInventoryLoadingCell { + McpInventoryLoadingCell::new(animations_enabled) +} + /// Renders a completed (or interrupted) request_user_input exchange in history. #[derive(Debug)] pub(crate) struct RequestUserInputResultCell { @@ -2542,6 +2767,7 @@ mod tests { use codex_core::config::Config; use codex_core::config::ConfigBuilder; use codex_core::config::types::McpServerConfig; + use codex_core::config::types::McpServerDisabledReason; use codex_core::config::types::McpServerTransportConfig; use codex_otel::RuntimeMetricTotals; use codex_otel::RuntimeMetricsSummary; @@ -2961,6 +3187,61 @@ mod tests { insta::assert_snapshot!(rendered); } + #[tokio::test] + async fn mcp_tools_output_from_statuses_renders_status_only_servers() { + let mut config = test_config().await; + let servers = HashMap::from([( + "disabled".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: "disabled-server".to_string(), + args: vec![], + env: None, + env_vars: vec![], + cwd: None, + }, + enabled: false, + required: false, + disabled_reason: Some(McpServerDisabledReason::Unknown), + startup_timeout_sec: None, + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth_resource: None, + }, + )]); + config + .mcp_servers + .set(servers) + .expect("test mcp servers should accept any configuration"); + + let statuses = vec![McpServerStatus { + name: "plugin_docs".to_string(), + tools: HashMap::from([( + "lookup".to_string(), + Tool { + description: None, + name: "lookup".to_string(), + title: None, + input_schema: serde_json::json!({"type": "object", "properties": {}}), + output_schema: None, + annotations: None, + icons: None, + meta: None, + }, + )]), + resources: Vec::new(), + resource_templates: Vec::new(), + auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, + }]; + + let cell = new_mcp_tools_output_from_statuses(&config, &statuses); + let rendered = render_lines(&cell.display_lines(120)).join("\n"); + + insta::assert_snapshot!(rendered); + } + #[test] fn empty_agent_message_cell_transcript() { let cell = AgentMessageCell::new(vec![Line::default()], false); @@ -3188,6 +3469,14 @@ mod tests { insta::assert_snapshot!(rendered); } + #[test] + fn mcp_inventory_loading_snapshot() { + let cell = new_mcp_inventory_loading(/*animations_enabled*/ true); + let rendered = render_lines(&cell.display_lines(80)).join("\n"); + + insta::assert_snapshot!(rendered); + } + #[test] fn completed_mcp_tool_call_success_snapshot() { let invocation = McpInvocation { diff --git a/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_inventory_loading_snapshot.snap b/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_inventory_loading_snapshot.snap new file mode 100644 index 00000000000..d01c31bd6a9 --- /dev/null +++ b/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_inventory_loading_snapshot.snap @@ -0,0 +1,6 @@ +--- +source: tui_app_server/src/history_cell.rs +assertion_line: 3477 +expression: rendered +--- +• Loading MCP inventory… diff --git a/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap b/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap new file mode 100644 index 00000000000..3cac57046e0 --- /dev/null +++ b/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap @@ -0,0 +1,18 @@ +--- +source: tui_app_server/src/history_cell.rs +assertion_line: 3202 +expression: rendered +--- +/mcp + +🔌 MCP Tools + + • disabled (disabled) + • Reason: unknown + + • plugin_docs + • Status: enabled + • Auth: Unsupported + • Tools: lookup + • Resources: (none) + • Resource templates: (none) From 1f3194f6024be6c206989923abda7f7327b512c6 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 17 Mar 2026 10:56:24 -0300 Subject: [PATCH 2/7] docs: add rustdoc for async MCP inventory flow Documentation-only pass over the MCP inventory change: adds doc comments to all new public/crate-visible functions and types. --- codex-rs/tui_app_server/src/app.rs | 21 +++++++++++++++++++++ codex-rs/tui_app_server/src/chatwidget.rs | 9 +++++++++ codex-rs/tui_app_server/src/history_cell.rs | 19 +++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/codex-rs/tui_app_server/src/app.rs b/codex-rs/tui_app_server/src/app.rs index 056f6c65c6e..36d8c945a3e 100644 --- a/codex-rs/tui_app_server/src/app.rs +++ b/codex-rs/tui_app_server/src/app.rs @@ -1544,6 +1544,13 @@ impl App { Ok(()) } + /// Spawn a background task that fetches the full MCP server inventory from the + /// app-server via paginated RPCs, then delivers the result back through + /// `AppEvent::McpInventoryLoaded`. + /// + /// The spawned task is fire-and-forget: no `JoinHandle` is stored, so a stale + /// result may arrive after the user has moved on. This is harmless — the result + /// is rendered into the current history regardless. fn fetch_mcp_inventory(&mut self, app_server: &AppServerSession) { let request_handle = app_server.request_handle(); let app_event_tx = self.app_event_tx.clone(); @@ -1555,6 +1562,11 @@ impl App { }); } + /// Process the completed MCP inventory fetch: clear the loading spinner, then + /// render either the full tool/resource listing or an error into chat history. + /// + /// When both the local config and the app-server report zero servers, a special + /// "empty" cell is shown instead of the full table. fn handle_mcp_inventory_result(&mut self, result: Result, String>) { let config = self.chat_widget.config_ref().clone(); self.chat_widget.clear_mcp_inventory_loading(); @@ -4472,6 +4484,11 @@ impl App { } } +/// Collect every MCP server status from the app-server by walking the paginated +/// `mcpServerStatus/list` RPC until no `next_cursor` is returned. +/// +/// All pages are eagerly gathered into a single `Vec` so the caller can render +/// the inventory atomically. Each page requests up to 100 entries. async fn fetch_all_mcp_server_statuses( request_handle: AppServerRequestHandle, ) -> Result> { @@ -4501,6 +4518,10 @@ async fn fetch_all_mcp_server_statuses( Ok(statuses) } +/// Convert flat `McpServerStatus` responses into the per-server maps used by the +/// in-process MCP subsystem (tools keyed as `mcp__{server}__{tool}`, plus +/// per-server resource/template/auth maps). Test-only because the app-server TUI +/// renders directly from `McpServerStatus` rather than these maps. #[cfg(test)] fn mcp_inventory_maps_from_statuses( statuses: Vec, diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index 10db040c0c4..8da680ad6ba 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -8217,6 +8217,11 @@ impl ChatWidget { PlainHistoryCell::new(vec![line.into()]) } + /// Begin the asynchronous MCP inventory flow: show a loading spinner and + /// request the app-server fetch via `AppEvent::FetchMcpInventory`. + /// + /// The spinner lives in `active_cell` and is cleared by + /// [`clear_mcp_inventory_loading`] once the result arrives. pub(crate) fn add_mcp_output(&mut self) { self.flush_answer_stream_with_separator(); self.flush_active_cell(); @@ -8228,6 +8233,10 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::FetchMcpInventory); } + /// Remove the MCP loading spinner if it is still the active cell. + /// + /// Uses `Any`-based type checking so that a late-arriving inventory result + /// does not accidentally clear an unrelated cell that was set in the meantime. pub(crate) fn clear_mcp_inventory_loading(&mut self) { let Some(active) = self.active_cell.as_ref() else { return; diff --git a/codex-rs/tui_app_server/src/history_cell.rs b/codex-rs/tui_app_server/src/history_cell.rs index bb2d9e6989e..c8663519b6a 100644 --- a/codex-rs/tui_app_server/src/history_cell.rs +++ b/codex-rs/tui_app_server/src/history_cell.rs @@ -1965,6 +1965,16 @@ pub(crate) fn new_mcp_tools_output( PlainHistoryCell { lines } } +/// Build the `/mcp` history cell from app-server `McpServerStatus` responses. +/// +/// The server list is the union of servers declared in local config and servers +/// reported by the app-server, sorted alphabetically. Disabled servers (from +/// config) are rendered with a `(disabled)` tag and skip tool/resource details. +/// Servers that appear only in the status response (e.g., plugin-injected +/// servers) are rendered with whatever the app-server reported. +/// +/// This mirrors the layout of [`new_mcp_tools_output`] but sources data from +/// the paginated RPC response rather than the in-process `McpManager`. pub(crate) fn new_mcp_tools_output_from_statuses( config: &Config, statuses: &[McpServerStatus], @@ -2166,6 +2176,14 @@ pub(crate) fn new_error_event(message: String) -> PlainHistoryCell { PlainHistoryCell { lines } } +/// A transient history cell that shows an animated spinner while the MCP +/// inventory RPC is in flight. +/// +/// Inserted as the `active_cell` by `ChatWidget::add_mcp_output()` and removed +/// by `ChatWidget::clear_mcp_inventory_loading()` once the fetch completes. +/// `clear_mcp_inventory_loading` uses `Any::is::()` to +/// ensure it only clears a cell of this exact type, avoiding interference with +/// unrelated active cells. #[derive(Debug)] pub(crate) struct McpInventoryLoadingCell { start_time: Instant, @@ -2202,6 +2220,7 @@ impl HistoryCell for McpInventoryLoadingCell { } } +/// Convenience constructor for [`McpInventoryLoadingCell`]. pub(crate) fn new_mcp_inventory_loading(animations_enabled: bool) -> McpInventoryLoadingCell { McpInventoryLoadingCell::new(animations_enabled) } From 721b6f850fa5fdd162fe8d85d22ace1e2a2d2aec Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 17 Mar 2026 11:53:33 -0300 Subject: [PATCH 3/7] codex: address PR review feedback (#14931) Derive the app-server TUI `/mcp` inventory from `mcpServerStatus/list` results only, instead of merging in local-only configured servers. This keeps remote inventory output aligned with the connected app-server. Retain local config only as display enrichment for servers returned by the app-server, and update the snapshot coverage to match the new remote-authoritative rendering behavior. --- codex-rs/tui_app_server/src/history_cell.rs | 36 +++++-------------- ..._statuses_renders_status_only_servers.snap | 4 +-- 2 files changed, 9 insertions(+), 31 deletions(-) diff --git a/codex-rs/tui_app_server/src/history_cell.rs b/codex-rs/tui_app_server/src/history_cell.rs index c8663519b6a..36700d34ba2 100644 --- a/codex-rs/tui_app_server/src/history_cell.rs +++ b/codex-rs/tui_app_server/src/history_cell.rs @@ -1967,11 +1967,9 @@ pub(crate) fn new_mcp_tools_output( /// Build the `/mcp` history cell from app-server `McpServerStatus` responses. /// -/// The server list is the union of servers declared in local config and servers -/// reported by the app-server, sorted alphabetically. Disabled servers (from -/// config) are rendered with a `(disabled)` tag and skip tool/resource details. -/// Servers that appear only in the status response (e.g., plugin-injected -/// servers) are rendered with whatever the app-server reported. +/// The server list comes directly from the app-server status response, sorted +/// alphabetically. Local config is only used to enrich returned servers with +/// transport details such as command, URL, cwd, and environment display. /// /// This mirrors the layout of [`new_mcp_tools_output`] but sources data from /// the paginated RPC response rather than the in-process `McpManager`. @@ -1991,12 +1989,7 @@ pub(crate) fn new_mcp_tools_output_from_statuses( statuses_by_name.insert(status.name.as_str(), status); } - let mut server_names: Vec = config.mcp_servers.keys().cloned().collect(); - for status in statuses { - if !server_names.iter().any(|name| name == &status.name) { - server_names.push(status.name.clone()); - } - } + let mut server_names: Vec = statuses.iter().map(|status| status.name.clone()).collect(); server_names.sort(); let has_any_tools = statuses.iter().any(|status| !status.tools.is_empty()); @@ -2008,20 +2001,7 @@ pub(crate) fn new_mcp_tools_output_from_statuses( for server in server_names { let cfg = config.mcp_servers.get().get(server.as_str()); let status = statuses_by_name.get(server.as_str()).copied(); - let mut header: Vec> = vec![" • ".into(), server.clone().into()]; - - if let Some(cfg) = cfg - && !cfg.enabled - { - header.push(" ".into()); - header.push("(disabled)".red()); - lines.push(header.into()); - if let Some(reason) = cfg.disabled_reason.as_ref().map(ToString::to_string) { - lines.push(vec![" • Reason: ".into(), reason.dim()].into()); - } - lines.push(Line::from("")); - continue; - } + let header: Vec> = vec![" • ".into(), server.clone().into()]; lines.push(header.into()); lines.push(vec![" • Status: ".into(), "enabled".green()].into()); @@ -3210,11 +3190,11 @@ mod tests { async fn mcp_tools_output_from_statuses_renders_status_only_servers() { let mut config = test_config().await; let servers = HashMap::from([( - "disabled".to_string(), + "plugin_docs".to_string(), McpServerConfig { transport: McpServerTransportConfig::Stdio { - command: "disabled-server".to_string(), - args: vec![], + command: "docs-server".to_string(), + args: vec!["--stdio".to_string()], env: None, env_vars: vec![], cwd: None, diff --git a/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap b/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap index 3cac57046e0..2f6ecb4cdeb 100644 --- a/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap +++ b/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap @@ -7,12 +7,10 @@ expression: rendered 🔌 MCP Tools - • disabled (disabled) - • Reason: unknown - • plugin_docs • Status: enabled • Auth: Unsupported + • Command: docs-server --stdio • Tools: lookup • Resources: (none) • Resource templates: (none) From 6ab7c0dd91a929e7750ee316d37acc4f5a3de7ce Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 17 Mar 2026 12:04:15 -0300 Subject: [PATCH 4/7] fix(tui): name test-only mcp inventory tuple Add a `McpInventoryMaps` alias for the test helper that converts `McpServerStatus` entries into the in-process map layout. This keeps the app-server `/mcp` coverage intact while avoiding the `clippy::type_complexity` failure that broke rust-ci on the follow-up review fix. --- codex-rs/tui_app_server/src/app.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/codex-rs/tui_app_server/src/app.rs b/codex-rs/tui_app_server/src/app.rs index 36d8c945a3e..0a8c216bc90 100644 --- a/codex-rs/tui_app_server/src/app.rs +++ b/codex-rs/tui_app_server/src/app.rs @@ -4523,14 +4523,15 @@ async fn fetch_all_mcp_server_statuses( /// per-server resource/template/auth maps). Test-only because the app-server TUI /// renders directly from `McpServerStatus` rather than these maps. #[cfg(test)] -fn mcp_inventory_maps_from_statuses( - statuses: Vec, -) -> ( +type McpInventoryMaps = ( HashMap, HashMap>, HashMap>, HashMap, -) { +); + +#[cfg(test)] +fn mcp_inventory_maps_from_statuses(statuses: Vec) -> McpInventoryMaps { let mut tools = HashMap::new(); let mut resources = HashMap::new(); let mut resource_templates = HashMap::new(); From 5e3202ee813c65154e53e37eed47334be826cae8 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 17 Mar 2026 13:48:22 -0300 Subject: [PATCH 5/7] fix(tui): tighten app-server /mcp rendering Remove the RPC-backed `/mcp` status row so the app-server TUI no longer claims servers are enabled without authoritative data from `mcpServerStatus/list`. Also clear committed MCP loading cells when the async inventory result arrives, so transient spinner rows do not remain in history after active cell churn. --- codex-rs/tui_app_server/src/app.rs | 37 +++++++++++++++++++ codex-rs/tui_app_server/src/history_cell.rs | 8 ++-- ..._statuses_renders_status_only_servers.snap | 2 - 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/codex-rs/tui_app_server/src/app.rs b/codex-rs/tui_app_server/src/app.rs index 0a8c216bc90..6a5257b2bf2 100644 --- a/codex-rs/tui_app_server/src/app.rs +++ b/codex-rs/tui_app_server/src/app.rs @@ -1570,6 +1570,7 @@ impl App { fn handle_mcp_inventory_result(&mut self, result: Result, String>) { let config = self.chat_widget.config_ref().clone(); self.chat_widget.clear_mcp_inventory_loading(); + self.clear_committed_mcp_inventory_loading(); let statuses = match result { Ok(statuses) => statuses, @@ -1592,6 +1593,23 @@ impl App { )); } + fn clear_committed_mcp_inventory_loading(&mut self) { + let Some(last_cell) = self.transcript_cells.last() else { + return; + }; + if !last_cell + .as_any() + .is::() + { + return; + } + + self.transcript_cells.pop(); + if let Some(Overlay::Transcript(overlay)) = &mut self.overlay { + overlay.replace_cells(self.transcript_cells.clone()); + } + } + async fn try_submit_active_thread_op_via_app_server( &mut self, app_server: &mut AppServerSession, @@ -4680,6 +4698,25 @@ mod tests { ); } + #[tokio::test] + async fn handle_mcp_inventory_result_clears_committed_loading_cell() { + let mut app = make_test_app().await; + app.transcript_cells + .push(Arc::new(history_cell::new_mcp_inventory_loading( + /*animations_enabled*/ false, + ))); + + app.handle_mcp_inventory_result(Ok(vec![McpServerStatus { + name: "docs".to_string(), + tools: HashMap::new(), + resources: Vec::new(), + resource_templates: Vec::new(), + auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, + }])); + + assert_eq!(app.transcript_cells.len(), 0); + } + #[test] fn startup_waiting_gate_is_only_for_fresh_or_exit_session_selection() { assert_eq!( diff --git a/codex-rs/tui_app_server/src/history_cell.rs b/codex-rs/tui_app_server/src/history_cell.rs index 36700d34ba2..4ff095881bd 100644 --- a/codex-rs/tui_app_server/src/history_cell.rs +++ b/codex-rs/tui_app_server/src/history_cell.rs @@ -2004,7 +2004,6 @@ pub(crate) fn new_mcp_tools_output_from_statuses( let header: Vec> = vec![" • ".into(), server.clone().into()]; lines.push(header.into()); - lines.push(vec![" • Status: ".into(), "enabled".green()].into()); let auth_status = status .map(|status| match status.auth_status { codex_app_server_protocol::McpAuthStatus::Unsupported => McpAuthStatus::Unsupported, @@ -2160,10 +2159,9 @@ pub(crate) fn new_error_event(message: String) -> PlainHistoryCell { /// inventory RPC is in flight. /// /// Inserted as the `active_cell` by `ChatWidget::add_mcp_output()` and removed -/// by `ChatWidget::clear_mcp_inventory_loading()` once the fetch completes. -/// `clear_mcp_inventory_loading` uses `Any::is::()` to -/// ensure it only clears a cell of this exact type, avoiding interference with -/// unrelated active cells. +/// once the fetch completes. The app removes committed copies from transcript +/// history, while `ChatWidget::clear_mcp_inventory_loading()` only clears the +/// in-flight `active_cell`. #[derive(Debug)] pub(crate) struct McpInventoryLoadingCell { start_time: Instant, diff --git a/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap b/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap index 2f6ecb4cdeb..6c95cc44333 100644 --- a/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap +++ b/codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap @@ -1,6 +1,5 @@ --- source: tui_app_server/src/history_cell.rs -assertion_line: 3202 expression: rendered --- /mcp @@ -8,7 +7,6 @@ expression: rendered 🔌 MCP Tools • plugin_docs - • Status: enabled • Auth: Unsupported • Command: docs-server --stdio • Tools: lookup From b25954b9e953650d6cb2df85ce1cfcad54325531 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 17 Mar 2026 14:01:30 -0300 Subject: [PATCH 6/7] fix(tui): clear stale mcp loading cells Remove the most recent committed `McpInventoryLoadingCell` from transcript history when `/mcp` finishes loading, instead of only checking the transcript tail. This keeps stale loading rows from surviving when later output was committed before the inventory RPC completed. --- codex-rs/tui_app_server/src/app.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/codex-rs/tui_app_server/src/app.rs b/codex-rs/tui_app_server/src/app.rs index 6a5257b2bf2..aa4bf4082d2 100644 --- a/codex-rs/tui_app_server/src/app.rs +++ b/codex-rs/tui_app_server/src/app.rs @@ -1594,17 +1594,15 @@ impl App { } fn clear_committed_mcp_inventory_loading(&mut self) { - let Some(last_cell) = self.transcript_cells.last() else { + let Some(index) = self + .transcript_cells + .iter() + .rposition(|cell| cell.as_any().is::()) + else { return; }; - if !last_cell - .as_any() - .is::() - { - return; - } - self.transcript_cells.pop(); + self.transcript_cells.remove(index); if let Some(Overlay::Transcript(overlay)) = &mut self.overlay { overlay.replace_cells(self.transcript_cells.clone()); } From 946f92ff6efaba12e0324aa3e35f932274404bac Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 17 Mar 2026 14:44:03 -0300 Subject: [PATCH 7/7] docs(tui): note /mcp async tradeoff Document the app-server `/mcp` async fetch tradeoff next to the fire-and-forget request path. This records why late completions are currently accepted as a low- severity UI artifact instead of introducing request-token invalidation state in this change. --- codex-rs/tui_app_server/src/app.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/codex-rs/tui_app_server/src/app.rs b/codex-rs/tui_app_server/src/app.rs index aa4bf4082d2..282425209af 100644 --- a/codex-rs/tui_app_server/src/app.rs +++ b/codex-rs/tui_app_server/src/app.rs @@ -1549,8 +1549,10 @@ impl App { /// `AppEvent::McpInventoryLoaded`. /// /// The spawned task is fire-and-forget: no `JoinHandle` is stored, so a stale - /// result may arrive after the user has moved on. This is harmless — the result - /// is rendered into the current history regardless. + /// result may arrive after the user has moved on. We currently accept that + /// tradeoff because the effect is limited to stale inventory output in history, + /// while request-token invalidation would add cross-cutting async state for a + /// low-severity path. fn fetch_mcp_inventory(&mut self, app_server: &AppServerSession) { let request_handle = app_server.request_handle(); let app_event_tx = self.app_event_tx.clone();