diff --git a/codex-rs/tui/src/app/app_server_events.rs b/codex-rs/tui/src/app/app_server_events.rs index 413e8b8c80..307b4d3ba8 100644 --- a/codex-rs/tui/src/app/app_server_events.rs +++ b/codex-rs/tui/src/app/app_server_events.rs @@ -95,12 +95,11 @@ impl App { } ServerNotification::ExternalAgentConfigImportCompleted(_) => { let cwd = self.chat_widget.config_ref().cwd.to_path_buf(); - if let Err(err) = self.refresh_in_memory_config_from_disk().await { - tracing::warn!( - error = %err, - "failed to refresh config after external agent config import" - ); - } + self.refresh_config_layer_stack_from_app_server_best_effort( + app_server_client, + "external agent config import", + ) + .await; self.chat_widget.refresh_plugin_mentions(); self.chat_widget.submit_op(AppCommand::reload_user_config()); self.fetch_plugins_list(app_server_client, cwd); diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index c323a2d5bf..a27d353086 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -6,6 +6,8 @@ use super::plugin_mentions::fetch_plugin_mentions; use super::*; +use codex_app_server_protocol::ConfigReadParams; +use codex_app_server_protocol::ConfigReadResponse; use codex_app_server_protocol::MarketplaceAddParams; use codex_app_server_protocol::MarketplaceAddResponse; use codex_app_server_protocol::MarketplaceRemoveParams; @@ -14,7 +16,9 @@ use codex_app_server_protocol::MarketplaceUpgradeParams; use codex_app_server_protocol::MarketplaceUpgradeResponse; use codex_app_server_protocol::RequestId; +use codex_config::types::McpServerConfig; +use crate::app_event::McpInventory; use crate::hooks_rpc::fetch_hooks_list; use crate::hooks_rpc::write_hook_trust; use crate::hooks_rpc::write_hook_trusts; @@ -26,10 +30,11 @@ impl App { app_server: &AppServerSession, detail: McpServerStatusDetail, ) { + let cwd = self.chat_widget.config_ref().cwd.to_path_buf(); 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, detail) + let result = fetch_mcp_inventory_from_app_server(request_handle, detail, cwd) .await .map_err(|err| err.to_string()); app_event_tx.send(AppEvent::McpInventoryLoaded { result, detail }); @@ -360,16 +365,12 @@ impl App { } pub(super) fn refresh_plugin_mentions(&mut self, app_server: &AppServerSession) { - let config = self.config.clone(); + let cwd = self.chat_widget.config_ref().cwd.to_path_buf(); let request_handle = app_server.request_handle(); let app_event_tx = self.app_event_tx.clone(); - if !config.features.enabled(Feature::Plugins) { - app_event_tx.send(AppEvent::PluginMentionsLoaded { plugins: None }); - return; - } tokio::spawn(async move { - match fetch_plugin_mentions(request_handle, config).await { + match fetch_plugin_mentions(request_handle, cwd).await { Ok(plugins) => { app_event_tx.send(AppEvent::PluginMentionsLoaded { plugins: Some(plugins), @@ -505,19 +506,18 @@ 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. + /// When app-server status and config both report zero servers, a special "empty" cell is shown + /// instead of the full table. pub(super) fn handle_mcp_inventory_result( &mut self, - result: Result, String>, + result: Result, detail: McpServerStatusDetail, ) { - 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, + let inventory = match result { + Ok(inventory) => inventory, Err(err) => { self.chat_widget .add_error_message(format!("Failed to load MCP inventory: {err}")); @@ -525,7 +525,7 @@ impl App { } }; - if config.mcp_servers.get().is_empty() && statuses.is_empty() { + if inventory.statuses.is_empty() && inventory.configured_servers.is_empty() { self.chat_widget .add_to_history(history_cell::empty_mcp_output()); return; @@ -533,7 +533,9 @@ impl App { self.chat_widget .add_to_history(history_cell::new_mcp_tools_output_from_statuses( - &config, &statuses, detail, + &inventory.configured_servers, + &inventory.statuses, + detail, )); } @@ -553,6 +555,64 @@ impl App { } } +pub(super) async fn fetch_mcp_inventory_from_app_server( + request_handle: AppServerRequestHandle, + detail: McpServerStatusDetail, + cwd: PathBuf, +) -> Result { + let configured_servers = match fetch_app_server_mcp_config(request_handle.clone(), cwd).await { + Ok(configured_servers) => configured_servers, + Err(err) => { + tracing::warn!( + error = %err, + "config/read failed while loading MCP inventory config; rendering status data only" + ); + HashMap::new() + } + }; + let statuses = fetch_all_mcp_server_statuses(request_handle, detail).await?; + Ok(McpInventory { + statuses, + configured_servers, + }) +} + +pub(super) async fn fetch_app_server_config( + request_handle: AppServerRequestHandle, + cwd: Option, + include_layers: bool, +) -> Result { + let request_id = RequestId::String(format!("config-read-{}", Uuid::new_v4())); + request_handle + .request_typed(ClientRequest::ConfigRead { + request_id, + params: ConfigReadParams { + include_layers, + cwd: cwd.map(|cwd| cwd.display().to_string()), + }, + }) + .await + .wrap_err("config/read failed in TUI") +} + +async fn fetch_app_server_mcp_config( + request_handle: AppServerRequestHandle, + cwd: PathBuf, +) -> Result> { + let response = + fetch_app_server_config(request_handle, Some(cwd), /*include_layers*/ false).await?; + mcp_servers_from_app_server_config(response.config) +} + +fn mcp_servers_from_app_server_config( + config: codex_app_server_protocol::Config, +) -> Result> { + let Some(mcp_servers) = config.additional.get("mcp_servers") else { + return Ok(HashMap::new()); + }; + serde_json::from_value(mcp_servers.clone()).wrap_err("failed to decode app-server MCP config") +} + pub(super) async fn fetch_all_mcp_server_statuses( request_handle: AppServerRequestHandle, detail: McpServerStatusDetail, diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 6f4fda608e..6b243f6d5c 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -4,7 +4,11 @@ //! and the ChatWidget copy of session settings, keeping persistence-heavy code out of the main app //! loop. +use super::background_requests::fetch_app_server_config; use super::*; +use codex_app_server_protocol::ConfigLayer as AppServerConfigLayer; +use codex_config::ConfigLayerEntry; +use codex_config::ConfigLayerStack; impl App { pub(super) async fn rebuild_config_for_cwd(&self, cwd: PathBuf) -> Result { @@ -41,6 +45,40 @@ impl App { } } + pub(super) async fn refresh_config_layer_stack_from_app_server_best_effort( + &mut self, + app_server: &AppServerSession, + action: &str, + ) { + let cwd = self.chat_widget.config_ref().cwd.to_path_buf(); + match fetch_app_server_config( + app_server.request_handle(), + Some(cwd), + /*include_layers*/ true, + ) + .await + .and_then(|response| { + let Some(layers) = response.layers else { + return Err(color_eyre::eyre::eyre!( + "config/read response did not include layers" + )); + }; + config_layer_stack_from_app_server_layers(layers, &self.config.config_layer_stack) + }) { + Ok(config_layer_stack) => { + self.config.config_layer_stack = config_layer_stack; + self.chat_widget.sync_plugin_mentions_config(&self.config); + } + Err(err) => { + tracing::warn!( + error = %err, + action, + "failed to refresh config layers from app server" + ); + } + } + } + pub(super) async fn rebuild_config_for_resume_or_fallback( &mut self, current_cwd: &Path, @@ -584,12 +622,46 @@ impl App { } } +fn config_layer_stack_from_app_server_layers( + layers: Vec, + current_stack: &ConfigLayerStack, +) -> Result { + let mut layers = layers + .into_iter() + .map(config_layer_entry_from_app_server_layer) + .collect::>>()?; + // config/read returns layers highest-precedence first; ConfigLayerStack stores them lowest + // precedence first. + layers.reverse(); + ConfigLayerStack::new( + layers, + current_stack.requirements().clone(), + current_stack.requirements_toml().clone(), + ) + .wrap_err("invalid app-server config layer stack") +} + +fn config_layer_entry_from_app_server_layer( + layer: AppServerConfigLayer, +) -> Result { + let config = serde_json::from_value(layer.config) + .wrap_err("failed to decode app-server config layer")?; + let mut entry = if let Some(disabled_reason) = layer.disabled_reason { + ConfigLayerEntry::new_disabled(layer.name, config, disabled_reason) + } else { + ConfigLayerEntry::new(layer.name, config) + }; + entry.version = layer.version; + Ok(entry) +} + #[cfg(test)] mod tests { use super::*; use crate::app::test_support::app_enabled_in_effective_config; use crate::app::test_support::make_test_app; use crate::test_support::PathBufExt; + use codex_app_server_protocol::ConfigLayerSource; use codex_protocol::models::PermissionProfile; use pretty_assertions::assert_eq; use tempfile::tempdir; @@ -651,6 +723,50 @@ mod tests { Ok(()) } + #[test] + fn config_layer_stack_from_app_server_layers_uses_server_user_config() -> Result<()> { + let user_file = AbsolutePathBuf::try_from(PathBuf::from("/tmp/codex/config.toml"))?; + let stack = config_layer_stack_from_app_server_layers( + vec![AppServerConfigLayer { + name: ConfigLayerSource::User { + file: user_file, + profile: None, + }, + version: "server-version".to_string(), + config: serde_json::json!({ + "marketplaces": { + "server-marketplace": { + "source_type": "git", + }, + }, + }), + disabled_reason: None, + }], + &ConfigLayerStack::default(), + )?; + + let user_config = stack + .effective_user_config() + .expect("server user config layer"); + let source_type = user_config + .get("marketplaces") + .and_then(toml::Value::as_table) + .and_then(|marketplaces| marketplaces.get("server-marketplace")) + .and_then(toml::Value::as_table) + .and_then(|marketplace| marketplace.get("source_type")) + .and_then(toml::Value::as_str); + + assert_eq!(source_type, Some("git")); + assert_eq!( + stack + .get_active_user_layer() + .expect("active user layer") + .version, + "server-version" + ); + Ok(()) + } + #[tokio::test] async fn refresh_in_memory_config_from_disk_best_effort_keeps_current_config_on_error() -> Result<()> { diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index ddb11c4520..b204996b83 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -502,9 +502,11 @@ impl App { self.chat_widget .on_marketplace_add_loaded(cwd.clone(), source, result); if add_succeeded && self.chat_widget.config_ref().cwd.as_path() == cwd.as_path() { - if let Err(err) = self.refresh_in_memory_config_from_disk().await { - tracing::warn!(error = %err, "failed to refresh config after marketplace add"); - } + self.refresh_config_layer_stack_from_app_server_best_effort( + app_server, + "marketplace add", + ) + .await; self.fetch_plugins_list(app_server, cwd); } } @@ -512,12 +514,11 @@ impl App { let marketplace_contents_changed = matches!(&result, Ok(response) if !response.upgraded_roots.is_empty()); if marketplace_contents_changed { - if let Err(err) = self.refresh_in_memory_config_from_disk().await { - tracing::warn!( - error = %err, - "failed to refresh config after marketplace upgrade" - ); - } + self.refresh_config_layer_stack_from_app_server_best_effort( + app_server, + "marketplace upgrade", + ) + .await; self.chat_widget.refresh_plugin_mentions(); self.chat_widget.submit_op(AppCommand::reload_user_config()); } @@ -554,9 +555,11 @@ impl App { ); if remove_succeeded && self.chat_widget.config_ref().cwd.as_path() == cwd.as_path() { - if let Err(err) = self.refresh_in_memory_config_from_disk().await { - tracing::warn!(error = %err, "failed to refresh config after marketplace remove"); - } + self.refresh_config_layer_stack_from_app_server_best_effort( + app_server, + "marketplace remove", + ) + .await; self.chat_widget.refresh_plugin_mentions(); self.chat_widget.submit_op(AppCommand::reload_user_config()); self.fetch_plugins_list(app_server, cwd); @@ -605,9 +608,11 @@ impl App { } => { let install_succeeded = result.is_ok(); if install_succeeded { - if let Err(err) = self.refresh_in_memory_config_from_disk().await { - tracing::warn!(error = %err, "failed to refresh config after plugin install"); - } + self.refresh_config_layer_stack_from_app_server_best_effort( + app_server, + "plugin install", + ) + .await; self.chat_widget.refresh_plugin_mentions(); self.chat_widget.submit_op(AppCommand::reload_user_config()); } @@ -661,12 +666,11 @@ impl App { self.pending_plugin_enabled_writes.remove(&plugin_id); let update_succeeded = result.is_ok(); if update_succeeded { - if let Err(err) = self.refresh_in_memory_config_from_disk().await { - tracing::warn!( - error = %err, - "failed to refresh config after plugin toggle" - ); - } + self.refresh_config_layer_stack_from_app_server_best_effort( + app_server, + "plugin toggle", + ) + .await; self.chat_widget.refresh_plugin_mentions(); self.chat_widget.submit_op(AppCommand::reload_user_config()); } @@ -1238,12 +1242,11 @@ impl App { } => { let uninstall_succeeded = result.is_ok(); if uninstall_succeeded { - if let Err(err) = self.refresh_in_memory_config_from_disk().await { - tracing::warn!( - error = %err, - "failed to refresh config after plugin uninstall" - ); - } + self.refresh_config_layer_stack_from_app_server_best_effort( + app_server, + "plugin uninstall", + ) + .await; self.chat_widget.refresh_plugin_mentions(); self.chat_widget.submit_op(AppCommand::reload_user_config()); } @@ -1261,10 +1264,7 @@ impl App { AppEvent::RefreshPluginMentions => { self.refresh_plugin_mentions(app_server); } - AppEvent::PluginMentionsLoaded { mut plugins } => { - if !self.config.features.enabled(Feature::Plugins) { - plugins = None; - } + AppEvent::PluginMentionsLoaded { plugins } => { self.chat_widget.on_plugin_mentions_loaded(plugins); } AppEvent::PersistPersonalitySelection { personality } => { diff --git a/codex-rs/tui/src/app/plugin_mentions.rs b/codex-rs/tui/src/app/plugin_mentions.rs index ef300896bc..b0d192935a 100644 --- a/codex-rs/tui/src/app/plugin_mentions.rs +++ b/codex-rs/tui/src/app/plugin_mentions.rs @@ -1,109 +1,108 @@ //! Plugin mention capability enrichment for the TUI. //! -//! Mention inventory comes from app-server `plugin/list`, while mention eligibility still reuses -//! the older local bulk capability summaries. That keeps the feature app-server-shaped without -//! paying for a `plugin/read` per plugin. +//! Mention inventory comes from app-server `plugin/list`, then eligible plugins are hydrated with +//! `plugin/read` so the TUI does not derive mention capabilities from the client machine's local +//! config or plugin cache. +use super::background_requests::fetch_plugin_detail; use super::background_requests::request_plugin_list; use super::*; +use codex_app_server_protocol::PluginDetail; use codex_app_server_protocol::PluginListResponse; -use codex_app_server_protocol::PluginMarketplaceEntry; +use codex_app_server_protocol::PluginReadParams; use codex_app_server_protocol::PluginSummary; -use codex_core_plugins::PluginsManager; +use codex_plugin::AppConnectorId; use codex_plugin::PluginCapabilitySummary; -use std::collections::HashMap; - -#[derive(Debug, Clone)] -struct PluginMentionEntry { - config_name: String, - display_name: String, - description: Option, -} - -impl PluginMentionEntry { - fn capability_summary( - self, - capabilities_by_config_name: &HashMap, - ) -> Option { - let capabilities = capabilities_by_config_name.get(&self.config_name)?; - Some(PluginCapabilitySummary { - config_name: self.config_name, - display_name: self.display_name, - description: self.description, - has_skills: capabilities.has_skills, - mcp_server_names: capabilities.mcp_server_names.clone(), - app_connector_ids: capabilities.app_connector_ids.clone(), - }) - } -} pub(super) async fn fetch_plugin_mentions( request_handle: AppServerRequestHandle, - config: crate::legacy_core::config::Config, + cwd: PathBuf, ) -> Result> { - let response = request_plugin_list(request_handle, config.cwd.to_path_buf()).await?; - let mention_entries = plugin_mention_entries_from_list_response(response); - let capabilities_by_config_name = load_plugin_mention_capabilities(&config).await; + let response = request_plugin_list(request_handle.clone(), cwd).await?; + let mention_reads = plugin_mention_reads_from_list_response(response); - Ok(mention_entries - .into_iter() - .filter_map(|entry| entry.capability_summary(&capabilities_by_config_name)) - .collect()) -} - -async fn load_plugin_mention_capabilities( - config: &crate::legacy_core::config::Config, -) -> HashMap { - let plugins_input = config.plugins_config_input(); - PluginsManager::new(config.codex_home.to_path_buf()) - .plugins_for_config(&plugins_input) - .await - .capability_summaries() - .iter() - .cloned() - .map(|summary| (summary.config_name.clone(), summary)) - .collect() -} - -fn plugin_mention_entries_from_list_response( - response: PluginListResponse, -) -> Vec { - response - .marketplaces - .into_iter() - .flat_map(plugin_mention_entries_from_marketplace) - .collect() -} - -fn plugin_mention_entries_from_marketplace( - marketplace: PluginMarketplaceEntry, -) -> Vec { - let marketplace_name = marketplace.name; - marketplace - .plugins - .into_iter() - .filter_map(|plugin| plugin_mention_entry(&marketplace_name, plugin)) - .collect() -} - -fn plugin_mention_entry( - marketplace_name: &str, - plugin: PluginSummary, -) -> Option { - if !plugin_is_eligible_for_mentions(&plugin) { - return None; + let mut capabilities = Vec::new(); + for read_params in mention_reads { + let plugin_name = read_params.plugin_name.clone(); + match fetch_plugin_detail(request_handle.clone(), read_params).await { + Ok(response) => { + if let Some(summary) = plugin_detail_capability_summary(response.plugin) { + capabilities.push(summary); + } + } + Err(err) => { + tracing::warn!( + error = %err, + plugin_name, + "plugin/read failed while refreshing plugin mention candidates" + ); + } + } } + Ok(capabilities) +} - let config_name = plugin_mention_config_name(marketplace_name, &plugin)?; - Some(PluginMentionEntry { +fn plugin_detail_capability_summary(detail: PluginDetail) -> Option { + let config_name = plugin_mention_config_name(&detail.marketplace_name, &detail.summary)?; + Some(PluginCapabilitySummary { config_name, - display_name: plugin_mention_display_name(&plugin), - description: plugin_mention_description(&plugin), + display_name: plugin_mention_display_name(&detail.summary), + description: plugin_mention_description(&detail.summary), + has_skills: detail.skills.iter().any(|skill| skill.enabled), + mcp_server_names: detail.mcp_servers, + app_connector_ids: detail + .apps + .into_iter() + .map(|app| AppConnectorId(app.id)) + .collect(), }) } -fn plugin_is_eligible_for_mentions(plugin: &PluginSummary) -> bool { - plugin.installed && plugin.enabled +fn plugin_mention_reads_from_list_response(response: PluginListResponse) -> Vec { + response + .marketplaces + .into_iter() + .flat_map(|marketplace| { + let marketplace_name = marketplace.name; + let marketplace_path = marketplace.path; + marketplace.plugins.into_iter().filter_map(move |plugin| { + plugin_mention_read(&marketplace_name, marketplace_path.clone(), plugin) + }) + }) + .collect() +} + +fn plugin_mention_read( + marketplace_name: &str, + marketplace_path: Option, + plugin: PluginSummary, +) -> Option { + if !plugin.installed || !plugin.enabled { + return None; + } + + plugin_mention_config_name(marketplace_name, &plugin)?; + if let Some(marketplace_path) = marketplace_path { + Some(PluginReadParams { + marketplace_path: Some(marketplace_path), + remote_marketplace_name: None, + plugin_name: plugin.name, + }) + } else { + let Some(remote_plugin_id) = plugin.remote_plugin_id else { + tracing::warn!( + plugin_name = plugin.name, + marketplace_name, + "skipping remote plugin mention without remote plugin id" + ); + return None; + }; + Some(PluginReadParams { + marketplace_path: None, + remote_marketplace_name: Some(marketplace_name.to_string()), + plugin_name: remote_plugin_id, + }) + } } fn plugin_mention_config_name(marketplace_name: &str, plugin: &PluginSummary) -> Option { @@ -145,3 +144,127 @@ fn plugin_mention_description(plugin: &PluginSummary) -> Option { .filter(|description| !description.is_empty()) .map(str::to_string) } + +#[cfg(test)] +mod tests { + use super::*; + use codex_app_server_protocol::AppSummary; + use codex_app_server_protocol::PluginAuthPolicy; + use codex_app_server_protocol::PluginAvailability; + use codex_app_server_protocol::PluginInstallPolicy; + use codex_app_server_protocol::PluginMarketplaceEntry; + use codex_app_server_protocol::PluginSource; + use codex_app_server_protocol::SkillSummary; + use pretty_assertions::assert_eq; + + fn test_absolute_path(path: &str) -> AbsolutePathBuf { + AbsolutePathBuf::try_from(PathBuf::from(path)).expect("absolute test path") + } + + fn plugin_summary(name: &str, installed: bool, enabled: bool) -> PluginSummary { + PluginSummary { + id: format!("{name}@test-marketplace"), + remote_plugin_id: None, + local_version: None, + name: name.to_string(), + share_context: None, + source: PluginSource::Remote, + installed, + enabled, + install_policy: PluginInstallPolicy::Available, + auth_policy: PluginAuthPolicy::OnUse, + availability: PluginAvailability::Available, + interface: None, + keywords: Vec::new(), + } + } + + #[test] + fn plugin_mentions_use_app_server_detail_capabilities() { + let summary = plugin_detail_capability_summary(PluginDetail { + marketplace_name: "test-marketplace".to_string(), + marketplace_path: Some(test_absolute_path("/marketplace")), + summary: plugin_summary("calendar", /*installed*/ true, /*enabled*/ true), + description: None, + skills: vec![SkillSummary { + name: "schedule".to_string(), + description: "Schedule things".to_string(), + short_description: None, + interface: None, + path: None, + enabled: true, + }], + hooks: Vec::new(), + apps: vec![AppSummary { + id: "calendar-app".to_string(), + name: "Calendar".to_string(), + description: None, + install_url: None, + needs_auth: false, + }], + mcp_servers: vec!["calendar-mcp".to_string()], + }) + .expect("valid plugin detail"); + + assert_eq!( + summary, + PluginCapabilitySummary { + config_name: "calendar@test-marketplace".to_string(), + display_name: "calendar".to_string(), + description: None, + has_skills: true, + mcp_server_names: vec!["calendar-mcp".to_string()], + app_connector_ids: vec![AppConnectorId("calendar-app".to_string())], + } + ); + } + + #[test] + fn plugin_mention_reads_skip_disabled_and_handle_local_and_remote_sources() { + let marketplace_path = test_absolute_path("/marketplace"); + let mut remote_plugin = plugin_summary( + "remote_name", + /*installed*/ true, + /*enabled*/ true, + ); + remote_plugin.remote_plugin_id = Some("plugins~remote_id".to_string()); + + let reads = plugin_mention_reads_from_list_response(PluginListResponse { + marketplaces: vec![ + PluginMarketplaceEntry { + name: "test-marketplace".to_string(), + path: Some(marketplace_path.clone()), + interface: None, + plugins: vec![ + plugin_summary("enabled", /*installed*/ true, /*enabled*/ true), + plugin_summary("disabled", /*installed*/ true, /*enabled*/ false), + ], + }, + PluginMarketplaceEntry { + name: "chatgpt-global".to_string(), + path: None, + interface: None, + plugins: vec![remote_plugin], + }, + ], + marketplace_load_errors: Vec::new(), + featured_plugin_ids: Vec::new(), + }); + + assert_eq!( + reads, + vec![ + PluginReadParams { + marketplace_path: Some(marketplace_path), + remote_marketplace_name: None, + plugin_name: "enabled".to_string(), + }, + PluginReadParams { + marketplace_path: None, + remote_marketplace_name: Some("chatgpt-global".to_string()), + plugin_name: "plugins~remote_id".to_string(), + }, + ] + ); + } +} diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index c42f42c257..a1d572c011 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -117,19 +117,74 @@ async fn handle_mcp_inventory_result_clears_committed_loading_cell() { ))); 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, - }]), + Ok(crate::app_event::McpInventory { + statuses: vec![McpServerStatus { + name: "docs".to_string(), + tools: HashMap::new(), + resources: Vec::new(), + resource_templates: Vec::new(), + auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, + }], + configured_servers: HashMap::new(), + }), McpServerStatusDetail::ToolsAndAuthOnly, ); assert_eq!(app.transcript_cells.len(), 0); } +#[tokio::test] +async fn handle_mcp_inventory_result_uses_server_empty_inventory_not_client_config() { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let local_server: codex_config::types::McpServerConfig = + toml::from_str("command = \"local-only\"").expect("valid test MCP config"); + let local_servers = HashMap::from([("local_only".to_string(), local_server)]); + app.config + .mcp_servers + .set(local_servers) + .expect("test config accepts MCP servers"); + + app.handle_mcp_inventory_result( + Ok(crate::app_event::McpInventory { + statuses: Vec::new(), + configured_servers: HashMap::new(), + }), + McpServerStatusDetail::ToolsAndAuthOnly, + ); + + let cell = match app_event_rx.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => cell, + other => panic!("expected MCP inventory history cell, got {other:?}"), + }; + let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 120)); + assert!(rendered.contains("No MCP servers configured.")); + assert!(!rendered.contains("local-only")); +} + +#[tokio::test] +async fn handle_mcp_inventory_result_uses_server_config_when_status_inventory_is_empty() { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let server_config: codex_config::types::McpServerConfig = + toml::from_str("command = \"server-only\"").expect("valid test MCP config"); + + app.handle_mcp_inventory_result( + Ok(crate::app_event::McpInventory { + statuses: Vec::new(), + configured_servers: HashMap::from([("remote_only".to_string(), server_config)]), + }), + McpServerStatusDetail::Full, + ); + + let cell = match app_event_rx.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => cell, + other => panic!("expected MCP inventory history cell, got {other:?}"), + }; + let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 120)); + assert!(!rendered.contains("No MCP servers configured.")); + assert!(rendered.contains("remote_only")); + assert!(rendered.contains("Command: server-only")); +} + #[test] fn bypass_hook_trust_startup_warning_snapshot() { let rendered = lines_to_single_string( diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 57ec371749..28fe15f5aa 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -8,6 +8,7 @@ //! Exit is modelled explicitly via `AppEvent::Exit(ExitMode)` so callers can request shutdown-first //! quits without reaching into the app loop or coupling to shutdown/exit sequencing. +use std::collections::HashMap; use std::path::PathBuf; use codex_app_server_protocol::AddCreditsNudgeCreditType; @@ -39,6 +40,7 @@ use crate::bottom_pane::TerminalTitleItem; use crate::chatwidget::UserMessage; use codex_app_server_protocol::AskForApproval; use codex_config::types::ApprovalsReviewer; +use codex_config::types::McpServerConfig; use codex_features::Feature; use codex_plugin::PluginCapabilitySummary; use codex_protocol::config_types::CollaborationModeMask; @@ -108,6 +110,12 @@ pub(crate) struct ConnectorsSnapshot { pub(crate) connectors: Vec, } +#[derive(Debug)] +pub(crate) struct McpInventory { + pub(crate) statuses: Vec, + pub(crate) configured_servers: HashMap, +} + /// Distinguishes why a rate-limit refresh was requested so the completion /// handler can route the result correctly. /// @@ -538,7 +546,7 @@ pub(crate) enum AppEvent { /// Result of fetching MCP inventory via app-server RPCs. McpInventoryLoaded { - result: Result, String>, + result: Result, detail: McpServerStatusDetail, }, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index d420162914..5e1e88468b 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1356,10 +1356,6 @@ impl ChatWidget { } fn plugins_for_mentions(&self) -> Option<&[PluginCapabilitySummary]> { - if !self.config.features.enabled(Feature::Plugins) { - return None; - } - self.bottom_pane.plugins().map(Vec::as_slice) } @@ -1730,11 +1726,6 @@ impl ChatWidget { } pub(crate) fn refresh_plugin_mentions(&mut self) { - if !self.config.features.enabled(Feature::Plugins) { - self.bottom_pane.set_plugin_mentions(/*plugins*/ None); - return; - } - self.app_event_tx.send(AppEvent::RefreshPluginMentions); } diff --git a/codex-rs/tui/src/chatwidget/plugins.rs b/codex-rs/tui/src/chatwidget/plugins.rs index 746f42fce9..d67b05a912 100644 --- a/codex-rs/tui/src/chatwidget/plugins.rs +++ b/codex-rs/tui/src/chatwidget/plugins.rs @@ -34,7 +34,6 @@ use codex_app_server_protocol::PluginReadResponse; use codex_app_server_protocol::PluginSummary; use codex_app_server_protocol::PluginUninstallResponse; use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME; -use codex_features::Feature; use codex_utils_absolute_path::AbsolutePathBuf; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -169,14 +168,6 @@ pub(super) enum PluginsCacheState { impl ChatWidget { pub(crate) fn add_plugins_output(&mut self) { - if !self.config.features.enabled(Feature::Plugins) { - self.add_info_message( - "Plugins are disabled.".to_string(), - Some("Enable the plugins feature to use /plugins.".to_string()), - ); - return; - } - self.plugins_active_tab_id = Some(ALL_PLUGINS_TAB_ID.to_string()); self.prefetch_plugins(); diff --git a/codex-rs/tui/src/history_cell/mcp.rs b/codex-rs/tui/src/history_cell/mcp.rs index 435cc6164f..bc2b7f8b82 100644 --- a/codex-rs/tui/src/history_cell/mcp.rs +++ b/codex-rs/tui/src/history_cell/mcp.rs @@ -1,6 +1,7 @@ //! MCP tool-call, inventory, and output history cells. use super::*; +use codex_config::McpServerConfig; #[derive(Debug)] struct CompletedMcpToolCallWithImageOutput { @@ -511,15 +512,15 @@ pub(crate) fn new_mcp_tools_output( /// Build the `/mcp` history cell from app-server `McpServerStatus` responses. /// -/// 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. +/// The server list comes from app-server status plus app-server config, sorted alphabetically. +/// Config enriches returned servers with transport details such as command, URL, cwd, and +/// environment display, and keeps configured-only servers visible when status inventory is empty. /// /// This mirrors the layout of [`new_mcp_tools_output`] but sources data from /// the paginated RPC response rather than the in-process `McpManager`. The /// `detail` flag controls whether resources and resource templates are rendered. pub(crate) fn new_mcp_tools_output_from_statuses( - config: &Config, + configured_servers: &HashMap, statuses: &[McpServerStatus], detail: McpServerStatusDetail, ) -> PlainHistoryCell { @@ -536,6 +537,12 @@ pub(crate) fn new_mcp_tools_output_from_statuses( } let mut server_names: Vec = statuses.iter().map(|status| status.name.clone()).collect(); + server_names.extend( + configured_servers + .keys() + .filter(|server| !statuses_by_name.contains_key(server.as_str())) + .cloned(), + ); server_names.sort(); let has_any_tools = statuses.iter().any(|status| !status.tools.is_empty()); @@ -545,7 +552,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 cfg = configured_servers.get(server.as_str()); let status = statuses_by_name.get(server.as_str()).copied(); let header: Vec> = vec![" • ".into(), server.clone().into()]; diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__mcp_tools_output_from_statuses_renders_configured_server_without_status.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__mcp_tools_output_from_statuses_renders_configured_server_without_status.snap new file mode 100644 index 0000000000..963883e5e8 --- /dev/null +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__mcp_tools_output_from_statuses_renders_configured_server_without_status.snap @@ -0,0 +1,17 @@ +--- +source: tui/src/history_cell/tests.rs +expression: rendered +--- +/mcp + +🔌 MCP Tools + + • No MCP tools available. + + • server_only + • Status: enabled + • Auth: Unsupported + • Command: server-only --stdio + • Tools: (none) + • Resources: (none) + • Resource templates: (none) diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index b0b87cdd00..f4c2734ae0 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -835,7 +835,7 @@ async fn mcp_tools_output_from_statuses_renders_status_only_servers() { }]; let cell = new_mcp_tools_output_from_statuses( - &config, + config.mcp_servers.get(), &statuses, McpServerStatusDetail::ToolsAndAuthOnly, ); @@ -892,7 +892,24 @@ async fn mcp_tools_output_from_statuses_renders_verbose_inventory() { auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, }]; - let cell = new_mcp_tools_output_from_statuses(&config, &statuses, McpServerStatusDetail::Full); + let cell = new_mcp_tools_output_from_statuses( + config.mcp_servers.get(), + &statuses, + McpServerStatusDetail::Full, + ); + let rendered = render_lines(&cell.display_lines(/*width*/ 120)).join("\n"); + + insta::assert_snapshot!(rendered); +} + +#[test] +fn mcp_tools_output_from_statuses_renders_configured_server_without_status() { + let servers = HashMap::from([( + "server_only".to_string(), + stdio_server_config("server-only", vec!["--stdio"], /*env*/ None, vec![]), + )]); + + let cell = new_mcp_tools_output_from_statuses(&servers, &[], McpServerStatusDetail::Full); let rendered = render_lines(&cell.display_lines(/*width*/ 120)).join("\n"); insta::assert_snapshot!(rendered);