diff --git a/codex-rs/core-plugins/src/app_mcp_routing.rs b/codex-rs/core-plugins/src/app_mcp_routing.rs new file mode 100644 index 000000000..18cacb414 --- /dev/null +++ b/codex-rs/core-plugins/src/app_mcp_routing.rs @@ -0,0 +1,32 @@ +use codex_app_server_protocol::AuthMode; +use codex_plugin::AppDeclaration; +use std::collections::HashMap; +use std::collections::HashSet; + +pub fn apps_route_available(auth_mode: Option) -> bool { + auth_mode.is_some_and(AuthMode::uses_codex_backend) +} + +pub(crate) fn apply_app_mcp_routing_policy( + apps: &mut Vec, + mcp_servers: &mut HashMap, + auth_mode: Option, + plugin_active: bool, +) { + if !apps_route_available(auth_mode) { + apps.clear(); + return; + } + + if plugin_active && !apps.is_empty() { + let app_declaration_names = apps + .iter() + .map(|app| app.name.as_str()) + .collect::>(); + mcp_servers.retain(|name, _| !app_declaration_names.contains(name.as_str())); + } +} + +#[cfg(test)] +#[path = "app_mcp_routing_tests.rs"] +mod tests; diff --git a/codex-rs/core-plugins/src/app_mcp_routing_tests.rs b/codex-rs/core-plugins/src/app_mcp_routing_tests.rs new file mode 100644 index 000000000..d8050a4a6 --- /dev/null +++ b/codex-rs/core-plugins/src/app_mcp_routing_tests.rs @@ -0,0 +1,99 @@ +use super::*; +use codex_plugin::AppConnectorId; +use pretty_assertions::assert_eq; +use std::collections::HashMap; + +fn app(name: &str) -> AppDeclaration { + AppDeclaration { + name: name.to_string(), + connector_id: AppConnectorId(format!("connector_{name}")), + category: None, + } +} + +fn mcp_servers(mcp_servers: impl IntoIterator) -> HashMap { + mcp_servers + .into_iter() + .map(|(name, value)| (name.to_string(), value)) + .collect::>() +} + +fn sorted_app_names(apps: &[AppDeclaration]) -> Vec { + let mut names = apps.iter().map(|app| app.name.clone()).collect::>(); + names.sort(); + names +} + +fn sorted_mcp_server_names(mcp_servers: &HashMap) -> Vec { + let mut names = mcp_servers.keys().cloned().collect::>(); + names.sort(); + names +} + +#[test] +fn apps_route_available_tracks_auth_mode() { + assert!(apps_route_available(Some(AuthMode::Chatgpt))); + assert!(apps_route_available(Some(AuthMode::AgentIdentity))); + assert!(!apps_route_available(Some(AuthMode::ApiKey))); + assert!(!apps_route_available(/*auth_mode*/ None)); +} + +#[test] +fn app_mcp_routing_clears_apps_when_apps_route_is_unavailable() { + let mut apps = vec![app("linear")]; + let mut mcp_servers = mcp_servers([("linear", 1), ("docs", 2)]); + + apply_app_mcp_routing_policy( + &mut apps, + &mut mcp_servers, + Some(AuthMode::ApiKey), + /*plugin_active*/ true, + ); + + assert!(apps.is_empty()); + assert_eq!( + sorted_mcp_server_names(&mcp_servers), + vec!["docs".to_string(), "linear".to_string()] + ); +} + +#[test] +fn app_mcp_routing_preserves_apps_and_removes_conflicting_mcp_with_apps_route() { + let mut apps = vec![app("linear"), app("notion")]; + let mut mcp_servers = mcp_servers([("linear", 1), ("docs", 2), ("notion", 3)]); + + apply_app_mcp_routing_policy( + &mut apps, + &mut mcp_servers, + Some(AuthMode::Chatgpt), + /*plugin_active*/ true, + ); + + assert_eq!( + sorted_app_names(&apps), + vec!["linear".to_string(), "notion".to_string()] + ); + assert_eq!( + sorted_mcp_server_names(&mcp_servers), + vec!["docs".to_string()] + ); +} + +#[test] +fn app_mcp_routing_preserves_mcp_conflicts_when_plugin_is_inactive() { + let mut apps = vec![app("linear")]; + let mut mcp_servers = mcp_servers([("linear", 1), ("docs", 2)]); + + apply_app_mcp_routing_policy( + &mut apps, + &mut mcp_servers, + Some(AuthMode::Chatgpt), + /*plugin_active*/ false, + ); + + assert_eq!(sorted_app_names(&apps), vec!["linear".to_string()]); + assert_eq!( + sorted_mcp_server_names(&mcp_servers), + vec!["docs".to_string(), "linear".to_string()] + ); +} diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index da13b03aa..126f7b37e 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -1,3 +1,4 @@ +mod app_mcp_routing; mod discoverable; pub mod installed_marketplaces; pub mod loader; @@ -24,6 +25,7 @@ pub const OPENAI_BUNDLED_MARKETPLACE_NAME: &str = "openai-bundled"; pub type LoadedPlugin = codex_plugin::LoadedPlugin; pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome; +pub use app_mcp_routing::apps_route_available; pub use discoverable::ToolSuggestDiscoverablePlugin; pub use discoverable::ToolSuggestPluginDiscoveryInput; pub use loader::PluginHookLoadOutcome; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index ad7654765..f156f7848 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1,4 +1,6 @@ use crate::OPENAI_CURATED_MARKETPLACE_NAME; +use crate::app_mcp_routing::apply_app_mcp_routing_policy; +use crate::app_mcp_routing::apps_route_available; use crate::manifest::PluginManifestHooks; use crate::manifest::PluginManifestPaths; use crate::manifest::load_plugin_manifest; @@ -1085,20 +1087,17 @@ pub async fn load_plugin_mcp_servers( auth_mode: Option, ) -> HashMap { let mut mcp_servers = load_declared_plugin_mcp_servers(plugin_root).await; - if !auth_mode.is_some_and(AuthMode::uses_codex_backend) || mcp_servers.is_empty() { + if !apps_route_available(auth_mode) || mcp_servers.is_empty() { return mcp_servers; } - let app_declarations = load_plugin_apps(plugin_root).await; - if app_declarations.is_empty() { - return mcp_servers; - } - - let app_declaration_names = app_declarations - .iter() - .map(|app| app.name.as_str()) - .collect::>(); - mcp_servers.retain(|name, _| !app_declaration_names.contains(name.as_str())); + let mut app_declarations = load_plugin_apps(plugin_root).await; + apply_app_mcp_routing_policy( + &mut app_declarations, + &mut mcp_servers, + auth_mode, + /*plugin_active*/ true, + ); mcp_servers } diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index b973dc729..6d2022ae5 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -1,5 +1,6 @@ use super::PluginLoadOutcome; use crate::OPENAI_CURATED_MARKETPLACE_NAME; +use crate::app_mcp_routing::apply_app_mcp_routing_policy; use crate::installed_marketplaces::installed_marketplace_roots_from_layer_stack; use crate::loader::PluginHookLoadOutcome; use crate::loader::configured_curated_plugin_ids_from_codex_home; @@ -210,23 +211,15 @@ fn project_plugin_load_outcome_for_auth( outcome: PluginLoadOutcome, auth_mode: Option, ) -> PluginLoadOutcome { - let apps_route_available = auth_mode.is_some_and(AuthMode::uses_codex_backend); let mut plugins = outcome.plugins().to_vec(); for plugin in &mut plugins { - if apps_route_available { - if plugin.is_active() && !plugin.apps.is_empty() { - let app_declaration_names = plugin - .apps - .iter() - .map(|app| app.name.as_str()) - .collect::>(); - plugin - .mcp_servers - .retain(|name, _| !app_declaration_names.contains(name.as_str())); - } - } else { - plugin.apps.clear(); - } + let plugin_active = plugin.is_active(); + apply_app_mcp_routing_policy( + &mut plugin.apps, + &mut plugin.mcp_servers, + auth_mode, + plugin_active, + ); } PluginLoadOutcome::from_plugins(plugins) }