diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index 838dd8adf4c..311ab29d534 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -340,12 +340,11 @@ 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; 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/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/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 4bb9431f422..2d60ed1353f 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -205,24 +205,20 @@ 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(), - config.cwd.to_path_buf(), - ); + // This threadless status path has no turn cwd or turn-selected + // 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()); tokio::spawn(async move { Self::list_mcp_server_status_task( outgoing, request, params, - config, mcp_config, auth, - runtime_environment, + runtime_context, ) .await; }); @@ -233,18 +229,16 @@ impl McpRequestProcessor { outgoing: Arc, request_id: ConnectionRequestId, params: ListMcpServerStatusParams, - 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(), params, - config, mcp_config, auth, - runtime_environment, + runtime_context, ) .await; outgoing.send_result(request_id, result).await; @@ -253,10 +247,9 @@ impl McpRequestProcessor { async fn list_mcp_server_status_response( request_id: String, params: ListMcpServerStatusParams, - 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,31 +260,25 @@ impl McpRequestProcessor { &mcp_config, auth.as_ref(), request_id, - runtime_environment, + runtime_context, detail, ) .await; - let effective_servers = effective_mcp_servers(&mcp_config, auth.as_ref()); let McpServerStatusSnapshot { tools_by_server, resources, resource_templates, auth_statuses, + mut server_names, } = snapshot; - - let mut server_names: Vec = config - .mcp_servers - .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()) - .collect(); + server_names.extend( + auth_statuses + .keys() + .cloned() + .chain(resources.keys().cloned()) + .chain(resource_templates.keys().cloned()), + ); server_names.sort(); server_names.dedup(); @@ -367,21 +354,18 @@ 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(), - config.cwd.to_path_buf(), - ); + // This threadless resource-read path has no turn cwd or turn-selected + // 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(); tokio::spawn(async move { let result = read_mcp_resource_without_thread( &mcp_config, auth.as_ref(), - runtime_environment, + runtime_context, &server, &uri, ) diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 61fd363b2ef..fd45a8a732c 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -959,9 +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, &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 @@ -1037,8 +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, &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) } }; @@ -1506,7 +1513,7 @@ impl PluginRequestProcessor { connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( config, /*force_refetch*/ true, - &environment_manager + Arc::clone(&environment_manager) ), ); @@ -1772,7 +1779,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(); diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 5c66a27d84c..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(), - experimental_environment: None, + 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.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..181d95d0ec9 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: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), 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: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), 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: 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, }, - experimental_environment: None, + 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/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..1a06d9b9a08 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 server_names: Vec, } 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(), + server_names: Vec::new(), }; } @@ -342,6 +344,8 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( ) .await; + let server_names = mcp_servers.keys().cloned().collect(); + let (tx_event, rx_event) = unbounded(); drop(rx_event); @@ -353,7 +357,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 +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, + server_names, detail, ) .await; @@ -451,7 +456,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig { http_headers, env_http_headers: None, }, - experimental_environment: None, + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -575,6 +580,7 @@ fn convert_mcp_resource_templates( async fn collect_mcp_server_status_snapshot_from_manager( mcp_connection_manager: &McpConnectionManager, auth_status_entries: HashMap, + server_names: Vec, detail: McpSnapshotDetail, ) -> McpServerStatusSnapshot { let (tools, resources, resource_templates) = tokio::join!( @@ -613,6 +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), + server_names, } } diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 7566bf9bdea..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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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 687576dfaca..c75895842b2 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -28,7 +28,7 @@ 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 crate::runtime::emit_duration; use crate::server::EffectiveMcpServer; use crate::server::McpServerLaunch; @@ -142,7 +142,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 +170,7 @@ impl AsyncManagedClient { &server_name, server.clone(), store_mode, - runtime_environment, + runtime_context, runtime_auth_provider, ) .await?, @@ -560,29 +560,17 @@ 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 is_local_environment = config.is_local_environment(); + let McpServerConfig { transport, .. } = config; match transport { McpServerTransportConfig::Stdio { @@ -599,27 +587,24 @@ 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 { - return Err(StartupOutcomeError::from(anyhow!( - "remote MCP server requires a runtime 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.local_stdio_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_environment.fallback_cwd(), - )) - } else { - Arc::new(LocalStdioServerLauncher::new( - runtime_environment.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. RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd, launcher) .await .map_err(|err| StartupOutcomeError::from(anyhow!(err))) @@ -630,16 +615,10 @@ 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" - ))); - }; - environment.get_http_client() - } else { - Arc::new(ReqwestHttpClient) - }; + 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 dc1fcec2198..5404bf1eb53 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -10,6 +10,7 @@ use std::sync::Arc; use std::time::Duration; use codex_exec_server::Environment; +use codex_exec_server::EnvironmentManager; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::SandboxPolicy; @@ -28,81 +29,85 @@ pub struct SandboxState { pub use_legacy_landlock: bool, } -/// Runtime placement information used when starting MCP server transports. +/// Runtime context used when resolving per-server MCP environments. /// -/// `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 local stdio fallback cwd used when a local +/// stdio server omits its own working directory. #[derive(Clone)] -pub struct McpRuntimeEnvironment { - environment: Option>, - local_environment: Option>, - fallback_cwd: PathBuf, +pub struct McpRuntimeContext { + environment_manager: Arc, + local_stdio_fallback_cwd: PathBuf, } -impl McpRuntimeEnvironment { +impl McpRuntimeContext { pub fn new( - environment: Option>, - local_environment: Option>, - fallback_cwd: PathBuf, + environment_manager: Arc, + local_stdio_fallback_cwd: PathBuf, ) -> Self { Self { - environment, - local_environment, - fallback_cwd, + environment_manager, + local_stdio_fallback_cwd, } } - pub(crate) fn environment(&self) -> Option> { - self.environment.as_ref().map(Arc::clone) + pub(crate) fn local_stdio_fallback_cwd(&self) -> PathBuf { + self.local_stdio_fallback_cwd.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>, 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)?; } - 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" + 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" )), - }, - Some(environment) => Some(format!( - "unsupported experimental_environment `{environment}` for MCP server `{server_name}`" - )), + codex_config::McpServerTransportConfig::StreamableHttp { .. } => Ok(None), + }; } + + Err(format!( + "MCP server `{server_name}` references unknown environment id `{}`", + config.environment_id + )) + } +} + +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)]) { @@ -115,13 +120,15 @@ 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; use pretty_assertions::assert_eq; use super::*; - fn stdio_server(experimental_environment: Option<&str>) -> McpServerConfig { + fn stdio_server(environment_id: &str) -> McpServerConfig { McpServerConfig { transport: McpServerTransportConfig::Stdio { command: "echo".to_string(), @@ -130,7 +137,7 @@ mod tests { env_vars: Vec::new(), cwd: None, }, - experimental_environment: experimental_environment.map(str::to_string), + environment_id: environment_id.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -147,7 +154,7 @@ mod tests { } } - fn http_server(experimental_environment: Option<&str>) -> McpServerConfig { + fn http_server(environment_id: &str) -> McpServerConfig { McpServerConfig { transport: McpServerTransportConfig::StreamableHttp { url: "http://127.0.0.1:1".to_string(), @@ -155,111 +162,135 @@ 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.to_string(), + ..stdio_server(environment_id) } } #[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"), ); + let error = match runtime_context + .resolve_server_environment("stdio", &stdio_server(DEFAULT_MCP_SERVER_ENVIRONMENT_ID)) + { + Ok(_) => panic!("local stdio MCP should require a local environment"), + Err(error) => error, + }; 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()) + error, + "local stdio MCP server `stdio` requires a local environment" ); } #[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 - ); + let resolved_runtime = match runtime_context + .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!(resolved_runtime.is_none()); } #[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"), ); + 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_environment.startup_unavailable_reason( - "stdio", - &stdio_server(/*experimental_environment*/ Some("remote")), - ), - Some("remote MCP server `stdio` requires a remote environment".to_string()) + error, + "MCP server `stdio` references unknown environment id `remote`" ); } - #[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 - ); - assert_eq!( - runtime_environment.startup_unavailable_reason( - "http", - &http_server(/*experimental_environment*/ Some("remote")), - ), - None - ); + 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_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!(resolved_runtime.is_some()); + } } #[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"), + async fn local_stdio_accepts_local_environment_when_available() { + let runtime_context = McpRuntimeContext::new( + Arc::new(EnvironmentManager::default_for_tests()), + PathBuf::from("/tmp"), ); - let runtime_environment = McpRuntimeEnvironment::new( - Some(remote_environment), - Some(local_environment), + + let resolved_runtime = match runtime_context + .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!(resolved_runtime.is_some()); + } + + #[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_environment("stdio", &remote_stdio) { + Ok(_) => panic!("remote stdio MCP should require absolute cwd"), + Err(error) => error, + }; assert_eq!( - runtime_environment.startup_unavailable_reason( - "stdio", - &stdio_server(/*experimental_environment*/ None), - ), - None + error, + "remote stdio MCP server `stdio` requires an absolute cwd, got `relative`" ); } } 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 aaad3444fdc..7e062e29984 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 !config.is_local_environment() { + 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 0215e7b8086..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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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.rs b/codex-rs/config/src/mcp_types.rs index 8dfceec37e9..0d30ba8b23f 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 { @@ -128,9 +131,8 @@ pub struct McpServerConfig { #[serde(flatten)] pub transport: McpServerTransportConfig, - /// Experimental environment selector for where Codex should start this MCP server. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub experimental_environment: 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")] @@ -190,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() @@ -231,7 +237,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 +285,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, @@ -350,9 +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, - experimental_environment, + environment_id, startup_timeout_sec, tool_timeout_sec, enabled: enabled.unwrap_or_else(default_enabled), @@ -385,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 f3971154250..5f3933ce43e 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -51,6 +51,62 @@ fn deserialize_stdio_command_server_config_with_args() { assert!(cfg.enabled); } +#[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 = match toml::from_str(&format!( + r#" + command = "echo" + environment_id = "remote" + cwd = {cwd:?} + "# + )) { + Ok(cfg) => cfg, + Err(error) => panic!("remote stdio MCP should accept absolute cwd: {error}"), + }; + + 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( @@ -402,7 +458,7 @@ fn deserialize_ignores_unknown_server_fields() { env_vars: Vec::new(), cwd: None, }, - experimental_environment: None, + environment_id: crate::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.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 fa0b485fa2c..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, }, - experimental_environment: 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, }, - experimental_environment: 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, }, - experimental_environment: 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, }, - experimental_environment: None, + environment_id: "local".to_string(), 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..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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5144,9 +5144,9 @@ 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()), }, - experimental_environment: Some("remote".to_string()), + environment_id: "remote".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -5184,13 +5184,13 @@ 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:?}"), } 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, "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: 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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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()), }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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(), )])), }, - experimental_environment: None, + 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(), )])), }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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(), )])), }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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.rs b/codex-rs/core/src/config/edit.rs index f467c76cf97..1b5cd879c45 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 !config.is_local_environment() { + 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 30f82db29ba..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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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/connectors.rs b/codex-rs/core/src/connectors.rs index de41919a895..8593686c704 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; @@ -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,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(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..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, }, - experimental_environment: None, + 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, }, - experimental_environment: None, + 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/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index f9984c7f2d6..0d6b8ff1ac5 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,14 +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::McpRuntimeEnvironment::new( - Some(environment), - session.services.environment_manager.try_local_environment(), - { - #[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/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..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, }, - experimental_environment: 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 80290e31bae..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, }, - experimental_environment: None, + 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 8e85c5d5eef..94222deab8b 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_experimental_environment() -> 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(|| codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.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 { - experimental_environment: Option, + environment_id: String, supports_parallel_tool_calls: bool, tool_timeout_sec: Option, } +impl Default for TestMcpServerOptions { + fn default() -> Self { + Self { + environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), + supports_parallel_tool_calls: false, + tool_timeout_sec: None, + } + } +} + fn stdio_transport( command: String, env: Option>, @@ -326,7 +337,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 +491,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 +614,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() }, ); @@ -623,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 { - experimental_environment: remote_aware_experimental_environment(), - ..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)] @@ -775,7 +733,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 +830,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 +947,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 +1029,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 +1162,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 +1298,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 +1400,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 +1518,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 +1610,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 +1793,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 +1979,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..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, }, - experimental_environment: 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, }, - experimental_environment: 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, }, - experimental_environment: 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 f576e55a3d0..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, }, - experimental_environment: 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 be9267934d6..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, }, - experimental_environment: 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, }, - experimental_environment: 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, }, - experimental_environment: None, + environment_id: "local".to_string(), enabled: true, required: false, supports_parallel_tool_calls: false, 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,