Merge branch 'main' of github.com:openai/codex into dev/mzeng/plugin_mcp_cleanup

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
                 {

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::<AppSummary>::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<StdMutex<serde_json::Value>>,

codex-rs/core/src/plugins/manager.rs

@@ -324,7 +324,11 @@ impl PluginLoadOutcome {
 
     pub fn effective_mcp_servers(&self) -> HashMap<String, McpServerConfig> {
         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())

codex-rs/core/src/plugins/manager_tests.rs

@@ -178,13 +178,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();