From 9cd11e9e62c48ea6127f8b9f6ae2e013e4e4ff00 Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 10 Jun 2026 12:54:21 +0200 Subject: [PATCH] Use plugin-service MCP as the hosted plugin runtime (#27198) ## Stack - Base: #27191 - This PR is the third vertical and should be reviewed against `jif/external-plugins-2`, not `main`. ## Why #27191 moves the host-owned Apps MCP registration behind an extension contributor, but deliberately preserves the existing endpoint-selection feature while that contribution contract lands. App-server can therefore resolve the server through extensions, yet the hosted plugin endpoint is still selected through temporary `apps_mcp_path_override` plumbing. That is not the long-term plugin model. A plugin can bundle skills, connectors, MCP servers, and hooks, and those components do not all need the same source or execution environment. In particular, an authenticated HTTP MCP server can expose plugin capabilities directly from a backend without an executor or an orchestrator filesystem. This PR completes that hosted vertical. App-server's MCP extension now owns the aggregate hosted plugin runtime at `/ps/mcp`. Connector actions continue to arrive as MCP tools, while backend-provided skills arrive as MCP resources and use Codex's existing resource list/read paths. No second backend client, skill filesystem, or generic plugin activation framework is introduced. The backend route remains the hosted implementation. This change replaces Codex's temporary endpoint-selection mechanism, not the service behind the endpoint. ## What changed ### Hosted plugin runtime The MCP extension now contributes `codex_apps` as the hosted plugin runtime rather than as a configurable Apps endpoint: - `https://chatgpt.com` resolves to `https://chatgpt.com/backend-api/ps/mcp`; - a bare custom ChatGPT base resolves to `/api/codex/ps/mcp`; - the existing product-SKU header and ChatGPT authentication behavior are preserved; - executor availability is never consulted for this streamable HTTP transport. The same MCP connection carries both component shapes supported by the hosted endpoint: - connector actions are discovered and invoked as MCP tools; - hosted skills are enumerated and read as MCP resources through the existing `list_mcp_resources` and `read_mcp_resource` paths. This keeps component access in the subsystem that already owns the protocol instead of downloading backend skills into an orchestrator filesystem or inventing a parallel hosted-skill client. ### Explicit runtime ordering `McpManager` now resolves the reserved `codex_apps` entry in three ordered phases: 1. install the legacy Apps fallback for compatibility; 2. apply ordered extension `Set` or `Remove` overlays; 3. apply the final ChatGPT-auth gate without synthesizing the server again. This ordering is important: - an ordinary configured or plugin MCP server cannot claim the auth-bearing `codex_apps` name; - an extension-contributed hosted runtime wins over the fallback; - an extension `Remove` remains authoritative; - a host without the MCP extension retains the legacy Apps endpoint and current local-only behavior. The temporary `legacy_apps_mcp_loader_enabled` coordination flag is no longer needed. ### Remove the path override The `apps_mcp_path_override` feature and its runtime plumbing are removed, including: - the feature registry entry and structured feature config; - `Config` and `McpConfig` fields; - config schema output; - config-lock materialization; - URL override handling in `codex-mcp`. Existing boolean and structured forms still deserialize as ignored compatibility input. They are omitted from new serialized config, and config-lock comparison normalizes the removed input so older locks remain replayable. ### App-server coverage App-server MCP fixtures now serve the hosted route at `/api/codex/ps/mcp`. Existing resource-read and tool/elicitation flows therefore exercise the extension-owned endpoint rather than succeeding through the legacy fallback. The stack also adds the missing `codex_chatgpt::connectors` re-export for the manager-backed connector helper introduced in #27191. ## Compatibility - App-server installs the extension and uses `/ps/mcp` for the hosted runtime. - CLI and other hosts that do not install the extension retain the legacy Apps endpoint. - Apps disabled or non-ChatGPT authentication removes `codex_apps` from the effective runtime view. - Existing local plugins, local skills, executor-selected skills, configured MCP servers, and MCP OAuth behavior are otherwise unchanged. - Backend plugin enablement remains account/workspace state owned by the hosted endpoint; this PR does not add thread-local backend plugin selection. ## Architectural fit The stack now proves two independent runtime shapes: 1. #27184 resolves filesystem-backed skills through the executor that owns a selected root. 2. #27191 and this PR resolve a backend-hosted HTTP MCP through an extension with no executor. Together they preserve the intended separation: - selection identifies a plugin/root when explicit selection is needed; - each component's owning extension resolves its concrete access mechanism; - execution stays with the runtime required by that component; - existing skills, MCP, connector, and hook subsystems remain the downstream consumers. ## Planned follow-ups 1. **Executor stdio MCP:** selecting an executor plugin registers a manifest-declared stdio MCP server and executes it in the environment that owns the plugin. 2. **Optional backend selection:** only if CCA needs thread-local selection distinct from backend account/workspace enablement, add a concrete backend-owned capability location and surface those selected skills through the skills catalog. 3. **Connector metadata and hooks:** activate those plugin components through their existing owning subsystems, with executor hooks remaining environment-bound. 4. **Propagation and persistence:** define explicit resume, fork, subagent, refresh, and environment-removal semantics once selected roots have multiple real consumers. 5. **Local convergence:** migrate legacy local skill, MCP, connector, and hook paths behind their owning extensions one vertical at a time, then remove duplicate core managers and compatibility plumbing after parity. ## Verification Coverage in this change exercises: - extension-owned `/backend-api/ps/mcp` registration without an executor; - preservation of the legacy endpoint in hosts without the extension; - extension `Set` and `Remove` precedence over the legacy fallback; - ChatGPT-auth gating for the reserved server; - hosted MCP resource reads with and without an active thread; - connector tool invocation and MCP elicitation through the hosted route; - ignored boolean and structured forms of the removed path override; - config-lock replay compatibility for the removed feature. `cargo check -p codex-features -p codex-mcp-extension -p codex-app-server` passes. Tests and Clippy were not run locally under the current development instruction; CI provides the full validation pass. --- .../app-server/tests/suite/v2/app_list.rs | 2 +- .../app-server/tests/suite/v2/mcp_resource.rs | 2 +- .../tests/suite/v2/mcp_server_elicitation.rs | 2 +- .../tests/suite/v2/plugin_install.rs | 2 +- .../app-server/tests/suite/v2/plugin_read.rs | 2 +- codex-rs/codex-mcp/src/lib.rs | 2 +- codex-rs/codex-mcp/src/mcp/mod.rs | 83 ++++++++---------- codex-rs/codex-mcp/src/mcp/mod_tests.rs | 83 ++++-------------- codex-rs/config/src/schema.rs | 29 ++++++- codex-rs/core/config.schema.json | 60 +++++++------ codex-rs/core/src/config/config_tests.rs | 83 ------------------ codex-rs/core/src/config/mod.rs | 24 ------ codex-rs/core/src/config_lock.rs | 3 + codex-rs/core/src/mcp.rs | 23 +++-- codex-rs/core/src/session/config_lock.rs | 31 +++++-- .../core/src/tools/handlers/mcp_resource.rs | 10 ++- .../list_mcp_resource_templates.rs | 2 +- .../mcp_resource/list_mcp_resources.rs | 2 +- .../mcp_resource/read_mcp_resource.rs | 2 +- .../src/tools/handlers/mcp_resource_tests.rs | 37 ++++++++ .../ext/extension-api/src/contributors/mcp.rs | 9 -- codex-rs/ext/mcp/src/lib.rs | 11 ++- codex-rs/ext/mcp/tests/hosted_apps_mcp.rs | 85 +++++++++++++++++-- codex-rs/features/src/feature_configs.rs | 16 +--- codex-rs/features/src/lib.rs | 32 +++---- codex-rs/features/src/tests.rs | 21 +++++ codex-rs/thread-manager-sample/src/main.rs | 1 - 27 files changed, 334 insertions(+), 325 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/app_list.rs b/codex-rs/app-server/tests/suite/v2/app_list.rs index c9615d3a2..ace43d284 100644 --- a/codex-rs/app-server/tests/suite/v2/app_list.rs +++ b/codex-rs/app-server/tests/suite/v2/app_list.rs @@ -1516,7 +1516,7 @@ async fn start_apps_server_with_delays_and_control_inner( get(workspace_settings_response), ) .with_state(state) - .nest_service("/api/codex/apps", mcp_service); + .nest_service("/api/codex/ps/mcp", mcp_service); let handle = tokio::spawn(async move { let _ = axum::serve(listener, router).await; diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs index d8ecb8899..2e3ffcb06 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -256,7 +256,7 @@ async fn start_resource_apps_mcp_server() -> Result<(String, JoinHandle<()>)> { Arc::new(LocalSessionManager::default()), StreamableHttpServerConfig::default(), ); - let router = Router::new().nest_service("/api/codex/apps", mcp_service); + let router = Router::new().nest_service("/api/codex/ps/mcp", mcp_service); let apps_server_handle = tokio::spawn(async move { let _ = axum::serve(listener, router).await; }); diff --git a/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs b/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs index 8d0387ee9..b4bd1d377 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs @@ -406,7 +406,7 @@ async fn start_apps_server() -> Result<(String, JoinHandle<()>)> { get(list_directory_connectors), ) .with_state(state) - .nest_service("/api/codex/apps", mcp_service); + .nest_service("/api/codex/ps/mcp", mcp_service); let handle = tokio::spawn(async move { let _ = axum::serve(listener, router).await; diff --git a/codex-rs/app-server/tests/suite/v2/plugin_install.rs b/codex-rs/app-server/tests/suite/v2/plugin_install.rs index 5ebe4899d..ca490ad5e 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -1289,7 +1289,7 @@ async fn start_apps_server( get(list_directory_connectors), ) .with_state(state) - .nest_service("/api/codex/apps", mcp_service); + .nest_service("/api/codex/ps/mcp", mcp_service); let handle = tokio::spawn(async move { let _ = axum::serve(listener, router).await; diff --git a/codex-rs/app-server/tests/suite/v2/plugin_read.rs b/codex-rs/app-server/tests/suite/v2/plugin_read.rs index e675a6172..4d8291a32 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_read.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_read.rs @@ -1914,7 +1914,7 @@ async fn start_apps_server( get(list_directory_connectors), ) .with_state(state) - .nest_service("/api/codex/apps", mcp_service); + .nest_service("/api/codex/ps/mcp", mcp_service); let handle = tokio::spawn(async move { let _ = axum::serve(listener, router).await; diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index a56365277..f0c7c3c8c 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -29,8 +29,8 @@ pub use mcp::configured_mcp_servers; pub use mcp::effective_mcp_servers; pub use mcp::effective_mcp_servers_from_configured; pub use mcp::host_owned_codex_apps_enabled; +pub use mcp::hosted_plugin_runtime_mcp_server_config; pub use mcp::tool_plugin_provenance; -pub use mcp::with_codex_apps_mcp; pub use mcp::McpServerStatusSnapshot; pub use mcp::McpSnapshotDetail; diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index adc2cf615..e6b17b14c 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -107,8 +107,6 @@ pub struct McpPermissionPromptAutoApproveContext { pub struct McpConfig { /// Base URL for ChatGPT-hosted app MCP servers, copied from the root config. pub chatgpt_base_url: String, - /// Optional path override for the host-owned apps MCP server. - pub apps_mcp_path_override: Option, /// Optional product SKU forwarded to the host-owned apps MCP server. pub apps_mcp_product_sku: Option, /// Codex home directory used for MCP OAuth state and app-tool cache files. @@ -129,22 +127,18 @@ pub struct McpConfig { pub use_legacy_landlock: bool, /// Whether the app MCP integration is enabled by config. /// - /// ChatGPT auth is checked separately at runtime before the host-owned apps - /// MCP server is added. + /// ChatGPT auth is checked separately before a materialized host-owned Apps + /// server can be used. pub apps_enabled: bool, - /// Whether to synthesize the legacy host-owned Apps MCP server. - /// - /// Hosts that install an MCP extension for this server disable the legacy - /// loader and contribute the server through the normal runtime overlay. - pub legacy_apps_mcp_loader_enabled: bool, /// Whether model-visible MCP tool namespaces should keep the legacy /// `mcp__` prefix. pub prefix_mcp_tool_names: bool, /// Client-side elicitation capabilities advertised during MCP initialization. pub client_elicitation_capability: ElicitationCapability, - /// Config-backed MCP servers keyed by server name. + /// Materialized MCP servers keyed by server name. /// - /// Runtime-only additions are merged later by [`effective_mcp_servers`]. + /// A host may add compatibility built-ins and extension overlays before + /// calling runtime entry points in this crate. pub configured_mcp_servers: HashMap, /// Winning plugin owner for plugin-provided MCP servers, keyed by server name. pub plugin_ids_by_mcp_server_name: HashMap, @@ -219,32 +213,6 @@ impl ToolPluginProvenance { } } -pub fn with_codex_apps_mcp( - mut servers: HashMap, - auth: Option<&CodexAuth>, - config: &McpConfig, -) -> HashMap { - if !config.legacy_apps_mcp_loader_enabled { - if !host_owned_codex_apps_enabled(config, auth) { - servers.remove(CODEX_APPS_MCP_SERVER_NAME); - } - return servers; - } - if host_owned_codex_apps_enabled(config, auth) { - servers.insert( - CODEX_APPS_MCP_SERVER_NAME.to_string(), - EffectiveMcpServer::configured(codex_apps_mcp_server_config( - &config.chatgpt_base_url, - config.apps_mcp_path_override.as_deref(), - config.apps_mcp_product_sku.as_deref(), - )), - ); - } else { - servers.remove(CODEX_APPS_MCP_SERVER_NAME); - } - servers -} - pub fn host_owned_codex_apps_enabled(config: &McpConfig, auth: Option<&CodexAuth>) -> bool { config.apps_enabled && auth.is_some_and(CodexAuth::uses_codex_backend) } @@ -260,16 +228,23 @@ pub fn effective_mcp_servers( effective_mcp_servers_from_configured(configured_mcp_servers(config), config, auth) } +/// Converts a materialized server map to its auth-gated runtime view. +/// +/// Compatibility built-ins and extension overlays must already be reflected in +/// `configured_servers`; this function does not synthesize missing servers. pub fn effective_mcp_servers_from_configured( configured_servers: HashMap, config: &McpConfig, auth: Option<&CodexAuth>, ) -> HashMap { - let servers = configured_servers + let mut servers = configured_servers .into_iter() .map(|(name, server)| (name, EffectiveMcpServer::configured(server))) .collect::>(); - with_codex_apps_mcp(servers, auth, config) + if !host_owned_codex_apps_enabled(config, auth) { + servers.remove(CODEX_APPS_MCP_SERVER_NAME); + } + servers } pub fn tool_plugin_provenance(config: &McpConfig) -> ToolPluginProvenance { @@ -441,7 +416,7 @@ fn normalize_codex_apps_base_url(base_url: &str) -> String { base_url } -fn codex_apps_mcp_url_for_base_url(base_url: &str, apps_mcp_path_override: Option<&str>) -> String { +fn codex_apps_mcp_url_for_base_url(base_url: &str) -> String { let base_url = normalize_codex_apps_base_url(base_url); let (base_url, default_path) = if base_url.contains("/backend-api") { (base_url, "wham/apps") @@ -450,18 +425,34 @@ fn codex_apps_mcp_url_for_base_url(base_url: &str, apps_mcp_path_override: Optio } else { (format!("{base_url}/api/codex"), "apps") }; - let path = apps_mcp_path_override - .unwrap_or(default_path) - .trim_start_matches('/'); - format!("{base_url}/{path}") + format!("{base_url}/{default_path}") } pub fn codex_apps_mcp_server_config( chatgpt_base_url: &str, - apps_mcp_path_override: Option<&str>, apps_mcp_product_sku: Option<&str>, ) -> McpServerConfig { - let url = codex_apps_mcp_url_for_base_url(chatgpt_base_url, apps_mcp_path_override); + mcp_server_config_for_url( + codex_apps_mcp_url_for_base_url(chatgpt_base_url), + apps_mcp_product_sku, + ) +} + +/// Builds the ChatGPT-hosted plugin runtime served by plugin-service. +pub fn hosted_plugin_runtime_mcp_server_config( + chatgpt_base_url: &str, + apps_mcp_product_sku: Option<&str>, +) -> McpServerConfig { + let base_url = normalize_codex_apps_base_url(chatgpt_base_url); + let base_url = if base_url.contains("/backend-api") || base_url.contains("/api/codex") { + base_url + } else { + format!("{base_url}/api/codex") + }; + mcp_server_config_for_url(format!("{base_url}/ps/mcp"), apps_mcp_product_sku) +} + +fn mcp_server_config_for_url(url: String, apps_mcp_product_sku: Option<&str>) -> McpServerConfig { let http_headers = apps_mcp_product_sku.map(|product_sku| { HashMap::from([("X-OpenAI-Product-Sku".to_string(), product_sku.to_string())]) }); diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index fb1af362c..767f20611 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -16,7 +16,6 @@ use std::path::PathBuf; fn test_mcp_config(codex_home: PathBuf) -> McpConfig { McpConfig { chatgpt_base_url: "https://chatgpt.com".to_string(), - apps_mcp_path_override: None, apps_mcp_product_sku: None, codex_home, mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode::default(), @@ -27,7 +26,6 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig { codex_linux_sandbox_exe: None, use_legacy_landlock: false, apps_enabled: false, - legacy_apps_mcp_loader_enabled: true, prefix_mcp_tool_names: true, client_elicitation_capability: ElicitationCapability::default(), configured_mcp_servers: HashMap::new(), @@ -178,52 +176,27 @@ fn tool_plugin_provenance_collects_app_and_mcp_sources() { #[test] fn codex_apps_mcp_url_for_base_url_keeps_existing_paths() { assert_eq!( - codex_apps_mcp_url_for_base_url( - "https://chatgpt.com/backend-api", - /*apps_mcp_path_override*/ None, - ), + codex_apps_mcp_url_for_base_url("https://chatgpt.com/backend-api"), "https://chatgpt.com/backend-api/wham/apps" ); assert_eq!( - codex_apps_mcp_url_for_base_url( - "https://chat.openai.com", - /*apps_mcp_path_override*/ None, - ), + codex_apps_mcp_url_for_base_url("https://chat.openai.com"), "https://chat.openai.com/backend-api/wham/apps" ); assert_eq!( - codex_apps_mcp_url_for_base_url( - "http://localhost:8080/api/codex", - /*apps_mcp_path_override*/ None, - ), + codex_apps_mcp_url_for_base_url("http://localhost:8080/api/codex"), "http://localhost:8080/api/codex/apps" ); assert_eq!( - codex_apps_mcp_url_for_base_url( - "http://localhost:8080", - /*apps_mcp_path_override*/ None, - ), + codex_apps_mcp_url_for_base_url("http://localhost:8080"), "http://localhost:8080/api/codex/apps" ); } #[test] fn codex_apps_server_config_uses_legacy_codex_apps_path() { - let mut config = test_mcp_config(PathBuf::from("/tmp")); - let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); - - let mut servers = with_codex_apps_mcp(HashMap::new(), /*auth*/ None, &config); - assert!(!servers.contains_key(CODEX_APPS_MCP_SERVER_NAME)); - - config.apps_enabled = true; - - servers = with_codex_apps_mcp(servers, Some(&auth), &config); - let server = servers - .get(CODEX_APPS_MCP_SERVER_NAME) - .expect("codex apps should be present when apps is enabled"); - let config = server - .configured_config() - .expect("codex apps should use configured transport"); + let config = + codex_apps_mcp_server_config("https://chatgpt.com", /*apps_mcp_product_sku*/ None); let url = match &config.transport { McpServerTransportConfig::StreamableHttp { url, .. } => url, _ => panic!("expected streamable http transport for codex apps"), @@ -232,42 +205,9 @@ fn codex_apps_server_config_uses_legacy_codex_apps_path() { assert_eq!(url, "https://chatgpt.com/backend-api/wham/apps"); } -#[test] -fn codex_apps_server_config_uses_configured_apps_mcp_path_override() { - let mut config = test_mcp_config(PathBuf::from("/tmp")); - config.apps_mcp_path_override = Some("/custom/mcp".to_string()); - config.apps_enabled = true; - let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); - - let servers = with_codex_apps_mcp(HashMap::new(), Some(&auth), &config); - let server = servers - .get(CODEX_APPS_MCP_SERVER_NAME) - .expect("codex apps should be present when apps is enabled"); - let config = server - .configured_config() - .expect("codex apps should use configured transport"); - let url = match &config.transport { - McpServerTransportConfig::StreamableHttp { url, .. } => url, - _ => panic!("expected streamable http transport for codex apps"), - }; - - assert_eq!(url, "https://chatgpt.com/backend-api/custom/mcp"); -} - #[test] fn codex_apps_server_config_forwards_configured_product_sku_header() { - let mut config = test_mcp_config(PathBuf::from("/tmp")); - config.apps_mcp_product_sku = Some("tpp".to_string()); - config.apps_enabled = true; - let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); - - let servers = with_codex_apps_mcp(HashMap::new(), Some(&auth), &config); - let server = servers - .get(CODEX_APPS_MCP_SERVER_NAME) - .expect("codex apps should be present when apps is enabled"); - let config = server - .configured_config() - .expect("codex apps should use configured transport"); + let config = codex_apps_mcp_server_config("https://chatgpt.com", Some("tpp")); match &config.transport { McpServerTransportConfig::StreamableHttp { @@ -289,7 +229,7 @@ fn codex_apps_server_config_forwards_configured_product_sku_header() { } #[tokio::test] -async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { +async fn effective_mcp_servers_preserve_runtime_servers() { let codex_home = tempfile::tempdir().expect("tempdir"); let mut config = test_mcp_config(codex_home.path().to_path_buf()); config.apps_enabled = true; @@ -345,6 +285,13 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { tools: HashMap::new(), }, ); + 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(), + ), + ); let effective = effective_mcp_servers(&config, Some(&auth)); diff --git a/codex-rs/config/src/schema.rs b/codex-rs/config/src/schema.rs index 6b012494d..c641f1703 100644 --- a/codex-rs/config/src/schema.rs +++ b/codex-rs/config/src/schema.rs @@ -9,6 +9,7 @@ use schemars::schema::ObjectValidation; use schemars::schema::RootSchema; use schemars::schema::Schema; use schemars::schema::SchemaObject; +use schemars::schema::SubschemaValidation; use serde_json::Map; use serde_json::Value; use std::path::Path; @@ -46,9 +47,7 @@ pub fn features_schema(schema_gen: &mut SchemaGenerator) -> Schema { if feature.id == codex_features::Feature::AppsMcpPathOverride { validation.properties.insert( feature.key.to_string(), - schema_gen.subschema_for::>(), + removed_apps_mcp_path_override_schema(schema_gen), ); continue; } @@ -76,6 +75,30 @@ pub fn features_schema(schema_gen: &mut SchemaGenerator) -> Schema { Schema::Object(object) } +fn removed_apps_mcp_path_override_schema(schema_gen: &mut SchemaGenerator) -> Schema { + let mut config_validation = ObjectValidation::default(); + config_validation + .properties + .insert("enabled".to_string(), schema_gen.subschema_for::()); + config_validation + .properties + .insert("path".to_string(), schema_gen.subschema_for::()); + config_validation.additional_properties = Some(Box::new(Schema::Bool(false))); + + let config = Schema::Object(SchemaObject { + instance_type: Some(InstanceType::Object.into()), + object: Some(Box::new(config_validation)), + ..Default::default() + }); + Schema::Object(SchemaObject { + subschemas: Some(Box::new(SubschemaValidation { + any_of: Some(vec![schema_gen.subschema_for::(), config]), + ..Default::default() + })), + ..Default::default() + }) +} + /// Schema for the `[mcp_servers]` map using the raw input shape. pub fn mcp_servers_schema(schema_gen: &mut SchemaGenerator) -> Schema { let mut object = SchemaObject { diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index b30b868bc..9ed02fdd1 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -226,18 +226,6 @@ }, "type": "object" }, - "AppsMcpPathOverrideConfigToml": { - "additionalProperties": false, - "properties": { - "enabled": { - "type": "boolean" - }, - "path": { - "type": "string" - } - }, - "type": "object" - }, "AskForApproval": { "description": "Determines the conditions under which the user is consulted to approve running the command proposed by Codex.", "oneOf": [ @@ -408,7 +396,23 @@ "type": "boolean" }, "apps_mcp_path_override": { - "$ref": "#/definitions/FeatureToml_for_AppsMcpPathOverrideConfigToml" + "anyOf": [ + { + "type": "boolean" + }, + { + "additionalProperties": false, + "properties": { + "enabled": { + "type": "boolean" + }, + "path": { + "type": "string" + } + }, + "type": "object" + } + ] }, "auth_elicitation": { "type": "boolean" @@ -836,16 +840,6 @@ }, "type": "object" }, - "FeatureToml_for_AppsMcpPathOverrideConfigToml": { - "anyOf": [ - { - "type": "boolean" - }, - { - "$ref": "#/definitions/AppsMcpPathOverrideConfigToml" - } - ] - }, "FeatureToml_for_CodeModeConfigToml": { "anyOf": [ { @@ -4534,7 +4528,23 @@ "type": "boolean" }, "apps_mcp_path_override": { - "$ref": "#/definitions/FeatureToml_for_AppsMcpPathOverrideConfigToml" + "anyOf": [ + { + "type": "boolean" + }, + { + "additionalProperties": false, + "properties": { + "enabled": { + "type": "boolean" + }, + "path": { + "type": "string" + } + }, + "type": "object" + } + ] }, "auth_elicitation": { "type": "boolean" @@ -5224,4 +5234,4 @@ }, "title": "ConfigToml", "type": "object" -} +} \ No newline at end of file diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 3a3b490d6..c19fe76fb 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -5500,14 +5500,9 @@ async fn to_mcp_config_preserves_apps_feature_from_config() -> std::io::Result<( .await?; let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); - config.apps_mcp_path_override = Some("/custom/mcp".to_string()); config.apps_mcp_product_sku = Some("tpp".to_string()); let mcp_config = config.to_mcp_config(&plugins_manager).await; assert!(mcp_config.apps_enabled); - assert_eq!( - mcp_config.apps_mcp_path_override.as_deref(), - Some("/custom/mcp") - ); assert_eq!(mcp_config.apps_mcp_product_sku.as_deref(), Some("tpp")); let _ = config.features.disable(Feature::Apps); @@ -8862,84 +8857,6 @@ allow_login_shell = false Ok(()) } -#[tokio::test] -async fn config_loads_apps_mcp_path_override_from_feature_config() -> std::io::Result<()> { - let codex_home = TempDir::new()?; - let toml = r#" -model = "gpt-5.4" - -[features.apps_mcp_path_override] -path = "/custom/mcp" -"#; - let cfg: ConfigToml = - toml::from_str(toml).expect("TOML deserialization should succeed for apps MCP feature"); - - let config = Config::load_from_base_config_with_overrides( - cfg, - ConfigOverrides::default(), - codex_home.abs(), - ) - .await?; - - assert_eq!( - config.apps_mcp_path_override.as_deref(), - Some("/custom/mcp") - ); - Ok(()) -} - -#[tokio::test] -async fn config_defaults_enabled_apps_mcp_path_override_to_plugin_service() -> std::io::Result<()> { - let codex_home = TempDir::new()?; - let toml = r#" -model = "gpt-5.4" - -[features] -apps_mcp_path_override = true -"#; - let cfg: ConfigToml = - toml::from_str(toml).expect("TOML deserialization should succeed for apps MCP feature"); - - let config = Config::load_from_base_config_with_overrides( - cfg, - ConfigOverrides::default(), - codex_home.abs(), - ) - .await?; - - assert!(config.features.enabled(Feature::AppsMcpPathOverride)); - assert_eq!(config.apps_mcp_path_override.as_deref(), Some("/ps/mcp")); - Ok(()) -} - -#[tokio::test] -async fn config_preserves_explicit_apps_mcp_path_override_path() -> std::io::Result<()> { - let codex_home = TempDir::new()?; - let toml = r#" -model = "gpt-5.4" - -[features.apps_mcp_path_override] -enabled = true -path = "/custom/mcp" -"#; - let cfg: ConfigToml = - toml::from_str(toml).expect("TOML deserialization should succeed for apps MCP feature"); - - let config = Config::load_from_base_config_with_overrides( - cfg, - ConfigOverrides::default(), - codex_home.abs(), - ) - .await?; - - assert_eq!( - config.apps_mcp_path_override.as_deref(), - Some("/custom/mcp") - ); - assert!(config.features.enabled(Feature::AppsMcpPathOverride)); - Ok(()) -} - #[tokio::test] async fn config_loads_apps_mcp_product_sku_from_toml() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 5c2a93bc4..cadb79721 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -58,7 +58,6 @@ use codex_config::types::WindowsSandboxModeToml; use codex_core_plugins::PluginsConfigInput; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::LOCAL_FS; -use codex_features::AppsMcpPathOverrideConfigToml; use codex_features::CodeModeConfigToml; use codex_features::Feature; use codex_features::FeatureConfigSource; @@ -931,9 +930,6 @@ pub struct Config { /// Base URL for requests to ChatGPT (as opposed to the OpenAI API). pub chatgpt_base_url: String, - /// Optional path override for the host-owned apps MCP server. - pub apps_mcp_path_override: Option, - /// Optional product SKU forwarded to the host-owned apps MCP server. pub apps_mcp_product_sku: Option, @@ -1415,7 +1411,6 @@ impl Config { McpConfig { chatgpt_base_url: self.chatgpt_base_url.clone(), - apps_mcp_path_override: self.apps_mcp_path_override.clone(), apps_mcp_product_sku: self.apps_mcp_product_sku.clone(), codex_home: self.codex_home.to_path_buf(), mcp_oauth_credentials_store_mode: self.mcp_oauth_credentials_store_mode, @@ -1428,7 +1423,6 @@ impl Config { codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(), use_legacy_landlock: self.features.use_legacy_landlock(), apps_enabled: self.features.enabled(Feature::Apps), - legacy_apps_mcp_loader_enabled: true, prefix_mcp_tool_names: self.prefix_mcp_tool_names(), client_elicitation_capability: if self.features.enabled(Feature::AuthElicitation) { ElicitationCapability { @@ -2413,15 +2407,6 @@ fn multi_agent_v2_toml_config(features: Option<&FeaturesToml>) -> Option<&MultiA } } -fn apps_mcp_path_override_toml_config( - features: Option<&FeaturesToml>, -) -> Option<&AppsMcpPathOverrideConfigToml> { - match features?.apps_mcp_path_override.as_ref()? { - FeatureToml::Enabled(_) => None, - FeatureToml::Config(config) => Some(config), - } -} - fn network_proxy_toml_config(features: Option<&FeaturesToml>) -> Option<&NetworkProxyConfigToml> { match features?.network_proxy.as_ref()? { FeatureToml::Enabled(_) => None, @@ -3043,14 +3028,6 @@ impl Config { resolve_experimental_request_user_input_enabled(&cfg); let code_mode = resolve_code_mode_config(&cfg); let multi_agent_v2 = resolve_multi_agent_v2_config(&cfg); - let apps_mcp_path_override = if features.enabled(Feature::AppsMcpPathOverride) { - let base = apps_mcp_path_override_toml_config(cfg.features.as_ref()); - base.and_then(|config| config.path.as_ref()) - .cloned() - .or_else(|| Some("/ps/mcp".to_string())) - } else { - None - }; let terminal_resize_reflow = resolve_terminal_resize_reflow_config(&cfg); let agent_roles = @@ -3553,7 +3530,6 @@ impl Config { chatgpt_base_url: cfg .chatgpt_base_url .unwrap_or("https://chatgpt.com/backend-api/".to_string()), - apps_mcp_path_override, apps_mcp_product_sku: cfg.apps_mcp_product_sku.clone(), realtime_audio: cfg .audio diff --git a/codex-rs/core/src/config_lock.rs b/codex-rs/core/src/config_lock.rs index 14ee7e110..f99ded0bf 100644 --- a/codex-rs/core/src/config_lock.rs +++ b/codex-rs/core/src/config_lock.rs @@ -125,6 +125,9 @@ fn config_lock_for_comparison( ) -> ConfigLockfileToml { let mut lockfile = lockfile.clone(); clear_config_lock_debug_controls(&mut lockfile.config); + if let Some(features) = lockfile.config.features.as_mut() { + features.clear_removed_compatibility_entries(); + } if options.allow_codex_version_mismatch { lockfile.codex_version.clear(); } diff --git a/codex-rs/core/src/mcp.rs b/codex-rs/core/src/mcp.rs index 1b693d70f..71571deae 100644 --- a/codex-rs/core/src/mcp.rs +++ b/codex-rs/core/src/mcp.rs @@ -11,6 +11,7 @@ use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::EffectiveMcpServer; use codex_mcp::McpConfig; use codex_mcp::ToolPluginProvenance; +use codex_mcp::codex_apps_mcp_server_config; use codex_mcp::configured_mcp_servers; use codex_mcp::effective_mcp_servers; use codex_mcp::tool_plugin_provenance as collect_tool_plugin_provenance; @@ -40,16 +41,24 @@ impl McpManager { } } - /// Returns the MCP config after applying runtime-only extension overlays. + /// Returns the MCP config after applying compatibility built-ins and + /// runtime-only extension overlays. pub async fn runtime_config(&self, config: &Config) -> McpConfig { let mut mcp_config = config.to_mcp_config(self.plugins_manager.as_ref()).await; - let contributions = self.contributions(config).await; - if contributions - .iter() - .any(|contribution| contribution.name() == CODEX_APPS_MCP_SERVER_NAME) - { - mcp_config.legacy_apps_mcp_loader_enabled = false; + if mcp_config.apps_enabled { + mcp_config.configured_mcp_servers.insert( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + codex_apps_mcp_server_config( + &mcp_config.chatgpt_base_url, + mcp_config.apps_mcp_product_sku.as_deref(), + ), + ); + } else { + mcp_config + .configured_mcp_servers + .remove(CODEX_APPS_MCP_SERVER_NAME); } + let contributions = self.contributions(config).await; Self::apply_to_configured_servers(&contributions, &mut mcp_config.configured_mcp_servers); mcp_config } diff --git a/codex-rs/core/src/session/config_lock.rs b/codex-rs/core/src/session/config_lock.rs index c081a2aee..ec2056581 100644 --- a/codex-rs/core/src/session/config_lock.rs +++ b/codex-rs/core/src/session/config_lock.rs @@ -2,7 +2,6 @@ use anyhow::Context; use codex_config::config_toml::ConfigLockfileToml; use codex_config::config_toml::ConfigToml; use codex_config::types::MemoriesToml; -use codex_features::AppsMcpPathOverrideConfigToml; use codex_features::Feature; use codex_features::FeatureToml; use codex_features::FeaturesToml; @@ -149,10 +148,6 @@ fn save_config_resolved_fields( resolved_config_to_toml(&config.multi_agent_v2, "features.multi_agent_v2")?; multi_agent_v2.enabled = Some(config.features.enabled(Feature::MultiAgentV2)); features.multi_agent_v2 = Some(FeatureToml::Config(multi_agent_v2)); - features.apps_mcp_path_override = Some(FeatureToml::Config(AppsMcpPathOverrideConfigToml { - enabled: Some(config.features.enabled(Feature::AppsMcpPathOverride)), - path: config.apps_mcp_path_override.clone(), - })); lock_config.memories = Some(resolved_config_to_toml::( &config.memories, "memories", @@ -325,6 +320,32 @@ mod tests { assert!(message.contains("model = "), "{message}"); } + #[tokio::test] + async fn lock_validation_ignores_removed_apps_mcp_path_override() { + let sc = crate::session::tests::make_session_configuration_for_tests().await; + let actual = sc.to_config_lockfile_toml().expect("lock should serialize"); + let mut expected_value = toml::Value::try_from(&actual).expect("lock should become TOML"); + expected_value["config"]["features"] + .as_table_mut() + .expect("features should be a table") + .insert( + "apps_mcp_path_override".to_string(), + toml::Value::Table(toml::Table::from_iter([ + ("enabled".to_string(), toml::Value::Boolean(true)), + ( + "path".to_string(), + toml::Value::String("/custom/mcp".to_string()), + ), + ])), + ); + let expected: ConfigLockfileToml = expected_value + .try_into() + .expect("lock with removed input should deserialize"); + + validate_config_lock_replay(&expected, &actual, ConfigLockReplayOptions::default()) + .expect("removed compatibility input should not cause lock drift"); + } + #[tokio::test] async fn lock_validation_rejects_codex_version_mismatch_by_default() { let sc = crate::session::tests::make_session_configuration_for_tests().await; diff --git a/codex-rs/core/src/tools/handlers/mcp_resource.rs b/codex-rs/core/src/tools/handlers/mcp_resource.rs index 9357a9722..e2f83859b 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource.rs @@ -7,6 +7,8 @@ use codex_protocol::items::McpToolCallItem; use codex_protocol::items::McpToolCallStatus; use codex_protocol::items::TurnItem; use codex_protocol::mcp::CallToolResult; +use codex_protocol::protocol::TruncationPolicy; +use codex_utils_output_truncation::truncate_text; use rmcp::model::ListResourceTemplatesResult; use rmcp::model::ListResourcesResult; use rmcp::model::ReadResourceResult; @@ -270,7 +272,10 @@ fn normalize_required_string(field: &str, value: String) -> Result(payload: T) -> Result +fn serialize_function_output( + payload: T, + truncation_policy: TruncationPolicy, +) -> Result where T: Serialize, { @@ -279,6 +284,9 @@ where "failed to serialize MCP resource response: {err}" )) })?; + // Match regular MCP tool outputs by bounding the copy persisted to the + // rollout and injected into model context. + let content = truncate_text(&content, truncation_policy * 1.2); Ok(FunctionToolOutput::from_text(content, Some(true))) } diff --git a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs index b1e82aa8a..c46df13b1 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs @@ -117,7 +117,7 @@ impl ToolExecutor for ListMcpResourceTemplatesHandler { .await; match payload_result { - Ok(payload) => match serialize_function_output(payload) { + Ok(payload) => match serialize_function_output(payload, turn.truncation_policy) { Ok(output) => { let content = function_call_output_content_items_to_text(&output.body) .unwrap_or_default(); diff --git a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs index c2cdec8b1..f7fcad342 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs @@ -115,7 +115,7 @@ impl ToolExecutor for ListMcpResourcesHandler { .await; match payload_result { - Ok(payload) => match serialize_function_output(payload) { + Ok(payload) => match serialize_function_output(payload, turn.truncation_policy) { Ok(output) => { let content = function_call_output_content_items_to_text(&output.body) .unwrap_or_default(); diff --git a/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs b/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs index 126c5d85e..f059d12c8 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs @@ -93,7 +93,7 @@ impl ToolExecutor for ReadMcpResourceHandler { .await; match payload_result { - Ok(payload) => match serialize_function_output(payload) { + Ok(payload) => match serialize_function_output(payload, turn.truncation_policy) { Ok(output) => { let content = function_call_output_content_items_to_text(&output.body) .unwrap_or_default(); diff --git a/codex-rs/core/src/tools/handlers/mcp_resource_tests.rs b/codex-rs/core/src/tools/handlers/mcp_resource_tests.rs index 8a8410b0b..b52ab1681 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource_tests.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource_tests.rs @@ -1,6 +1,7 @@ use super::*; use pretty_assertions::assert_eq; use rmcp::model::AnnotateAble; +use rmcp::model::ResourceContents; use serde_json::json; fn resource(uri: &str, name: &str) -> Resource { @@ -123,3 +124,39 @@ fn template_with_server_serializes_server_field() { }) ); } + +#[test] +fn serialize_function_output_preserves_small_payload() { + let payload = json!({"server": "hosted", "resources": []}); + let expected = serde_json::to_string(&payload).expect("serialize payload"); + + let output = serialize_function_output(payload, TruncationPolicy::Bytes(1_024)) + .expect("serialize function output") + .into_text(); + + assert_eq!(output, expected); +} + +#[test] +fn serialize_function_output_caps_read_resource_payload() { + let truncation_policy = TruncationPolicy::Bytes(8_000); + let payload = ReadResourcePayload { + server: "hosted".to_string(), + uri: "skill://large/SKILL.md".to_string(), + result: ReadResourceResult::new(vec![ResourceContents::TextResourceContents { + uri: "skill://large/SKILL.md".to_string(), + mime_type: Some("text/markdown".to_string()), + text: "x".repeat(16_000), + meta: None, + }]), + }; + let serialized = serde_json::to_string(&payload).expect("serialize payload"); + let expected = truncate_text(&serialized, truncation_policy * 1.2); + + let output = serialize_function_output(payload, truncation_policy) + .expect("serialize bounded function output") + .into_text(); + + assert_ne!(output, serialized); + assert_eq!(output, expected); +} diff --git a/codex-rs/ext/extension-api/src/contributors/mcp.rs b/codex-rs/ext/extension-api/src/contributors/mcp.rs index a55715692..7fdedcdda 100644 --- a/codex-rs/ext/extension-api/src/contributors/mcp.rs +++ b/codex-rs/ext/extension-api/src/contributors/mcp.rs @@ -11,12 +11,3 @@ pub enum McpServerContribution { /// Removes a named MCP server. Remove { name: String }, } - -impl McpServerContribution { - /// Returns the stable server name owned by this contribution. - pub fn name(&self) -> &str { - match self { - Self::Set { name, .. } | Self::Remove { name } => name, - } - } -} diff --git a/codex-rs/ext/mcp/src/lib.rs b/codex-rs/ext/mcp/src/lib.rs index 4a7838ccc..8fad361a4 100644 --- a/codex-rs/ext/mcp/src/lib.rs +++ b/codex-rs/ext/mcp/src/lib.rs @@ -3,12 +3,12 @@ use codex_extension_api::ExtensionRegistryBuilder; use codex_extension_api::McpServerContribution; use codex_extension_api::McpServerContributor; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; -use codex_mcp::codex_apps_mcp_server_config; +use codex_mcp::hosted_plugin_runtime_mcp_server_config; -struct HostedAppsMcpExtension; +struct HostedPluginRuntimeExtension; #[async_trait::async_trait] -impl McpServerContributor for HostedAppsMcpExtension { +impl McpServerContributor for HostedPluginRuntimeExtension { async fn contribute(&self, config: &Config) -> Vec { let name = CODEX_APPS_MCP_SERVER_NAME.to_string(); if !config.features.enabled(codex_features::Feature::Apps) { @@ -17,9 +17,8 @@ impl McpServerContributor for HostedAppsMcpExtension { vec![McpServerContribution::Set { name, - config: Box::new(codex_apps_mcp_server_config( + config: Box::new(hosted_plugin_runtime_mcp_server_config( &config.chatgpt_base_url, - config.apps_mcp_path_override.as_deref(), config.apps_mcp_product_sku.as_deref(), )), }] @@ -27,5 +26,5 @@ impl McpServerContributor for HostedAppsMcpExtension { } pub fn install(builder: &mut ExtensionRegistryBuilder) { - builder.mcp_server_contributor(std::sync::Arc::new(HostedAppsMcpExtension)); + builder.mcp_server_contributor(std::sync::Arc::new(HostedPluginRuntimeExtension)); } diff --git a/codex-rs/ext/mcp/tests/hosted_apps_mcp.rs b/codex-rs/ext/mcp/tests/hosted_apps_mcp.rs index 0d025df95..bbfa07c14 100644 --- a/codex-rs/ext/mcp/tests/hosted_apps_mcp.rs +++ b/codex-rs/ext/mcp/tests/hosted_apps_mcp.rs @@ -6,20 +6,22 @@ use codex_core::config::Config; use codex_core::config::ConfigBuilder; use codex_core_plugins::PluginsManager; use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::McpServerContribution; +use codex_extension_api::McpServerContributor; use codex_login::CodexAuth; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use pretty_assertions::assert_eq; +type TestResult = Result<(), Box>; + #[tokio::test] -async fn contributes_hosted_apps_mcp_without_an_executor() -> Result<(), Box> -{ +async fn contributes_hosted_plugin_runtime_without_an_executor() -> TestResult { let codex_home = tempfile::tempdir()?; let config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) .fallback_cwd(Some(codex_home.path().to_path_buf())) .cli_overrides(vec![ ("features.apps".to_string(), true.into()), - ("features.apps_mcp_path_override".to_string(), true.into()), ("chatgpt_base_url".to_string(), "https://chatgpt.com".into()), ]) .build() @@ -27,15 +29,13 @@ async fn contributes_hosted_apps_mcp_without_an_executor() -> Result<(), Box Result<(), Box Result<(), Box> { +async fn legacy_fallback_overwrites_reserved_config_without_an_extension() -> TestResult { + let codex_home = tempfile::tempdir()?; + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .cli_overrides(vec![ + ("features.apps".to_string(), true.into()), + ( + "mcp_servers.codex_apps.url".to_string(), + "https://example.com/mcp".into(), + ), + ]) + .build() + .await?; + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let manager = McpManager::new(Arc::new(PluginsManager::new( + config.codex_home.to_path_buf(), + ))); + + let servers = manager.effective_servers(&config, Some(&auth)).await; + let server = servers + .get(CODEX_APPS_MCP_SERVER_NAME) + .and_then(|server| server.configured_config()) + .ok_or("legacy Apps MCP should be present")?; + let McpServerTransportConfig::StreamableHttp { url, .. } = &server.transport else { + panic!("legacy Apps MCP should use streamable HTTP"); + }; + assert_eq!(url, "https://chatgpt.com/backend-api/wham/apps"); + + Ok(()) +} + +#[tokio::test] +async fn extension_can_remove_legacy_fallback_while_apps_are_enabled() -> TestResult { + let codex_home = tempfile::tempdir()?; + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .cli_overrides(vec![("features.apps".to_string(), true.into())]) + .build() + .await?; + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let mut builder = ExtensionRegistryBuilder::new(); + builder.mcp_server_contributor(Arc::new(RemoveCodexApps)); + let manager = McpManager::new_with_extensions( + Arc::new(PluginsManager::new(config.codex_home.to_path_buf())), + Arc::new(builder.build()), + ); + + let servers = manager.effective_servers(&config, Some(&auth)).await; + + assert!(!servers.contains_key(CODEX_APPS_MCP_SERVER_NAME)); + Ok(()) +} + +#[tokio::test] +async fn hosted_apps_mcp_requires_chatgpt_auth() -> TestResult { let codex_home = tempfile::tempdir()?; let config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) @@ -61,7 +117,7 @@ async fn hosted_apps_mcp_requires_chatgpt_auth() -> Result<(), Box Result<(), Box> { +async fn disabled_apps_remove_reserved_server_config() -> TestResult { let codex_home = tempfile::tempdir()?; let config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) @@ -91,3 +147,14 @@ fn installed_manager(config: &Config) -> McpManager { Arc::new(builder.build()), ) } + +struct RemoveCodexApps; + +#[async_trait::async_trait] +impl McpServerContributor for RemoveCodexApps { + async fn contribute(&self, _config: &Config) -> Vec { + vec![McpServerContribution::Remove { + name: CODEX_APPS_MCP_SERVER_NAME.to_string(), + }] + } +} diff --git a/codex-rs/features/src/feature_configs.rs b/codex-rs/features/src/feature_configs.rs index b7f666c21..f59b89e9a 100644 --- a/codex-rs/features/src/feature_configs.rs +++ b/codex-rs/features/src/feature_configs.rs @@ -70,21 +70,11 @@ impl FeatureConfig for MultiAgentV2ConfigToml { #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] #[serde(deny_unknown_fields)] -pub struct AppsMcpPathOverrideConfigToml { +pub(crate) struct RemovedAppsMcpPathOverrideConfigToml { #[serde(skip_serializing_if = "Option::is_none")] - pub enabled: Option, + enabled: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub path: Option, -} - -impl FeatureConfig for AppsMcpPathOverrideConfigToml { - fn enabled(&self) -> Option { - self.enabled.or(self.path.as_ref().map(|_| true)) - } - - fn set_enabled(&mut self, enabled: bool) { - self.enabled = Some(enabled); - } + path: Option, } #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 8ee05aed2..1777ceef0 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -16,13 +16,13 @@ use toml::Table; mod feature_configs; mod legacy; -pub use feature_configs::AppsMcpPathOverrideConfigToml; pub use feature_configs::CodeModeConfigToml; pub use feature_configs::MultiAgentV2ConfigToml; pub use feature_configs::NetworkProxyConfigToml; pub use feature_configs::NetworkProxyDomainPermissionToml; pub use feature_configs::NetworkProxyModeToml; pub use feature_configs::NetworkProxyUnixSocketPermissionToml; +use feature_configs::RemovedAppsMcpPathOverrideConfigToml; use legacy::LegacyFeatureToggles; pub use legacy::legacy_feature_keys; @@ -143,7 +143,7 @@ pub enum Feature { Apps, /// Enable MCP apps. EnableMcpApps, - /// Use the new path for the host-owned apps MCP server. + /// Removed compatibility flag for the legacy Apps MCP path override. AppsMcpPathOverride, /// Removed compatibility flag retained as a no-op now that tool_search is always enabled. ToolSearch, @@ -443,7 +443,7 @@ impl Features { "apply_patch_freeform" => { continue; } - "tool_search" => { + "tool_search" | "apps_mcp_path_override" => { continue; } "image_detail_original" => { @@ -605,8 +605,9 @@ pub struct FeaturesToml { pub code_mode: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] pub multi_agent_v2: Option>, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub apps_mcp_path_override: Option>, + #[serde(default, rename = "apps_mcp_path_override", skip_serializing)] + #[schemars(skip)] + removed_apps_mcp_path_override: Option>, pub network_proxy: Option>, /// Boolean feature toggles keyed by canonical or legacy feature name. #[serde(flatten)] @@ -621,6 +622,13 @@ impl Features { } impl FeaturesToml { + /// Removes compatibility-only inputs that no longer affect runtime + /// behavior or belong in newly materialized config. + pub fn clear_removed_compatibility_entries(&mut self) { + self.removed_apps_mcp_path_override = None; + self.entries.remove("apps_mcp_path_override"); + } + pub fn entries(&self) -> BTreeMap { let mut entries = self.entries.clone(); if let Some(enabled) = self.code_mode.as_ref().and_then(FeatureToml::enabled) { @@ -629,13 +637,6 @@ impl FeaturesToml { if let Some(enabled) = self.multi_agent_v2.as_ref().and_then(FeatureToml::enabled) { entries.insert(Feature::MultiAgentV2.key().to_string(), enabled); } - if let Some(enabled) = self - .apps_mcp_path_override - .as_ref() - .and_then(FeatureToml::enabled) - { - entries.insert(Feature::AppsMcpPathOverride.key().to_string(), enabled); - } if let Some(enabled) = self.network_proxy.as_ref().and_then(FeatureToml::enabled) { entries.insert(Feature::NetworkProxy.key().to_string(), enabled); } @@ -643,10 +644,11 @@ impl FeaturesToml { } pub fn materialize_resolved_enabled(&mut self, features: &Features) { + self.clear_removed_compatibility_entries(); let Self { code_mode, multi_agent_v2, - apps_mcp_path_override, + removed_apps_mcp_path_override: _, network_proxy, entries, } = self; @@ -659,8 +661,6 @@ impl FeaturesToml { materialize_resolved_feature_enabled(code_mode, enabled); } else if spec.id == Feature::MultiAgentV2 { materialize_resolved_feature_enabled(multi_agent_v2, enabled); - } else if spec.id == Feature::AppsMcpPathOverride { - materialize_resolved_feature_enabled(apps_mcp_path_override, enabled); } else if spec.id == Feature::NetworkProxy { materialize_resolved_feature_enabled(network_proxy, enabled); } else { @@ -987,7 +987,7 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::AppsMcpPathOverride, key: "apps_mcp_path_override", - stage: Stage::UnderDevelopment, + stage: Stage::Removed, default_enabled: false, }, FeatureSpec { diff --git a/codex-rs/features/src/tests.rs b/codex-rs/features/src/tests.rs index aa5e0174b..e230c4535 100644 --- a/codex-rs/features/src/tests.rs +++ b/codex-rs/features/src/tests.rs @@ -83,6 +83,27 @@ fn plugin_hooks_is_removed_and_disabled_by_default() { assert_eq!(feature_for_key("plugin_hooks"), Some(Feature::PluginHooks)); } +#[test] +fn removed_apps_mcp_path_override_shapes_are_ignored() { + let features = [ + toml::from_str::("apps_mcp_path_override = true") + .expect("boolean compatibility form should deserialize"), + toml::from_str::( + r#" +[apps_mcp_path_override] +enabled = true +path = "/custom/mcp" +"#, + ) + .expect("structured compatibility form should deserialize"), + ]; + + assert_eq!( + features.map(|features| features.entries()), + [BTreeMap::new(), BTreeMap::new()] + ); +} + #[test] fn code_mode_only_requires_code_mode() { let mut features = Features::with_defaults(); diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index 97bfea828..1bea0cae8 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -251,7 +251,6 @@ fn new_config(model: Option, arg0_paths: Arg0DispatchPaths) -> anyhow::R model_catalog: None, model_verbosity: None, chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(), - apps_mcp_path_override: None, apps_mcp_product_sku: None, realtime_audio: RealtimeAudioConfig::default(), experimental_realtime_ws_base_url: None,