mirror of
https://github.com/openai/codex.git
synced 2026-11-01 19:33:35 +00:00
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
This commit is contained in:
@@ -66,6 +66,7 @@ impl McpServerContributor<Config> for HostedVerificationServer {
|
||||
Box::pin(async move {
|
||||
vec![McpServerContribution::HostedApps {
|
||||
config: Box::new(self.0.clone()),
|
||||
protocol_mode: None,
|
||||
}]
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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()),
|
||||
|
||||
@@ -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) => {}
|
||||
|
||||
@@ -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))),
|
||||
});
|
||||
|
||||
@@ -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<Config> for AppsExtensionEndpoint {
|
||||
fn id(&self) -> &'static str {
|
||||
@@ -36,16 +51,7 @@ impl McpServerContributor<Config> for AppsExtensionEndpoint {
|
||||
&'a self,
|
||||
_context: McpServerContributionContext<'a, Config>,
|
||||
) -> ExtensionFuture<'a, Vec<McpServerContribution>> {
|
||||
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(())
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
}]
|
||||
})
|
||||
}
|
||||
|
||||
@@ -161,7 +161,10 @@ impl McpServerContributor<Config> 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(),
|
||||
|
||||
@@ -144,7 +144,11 @@ pub enum McpServerContribution {
|
||||
protocol_mode: crate::McpProtocolMode,
|
||||
},
|
||||
/// Registers the controller-owned Apps server under its reserved name.
|
||||
HostedApps { config: Box<McpServerConfig> },
|
||||
HostedApps {
|
||||
config: Box<McpServerConfig>,
|
||||
/// Overrides the HTTP protocol mode, or uses the hosted Apps default when absent.
|
||||
protocol_mode: Option<crate::McpProtocolMode>,
|
||||
},
|
||||
/// Registers a server declared by a plugin selected for this thread.
|
||||
SelectedPlugin {
|
||||
name: String,
|
||||
|
||||
@@ -44,6 +44,7 @@ impl McpServerContributor<Config> for HostedPluginRuntimeExtension {
|
||||
config.apps_mcp_product_sku.as_deref(),
|
||||
context.originator(),
|
||||
)),
|
||||
protocol_mode: None,
|
||||
}]
|
||||
})
|
||||
}
|
||||
|
||||
@@ -31,7 +31,13 @@ async fn hosted_plugin_runtime_forwards_thread_originator() -> Result<(), Box<dy
|
||||
/*executor_capability_discovery*/ None,
|
||||
))
|
||||
.await;
|
||||
let [McpServerContribution::HostedApps { config: server }] = contributions.as_slice() else {
|
||||
let [
|
||||
McpServerContribution::HostedApps {
|
||||
config: server,
|
||||
protocol_mode: None,
|
||||
},
|
||||
] = contributions.as_slice()
|
||||
else {
|
||||
panic!("hosted plugin runtime should contribute one server");
|
||||
};
|
||||
let McpServerTransportConfig::StreamableHttp { http_headers, .. } = &server.transport else {
|
||||
|
||||
Reference in New Issue
Block a user