From 03f436d5aee1debad692cb54cbdbfe36d1b9c5eb Mon Sep 17 00:00:00 2001 From: Felix Xia Date: Wed, 10 Jun 2026 21:20:44 +0100 Subject: [PATCH 1/2] Gate plugin MCP surfaces by auth route --- codex-rs/codex-mcp/src/mcp/mod.rs | 15 +- codex-rs/codex-mcp/src/mcp/mod_tests.rs | 198 ++++++++++++++++++------ 2 files changed, 168 insertions(+), 45 deletions(-) diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index e6b17b14c356..273bafec9a2c 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -217,6 +217,17 @@ pub fn host_owned_codex_apps_enabled(config: &McpConfig, auth: Option<&CodexAuth config.apps_enabled && auth.is_some_and(CodexAuth::uses_codex_backend) } +fn plugin_owned_mcp_server_routes_to_app_surface(config: &McpConfig, server_name: &str) -> bool { + let Some(plugin_id) = config.plugin_ids_by_mcp_server_name.get(server_name) else { + return false; + }; + + config + .plugin_capability_summaries + .iter() + .any(|plugin| plugin.config_name == *plugin_id && !plugin.app_connector_ids.is_empty()) +} + pub fn configured_mcp_servers(config: &McpConfig) -> HashMap { config.configured_mcp_servers.clone() } @@ -241,7 +252,9 @@ pub fn effective_mcp_servers_from_configured( .into_iter() .map(|(name, server)| (name, EffectiveMcpServer::configured(server))) .collect::>(); - if !host_owned_codex_apps_enabled(config, auth) { + if host_owned_codex_apps_enabled(config, auth) { + servers.retain(|name, _| !plugin_owned_mcp_server_routes_to_app_surface(config, name)); + } else { servers.remove(CODEX_APPS_MCP_SERVER_NAME); } servers diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 767f20611ffb..170755bccfae 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -34,6 +34,57 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig { } } +fn streamable_mcp_server_config(url: &str) -> McpServerConfig { + McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url: url.to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }, + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), + enabled: true, + required: false, + supports_parallel_tool_calls: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + default_tools_approval_mode: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth: None, + oauth_resource: None, + tools: HashMap::new(), + } +} + +fn plugin_summary( + config_name: &str, + app_connector_ids: &[&str], + mcp_server_names: &[&str], +) -> PluginCapabilitySummary { + PluginCapabilitySummary { + config_name: config_name.to_string(), + display_name: config_name.to_string(), + app_connector_ids: app_connector_ids + .iter() + .map(|id| AppConnectorId((*id).to_string())) + .collect(), + mcp_server_names: mcp_server_names + .iter() + .map(|name| (*name).to_string()) + .collect(), + ..PluginCapabilitySummary::default() + } +} + +fn sorted_server_names(servers: &HashMap) -> Vec { + let mut names = servers.keys().cloned().collect::>(); + names.sort(); + names +} + #[test] fn qualified_mcp_tool_name_prefix_sanitizes_server_names_without_lowercasing() { assert_eq!( @@ -228,6 +279,107 @@ fn codex_apps_server_config_forwards_configured_product_sku_header() { } } +#[test] +fn effective_mcp_servers_routes_plugin_surfaces_by_auth() { + #[derive(Clone, Copy)] + enum AuthKind { + ChatGpt, + ApiKey, + } + + struct Case { + name: &'static str, + apps_enabled: bool, + auth: AuthKind, + plugin_owned_mcp: bool, + app_connector_ids: &'static [&'static str], + expected_server_names: &'static [&'static str], + } + + let cases = [ + Case { + name: "api key keeps plugin mcp and removes codex apps", + apps_enabled: true, + auth: AuthKind::ApiKey, + plugin_owned_mcp: true, + app_connector_ids: &["connector_sample"], + expected_server_names: &["plugin-mcp"], + }, + Case { + name: "chatgpt uses app route for dual-surface plugin", + apps_enabled: true, + auth: AuthKind::ChatGpt, + plugin_owned_mcp: true, + app_connector_ids: &["connector_sample"], + expected_server_names: &[CODEX_APPS_MCP_SERVER_NAME], + }, + Case { + name: "chatgpt keeps mcp-only plugin mcp", + apps_enabled: true, + auth: AuthKind::ChatGpt, + plugin_owned_mcp: true, + app_connector_ids: &[], + expected_server_names: &[CODEX_APPS_MCP_SERVER_NAME, "plugin-mcp"], + }, + Case { + name: "apps disabled keeps plugin mcp and removes codex apps", + apps_enabled: false, + auth: AuthKind::ChatGpt, + plugin_owned_mcp: true, + app_connector_ids: &["connector_sample"], + expected_server_names: &["plugin-mcp"], + }, + Case { + name: "chatgpt keeps user-configured mcp even with matching plugin metadata", + apps_enabled: true, + auth: AuthKind::ChatGpt, + plugin_owned_mcp: false, + app_connector_ids: &["connector_sample"], + expected_server_names: &[CODEX_APPS_MCP_SERVER_NAME, "plugin-mcp"], + }, + ]; + + for case in cases { + let codex_home = tempfile::tempdir().expect("tempdir"); + let mut config = test_mcp_config(codex_home.path().to_path_buf()); + config.apps_enabled = case.apps_enabled; + config.configured_mcp_servers.insert( + "plugin-mcp".to_string(), + streamable_mcp_server_config("https://plugin.example/mcp"), + ); + config.configured_mcp_servers.insert( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + codex_apps_mcp_server_config( + &config.chatgpt_base_url, + config.apps_mcp_product_sku.as_deref(), + ), + ); + if case.plugin_owned_mcp { + config + .plugin_ids_by_mcp_server_name + .insert("plugin-mcp".to_string(), "sample@personal".to_string()); + } + config.plugin_capability_summaries = vec![plugin_summary( + "sample@personal", + case.app_connector_ids, + &["plugin-mcp"], + )]; + + let auth = match case.auth { + AuthKind::ChatGpt => CodexAuth::create_dummy_chatgpt_auth_for_testing(), + AuthKind::ApiKey => CodexAuth::from_api_key("test-api-key"), + }; + let effective = effective_mcp_servers(&config, Some(&auth)); + + assert_eq!( + sorted_server_names(&effective), + case.expected_server_names, + "{}", + case.name + ); + } +} + #[tokio::test] async fn effective_mcp_servers_preserve_runtime_servers() { let codex_home = tempfile::tempdir().expect("tempdir"); @@ -237,53 +389,11 @@ async fn effective_mcp_servers_preserve_runtime_servers() { config.configured_mcp_servers.insert( "sample".to_string(), - McpServerConfig { - transport: McpServerTransportConfig::StreamableHttp { - url: "https://user.example/mcp".to_string(), - bearer_token_env_var: None, - http_headers: None, - env_http_headers: None, - }, - environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), - enabled: true, - required: false, - supports_parallel_tool_calls: false, - disabled_reason: None, - startup_timeout_sec: None, - tool_timeout_sec: None, - default_tools_approval_mode: None, - enabled_tools: None, - disabled_tools: None, - scopes: None, - oauth: None, - oauth_resource: None, - tools: HashMap::new(), - }, + streamable_mcp_server_config("https://user.example/mcp"), ); config.configured_mcp_servers.insert( "docs".to_string(), - McpServerConfig { - transport: McpServerTransportConfig::StreamableHttp { - url: "https://docs.example/mcp".to_string(), - bearer_token_env_var: None, - http_headers: None, - env_http_headers: None, - }, - environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), - enabled: true, - required: false, - supports_parallel_tool_calls: false, - disabled_reason: None, - startup_timeout_sec: None, - tool_timeout_sec: None, - default_tools_approval_mode: None, - enabled_tools: None, - disabled_tools: None, - scopes: None, - oauth: None, - oauth_resource: None, - tools: HashMap::new(), - }, + streamable_mcp_server_config("https://docs.example/mcp"), ); config.configured_mcp_servers.insert( CODEX_APPS_MCP_SERVER_NAME.to_string(), From 945e20ae93fe3cc71bb4ad260f1ee42d95dc584a Mon Sep 17 00:00:00 2001 From: Felix Xia Date: Wed, 10 Jun 2026 21:26:44 +0100 Subject: [PATCH 2/2] Cover auth-routed plugin tool visibility --- codex-rs/core/tests/suite/plugins.rs | 122 ++++++++++++++++++++++++--- 1 file changed, 109 insertions(+), 13 deletions(-) diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index 412b5edbbcad..4d3aaba46274 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -8,6 +8,7 @@ use std::time::Instant; use anyhow::Result; use codex_features::Feature; use codex_login::CodexAuth; +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use core_test_support::apps_test_server::AppsTestServer; @@ -138,6 +139,25 @@ async fn build_apps_enabled_plugin_test_codex( .expect("create new conversation")) } +async fn build_api_key_plugin_test_codex( + server: &MockServer, + codex_home: Arc, +) -> Result { + let mut builder = test_codex() + .with_home(codex_home) + .with_auth(CodexAuth::from_api_key("Test API Key")) + .with_config(move |config| { + config + .features + .enable(Feature::Apps) + .expect("test config should allow feature update"); + }); + Ok(builder + .build(server) + .await + .expect("create new conversation")) +} + fn tool_names(body: &serde_json::Value) -> Vec { body.get("tools") .and_then(serde_json::Value::as_array) @@ -230,7 +250,7 @@ async fn capability_sections_render_in_developer_message_in_order() -> Result<() } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn explicit_plugin_mentions_inject_plugin_guidance() -> Result<()> { +async fn explicit_plugin_mentions_use_apps_for_chatgpt_dual_surface_plugins() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; let apps_server = AppsTestServer::mount_with_connector_name(&server, "Google Calendar").await?; @@ -256,7 +276,7 @@ async fn explicit_plugin_mentions_inject_plugin_guidance() -> Result<()> { build_apps_enabled_plugin_test_codex(&server, codex_home, apps_server.chatgpt_base_url) .await?; let codex = Arc::clone(&test_codex.codex); - wait_for_mcp_server(&codex, "sample").await?; + wait_for_mcp_server(&codex, CODEX_APPS_MCP_SERVER_NAME).await?; codex .submit(Op::UserInput { @@ -281,10 +301,10 @@ async fn explicit_plugin_mentions_inject_plugin_guidance() -> Result<()> { "expected plugin skills guidance: {developer_messages:?}" ); assert!( - developer_messages + !developer_messages .iter() .any(|text| text.contains("MCP servers from this plugin")), - "expected visible plugin MCP guidance: {developer_messages:?}" + "expected plugin MCP guidance to be suppressed for ChatGPT auth: {developer_messages:?}" ); assert!( developer_messages @@ -300,16 +320,9 @@ async fn explicit_plugin_mentions_inject_plugin_guidance() -> Result<()> { .any(|name| name == "mcp__codex_apps__google_calendar"), "expected plugin app tools to become visible for this turn: {request_tools:?}" ); - let echo_tool = request - .tool_by_name("mcp__sample", "echo") - .expect("plugin MCP tool should be present"); - let echo_description = echo_tool - .get("description") - .and_then(serde_json::Value::as_str) - .expect("plugin MCP tool description should be present"); assert!( - echo_description.contains("This tool is part of plugin `sample`."), - "expected plugin MCP provenance in tool description: {echo_description:?}" + request.tool_by_name("mcp__sample", "echo").is_none(), + "expected plugin MCP tool to be suppressed for ChatGPT auth" ); let calendar_tool = request .tool_by_name("mcp__codex_apps__google_calendar", "_create_event") @@ -326,6 +339,89 @@ async fn explicit_plugin_mentions_inject_plugin_guidance() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn explicit_plugin_mentions_use_mcp_for_api_key_dual_surface_plugins() -> Result<()> { + skip_if_no_network!(Ok(())); + let server = start_mock_server().await; + let mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + + let codex_home = Arc::new(TempDir::new()?); + let rmcp_test_server_bin = match stdio_server_bin() { + Ok(bin) => bin, + Err(err) => { + eprintln!("test_stdio_server binary not available, skipping test: {err}"); + return Ok(()); + } + }; + write_plugin_skill_plugin(codex_home.as_ref()); + write_plugin_mcp_plugin(codex_home.as_ref(), &rmcp_test_server_bin); + write_plugin_app_plugin(codex_home.as_ref()); + + let test_codex = build_api_key_plugin_test_codex(&server, codex_home).await?; + let codex = Arc::clone(&test_codex.codex); + wait_for_mcp_server(&codex, "sample").await?; + + codex + .submit(Op::UserInput { + items: vec![codex_protocol::user_input::UserInput::Mention { + name: "sample".into(), + path: format!("plugin://{SAMPLE_PLUGIN_CONFIG_NAME}"), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request = mock.single_request(); + let developer_messages = request.message_input_texts("developer"); + assert!( + developer_messages + .iter() + .any(|text| text.contains("Skills from this plugin")), + "expected plugin skills guidance: {developer_messages:?}" + ); + assert!( + developer_messages + .iter() + .any(|text| text.contains("MCP servers from this plugin")), + "expected visible plugin MCP guidance: {developer_messages:?}" + ); + assert!( + !developer_messages + .iter() + .any(|text| text.contains("Apps from this plugin")), + "expected plugin app guidance to be suppressed for API-key auth: {developer_messages:?}" + ); + let request_body = request.body_json(); + let request_tools = tool_names(&request_body); + assert!( + !request_tools + .iter() + .any(|name| name == "mcp__codex_apps__google_calendar"), + "expected plugin app tools to be hidden for API-key auth: {request_tools:?}" + ); + let echo_tool = request + .tool_by_name("mcp__sample", "echo") + .expect("plugin MCP tool should be present"); + let echo_description = echo_tool + .get("description") + .and_then(serde_json::Value::as_str) + .expect("plugin MCP tool description should be present"); + assert!( + echo_description.contains("This tool is part of plugin `sample`."), + "expected plugin MCP provenance in tool description: {echo_description:?}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn explicit_plugin_mentions_track_plugin_used_analytics() -> Result<()> { skip_if_no_network!(Ok(()));