From 080d871625c669bc0ddd900e05563c979452aa1c Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 15:24:52 -0700 Subject: [PATCH 01/27] Refine MCP explicit environment candidate --- .../codex_app_server_protocol.schemas.json | 6 +- .../codex_app_server_protocol.v2.schemas.json | 10 +- .../json/v2/ListMcpServerStatusResponse.json | 6 +- .../schema/typescript/v2/McpServerStatus.ts | 2 +- .../src/protocol/v2/mcp.rs | 1 + codex-rs/app-server/README.md | 2 +- codex-rs/app-server/src/request_processors.rs | 2 +- .../src/request_processors/mcp_processor.rs | 38 +-- codex-rs/cli/src/mcp_cmd.rs | 2 +- codex-rs/codex-mcp/src/connection_manager.rs | 6 +- .../codex-mcp/src/connection_manager_tests.rs | 14 +- codex-rs/codex-mcp/src/lib.rs | 2 +- codex-rs/codex-mcp/src/mcp/mod.rs | 29 ++- codex-rs/codex-mcp/src/mcp/mod_tests.rs | 4 +- codex-rs/codex-mcp/src/rmcp_client.rs | 67 +++--- codex-rs/codex-mcp/src/runtime.rs | 227 +++++++++--------- codex-rs/config/src/mcp_edit.rs | 4 +- codex-rs/config/src/mcp_edit_tests.rs | 4 +- codex-rs/config/src/mcp_types.rs | 10 +- codex-rs/config/src/mcp_types_tests.rs | 2 +- codex-rs/core-plugins/src/manager_tests.rs | 8 +- codex-rs/core/config.schema.json | 2 +- codex-rs/core/src/config/config_tests.rs | 36 +-- codex-rs/core/src/config/edit.rs | 4 +- codex-rs/core/src/config/edit_tests.rs | 14 +- codex-rs/core/src/connectors.rs | 8 +- codex-rs/core/src/mcp_skill_dependencies.rs | 4 +- codex-rs/core/src/mcp_tool_call_tests.rs | 10 +- codex-rs/core/src/session/mcp.rs | 16 +- codex-rs/core/src/session/mod.rs | 2 +- codex-rs/core/src/session/session.rs | 14 +- codex-rs/core/tests/suite/code_mode.rs | 2 +- codex-rs/core/tests/suite/hooks_mcp.rs | 2 +- codex-rs/core/tests/suite/rmcp_client.rs | 34 +-- codex-rs/core/tests/suite/search_tool.rs | 6 +- codex-rs/core/tests/suite/sqlite_state.rs | 2 +- codex-rs/core/tests/suite/truncation.rs | 6 +- 37 files changed, 300 insertions(+), 308 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index dda285476d6..a764610f104 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -11039,6 +11039,9 @@ "authStatus": { "$ref": "#/definitions/v2/McpAuthStatus" }, + "environmentId": { + "type": "string" + }, "name": { "type": "string" }, @@ -11063,6 +11066,7 @@ }, "required": [ "authStatus", + "environmentId", "name", "resourceTemplates", "resources", @@ -19033,4 +19037,4 @@ }, "title": "CodexAppServerProtocol", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 3a23ba3a69d..010d83b79e4 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -7568,6 +7568,9 @@ "authStatus": { "$ref": "#/definitions/McpAuthStatus" }, + "environmentId": { + "type": "string" + }, "name": { "type": "string" }, @@ -7591,8 +7594,9 @@ } }, "required": [ - "authStatus", - "name", + "authStatus", + "environmentId", + "name", "resourceTemplates", "resources", "tools" @@ -16856,4 +16860,4 @@ }, "title": "CodexAppServerProtocolV2", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json index fc181c2702e..645b8759637 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json @@ -15,6 +15,9 @@ "authStatus": { "$ref": "#/definitions/McpAuthStatus" }, + "environmentId": { + "type": "string" + }, "name": { "type": "string" }, @@ -39,6 +42,7 @@ }, "required": [ "authStatus", + "environmentId", "name", "resourceTemplates", "resources", @@ -188,4 +192,4 @@ ], "title": "ListMcpServerStatusResponse", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts index 430494e2687..3d4c57cd2d5 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts @@ -6,4 +6,4 @@ import type { ResourceTemplate } from "../ResourceTemplate"; import type { Tool } from "../Tool"; import type { McpAuthStatus } from "./McpAuthStatus"; -export type McpServerStatus = { name: string, tools: { [key in string]?: Tool }, resources: Array, resourceTemplates: Array, authStatus: McpAuthStatus, }; +export type McpServerStatus = { name: string, tools: { [key in string]?: Tool }, resources: Array, resourceTemplates: Array, authStatus: McpAuthStatus, environmentId: string, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs b/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs index 9fd93840768..16cb9d7aed3 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs @@ -55,6 +55,7 @@ pub struct McpServerStatus { pub resources: Vec, pub resource_templates: Vec, pub auth_status: McpAuthStatus, + pub environment_id: String, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 2ceffc86fec..bed188c0bf5 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -216,7 +216,7 @@ Example with notification opt-out: - `mcpServer/oauth/login` — start an OAuth login for a configured MCP server; returns an `authorization_url` and later emits `mcpServer/oauthLogin/completed` once the browser flow finishes. - `tool/requestUserInput` — prompt the user with 1–3 short questions for a tool call and return their answers (experimental). - `config/mcpServer/reload` — reload MCP server config from disk and queue a refresh for loaded threads (applied on each thread's next active turn); returns `{}`. Use this after editing `config.toml` without restarting the server. -- `mcpServerStatus/list` — enumerate configured MCP servers with their tools and auth status, plus resources/resource templates for `full` detail; supports cursor+limit pagination. If `detail` is omitted, the server defaults to `full`. +- `mcpServerStatus/list` — enumerate configured MCP servers with their tools, auth status, and resolved `environmentId`, plus resources/resource templates for `full` detail; supports cursor+limit pagination. If `detail` is omitted, the server defaults to `full`. - `mcpServer/resource/read` — read a resource from a configured MCP server by optional `threadId`, `server`, and `uri`, returning text/blob resource `contents`. If `threadId` is omitted, the server reads from the latest MCP config directly. - `mcpServer/tool/call` — call a tool on a thread's configured MCP server by `threadId`, `server`, `tool`, optional `arguments`, and optional `_meta`, returning the MCP tool result. - `windowsSandbox/setupStart` — start Windows sandbox setup for the selected mode (`elevated` or `unelevated`); accepts an optional absolute `cwd` to target setup for a specific workspace, returns `{ started: true }` immediately, and later emits `windowsSandbox/setupCompleted`. diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index 838dd8adf4c..6942049d1b4 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -340,7 +340,7 @@ use codex_login::complete_device_code_login; use codex_login::login_with_api_key; use codex_login::request_device_code; use codex_login::run_login_server; -use codex_mcp::McpRuntimeEnvironment; +use codex_mcp::McpRuntimeContext; use codex_mcp::McpServerStatusSnapshot; use codex_mcp::McpSnapshotDetail; use codex_mcp::collect_mcp_server_status_snapshot_with_detail; diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 4bb9431f422..1f38736d027 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -205,12 +205,10 @@ impl McpRequestProcessor { .await; let auth = self.auth_manager.auth().await; let environment_manager = self.thread_manager.environment_manager(); - // Status listing has no turn cwd. Prefer the configured default env, - // then configured local if present; do not manufacture a hidden local - // env in no-local modes. - let runtime_environment = McpRuntimeEnvironment::new( - environment_manager.default_or_local_environment(), - environment_manager.try_local_environment(), + // Status listing has no turn cwd. Use config cwd as the stdio fallback + // and resolve each server against the configured environment registry. + let runtime_context = McpRuntimeContext::new( + Arc::clone(&environment_manager), config.cwd.to_path_buf(), ); @@ -222,7 +220,7 @@ impl McpRequestProcessor { config, mcp_config, auth, - runtime_environment, + runtime_context, ) .await; }); @@ -236,7 +234,7 @@ impl McpRequestProcessor { config: Config, mcp_config: codex_mcp::McpConfig, auth: Option, - runtime_environment: McpRuntimeEnvironment, + runtime_context: McpRuntimeContext, ) { let result = Self::list_mcp_server_status_response( request_id.request_id.to_string(), @@ -244,7 +242,7 @@ impl McpRequestProcessor { config, mcp_config, auth, - runtime_environment, + runtime_context, ) .await; outgoing.send_result(request_id, result).await; @@ -256,7 +254,7 @@ impl McpRequestProcessor { config: Config, mcp_config: codex_mcp::McpConfig, auth: Option, - runtime_environment: McpRuntimeEnvironment, + runtime_context: McpRuntimeContext, ) -> Result { let detail = match params.detail.unwrap_or(McpServerStatusDetail::Full) { McpServerStatusDetail::Full => McpSnapshotDetail::Full, @@ -267,7 +265,7 @@ impl McpRequestProcessor { &mcp_config, auth.as_ref(), request_id, - runtime_environment, + runtime_context, detail, ) .await; @@ -278,6 +276,7 @@ impl McpRequestProcessor { resources, resource_templates, auth_statuses, + environment_ids, } = snapshot; let mut server_names: Vec = config @@ -326,6 +325,10 @@ impl McpRequestProcessor { .cloned() .unwrap_or(CoreMcpAuthStatus::Unsupported) .into(), + environment_id: environment_ids + .get(name) + .cloned() + .unwrap_or_else(|| codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string()), }) .collect(); @@ -367,12 +370,11 @@ impl McpRequestProcessor { .await; let auth = self.auth_manager.auth().await; let environment_manager = self.thread_manager.environment_manager(); - // Resource reads without a thread have no turn cwd. Prefer the - // configured default env, then configured local if present; do not - // manufacture a hidden local env in no-local modes. - let runtime_environment = McpRuntimeEnvironment::new( - environment_manager.default_or_local_environment(), - environment_manager.try_local_environment(), + // Resource reads without a thread have no turn cwd. Use config cwd as + // the stdio fallback and resolve each server against the configured + // environment registry. + let runtime_context = McpRuntimeContext::new( + Arc::clone(&environment_manager), config.cwd.to_path_buf(), ); let request_id = request_id.clone(); @@ -381,7 +383,7 @@ impl McpRequestProcessor { let result = read_mcp_resource_without_thread( &mcp_config, auth.as_ref(), - runtime_environment, + runtime_context, &server, &uri, ) diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 5c66a27d84c..2d4243a351b 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -301,7 +301,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re let new_entry = McpServerConfig { transport: transport.clone(), - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 8144f5b2278..f647aee1b90 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -27,7 +27,7 @@ use crate::rmcp_client::MCP_TOOLS_LIST_DURATION_METRIC; use crate::rmcp_client::ManagedClient; use crate::rmcp_client::StartupOutcomeError; use crate::rmcp_client::list_tools_for_client_uncached; -use crate::runtime::McpRuntimeEnvironment; +use crate::runtime::McpRuntimeContext; use crate::runtime::emit_duration; use crate::server::EffectiveMcpServer; use crate::server::McpServerMetadata; @@ -176,7 +176,7 @@ impl McpConnectionManager { submit_id: String, tx_event: Sender, initial_permission_profile: PermissionProfile, - runtime_environment: McpRuntimeEnvironment, + runtime_context: McpRuntimeContext, codex_home: PathBuf, codex_apps_tools_cache_key: CodexAppsToolsCacheKey, host_owned_codex_apps_enabled: bool, @@ -248,7 +248,7 @@ impl McpConnectionManager { elicitation_requests.clone(), codex_apps_tools_cache_context, Arc::clone(&tool_plugin_provenance), - runtime_environment.clone(), + runtime_context.clone(), runtime_auth_provider, client_elicitation_capability.clone(), ); diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 952bc8207b0..fa26b12043f 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -18,6 +18,7 @@ use crate::tools::normalize_tools_for_model; use crate::tools::tool_with_model_visible_input_schema; use codex_config::Constrained; use codex_config::McpServerConfig; +use codex_exec_server::EnvironmentManager; use codex_protocol::ToolName; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::GranularApprovalConfig; @@ -864,7 +865,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -889,7 +890,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -915,9 +916,8 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { String::new(), tx_event, PermissionProfile::default(), - McpRuntimeEnvironment::new( - /*environment*/ None, - /*local_environment*/ None, + McpRuntimeContext::new( + Arc::new(EnvironmentManager::without_environments()), PathBuf::from("/tmp"), ), codex_home.path().to_path_buf(), @@ -988,7 +988,7 @@ fn mcp_init_error_display_prompts_for_github_pat() { http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1041,7 +1041,7 @@ fn mcp_init_error_display_reports_generic_errors() { http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index 9d1fe223d64..7b45bcc7a78 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -3,7 +3,7 @@ pub use elicitation::ElicitationReviewRequest; pub use elicitation::ElicitationReviewer; pub use elicitation::ElicitationReviewerHandle; pub use rmcp_client::MCP_SANDBOX_STATE_META_CAPABILITY; -pub use runtime::McpRuntimeEnvironment; +pub use runtime::McpRuntimeContext; pub use runtime::SandboxState; pub use tools::ToolInfo; diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 3d1969de0cd..ea14f6cb10b 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -38,7 +38,7 @@ use serde_json::Value; use crate::codex_apps::codex_apps_tools_cache_key; use crate::connection_manager::McpConnectionManager; -use crate::runtime::McpRuntimeEnvironment; +use crate::runtime::McpRuntimeContext; use crate::server::EffectiveMcpServer; pub const CODEX_APPS_MCP_SERVER_NAME: &str = "codex_apps"; @@ -261,7 +261,7 @@ pub fn tool_plugin_provenance(config: &McpConfig) -> ToolPluginProvenance { pub async fn read_mcp_resource( config: &McpConfig, auth: Option<&CodexAuth>, - runtime_environment: McpRuntimeEnvironment, + runtime_context: McpRuntimeContext, server: &str, uri: &str, ) -> anyhow::Result { @@ -284,7 +284,7 @@ pub async fn read_mcp_resource( String::new(), tx_event, PermissionProfile::default(), - runtime_environment, + runtime_context, config.codex_home.clone(), codex_apps_tools_cache_key(auth), host_owned_codex_apps_enabled, @@ -314,13 +314,14 @@ pub struct McpServerStatusSnapshot { pub resources: HashMap>, pub resource_templates: HashMap>, pub auth_statuses: HashMap, + pub environment_ids: HashMap, } pub async fn collect_mcp_server_status_snapshot_with_detail( config: &McpConfig, auth: Option<&CodexAuth>, submit_id: String, - runtime_environment: McpRuntimeEnvironment, + runtime_context: McpRuntimeContext, detail: McpSnapshotDetail, ) -> McpServerStatusSnapshot { let mcp_servers = effective_mcp_servers(config, auth); @@ -332,6 +333,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( resources: HashMap::new(), resource_templates: HashMap::new(), auth_statuses: HashMap::new(), + environment_ids: HashMap::new(), }; } @@ -342,6 +344,17 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( ) .await; + let environment_ids = mcp_servers + .iter() + .map(|(server_name, server)| { + let environment_id = server + .configured_config() + .map(|config| runtime_context.status_environment_id(server_name, config)) + .unwrap_or_else(|| codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string()); + (server_name.clone(), environment_id) + }) + .collect(); + let (tx_event, rx_event) = unbounded(); drop(rx_event); @@ -353,7 +366,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( submit_id, tx_event, PermissionProfile::default(), - runtime_environment, + runtime_context, config.codex_home.clone(), codex_apps_tools_cache_key(auth), host_owned_codex_apps_enabled, @@ -367,6 +380,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( let snapshot = collect_mcp_server_status_snapshot_from_manager( &mcp_connection_manager, auth_status_entries, + environment_ids, detail, ) .await; @@ -451,7 +465,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig { http_headers, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -463,6 +477,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig { disabled_tools: None, scopes: None, oauth: None, + environment_id: None, oauth_resource: None, tools: HashMap::new(), } @@ -575,6 +590,7 @@ fn convert_mcp_resource_templates( async fn collect_mcp_server_status_snapshot_from_manager( mcp_connection_manager: &McpConnectionManager, auth_status_entries: HashMap, + environment_ids: HashMap, detail: McpSnapshotDetail, ) -> McpServerStatusSnapshot { let (tools, resources, resource_templates) = tokio::join!( @@ -613,6 +629,7 @@ async fn collect_mcp_server_status_snapshot_from_manager( resources: convert_mcp_resources(resources), resource_templates: convert_mcp_resource_templates(resource_templates), auth_statuses: auth_statuses_from_entries(&auth_status_entries), + environment_ids, } } diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 7566bf9bdea..31a098bb792 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -316,7 +316,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -341,7 +341,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 687576dfaca..e32e5b12fcf 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -28,7 +28,8 @@ use crate::codex_apps::write_cached_codex_apps_tools_if_needed; use crate::elicitation::ElicitationRequestManager; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::ToolPluginProvenance; -use crate::runtime::McpRuntimeEnvironment; +use crate::runtime::McpRuntimeContext; +use codex_exec_server::LOCAL_ENVIRONMENT_ID; use crate::runtime::emit_duration; use crate::server::EffectiveMcpServer; use crate::server::McpServerLaunch; @@ -142,7 +143,7 @@ impl AsyncManagedClient { elicitation_requests: ElicitationRequestManager, codex_apps_tools_cache_context: Option, tool_plugin_provenance: Arc, - runtime_environment: McpRuntimeEnvironment, + runtime_context: McpRuntimeContext, runtime_auth_provider: Option, client_elicitation_capability: ElicitationCapability, ) -> Self { @@ -170,7 +171,7 @@ impl AsyncManagedClient { &server_name, server.clone(), store_mode, - runtime_environment, + runtime_context, runtime_auth_provider, ) .await?, @@ -560,29 +561,16 @@ async fn make_rmcp_client( server_name: &str, server: EffectiveMcpServer, store_mode: OAuthCredentialsStoreMode, - runtime_environment: McpRuntimeEnvironment, + runtime_context: McpRuntimeContext, runtime_auth_provider: Option, ) -> Result { let config = match server.launch() { McpServerLaunch::Configured(config) => config.as_ref().clone(), }; - if let Some(reason) = runtime_environment.startup_unavailable_reason(server_name, &config) { - return Err(StartupOutcomeError::from(anyhow!(reason))); - } - let McpServerConfig { - transport, - experimental_environment, - .. - } = config; - let remote_environment = match experimental_environment.as_deref() { - None | Some("local") => false, - Some("remote") => true, - Some(environment) => { - return Err(StartupOutcomeError::from(anyhow!( - "unsupported experimental_environment `{environment}` for MCP server `{server_name}`" - ))); - } - }; + let resolved_environment = runtime_context + .resolve_server_environment(server_name, &config) + .map_err(|err| StartupOutcomeError::from(anyhow!(err)))?; + let McpServerConfig { transport, .. } = config; match transport { McpServerTransportConfig::Stdio { @@ -599,22 +587,20 @@ async fn make_rmcp_client( .map(|(key, value)| (key.into(), value.into())) .collect::>() }); - let launcher = if remote_environment { - // Preflight should reject this first, but keep client startup - // defensive if optional runtime placement is mis-threaded. - let Some(environment) = runtime_environment.environment() else { + let launcher = if resolved_environment.environment_id == LOCAL_ENVIRONMENT_ID { + Arc::new(LocalStdioServerLauncher::new( + runtime_context.fallback_cwd(), + )) as Arc + } else { + let Some(environment) = resolved_environment.environment else { return Err(StartupOutcomeError::from(anyhow!( - "remote MCP server requires a runtime environment" + "MCP server `{server_name}` resolved without an execution environment" ))); }; Arc::new(ExecutorStdioServerLauncher::new( environment.get_exec_backend(), - runtime_environment.fallback_cwd(), + runtime_context.fallback_cwd(), )) - } else { - Arc::new(LocalStdioServerLauncher::new( - runtime_environment.fallback_cwd(), - )) as Arc }; // `RmcpClient` always sees a launched MCP stdio server. The @@ -630,16 +616,17 @@ async fn make_rmcp_client( env_http_headers, bearer_token_env_var, } => { - let http_client: Arc = if remote_environment { - let Some(environment) = runtime_environment.environment() else { - return Err(StartupOutcomeError::from(anyhow!( - "remote MCP server requires a runtime environment" - ))); + let http_client: Arc = + if resolved_environment.environment_id == LOCAL_ENVIRONMENT_ID { + Arc::new(ReqwestHttpClient) + } else { + let Some(environment) = resolved_environment.environment else { + return Err(StartupOutcomeError::from(anyhow!( + "MCP server `{server_name}` resolved without an HTTP environment" + ))); + }; + environment.get_http_client() }; - environment.get_http_client() - } else { - Arc::new(ReqwestHttpClient) - }; let resolved_bearer_token = match resolve_bearer_token(server_name, bearer_token_env_var.as_deref()) { Ok(token) => token, diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index dc1fcec2198..25c43585e2a 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -10,6 +10,8 @@ use std::sync::Arc; use std::time::Duration; use codex_exec_server::Environment; +use codex_exec_server::EnvironmentManager; +use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::SandboxPolicy; @@ -28,80 +30,88 @@ pub struct SandboxState { pub use_legacy_landlock: bool, } -/// Runtime placement information used when starting MCP server transports. +/// Resolved environment placement for one MCP server startup. +#[derive(Clone)] +pub struct ResolvedMcpEnvironment { + pub environment_id: String, + pub environment: Option>, +} + +/// Runtime context used when resolving per-server MCP environment placement. /// -/// `McpConfig` describes what servers exist. This value describes which -/// selected/default environment MCP servers should use for the current caller. -/// Keep it explicit at manager construction time so status/snapshot paths and -/// real sessions make the same placement decision. `fallback_cwd` is not a -/// per-server override; it is used when a stdio server omits `cwd` and the -/// launcher needs a concrete process working directory. `local_environment` -/// is separate because a remote selected/default environment can coexist with -/// an explicitly configured local environment that may launch local stdio MCPs. +/// `McpConfig` describes what servers exist. This value carries the canonical +/// environment registry plus the fallback cwd used when a stdio server omits +/// its own working directory. #[derive(Clone)] -pub struct McpRuntimeEnvironment { - environment: Option>, - local_environment: Option>, +pub struct McpRuntimeContext { + environment_manager: Arc, fallback_cwd: PathBuf, } -impl McpRuntimeEnvironment { - pub fn new( - environment: Option>, - local_environment: Option>, - fallback_cwd: PathBuf, - ) -> Self { +impl McpRuntimeContext { + pub fn new(environment_manager: Arc, fallback_cwd: PathBuf) -> Self { Self { - environment, - local_environment, + environment_manager, fallback_cwd, } } - pub(crate) fn environment(&self) -> Option> { - self.environment.as_ref().map(Arc::clone) - } - pub(crate) fn fallback_cwd(&self) -> PathBuf { self.fallback_cwd.clone() } - pub(crate) fn startup_unavailable_reason( + pub(crate) fn resolve_server_environment( &self, server_name: &str, config: &codex_config::McpServerConfig, - ) -> Option { - // This is intentionally narrower than "no env means no MCP": local - // stdio needs a local process launcher, while local HTTP can still use - // the ambient HTTP client with no local environment configured. - match config.experimental_environment.as_deref() { - None | Some("local") => { - // Local stdio only needs an explicitly configured local - // launcher. The selected/default MCP environment can be remote - // when both local and remote environments are configured. - if self.local_environment.is_none() - && matches!( - config.transport, - codex_config::McpServerTransportConfig::Stdio { .. } - ) - { - Some(format!( - "local stdio MCP server `{server_name}` requires a local environment" - )) - } else { - None - } + ) -> Result { + let environment_id = config + .environment_id + .clone() + .unwrap_or_else(|| LOCAL_ENVIRONMENT_ID.to_string()); + if environment_id == LOCAL_ENVIRONMENT_ID { + let local_environment = self.environment_manager.try_local_environment(); + if local_environment.is_none() + && matches!( + config.transport, + codex_config::McpServerTransportConfig::Stdio { .. } + ) + { + return Err(format!( + "local stdio MCP server `{server_name}` requires a local environment" + )); } - Some("remote") => match self.environment.as_ref() { - Some(environment) if environment.is_remote() => None, - _ => Some(format!( - "remote MCP server `{server_name}` requires a remote environment" - )), - }, - Some(environment) => Some(format!( - "unsupported experimental_environment `{environment}` for MCP server `{server_name}`" - )), + return Ok(ResolvedMcpEnvironment { + environment_id, + environment: local_environment, + }); } + + let environment = self + .environment_manager + .get_environment(&environment_id) + .ok_or_else(|| { + format!("MCP server `{server_name}` references unknown environment id `{environment_id}`") + })?; + Ok(ResolvedMcpEnvironment { + environment_id, + environment: Some(environment), + }) + } + + pub(crate) fn status_environment_id( + &self, + server_name: &str, + config: &codex_config::McpServerConfig, + ) -> String { + self.resolve_server_environment(server_name, config) + .map(|resolved| resolved.environment_id) + .unwrap_or_else(|_| { + config + .environment_id + .clone() + .unwrap_or_else(|| LOCAL_ENVIRONMENT_ID.to_string()) + }) } } @@ -117,11 +127,12 @@ mod tests { use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; + use codex_exec_server::EnvironmentManager; use pretty_assertions::assert_eq; use super::*; - fn stdio_server(experimental_environment: Option<&str>) -> McpServerConfig { + fn stdio_server(environment_id: Option<&str>) -> McpServerConfig { McpServerConfig { transport: McpServerTransportConfig::Stdio { command: "echo".to_string(), @@ -130,7 +141,7 @@ mod tests { env_vars: Vec::new(), cwd: None, }, - experimental_environment: experimental_environment.map(str::to_string), + environment_id: environment_id.map(str::to_string), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -147,7 +158,7 @@ mod tests { } } - fn http_server(experimental_environment: Option<&str>) -> McpServerConfig { + fn http_server(environment_id: Option<&str>) -> McpServerConfig { McpServerConfig { transport: McpServerTransportConfig::StreamableHttp { url: "http://127.0.0.1:1".to_string(), @@ -155,111 +166,89 @@ mod tests { http_headers: None, env_http_headers: None, }, - experimental_environment: experimental_environment.map(str::to_string), - ..stdio_server(/*experimental_environment*/ None) + environment_id: environment_id.map(str::to_string), + ..stdio_server(/*environment_id*/ None) } } #[test] fn local_stdio_requires_local_stdio_availability() { - let runtime_environment = McpRuntimeEnvironment::new( - /*environment*/ None, - /*local_environment*/ None, + let runtime_context = McpRuntimeContext::new( + Arc::new(EnvironmentManager::without_environments()), PathBuf::from("/tmp"), ); assert_eq!( - runtime_environment.startup_unavailable_reason( - "stdio", - &stdio_server(/*experimental_environment*/ None) - ), - Some("local stdio MCP server `stdio` requires a local environment".to_string()) + runtime_context + .resolve_server_environment("stdio", &stdio_server(/*environment_id*/ None)), + Err("local stdio MCP server `stdio` requires a local environment".to_string()) ); } #[test] fn local_http_does_not_require_local_stdio_availability() { - let runtime_environment = McpRuntimeEnvironment::new( - /*environment*/ None, - /*local_environment*/ None, + let runtime_context = McpRuntimeContext::new( + Arc::new(EnvironmentManager::without_environments()), PathBuf::from("/tmp"), ); assert_eq!( - runtime_environment.startup_unavailable_reason( - "http", - &http_server(/*experimental_environment*/ None) - ), - None + runtime_context.status_environment_id("http", &http_server(/*environment_id*/ None)), + "local".to_string() ); } #[test] - fn remote_stdio_requires_remote_environment() { - let runtime_environment = McpRuntimeEnvironment::new( - /*environment*/ None, - /*local_environment*/ None, + fn unknown_explicit_environment_is_rejected() { + let runtime_context = McpRuntimeContext::new( + Arc::new(EnvironmentManager::without_environments()), PathBuf::from("/tmp"), ); assert_eq!( - runtime_environment.startup_unavailable_reason( + runtime_context.resolve_server_environment( "stdio", - &stdio_server(/*experimental_environment*/ Some("remote")), + &stdio_server(/*environment_id*/ Some("remote")), ), - Some("remote MCP server `stdio` requires a remote environment".to_string()) + Err("MCP server `stdio` references unknown environment id `remote`".to_string()) ); } - #[test] - fn remote_stdio_and_http_accept_remote_environment() { - let environment = Arc::new( - Environment::create_for_tests(Some("ws://127.0.0.1:8765".to_string())) - .expect("remote environment"), - ); - let runtime_environment = McpRuntimeEnvironment::new( - Some(environment), - /*local_environment*/ None, + #[tokio::test] + async fn explicit_remote_stdio_and_http_accept_named_environment() { + let runtime_context = McpRuntimeContext::new( + Arc::new( + EnvironmentManager::create_for_tests( + Some("ws://127.0.0.1:8765".to_string()), + /*local_runtime_paths*/ None, + ) + .await, + ), PathBuf::from("/tmp"), ); assert_eq!( - runtime_environment.startup_unavailable_reason( - "stdio", - &stdio_server(/*experimental_environment*/ Some("remote")), - ), - None + runtime_context + .status_environment_id("stdio", &stdio_server(/*environment_id*/ Some("remote"))), + Some("remote".to_string()) ); assert_eq!( - runtime_environment.startup_unavailable_reason( - "http", - &http_server(/*experimental_environment*/ Some("remote")), - ), - None + runtime_context + .status_environment_id("http", &http_server(/*environment_id*/ Some("remote"))), + Some("remote".to_string()) ); } - #[tokio::test] - async fn local_stdio_accepts_remote_runtime_when_local_environment_exists() { - let remote_environment = Arc::new( - Environment::create_for_tests(Some("ws://127.0.0.1:8765".to_string())) - .expect("remote environment"), - ); - let local_environment = Arc::new( - Environment::create_for_tests(/*exec_server_url*/ None).expect("local environment"), - ); - let runtime_environment = McpRuntimeEnvironment::new( - Some(remote_environment), - Some(local_environment), + #[test] + fn local_stdio_accepts_local_environment_when_available() { + let runtime_context = McpRuntimeContext::new( + Arc::new(EnvironmentManager::default_for_tests()), PathBuf::from("/tmp"), ); assert_eq!( - runtime_environment.startup_unavailable_reason( - "stdio", - &stdio_server(/*experimental_environment*/ None), - ), - None + runtime_context.status_environment_id("stdio", &stdio_server(/*environment_id*/ None)), + "local".to_string() ); } } diff --git a/codex-rs/config/src/mcp_edit.rs b/codex-rs/config/src/mcp_edit.rs index aaad3444fdc..0ea441ade36 100644 --- a/codex-rs/config/src/mcp_edit.rs +++ b/codex-rs/config/src/mcp_edit.rs @@ -175,8 +175,8 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem { if !config.enabled { entry["enabled"] = value(false); } - if let Some(environment) = &config.experimental_environment { - entry["experimental_environment"] = value(environment.clone()); + if let Some(environment_id) = &config.environment_id { + entry["environment_id"] = value(environment_id.clone()); } if config.required { entry["required"] = value(true); diff --git a/codex-rs/config/src/mcp_edit_tests.rs b/codex-rs/config/src/mcp_edit_tests.rs index 0215e7b8086..c54077bd131 100644 --- a/codex-rs/config/src/mcp_edit_tests.rs +++ b/codex-rs/config/src/mcp_edit_tests.rs @@ -23,7 +23,7 @@ async fn replace_mcp_servers_serializes_per_tool_approval_overrides() -> anyhow: env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: true, @@ -101,7 +101,7 @@ async fn replace_mcp_servers_serializes_oauth_client_id() -> anyhow::Result<()> http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index 8dfceec37e9..8aa9aca9823 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -128,9 +128,9 @@ pub struct McpServerConfig { #[serde(flatten)] pub transport: McpServerTransportConfig, - /// Experimental environment selector for where Codex should start this MCP server. + /// Explicit environment id for where Codex should start this MCP server. #[serde(default, skip_serializing_if = "Option::is_none")] - pub experimental_environment: Option, + pub environment_id: Option, /// When `false`, Codex skips initializing this MCP server. #[serde(default = "default_enabled")] @@ -231,7 +231,7 @@ pub struct RawMcpServerConfig { // shared #[serde(default)] - pub experimental_environment: Option, + pub environment_id: Option, #[serde(default)] pub startup_timeout_sec: Option, #[serde(default)] @@ -279,7 +279,7 @@ impl TryFrom for McpServerConfig { url, bearer_token, bearer_token_env_var, - experimental_environment, + environment_id, startup_timeout_sec, startup_timeout_ms, tool_timeout_sec, @@ -352,7 +352,7 @@ impl TryFrom for McpServerConfig { Ok(Self { transport, - experimental_environment, + environment_id, startup_timeout_sec, tool_timeout_sec, enabled: enabled.unwrap_or_else(default_enabled), diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index f3971154250..f50e1589de5 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -402,7 +402,7 @@ fn deserialize_ignores_unknown_server_fields() { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index fa0b485fa2c..e6f52cb5b45 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -235,7 +235,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -738,7 +738,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() { http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -850,7 +850,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1012,7 +1012,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index e810ce5f7ce..6f85785f2ee 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2304,7 +2304,7 @@ }, "type": "array" }, - "experimental_environment": { + "environment_id": { "default": null, "type": "string" }, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 46f29b32a82..625d1db7b5c 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -132,7 +132,7 @@ fn stdio_mcp(command: &str) -> McpServerConfig { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -157,7 +157,7 @@ fn http_mcp(url: &str) -> McpServerConfig { http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5146,7 +5146,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - experimental_environment: Some("remote".to_string()), + environment_id: Some("remote".to_string()), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5190,7 +5190,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { } assert_eq!(docs.startup_timeout_sec, Some(Duration::from_secs(3))); assert_eq!(docs.tool_timeout_sec, Some(Duration::from_secs(5))); - assert_eq!(docs.experimental_environment.as_deref(), Some("remote")); + assert_eq!(docs.environment_id.as_deref(), Some("remote")); assert!(docs.enabled); let empty = BTreeMap::new(); @@ -5493,7 +5493,7 @@ async fn replace_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5570,7 +5570,7 @@ async fn replace_mcp_servers_serializes_env_vars() -> anyhow::Result<()> { env_vars: vec!["ALPHA".into(), "BETA".into()], cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5632,7 +5632,7 @@ async fn replace_mcp_servers_serializes_sourced_env_vars() -> anyhow::Result<()> ], cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5684,7 +5684,7 @@ async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: Some(cwd_path.clone()), }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5739,7 +5739,7 @@ async fn replace_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5810,7 +5810,7 @@ async fn replace_mcp_servers_streamable_http_serializes_custom_headers() -> anyh "DOCS_AUTH".to_string(), )])), }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5893,7 +5893,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh "DOCS_AUTH".to_string(), )])), }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5929,7 +5929,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6000,7 +6000,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() "DOCS_AUTH".to_string(), )])), }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6026,7 +6026,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6115,7 +6115,7 @@ async fn replace_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: false, required: false, supports_parallel_tool_calls: false, @@ -6166,7 +6166,7 @@ async fn replace_mcp_servers_serializes_required_flag() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: true, supports_parallel_tool_calls: false, @@ -6217,7 +6217,7 @@ async fn replace_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6272,7 +6272,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index f467c76cf97..d5eca0f5e75 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -300,8 +300,8 @@ mod document_helpers { if !config.enabled { entry["enabled"] = value(false); } - if let Some(environment) = &config.experimental_environment { - entry["experimental_environment"] = value(environment.clone()); + if let Some(environment_id) = &config.environment_id { + entry["environment_id"] = value(environment_id.clone()); } if config.required { entry["required"] = value(true); diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index 30f82db29ba..99c91ae2608 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -945,7 +945,7 @@ fn blocking_replace_mcp_servers_round_trips() { env_vars: vec!["FOO".into()], cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: true, @@ -975,7 +975,7 @@ fn blocking_replace_mcp_servers_round_trips() { ), env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: false, required: false, supports_parallel_tool_calls: false, @@ -1047,7 +1047,7 @@ fn blocking_replace_mcp_servers_serializes_tool_approval_overrides() { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1112,7 +1112,7 @@ foo = { command = "cmd" } env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1167,7 +1167,7 @@ foo = { command = "cmd" } # keep me env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: false, required: false, supports_parallel_tool_calls: false, @@ -1221,7 +1221,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1276,7 +1276,7 @@ foo = { command = "cmd" } env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: false, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index de41919a895..48338bd2c87 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -35,7 +35,7 @@ use codex_login::CodexAuth; use codex_login::default_client::originator; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::McpConnectionManager; -use codex_mcp::McpRuntimeEnvironment; +use codex_mcp::McpRuntimeContext; use codex_mcp::ToolInfo; use codex_mcp::ToolPluginProvenance; use codex_mcp::codex_apps_tools_cache_key; @@ -272,11 +272,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager( PermissionProfile::default(), // Connector discovery is threadless. Use an actually configured env if // one exists, but do not reintroduce the old hidden-local fallback. - McpRuntimeEnvironment::new( - environment_manager.default_or_local_environment(), - environment_manager.try_local_environment(), - config.cwd.to_path_buf(), - ), + McpRuntimeContext::new(Arc::clone(&environment_manager), config.cwd.to_path_buf()), config.codex_home.to_path_buf(), codex_apps_tools_cache_key(auth.as_ref()), host_owned_codex_apps_enabled, diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index b8552596b3d..a6ab9664d04 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -361,7 +361,7 @@ fn mcp_dependency_to_server_config( http_headers: None, env_http_headers: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -391,7 +391,7 @@ fn mcp_dependency_to_server_config( env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index f9984c7f2d6..d2325bb1266 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1255,11 +1255,6 @@ fn codex_apps_auth_failure_metadata() -> McpToolApprovalMetadata { async fn install_host_owned_codex_apps_manager(session: &Session, turn_context: &TurnContext) { let auth = session.services.auth_manager.auth().await; - let environment = session - .services - .environment_manager - .default_or_local_environment() - .expect("test session should have an MCP runtime environment"); let (manager, _cancel_token) = codex_mcp::McpConnectionManager::new( &HashMap::new(), turn_context.config.mcp_oauth_credentials_store_mode, @@ -1268,9 +1263,8 @@ async fn install_host_owned_codex_apps_manager(session: &Session, turn_context: turn_context.sub_id.clone(), session.get_tx_event(), turn_context.permission_profile(), - codex_mcp::McpRuntimeEnvironment::new( - Some(environment), - session.services.environment_manager.try_local_environment(), + codex_mcp::McpRuntimeContext::new( + Arc::clone(&session.services.environment_manager), { #[allow(deprecated)] turn_context.cwd.to_path_buf() diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 738a328671c..40cb45d2acc 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -325,17 +325,13 @@ impl Session { host_owned_codex_apps_enabled(&mcp_config, auth.as_ref()); let auth_statuses = compute_auth_statuses(mcp_servers.iter(), store_mode, auth.as_ref()).await; - let mcp_runtime_environment = match turn_context.environments.primary() { - Some(turn_environment) => McpRuntimeEnvironment::new( - Some(Arc::clone(&turn_environment.environment)), - self.services.environment_manager.try_local_environment(), + let mcp_runtime_context = match turn_context.environments.primary() { + Some(turn_environment) => McpRuntimeContext::new( + Arc::clone(&self.services.environment_manager), turn_environment.cwd.to_path_buf(), ), - None => McpRuntimeEnvironment::new( - self.services - .environment_manager - .default_or_local_environment(), - self.services.environment_manager.try_local_environment(), + None => McpRuntimeContext::new( + Arc::clone(&self.services.environment_manager), #[allow(deprecated)] turn_context.cwd.to_path_buf(), ), @@ -353,7 +349,7 @@ impl Session { turn_context.sub_id.clone(), self.get_tx_event(), turn_context.permission_profile(), - mcp_runtime_environment, + mcp_runtime_context, config.codex_home.to_path_buf(), codex_apps_tools_cache_key(auth.as_ref()), host_owned_codex_apps_enabled, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index c12c29e7e17..c73b43a8384 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -61,7 +61,7 @@ use codex_login::CodexAuth; use codex_login::auth_env_telemetry::collect_auth_env_telemetry; use codex_login::default_client::originator; use codex_mcp::McpConnectionManager; -use codex_mcp::McpRuntimeEnvironment; +use codex_mcp::McpRuntimeContext; use codex_mcp::codex_apps_tools_cache_key; use codex_models_manager::manager::RefreshStrategy; use codex_models_manager::manager::SharedModelsManager; diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index cac4806bedb..8ed010172e7 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1135,15 +1135,13 @@ impl Session { })? .primary() .cloned(); - let mcp_runtime_environment = match turn_environment { - Some(turn_environment) => McpRuntimeEnvironment::new( - Some(Arc::clone(&turn_environment.environment)), - sess.services.environment_manager.try_local_environment(), + let mcp_runtime_context = match turn_environment { + Some(turn_environment) => McpRuntimeContext::new( + Arc::clone(&sess.services.environment_manager), turn_environment.cwd.to_path_buf(), ), - None => McpRuntimeEnvironment::new( - sess.services.environment_manager.default_or_local_environment(), - sess.services.environment_manager.try_local_environment(), + None => McpRuntimeContext::new( + Arc::clone(&sess.services.environment_manager), session_configuration.cwd.to_path_buf(), ), }; @@ -1155,7 +1153,7 @@ impl Session { INITIAL_SUBMIT_ID.to_owned(), tx_event.clone(), session_configuration.permission_profile(), - mcp_runtime_environment, + mcp_runtime_context, config.codex_home.to_path_buf(), codex_apps_tools_cache_key(auth), host_owned_codex_apps_enabled, diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index b733d7bded8..5ef93ccf971 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -242,7 +242,7 @@ async fn run_code_mode_turn_with_rmcp_config( env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/hooks_mcp.rs b/codex-rs/core/tests/suite/hooks_mcp.rs index 80290e31bae..2ff87516616 100644 --- a/codex-rs/core/tests/suite/hooks_mcp.rs +++ b/codex-rs/core/tests/suite/hooks_mcp.rs @@ -182,7 +182,7 @@ fn insert_rmcp_test_server(config: &mut Config, command: String, approval_mode: env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 8e85c5d5eef..08f3ec75ec5 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -139,7 +139,7 @@ enum McpCallEvent { const REMOTE_MCP_ENVIRONMENT: &str = "remote"; -fn remote_aware_experimental_environment() -> Option { +fn remote_aware_environment_id() -> Option { // These tests run locally in normal CI and against the Docker-backed // executor in full-ci. Match that shared test environment instead of // parameterizing each stdio MCP test with its own local/remote cases. @@ -287,7 +287,7 @@ async fn wait_for_mcp_server(fixture: &TestCodex, server_name: &str) -> anyhow:: #[derive(Default)] struct TestMcpServerOptions { - experimental_environment: Option, + environment_id: Option, supports_parallel_tool_calls: bool, tool_timeout_sec: Option, } @@ -326,7 +326,7 @@ fn insert_mcp_server( server_name.to_string(), McpServerConfig { transport, - experimental_environment: options.experimental_environment, + environment_id: options.environment_id, enabled: true, required: false, supports_parallel_tool_calls: options.supports_parallel_tool_calls, @@ -480,7 +480,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { Vec::new(), ), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -603,7 +603,7 @@ async fn stdio_server_uses_configured_cwd_before_runtime_fallback() -> anyhow::R Some(configured_cwd), ), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -656,7 +656,7 @@ async fn remote_stdio_server_uses_runtime_fallback_cwd_when_config_omits_cwd() - Vec::new(), ), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -775,7 +775,7 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()> server_name, stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -872,7 +872,7 @@ async fn stdio_mcp_parallel_tool_calls_default_false_runs_serially() -> anyhow:: server_name, stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), tool_timeout_sec: Some(Duration::from_secs(2)), ..Default::default() }, @@ -989,7 +989,7 @@ async fn stdio_mcp_parallel_tool_calls_opt_in_runs_concurrently() -> anyhow::Res server_name, stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), supports_parallel_tool_calls: true, tool_timeout_sec: Some(Duration::from_secs(2)), }, @@ -1071,7 +1071,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { Vec::new(), ), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -1204,7 +1204,7 @@ async fn stdio_image_responses_preserve_original_detail_metadata() -> anyhow::Re server_name, stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -1340,7 +1340,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re Vec::new(), ), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -1442,7 +1442,7 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> { vec!["MCP_TEST_VALUE".into()], ), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -1560,7 +1560,7 @@ async fn stdio_server_propagates_explicit_local_env_var_source() -> anyhow::Resu }], ), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -1652,7 +1652,7 @@ async fn remote_stdio_env_var_source_does_not_copy_local_env() -> anyhow::Result }], ), TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -1835,7 +1835,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { env_http_headers: None, }, TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); @@ -2021,7 +2021,7 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { env_http_headers: None, }, TestMcpServerOptions { - experimental_environment: remote_aware_experimental_environment(), + environment_id: remote_aware_environment_id(), ..Default::default() }, ); diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index 706ff4871e0..085288ba15b 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -1012,7 +1012,7 @@ async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, disabled_reason: None, @@ -1141,7 +1141,7 @@ async fn tool_search_surfaced_mcp_tool_errors_are_returned_to_model() -> Result< env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, disabled_reason: None, @@ -1288,7 +1288,7 @@ async fn tool_search_uses_non_app_mcp_server_instructions_as_namespace_descripti env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, disabled_reason: None, diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index f576e55a3d0..4ea994dc652 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -376,7 +376,7 @@ async fn mcp_call_marks_thread_memory_mode_polluted_when_configured() -> Result< env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index be9267934d6..976ffd487ac 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -387,7 +387,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -487,7 +487,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -775,7 +775,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: None, enabled: true, required: false, supports_parallel_tool_calls: false, From e2b7fc978279ea1075285ce6828ba4673413f416 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 15:52:37 -0700 Subject: [PATCH 02/27] Fix duplicate MCP environment field --- codex-rs/codex-mcp/src/mcp/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index ea14f6cb10b..9c6771c1849 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -477,7 +477,6 @@ fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig { disabled_tools: None, scopes: None, oauth: None, - environment_id: None, oauth_resource: None, tools: HashMap::new(), } From ab444d92346b52751cfe743b860727f3b0a86000 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 15:56:04 -0700 Subject: [PATCH 03/27] Pass MCP runtime context env manager by arc --- .../app-server/src/request_processors/apps_processor.rs | 2 +- .../app-server/src/request_processors/config_processor.rs | 2 +- codex-rs/app-server/src/request_processors/plugins.rs | 4 ++-- codex-rs/core/src/connectors.rs | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/apps_processor.rs b/codex-rs/app-server/src/request_processors/apps_processor.rs index 5a1c9ffe1c4..49a6615f1c5 100644 --- a/codex-rs/app-server/src/request_processors/apps_processor.rs +++ b/codex-rs/app-server/src/request_processors/apps_processor.rs @@ -171,7 +171,7 @@ impl AppsRequestProcessor { connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( &accessible_config, force_refetch, - &environment_manager, + Arc::clone(&environment_manager), ) .await .map_err(|err| format!("failed to load accessible apps: {err}")); diff --git a/codex-rs/app-server/src/request_processors/config_processor.rs b/codex-rs/app-server/src/request_processors/config_processor.rs index dd6e8e3c92e..8f50a3c216a 100644 --- a/codex-rs/app-server/src/request_processors/config_processor.rs +++ b/codex-rs/app-server/src/request_processors/config_processor.rs @@ -220,7 +220,7 @@ impl ConfigRequestProcessor { connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( &config, /*force_refetch*/ true, - &environment_manager, + Arc::clone(&environment_manager), ), ); let all_connectors = match all_connectors_result { diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 61fd363b2ef..ce89ad5aa51 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1506,7 +1506,7 @@ impl PluginRequestProcessor { connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( config, /*force_refetch*/ true, - &environment_manager + Arc::clone(&environment_manager) ), ); @@ -1795,7 +1795,7 @@ async fn load_plugin_app_summaries( match connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( config, /*force_refetch*/ false, - environment_manager, + Arc::clone(&environment_manager), ) .await { diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 48338bd2c87..8593686c704 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -203,7 +203,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( list_accessible_connectors_from_mcp_tools_with_environment_manager( config, force_refetch, - &environment_manager, + Arc::new(environment_manager), ) .await } @@ -211,7 +211,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager( config: &Config, force_refetch: bool, - environment_manager: &EnvironmentManager, + environment_manager: Arc, ) -> anyhow::Result { let auth_manager = AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; @@ -272,7 +272,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager( PermissionProfile::default(), // Connector discovery is threadless. Use an actually configured env if // one exists, but do not reintroduce the old hidden-local fallback. - McpRuntimeContext::new(Arc::clone(&environment_manager), config.cwd.to_path_buf()), + McpRuntimeContext::new(environment_manager, config.cwd.to_path_buf()), config.codex_home.to_path_buf(), codex_apps_tools_cache_key(auth.as_ref()), host_owned_codex_apps_enabled, From 60a4f5f70068bee792139bfa2561f4b5976d0d56 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 15:59:57 -0700 Subject: [PATCH 04/27] Pass plugin connector env manager by arc --- .../src/request_processors/plugins.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index ce89ad5aa51..8c237b253b0 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -960,7 +960,11 @@ impl PluginRequestProcessor { }; let environment_manager = self.thread_manager.environment_manager(); let app_summaries = - load_plugin_app_summaries(&config, &outcome.plugin.apps, &environment_manager) + load_plugin_app_summaries( + &config, + &outcome.plugin.apps, + Arc::clone(&environment_manager), + ) .await; let visible_skills = outcome .plugin @@ -1038,7 +1042,12 @@ impl PluginRequestProcessor { .collect::>(); let environment_manager = self.thread_manager.environment_manager(); let app_summaries = - load_plugin_app_summaries(&config, &plugin_apps, &environment_manager).await; + load_plugin_app_summaries( + &config, + &plugin_apps, + Arc::clone(&environment_manager), + ) + .await; remote_plugin_detail_to_info(remote_detail, app_summaries) } }; @@ -1772,7 +1781,7 @@ impl PluginRequestProcessor { async fn load_plugin_app_summaries( config: &Config, plugin_apps: &[codex_plugin::AppConnectorId], - environment_manager: &EnvironmentManager, + environment_manager: Arc, ) -> Vec { if plugin_apps.is_empty() { return Vec::new(); @@ -1795,7 +1804,7 @@ async fn load_plugin_app_summaries( match connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( config, /*force_refetch*/ false, - Arc::clone(&environment_manager), + environment_manager, ) .await { From 9d5822206459d953cf7a2d10c2a9e2a32e888ad9 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 16:01:41 -0700 Subject: [PATCH 05/27] Clarify MCP status env reporting --- .../src/request_processors/mcp_processor.rs | 7 ++-- codex-rs/codex-mcp/src/mcp/mod.rs | 2 +- codex-rs/codex-mcp/src/runtime.rs | 40 +++++++++---------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 1f38736d027..05fa8054769 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -206,7 +206,8 @@ impl McpRequestProcessor { let auth = self.auth_manager.auth().await; let environment_manager = self.thread_manager.environment_manager(); // Status listing has no turn cwd. Use config cwd as the stdio fallback - // and resolve each server against the configured environment registry. + // and the shared environment registry for explicit MCP env ids; an + // omitted MCP env id still means local. let runtime_context = McpRuntimeContext::new( Arc::clone(&environment_manager), config.cwd.to_path_buf(), @@ -371,8 +372,8 @@ impl McpRequestProcessor { let auth = self.auth_manager.auth().await; let environment_manager = self.thread_manager.environment_manager(); // Resource reads without a thread have no turn cwd. Use config cwd as - // the stdio fallback and resolve each server against the configured - // environment registry. + // the stdio fallback and the shared environment registry for explicit + // MCP env ids; an omitted MCP env id still means local. let runtime_context = McpRuntimeContext::new( Arc::clone(&environment_manager), config.cwd.to_path_buf(), diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 9c6771c1849..c331f61980a 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -349,7 +349,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( .map(|(server_name, server)| { let environment_id = server .configured_config() - .map(|config| runtime_context.status_environment_id(server_name, config)) + .map(McpRuntimeContext::configured_or_default_environment_id) .unwrap_or_else(|| codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string()); (server_name.clone(), environment_id) }) diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index 25c43585e2a..715bd4ec347 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -99,19 +99,13 @@ impl McpRuntimeContext { }) } - pub(crate) fn status_environment_id( - &self, - server_name: &str, + pub(crate) fn configured_or_default_environment_id( config: &codex_config::McpServerConfig, ) -> String { - self.resolve_server_environment(server_name, config) - .map(|resolved| resolved.environment_id) - .unwrap_or_else(|_| { - config - .environment_id - .clone() - .unwrap_or_else(|| LOCAL_ENVIRONMENT_ID.to_string()) - }) + config + .environment_id + .clone() + .unwrap_or_else(|| LOCAL_ENVIRONMENT_ID.to_string()) } } @@ -193,7 +187,9 @@ mod tests { ); assert_eq!( - runtime_context.status_environment_id("http", &http_server(/*environment_id*/ None)), + McpRuntimeContext::configured_or_default_environment_id(&http_server( + /*environment_id*/ None, + )), "local".to_string() ); } @@ -228,14 +224,16 @@ mod tests { ); assert_eq!( - runtime_context - .status_environment_id("stdio", &stdio_server(/*environment_id*/ Some("remote"))), - Some("remote".to_string()) + McpRuntimeContext::configured_or_default_environment_id(&stdio_server( + /*environment_id*/ Some("remote"), + )), + "remote".to_string() ); assert_eq!( - runtime_context - .status_environment_id("http", &http_server(/*environment_id*/ Some("remote"))), - Some("remote".to_string()) + McpRuntimeContext::configured_or_default_environment_id(&http_server( + /*environment_id*/ Some("remote"), + )), + "remote".to_string() ); } @@ -247,8 +245,10 @@ mod tests { ); assert_eq!( - runtime_context.status_environment_id("stdio", &stdio_server(/*environment_id*/ None)), - "local".to_string() + McpRuntimeContext::configured_or_default_environment_id(&stdio_server( + /*environment_id*/ None, + )), + "local".to_string() ); } } From 3b8b6b19c62faf9da6da14077f7b4d81b1c59dca Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 17:08:13 -0700 Subject: [PATCH 06/27] test app-server MCP environment matrix --- .../app-server/tests/suite/v2/mcp_tool.rs | 274 ++++++++++++++++++ 1 file changed, 274 insertions(+) diff --git a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs index 46e9b2bf07a..a28faa2af08 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -1,9 +1,15 @@ use std::borrow::Cow; use std::collections::BTreeMap; +use std::path::Path; +use std::process::Command as StdCommand; use std::sync::Arc; use std::time::Duration; +use std::time::SystemTime; +use std::time::UNIX_EPOCH; +use anyhow::Context as _; use anyhow::Result; +use anyhow::ensure; use app_test_support::McpProcess; use app_test_support::create_final_assistant_message_sse_response; use app_test_support::create_mock_responses_server_sequence; @@ -13,11 +19,14 @@ use axum::Router; use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::ListMcpServerStatusParams; +use codex_app_server_protocol::ListMcpServerStatusResponse; use codex_app_server_protocol::McpElicitationSchema; use codex_app_server_protocol::McpServerElicitationAction; use codex_app_server_protocol::McpServerElicitationRequest; use codex_app_server_protocol::McpServerElicitationRequestParams; use codex_app_server_protocol::McpServerElicitationRequestResponse; +use codex_app_server_protocol::McpServerStatusDetail; use codex_app_server_protocol::McpServerToolCallParams; use codex_app_server_protocol::McpServerToolCallResponse; use codex_app_server_protocol::McpToolCallStatus; @@ -29,8 +38,11 @@ use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput as V2UserInput; +use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; +use core_test_support::get_remote_test_env; use core_test_support::responses; +use core_test_support::stdio_server_bin; use pretty_assertions::assert_eq; use rmcp::handler::server::ServerHandler; use rmcp::model::BooleanSchema; @@ -68,6 +80,7 @@ const ELICITATION_MESSAGE: &str = "Allow this request?"; const URL_ELICITATION_TRIGGER_MESSAGE: &str = "auth"; const URL_ELICITATION_MESSAGE: &str = "Sign in to GitHub to continue."; const URL_ELICITATION_URL: &str = "https://github.example/login/device"; +const REMOTE_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_TEST_REMOTE_EXEC_SERVER_URL"; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_returns_tool_result() -> Result<()> { @@ -187,6 +200,121 @@ async fn mcp_server_tool_call_returns_error_for_unknown_thread() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn mcp_stdio_servers_use_configured_local_and_remote_environments() -> Result<()> { + let Some(remote_env) = get_remote_test_env() else { + return Ok(()); + }; + let responses_server = responses::start_mock_server().await; + let codex_home = TempDir::new()?; + write_mock_responses_config_toml( + codex_home.path(), + &responses_server.uri(), + &BTreeMap::new(), + /*auto_compact_limit*/ 1024, + /*requires_openai_auth*/ None, + "mock_provider", + "compact", + )?; + + let local_stdio_server_bin = stdio_server_bin()?; + let remote_stdio_server_bin = copy_binary_to_remote_env( + &remote_env.container_name, + Path::new(&local_stdio_server_bin), + )?; + write_remote_environment_config(codex_home.path())?; + append_stdio_mcp_config( + codex_home.path(), + &local_stdio_server_bin, + &remote_stdio_server_bin, + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + let thread_id = start_test_thread(&mut mcp).await?; + + assert_echo_tool_call(&mut mcp, &thread_id, "remote_stdio", "hello remote").await?; + assert_echo_tool_call(&mut mcp, &thread_id, "local_stdio", "hello local").await?; + + let request_id = mcp + .send_list_mcp_server_status_request(ListMcpServerStatusParams { + cursor: None, + limit: None, + detail: Some(McpServerStatusDetail::ToolsAndAuthOnly), + }) + .await?; + let response = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: ListMcpServerStatusResponse = to_response(response)?; + let environment_ids = response + .data + .into_iter() + .map(|status| (status.name, status.environment_id)) + .collect::>(); + assert_eq!( + environment_ids, + BTreeMap::from([ + ("local_stdio".to_string(), "local".to_string()), + ("remote_stdio".to_string(), "remote".to_string()), + ]) + ); + + Ok(()) +} + +#[tokio::test] +async fn mcp_stdio_server_requires_local_environment_when_config_omits_environment_id() -> Result<()> +{ + let responses_server = responses::start_mock_server().await; + let codex_home = TempDir::new()?; + write_mock_responses_config_toml( + codex_home.path(), + &responses_server.uri(), + &BTreeMap::new(), + /*auto_compact_limit*/ 1024, + /*requires_openai_auth*/ None, + "mock_provider", + "compact", + )?; + append_local_stdio_mcp_config(codex_home.path(), &stdio_server_bin()?)?; + + let mut mcp = McpProcess::new_with_env( + codex_home.path(), + &[(CODEX_EXEC_SERVER_URL_ENV_VAR, Some("none"))], + ) + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + let thread_id = start_test_thread(&mut mcp).await?; + + let request_id = mcp + .send_mcp_server_tool_call_request(McpServerToolCallParams { + thread_id, + server: "local_stdio".to_string(), + tool: "echo".to_string(), + arguments: Some(json!({ + "message": "hello local", + })), + meta: None, + }) + .await?; + let error: JSONRPCError = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + assert!( + error.error.message.contains( + "failed to get client: MCP startup failed: local stdio MCP server `local_stdio` requires a local environment" + ), + "unexpected MCP tool-call error: {error:?}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_round_trips_elicitation() -> Result<()> { let responses_server = responses::start_mock_server().await; @@ -297,6 +425,152 @@ url = "{mcp_server_url}/mcp" Ok(()) } +async fn start_test_thread(mcp: &mut McpProcess) -> Result { + let thread_start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let thread_start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response(thread_start_resp)?; + Ok(thread.id) +} + +async fn assert_echo_tool_call( + mcp: &mut McpProcess, + thread_id: &str, + server: &str, + message: &str, +) -> Result<()> { + let request_id = mcp + .send_mcp_server_tool_call_request(McpServerToolCallParams { + thread_id: thread_id.to_string(), + server: server.to_string(), + tool: "echo".to_string(), + arguments: Some(json!({ + "message": message, + })), + meta: None, + }) + .await?; + let response = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: McpServerToolCallResponse = to_response(response)?; + assert_eq!( + response.structured_content, + Some(json!({ + "echo": format!("ECHOING: {message}"), + "env": null, + })) + ); + assert_eq!(response.is_error, Some(false)); + Ok(()) +} + +fn append_stdio_mcp_config( + codex_home: &Path, + local_stdio_server_bin: &str, + remote_stdio_server_bin: &str, +) -> Result<()> { + let config_path = codex_home.join("config.toml"); + let mut config_toml = std::fs::read_to_string(&config_path)?; + config_toml.push_str(&format!( + r#" +[mcp_servers.local_stdio] +command = {local_stdio_server_bin:?} + +[mcp_servers.remote_stdio] +environment_id = "remote" +command = {remote_stdio_server_bin:?} +cwd = "/tmp" +"# + )); + std::fs::write(config_path, config_toml)?; + Ok(()) +} + +fn append_local_stdio_mcp_config(codex_home: &Path, local_stdio_server_bin: &str) -> Result<()> { + let config_path = codex_home.join("config.toml"); + let mut config_toml = std::fs::read_to_string(&config_path)?; + config_toml.push_str(&format!( + r#" +[mcp_servers.local_stdio] +command = {local_stdio_server_bin:?} +"# + )); + std::fs::write(config_path, config_toml)?; + Ok(()) +} + +fn write_remote_environment_config(codex_home: &Path) -> Result<()> { + let remote_exec_server_url = std::env::var(REMOTE_EXEC_SERVER_URL_ENV_VAR) + .with_context(|| format!("{REMOTE_EXEC_SERVER_URL_ENV_VAR} must be set"))?; + std::fs::write( + codex_home.join("environments.toml"), + format!( + r#"default = "remote" + +[[environments]] +id = "remote" +url = {remote_exec_server_url:?} +"# + ), + )?; + Ok(()) +} + +fn copy_binary_to_remote_env(container_name: &str, host_path: &Path) -> Result { + let remote_path = unique_remote_path("test_stdio_server")?; + let mkdir_output = StdCommand::new("docker") + .args([ + "exec", + container_name, + "mkdir", + "-p", + "/tmp/codex-remote-env", + ]) + .output() + .context("create remote MCP test binary directory")?; + ensure!( + mkdir_output.status.success(), + "docker mkdir remote MCP test binary directory failed: stdout={} stderr={}", + String::from_utf8_lossy(&mkdir_output.stdout).trim(), + String::from_utf8_lossy(&mkdir_output.stderr).trim() + ); + + let container_target = format!("{container_name}:{remote_path}"); + let copy_output = StdCommand::new("docker") + .arg("cp") + .arg(host_path) + .arg(&container_target) + .output() + .with_context(|| format!("copy {} to remote MCP test env", host_path.display()))?; + ensure!( + copy_output.status.success(), + "docker cp remote MCP test binary failed: stdout={} stderr={}", + String::from_utf8_lossy(©_output.stdout).trim(), + String::from_utf8_lossy(©_output.stderr).trim() + ); + + Ok(remote_path) +} + +fn unique_remote_path(binary_name: &str) -> Result { + let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); + Ok(format!( + "/tmp/codex-remote-env/{binary_name}-{}-{unique_suffix}", + std::process::id() + )) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_forwards_url_elicitation() -> Result<()> { let responses_server = responses::start_mock_server().await; From 8831901e10f0da39e0f83a067ba71babe56684c9 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 17:36:24 -0700 Subject: [PATCH 07/27] Clarify threadless MCP comments --- .../src/request_processors/mcp_processor.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 05fa8054769..e8b4026ab01 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -205,9 +205,10 @@ impl McpRequestProcessor { .await; let auth = self.auth_manager.auth().await; let environment_manager = self.thread_manager.environment_manager(); - // Status listing has no turn cwd. Use config cwd as the stdio fallback - // and the shared environment registry for explicit MCP env ids; an - // omitted MCP env id still means local. + // This threadless status path has no turn cwd or turn-selected + // environment. Use config cwd as the stdio fallback and the shared + // environment registry only for explicit MCP env ids; an omitted MCP + // env id still means local. let runtime_context = McpRuntimeContext::new( Arc::clone(&environment_manager), config.cwd.to_path_buf(), @@ -371,9 +372,10 @@ impl McpRequestProcessor { .await; let auth = self.auth_manager.auth().await; let environment_manager = self.thread_manager.environment_manager(); - // Resource reads without a thread have no turn cwd. Use config cwd as - // the stdio fallback and the shared environment registry for explicit - // MCP env ids; an omitted MCP env id still means local. + // This threadless resource-read path has no turn cwd or turn-selected + // environment. Use config cwd as the stdio fallback and the shared + // environment registry only for explicit MCP env ids; an omitted MCP + // env id still means local. let runtime_context = McpRuntimeContext::new( Arc::clone(&environment_manager), config.cwd.to_path_buf(), From ee19ab9c20ee6358ff1e31af9599e4b70b990ad5 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 17:40:36 -0700 Subject: [PATCH 08/27] Clarify MCP default environment --- codex-rs/codex-mcp/src/runtime.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index 715bd4ec347..dfa84844eb0 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -65,6 +65,9 @@ impl McpRuntimeContext { server_name: &str, config: &codex_config::McpServerConfig, ) -> Result { + // MCP configs without an explicit environment keep their historical + // behavior: they resolve to the local environment instead of selecting + // a named remote executor. let environment_id = config .environment_id .clone() From 5a5a758208f96f84165b47dce7a560006013ad37 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 17:53:20 -0700 Subject: [PATCH 09/27] Normalize MCP environment ids --- .../src/request_processors/mcp_processor.rs | 12 +-- .../src/request_processors/plugins.rs | 26 +++-- codex-rs/cli/src/mcp_cmd.rs | 2 +- .../codex-mcp/src/connection_manager_tests.rs | 8 +- codex-rs/codex-mcp/src/mcp/mod.rs | 4 +- codex-rs/codex-mcp/src/mcp/mod_tests.rs | 4 +- codex-rs/codex-mcp/src/rmcp_client.rs | 2 +- codex-rs/codex-mcp/src/runtime.rs | 102 +++++++----------- codex-rs/config/src/mcp_edit.rs | 4 +- codex-rs/config/src/mcp_edit_tests.rs | 4 +- codex-rs/config/src/mcp_types.rs | 7 +- codex-rs/config/src/mcp_types_tests.rs | 2 +- codex-rs/core-plugins/src/manager_tests.rs | 8 +- codex-rs/core/src/config/config_tests.rs | 36 +++---- codex-rs/core/src/config/edit.rs | 4 +- codex-rs/core/src/config/edit_tests.rs | 14 +-- codex-rs/core/src/mcp_skill_dependencies.rs | 4 +- codex-rs/core/src/mcp_tool_call_tests.rs | 11 +- codex-rs/core/tests/suite/code_mode.rs | 2 +- codex-rs/core/tests/suite/hooks_mcp.rs | 2 +- codex-rs/core/tests/suite/rmcp_client.rs | 19 +++- codex-rs/core/tests/suite/search_tool.rs | 6 +- codex-rs/core/tests/suite/sqlite_state.rs | 2 +- codex-rs/core/tests/suite/truncation.rs | 6 +- 24 files changed, 132 insertions(+), 159 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index e8b4026ab01..5fff443729a 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -209,10 +209,8 @@ impl McpRequestProcessor { // environment. Use config cwd as the stdio fallback and the shared // environment registry only for explicit MCP env ids; an omitted MCP // env id still means local. - let runtime_context = McpRuntimeContext::new( - Arc::clone(&environment_manager), - config.cwd.to_path_buf(), - ); + let runtime_context = + McpRuntimeContext::new(Arc::clone(&environment_manager), config.cwd.to_path_buf()); tokio::spawn(async move { Self::list_mcp_server_status_task( @@ -376,10 +374,8 @@ impl McpRequestProcessor { // environment. Use config cwd as the stdio fallback and the shared // environment registry only for explicit MCP env ids; an omitted MCP // env id still means local. - let runtime_context = McpRuntimeContext::new( - Arc::clone(&environment_manager), - config.cwd.to_path_buf(), - ); + let runtime_context = + McpRuntimeContext::new(Arc::clone(&environment_manager), config.cwd.to_path_buf()); let request_id = request_id.clone(); tokio::spawn(async move { diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 8c237b253b0..fd45a8a732c 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -959,13 +959,12 @@ impl PluginRequestProcessor { None => None, }; let environment_manager = self.thread_manager.environment_manager(); - let app_summaries = - load_plugin_app_summaries( - &config, - &outcome.plugin.apps, - Arc::clone(&environment_manager), - ) - .await; + let app_summaries = load_plugin_app_summaries( + &config, + &outcome.plugin.apps, + Arc::clone(&environment_manager), + ) + .await; let visible_skills = outcome .plugin .skills @@ -1041,13 +1040,12 @@ impl PluginRequestProcessor { .map(codex_plugin::AppConnectorId) .collect::>(); let environment_manager = self.thread_manager.environment_manager(); - let app_summaries = - load_plugin_app_summaries( - &config, - &plugin_apps, - Arc::clone(&environment_manager), - ) - .await; + let app_summaries = load_plugin_app_summaries( + &config, + &plugin_apps, + Arc::clone(&environment_manager), + ) + .await; remote_plugin_detail_to_info(remote_detail, app_summaries) } }; diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 2d4243a351b..060da0ea321 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -301,7 +301,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re let new_entry = McpServerConfig { transport: transport.clone(), - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index fa26b12043f..20593081902 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -865,7 +865,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -890,7 +890,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -988,7 +988,7 @@ fn mcp_init_error_display_prompts_for_github_pat() { http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1041,7 +1041,7 @@ fn mcp_init_error_display_reports_generic_errors() { http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index c331f61980a..1b5f44d0742 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -349,7 +349,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( .map(|(server_name, server)| { let environment_id = server .configured_config() - .map(McpRuntimeContext::configured_or_default_environment_id) + .map(|config| config.environment_id.clone()) .unwrap_or_else(|| codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string()); (server_name.clone(), environment_id) }) @@ -465,7 +465,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig { http_headers, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 31a098bb792..8724c8ce428 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -316,7 +316,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -341,7 +341,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index e32e5b12fcf..3433d84ecb3 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -29,7 +29,6 @@ use crate::elicitation::ElicitationRequestManager; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::ToolPluginProvenance; use crate::runtime::McpRuntimeContext; -use codex_exec_server::LOCAL_ENVIRONMENT_ID; use crate::runtime::emit_duration; use crate::server::EffectiveMcpServer; use crate::server::McpServerLaunch; @@ -47,6 +46,7 @@ use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::HttpClient; +use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_exec_server::ReqwestHttpClient; use codex_protocol::protocol::Event; use codex_rmcp_client::ExecutorStdioServerLauncher; diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index dfa84844eb0..5e66371f4f6 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -65,16 +65,12 @@ impl McpRuntimeContext { server_name: &str, config: &codex_config::McpServerConfig, ) -> Result { - // MCP configs without an explicit environment keep their historical - // behavior: they resolve to the local environment instead of selecting - // a named remote executor. - let environment_id = config - .environment_id - .clone() - .unwrap_or_else(|| LOCAL_ENVIRONMENT_ID.to_string()); - if environment_id == LOCAL_ENVIRONMENT_ID { - let local_environment = self.environment_manager.try_local_environment(); - if local_environment.is_none() + // MCP config parsing materializes an omitted environment id as `local`, + // so runtime resolution always starts from one explicit effective id. + let environment_id = config.environment_id.clone(); + let environment = self.environment_manager.get_environment(&environment_id); + if environment.is_none() { + if environment_id == LOCAL_ENVIRONMENT_ID && matches!( config.transport, codex_config::McpServerTransportConfig::Stdio { .. } @@ -84,32 +80,17 @@ impl McpRuntimeContext { "local stdio MCP server `{server_name}` requires a local environment" )); } - return Ok(ResolvedMcpEnvironment { - environment_id, - environment: local_environment, - }); + if environment_id != LOCAL_ENVIRONMENT_ID { + return Err(format!( + "MCP server `{server_name}` references unknown environment id `{environment_id}`" + )); + } } - - let environment = self - .environment_manager - .get_environment(&environment_id) - .ok_or_else(|| { - format!("MCP server `{server_name}` references unknown environment id `{environment_id}`") - })?; Ok(ResolvedMcpEnvironment { environment_id, - environment: Some(environment), + environment, }) } - - pub(crate) fn configured_or_default_environment_id( - config: &codex_config::McpServerConfig, - ) -> String { - config - .environment_id - .clone() - .unwrap_or_else(|| LOCAL_ENVIRONMENT_ID.to_string()) - } } pub(crate) fn emit_duration(metric: &str, duration: Duration, tags: &[(&str, &str)]) { @@ -129,7 +110,7 @@ mod tests { use super::*; - fn stdio_server(environment_id: Option<&str>) -> McpServerConfig { + fn stdio_server(environment_id: &str) -> McpServerConfig { McpServerConfig { transport: McpServerTransportConfig::Stdio { command: "echo".to_string(), @@ -138,7 +119,7 @@ mod tests { env_vars: Vec::new(), cwd: None, }, - environment_id: environment_id.map(str::to_string), + environment_id: environment_id.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -155,7 +136,7 @@ mod tests { } } - fn http_server(environment_id: Option<&str>) -> McpServerConfig { + fn http_server(environment_id: &str) -> McpServerConfig { McpServerConfig { transport: McpServerTransportConfig::StreamableHttp { url: "http://127.0.0.1:1".to_string(), @@ -163,8 +144,8 @@ mod tests { http_headers: None, env_http_headers: None, }, - environment_id: environment_id.map(str::to_string), - ..stdio_server(/*environment_id*/ None) + environment_id: environment_id.to_string(), + ..stdio_server(environment_id) } } @@ -177,7 +158,7 @@ mod tests { assert_eq!( runtime_context - .resolve_server_environment("stdio", &stdio_server(/*environment_id*/ None)), + .resolve_server_environment("stdio", &stdio_server(LOCAL_ENVIRONMENT_ID)), Err("local stdio MCP server `stdio` requires a local environment".to_string()) ); } @@ -189,12 +170,11 @@ mod tests { PathBuf::from("/tmp"), ); - assert_eq!( - McpRuntimeContext::configured_or_default_environment_id(&http_server( - /*environment_id*/ None, - )), - "local".to_string() - ); + let resolved_environment = runtime_context + .resolve_server_environment("http", &http_server(LOCAL_ENVIRONMENT_ID)) + .expect("local HTTP MCP should resolve"); + assert_eq!(resolved_environment.environment_id, LOCAL_ENVIRONMENT_ID); + assert!(resolved_environment.environment.is_none()); } #[test] @@ -205,10 +185,7 @@ mod tests { ); assert_eq!( - runtime_context.resolve_server_environment( - "stdio", - &stdio_server(/*environment_id*/ Some("remote")), - ), + runtime_context.resolve_server_environment("stdio", &stdio_server("remote")), Err("MCP server `stdio` references unknown environment id `remote`".to_string()) ); } @@ -226,18 +203,14 @@ mod tests { PathBuf::from("/tmp"), ); - assert_eq!( - McpRuntimeContext::configured_or_default_environment_id(&stdio_server( - /*environment_id*/ Some("remote"), - )), - "remote".to_string() - ); - assert_eq!( - McpRuntimeContext::configured_or_default_environment_id(&http_server( - /*environment_id*/ Some("remote"), - )), - "remote".to_string() - ); + for resolved_environment in [ + runtime_context.resolve_server_environment("stdio", &stdio_server("remote")), + runtime_context.resolve_server_environment("http", &http_server("remote")), + ] { + let resolved_environment = resolved_environment.expect("remote MCP should resolve"); + assert_eq!(resolved_environment.environment_id, "remote"); + assert!(resolved_environment.environment.is_some()); + } } #[test] @@ -247,11 +220,10 @@ mod tests { PathBuf::from("/tmp"), ); - assert_eq!( - McpRuntimeContext::configured_or_default_environment_id(&stdio_server( - /*environment_id*/ None, - )), - "local".to_string() - ); + let resolved_environment = runtime_context + .resolve_server_environment("stdio", &stdio_server(LOCAL_ENVIRONMENT_ID)) + .expect("local stdio MCP should resolve"); + assert_eq!(resolved_environment.environment_id, LOCAL_ENVIRONMENT_ID); + assert!(resolved_environment.environment.is_some()); } } diff --git a/codex-rs/config/src/mcp_edit.rs b/codex-rs/config/src/mcp_edit.rs index 0ea441ade36..24779ee513b 100644 --- a/codex-rs/config/src/mcp_edit.rs +++ b/codex-rs/config/src/mcp_edit.rs @@ -175,8 +175,8 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem { if !config.enabled { entry["enabled"] = value(false); } - if let Some(environment_id) = &config.environment_id { - entry["environment_id"] = value(environment_id.clone()); + if config.environment_id != "local" { + entry["environment_id"] = value(config.environment_id.clone()); } if config.required { entry["required"] = value(true); diff --git a/codex-rs/config/src/mcp_edit_tests.rs b/codex-rs/config/src/mcp_edit_tests.rs index c54077bd131..57a10c2d959 100644 --- a/codex-rs/config/src/mcp_edit_tests.rs +++ b/codex-rs/config/src/mcp_edit_tests.rs @@ -23,7 +23,7 @@ async fn replace_mcp_servers_serializes_per_tool_approval_overrides() -> anyhow: env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: true, @@ -101,7 +101,7 @@ async fn replace_mcp_servers_serializes_oauth_client_id() -> anyhow::Result<()> http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index 8aa9aca9823..01d4894b8fe 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -128,9 +128,8 @@ pub struct McpServerConfig { #[serde(flatten)] pub transport: McpServerTransportConfig, - /// Explicit environment id for where Codex should start this MCP server. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub environment_id: Option, + /// Effective environment id for where Codex should start this MCP server. + pub environment_id: String, /// When `false`, Codex skips initializing this MCP server. #[serde(default = "default_enabled")] @@ -352,7 +351,7 @@ impl TryFrom for McpServerConfig { Ok(Self { transport, - environment_id, + environment_id: environment_id.unwrap_or_else(|| "local".to_string()), startup_timeout_sec, tool_timeout_sec, enabled: enabled.unwrap_or_else(default_enabled), diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index f50e1589de5..ece085077a3 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -402,7 +402,7 @@ fn deserialize_ignores_unknown_server_fields() { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index e6f52cb5b45..71495d1e4fd 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -235,7 +235,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -738,7 +738,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() { http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -850,7 +850,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1012,7 +1012,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 625d1db7b5c..5114c9c20da 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -132,7 +132,7 @@ fn stdio_mcp(command: &str) -> McpServerConfig { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -157,7 +157,7 @@ fn http_mcp(url: &str) -> McpServerConfig { http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5146,7 +5146,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: Some("remote".to_string()), + environment_id: "remote".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5190,7 +5190,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { } assert_eq!(docs.startup_timeout_sec, Some(Duration::from_secs(3))); assert_eq!(docs.tool_timeout_sec, Some(Duration::from_secs(5))); - assert_eq!(docs.environment_id.as_deref(), Some("remote")); + assert_eq!(docs.environment_id, "remote"); assert!(docs.enabled); let empty = BTreeMap::new(); @@ -5493,7 +5493,7 @@ async fn replace_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5570,7 +5570,7 @@ async fn replace_mcp_servers_serializes_env_vars() -> anyhow::Result<()> { env_vars: vec!["ALPHA".into(), "BETA".into()], cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5632,7 +5632,7 @@ async fn replace_mcp_servers_serializes_sourced_env_vars() -> anyhow::Result<()> ], cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5684,7 +5684,7 @@ async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: Some(cwd_path.clone()), }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5739,7 +5739,7 @@ async fn replace_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5810,7 +5810,7 @@ async fn replace_mcp_servers_streamable_http_serializes_custom_headers() -> anyh "DOCS_AUTH".to_string(), )])), }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5893,7 +5893,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh "DOCS_AUTH".to_string(), )])), }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5929,7 +5929,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6000,7 +6000,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() "DOCS_AUTH".to_string(), )])), }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6026,7 +6026,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6115,7 +6115,7 @@ async fn replace_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: false, required: false, supports_parallel_tool_calls: false, @@ -6166,7 +6166,7 @@ async fn replace_mcp_servers_serializes_required_flag() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: true, supports_parallel_tool_calls: false, @@ -6217,7 +6217,7 @@ async fn replace_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6272,7 +6272,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index d5eca0f5e75..075af6f920d 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -300,8 +300,8 @@ mod document_helpers { if !config.enabled { entry["enabled"] = value(false); } - if let Some(environment_id) = &config.environment_id { - entry["environment_id"] = value(environment_id.clone()); + if config.environment_id != "local" { + entry["environment_id"] = value(config.environment_id.clone()); } if config.required { entry["required"] = value(true); diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index 99c91ae2608..15e79fad1f5 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -945,7 +945,7 @@ fn blocking_replace_mcp_servers_round_trips() { env_vars: vec!["FOO".into()], cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: true, @@ -975,7 +975,7 @@ fn blocking_replace_mcp_servers_round_trips() { ), env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: false, required: false, supports_parallel_tool_calls: false, @@ -1047,7 +1047,7 @@ fn blocking_replace_mcp_servers_serializes_tool_approval_overrides() { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1112,7 +1112,7 @@ foo = { command = "cmd" } env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1167,7 +1167,7 @@ foo = { command = "cmd" } # keep me env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: false, required: false, supports_parallel_tool_calls: false, @@ -1221,7 +1221,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1276,7 +1276,7 @@ foo = { command = "cmd" } env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: false, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index a6ab9664d04..0c821ee7065 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -361,7 +361,7 @@ fn mcp_dependency_to_server_config( http_headers: None, env_http_headers: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -391,7 +391,7 @@ fn mcp_dependency_to_server_config( env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index d2325bb1266..0d6b8ff1ac5 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1263,13 +1263,10 @@ async fn install_host_owned_codex_apps_manager(session: &Session, turn_context: turn_context.sub_id.clone(), session.get_tx_event(), turn_context.permission_profile(), - codex_mcp::McpRuntimeContext::new( - Arc::clone(&session.services.environment_manager), - { - #[allow(deprecated)] - turn_context.cwd.to_path_buf() - }, - ), + codex_mcp::McpRuntimeContext::new(Arc::clone(&session.services.environment_manager), { + #[allow(deprecated)] + turn_context.cwd.to_path_buf() + }), turn_context.config.codex_home.to_path_buf(), codex_mcp::codex_apps_tools_cache_key(auth.as_ref()), /*host_owned_codex_apps_enabled*/ true, diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index 5ef93ccf971..406981fad98 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -242,7 +242,7 @@ async fn run_code_mode_turn_with_rmcp_config( env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/hooks_mcp.rs b/codex-rs/core/tests/suite/hooks_mcp.rs index 2ff87516616..de0c6c11aac 100644 --- a/codex-rs/core/tests/suite/hooks_mcp.rs +++ b/codex-rs/core/tests/suite/hooks_mcp.rs @@ -182,7 +182,7 @@ fn insert_rmcp_test_server(config: &mut Config, command: String, approval_mode: env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 08f3ec75ec5..f52f00d643a 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -139,11 +139,13 @@ enum McpCallEvent { const REMOTE_MCP_ENVIRONMENT: &str = "remote"; -fn remote_aware_environment_id() -> Option { +fn remote_aware_environment_id() -> String { // These tests run locally in normal CI and against the Docker-backed // executor in full-ci. Match that shared test environment instead of // parameterizing each stdio MCP test with its own local/remote cases. - std::env::var_os(remote_env_env_var()).map(|_| REMOTE_MCP_ENVIRONMENT.to_string()) + std::env::var_os(remote_env_env_var()) + .map(|_| REMOTE_MCP_ENVIRONMENT.to_string()) + .unwrap_or_else(|| "local".to_string()) } /// Returns the stdio MCP test server command path for the active test placement. @@ -285,13 +287,22 @@ async fn wait_for_mcp_server(fixture: &TestCodex, server_name: &str) -> anyhow:: Ok(()) } -#[derive(Default)] struct TestMcpServerOptions { - environment_id: Option, + environment_id: String, supports_parallel_tool_calls: bool, tool_timeout_sec: Option, } +impl Default for TestMcpServerOptions { + fn default() -> Self { + Self { + environment_id: "local".to_string(), + supports_parallel_tool_calls: false, + tool_timeout_sec: None, + } + } +} + fn stdio_transport( command: String, env: Option>, diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index 085288ba15b..26531e9d2c9 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -1012,7 +1012,7 @@ async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, disabled_reason: None, @@ -1141,7 +1141,7 @@ async fn tool_search_surfaced_mcp_tool_errors_are_returned_to_model() -> Result< env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, disabled_reason: None, @@ -1288,7 +1288,7 @@ async fn tool_search_uses_non_app_mcp_server_instructions_as_namespace_descripti env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, disabled_reason: None, diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index 4ea994dc652..f5dd75670df 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -376,7 +376,7 @@ async fn mcp_call_marks_thread_memory_mode_polluted_when_configured() -> Result< env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index 976ffd487ac..b98c438f404 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -387,7 +387,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -487,7 +487,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -775,7 +775,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, From ca44cdf564218ebd09b34c145b1e48553764d37d Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 17:55:07 -0700 Subject: [PATCH 10/27] Preserve legacy MCP environment alias --- codex-rs/config/src/mcp_types.rs | 2 +- codex-rs/config/src/mcp_types_tests.rs | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index 01d4894b8fe..ff607dde2d3 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -229,7 +229,7 @@ pub struct RawMcpServerConfig { pub bearer_token_env_var: Option, // shared - #[serde(default)] + #[serde(default, alias = "experimental_environment")] pub environment_id: Option, #[serde(default)] pub startup_timeout_sec: Option, diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index ece085077a3..b837b00f10e 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -51,6 +51,19 @@ fn deserialize_stdio_command_server_config_with_args() { assert!(cfg.enabled); } +#[test] +fn deserialize_legacy_mcp_environment_alias() { + let cfg: McpServerConfig = toml::from_str( + r#" + command = "echo" + experimental_environment = "remote" + "#, + ) + .expect("should deserialize legacy MCP environment alias"); + + assert_eq!(cfg.environment_id, "remote"); +} + #[test] fn deserialize_stdio_command_server_config_with_arg_with_args_and_env() { let cfg: McpServerConfig = toml::from_str( From dda45568e699ffc003ce4fd02466c8a70a572b5c Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 18:05:08 -0700 Subject: [PATCH 11/27] Populate MCP status test environments --- codex-rs/tui/src/app/background_requests.rs | 2 ++ codex-rs/tui/src/app/tests.rs | 1 + codex-rs/tui/src/history_cell/tests.rs | 2 ++ 3 files changed, 5 insertions(+) diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index dcf1f672bee..d7a61f5aff6 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -1053,6 +1053,7 @@ mod tests { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, + environment_id: "local".to_string(), }, McpServerStatus { name: "disabled".to_string(), @@ -1060,6 +1061,7 @@ mod tests { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, + environment_id: "local".to_string(), }, ]; diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 92e824aec72..8570bda8d92 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -150,6 +150,7 @@ async fn handle_mcp_inventory_result_clears_committed_loading_cell() { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, + environment_id: "local".to_string(), }]), McpServerStatusDetail::ToolsAndAuthOnly, ); diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index 1307f9a828f..99beea09ea7 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -834,6 +834,7 @@ async fn mcp_tools_output_from_statuses_renders_status_only_servers() { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, + environment_id: "local".to_string(), }]; let cell = new_mcp_tools_output_from_statuses( @@ -892,6 +893,7 @@ async fn mcp_tools_output_from_statuses_renders_verbose_inventory() { mime_type: None, }], auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, + environment_id: "local".to_string(), }]; let cell = new_mcp_tools_output_from_statuses(&config, &statuses, McpServerStatusDetail::Full); From c08a9053163f036e905f7f924d4b01331e695bb0 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 18:11:48 -0700 Subject: [PATCH 12/27] Avoid MCP runtime result equality --- codex-rs/codex-mcp/src/runtime.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index 5e66371f4f6..bf37590ec92 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -156,10 +156,15 @@ mod tests { PathBuf::from("/tmp"), ); + let error = match runtime_context + .resolve_server_environment("stdio", &stdio_server(LOCAL_ENVIRONMENT_ID)) + { + Ok(_) => panic!("local stdio MCP should require a local environment"), + Err(error) => error, + }; assert_eq!( - runtime_context - .resolve_server_environment("stdio", &stdio_server(LOCAL_ENVIRONMENT_ID)), - Err("local stdio MCP server `stdio` requires a local environment".to_string()) + error, + "local stdio MCP server `stdio` requires a local environment" ); } @@ -184,9 +189,14 @@ mod tests { PathBuf::from("/tmp"), ); + let error = + match runtime_context.resolve_server_environment("stdio", &stdio_server("remote")) { + Ok(_) => panic!("unknown MCP environment should fail"), + Err(error) => error, + }; assert_eq!( - runtime_context.resolve_server_environment("stdio", &stdio_server("remote")), - Err("MCP server `stdio` references unknown environment id `remote`".to_string()) + error, + "MCP server `stdio` references unknown environment id `remote`" ); } From 984b7007c9ad5f006a6900abe86f65c4b267ea10 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 18:19:51 -0700 Subject: [PATCH 13/27] Fix MCP environment CI test setup --- codex-rs/app-server/BUILD.bazel | 1 + codex-rs/codex-mcp/src/runtime.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/codex-rs/app-server/BUILD.bazel b/codex-rs/app-server/BUILD.bazel index 6765141bdc4..fa9822fbccc 100644 --- a/codex-rs/app-server/BUILD.bazel +++ b/codex-rs/app-server/BUILD.bazel @@ -16,6 +16,7 @@ codex_rust_crate( }, extra_binaries = [ "//codex-rs/bwrap:bwrap", + "//codex-rs/rmcp-client:test_stdio_server", ], test_tags = ["no-sandbox"], ) diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index bf37590ec92..7813b5d13a6 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -223,8 +223,8 @@ mod tests { } } - #[test] - fn local_stdio_accepts_local_environment_when_available() { + #[tokio::test] + async fn local_stdio_accepts_local_environment_when_available() { let runtime_context = McpRuntimeContext::new( Arc::new(EnvironmentManager::default_for_tests()), PathBuf::from("/tmp"), From bc85985d4f05e7c9cb82663d6a0cfdf4045f8992 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 19:16:26 -0700 Subject: [PATCH 14/27] codex: fix CI failure on PR #23583 --- codex-rs/app-server/BUILD.bazel | 8 +++++-- .../app-server/tests/suite/v2/mcp_tool.rs | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/codex-rs/app-server/BUILD.bazel b/codex-rs/app-server/BUILD.bazel index fa9822fbccc..093f5eb6711 100644 --- a/codex-rs/app-server/BUILD.bazel +++ b/codex-rs/app-server/BUILD.bazel @@ -16,7 +16,11 @@ codex_rust_crate( }, extra_binaries = [ "//codex-rs/bwrap:bwrap", - "//codex-rs/rmcp-client:test_stdio_server", - ], + ] + select({ + "@platforms//os:windows": [], + "//conditions:default": [ + "//codex-rs/rmcp-client:test_stdio_server", + ], + }), test_tags = ["no-sandbox"], ) diff --git a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs index a28faa2af08..4d67b9ce661 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -1,14 +1,20 @@ use std::borrow::Cow; use std::collections::BTreeMap; +#[cfg(not(target_os = "windows"))] use std::path::Path; +#[cfg(not(target_os = "windows"))] use std::process::Command as StdCommand; use std::sync::Arc; use std::time::Duration; +#[cfg(not(target_os = "windows"))] use std::time::SystemTime; +#[cfg(not(target_os = "windows"))] use std::time::UNIX_EPOCH; +#[cfg(not(target_os = "windows"))] use anyhow::Context as _; use anyhow::Result; +#[cfg(not(target_os = "windows"))] use anyhow::ensure; use app_test_support::McpProcess; use app_test_support::create_final_assistant_message_sse_response; @@ -19,13 +25,16 @@ use axum::Router; use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; +#[cfg(not(target_os = "windows"))] use codex_app_server_protocol::ListMcpServerStatusParams; +#[cfg(not(target_os = "windows"))] use codex_app_server_protocol::ListMcpServerStatusResponse; use codex_app_server_protocol::McpElicitationSchema; use codex_app_server_protocol::McpServerElicitationAction; use codex_app_server_protocol::McpServerElicitationRequest; use codex_app_server_protocol::McpServerElicitationRequestParams; use codex_app_server_protocol::McpServerElicitationRequestResponse; +#[cfg(not(target_os = "windows"))] use codex_app_server_protocol::McpServerStatusDetail; use codex_app_server_protocol::McpServerToolCallParams; use codex_app_server_protocol::McpServerToolCallResponse; @@ -38,10 +47,13 @@ use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput as V2UserInput; +#[cfg(not(target_os = "windows"))] use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; +#[cfg(not(target_os = "windows"))] use core_test_support::get_remote_test_env; use core_test_support::responses; +#[cfg(not(target_os = "windows"))] use core_test_support::stdio_server_bin; use pretty_assertions::assert_eq; use rmcp::handler::server::ServerHandler; @@ -80,6 +92,7 @@ const ELICITATION_MESSAGE: &str = "Allow this request?"; const URL_ELICITATION_TRIGGER_MESSAGE: &str = "auth"; const URL_ELICITATION_MESSAGE: &str = "Sign in to GitHub to continue."; const URL_ELICITATION_URL: &str = "https://github.example/login/device"; +#[cfg(not(target_os = "windows"))] const REMOTE_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_TEST_REMOTE_EXEC_SERVER_URL"; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -200,6 +213,7 @@ async fn mcp_server_tool_call_returns_error_for_unknown_thread() -> Result<()> { Ok(()) } +#[cfg(not(target_os = "windows"))] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_stdio_servers_use_configured_local_and_remote_environments() -> Result<()> { let Some(remote_env) = get_remote_test_env() else { @@ -265,6 +279,7 @@ async fn mcp_stdio_servers_use_configured_local_and_remote_environments() -> Res Ok(()) } +#[cfg(not(target_os = "windows"))] #[tokio::test] async fn mcp_stdio_server_requires_local_environment_when_config_omits_environment_id() -> Result<()> { @@ -425,6 +440,7 @@ url = "{mcp_server_url}/mcp" Ok(()) } +#[cfg(not(target_os = "windows"))] async fn start_test_thread(mcp: &mut McpProcess) -> Result { let thread_start_id = mcp .send_thread_start_request(ThreadStartParams { @@ -441,6 +457,7 @@ async fn start_test_thread(mcp: &mut McpProcess) -> Result { Ok(thread.id) } +#[cfg(not(target_os = "windows"))] async fn assert_echo_tool_call( mcp: &mut McpProcess, thread_id: &str, @@ -475,6 +492,7 @@ async fn assert_echo_tool_call( Ok(()) } +#[cfg(not(target_os = "windows"))] fn append_stdio_mcp_config( codex_home: &Path, local_stdio_server_bin: &str, @@ -497,6 +515,7 @@ cwd = "/tmp" Ok(()) } +#[cfg(not(target_os = "windows"))] fn append_local_stdio_mcp_config(codex_home: &Path, local_stdio_server_bin: &str) -> Result<()> { let config_path = codex_home.join("config.toml"); let mut config_toml = std::fs::read_to_string(&config_path)?; @@ -510,6 +529,7 @@ command = {local_stdio_server_bin:?} Ok(()) } +#[cfg(not(target_os = "windows"))] fn write_remote_environment_config(codex_home: &Path) -> Result<()> { let remote_exec_server_url = std::env::var(REMOTE_EXEC_SERVER_URL_ENV_VAR) .with_context(|| format!("{REMOTE_EXEC_SERVER_URL_ENV_VAR} must be set"))?; @@ -527,6 +547,7 @@ url = {remote_exec_server_url:?} Ok(()) } +#[cfg(not(target_os = "windows"))] fn copy_binary_to_remote_env(container_name: &str, host_path: &Path) -> Result { let remote_path = unique_remote_path("test_stdio_server")?; let mkdir_output = StdCommand::new("docker") @@ -563,6 +584,7 @@ fn copy_binary_to_remote_env(container_name: &str, host_path: &Path) -> Result Result { let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); Ok(format!( From 7def3d484cb409637054c9aeff2b244f3b13e3a6 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 19:18:32 -0700 Subject: [PATCH 15/27] codex: fix CI failure on PR #23583 --- codex-rs/app-server/BUILD.bazel | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/codex-rs/app-server/BUILD.bazel b/codex-rs/app-server/BUILD.bazel index 093f5eb6711..fa9822fbccc 100644 --- a/codex-rs/app-server/BUILD.bazel +++ b/codex-rs/app-server/BUILD.bazel @@ -16,11 +16,7 @@ codex_rust_crate( }, extra_binaries = [ "//codex-rs/bwrap:bwrap", - ] + select({ - "@platforms//os:windows": [], - "//conditions:default": [ - "//codex-rs/rmcp-client:test_stdio_server", - ], - }), + "//codex-rs/rmcp-client:test_stdio_server", + ], test_tags = ["no-sandbox"], ) From 7e2c41df9a93916c0a780519691c1f2a68d6acdf Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 19:29:21 -0700 Subject: [PATCH 16/27] codex: fix CI failure on PR #23583 --- codex-rs/app-server/BUILD.bazel | 3 +++ defs.bzl | 24 +++++++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/codex-rs/app-server/BUILD.bazel b/codex-rs/app-server/BUILD.bazel index fa9822fbccc..9bdf1d3dd3a 100644 --- a/codex-rs/app-server/BUILD.bazel +++ b/codex-rs/app-server/BUILD.bazel @@ -18,5 +18,8 @@ codex_rust_crate( "//codex-rs/bwrap:bwrap", "//codex-rs/rmcp-client:test_stdio_server", ], + windows_cross_extra_binaries = [ + "//codex-rs/bwrap:bwrap", + ], test_tags = ["no-sandbox"], ) diff --git a/defs.bzl b/defs.bzl index 4af81ecef18..938b76e0814 100644 --- a/defs.bzl +++ b/defs.bzl @@ -210,7 +210,8 @@ def codex_rust_crate( test_shard_counts = {}, test_tags = [], unit_test_timeout = None, - extra_binaries = []): + extra_binaries = [], + windows_cross_extra_binaries = None): """Defines a Rust crate with library, binaries, and tests wired for Bazel + Cargo parity. The macro mirrors Cargo conventions: it builds a library when `src/` exists, @@ -255,6 +256,8 @@ def codex_rust_crate( generated from `src/**/*.rs`. extra_binaries: Additional binary labels to surface as test data and `CARGO_BIN_EXE_*` environment variables. These are only needed for binaries from a different crate. + windows_cross_extra_binaries: Optional extra binaries to surface for + generated Windows gnullvm integration tests. Defaults to extra_binaries. """ test_env = { # The launcher resolves an absolute workspace root at runtime so @@ -387,12 +390,23 @@ def codex_rust_crate( visibility = ["//visibility:public"], ) + windows_cross_sanitized_binaries = list(sanitized_binaries) + windows_cross_cargo_env = dict(cargo_env) + windows_cross_cargo_env_runfiles = dict(cargo_env_runfiles) + resolved_windows_cross_extra_binaries = extra_binaries if windows_cross_extra_binaries == None else windows_cross_extra_binaries + for binary_label in extra_binaries: sanitized_binaries.append(binary_label) binary = Label(binary_label).name cargo_env_runfiles[binary_label] = "CARGO_BIN_EXE_" + binary cargo_env["CARGO_BIN_EXE_" + binary] = "$(rlocationpath %s)" % binary_label + for binary_label in resolved_windows_cross_extra_binaries: + windows_cross_sanitized_binaries.append(binary_label) + binary = Label(binary_label).name + windows_cross_cargo_env_runfiles[binary_label] = "CARGO_BIN_EXE_" + binary + windows_cross_cargo_env["CARGO_BIN_EXE_" + binary] = "$(rlocationpath %s)" % binary_label + integration_test_kwargs = {} if integration_test_args: integration_test_kwargs["args"] = integration_test_args @@ -508,7 +522,7 @@ def codex_rust_crate( crate_name = test_crate_name, crate_root = test, srcs = [test], - data = native.glob(["tests/**"], allow_empty = True) + sanitized_binaries + test_data_extra, + data = native.glob(["tests/**"], allow_empty = True) + windows_cross_sanitized_binaries + test_data_extra, compile_data = native.glob(["tests/**"], allow_empty = True) + integration_compile_data_extra, deps = all_crate_deps(normal = True, normal_dev = True) + maybe_deps + deps_extra, rustc_flags = rustc_flags_extra + WINDOWS_RUSTC_LINK_FLAGS + [ @@ -516,7 +530,7 @@ def codex_rust_crate( "--remap-path-prefix=codex-rs=", ], rustc_env = rustc_env, - env = cargo_env, + env = windows_cross_cargo_env, target_compatible_with = WINDOWS_GNULLVM_ONLY, tags = test_tags + ["manual"], ) @@ -524,8 +538,8 @@ def codex_rust_crate( workspace_root_test( name = test_name + "-windows-cross", chdir_workspace_root = False, - env = cargo_env, - runfile_env = cargo_env_runfiles, + env = windows_cross_cargo_env, + runfile_env = windows_cross_cargo_env_runfiles, test_bin = ":" + windows_cross_test_binary, workspace_root_marker = "//codex-rs/utils/cargo-bin:repo_root.marker", target_compatible_with = WINDOWS_GNULLVM_ONLY, From 50376bdd6cd0c6a89b3c5c6899815117096c3efe Mon Sep 17 00:00:00 2001 From: starr-openai Date: Wed, 20 May 2026 00:16:22 -0700 Subject: [PATCH 17/27] codex: simplify MCP stdio environment launch --- codex-rs/app-server/BUILD.bazel | 4 - .../app-server/tests/suite/v2/mcp_tool.rs | 296 ------------------ codex-rs/codex-mcp/src/rmcp_client.rs | 27 +- codex-rs/codex-mcp/src/runtime.rs | 6 +- defs.bzl | 24 +- 5 files changed, 20 insertions(+), 337 deletions(-) diff --git a/codex-rs/app-server/BUILD.bazel b/codex-rs/app-server/BUILD.bazel index 9bdf1d3dd3a..6765141bdc4 100644 --- a/codex-rs/app-server/BUILD.bazel +++ b/codex-rs/app-server/BUILD.bazel @@ -16,10 +16,6 @@ codex_rust_crate( }, extra_binaries = [ "//codex-rs/bwrap:bwrap", - "//codex-rs/rmcp-client:test_stdio_server", - ], - windows_cross_extra_binaries = [ - "//codex-rs/bwrap:bwrap", ], test_tags = ["no-sandbox"], ) diff --git a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs index 4d67b9ce661..46e9b2bf07a 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -1,21 +1,9 @@ use std::borrow::Cow; use std::collections::BTreeMap; -#[cfg(not(target_os = "windows"))] -use std::path::Path; -#[cfg(not(target_os = "windows"))] -use std::process::Command as StdCommand; use std::sync::Arc; use std::time::Duration; -#[cfg(not(target_os = "windows"))] -use std::time::SystemTime; -#[cfg(not(target_os = "windows"))] -use std::time::UNIX_EPOCH; -#[cfg(not(target_os = "windows"))] -use anyhow::Context as _; use anyhow::Result; -#[cfg(not(target_os = "windows"))] -use anyhow::ensure; use app_test_support::McpProcess; use app_test_support::create_final_assistant_message_sse_response; use app_test_support::create_mock_responses_server_sequence; @@ -25,17 +13,11 @@ use axum::Router; use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; -#[cfg(not(target_os = "windows"))] -use codex_app_server_protocol::ListMcpServerStatusParams; -#[cfg(not(target_os = "windows"))] -use codex_app_server_protocol::ListMcpServerStatusResponse; use codex_app_server_protocol::McpElicitationSchema; use codex_app_server_protocol::McpServerElicitationAction; use codex_app_server_protocol::McpServerElicitationRequest; use codex_app_server_protocol::McpServerElicitationRequestParams; use codex_app_server_protocol::McpServerElicitationRequestResponse; -#[cfg(not(target_os = "windows"))] -use codex_app_server_protocol::McpServerStatusDetail; use codex_app_server_protocol::McpServerToolCallParams; use codex_app_server_protocol::McpServerToolCallResponse; use codex_app_server_protocol::McpToolCallStatus; @@ -47,14 +29,8 @@ use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput as V2UserInput; -#[cfg(not(target_os = "windows"))] -use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; -#[cfg(not(target_os = "windows"))] -use core_test_support::get_remote_test_env; use core_test_support::responses; -#[cfg(not(target_os = "windows"))] -use core_test_support::stdio_server_bin; use pretty_assertions::assert_eq; use rmcp::handler::server::ServerHandler; use rmcp::model::BooleanSchema; @@ -92,8 +68,6 @@ const ELICITATION_MESSAGE: &str = "Allow this request?"; const URL_ELICITATION_TRIGGER_MESSAGE: &str = "auth"; const URL_ELICITATION_MESSAGE: &str = "Sign in to GitHub to continue."; const URL_ELICITATION_URL: &str = "https://github.example/login/device"; -#[cfg(not(target_os = "windows"))] -const REMOTE_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_TEST_REMOTE_EXEC_SERVER_URL"; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_returns_tool_result() -> Result<()> { @@ -213,123 +187,6 @@ async fn mcp_server_tool_call_returns_error_for_unknown_thread() -> Result<()> { Ok(()) } -#[cfg(not(target_os = "windows"))] -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn mcp_stdio_servers_use_configured_local_and_remote_environments() -> Result<()> { - let Some(remote_env) = get_remote_test_env() else { - return Ok(()); - }; - let responses_server = responses::start_mock_server().await; - let codex_home = TempDir::new()?; - write_mock_responses_config_toml( - codex_home.path(), - &responses_server.uri(), - &BTreeMap::new(), - /*auto_compact_limit*/ 1024, - /*requires_openai_auth*/ None, - "mock_provider", - "compact", - )?; - - let local_stdio_server_bin = stdio_server_bin()?; - let remote_stdio_server_bin = copy_binary_to_remote_env( - &remote_env.container_name, - Path::new(&local_stdio_server_bin), - )?; - write_remote_environment_config(codex_home.path())?; - append_stdio_mcp_config( - codex_home.path(), - &local_stdio_server_bin, - &remote_stdio_server_bin, - )?; - - let mut mcp = McpProcess::new(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let thread_id = start_test_thread(&mut mcp).await?; - - assert_echo_tool_call(&mut mcp, &thread_id, "remote_stdio", "hello remote").await?; - assert_echo_tool_call(&mut mcp, &thread_id, "local_stdio", "hello local").await?; - - let request_id = mcp - .send_list_mcp_server_status_request(ListMcpServerStatusParams { - cursor: None, - limit: None, - detail: Some(McpServerStatusDetail::ToolsAndAuthOnly), - }) - .await?; - let response = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(request_id)), - ) - .await??; - let response: ListMcpServerStatusResponse = to_response(response)?; - let environment_ids = response - .data - .into_iter() - .map(|status| (status.name, status.environment_id)) - .collect::>(); - assert_eq!( - environment_ids, - BTreeMap::from([ - ("local_stdio".to_string(), "local".to_string()), - ("remote_stdio".to_string(), "remote".to_string()), - ]) - ); - - Ok(()) -} - -#[cfg(not(target_os = "windows"))] -#[tokio::test] -async fn mcp_stdio_server_requires_local_environment_when_config_omits_environment_id() -> Result<()> -{ - let responses_server = responses::start_mock_server().await; - let codex_home = TempDir::new()?; - write_mock_responses_config_toml( - codex_home.path(), - &responses_server.uri(), - &BTreeMap::new(), - /*auto_compact_limit*/ 1024, - /*requires_openai_auth*/ None, - "mock_provider", - "compact", - )?; - append_local_stdio_mcp_config(codex_home.path(), &stdio_server_bin()?)?; - - let mut mcp = McpProcess::new_with_env( - codex_home.path(), - &[(CODEX_EXEC_SERVER_URL_ENV_VAR, Some("none"))], - ) - .await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let thread_id = start_test_thread(&mut mcp).await?; - - let request_id = mcp - .send_mcp_server_tool_call_request(McpServerToolCallParams { - thread_id, - server: "local_stdio".to_string(), - tool: "echo".to_string(), - arguments: Some(json!({ - "message": "hello local", - })), - meta: None, - }) - .await?; - let error: JSONRPCError = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_error_message(RequestId::Integer(request_id)), - ) - .await??; - assert!( - error.error.message.contains( - "failed to get client: MCP startup failed: local stdio MCP server `local_stdio` requires a local environment" - ), - "unexpected MCP tool-call error: {error:?}" - ); - - Ok(()) -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_round_trips_elicitation() -> Result<()> { let responses_server = responses::start_mock_server().await; @@ -440,159 +297,6 @@ url = "{mcp_server_url}/mcp" Ok(()) } -#[cfg(not(target_os = "windows"))] -async fn start_test_thread(mcp: &mut McpProcess) -> Result { - let thread_start_id = mcp - .send_thread_start_request(ThreadStartParams { - model: Some("mock-model".to_string()), - ..Default::default() - }) - .await?; - let thread_start_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(thread_start_id)), - ) - .await??; - let ThreadStartResponse { thread, .. } = to_response(thread_start_resp)?; - Ok(thread.id) -} - -#[cfg(not(target_os = "windows"))] -async fn assert_echo_tool_call( - mcp: &mut McpProcess, - thread_id: &str, - server: &str, - message: &str, -) -> Result<()> { - let request_id = mcp - .send_mcp_server_tool_call_request(McpServerToolCallParams { - thread_id: thread_id.to_string(), - server: server.to_string(), - tool: "echo".to_string(), - arguments: Some(json!({ - "message": message, - })), - meta: None, - }) - .await?; - let response = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(request_id)), - ) - .await??; - let response: McpServerToolCallResponse = to_response(response)?; - assert_eq!( - response.structured_content, - Some(json!({ - "echo": format!("ECHOING: {message}"), - "env": null, - })) - ); - assert_eq!(response.is_error, Some(false)); - Ok(()) -} - -#[cfg(not(target_os = "windows"))] -fn append_stdio_mcp_config( - codex_home: &Path, - local_stdio_server_bin: &str, - remote_stdio_server_bin: &str, -) -> Result<()> { - let config_path = codex_home.join("config.toml"); - let mut config_toml = std::fs::read_to_string(&config_path)?; - config_toml.push_str(&format!( - r#" -[mcp_servers.local_stdio] -command = {local_stdio_server_bin:?} - -[mcp_servers.remote_stdio] -environment_id = "remote" -command = {remote_stdio_server_bin:?} -cwd = "/tmp" -"# - )); - std::fs::write(config_path, config_toml)?; - Ok(()) -} - -#[cfg(not(target_os = "windows"))] -fn append_local_stdio_mcp_config(codex_home: &Path, local_stdio_server_bin: &str) -> Result<()> { - let config_path = codex_home.join("config.toml"); - let mut config_toml = std::fs::read_to_string(&config_path)?; - config_toml.push_str(&format!( - r#" -[mcp_servers.local_stdio] -command = {local_stdio_server_bin:?} -"# - )); - std::fs::write(config_path, config_toml)?; - Ok(()) -} - -#[cfg(not(target_os = "windows"))] -fn write_remote_environment_config(codex_home: &Path) -> Result<()> { - let remote_exec_server_url = std::env::var(REMOTE_EXEC_SERVER_URL_ENV_VAR) - .with_context(|| format!("{REMOTE_EXEC_SERVER_URL_ENV_VAR} must be set"))?; - std::fs::write( - codex_home.join("environments.toml"), - format!( - r#"default = "remote" - -[[environments]] -id = "remote" -url = {remote_exec_server_url:?} -"# - ), - )?; - Ok(()) -} - -#[cfg(not(target_os = "windows"))] -fn copy_binary_to_remote_env(container_name: &str, host_path: &Path) -> Result { - let remote_path = unique_remote_path("test_stdio_server")?; - let mkdir_output = StdCommand::new("docker") - .args([ - "exec", - container_name, - "mkdir", - "-p", - "/tmp/codex-remote-env", - ]) - .output() - .context("create remote MCP test binary directory")?; - ensure!( - mkdir_output.status.success(), - "docker mkdir remote MCP test binary directory failed: stdout={} stderr={}", - String::from_utf8_lossy(&mkdir_output.stdout).trim(), - String::from_utf8_lossy(&mkdir_output.stderr).trim() - ); - - let container_target = format!("{container_name}:{remote_path}"); - let copy_output = StdCommand::new("docker") - .arg("cp") - .arg(host_path) - .arg(&container_target) - .output() - .with_context(|| format!("copy {} to remote MCP test env", host_path.display()))?; - ensure!( - copy_output.status.success(), - "docker cp remote MCP test binary failed: stdout={} stderr={}", - String::from_utf8_lossy(©_output.stdout).trim(), - String::from_utf8_lossy(©_output.stderr).trim() - ); - - Ok(remote_path) -} - -#[cfg(not(target_os = "windows"))] -fn unique_remote_path(binary_name: &str) -> Result { - let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); - Ok(format!( - "/tmp/codex-remote-env/{binary_name}-{}-{unique_suffix}", - std::process::id() - )) -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_forwards_url_elicitation() -> Result<()> { let responses_server = responses::start_mock_server().await; diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 3433d84ecb3..df2e1b1f3e7 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -50,7 +50,6 @@ use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_exec_server::ReqwestHttpClient; use codex_protocol::protocol::Event; use codex_rmcp_client::ExecutorStdioServerLauncher; -use codex_rmcp_client::LocalStdioServerLauncher; use codex_rmcp_client::RmcpClient; use codex_rmcp_client::StdioServerLauncher; use futures::future::BoxFuture; @@ -587,25 +586,19 @@ async fn make_rmcp_client( .map(|(key, value)| (key.into(), value.into())) .collect::>() }); - let launcher = if resolved_environment.environment_id == LOCAL_ENVIRONMENT_ID { - Arc::new(LocalStdioServerLauncher::new( - runtime_context.fallback_cwd(), - )) as Arc - } else { - let Some(environment) = resolved_environment.environment else { - return Err(StartupOutcomeError::from(anyhow!( - "MCP server `{server_name}` resolved without an execution environment" - ))); - }; - Arc::new(ExecutorStdioServerLauncher::new( - environment.get_exec_backend(), - runtime_context.fallback_cwd(), - )) + let Some(environment) = resolved_environment.environment else { + return Err(StartupOutcomeError::from(anyhow!( + "MCP server `{server_name}` resolved without an execution environment" + ))); }; + let launcher = Arc::new(ExecutorStdioServerLauncher::new( + environment.get_exec_backend(), + runtime_context.fallback_cwd(), + )) as Arc; // `RmcpClient` always sees a launched MCP stdio server. The - // launcher hides whether that means a local child process or an - // executor process whose stdin/stdout bytes cross the process API. + // executor-backed launcher hides whether the selected environment + // owns a local or remote process backend. RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd, launcher) .await .map_err(|err| StartupOutcomeError::from(anyhow!(err))) diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index 7813b5d13a6..3625e698689 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -68,7 +68,11 @@ impl McpRuntimeContext { // MCP config parsing materializes an omitted environment id as `local`, // so runtime resolution always starts from one explicit effective id. let environment_id = config.environment_id.clone(); - let environment = self.environment_manager.get_environment(&environment_id); + let environment = if environment_id == LOCAL_ENVIRONMENT_ID { + self.environment_manager.try_local_environment() + } else { + self.environment_manager.get_environment(&environment_id) + }; if environment.is_none() { if environment_id == LOCAL_ENVIRONMENT_ID && matches!( diff --git a/defs.bzl b/defs.bzl index 938b76e0814..4af81ecef18 100644 --- a/defs.bzl +++ b/defs.bzl @@ -210,8 +210,7 @@ def codex_rust_crate( test_shard_counts = {}, test_tags = [], unit_test_timeout = None, - extra_binaries = [], - windows_cross_extra_binaries = None): + extra_binaries = []): """Defines a Rust crate with library, binaries, and tests wired for Bazel + Cargo parity. The macro mirrors Cargo conventions: it builds a library when `src/` exists, @@ -256,8 +255,6 @@ def codex_rust_crate( generated from `src/**/*.rs`. extra_binaries: Additional binary labels to surface as test data and `CARGO_BIN_EXE_*` environment variables. These are only needed for binaries from a different crate. - windows_cross_extra_binaries: Optional extra binaries to surface for - generated Windows gnullvm integration tests. Defaults to extra_binaries. """ test_env = { # The launcher resolves an absolute workspace root at runtime so @@ -390,23 +387,12 @@ def codex_rust_crate( visibility = ["//visibility:public"], ) - windows_cross_sanitized_binaries = list(sanitized_binaries) - windows_cross_cargo_env = dict(cargo_env) - windows_cross_cargo_env_runfiles = dict(cargo_env_runfiles) - resolved_windows_cross_extra_binaries = extra_binaries if windows_cross_extra_binaries == None else windows_cross_extra_binaries - for binary_label in extra_binaries: sanitized_binaries.append(binary_label) binary = Label(binary_label).name cargo_env_runfiles[binary_label] = "CARGO_BIN_EXE_" + binary cargo_env["CARGO_BIN_EXE_" + binary] = "$(rlocationpath %s)" % binary_label - for binary_label in resolved_windows_cross_extra_binaries: - windows_cross_sanitized_binaries.append(binary_label) - binary = Label(binary_label).name - windows_cross_cargo_env_runfiles[binary_label] = "CARGO_BIN_EXE_" + binary - windows_cross_cargo_env["CARGO_BIN_EXE_" + binary] = "$(rlocationpath %s)" % binary_label - integration_test_kwargs = {} if integration_test_args: integration_test_kwargs["args"] = integration_test_args @@ -522,7 +508,7 @@ def codex_rust_crate( crate_name = test_crate_name, crate_root = test, srcs = [test], - data = native.glob(["tests/**"], allow_empty = True) + windows_cross_sanitized_binaries + test_data_extra, + data = native.glob(["tests/**"], allow_empty = True) + sanitized_binaries + test_data_extra, compile_data = native.glob(["tests/**"], allow_empty = True) + integration_compile_data_extra, deps = all_crate_deps(normal = True, normal_dev = True) + maybe_deps + deps_extra, rustc_flags = rustc_flags_extra + WINDOWS_RUSTC_LINK_FLAGS + [ @@ -530,7 +516,7 @@ def codex_rust_crate( "--remap-path-prefix=codex-rs=", ], rustc_env = rustc_env, - env = windows_cross_cargo_env, + env = cargo_env, target_compatible_with = WINDOWS_GNULLVM_ONLY, tags = test_tags + ["manual"], ) @@ -538,8 +524,8 @@ def codex_rust_crate( workspace_root_test( name = test_name + "-windows-cross", chdir_workspace_root = False, - env = windows_cross_cargo_env, - runfile_env = windows_cross_cargo_env_runfiles, + env = cargo_env, + runfile_env = cargo_env_runfiles, test_bin = ":" + windows_cross_test_binary, workspace_root_marker = "//codex-rs/utils/cargo-bin:repo_root.marker", target_compatible_with = WINDOWS_GNULLVM_ONLY, From 406cebaaa7ffc7c4ad7da2fac95f46f47d525f9c Mon Sep 17 00:00:00 2001 From: starr-openai Date: Wed, 20 May 2026 00:32:58 -0700 Subject: [PATCH 18/27] codex: centralize MCP local environment default --- .../src/request_processors/mcp_processor.rs | 16 +++--- codex-rs/codex-mcp/src/mcp/mod.rs | 10 +--- codex-rs/codex-mcp/src/rmcp_client.rs | 17 +----- codex-rs/codex-mcp/src/runtime.rs | 56 ++++++++++++++----- codex-rs/codex-mcp/src/server.rs | 6 ++ codex-rs/config/src/lib.rs | 1 + codex-rs/config/src/mcp_edit.rs | 2 +- codex-rs/config/src/mcp_types.rs | 10 +++- codex-rs/core/src/config/edit.rs | 2 +- 9 files changed, 71 insertions(+), 49 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 5fff443729a..5743e7cc942 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -270,7 +270,6 @@ impl McpRequestProcessor { ) .await; - let effective_servers = effective_mcp_servers(&mcp_config, auth.as_ref()); let McpServerStatusSnapshot { tools_by_server, resources, @@ -278,15 +277,16 @@ impl McpRequestProcessor { auth_statuses, environment_ids, } = snapshot; - - let mut server_names: Vec = config + let environment_ids = config .mcp_servers + .iter() + .map(|(name, config)| (name.clone(), config.environment_id.clone())) + .chain(environment_ids) + .collect::>(); + + let mut server_names: Vec = environment_ids .keys() .cloned() - // Include runtime-added/plugin MCP servers that are present in the - // effective runtime config even when they are not user-declared in - // `config.mcp_servers`. - .chain(effective_servers.keys().cloned()) .chain(auth_statuses.keys().cloned()) .chain(resources.keys().cloned()) .chain(resource_templates.keys().cloned()) @@ -328,7 +328,7 @@ impl McpRequestProcessor { environment_id: environment_ids .get(name) .cloned() - .unwrap_or_else(|| codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string()), + .expect("listed MCP server should have an environment id"), }) .collect(); diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 1b5f44d0742..66c2d1b3fde 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -346,13 +346,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( let environment_ids = mcp_servers .iter() - .map(|(server_name, server)| { - let environment_id = server - .configured_config() - .map(|config| config.environment_id.clone()) - .unwrap_or_else(|| codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string()); - (server_name.clone(), environment_id) - }) + .map(|(server_name, server)| (server_name.clone(), server.environment_id().to_string())) .collect(); let (tx_event, rx_event) = unbounded(); @@ -465,7 +459,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig { http_headers, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index df2e1b1f3e7..e8e44da9da6 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -45,9 +45,6 @@ use codex_async_utils::OrCancelExt; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; use codex_config::types::OAuthCredentialsStoreMode; -use codex_exec_server::HttpClient; -use codex_exec_server::LOCAL_ENVIRONMENT_ID; -use codex_exec_server::ReqwestHttpClient; use codex_protocol::protocol::Event; use codex_rmcp_client::ExecutorStdioServerLauncher; use codex_rmcp_client::RmcpClient; @@ -586,7 +583,7 @@ async fn make_rmcp_client( .map(|(key, value)| (key.into(), value.into())) .collect::>() }); - let Some(environment) = resolved_environment.environment else { + let Some(environment) = resolved_environment.environment() else { return Err(StartupOutcomeError::from(anyhow!( "MCP server `{server_name}` resolved without an execution environment" ))); @@ -609,17 +606,7 @@ async fn make_rmcp_client( env_http_headers, bearer_token_env_var, } => { - let http_client: Arc = - if resolved_environment.environment_id == LOCAL_ENVIRONMENT_ID { - Arc::new(ReqwestHttpClient) - } else { - let Some(environment) = resolved_environment.environment else { - return Err(StartupOutcomeError::from(anyhow!( - "MCP server `{server_name}` resolved without an HTTP environment" - ))); - }; - environment.get_http_client() - }; + let http_client = resolved_environment.http_client(); let resolved_bearer_token = match resolve_bearer_token(server_name, bearer_token_env_var.as_deref()) { Ok(token) => token, diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index 3625e698689..ded002f4a2f 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -9,9 +9,11 @@ use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; +use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; use codex_exec_server::Environment; use codex_exec_server::EnvironmentManager; -use codex_exec_server::LOCAL_ENVIRONMENT_ID; +use codex_exec_server::HttpClient; +use codex_exec_server::ReqwestHttpClient; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::SandboxPolicy; @@ -33,8 +35,25 @@ pub struct SandboxState { /// Resolved environment placement for one MCP server startup. #[derive(Clone)] pub struct ResolvedMcpEnvironment { - pub environment_id: String, - pub environment: Option>, + environment_id: String, + environment: Option>, +} + +impl ResolvedMcpEnvironment { + pub(crate) fn environment_id(&self) -> &str { + &self.environment_id + } + + pub(crate) fn environment(&self) -> Option> { + self.environment.as_ref().map(Arc::clone) + } + + pub(crate) fn http_client(&self) -> Arc { + self.environment.as_ref().map_or_else( + || Arc::new(ReqwestHttpClient) as Arc, + |environment| environment.get_http_client(), + ) + } } /// Runtime context used when resolving per-server MCP environment placement. @@ -68,13 +87,14 @@ impl McpRuntimeContext { // MCP config parsing materializes an omitted environment id as `local`, // so runtime resolution always starts from one explicit effective id. let environment_id = config.environment_id.clone(); - let environment = if environment_id == LOCAL_ENVIRONMENT_ID { + let is_local_environment = config.is_local_environment(); + let environment = if is_local_environment { self.environment_manager.try_local_environment() } else { self.environment_manager.get_environment(&environment_id) }; if environment.is_none() { - if environment_id == LOCAL_ENVIRONMENT_ID + if is_local_environment && matches!( config.transport, codex_config::McpServerTransportConfig::Stdio { .. } @@ -84,7 +104,7 @@ impl McpRuntimeContext { "local stdio MCP server `{server_name}` requires a local environment" )); } - if environment_id != LOCAL_ENVIRONMENT_ID { + if !is_local_environment { return Err(format!( "MCP server `{server_name}` references unknown environment id `{environment_id}`" )); @@ -161,7 +181,7 @@ mod tests { ); let error = match runtime_context - .resolve_server_environment("stdio", &stdio_server(LOCAL_ENVIRONMENT_ID)) + .resolve_server_environment("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) { Ok(_) => panic!("local stdio MCP should require a local environment"), Err(error) => error, @@ -180,10 +200,13 @@ mod tests { ); let resolved_environment = runtime_context - .resolve_server_environment("http", &http_server(LOCAL_ENVIRONMENT_ID)) + .resolve_server_environment("http", &http_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) .expect("local HTTP MCP should resolve"); - assert_eq!(resolved_environment.environment_id, LOCAL_ENVIRONMENT_ID); - assert!(resolved_environment.environment.is_none()); + assert_eq!( + resolved_environment.environment_id(), + DEFAULT_MCP_SERVER_ENVIRONMENT_ID + ); + assert!(resolved_environment.environment().is_none()); } #[test] @@ -222,8 +245,8 @@ mod tests { runtime_context.resolve_server_environment("http", &http_server("remote")), ] { let resolved_environment = resolved_environment.expect("remote MCP should resolve"); - assert_eq!(resolved_environment.environment_id, "remote"); - assert!(resolved_environment.environment.is_some()); + assert_eq!(resolved_environment.environment_id(), "remote"); + assert!(resolved_environment.environment().is_some()); } } @@ -235,9 +258,12 @@ mod tests { ); let resolved_environment = runtime_context - .resolve_server_environment("stdio", &stdio_server(LOCAL_ENVIRONMENT_ID)) + .resolve_server_environment("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) .expect("local stdio MCP should resolve"); - assert_eq!(resolved_environment.environment_id, LOCAL_ENVIRONMENT_ID); - assert!(resolved_environment.environment.is_some()); + assert_eq!( + resolved_environment.environment_id(), + DEFAULT_MCP_SERVER_ENVIRONMENT_ID + ); + assert!(resolved_environment.environment().is_some()); } } diff --git a/codex-rs/codex-mcp/src/server.rs b/codex-rs/codex-mcp/src/server.rs index d8fb5a11f26..aa69e67313d 100644 --- a/codex-rs/codex-mcp/src/server.rs +++ b/codex-rs/codex-mcp/src/server.rs @@ -30,6 +30,12 @@ impl EffectiveMcpServer { } } + pub fn environment_id(&self) -> &str { + match &self.launch { + McpServerLaunch::Configured(config) => &config.environment_id, + } + } + pub fn enabled(&self) -> bool { match &self.launch { McpServerLaunch::Configured(config) => config.enabled, diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 32065e36823..5ae2688036a 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -94,6 +94,7 @@ pub use marketplace_edit::remove_user_marketplace_config; pub use mcp_edit::ConfigEditsBuilder; pub use mcp_edit::load_global_mcp_servers; pub use mcp_types::AppToolApproval; +pub use mcp_types::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; pub use mcp_types::McpServerConfig; pub use mcp_types::McpServerDisabledReason; pub use mcp_types::McpServerEnvVar; diff --git a/codex-rs/config/src/mcp_edit.rs b/codex-rs/config/src/mcp_edit.rs index 24779ee513b..7e062e29984 100644 --- a/codex-rs/config/src/mcp_edit.rs +++ b/codex-rs/config/src/mcp_edit.rs @@ -175,7 +175,7 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem { if !config.enabled { entry["enabled"] = value(false); } - if config.environment_id != "local" { + if !config.is_local_environment() { entry["environment_id"] = value(config.environment_id.clone()); } if config.required { diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index ff607dde2d3..5d8291805f9 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -13,6 +13,9 @@ use serde::de::Error as SerdeError; use crate::RequirementSource; +/// Effective MCP environment id when config omits `environment_id`. +pub const DEFAULT_MCP_SERVER_ENVIRONMENT_ID: &str = "local"; + #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, Default, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum AppToolApproval { @@ -189,6 +192,10 @@ pub struct McpServerConfig { } impl McpServerConfig { + pub fn is_local_environment(&self) -> bool { + self.environment_id == DEFAULT_MCP_SERVER_ENVIRONMENT_ID + } + pub fn oauth_client_id(&self) -> Option<&str> { self.oauth .as_ref() @@ -351,7 +358,8 @@ impl TryFrom for McpServerConfig { Ok(Self { transport, - environment_id: environment_id.unwrap_or_else(|| "local".to_string()), + environment_id: environment_id + .unwrap_or_else(|| DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string()), startup_timeout_sec, tool_timeout_sec, enabled: enabled.unwrap_or_else(default_enabled), diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index 075af6f920d..1b5cd879c45 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -300,7 +300,7 @@ mod document_helpers { if !config.enabled { entry["enabled"] = value(false); } - if config.environment_id != "local" { + if !config.is_local_environment() { entry["environment_id"] = value(config.environment_id.clone()); } if config.required { From 8ed88b545cf055e4c07910a46e575e43f3cb5b26 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Wed, 20 May 2026 10:20:48 -0700 Subject: [PATCH 19/27] codex: restore local MCP stdio launcher --- codex-rs/codex-mcp/src/rmcp_client.rs | 32 +++++++++++++++++---------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index e8e44da9da6..7e7a29547fa 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -47,6 +47,7 @@ use codex_config::McpServerTransportConfig; use codex_config::types::OAuthCredentialsStoreMode; use codex_protocol::protocol::Event; use codex_rmcp_client::ExecutorStdioServerLauncher; +use codex_rmcp_client::LocalStdioServerLauncher; use codex_rmcp_client::RmcpClient; use codex_rmcp_client::StdioServerLauncher; use futures::future::BoxFuture; @@ -566,6 +567,7 @@ async fn make_rmcp_client( let resolved_environment = runtime_context .resolve_server_environment(server_name, &config) .map_err(|err| StartupOutcomeError::from(anyhow!(err)))?; + let is_local_environment = config.is_local_environment(); let McpServerConfig { transport, .. } = config; match transport { @@ -583,19 +585,25 @@ async fn make_rmcp_client( .map(|(key, value)| (key.into(), value.into())) .collect::>() }); - let Some(environment) = resolved_environment.environment() else { - return Err(StartupOutcomeError::from(anyhow!( - "MCP server `{server_name}` resolved without an execution environment" - ))); + let launcher = if is_local_environment { + // TODO(starr): Unify local stdio MCP launch with + // `ExecutorStdioServerLauncher` once the executor-backed path + // preserves `LocalStdioServerLauncher` semantics. + Arc::new(LocalStdioServerLauncher::new( + runtime_context.fallback_cwd(), + )) as Arc + } else { + let Some(environment) = resolved_environment.environment() else { + return Err(StartupOutcomeError::from(anyhow!( + "MCP server `{server_name}` resolved without an execution environment" + ))); + }; + Arc::new(ExecutorStdioServerLauncher::new( + environment.get_exec_backend(), + runtime_context.fallback_cwd(), + )) as Arc }; - let launcher = Arc::new(ExecutorStdioServerLauncher::new( - environment.get_exec_backend(), - runtime_context.fallback_cwd(), - )) as Arc; - - // `RmcpClient` always sees a launched MCP stdio server. The - // executor-backed launcher hides whether the selected environment - // owns a local or remote process backend. + RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd, launcher) .await .map_err(|err| StartupOutcomeError::from(anyhow!(err))) From 726ff39571f2f215636dee1e0dc8f4bbe8b0be9e Mon Sep 17 00:00:00 2001 From: starr-openai Date: Wed, 20 May 2026 10:41:01 -0700 Subject: [PATCH 20/27] codex: remove dead MCP env residue --- codex-rs/app-server/src/request_processors.rs | 1 - codex-rs/codex-mcp/src/runtime.rs | 21 ++----------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index 6942049d1b4..311ab29d534 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -345,7 +345,6 @@ use codex_mcp::McpServerStatusSnapshot; use codex_mcp::McpSnapshotDetail; use codex_mcp::collect_mcp_server_status_snapshot_with_detail; use codex_mcp::discover_supported_scopes; -use codex_mcp::effective_mcp_servers; use codex_mcp::read_mcp_resource as read_mcp_resource_without_thread; use codex_mcp::resolve_oauth_scopes; use codex_memories_write::clear_memory_roots_contents; diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index ded002f4a2f..dfcea7d2033 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -9,7 +9,6 @@ use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; -use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; use codex_exec_server::Environment; use codex_exec_server::EnvironmentManager; use codex_exec_server::HttpClient; @@ -35,15 +34,10 @@ pub struct SandboxState { /// Resolved environment placement for one MCP server startup. #[derive(Clone)] pub struct ResolvedMcpEnvironment { - environment_id: String, environment: Option>, } impl ResolvedMcpEnvironment { - pub(crate) fn environment_id(&self) -> &str { - &self.environment_id - } - pub(crate) fn environment(&self) -> Option> { self.environment.as_ref().map(Arc::clone) } @@ -110,10 +104,7 @@ impl McpRuntimeContext { )); } } - Ok(ResolvedMcpEnvironment { - environment_id, - environment, - }) + Ok(ResolvedMcpEnvironment { environment }) } } @@ -127,6 +118,7 @@ pub(crate) fn emit_duration(metric: &str, duration: Duration, tags: &[(&str, &st mod tests { use std::collections::HashMap; + use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; use codex_exec_server::EnvironmentManager; @@ -202,10 +194,6 @@ mod tests { let resolved_environment = runtime_context .resolve_server_environment("http", &http_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) .expect("local HTTP MCP should resolve"); - assert_eq!( - resolved_environment.environment_id(), - DEFAULT_MCP_SERVER_ENVIRONMENT_ID - ); assert!(resolved_environment.environment().is_none()); } @@ -245,7 +233,6 @@ mod tests { runtime_context.resolve_server_environment("http", &http_server("remote")), ] { let resolved_environment = resolved_environment.expect("remote MCP should resolve"); - assert_eq!(resolved_environment.environment_id(), "remote"); assert!(resolved_environment.environment().is_some()); } } @@ -260,10 +247,6 @@ mod tests { let resolved_environment = runtime_context .resolve_server_environment("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) .expect("local stdio MCP should resolve"); - assert_eq!( - resolved_environment.environment_id(), - DEFAULT_MCP_SERVER_ENVIRONMENT_ID - ); assert!(resolved_environment.environment().is_some()); } } From 89b33cc24798829e546307f6cd7c0b7e109e8f74 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Wed, 20 May 2026 14:04:38 -0700 Subject: [PATCH 21/27] Model MCP runtime placement explicitly --- .../codex_app_server_protocol.schemas.json | 50 ++++- .../codex_app_server_protocol.v2.schemas.json | 54 ++++- .../json/v2/ListMcpServerStatusResponse.json | 50 ++++- .../v2/McpServerRuntimePlacement.ts | 5 + .../schema/typescript/v2/McpServerStatus.ts | 3 +- .../schema/typescript/v2/index.ts | 1 + .../src/protocol/v2/mcp.rs | 14 +- codex-rs/app-server/README.md | 2 +- .../src/request_processors/mcp_processor.rs | 48 +++-- codex-rs/codex-mcp/src/lib.rs | 1 + codex-rs/codex-mcp/src/mcp/mod.rs | 14 +- codex-rs/codex-mcp/src/rmcp_client.rs | 39 ++-- codex-rs/codex-mcp/src/runtime.rs | 192 +++++++++++++----- codex-rs/codex-mcp/src/server.rs | 6 +- codex-rs/config/src/mcp_types.rs | 31 ++- codex-rs/config/src/mcp_types_tests.rs | 62 +++++- codex-rs/core/src/config/config_tests.rs | 2 +- codex-rs/tui/src/app/background_requests.rs | 6 +- codex-rs/tui/src/app/tests.rs | 2 +- codex-rs/tui/src/history_cell/tests.rs | 4 +- 20 files changed, 454 insertions(+), 132 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/McpServerRuntimePlacement.ts diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index a764610f104..2fab758bcd7 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -11025,6 +11025,46 @@ "title": "McpServerRefreshResponse", "type": "object" }, + "McpServerRuntimePlacement": { + "oneOf": [ + { + "properties": { + "type": { + "enum": [ + "orchestrator" + ], + "title": "OrchestratorMcpServerRuntimePlacementType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "OrchestratorMcpServerRuntimePlacement", + "type": "object" + }, + { + "properties": { + "environmentId": { + "type": "string" + }, + "type": { + "enum": [ + "environment" + ], + "title": "EnvironmentMcpServerRuntimePlacementType", + "type": "string" + } + }, + "required": [ + "environmentId", + "type" + ], + "title": "EnvironmentMcpServerRuntimePlacement", + "type": "object" + } + ] + }, "McpServerStartupState": { "enum": [ "starting", @@ -11039,9 +11079,6 @@ "authStatus": { "$ref": "#/definitions/v2/McpAuthStatus" }, - "environmentId": { - "type": "string" - }, "name": { "type": "string" }, @@ -11057,6 +11094,9 @@ }, "type": "array" }, + "runtimePlacement": { + "$ref": "#/definitions/v2/McpServerRuntimePlacement" + }, "tools": { "additionalProperties": { "$ref": "#/definitions/v2/Tool" @@ -11066,10 +11106,10 @@ }, "required": [ "authStatus", - "environmentId", "name", "resourceTemplates", "resources", + "runtimePlacement", "tools" ], "type": "object" @@ -19037,4 +19077,4 @@ }, "title": "CodexAppServerProtocol", "type": "object" -} +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 010d83b79e4..2f81fcecc1d 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -7554,6 +7554,46 @@ "title": "McpServerRefreshResponse", "type": "object" }, + "McpServerRuntimePlacement": { + "oneOf": [ + { + "properties": { + "type": { + "enum": [ + "orchestrator" + ], + "title": "OrchestratorMcpServerRuntimePlacementType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "OrchestratorMcpServerRuntimePlacement", + "type": "object" + }, + { + "properties": { + "environmentId": { + "type": "string" + }, + "type": { + "enum": [ + "environment" + ], + "title": "EnvironmentMcpServerRuntimePlacementType", + "type": "string" + } + }, + "required": [ + "environmentId", + "type" + ], + "title": "EnvironmentMcpServerRuntimePlacement", + "type": "object" + } + ] + }, "McpServerStartupState": { "enum": [ "starting", @@ -7568,9 +7608,6 @@ "authStatus": { "$ref": "#/definitions/McpAuthStatus" }, - "environmentId": { - "type": "string" - }, "name": { "type": "string" }, @@ -7586,6 +7623,9 @@ }, "type": "array" }, + "runtimePlacement": { + "$ref": "#/definitions/McpServerRuntimePlacement" + }, "tools": { "additionalProperties": { "$ref": "#/definitions/Tool" @@ -7594,11 +7634,11 @@ } }, "required": [ - "authStatus", - "environmentId", - "name", + "authStatus", + "name", "resourceTemplates", "resources", + "runtimePlacement", "tools" ], "type": "object" @@ -16860,4 +16900,4 @@ }, "title": "CodexAppServerProtocolV2", "type": "object" -} +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json index 645b8759637..36aa6ba7a2c 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json @@ -10,14 +10,51 @@ ], "type": "string" }, + "McpServerRuntimePlacement": { + "oneOf": [ + { + "properties": { + "type": { + "enum": [ + "orchestrator" + ], + "title": "OrchestratorMcpServerRuntimePlacementType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "OrchestratorMcpServerRuntimePlacement", + "type": "object" + }, + { + "properties": { + "environmentId": { + "type": "string" + }, + "type": { + "enum": [ + "environment" + ], + "title": "EnvironmentMcpServerRuntimePlacementType", + "type": "string" + } + }, + "required": [ + "environmentId", + "type" + ], + "title": "EnvironmentMcpServerRuntimePlacement", + "type": "object" + } + ] + }, "McpServerStatus": { "properties": { "authStatus": { "$ref": "#/definitions/McpAuthStatus" }, - "environmentId": { - "type": "string" - }, "name": { "type": "string" }, @@ -33,6 +70,9 @@ }, "type": "array" }, + "runtimePlacement": { + "$ref": "#/definitions/McpServerRuntimePlacement" + }, "tools": { "additionalProperties": { "$ref": "#/definitions/Tool" @@ -42,10 +82,10 @@ }, "required": [ "authStatus", - "environmentId", "name", "resourceTemplates", "resources", + "runtimePlacement", "tools" ], "type": "object" @@ -192,4 +232,4 @@ ], "title": "ListMcpServerStatusResponse", "type": "object" -} +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerRuntimePlacement.ts b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerRuntimePlacement.ts new file mode 100644 index 00000000000..29cf2a1e132 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerRuntimePlacement.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type McpServerRuntimePlacement = { "type": "orchestrator" } | { "type": "environment", environmentId: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts index 3d4c57cd2d5..b992473e4e2 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts @@ -5,5 +5,6 @@ import type { Resource } from "../Resource"; import type { ResourceTemplate } from "../ResourceTemplate"; import type { Tool } from "../Tool"; import type { McpAuthStatus } from "./McpAuthStatus"; +import type { McpServerRuntimePlacement } from "./McpServerRuntimePlacement"; -export type McpServerStatus = { name: string, tools: { [key in string]?: Tool }, resources: Array, resourceTemplates: Array, authStatus: McpAuthStatus, environmentId: string, }; +export type McpServerStatus = { name: string, tools: { [key in string]?: Tool }, resources: Array, resourceTemplates: Array, authStatus: McpAuthStatus, runtimePlacement: McpServerRuntimePlacement, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index e9b8993c26e..6126b2a6087 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -217,6 +217,7 @@ export type { McpServerOauthLoginCompletedNotification } from "./McpServerOauthL export type { McpServerOauthLoginParams } from "./McpServerOauthLoginParams"; export type { McpServerOauthLoginResponse } from "./McpServerOauthLoginResponse"; export type { McpServerRefreshResponse } from "./McpServerRefreshResponse"; +export type { McpServerRuntimePlacement } from "./McpServerRuntimePlacement"; export type { McpServerStartupState } from "./McpServerStartupState"; export type { McpServerStatus } from "./McpServerStatus"; export type { McpServerStatusDetail } from "./McpServerStatusDetail"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs b/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs index 16cb9d7aed3..cf1068e1320 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs @@ -55,7 +55,19 @@ pub struct McpServerStatus { pub resources: Vec, pub resource_templates: Vec, pub auth_status: McpAuthStatus, - pub environment_id: String, + pub runtime_placement: McpServerRuntimePlacement, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase", tag = "type")] +#[ts(rename_all = "camelCase", tag = "type", export_to = "v2/")] +pub enum McpServerRuntimePlacement { + Orchestrator, + Environment { + #[serde(rename = "environmentId")] + #[ts(rename = "environmentId")] + environment_id: String, + }, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index bed188c0bf5..0ed8fb1041d 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -216,7 +216,7 @@ Example with notification opt-out: - `mcpServer/oauth/login` — start an OAuth login for a configured MCP server; returns an `authorization_url` and later emits `mcpServer/oauthLogin/completed` once the browser flow finishes. - `tool/requestUserInput` — prompt the user with 1–3 short questions for a tool call and return their answers (experimental). - `config/mcpServer/reload` — reload MCP server config from disk and queue a refresh for loaded threads (applied on each thread's next active turn); returns `{}`. Use this after editing `config.toml` without restarting the server. -- `mcpServerStatus/list` — enumerate configured MCP servers with their tools, auth status, and resolved `environmentId`, plus resources/resource templates for `full` detail; supports cursor+limit pagination. If `detail` is omitted, the server defaults to `full`. +- `mcpServerStatus/list` — enumerate configured MCP servers with their tools, auth status, and resolved `runtimePlacement`, plus resources/resource templates for `full` detail; supports cursor+limit pagination. If `detail` is omitted, the server defaults to `full`. - `mcpServer/resource/read` — read a resource from a configured MCP server by optional `threadId`, `server`, and `uri`, returning text/blob resource `contents`. If `threadId` is omitted, the server reads from the latest MCP config directly. - `mcpServer/tool/call` — call a tool on a thread's configured MCP server by `threadId`, `server`, `tool`, optional `arguments`, and optional `_meta`, returning the MCP tool result. - `windowsSandbox/setupStart` — start Windows sandbox setup for the selected mode (`elevated` or `unelevated`); accepts an optional absolute `cwd` to target setup for a specific workspace, returns `{ started: true }` immediately, and later emits `windowsSandbox/setupCompleted`. diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 5743e7cc942..2e22644cf0c 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -206,9 +206,8 @@ impl McpRequestProcessor { let auth = self.auth_manager.auth().await; let environment_manager = self.thread_manager.environment_manager(); // This threadless status path has no turn cwd or turn-selected - // environment. Use config cwd as the stdio fallback and the shared - // environment registry only for explicit MCP env ids; an omitted MCP - // env id still means local. + // environment. Use config cwd only as the local stdio fallback; named + // environment stdio MCPs must declare their own absolute cwd. let runtime_context = McpRuntimeContext::new(Arc::clone(&environment_manager), config.cwd.to_path_buf()); @@ -275,16 +274,21 @@ impl McpRequestProcessor { resources, resource_templates, auth_statuses, - environment_ids, + runtime_placements, } = snapshot; - let environment_ids = config + let runtime_placements = config .mcp_servers .iter() - .map(|(name, config)| (name.clone(), config.environment_id.clone())) - .chain(environment_ids) + .map(|(name, config)| { + ( + name.clone(), + codex_mcp::McpServerRuntimePlacement::from_config(config), + ) + }) + .chain(runtime_placements) .collect::>(); - let mut server_names: Vec = environment_ids + let mut server_names: Vec = runtime_placements .keys() .cloned() .chain(auth_statuses.keys().cloned()) @@ -325,10 +329,12 @@ impl McpRequestProcessor { .cloned() .unwrap_or(CoreMcpAuthStatus::Unsupported) .into(), - environment_id: environment_ids - .get(name) - .cloned() - .expect("listed MCP server should have an environment id"), + runtime_placement: app_server_runtime_placement( + runtime_placements + .get(name) + .cloned() + .expect("listed MCP server should have a runtime placement"), + ), }) .collect(); @@ -371,9 +377,8 @@ impl McpRequestProcessor { let auth = self.auth_manager.auth().await; let environment_manager = self.thread_manager.environment_manager(); // This threadless resource-read path has no turn cwd or turn-selected - // environment. Use config cwd as the stdio fallback and the shared - // environment registry only for explicit MCP env ids; an omitted MCP - // env id still means local. + // environment. Use config cwd only as the local stdio fallback; named + // environment stdio MCPs must declare their own absolute cwd. let runtime_context = McpRuntimeContext::new(Arc::clone(&environment_manager), config.cwd.to_path_buf()); let request_id = request_id.clone(); @@ -433,6 +438,19 @@ impl McpRequestProcessor { } } +fn app_server_runtime_placement( + placement: codex_mcp::McpServerRuntimePlacement, +) -> codex_app_server_protocol::McpServerRuntimePlacement { + match placement { + codex_mcp::McpServerRuntimePlacement::Orchestrator => { + codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator + } + codex_mcp::McpServerRuntimePlacement::Environment { environment_id } => { + codex_app_server_protocol::McpServerRuntimePlacement::Environment { environment_id } + } + } +} + fn with_mcp_tool_call_thread_id_meta( meta: Option, thread_id: &str, diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index 7b45bcc7a78..d097e2abc10 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -4,6 +4,7 @@ pub use elicitation::ElicitationReviewer; pub use elicitation::ElicitationReviewerHandle; pub use rmcp_client::MCP_SANDBOX_STATE_META_CAPABILITY; pub use runtime::McpRuntimeContext; +pub use runtime::McpServerRuntimePlacement; pub use runtime::SandboxState; pub use tools::ToolInfo; diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 66c2d1b3fde..a29533b7f57 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -314,7 +314,7 @@ pub struct McpServerStatusSnapshot { pub resources: HashMap>, pub resource_templates: HashMap>, pub auth_statuses: HashMap, - pub environment_ids: HashMap, + pub runtime_placements: HashMap, } pub async fn collect_mcp_server_status_snapshot_with_detail( @@ -333,7 +333,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( resources: HashMap::new(), resource_templates: HashMap::new(), auth_statuses: HashMap::new(), - environment_ids: HashMap::new(), + runtime_placements: HashMap::new(), }; } @@ -344,9 +344,9 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( ) .await; - let environment_ids = mcp_servers + let runtime_placements = mcp_servers .iter() - .map(|(server_name, server)| (server_name.clone(), server.environment_id().to_string())) + .map(|(server_name, server)| (server_name.clone(), server.runtime_placement())) .collect(); let (tx_event, rx_event) = unbounded(); @@ -374,7 +374,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( let snapshot = collect_mcp_server_status_snapshot_from_manager( &mcp_connection_manager, auth_status_entries, - environment_ids, + runtime_placements, detail, ) .await; @@ -583,7 +583,7 @@ fn convert_mcp_resource_templates( async fn collect_mcp_server_status_snapshot_from_manager( mcp_connection_manager: &McpConnectionManager, auth_status_entries: HashMap, - environment_ids: HashMap, + runtime_placements: HashMap, detail: McpSnapshotDetail, ) -> McpServerStatusSnapshot { let (tools, resources, resource_templates) = tokio::join!( @@ -622,7 +622,7 @@ async fn collect_mcp_server_status_snapshot_from_manager( resources: convert_mcp_resources(resources), resource_templates: convert_mcp_resource_templates(resource_templates), auth_statuses: auth_statuses_from_entries(&auth_status_entries), - environment_ids, + runtime_placements, } } diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 7e7a29547fa..459e98d745b 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -564,10 +564,9 @@ async fn make_rmcp_client( let config = match server.launch() { McpServerLaunch::Configured(config) => config.as_ref().clone(), }; - let resolved_environment = runtime_context - .resolve_server_environment(server_name, &config) + let resolved_runtime = runtime_context + .resolve_server_runtime(server_name, &config) .map_err(|err| StartupOutcomeError::from(anyhow!(err)))?; - let is_local_environment = config.is_local_environment(); let McpServerConfig { transport, .. } = config; match transport { @@ -585,23 +584,21 @@ async fn make_rmcp_client( .map(|(key, value)| (key.into(), value.into())) .collect::>() }); - let launcher = if is_local_environment { - // TODO(starr): Unify local stdio MCP launch with - // `ExecutorStdioServerLauncher` once the executor-backed path - // preserves `LocalStdioServerLauncher` semantics. - Arc::new(LocalStdioServerLauncher::new( - runtime_context.fallback_cwd(), - )) as Arc - } else { - let Some(environment) = resolved_environment.environment() else { - return Err(StartupOutcomeError::from(anyhow!( - "MCP server `{server_name}` resolved without an execution environment" - ))); - }; - Arc::new(ExecutorStdioServerLauncher::new( - environment.get_exec_backend(), - runtime_context.fallback_cwd(), - )) as Arc + let launcher = match &resolved_runtime { + crate::runtime::ResolvedMcpServerRuntime::Orchestrator => { + // TODO(starr): Unify local stdio MCP launch with + // `ExecutorStdioServerLauncher` once the executor-backed path + // preserves `LocalStdioServerLauncher` semantics. + Arc::new(LocalStdioServerLauncher::new( + runtime_context.fallback_cwd(), + )) as Arc + } + crate::runtime::ResolvedMcpServerRuntime::Environment(environment) => { + Arc::new(ExecutorStdioServerLauncher::new( + environment.get_exec_backend(), + runtime_context.fallback_cwd(), + )) as Arc + } }; RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd, launcher) @@ -614,7 +611,7 @@ async fn make_rmcp_client( env_http_headers, bearer_token_env_var, } => { - let http_client = resolved_environment.http_client(); + let http_client = resolved_runtime.http_client(); let resolved_bearer_token = match resolve_bearer_token(server_name, bearer_token_env_var.as_deref()) { Ok(token) => token, diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index dfcea7d2033..4d77d16081b 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -31,30 +31,48 @@ pub struct SandboxState { pub use_legacy_landlock: bool, } -/// Resolved environment placement for one MCP server startup. -#[derive(Clone)] -pub struct ResolvedMcpEnvironment { - environment: Option>, +/// Effective runtime placement for one MCP server. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum McpServerRuntimePlacement { + /// The orchestrator owns the transport directly. + Orchestrator, + /// The selected named environment owns the transport. + Environment { environment_id: String }, } -impl ResolvedMcpEnvironment { - pub(crate) fn environment(&self) -> Option> { - self.environment.as_ref().map(Arc::clone) +impl McpServerRuntimePlacement { + pub fn from_config(config: &codex_config::McpServerConfig) -> Self { + if config.is_local_environment() { + Self::Orchestrator + } else { + Self::Environment { + environment_id: config.environment_id.clone(), + } + } } +} +/// Resolved runtime handle for one MCP server startup. +#[derive(Clone)] +pub(crate) enum ResolvedMcpServerRuntime { + Orchestrator, + Environment(Arc), +} + +impl ResolvedMcpServerRuntime { pub(crate) fn http_client(&self) -> Arc { - self.environment.as_ref().map_or_else( - || Arc::new(ReqwestHttpClient) as Arc, - |environment| environment.get_http_client(), - ) + match self { + Self::Orchestrator => Arc::new(ReqwestHttpClient) as Arc, + Self::Environment(environment) => environment.get_http_client(), + } } } /// Runtime context used when resolving per-server MCP environment placement. /// /// `McpConfig` describes what servers exist. This value carries the canonical -/// environment registry plus the fallback cwd used when a stdio server omits -/// its own working directory. +/// environment registry plus the local stdio fallback cwd used when a local +/// stdio server omits its own working directory. #[derive(Clone)] pub struct McpRuntimeContext { environment_manager: Arc, @@ -73,41 +91,62 @@ impl McpRuntimeContext { self.fallback_cwd.clone() } - pub(crate) fn resolve_server_environment( + pub(crate) fn resolve_server_runtime( &self, server_name: &str, config: &codex_config::McpServerConfig, - ) -> Result { - // MCP config parsing materializes an omitted environment id as `local`, - // so runtime resolution always starts from one explicit effective id. - let environment_id = config.environment_id.clone(); - let is_local_environment = config.is_local_environment(); - let environment = if is_local_environment { - self.environment_manager.try_local_environment() - } else { - self.environment_manager.get_environment(&environment_id) - }; - if environment.is_none() { - if is_local_environment - && matches!( - config.transport, - codex_config::McpServerTransportConfig::Stdio { .. } - ) - { - return Err(format!( - "local stdio MCP server `{server_name}` requires a local environment" - )); + ) -> Result { + match McpServerRuntimePlacement::from_config(config) { + McpServerRuntimePlacement::Orchestrator => { + if self.environment_manager.try_local_environment().is_none() + && matches!( + config.transport, + codex_config::McpServerTransportConfig::Stdio { .. } + ) + { + return Err(format!( + "local stdio MCP server `{server_name}` requires a local environment" + )); + } + Ok(ResolvedMcpServerRuntime::Orchestrator) } - if !is_local_environment { - return Err(format!( - "MCP server `{server_name}` references unknown environment id `{environment_id}`" - )); + McpServerRuntimePlacement::Environment { environment_id } => { + let environment = self + .environment_manager + .get_environment(&environment_id) + .ok_or_else(|| { + format!( + "MCP server `{server_name}` references unknown environment id `{environment_id}`" + ) + })?; + ensure_remote_stdio_cwd(server_name, config)?; + Ok(ResolvedMcpServerRuntime::Environment(environment)) } } - Ok(ResolvedMcpEnvironment { environment }) } } +fn ensure_remote_stdio_cwd( + server_name: &str, + config: &codex_config::McpServerConfig, +) -> Result<(), String> { + let codex_config::McpServerTransportConfig::Stdio { cwd, .. } = &config.transport else { + return Ok(()); + }; + let Some(cwd) = cwd else { + return Err(format!( + "remote stdio MCP server `{server_name}` requires an absolute cwd" + )); + }; + if cwd.is_absolute() { + return Ok(()); + } + Err(format!( + "remote stdio MCP server `{server_name}` requires an absolute cwd, got `{}`", + cwd.display() + )) +} + pub(crate) fn emit_duration(metric: &str, duration: Duration, tags: &[(&str, &str)]) { if let Some(metrics) = codex_otel::global() { let _ = metrics.record_duration(metric, duration, tags); @@ -173,7 +212,7 @@ mod tests { ); let error = match runtime_context - .resolve_server_environment("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) + .resolve_server_runtime("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) { Ok(_) => panic!("local stdio MCP should require a local environment"), Err(error) => error, @@ -191,10 +230,13 @@ mod tests { PathBuf::from("/tmp"), ); - let resolved_environment = runtime_context - .resolve_server_environment("http", &http_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) + let resolved_runtime = runtime_context + .resolve_server_runtime("http", &http_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) .expect("local HTTP MCP should resolve"); - assert!(resolved_environment.environment().is_none()); + assert!(matches!( + resolved_runtime, + ResolvedMcpServerRuntime::Orchestrator + )); } #[test] @@ -204,11 +246,10 @@ mod tests { PathBuf::from("/tmp"), ); - let error = - match runtime_context.resolve_server_environment("stdio", &stdio_server("remote")) { - Ok(_) => panic!("unknown MCP environment should fail"), - Err(error) => error, - }; + let error = match runtime_context.resolve_server_runtime("stdio", &stdio_server("remote")) { + Ok(_) => panic!("unknown MCP environment should fail"), + Err(error) => error, + }; assert_eq!( error, "MCP server `stdio` references unknown environment id `remote`" @@ -228,12 +269,20 @@ mod tests { PathBuf::from("/tmp"), ); - for resolved_environment in [ - runtime_context.resolve_server_environment("stdio", &stdio_server("remote")), - runtime_context.resolve_server_environment("http", &http_server("remote")), + let mut remote_stdio = stdio_server("remote"); + let McpServerTransportConfig::Stdio { cwd, .. } = &mut remote_stdio.transport else { + unreachable!("stdio helper should build stdio transport"); + }; + *cwd = Some(std::env::temp_dir()); + for resolved_runtime in [ + runtime_context.resolve_server_runtime("stdio", &remote_stdio), + runtime_context.resolve_server_runtime("http", &http_server("remote")), ] { - let resolved_environment = resolved_environment.expect("remote MCP should resolve"); - assert!(resolved_environment.environment().is_some()); + let resolved_runtime = resolved_runtime.expect("remote MCP should resolve"); + assert!(matches!( + resolved_runtime, + ResolvedMcpServerRuntime::Environment(_) + )); } } @@ -244,9 +293,40 @@ mod tests { PathBuf::from("/tmp"), ); - let resolved_environment = runtime_context - .resolve_server_environment("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) + let resolved_runtime = runtime_context + .resolve_server_runtime("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) .expect("local stdio MCP should resolve"); - assert!(resolved_environment.environment().is_some()); + assert!(matches!( + resolved_runtime, + ResolvedMcpServerRuntime::Orchestrator + )); + } + + #[tokio::test] + async fn remote_stdio_requires_absolute_cwd() { + let runtime_context = McpRuntimeContext::new( + Arc::new( + EnvironmentManager::create_for_tests( + Some("ws://127.0.0.1:8765".to_string()), + /*local_runtime_paths*/ None, + ) + .await, + ), + PathBuf::from("/tmp"), + ); + let mut remote_stdio = stdio_server("remote"); + let McpServerTransportConfig::Stdio { cwd, .. } = &mut remote_stdio.transport else { + unreachable!("stdio helper should build stdio transport"); + }; + *cwd = Some(PathBuf::from("relative")); + + let error = match runtime_context.resolve_server_runtime("stdio", &remote_stdio) { + Ok(_) => panic!("remote stdio MCP should require absolute cwd"), + Err(error) => error, + }; + assert_eq!( + error, + "remote stdio MCP server `stdio` requires an absolute cwd, got `relative`" + ); } } diff --git a/codex-rs/codex-mcp/src/server.rs b/codex-rs/codex-mcp/src/server.rs index aa69e67313d..9dd84f132b4 100644 --- a/codex-rs/codex-mcp/src/server.rs +++ b/codex-rs/codex-mcp/src/server.rs @@ -1,6 +1,8 @@ use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; +use crate::runtime::McpServerRuntimePlacement; + /// The runtime launch strategy for an effective MCP server. #[derive(Debug, Clone)] pub(crate) enum McpServerLaunch { @@ -30,9 +32,9 @@ impl EffectiveMcpServer { } } - pub fn environment_id(&self) -> &str { + pub fn runtime_placement(&self) -> McpServerRuntimePlacement { match &self.launch { - McpServerLaunch::Configured(config) => &config.environment_id, + McpServerLaunch::Configured(config) => McpServerRuntimePlacement::from_config(config), } } diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index 5d8291805f9..972581b8799 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -356,10 +356,13 @@ impl TryFrom for McpServerConfig { return Err("invalid transport".to_string()); }; + let environment_id = + environment_id.unwrap_or_else(|| DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string()); + validate_remote_stdio_cwd(&transport, &environment_id)?; + Ok(Self { transport, - environment_id: environment_id - .unwrap_or_else(|| DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string()), + environment_id, startup_timeout_sec, tool_timeout_sec, enabled: enabled.unwrap_or_else(default_enabled), @@ -392,6 +395,30 @@ const fn default_enabled() -> bool { true } +fn validate_remote_stdio_cwd( + transport: &McpServerTransportConfig, + environment_id: &str, +) -> Result<(), String> { + if environment_id == DEFAULT_MCP_SERVER_ENVIRONMENT_ID { + return Ok(()); + } + let McpServerTransportConfig::Stdio { cwd, .. } = transport else { + return Ok(()); + }; + let Some(cwd) = cwd else { + return Err(format!( + "remote stdio MCP servers require an absolute cwd when environment_id is `{environment_id}`" + )); + }; + if cwd.is_absolute() { + return Ok(()); + } + Err(format!( + "remote stdio MCP servers require an absolute cwd when environment_id is `{environment_id}`, got `{}`", + cwd.display() + )) +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema)] #[serde(untagged, deny_unknown_fields, rename_all = "snake_case")] pub enum McpServerTransportConfig { diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index b837b00f10e..239955d5cae 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -53,17 +53,73 @@ fn deserialize_stdio_command_server_config_with_args() { #[test] fn deserialize_legacy_mcp_environment_alias() { - let cfg: McpServerConfig = toml::from_str( + let cwd = std::env::temp_dir(); + let cfg: McpServerConfig = toml::from_str(&format!( r#" command = "echo" experimental_environment = "remote" - "#, - ) + cwd = {cwd:?} + "# + )) .expect("should deserialize legacy MCP environment alias"); assert_eq!(cfg.environment_id, "remote"); } +#[test] +fn deserialize_remote_stdio_server_requires_absolute_cwd() { + let missing_cwd = toml::from_str::( + r#" + command = "echo" + environment_id = "remote" + "#, + ) + .expect_err("remote stdio MCP should require cwd"); + assert!( + missing_cwd + .to_string() + .contains("remote stdio MCP servers require an absolute cwd"), + "unexpected error: {missing_cwd}" + ); + + let relative_cwd = toml::from_str::( + r#" + command = "echo" + environment_id = "remote" + cwd = "relative" + "#, + ) + .expect_err("remote stdio MCP should require absolute cwd"); + assert!( + relative_cwd.to_string().contains("got `relative`"), + "unexpected error: {relative_cwd}" + ); +} + +#[test] +fn deserialize_remote_stdio_server_accepts_absolute_cwd() { + let cwd = std::env::temp_dir(); + let cfg: McpServerConfig = toml::from_str(&format!( + r#" + command = "echo" + environment_id = "remote" + cwd = {cwd:?} + "# + )) + .expect("remote stdio MCP should accept absolute cwd"); + + assert_eq!( + cfg.transport, + McpServerTransportConfig::Stdio { + command: "echo".to_string(), + args: vec![], + env: None, + env_vars: Vec::new(), + cwd: Some(cwd), + } + ); +} + #[test] fn deserialize_stdio_command_server_config_with_arg_with_args_and_env() { let cfg: McpServerConfig = toml::from_str( diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 5114c9c20da..61724474f82 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -5144,7 +5144,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { args: vec!["hello".to_string()], env: None, env_vars: Vec::new(), - cwd: None, + cwd: Some(codex_home.path().to_path_buf()), }, environment_id: "remote".to_string(), enabled: true, diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index d7a61f5aff6..bcc73ca04dd 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -1053,7 +1053,8 @@ mod tests { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, - environment_id: "local".to_string(), + runtime_placement: + codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator, }, McpServerStatus { name: "disabled".to_string(), @@ -1061,7 +1062,8 @@ mod tests { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, - environment_id: "local".to_string(), + runtime_placement: + codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator, }, ]; diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 8570bda8d92..8974e9db336 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -150,7 +150,7 @@ async fn handle_mcp_inventory_result_clears_committed_loading_cell() { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, - environment_id: "local".to_string(), + runtime_placement: codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator, }]), McpServerStatusDetail::ToolsAndAuthOnly, ); diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index 99beea09ea7..8a07d6e794a 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -834,7 +834,7 @@ async fn mcp_tools_output_from_statuses_renders_status_only_servers() { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, - environment_id: "local".to_string(), + runtime_placement: codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator, }]; let cell = new_mcp_tools_output_from_statuses( @@ -893,7 +893,7 @@ async fn mcp_tools_output_from_statuses_renders_verbose_inventory() { mime_type: None, }], auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, - environment_id: "local".to_string(), + runtime_placement: codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator, }]; let cell = new_mcp_tools_output_from_statuses(&config, &statuses, McpServerStatusDetail::Full); From 8bc1eee16198d5fdbe0f7854986d0f126a69d384 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Thu, 21 May 2026 13:44:39 +0200 Subject: [PATCH 22/27] codex: remove MCP expect calls --- .../src/request_processors/mcp_processor.rs | 30 +++++++++---------- codex-rs/codex-mcp/src/runtime.rs | 19 ++++++++---- codex-rs/config/src/mcp_types_tests.rs | 16 ++++++---- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 2e22644cf0c..40cb056fbad 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -319,24 +319,24 @@ impl McpRequestProcessor { let data: Vec = server_names[start..end] .iter() - .map(|name| McpServerStatus { - name: name.clone(), - tools: tools_by_server.get(name).cloned().unwrap_or_default(), - resources: resources.get(name).cloned().unwrap_or_default(), - resource_templates: resource_templates.get(name).cloned().unwrap_or_default(), - auth_status: auth_statuses - .get(name) - .cloned() - .unwrap_or(CoreMcpAuthStatus::Unsupported) - .into(), - runtime_placement: app_server_runtime_placement( - runtime_placements + .map(|name| { + let runtime_placement = runtime_placements.get(name).cloned().ok_or_else(|| { + internal_error(format!("MCP server `{name}` missing runtime placement")) + })?; + Ok(McpServerStatus { + name: name.clone(), + tools: tools_by_server.get(name).cloned().unwrap_or_default(), + resources: resources.get(name).cloned().unwrap_or_default(), + resource_templates: resource_templates.get(name).cloned().unwrap_or_default(), + auth_status: auth_statuses .get(name) .cloned() - .expect("listed MCP server should have a runtime placement"), - ), + .unwrap_or(CoreMcpAuthStatus::Unsupported) + .into(), + runtime_placement: app_server_runtime_placement(runtime_placement), + }) }) - .collect(); + .collect::, JSONRPCErrorError>>()?; let next_cursor = if end < total { Some(end.to_string()) diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index 4d77d16081b..0694edaf191 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -230,9 +230,12 @@ mod tests { PathBuf::from("/tmp"), ); - let resolved_runtime = runtime_context + let resolved_runtime = match runtime_context .resolve_server_runtime("http", &http_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) - .expect("local HTTP MCP should resolve"); + { + Ok(resolved_runtime) => resolved_runtime, + Err(error) => panic!("local HTTP MCP should resolve: {error}"), + }; assert!(matches!( resolved_runtime, ResolvedMcpServerRuntime::Orchestrator @@ -278,7 +281,10 @@ mod tests { runtime_context.resolve_server_runtime("stdio", &remote_stdio), runtime_context.resolve_server_runtime("http", &http_server("remote")), ] { - let resolved_runtime = resolved_runtime.expect("remote MCP should resolve"); + let resolved_runtime = match resolved_runtime { + Ok(resolved_runtime) => resolved_runtime, + Err(error) => panic!("remote MCP should resolve: {error}"), + }; assert!(matches!( resolved_runtime, ResolvedMcpServerRuntime::Environment(_) @@ -293,9 +299,12 @@ mod tests { PathBuf::from("/tmp"), ); - let resolved_runtime = runtime_context + let resolved_runtime = match runtime_context .resolve_server_runtime("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) - .expect("local stdio MCP should resolve"); + { + Ok(resolved_runtime) => resolved_runtime, + Err(error) => panic!("local stdio MCP should resolve: {error}"), + }; assert!(matches!( resolved_runtime, ResolvedMcpServerRuntime::Orchestrator diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index 239955d5cae..3e94e1fa587 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -54,14 +54,16 @@ fn deserialize_stdio_command_server_config_with_args() { #[test] fn deserialize_legacy_mcp_environment_alias() { let cwd = std::env::temp_dir(); - let cfg: McpServerConfig = toml::from_str(&format!( + let cfg: McpServerConfig = match toml::from_str(&format!( r#" command = "echo" experimental_environment = "remote" cwd = {cwd:?} "# - )) - .expect("should deserialize legacy MCP environment alias"); + )) { + Ok(cfg) => cfg, + Err(error) => panic!("should deserialize legacy MCP environment alias: {error}"), + }; assert_eq!(cfg.environment_id, "remote"); } @@ -99,14 +101,16 @@ fn deserialize_remote_stdio_server_requires_absolute_cwd() { #[test] fn deserialize_remote_stdio_server_accepts_absolute_cwd() { let cwd = std::env::temp_dir(); - let cfg: McpServerConfig = toml::from_str(&format!( + let cfg: McpServerConfig = match toml::from_str(&format!( r#" command = "echo" environment_id = "remote" cwd = {cwd:?} "# - )) - .expect("remote stdio MCP should accept absolute cwd"); + )) { + Ok(cfg) => cfg, + Err(error) => panic!("remote stdio MCP should accept absolute cwd: {error}"), + }; assert_eq!( cfg.transport, From a6bd6d3ab13e9096ff8a25e69959fefe2f609d72 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Thu, 21 May 2026 13:56:10 +0200 Subject: [PATCH 23/27] codex: fix MCP config round-trip test --- codex-rs/core/src/config/config_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 61724474f82..ae741a4c30e 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -5184,7 +5184,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { assert_eq!(args, &vec!["hello".to_string()]); assert!(env.is_none()); assert!(env_vars.is_empty()); - assert!(cwd.is_none()); + assert_eq!(cwd, &Some(codex_home.path().to_path_buf())); } other => panic!("unexpected transport {other:?}"), } From 78130db50e9459ab125633540b5f95bbbb0fbe03 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Thu, 21 May 2026 14:17:17 +0200 Subject: [PATCH 24/27] codex: simplify MCP runtime environment routing --- .../codex_app_server_protocol.schemas.json | 44 ------ .../codex_app_server_protocol.v2.schemas.json | 44 ------ .../json/v2/ListMcpServerStatusResponse.json | 44 ------ .../v2/McpServerRuntimePlacement.ts | 5 - .../schema/typescript/v2/McpServerStatus.ts | 3 +- .../schema/typescript/v2/index.ts | 1 - .../src/protocol/v2/mcp.rs | 13 -- codex-rs/app-server/README.md | 2 +- .../src/request_processors/mcp_processor.rs | 73 +++------- codex-rs/codex-mcp/src/lib.rs | 1 - codex-rs/codex-mcp/src/mcp/mod.rs | 15 +- codex-rs/codex-mcp/src/rmcp_client.rs | 44 +++--- codex-rs/codex-mcp/src/runtime.rs | 128 ++++++------------ codex-rs/codex-mcp/src/server.rs | 8 -- codex-rs/tui/src/app/background_requests.rs | 4 - codex-rs/tui/src/app/tests.rs | 1 - codex-rs/tui/src/history_cell/tests.rs | 2 - 17 files changed, 93 insertions(+), 339 deletions(-) delete mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/McpServerRuntimePlacement.ts diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 2fab758bcd7..dda285476d6 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -11025,46 +11025,6 @@ "title": "McpServerRefreshResponse", "type": "object" }, - "McpServerRuntimePlacement": { - "oneOf": [ - { - "properties": { - "type": { - "enum": [ - "orchestrator" - ], - "title": "OrchestratorMcpServerRuntimePlacementType", - "type": "string" - } - }, - "required": [ - "type" - ], - "title": "OrchestratorMcpServerRuntimePlacement", - "type": "object" - }, - { - "properties": { - "environmentId": { - "type": "string" - }, - "type": { - "enum": [ - "environment" - ], - "title": "EnvironmentMcpServerRuntimePlacementType", - "type": "string" - } - }, - "required": [ - "environmentId", - "type" - ], - "title": "EnvironmentMcpServerRuntimePlacement", - "type": "object" - } - ] - }, "McpServerStartupState": { "enum": [ "starting", @@ -11094,9 +11054,6 @@ }, "type": "array" }, - "runtimePlacement": { - "$ref": "#/definitions/v2/McpServerRuntimePlacement" - }, "tools": { "additionalProperties": { "$ref": "#/definitions/v2/Tool" @@ -11109,7 +11066,6 @@ "name", "resourceTemplates", "resources", - "runtimePlacement", "tools" ], "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 2f81fcecc1d..3a23ba3a69d 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -7554,46 +7554,6 @@ "title": "McpServerRefreshResponse", "type": "object" }, - "McpServerRuntimePlacement": { - "oneOf": [ - { - "properties": { - "type": { - "enum": [ - "orchestrator" - ], - "title": "OrchestratorMcpServerRuntimePlacementType", - "type": "string" - } - }, - "required": [ - "type" - ], - "title": "OrchestratorMcpServerRuntimePlacement", - "type": "object" - }, - { - "properties": { - "environmentId": { - "type": "string" - }, - "type": { - "enum": [ - "environment" - ], - "title": "EnvironmentMcpServerRuntimePlacementType", - "type": "string" - } - }, - "required": [ - "environmentId", - "type" - ], - "title": "EnvironmentMcpServerRuntimePlacement", - "type": "object" - } - ] - }, "McpServerStartupState": { "enum": [ "starting", @@ -7623,9 +7583,6 @@ }, "type": "array" }, - "runtimePlacement": { - "$ref": "#/definitions/McpServerRuntimePlacement" - }, "tools": { "additionalProperties": { "$ref": "#/definitions/Tool" @@ -7638,7 +7595,6 @@ "name", "resourceTemplates", "resources", - "runtimePlacement", "tools" ], "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json index 36aa6ba7a2c..fc181c2702e 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ListMcpServerStatusResponse.json @@ -10,46 +10,6 @@ ], "type": "string" }, - "McpServerRuntimePlacement": { - "oneOf": [ - { - "properties": { - "type": { - "enum": [ - "orchestrator" - ], - "title": "OrchestratorMcpServerRuntimePlacementType", - "type": "string" - } - }, - "required": [ - "type" - ], - "title": "OrchestratorMcpServerRuntimePlacement", - "type": "object" - }, - { - "properties": { - "environmentId": { - "type": "string" - }, - "type": { - "enum": [ - "environment" - ], - "title": "EnvironmentMcpServerRuntimePlacementType", - "type": "string" - } - }, - "required": [ - "environmentId", - "type" - ], - "title": "EnvironmentMcpServerRuntimePlacement", - "type": "object" - } - ] - }, "McpServerStatus": { "properties": { "authStatus": { @@ -70,9 +30,6 @@ }, "type": "array" }, - "runtimePlacement": { - "$ref": "#/definitions/McpServerRuntimePlacement" - }, "tools": { "additionalProperties": { "$ref": "#/definitions/Tool" @@ -85,7 +42,6 @@ "name", "resourceTemplates", "resources", - "runtimePlacement", "tools" ], "type": "object" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerRuntimePlacement.ts b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerRuntimePlacement.ts deleted file mode 100644 index 29cf2a1e132..00000000000 --- a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerRuntimePlacement.ts +++ /dev/null @@ -1,5 +0,0 @@ -// GENERATED CODE! DO NOT MODIFY BY HAND! - -// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. - -export type McpServerRuntimePlacement = { "type": "orchestrator" } | { "type": "environment", environmentId: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts index b992473e4e2..430494e2687 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatus.ts @@ -5,6 +5,5 @@ import type { Resource } from "../Resource"; import type { ResourceTemplate } from "../ResourceTemplate"; import type { Tool } from "../Tool"; import type { McpAuthStatus } from "./McpAuthStatus"; -import type { McpServerRuntimePlacement } from "./McpServerRuntimePlacement"; -export type McpServerStatus = { name: string, tools: { [key in string]?: Tool }, resources: Array, resourceTemplates: Array, authStatus: McpAuthStatus, runtimePlacement: McpServerRuntimePlacement, }; +export type McpServerStatus = { name: string, tools: { [key in string]?: Tool }, resources: Array, resourceTemplates: Array, authStatus: McpAuthStatus, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 6126b2a6087..e9b8993c26e 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -217,7 +217,6 @@ export type { McpServerOauthLoginCompletedNotification } from "./McpServerOauthL export type { McpServerOauthLoginParams } from "./McpServerOauthLoginParams"; export type { McpServerOauthLoginResponse } from "./McpServerOauthLoginResponse"; export type { McpServerRefreshResponse } from "./McpServerRefreshResponse"; -export type { McpServerRuntimePlacement } from "./McpServerRuntimePlacement"; export type { McpServerStartupState } from "./McpServerStartupState"; export type { McpServerStatus } from "./McpServerStatus"; export type { McpServerStatusDetail } from "./McpServerStatusDetail"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs b/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs index cf1068e1320..9fd93840768 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs @@ -55,19 +55,6 @@ pub struct McpServerStatus { pub resources: Vec, pub resource_templates: Vec, pub auth_status: McpAuthStatus, - pub runtime_placement: McpServerRuntimePlacement, -} - -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] -#[serde(rename_all = "camelCase", tag = "type")] -#[ts(rename_all = "camelCase", tag = "type", export_to = "v2/")] -pub enum McpServerRuntimePlacement { - Orchestrator, - Environment { - #[serde(rename = "environmentId")] - #[ts(rename = "environmentId")] - environment_id: String, - }, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 0ed8fb1041d..2ceffc86fec 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -216,7 +216,7 @@ Example with notification opt-out: - `mcpServer/oauth/login` — start an OAuth login for a configured MCP server; returns an `authorization_url` and later emits `mcpServer/oauthLogin/completed` once the browser flow finishes. - `tool/requestUserInput` — prompt the user with 1–3 short questions for a tool call and return their answers (experimental). - `config/mcpServer/reload` — reload MCP server config from disk and queue a refresh for loaded threads (applied on each thread's next active turn); returns `{}`. Use this after editing `config.toml` without restarting the server. -- `mcpServerStatus/list` — enumerate configured MCP servers with their tools, auth status, and resolved `runtimePlacement`, plus resources/resource templates for `full` detail; supports cursor+limit pagination. If `detail` is omitted, the server defaults to `full`. +- `mcpServerStatus/list` — enumerate configured MCP servers with their tools and auth status, plus resources/resource templates for `full` detail; supports cursor+limit pagination. If `detail` is omitted, the server defaults to `full`. - `mcpServer/resource/read` — read a resource from a configured MCP server by optional `threadId`, `server`, and `uri`, returning text/blob resource `contents`. If `threadId` is omitted, the server reads from the latest MCP config directly. - `mcpServer/tool/call` — call a tool on a thread's configured MCP server by `threadId`, `server`, `tool`, optional `arguments`, and optional `_meta`, returning the MCP tool result. - `windowsSandbox/setupStart` — start Windows sandbox setup for the selected mode (`elevated` or `unelevated`); accepts an optional absolute `cwd` to target setup for a specific workspace, returns `{ started: true }` immediately, and later emits `windowsSandbox/setupCompleted`. diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 40cb056fbad..2d60ed1353f 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -216,7 +216,6 @@ impl McpRequestProcessor { outgoing, request, params, - config, mcp_config, auth, runtime_context, @@ -230,7 +229,6 @@ impl McpRequestProcessor { outgoing: Arc, request_id: ConnectionRequestId, params: ListMcpServerStatusParams, - config: Config, mcp_config: codex_mcp::McpConfig, auth: Option, runtime_context: McpRuntimeContext, @@ -238,7 +236,6 @@ impl McpRequestProcessor { let result = Self::list_mcp_server_status_response( request_id.request_id.to_string(), params, - config, mcp_config, auth, runtime_context, @@ -250,7 +247,6 @@ impl McpRequestProcessor { async fn list_mcp_server_status_response( request_id: String, params: ListMcpServerStatusParams, - config: Config, mcp_config: codex_mcp::McpConfig, auth: Option, runtime_context: McpRuntimeContext, @@ -274,27 +270,15 @@ impl McpRequestProcessor { resources, resource_templates, auth_statuses, - runtime_placements, + mut server_names, } = snapshot; - let runtime_placements = config - .mcp_servers - .iter() - .map(|(name, config)| { - ( - name.clone(), - codex_mcp::McpServerRuntimePlacement::from_config(config), - ) - }) - .chain(runtime_placements) - .collect::>(); - - let mut server_names: Vec = runtime_placements - .keys() - .cloned() - .chain(auth_statuses.keys().cloned()) - .chain(resources.keys().cloned()) - .chain(resource_templates.keys().cloned()) - .collect(); + server_names.extend( + auth_statuses + .keys() + .cloned() + .chain(resources.keys().cloned()) + .chain(resource_templates.keys().cloned()), + ); server_names.sort(); server_names.dedup(); @@ -319,24 +303,18 @@ impl McpRequestProcessor { let data: Vec = server_names[start..end] .iter() - .map(|name| { - let runtime_placement = runtime_placements.get(name).cloned().ok_or_else(|| { - internal_error(format!("MCP server `{name}` missing runtime placement")) - })?; - Ok(McpServerStatus { - name: name.clone(), - tools: tools_by_server.get(name).cloned().unwrap_or_default(), - resources: resources.get(name).cloned().unwrap_or_default(), - resource_templates: resource_templates.get(name).cloned().unwrap_or_default(), - auth_status: auth_statuses - .get(name) - .cloned() - .unwrap_or(CoreMcpAuthStatus::Unsupported) - .into(), - runtime_placement: app_server_runtime_placement(runtime_placement), - }) + .map(|name| McpServerStatus { + name: name.clone(), + tools: tools_by_server.get(name).cloned().unwrap_or_default(), + resources: resources.get(name).cloned().unwrap_or_default(), + resource_templates: resource_templates.get(name).cloned().unwrap_or_default(), + auth_status: auth_statuses + .get(name) + .cloned() + .unwrap_or(CoreMcpAuthStatus::Unsupported) + .into(), }) - .collect::, JSONRPCErrorError>>()?; + .collect(); let next_cursor = if end < total { Some(end.to_string()) @@ -438,19 +416,6 @@ impl McpRequestProcessor { } } -fn app_server_runtime_placement( - placement: codex_mcp::McpServerRuntimePlacement, -) -> codex_app_server_protocol::McpServerRuntimePlacement { - match placement { - codex_mcp::McpServerRuntimePlacement::Orchestrator => { - codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator - } - codex_mcp::McpServerRuntimePlacement::Environment { environment_id } => { - codex_app_server_protocol::McpServerRuntimePlacement::Environment { environment_id } - } - } -} - fn with_mcp_tool_call_thread_id_meta( meta: Option, thread_id: &str, diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index d097e2abc10..7b45bcc7a78 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -4,7 +4,6 @@ pub use elicitation::ElicitationReviewer; pub use elicitation::ElicitationReviewerHandle; pub use rmcp_client::MCP_SANDBOX_STATE_META_CAPABILITY; pub use runtime::McpRuntimeContext; -pub use runtime::McpServerRuntimePlacement; pub use runtime::SandboxState; pub use tools::ToolInfo; diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index a29533b7f57..1a06d9b9a08 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -314,7 +314,7 @@ pub struct McpServerStatusSnapshot { pub resources: HashMap>, pub resource_templates: HashMap>, pub auth_statuses: HashMap, - pub runtime_placements: HashMap, + pub server_names: Vec, } pub async fn collect_mcp_server_status_snapshot_with_detail( @@ -333,7 +333,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( resources: HashMap::new(), resource_templates: HashMap::new(), auth_statuses: HashMap::new(), - runtime_placements: HashMap::new(), + server_names: Vec::new(), }; } @@ -344,10 +344,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( ) .await; - let runtime_placements = mcp_servers - .iter() - .map(|(server_name, server)| (server_name.clone(), server.runtime_placement())) - .collect(); + let server_names = mcp_servers.keys().cloned().collect(); let (tx_event, rx_event) = unbounded(); drop(rx_event); @@ -374,7 +371,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( let snapshot = collect_mcp_server_status_snapshot_from_manager( &mcp_connection_manager, auth_status_entries, - runtime_placements, + server_names, detail, ) .await; @@ -583,7 +580,7 @@ fn convert_mcp_resource_templates( async fn collect_mcp_server_status_snapshot_from_manager( mcp_connection_manager: &McpConnectionManager, auth_status_entries: HashMap, - runtime_placements: HashMap, + server_names: Vec, detail: McpSnapshotDetail, ) -> McpServerStatusSnapshot { let (tools, resources, resource_templates) = tokio::join!( @@ -622,7 +619,7 @@ async fn collect_mcp_server_status_snapshot_from_manager( resources: convert_mcp_resources(resources), resource_templates: convert_mcp_resource_templates(resource_templates), auth_statuses: auth_statuses_from_entries(&auth_status_entries), - runtime_placements, + server_names, } } diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 459e98d745b..de488ff3b91 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -45,6 +45,8 @@ use codex_async_utils::OrCancelExt; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; use codex_config::types::OAuthCredentialsStoreMode; +use codex_exec_server::HttpClient; +use codex_exec_server::ReqwestHttpClient; use codex_protocol::protocol::Event; use codex_rmcp_client::ExecutorStdioServerLauncher; use codex_rmcp_client::LocalStdioServerLauncher; @@ -564,9 +566,10 @@ async fn make_rmcp_client( let config = match server.launch() { McpServerLaunch::Configured(config) => config.as_ref().clone(), }; - let resolved_runtime = runtime_context - .resolve_server_runtime(server_name, &config) + let resolved_environment = runtime_context + .resolve_server_environment(server_name, &config) .map_err(|err| StartupOutcomeError::from(anyhow!(err)))?; + let is_local_environment = config.is_local_environment(); let McpServerConfig { transport, .. } = config; match transport { @@ -584,21 +587,23 @@ async fn make_rmcp_client( .map(|(key, value)| (key.into(), value.into())) .collect::>() }); - let launcher = match &resolved_runtime { - crate::runtime::ResolvedMcpServerRuntime::Orchestrator => { - // TODO(starr): Unify local stdio MCP launch with - // `ExecutorStdioServerLauncher` once the executor-backed path - // preserves `LocalStdioServerLauncher` semantics. - Arc::new(LocalStdioServerLauncher::new( - runtime_context.fallback_cwd(), - )) as Arc - } - crate::runtime::ResolvedMcpServerRuntime::Environment(environment) => { - Arc::new(ExecutorStdioServerLauncher::new( - environment.get_exec_backend(), - runtime_context.fallback_cwd(), - )) as Arc - } + let launcher = if is_local_environment { + // TODO(starr): Unify local stdio MCP launch with + // `ExecutorStdioServerLauncher` once the executor-backed path + // preserves `LocalStdioServerLauncher` semantics. + Arc::new(LocalStdioServerLauncher::new( + runtime_context.fallback_cwd(), + )) as Arc + } else { + let Some(environment) = resolved_environment.as_ref() else { + unreachable!( + "non-local stdio MCP servers resolve an environment before launch" + ); + }; + Arc::new(ExecutorStdioServerLauncher::new( + environment.get_exec_backend(), + runtime_context.fallback_cwd(), + )) as Arc }; RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd, launcher) @@ -611,7 +616,10 @@ async fn make_rmcp_client( env_http_headers, bearer_token_env_var, } => { - let http_client = resolved_runtime.http_client(); + let http_client = resolved_environment.as_ref().map_or_else( + || Arc::new(ReqwestHttpClient) as Arc, + |environment| environment.get_http_client(), + ); let resolved_bearer_token = match resolve_bearer_token(server_name, bearer_token_env_var.as_deref()) { Ok(token) => token, diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index 0694edaf191..b7ef8028c26 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -11,8 +11,6 @@ use std::time::Duration; use codex_exec_server::Environment; use codex_exec_server::EnvironmentManager; -use codex_exec_server::HttpClient; -use codex_exec_server::ReqwestHttpClient; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::SandboxPolicy; @@ -31,44 +29,7 @@ pub struct SandboxState { pub use_legacy_landlock: bool, } -/// Effective runtime placement for one MCP server. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum McpServerRuntimePlacement { - /// The orchestrator owns the transport directly. - Orchestrator, - /// The selected named environment owns the transport. - Environment { environment_id: String }, -} - -impl McpServerRuntimePlacement { - pub fn from_config(config: &codex_config::McpServerConfig) -> Self { - if config.is_local_environment() { - Self::Orchestrator - } else { - Self::Environment { - environment_id: config.environment_id.clone(), - } - } - } -} - -/// Resolved runtime handle for one MCP server startup. -#[derive(Clone)] -pub(crate) enum ResolvedMcpServerRuntime { - Orchestrator, - Environment(Arc), -} - -impl ResolvedMcpServerRuntime { - pub(crate) fn http_client(&self) -> Arc { - match self { - Self::Orchestrator => Arc::new(ReqwestHttpClient) as Arc, - Self::Environment(environment) => environment.get_http_client(), - } - } -} - -/// Runtime context used when resolving per-server MCP environment placement. +/// Runtime context used when resolving per-server MCP environments. /// /// `McpConfig` describes what servers exist. This value carries the canonical /// environment registry plus the local stdio fallback cwd used when a local @@ -91,38 +52,37 @@ impl McpRuntimeContext { self.fallback_cwd.clone() } - pub(crate) fn resolve_server_runtime( + pub(crate) fn resolve_server_environment( &self, server_name: &str, config: &codex_config::McpServerConfig, - ) -> Result { - match McpServerRuntimePlacement::from_config(config) { - McpServerRuntimePlacement::Orchestrator => { - if self.environment_manager.try_local_environment().is_none() - && matches!( - config.transport, - codex_config::McpServerTransportConfig::Stdio { .. } - ) - { - return Err(format!( - "local stdio MCP server `{server_name}` requires a local environment" - )); - } - Ok(ResolvedMcpServerRuntime::Orchestrator) - } - McpServerRuntimePlacement::Environment { environment_id } => { - let environment = self - .environment_manager - .get_environment(&environment_id) - .ok_or_else(|| { - format!( - "MCP server `{server_name}` references unknown environment id `{environment_id}`" - ) - })?; + ) -> Result>, String> { + // Resolve `"local"` through the shared registry when available. Local + // HTTP is the one current exception: it can use the ambient HTTP client + // even when no local Environment is configured. + if let Some(environment) = self + .environment_manager + .get_environment(&config.environment_id) + { + if !config.is_local_environment() { ensure_remote_stdio_cwd(server_name, config)?; - Ok(ResolvedMcpServerRuntime::Environment(environment)) } + return Ok(Some(environment)); + } + + if config.is_local_environment() { + return match config.transport { + codex_config::McpServerTransportConfig::Stdio { .. } => Err(format!( + "local stdio MCP server `{server_name}` requires a local environment" + )), + codex_config::McpServerTransportConfig::StreamableHttp { .. } => Ok(None), + }; } + + Err(format!( + "MCP server `{server_name}` references unknown environment id `{}`", + config.environment_id + )) } } @@ -212,7 +172,7 @@ mod tests { ); let error = match runtime_context - .resolve_server_runtime("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) + .resolve_server_environment("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) { Ok(_) => panic!("local stdio MCP should require a local environment"), Err(error) => error, @@ -231,15 +191,12 @@ mod tests { ); let resolved_runtime = match runtime_context - .resolve_server_runtime("http", &http_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) + .resolve_server_environment("http", &http_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) { Ok(resolved_runtime) => resolved_runtime, Err(error) => panic!("local HTTP MCP should resolve: {error}"), }; - assert!(matches!( - resolved_runtime, - ResolvedMcpServerRuntime::Orchestrator - )); + assert_eq!(resolved_runtime, None); } #[test] @@ -249,10 +206,11 @@ mod tests { PathBuf::from("/tmp"), ); - let error = match runtime_context.resolve_server_runtime("stdio", &stdio_server("remote")) { - Ok(_) => panic!("unknown MCP environment should fail"), - Err(error) => error, - }; + let error = + match runtime_context.resolve_server_environment("stdio", &stdio_server("remote")) { + Ok(_) => panic!("unknown MCP environment should fail"), + Err(error) => error, + }; assert_eq!( error, "MCP server `stdio` references unknown environment id `remote`" @@ -278,17 +236,14 @@ mod tests { }; *cwd = Some(std::env::temp_dir()); for resolved_runtime in [ - runtime_context.resolve_server_runtime("stdio", &remote_stdio), - runtime_context.resolve_server_runtime("http", &http_server("remote")), + runtime_context.resolve_server_environment("stdio", &remote_stdio), + runtime_context.resolve_server_environment("http", &http_server("remote")), ] { let resolved_runtime = match resolved_runtime { Ok(resolved_runtime) => resolved_runtime, Err(error) => panic!("remote MCP should resolve: {error}"), }; - assert!(matches!( - resolved_runtime, - ResolvedMcpServerRuntime::Environment(_) - )); + assert!(resolved_runtime.is_some()); } } @@ -300,15 +255,12 @@ mod tests { ); let resolved_runtime = match runtime_context - .resolve_server_runtime("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) + .resolve_server_environment("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) { Ok(resolved_runtime) => resolved_runtime, Err(error) => panic!("local stdio MCP should resolve: {error}"), }; - assert!(matches!( - resolved_runtime, - ResolvedMcpServerRuntime::Orchestrator - )); + assert!(resolved_runtime.is_some()); } #[tokio::test] @@ -329,7 +281,7 @@ mod tests { }; *cwd = Some(PathBuf::from("relative")); - let error = match runtime_context.resolve_server_runtime("stdio", &remote_stdio) { + let error = match runtime_context.resolve_server_environment("stdio", &remote_stdio) { Ok(_) => panic!("remote stdio MCP should require absolute cwd"), Err(error) => error, }; diff --git a/codex-rs/codex-mcp/src/server.rs b/codex-rs/codex-mcp/src/server.rs index 9dd84f132b4..d8fb5a11f26 100644 --- a/codex-rs/codex-mcp/src/server.rs +++ b/codex-rs/codex-mcp/src/server.rs @@ -1,8 +1,6 @@ use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; -use crate::runtime::McpServerRuntimePlacement; - /// The runtime launch strategy for an effective MCP server. #[derive(Debug, Clone)] pub(crate) enum McpServerLaunch { @@ -32,12 +30,6 @@ impl EffectiveMcpServer { } } - pub fn runtime_placement(&self) -> McpServerRuntimePlacement { - match &self.launch { - McpServerLaunch::Configured(config) => McpServerRuntimePlacement::from_config(config), - } - } - pub fn enabled(&self) -> bool { match &self.launch { McpServerLaunch::Configured(config) => config.enabled, diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index bcc73ca04dd..dcf1f672bee 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -1053,8 +1053,6 @@ mod tests { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, - runtime_placement: - codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator, }, McpServerStatus { name: "disabled".to_string(), @@ -1062,8 +1060,6 @@ mod tests { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, - runtime_placement: - codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator, }, ]; diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 8974e9db336..92e824aec72 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -150,7 +150,6 @@ async fn handle_mcp_inventory_result_clears_committed_loading_cell() { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, - runtime_placement: codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator, }]), McpServerStatusDetail::ToolsAndAuthOnly, ); diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index 8a07d6e794a..1307f9a828f 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -834,7 +834,6 @@ async fn mcp_tools_output_from_statuses_renders_status_only_servers() { resources: Vec::new(), resource_templates: Vec::new(), auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, - runtime_placement: codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator, }]; let cell = new_mcp_tools_output_from_statuses( @@ -893,7 +892,6 @@ async fn mcp_tools_output_from_statuses_renders_verbose_inventory() { mime_type: None, }], auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported, - runtime_placement: codex_app_server_protocol::McpServerRuntimePlacement::Orchestrator, }]; let cell = new_mcp_tools_output_from_statuses(&config, &statuses, McpServerStatusDetail::Full); From a3aaddc4a37b5c48ffa9884ca37bb4af0d885e24 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Thu, 21 May 2026 14:32:28 +0200 Subject: [PATCH 25/27] codex: narrow MCP stdio cwd fallback --- codex-rs/codex-mcp/src/rmcp_client.rs | 3 +- codex-rs/codex-mcp/src/runtime.rs | 13 +++-- codex-rs/config/src/mcp_types.rs | 2 +- codex-rs/config/src/mcp_types_tests.rs | 17 ------ codex-rs/core/tests/suite/rmcp_client.rs | 53 ------------------- .../rmcp-client/src/stdio_server_launcher.rs | 23 ++++---- 6 files changed, 19 insertions(+), 92 deletions(-) diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index de488ff3b91..c75895842b2 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -592,7 +592,7 @@ async fn make_rmcp_client( // `ExecutorStdioServerLauncher` once the executor-backed path // preserves `LocalStdioServerLauncher` semantics. Arc::new(LocalStdioServerLauncher::new( - runtime_context.fallback_cwd(), + runtime_context.local_stdio_fallback_cwd(), )) as Arc } else { let Some(environment) = resolved_environment.as_ref() else { @@ -602,7 +602,6 @@ async fn make_rmcp_client( }; Arc::new(ExecutorStdioServerLauncher::new( environment.get_exec_backend(), - runtime_context.fallback_cwd(), )) as Arc }; diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index b7ef8028c26..3dbcb8de04b 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -37,19 +37,22 @@ pub struct SandboxState { #[derive(Clone)] pub struct McpRuntimeContext { environment_manager: Arc, - fallback_cwd: PathBuf, + local_stdio_fallback_cwd: PathBuf, } impl McpRuntimeContext { - pub fn new(environment_manager: Arc, fallback_cwd: PathBuf) -> Self { + pub fn new( + environment_manager: Arc, + local_stdio_fallback_cwd: PathBuf, + ) -> Self { Self { environment_manager, - fallback_cwd, + local_stdio_fallback_cwd, } } - pub(crate) fn fallback_cwd(&self) -> PathBuf { - self.fallback_cwd.clone() + pub(crate) fn local_stdio_fallback_cwd(&self) -> PathBuf { + self.local_stdio_fallback_cwd.clone() } pub(crate) fn resolve_server_environment( diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index 972581b8799..0d30ba8b23f 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -236,7 +236,7 @@ pub struct RawMcpServerConfig { pub bearer_token_env_var: Option, // shared - #[serde(default, alias = "experimental_environment")] + #[serde(default)] pub environment_id: Option, #[serde(default)] pub startup_timeout_sec: Option, diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index 3e94e1fa587..4612371d182 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -51,23 +51,6 @@ fn deserialize_stdio_command_server_config_with_args() { assert!(cfg.enabled); } -#[test] -fn deserialize_legacy_mcp_environment_alias() { - let cwd = std::env::temp_dir(); - let cfg: McpServerConfig = match toml::from_str(&format!( - r#" - command = "echo" - experimental_environment = "remote" - cwd = {cwd:?} - "# - )) { - Ok(cfg) => cfg, - Err(error) => panic!("should deserialize legacy MCP environment alias: {error}"), - }; - - assert_eq!(cfg.environment_id, "remote"); -} - #[test] fn deserialize_remote_stdio_server_requires_absolute_cwd() { let missing_cwd = toml::from_str::( diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index f52f00d643a..27d6d8cbbaa 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -634,59 +634,6 @@ async fn stdio_server_uses_configured_cwd_before_runtime_fallback() -> anyhow::R Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 1)] -#[serial(mcp_cwd)] -async fn remote_stdio_server_uses_runtime_fallback_cwd_when_config_omits_cwd() -> anyhow::Result<()> -{ - skip_if_no_network!(Ok(())); - if std::env::var_os(remote_env_env_var()).is_none() { - return Ok(()); - } - - let server = responses::start_mock_server().await; - let server_name = "rmcp_fallback_cwd"; - let expected_cwd = Arc::new(Mutex::new(None::)); - let expected_cwd_for_config = Arc::clone(&expected_cwd); - let rmcp_test_server_bin = remote_aware_stdio_server_bin()?; - - let fixture = test_codex() - .with_config(move |config| { - *expected_cwd_for_config - .lock() - .expect("expected cwd lock should not be poisoned") = - Some(config.cwd.to_path_buf()); - insert_mcp_server( - config, - server_name, - stdio_transport( - rmcp_test_server_bin, - Some(HashMap::from([( - "MCP_TEST_VALUE".to_string(), - "fallback-cwd".to_string(), - )])), - Vec::new(), - ), - TestMcpServerOptions { - environment_id: remote_aware_environment_id(), - ..Default::default() - }, - ); - }) - .build_with_remote_env(&server) - .await?; - - let expected_cwd = expected_cwd - .lock() - .expect("expected cwd lock should not be poisoned") - .clone() - .expect("test config should record runtime fallback cwd"); - let structured = call_cwd_tool(&server, &fixture, server_name, "call-fallback-cwd").await?; - - assert_cwd_tool_output(&structured, &expected_cwd); - server.verify().await; - Ok(()) -} - #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 1)] #[serial(mcp_cwd)] diff --git a/codex-rs/rmcp-client/src/stdio_server_launcher.rs b/codex-rs/rmcp-client/src/stdio_server_launcher.rs index 9928511bb6c..8b325066945 100644 --- a/codex-rs/rmcp-client/src/stdio_server_launcher.rs +++ b/codex-rs/rmcp-client/src/stdio_server_launcher.rs @@ -438,20 +438,12 @@ impl Drop for StdioServerProcessHandleInner { #[derive(Clone)] pub struct ExecutorStdioServerLauncher { exec_backend: Arc, - fallback_cwd: PathBuf, } impl ExecutorStdioServerLauncher { /// Creates a stdio server launcher backed by the executor process API. - /// - /// `fallback_cwd` is used only when the MCP server config omits `cwd`. - /// Executor `process/start` requires an explicit working directory, unlike - /// local `tokio::process::Command`, which can inherit the orchestrator cwd. - pub fn new(exec_backend: Arc, fallback_cwd: PathBuf) -> Self { - Self { - exec_backend, - fallback_cwd, - } + pub fn new(exec_backend: Arc) -> Self { + Self { exec_backend } } } @@ -461,8 +453,7 @@ impl StdioServerLauncher for ExecutorStdioServerLauncher { command: StdioServerCommand, ) -> BoxFuture<'static, io::Result> { let exec_backend = Arc::clone(&self.exec_backend); - let fallback_cwd = self.fallback_cwd.clone(); - async move { Self::launch_server(command, exec_backend, fallback_cwd).await }.boxed() + async move { Self::launch_server(command, exec_backend).await }.boxed() } } @@ -474,7 +465,6 @@ impl ExecutorStdioServerLauncher { async fn launch_server( command: StdioServerCommand, exec_backend: Arc, - fallback_cwd: PathBuf, ) -> io::Result { let StdioServerCommand { program, @@ -483,6 +473,11 @@ impl ExecutorStdioServerLauncher { env_vars, cwd, } = command; + let Some(cwd) = cwd else { + return Err(io::Error::other( + "executor stdio server requires an explicit cwd", + )); + }; let program_name = program.to_string_lossy().into_owned(); let envs = create_env_overlay_for_remote_mcp_server(env, &env_vars); let remote_env_vars = remote_mcp_env_var_names(&env_vars); @@ -500,7 +495,7 @@ impl ExecutorStdioServerLauncher { .start(ExecParams { process_id, argv, - cwd: cwd.unwrap_or(fallback_cwd), + cwd, env_policy: Some(Self::remote_env_policy(&remote_env_vars)), env, tty: false, From 32798e8d363684ba1b8785dffd0e2792eb504f96 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Thu, 21 May 2026 14:40:52 +0200 Subject: [PATCH 26/27] codex: reuse MCP local environment constant --- .../src/protocol/v2/tests.rs | 2 +- codex-rs/cli/src/mcp_cmd.rs | 2 +- .../codex-mcp/src/connection_manager_tests.rs | 8 ++--- codex-rs/codex-mcp/src/mcp/mod_tests.rs | 4 +-- codex-rs/config/src/mcp_edit_tests.rs | 4 +-- codex-rs/config/src/mcp_types_tests.rs | 2 +- codex-rs/core/src/config/config_tests.rs | 32 +++++++++---------- codex-rs/core/src/config/edit_tests.rs | 14 ++++---- codex-rs/core/src/mcp_skill_dependencies.rs | 4 +-- codex-rs/core/tests/suite/hooks_mcp.rs | 2 +- codex-rs/core/tests/suite/rmcp_client.rs | 4 +-- 11 files changed, 39 insertions(+), 39 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index bbe6bec3f2d..4aea31e1e77 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -3674,7 +3674,7 @@ fn turn_start_params_round_trip_environments() { assert_eq!( params.environments, Some(vec![TurnEnvironmentParams { - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), cwd: cwd.clone(), }]) ); diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 060da0ea321..f52d24160ba 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -301,7 +301,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re let new_entry = McpServerConfig { transport: transport.clone(), - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 20593081902..181d95d0ec9 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -865,7 +865,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -890,7 +890,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -988,7 +988,7 @@ fn mcp_init_error_display_prompts_for_github_pat() { http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1041,7 +1041,7 @@ fn mcp_init_error_display_reports_generic_errors() { http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 8724c8ce428..a8260de6c74 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -316,7 +316,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -341,7 +341,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/config/src/mcp_edit_tests.rs b/codex-rs/config/src/mcp_edit_tests.rs index 57a10c2d959..ff273676cfd 100644 --- a/codex-rs/config/src/mcp_edit_tests.rs +++ b/codex-rs/config/src/mcp_edit_tests.rs @@ -23,7 +23,7 @@ async fn replace_mcp_servers_serializes_per_tool_approval_overrides() -> anyhow: env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: true, @@ -101,7 +101,7 @@ async fn replace_mcp_servers_serializes_oauth_client_id() -> anyhow::Result<()> http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index 4612371d182..712c4339319 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -458,7 +458,7 @@ fn deserialize_ignores_unknown_server_fields() { env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index ae741a4c30e..9f40aec674c 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -132,7 +132,7 @@ fn stdio_mcp(command: &str) -> McpServerConfig { env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -157,7 +157,7 @@ fn http_mcp(url: &str) -> McpServerConfig { http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5493,7 +5493,7 @@ async fn replace_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5570,7 +5570,7 @@ async fn replace_mcp_servers_serializes_env_vars() -> anyhow::Result<()> { env_vars: vec!["ALPHA".into(), "BETA".into()], cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5632,7 +5632,7 @@ async fn replace_mcp_servers_serializes_sourced_env_vars() -> anyhow::Result<()> ], cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5684,7 +5684,7 @@ async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: Some(cwd_path.clone()), }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5739,7 +5739,7 @@ async fn replace_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5810,7 +5810,7 @@ async fn replace_mcp_servers_streamable_http_serializes_custom_headers() -> anyh "DOCS_AUTH".to_string(), )])), }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5893,7 +5893,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh "DOCS_AUTH".to_string(), )])), }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5929,7 +5929,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6000,7 +6000,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() "DOCS_AUTH".to_string(), )])), }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6026,7 +6026,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6115,7 +6115,7 @@ async fn replace_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: false, required: false, supports_parallel_tool_calls: false, @@ -6166,7 +6166,7 @@ async fn replace_mcp_servers_serializes_required_flag() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: true, supports_parallel_tool_calls: false, @@ -6217,7 +6217,7 @@ async fn replace_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -6272,7 +6272,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index 15e79fad1f5..740d819aed2 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -945,7 +945,7 @@ fn blocking_replace_mcp_servers_round_trips() { env_vars: vec!["FOO".into()], cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: true, @@ -975,7 +975,7 @@ fn blocking_replace_mcp_servers_round_trips() { ), env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: false, required: false, supports_parallel_tool_calls: false, @@ -1047,7 +1047,7 @@ fn blocking_replace_mcp_servers_serializes_tool_approval_overrides() { env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1112,7 +1112,7 @@ foo = { command = "cmd" } env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1167,7 +1167,7 @@ foo = { command = "cmd" } # keep me env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: false, required: false, supports_parallel_tool_calls: false, @@ -1221,7 +1221,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -1276,7 +1276,7 @@ foo = { command = "cmd" } env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: false, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 0c821ee7065..936f1b5fb5c 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -361,7 +361,7 @@ fn mcp_dependency_to_server_config( http_headers: None, env_http_headers: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -391,7 +391,7 @@ fn mcp_dependency_to_server_config( env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/hooks_mcp.rs b/codex-rs/core/tests/suite/hooks_mcp.rs index de0c6c11aac..5ee2cb05d76 100644 --- a/codex-rs/core/tests/suite/hooks_mcp.rs +++ b/codex-rs/core/tests/suite/hooks_mcp.rs @@ -182,7 +182,7 @@ fn insert_rmcp_test_server(config: &mut Config, command: String, approval_mode: env_vars: Vec::new(), cwd: None, }, - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 27d6d8cbbaa..94222deab8b 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -145,7 +145,7 @@ fn remote_aware_environment_id() -> String { // parameterizing each stdio MCP test with its own local/remote cases. std::env::var_os(remote_env_env_var()) .map(|_| REMOTE_MCP_ENVIRONMENT.to_string()) - .unwrap_or_else(|| "local".to_string()) + .unwrap_or_else(|| codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string()) } /// Returns the stdio MCP test server command path for the active test placement. @@ -296,7 +296,7 @@ struct TestMcpServerOptions { impl Default for TestMcpServerOptions { fn default() -> Self { Self { - environment_id: "local".to_string(), + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), supports_parallel_tool_calls: false, tool_timeout_sec: None, } From 563988cb01ce077e2bc46ac0bc962ac22aef83ab Mon Sep 17 00:00:00 2001 From: starr-openai Date: Thu, 21 May 2026 16:01:42 +0200 Subject: [PATCH 27/27] codex: fix PR 23583 CI compile errors --- codex-rs/app-server-protocol/src/protocol/v2/tests.rs | 2 +- codex-rs/codex-mcp/src/runtime.rs | 2 +- codex-rs/config/src/mcp_edit_tests.rs | 4 ++-- codex-rs/config/src/mcp_types_tests.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 4aea31e1e77..bbe6bec3f2d 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -3674,7 +3674,7 @@ fn turn_start_params_round_trip_environments() { assert_eq!( params.environments, Some(vec![TurnEnvironmentParams { - environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), + environment_id: "local".to_string(), cwd: cwd.clone(), }]) ); diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index 3dbcb8de04b..5404bf1eb53 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -199,7 +199,7 @@ mod tests { Ok(resolved_runtime) => resolved_runtime, Err(error) => panic!("local HTTP MCP should resolve: {error}"), }; - assert_eq!(resolved_runtime, None); + assert!(resolved_runtime.is_none()); } #[test] diff --git a/codex-rs/config/src/mcp_edit_tests.rs b/codex-rs/config/src/mcp_edit_tests.rs index ff273676cfd..030b0a02335 100644 --- a/codex-rs/config/src/mcp_edit_tests.rs +++ b/codex-rs/config/src/mcp_edit_tests.rs @@ -23,7 +23,7 @@ async fn replace_mcp_servers_serializes_per_tool_approval_overrides() -> anyhow: env_vars: Vec::new(), cwd: None, }, - environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), + environment_id: crate::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: true, @@ -101,7 +101,7 @@ async fn replace_mcp_servers_serializes_oauth_client_id() -> anyhow::Result<()> http_headers: None, env_http_headers: None, }, - environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), + environment_id: crate::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index 712c4339319..5f3933ce43e 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -458,7 +458,7 @@ fn deserialize_ignores_unknown_server_fields() { env_vars: Vec::new(), cwd: None, }, - environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), + environment_id: crate::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false,