From 6d75b525eaf53a69521ff9041e9aed57c2059905 Mon Sep 17 00:00:00 2001 From: teddywyly-oai Date: Wed, 16 Sep 2026 20:18:07 +0000 Subject: [PATCH] Allow hosted Apps MCP contributions to override the protocol mode (#46019) ## What changed Add an optional `protocol_mode` to `McpServerContribution::HostedApps` and apply it when registering the server. Preserve the hosted Apps default when no override is provided. ## Testing Add an integration test that selects MCP `2026-07-28` with the global feature disabled and completes a native user verification flow, including resuming the tool call with the verification response. Extend capability tests to confirm that the override preserves hosted Apps user verification without granting it to ordinary extension registrations. GitOrigin-RevId: e8662c45f964c00b0bb4503bbcfd8432d7e2fa70 --- .../tests/suite/v2/user_verification_mcp.rs | 1 + .../src/client_capabilities_tests.rs | 22 +++ codex-rs/core/src/mcp.rs | 21 +- codex-rs/core/tests/suite/code_mode.rs | 2 + .../tests/suite/mcp_extension_protocol.rs | 186 ++++++++++++++++-- codex-rs/core/tests/suite/mcp_tool_cache.rs | 1 + .../core/tests/suite/mcp_tool_exposure.rs | 5 +- .../ext/extension-api/src/contributors/mcp.rs | 6 +- codex-rs/ext/mcp/src/lib.rs | 1 + codex-rs/ext/mcp/src/lib_tests.rs | 8 +- 10 files changed, 224 insertions(+), 29 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/user_verification_mcp.rs b/codex-rs/app-server/tests/suite/v2/user_verification_mcp.rs index 3d79a64710..86609db01b 100644 --- a/codex-rs/app-server/tests/suite/v2/user_verification_mcp.rs +++ b/codex-rs/app-server/tests/suite/v2/user_verification_mcp.rs @@ -66,6 +66,7 @@ impl McpServerContributor for HostedVerificationServer { Box::pin(async move { vec![McpServerContribution::HostedApps { config: Box::new(self.0.clone()), + protocol_mode: None, }] }) } diff --git a/codex-rs/codex-mcp/src/client_capabilities_tests.rs b/codex-rs/codex-mcp/src/client_capabilities_tests.rs index be07334992..17545a16e7 100644 --- a/codex-rs/codex-mcp/src/client_capabilities_tests.rs +++ b/codex-rs/codex-mcp/src/client_capabilities_tests.rs @@ -68,6 +68,7 @@ fn user_verification_is_projected_only_to_the_host_owned_plugin_service() { use crate::catalog::ResolvedMcpCatalog; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::codex_apps_mcp_server_config; + use codex_rmcp_client::McpProtocolMode; let config = codex_apps_mcp_server_config( "https://example.com", @@ -88,6 +89,27 @@ fn user_verification_is_projected_only_to_the_host_owned_plugin_service() { ), json!({"form": {}, "userVerification": {}}), ), + ( + CODEX_APPS_MCP_SERVER_NAME, + McpServerRegistration::from_hosted_apps( + "host", + /*contribution_order*/ 0, + config.clone(), + ) + .with_protocol_mode(McpProtocolMode::V20260728), + json!({"form": {}, "userVerification": {}}), + ), + ( + CODEX_APPS_MCP_SERVER_NAME, + McpServerRegistration::from_extension( + CODEX_APPS_MCP_SERVER_NAME.into(), + "ordinary", + /*contribution_order*/ 0, + config.clone(), + ) + .with_protocol_mode(McpProtocolMode::V20260728), + json!({"form": {}}), + ), ( CODEX_APPS_MCP_SERVER_NAME, McpServerRegistration::from_config(CODEX_APPS_MCP_SERVER_NAME.into(), config.clone()), diff --git a/codex-rs/core/src/mcp.rs b/codex-rs/core/src/mcp.rs index 9766373ee8..77c469f4da 100644 --- a/codex-rs/core/src/mcp.rs +++ b/codex-rs/core/src/mcp.rs @@ -197,14 +197,19 @@ impl McpManager { .with_protocol_mode(protocol_mode), ))); } - McpServerContribution::HostedApps { config } => { - overlays.push(OrderedMcpOverlay::Set(Box::new( - McpServerRegistration::from_hosted_apps( - contributor.id(), - contribution_order, - *config, - ), - ))); + McpServerContribution::HostedApps { + config, + protocol_mode, + } => { + let mut registration = McpServerRegistration::from_hosted_apps( + contributor.id(), + contribution_order, + *config, + ); + if let Some(protocol_mode) = protocol_mode { + registration = registration.with_protocol_mode(protocol_mode); + } + overlays.push(OrderedMcpOverlay::Set(Box::new(registration))); } McpServerContribution::SelectedPlugin { ref plugin_id, .. } if disabled_plugin_ids.contains(plugin_id) => {} diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index 5cf309d7fd..d003f1af72 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -1619,6 +1619,7 @@ async fn code_mode_mcp_metadata_keeps_originating_window_after_compaction() -> R /*apps_mcp_product_sku*/ None, /*originator*/ None, )), + protocol_mode: None, }), gate: Mutex::new(Some((reached_tx, release_rx))), }); @@ -2070,6 +2071,7 @@ async fn code_mode_result_metadata_keeps_prepared_call_binding_across_runtime_re /*apps_mcp_product_sku*/ None, /*originator*/ None, )), + protocol_mode: None, }), gate: Mutex::new(Some((reached_tx, release_rx))), }); diff --git a/codex-rs/core/tests/suite/mcp_extension_protocol.rs b/codex-rs/core/tests/suite/mcp_extension_protocol.rs index 93d69096b1..f57cdb3d50 100644 --- a/codex-rs/core/tests/suite/mcp_extension_protocol.rs +++ b/codex-rs/core/tests/suite/mcp_extension_protocol.rs @@ -2,8 +2,10 @@ use std::collections::HashMap; use std::sync::Arc; +use std::time::Duration; use codex_config::McpServerConfig; +use codex_core::StartThreadOptions; use codex_core::config::Config; use codex_extension_api::ExtensionFuture; use codex_extension_api::ExtensionRegistryBuilder; @@ -12,20 +14,33 @@ use codex_extension_api::McpServerContribution; use codex_extension_api::McpServerContributionContext; use codex_extension_api::McpServerContributor; use codex_features::Feature; +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; +use codex_protocol::approvals::ElicitationRequest; +use codex_protocol::mcp::ClientMcpExtensions; +use codex_protocol::mcp::OPENAI_ELICITATION_EXTENSION_ID; +use codex_protocol::protocol::ElicitationAction; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; use core_test_support::apps_test_server::AppsTestServer; +use core_test_support::apps_test_server::apps_enabled_builder; use core_test_support::responses; use core_test_support::skip_if_no_network; use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; use core_test_support::wait_for_mcp_server; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; +use tokio::time::timeout; +use tokio_util::task::AbortOnDropHandle; +use wiremock::Mock; use wiremock::MockServer; +use wiremock::Request; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; -struct AppsExtensionEndpoint { - url: String, - protocol_mode: McpProtocolMode, -} +struct AppsExtensionEndpoint(McpServerContribution); impl McpServerContributor for AppsExtensionEndpoint { fn id(&self) -> &'static str { @@ -36,16 +51,7 @@ impl McpServerContributor for AppsExtensionEndpoint { &'a self, _context: McpServerContributionContext<'a, Config>, ) -> ExtensionFuture<'a, Vec> { - Box::pin(async move { - vec![McpServerContribution::SetWithProtocolMode { - name: "extension_apps".to_string(), - config: Box::new( - serde_json::from_value(json!({ "url": self.url })) - .expect("extension Apps MCP config"), - ), - protocol_mode: self.protocol_mode, - }] - }) + Box::pin(async move { vec![self.0.clone()] }) } } @@ -87,10 +93,13 @@ async fn extension_protocol_mode_does_not_change_other_http_servers() -> anyhow: .chatgpt_base_url ); let mut extensions = ExtensionRegistryBuilder::new(); - extensions.mcp_server_contributor(Arc::new(AppsExtensionEndpoint { - url: extension_url, - protocol_mode: extension_mode, - })); + extensions.mcp_server_contributor(Arc::new(AppsExtensionEndpoint( + McpServerContribution::SetWithProtocolMode { + name: "extension_apps".to_string(), + config: Box::new(serde_json::from_value(json!({ "url": extension_url }))?), + protocol_mode: extension_mode, + }, + ))); let fixture = test_codex() .with_extensions(Arc::new(extensions.build())) @@ -149,3 +158,144 @@ async fn extension_protocol_mode_does_not_change_other_http_servers() -> anyhow: Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn hosted_apps_protocol_override_preserves_native_verification() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let responses_server = responses::start_mock_server().await; + let apps_server = responses::start_mock_server().await; + Mock::given(method("POST")) + .and(path("/api/codex/ps/mcp")) + .respond_with(|request: &Request| { + let body: Value = request.body_json().unwrap(); + let result = match body["method"].as_str() { + Some("server/discover") => json!({ + "resultType": "complete", + "supportedVersions": ["2026-07-28"], + "capabilities": {"tools": {}}, + "_meta": {"io.modelcontextprotocol/serverInfo": { + "name": "hosted-verification", "version": "1.0.0" + }}, + "ttlMs": 0, "cacheScope": "private" + }), + Some("tools/list") => json!({ + "resultType": "complete", + "tools": [{"name": "verify_action", "inputSchema": {"type": "object"}}] + }), + Some("tools/call") if body.pointer("/params/inputResponses").is_none() => { + assert_eq!( + body["params"]["_meta"]["io.modelcontextprotocol/clientCapabilities"] + ["extensions"][OPENAI_ELICITATION_EXTENSION_ID], + json!({"userVerification": {}}) + ); + json!({ + "resultType": "input_required", "content": [], "isError": false, + "inputRequests": {"verification": { + "method": "openai/elicitation/create", + "params": { + "mode": "openai/userVerification", "title": "Verify action", + "description": "Verify the requested operation", "challenge": "AQID" + } + }}, + "requestState": "verification-state" + }) + } + Some("tools/call") => { + assert_eq!(body["params"]["requestState"], "verification-state"); + assert_eq!(body["params"]["inputResponses"], json!({ + "verification": {"action": "accept", "content": { + "credentialId": "AQID", "signature": "BAUG" + }} + })); + json!({ + "resultType": "complete", + "content": [{"type": "text", "text": "verified"}], "isError": false + }) + } + other => panic!("unexpected MCP method: {other:?}"), + }; + ResponseTemplate::new(200).set_body_json(json!({ + "jsonrpc": "2.0", "id": body["id"], "result": result + })) + }) + .mount(&apps_server) + .await; + let mut extensions = ExtensionRegistryBuilder::new(); + extensions.mcp_server_contributor(Arc::new(AppsExtensionEndpoint( + McpServerContribution::HostedApps { + config: Box::new(serde_json::from_value(json!({ + "url": format!("{}/api/codex/ps/mcp", apps_server.uri()) + }))?), + protocol_mode: Some(McpProtocolMode::V20260728), + }, + ))); + let fixture = apps_enabled_builder(apps_server.uri()) + .with_extensions(Arc::new(extensions.build())) + .with_config(|config| { + config.features.disable(Feature::Mcp20260728).unwrap(); + }) + .build_with_auto_env(&responses_server) + .await?; + let thread = fixture + .thread_manager + .start_thread(StartThreadOptions { + client_mcp_extensions: ClientMcpExtensions::new([( + OPENAI_ELICITATION_EXTENSION_ID.to_string(), + json!({"userVerification": {}}), + )]), + environments: Some(vec![fixture.executor_environment().selection().clone()]), + ..StartThreadOptions::new(fixture.config.clone()) + }) + .await? + .thread; + let caller = Arc::clone(&thread); + let call = AbortOnDropHandle::new(tokio::spawn(async move { + caller + .call_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "verify_action", + /*arguments*/ None, + /*meta*/ None, + ) + .await + })); + let EventMsg::ElicitationRequest(request) = wait_for_event(&thread, |event| { + matches!(event, EventMsg::ElicitationRequest(_)) + }) + .await + else { + unreachable!() + }; + assert_eq!(request.server_name, CODEX_APPS_MCP_SERVER_NAME); + assert_eq!( + request.request, + ElicitationRequest::UserVerification { + title: "Verify action".into(), + description: "Verify the requested operation".into(), + challenge: "AQID".into(), + } + ); + assert!(!call.is_finished()); + thread + .submit(Op::ResolveElicitation { + server_name: request.server_name, + request_id: request.id, + decision: ElicitationAction::Accept, + content: Some(json!({"credentialId": "AQID", "signature": "BAUG"})), + meta: None, + }) + .await?; + assert_eq!( + timeout(Duration::from_secs(/*secs*/ 10), call).await???, + codex_protocol::mcp::CallToolResult { + content: vec![json!({"type": "text", "text": "verified"})], + structured_content: None, + is_error: Some(false), + meta: None, + }, + ); + thread.shutdown_and_wait().await?; + fixture.codex.shutdown_and_wait().await?; + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mcp_tool_cache.rs b/codex-rs/core/tests/suite/mcp_tool_cache.rs index d4549c4cf3..846049c627 100644 --- a/codex-rs/core/tests/suite/mcp_tool_cache.rs +++ b/codex-rs/core/tests/suite/mcp_tool_cache.rs @@ -309,6 +309,7 @@ async fn apps_cache_filled_during_binding_capture_reaches_the_model() -> anyhow: serde_json::from_value(json!({ "url": self.0 })) .expect("child Apps MCP config"), ), + protocol_mode: None, }] }) } diff --git a/codex-rs/core/tests/suite/mcp_tool_exposure.rs b/codex-rs/core/tests/suite/mcp_tool_exposure.rs index 3b3016f6e1..4366e9fc85 100644 --- a/codex-rs/core/tests/suite/mcp_tool_exposure.rs +++ b/codex-rs/core/tests/suite/mcp_tool_exposure.rs @@ -161,7 +161,10 @@ impl McpServerContributor for AppsMcpServerContributor { .expect("test Apps MCP server config should be valid"), ); let contribution = if self.id == "hosted_plugin_runtime" { - McpServerContribution::HostedApps { config } + McpServerContribution::HostedApps { + config, + protocol_mode: None, + } } else { McpServerContribution::Set { name: CODEX_APPS_MCP_SERVER_NAME.to_string(), diff --git a/codex-rs/ext/extension-api/src/contributors/mcp.rs b/codex-rs/ext/extension-api/src/contributors/mcp.rs index aa3afdc41a..374dd77f8f 100644 --- a/codex-rs/ext/extension-api/src/contributors/mcp.rs +++ b/codex-rs/ext/extension-api/src/contributors/mcp.rs @@ -144,7 +144,11 @@ pub enum McpServerContribution { protocol_mode: crate::McpProtocolMode, }, /// Registers the controller-owned Apps server under its reserved name. - HostedApps { config: Box }, + HostedApps { + config: Box, + /// Overrides the HTTP protocol mode, or uses the hosted Apps default when absent. + protocol_mode: Option, + }, /// Registers a server declared by a plugin selected for this thread. SelectedPlugin { name: String, diff --git a/codex-rs/ext/mcp/src/lib.rs b/codex-rs/ext/mcp/src/lib.rs index 2e9d011c83..802c4a7080 100644 --- a/codex-rs/ext/mcp/src/lib.rs +++ b/codex-rs/ext/mcp/src/lib.rs @@ -44,6 +44,7 @@ impl McpServerContributor for HostedPluginRuntimeExtension { config.apps_mcp_product_sku.as_deref(), context.originator(), )), + protocol_mode: None, }] }) } diff --git a/codex-rs/ext/mcp/src/lib_tests.rs b/codex-rs/ext/mcp/src/lib_tests.rs index f2d8dbe562..a6c526acf5 100644 --- a/codex-rs/ext/mcp/src/lib_tests.rs +++ b/codex-rs/ext/mcp/src/lib_tests.rs @@ -31,7 +31,13 @@ async fn hosted_plugin_runtime_forwards_thread_originator() -> Result<(), Box