Allow extensions to select MCP protocol mode per HTTP server (#44571)

## Why

Extension-owned HTTP MCP servers previously inherited the default protocol mode. Extensions need to select a mode for their own server independently of other HTTP servers.

## What changed

- Add `McpServerContribution::SetWithProtocolMode` and re-export `McpProtocolMode` through the extension API.
- Carry the winning registration's protocol override through catalog resolution and materialization, and apply it to Streamable HTTP connections.
- Preserve existing defaults when no override is present. Selecting a protocol mode does not grant host-owned Apps cache access or environment authority.

## Testing

Add coverage for registration precedence and materialization, and extend cache isolation tests to cover explicit protocol selection. Add an integration test verifying that extension servers can select either the legacy or newer protocol while other HTTP servers retain the default mode.

GitOrigin-RevId: 606cecc03094a93258ffc433849f575020b81473
This commit is contained in:
thomas
2026-09-10 16:02:59 +00:00
committed by copyberry
parent 287e4f7dbf
commit 6baa076eb6
10 changed files with 342 additions and 66 deletions
+20 -3
View File
@@ -10,6 +10,7 @@ use codex_protocol::mcp_policy::EnvironmentMcpPolicy;
use codex_utils_path_uri::PathUri;
use crate::CODEX_APPS_MCP_SERVER_NAME;
use crate::McpProtocolMode;
/// Plugin identity retained with an MCP registration for tool attribution.
#[derive(Clone, Debug, PartialEq, Eq)]
@@ -137,6 +138,7 @@ pub struct McpServerRegistration {
name: String,
source: McpServerSource,
config: McpServerConfig,
protocol_mode: Option<McpProtocolMode>,
precedence: RegistrationPrecedence,
}
@@ -209,6 +211,12 @@ impl McpServerRegistration {
)
}
/// Overrides the protocol for this registration if it wins HTTP server resolution.
pub fn with_protocol_mode(mut self, protocol_mode: McpProtocolMode) -> Self {
self.protocol_mode = Some(protocol_mode);
self
}
/// Registers the controller-owned Apps server contributed by a host extension.
pub fn from_hosted_apps(
id: impl Into<String>,
@@ -237,6 +245,7 @@ impl McpServerRegistration {
name,
source,
config,
protocol_mode: None,
precedence,
}
}
@@ -471,6 +480,7 @@ impl McpCatalogBuilder {
ResolvedMcpServer {
source: registration.source,
config: registration.config,
protocol_mode: registration.protocol_mode,
},
))
}
@@ -492,6 +502,7 @@ impl McpCatalogBuilder {
pub struct ResolvedMcpServer {
source: McpServerSource,
config: McpServerConfig,
protocol_mode: Option<McpProtocolMode>,
}
impl ResolvedMcpServer {
@@ -502,6 +513,10 @@ impl ResolvedMcpServer {
pub fn config(&self) -> &McpServerConfig {
&self.config
}
pub fn protocol_mode(&self) -> Option<McpProtocolMode> {
self.protocol_mode
}
}
/// Immutable result of MCP registration resolution.
@@ -547,8 +562,8 @@ impl ResolvedMcpCatalog {
pub fn with_materialized_servers(&self, servers: HashMap<String, McpServerConfig>) -> Self {
let mut builder = Self::builder();
for (name, config) in servers {
let source = self
.server(&name)
let previous = self.server(&name);
let source = previous
.map(|server| server.source.clone())
.unwrap_or(McpServerSource::Config);
let precedence = match &source {
@@ -560,7 +575,9 @@ impl ResolvedMcpCatalog {
McpServerSource::Compatibility { .. } => RegistrationPrecedence::Compatibility,
McpServerSource::Extension { .. } => RegistrationPrecedence::Extension(0),
};
builder.register(McpServerRegistration::new(name, source, config, precedence));
let mut registration = McpServerRegistration::new(name, source, config, precedence);
registration.protocol_mode = previous.and_then(ResolvedMcpServer::protocol_mode);
builder.register(registration);
}
builder.build()
}
+61
View File
@@ -13,6 +13,7 @@ use codex_utils_path_uri::PathUri;
use pretty_assertions::assert_eq;
use crate::CODEX_APPS_MCP_SERVER_NAME;
use crate::McpProtocolMode;
use super::McpEnvironmentAuthority;
use super::McpPluginAttribution;
@@ -224,6 +225,7 @@ fn disabled_winner_remains_a_veto_when_the_catalog_is_extended() {
Some(&super::ResolvedMcpServer {
source: extension_source("hosted"),
config: expected,
protocol_mode: None,
})
);
}
@@ -256,6 +258,7 @@ fn disabled_discovered_plugin_remains_a_veto_for_runtime_overlays() {
Some(&super::ResolvedMcpServer {
source: extension_source("hosted"),
config: expected,
protocol_mode: None,
})
);
}
@@ -328,6 +331,7 @@ fn selected_plugins_override_discovered_plugins_but_not_config() {
Some(&super::ResolvedMcpServer {
source: selected_plugin_source("selected-alpha"),
config: selected,
protocol_mode: None,
})
);
assert_eq!(
@@ -354,6 +358,7 @@ fn selected_plugins_override_discovered_plugins_but_not_config() {
Some(&super::ResolvedMcpServer {
source: selected_plugin_source("selected-alpha"),
config: refreshed,
protocol_mode: None,
})
);
@@ -370,6 +375,7 @@ fn selected_plugins_override_discovered_plugins_but_not_config() {
Some(&super::ResolvedMcpServer {
source: McpServerSource::Config,
config: configured,
protocol_mode: None,
})
);
}
@@ -401,6 +407,7 @@ fn disabled_selected_plugin_does_not_veto_runtime_overlays() {
Some(&super::ResolvedMcpServer {
source: extension_source("hosted"),
config: extension,
protocol_mode: None,
})
);
}
@@ -426,6 +433,7 @@ fn equal_precedence_uses_insertion_order_not_source_identity() {
Some(&super::ResolvedMcpServer {
source: compatibility_source("a-second"),
config: server("https://second.example/mcp"),
protocol_mode: None,
})
);
let mut builder = catalog.to_builder();
@@ -448,6 +456,59 @@ fn equal_precedence_uses_insertion_order_not_source_identity() {
);
}
#[test]
fn extension_protocol_mode_follows_the_winner_through_materialization() {
let config = server("https://apps.example/mcp");
let mut builder = ResolvedMcpCatalog::builder();
builder.register(
McpServerRegistration::from_extension(
"apps".to_string(),
"loser",
/*contribution_order*/ 0,
config.clone(),
)
.with_protocol_mode(McpProtocolMode::V20260728),
);
builder.register(McpServerRegistration::from_extension(
"apps".to_string(),
"winner",
/*contribution_order*/ 1,
config.clone(),
));
let without_override = builder.build();
assert_eq!(
without_override
.server("apps")
.and_then(ResolvedMcpServer::protocol_mode),
None
);
let mut builder = ResolvedMcpCatalog::builder();
builder.register(
McpServerRegistration::from_extension(
"apps".to_string(),
"winner",
/*contribution_order*/ 1,
config,
)
.with_protocol_mode(McpProtocolMode::Legacy),
);
let with_override = builder.build();
assert!(!without_override.has_same_servers(&with_override));
let refreshed = server("https://refreshed.example/mcp");
let materialized = with_override
.with_materialized_servers(HashMap::from([("apps".to_string(), refreshed.clone())]));
assert_eq!(
materialized.server("apps"),
Some(&ResolvedMcpServer {
source: extension_source("winner"),
config: refreshed,
protocol_mode: Some(McpProtocolMode::Legacy),
})
);
}
#[test]
fn environment_policy_exempts_only_explicitly_host_owned_apps() {
let policy = EnvironmentMcpPolicy {
+11 -6
View File
@@ -314,12 +314,17 @@ impl McpConnectionSet {
};
let metadata = McpServerMetadata::from(&server);
let configured_config = server.config().clone();
let protocol_mode = if is_host_owned_codex_apps
&& matches!(
&configured_config.transport,
McpServerTransportConfig::StreamableHttp { .. }
) {
host_owned_apps_protocol_mode
let protocol_mode = if matches!(
&configured_config.transport,
McpServerTransportConfig::StreamableHttp { .. }
) {
registration
.and_then(crate::ResolvedMcpServer::protocol_mode)
.unwrap_or(if is_host_owned_codex_apps {
host_owned_apps_protocol_mode
} else {
default_protocol_mode
})
} else {
default_protocol_mode
};
@@ -2099,65 +2099,81 @@ async fn codex_apps_extension_does_not_share_host_owned_tools_cache() -> anyhow:
let server_config: McpServerConfig =
serde_json::from_value(serde_json::json!({ "url": "http://127.0.0.1:1" }))?;
let mut config = crate::mcp::tests::test_mcp_config(codex_home.path().to_path_buf());
config.host_owned_apps_protocol_mode = crate::McpProtocolMode::V20260728;
let mut catalog = crate::ResolvedMcpCatalog::builder();
catalog.register(crate::McpServerRegistration::from_extension(
CODEX_APPS_MCP_SERVER_NAME.to_string(),
"test-extension",
/*contribution_order*/ 0,
server_config.clone(),
));
config.mcp_server_catalog = catalog.build();
for (hosted_mode, extension_mode, expected_mode) in [
(
crate::McpProtocolMode::V20260728,
None,
crate::McpProtocolMode::Legacy,
),
(
crate::McpProtocolMode::Legacy,
Some(crate::McpProtocolMode::V20260728),
crate::McpProtocolMode::V20260728,
),
] {
let mut config = crate::mcp::tests::test_mcp_config(codex_home.path().to_path_buf());
config.host_owned_apps_protocol_mode = hosted_mode;
let mut registration = crate::McpServerRegistration::from_extension(
CODEX_APPS_MCP_SERVER_NAME.to_string(),
"test-extension",
/*contribution_order*/ 0,
server_config.clone(),
);
if let Some(mode) = extension_mode {
registration = registration.with_protocol_mode(mode);
}
let mut catalog = crate::ResolvedMcpCatalog::builder();
catalog.register(registration);
config.mcp_server_catalog = catalog.build();
let startup_cancellation_token = CancellationToken::new();
startup_cancellation_token.cancel();
let manager = McpConnectionSet::new(
/*previous*/ None,
McpPublicationGate::already_published(),
McpRuntimeInput {
startup_policy: McpStartupPolicy::Eager,
config: Arc::new(config),
plugins_available: false,
ready_selected_capability_roots: Vec::new(),
mcp_servers: HashMap::from([(
CODEX_APPS_MCP_SERVER_NAME.to_string(),
EffectiveMcpServer::configured(server_config),
)]),
submit_id: "cache-ownership-test".to_string(),
tx_event: None,
startup_cancellation_token,
runtime_context: McpRuntimeContext::new(
Arc::new(environment_manager_without_environments()),
codex_home.path().to_path_buf(),
),
codex_apps_tools_cache,
tool_catalog_cache: McpToolCatalogCache::default(),
codex_apps_tools_cache_key: cache_key,
client_mcp_extensions: ClientMcpExtensions::default(),
auth: None,
auth_manager: None,
elicitation_reviewer: None,
elicitation_lifecycle: None,
},
ElicitationRequestRouter::default(),
)
.await;
let startup_cancellation_token = CancellationToken::new();
startup_cancellation_token.cancel();
let manager = McpConnectionSet::new(
/*previous*/ None,
McpPublicationGate::already_published(),
McpRuntimeInput {
startup_policy: McpStartupPolicy::Eager,
config: Arc::new(config),
plugins_available: false,
ready_selected_capability_roots: Vec::new(),
mcp_servers: HashMap::from([(
CODEX_APPS_MCP_SERVER_NAME.to_string(),
EffectiveMcpServer::configured(server_config.clone()),
)]),
submit_id: "cache-ownership-test".to_string(),
tx_event: None,
startup_cancellation_token,
runtime_context: McpRuntimeContext::new(
Arc::new(environment_manager_without_environments()),
codex_home.path().to_path_buf(),
),
codex_apps_tools_cache: codex_apps_tools_cache.clone(),
tool_catalog_cache: McpToolCatalogCache::default(),
codex_apps_tools_cache_key: cache_key.clone(),
client_mcp_extensions: ClientMcpExtensions::default(),
auth: None,
auth_manager: None,
elicitation_reviewer: None,
elicitation_lifecycle: None,
},
ElicitationRequestRouter::default(),
)
.await;
let client = manager.test_client(CODEX_APPS_MCP_SERVER_NAME);
assert_eq!(
manager.servers[CODEX_APPS_MCP_SERVER_NAME].protocol_mode,
crate::McpProtocolMode::Legacy,
"an ordinary extension named codex_apps must not inherit the hosted protocol default"
);
assert!(
client.codex_apps_tools_cache_context.is_none(),
"an extension must not receive the host-owned Apps cache"
);
assert!(
!client.has_cached_tools(),
"an extension must not expose cached host-owned Apps tools"
);
let client = manager.test_client(CODEX_APPS_MCP_SERVER_NAME);
assert_eq!(
manager.servers[CODEX_APPS_MCP_SERVER_NAME].protocol_mode, expected_mode,
"an ordinary extension must use its own mode, not the hosted protocol default"
);
assert!(
client.codex_apps_tools_cache_context.is_none(),
"an extension must not receive the host-owned Apps cache"
);
assert!(
!client.has_cached_tools(),
"an extension must not expose cached host-owned Apps tools"
);
}
Ok(())
}
+15
View File
@@ -179,6 +179,21 @@ impl McpManager {
),
)));
}
McpServerContribution::SetWithProtocolMode {
name,
config,
protocol_mode,
} => {
overlays.push(OrderedMcpOverlay::Set(Box::new(
McpServerRegistration::from_extension(
name,
contributor.id(),
contribution_order,
*config,
)
.with_protocol_mode(protocol_mode),
)));
}
McpServerContribution::HostedApps { config } => {
overlays.push(OrderedMcpOverlay::Set(Box::new(
McpServerRegistration::from_hosted_apps(
@@ -0,0 +1,151 @@
//! Verifies that an extension can choose MCP protocol mode for its own HTTP server.
use std::collections::HashMap;
use std::sync::Arc;
use codex_config::McpServerConfig;
use codex_core::config::Config;
use codex_extension_api::ExtensionFuture;
use codex_extension_api::ExtensionRegistryBuilder;
use codex_extension_api::McpProtocolMode;
use codex_extension_api::McpServerContribution;
use codex_extension_api::McpServerContributionContext;
use codex_extension_api::McpServerContributor;
use codex_features::Feature;
use core_test_support::apps_test_server::AppsTestServer;
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_mcp_server;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serde_json::json;
use wiremock::MockServer;
struct AppsExtensionEndpoint {
url: String,
protocol_mode: McpProtocolMode,
}
impl McpServerContributor<Config> for AppsExtensionEndpoint {
fn id(&self) -> &'static str {
"apps_protocol_test"
}
fn contribute<'a>(
&'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,
}]
})
}
}
async fn mcp_methods(server: &MockServer) -> Vec<String> {
server
.received_requests()
.await
.expect("mock server should capture MCP startup requests")
.into_iter()
.filter(|request| request.url.path() == "/api/codex/ps/mcp")
.filter_map(|request| {
let body: Value = serde_json::from_slice(&request.body).ok()?;
body.get("method")?.as_str().map(str::to_string)
})
.collect()
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn extension_protocol_mode_does_not_change_other_http_servers() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
for (generic_mode, extension_mode) in [
(McpProtocolMode::Legacy, McpProtocolMode::V20260728),
(McpProtocolMode::V20260728, McpProtocolMode::Legacy),
] {
let responses_server = responses::start_mock_server().await;
let extension_server = responses::start_mock_server().await;
let extension_url = format!(
"{}/api/codex/ps/mcp",
AppsTestServer::mount(&extension_server)
.await?
.chatgpt_base_url
);
let third_party_server = responses::start_mock_server().await;
let third_party_url = format!(
"{}/api/codex/ps/mcp",
AppsTestServer::mount(&third_party_server)
.await?
.chatgpt_base_url
);
let mut extensions = ExtensionRegistryBuilder::new();
extensions.mcp_server_contributor(Arc::new(AppsExtensionEndpoint {
url: extension_url,
protocol_mode: extension_mode,
}));
let fixture = test_codex()
.with_extensions(Arc::new(extensions.build()))
.with_config(move |config| {
config
.features
.disable(Feature::Apps)
.expect("test config should disable hosted Apps");
if generic_mode == McpProtocolMode::V20260728 {
config
.features
.enable(Feature::Mcp20260728)
.expect("test config should enable generic MCP protocol");
} else {
config
.features
.disable(Feature::Mcp20260728)
.expect("test config should disable generic MCP protocol");
}
let third_party: McpServerConfig =
serde_json::from_value(json!({ "url": third_party_url }))
.expect("third-party MCP config");
config
.mcp_servers
.set(HashMap::from([("third_party".to_string(), third_party)]))
.expect("test config should accept MCP server");
})
.build_with_auto_env(&responses_server)
.await?;
wait_for_mcp_server(&fixture.codex, "extension_apps").await?;
let legacy = vec!["initialize", "notifications/initialized", "tools/list"];
let modern = vec![
"server/discover",
"initialize",
"notifications/initialized",
"tools/list",
];
assert_eq!(
mcp_methods(&extension_server).await,
if extension_mode == McpProtocolMode::V20260728 {
modern.clone()
} else {
legacy.clone()
}
);
assert_eq!(
mcp_methods(&third_party_server).await,
if generic_mode == McpProtocolMode::V20260728 {
modern
} else {
legacy
}
);
}
Ok(())
}
+1
View File
@@ -100,6 +100,7 @@ mod json_result;
mod live_cli;
mod mcp_auth_elicitation;
mod mcp_auth_refresh;
mod mcp_extension_protocol;
mod mcp_optional_startup_grace;
#[cfg(unix)]
mod mcp_refresh_cleanup;
@@ -135,6 +135,14 @@ pub enum McpServerContribution {
name: String,
config: Box<McpServerConfig>,
},
/// Adds an ordinary extension-owned server with its own HTTP protocol mode.
/// The mode applies only if this registration wins server resolution; it
/// does not grant controller-owned Apps cache or environment authority.
SetWithProtocolMode {
name: String,
config: Box<McpServerConfig>,
protocol_mode: crate::McpProtocolMode,
},
/// Registers the controller-owned Apps server under its reserved name.
HostedApps { config: Box<McpServerConfig> },
/// Registers a server declared by a plugin selected for this thread.
+1
View File
@@ -21,6 +21,7 @@ pub use capabilities::NoopResponseItemInjector;
pub use capabilities::ResponseItemInjectionFuture;
pub use capabilities::ResponseItemInjector;
pub use codex_context_fragments::ContextualUserFragment;
pub use codex_mcp::McpProtocolMode;
pub use codex_mcp::ToolInfo as McpToolInfo;
pub use codex_protocol::models::ContentItemKind;
pub use codex_protocol::models::ResponseItem;
@@ -367,6 +367,7 @@ async fn selected_plugin_contributions(
}),
McpServerContribution::SelectedPluginPackage { .. } => None,
McpServerContribution::Set { .. }
| McpServerContribution::SetWithProtocolMode { .. }
| McpServerContribution::HostedApps { .. }
| McpServerContribution::Remove { .. } => {
panic!("expected selected plugin contribution")