diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 1b02e4bb6c..b61b0fd3fb 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -5759,20 +5759,24 @@ impl CodexMessageProcessor { self.clear_plugin_related_caches(); - let plugin_mcp_servers = load_plugin_mcp_servers(result.installed_path.as_path()); + let plugin_apps = load_plugin_apps(result.installed_path.as_path()); - if !plugin_mcp_servers.is_empty() { - if let Err(err) = self.queue_mcp_server_refresh_for_config(&config).await { - warn!( - plugin = result.plugin_id.as_key(), - "failed to queue MCP refresh after plugin install: {err:?}" - ); + if plugin_apps.is_empty() { + let plugin_mcp_servers = + load_plugin_mcp_servers(result.installed_path.as_path()); + + if !plugin_mcp_servers.is_empty() { + if let Err(err) = self.queue_mcp_server_refresh_for_config(&config).await { + warn!( + plugin = result.plugin_id.as_key(), + "failed to queue MCP refresh after plugin install: {err:?}" + ); + } + self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers) + .await; } - self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers) - .await; } - let plugin_apps = load_plugin_apps(result.installed_path.as_path()); let apps_needing_auth = if plugin_apps.is_empty() || !config.features.apps_enabled(Some(&self.auth_manager)).await { diff --git a/codex-rs/app-server/tests/suite/v2/plugin_install.rs b/codex-rs/app-server/tests/suite/v2/plugin_install.rs index 8c597d94a3..c9100f8f1d 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -602,6 +602,73 @@ async fn plugin_install_makes_bundled_mcp_servers_available_to_followup_requests Ok(()) } +#[tokio::test] +async fn plugin_install_skips_bundled_mcp_servers_for_plugins_with_apps() -> Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + "[features]\nplugins = true\n", + )?; + let repo_root = TempDir::new()?; + write_plugin_marketplace( + repo_root.path(), + "debug", + "sample-plugin", + "./sample-plugin", + None, + None, + )?; + write_plugin_source(repo_root.path(), "sample-plugin", &["alpha"])?; + std::fs::write( + repo_root.path().join("sample-plugin/.mcp.json"), + r#"{ + "mcpServers": { + "sample-mcp": { + "command": "echo" + } + } +}"#, + )?; + let marketplace_path = + AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_plugin_install_request(PluginInstallParams { + marketplace_path, + plugin_name: "sample-plugin".to_string(), + force_remote_sync: false, + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginInstallResponse = to_response(response)?; + assert_eq!(response.apps_needing_auth, Vec::::new()); + + let request_id = mcp + .send_raw_request( + "mcpServer/oauth/login", + Some(json!({ + "name": "sample-mcp", + })), + ) + .await?; + let err = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + + assert_eq!(err.error.code, -32600); + assert_eq!(err.error.message, "No MCP server named 'sample-mcp' found."); + Ok(()) +} + #[derive(Clone)] struct AppsServerState { response: Arc>, diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index 9987bbbb94..a4473a30dc 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -324,7 +324,11 @@ impl PluginLoadOutcome { pub fn effective_mcp_servers(&self) -> HashMap { let mut mcp_servers = HashMap::new(); - for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) { + for plugin in self + .plugins + .iter() + .filter(|plugin| plugin.is_active() && plugin.apps.is_empty()) + { for (name, config) in &plugin.mcp_servers { mcp_servers .entry(name.clone()) diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index c443433803..52460fcc25 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -177,13 +177,63 @@ fn load_plugins_loads_default_skills_and_mcp_servers() { outcome.effective_skill_roots(), vec![plugin_root.join("skills")] ); - assert_eq!(outcome.effective_mcp_servers().len(), 1); + assert!(outcome.effective_mcp_servers().is_empty()); assert_eq!( outcome.effective_apps(), vec![AppConnectorId("connector_example".to_string())] ); } +#[test] +fn effective_mcp_servers_skip_plugins_with_connectors_defined() { + let codex_home = TempDir::new().unwrap(); + let http_server = |url: &str| McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url: url.to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: 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, + }; + let plugin = |config_name: &str| LoadedPlugin { + config_name: config_name.to_string(), + manifest_name: Some(config_name.to_string()), + manifest_description: None, + root: AbsolutePathBuf::try_from(codex_home.path().join(config_name)).unwrap(), + enabled: true, + skill_roots: Vec::new(), + mcp_servers: HashMap::new(), + apps: Vec::new(), + error: None, + }; + + let outcome = PluginLoadOutcome::from_plugins(vec![ + LoadedPlugin { + mcp_servers: HashMap::from([("alpha".to_string(), http_server("https://alpha"))]), + apps: vec![AppConnectorId("connector_example".to_string())], + ..plugin("alpha@test") + }, + LoadedPlugin { + mcp_servers: HashMap::from([("beta".to_string(), http_server("https://beta"))]), + ..plugin("beta@test") + }, + ]); + + assert_eq!( + outcome.effective_mcp_servers(), + HashMap::from([("beta".to_string(), http_server("https://beta"))]) + ); +} + #[test] fn plugin_telemetry_metadata_uses_default_mcp_config_path() { let codex_home = TempDir::new().unwrap();