From 8993a195b8bfa58b31f10320335cbba255d9d2b0 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 19 May 2026 04:23:47 +0000 Subject: [PATCH] Narrow app-server runtime capability follow-up --- codex-rs/app-server/src/message_processor.rs | 1 - .../src/request_processors/mcp_processor.rs | 94 +++++-------------- .../request_processors/thread_processor.rs | 36 +++---- .../thread_processor_tests.rs | 24 ----- 4 files changed, 34 insertions(+), 121 deletions(-) diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 4c6a71021c..c9284edc2a 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -388,7 +388,6 @@ impl MessageProcessor { Arc::clone(&thread_manager), outgoing.clone(), config_manager.clone(), - Arc::clone(&runtime_capabilities), ); let plugin_processor = PluginRequestProcessor::new( auth_manager.clone(), 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 644d135dce..548bb5be87 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -8,7 +8,6 @@ pub(crate) struct McpRequestProcessor { thread_manager: Arc, outgoing: Arc, config_manager: ConfigManager, - runtime_capabilities: Arc, } impl McpRequestProcessor { @@ -17,14 +16,12 @@ impl McpRequestProcessor { thread_manager: Arc, outgoing: Arc, config_manager: ConfigManager, - runtime_capabilities: Arc, ) -> Self { Self { auth_manager, thread_manager, outgoing, config_manager, - runtime_capabilities, } } @@ -207,12 +204,18 @@ impl McpRequestProcessor { .to_mcp_config(self.thread_manager.plugins_manager().as_ref()) .await; let auth = self.auth_manager.auth().await; - let runtime_environment = runtime_environment_without_thread( - self.thread_manager.environment_manager().as_ref(), - &self.runtime_capabilities, - config.cwd.to_path_buf(), - "list MCP server status without thread", - )?; + let environment_manager = self.thread_manager.environment_manager(); + let runtime_environment = match environment_manager.default_environment() { + Some(environment) => { + // Status listing has no turn cwd. This fallback is used only + // by executor-backed stdio MCPs whose config omits `cwd`. + McpRuntimeEnvironment::new(environment, config.cwd.to_path_buf()) + } + None => McpRuntimeEnvironment::new( + environment_manager.local_environment(), + config.cwd.to_path_buf(), + ), + }; tokio::spawn(async move { Self::list_mcp_server_status_task( @@ -366,12 +369,15 @@ impl McpRequestProcessor { .to_mcp_config(self.thread_manager.plugins_manager().as_ref()) .await; let auth = self.auth_manager.auth().await; - let runtime_environment = runtime_environment_without_thread( - self.thread_manager.environment_manager().as_ref(), - &self.runtime_capabilities, - config.cwd.to_path_buf(), - "read MCP resource without thread", - )?; + let runtime_environment = { + let environment_manager = self.thread_manager.environment_manager(); + let environment = environment_manager + .default_environment() + .unwrap_or_else(|| environment_manager.local_environment()); + // Resource reads without a thread have no turn cwd. This fallback + // is used only by executor-backed stdio MCPs whose config omits `cwd`. + McpRuntimeEnvironment::new(environment, config.cwd.to_path_buf()) + }; let request_id = request_id.clone(); tokio::spawn(async move { @@ -429,64 +435,6 @@ impl McpRequestProcessor { } } -fn runtime_environment_without_thread( - environment_manager: &EnvironmentManager, - runtime_capabilities: &RuntimeCapabilities, - cwd: PathBuf, - local_fallback_operation: &str, -) -> Result { - let environment = match environment_manager.default_environment() { - Some(environment) => environment, - None => runtime_capabilities - .require_local_environment(local_fallback_operation) - .map_err(|err| internal_error(err.to_string()))?, - }; - // Threadless MCP requests have no turn cwd. This fallback is used only - // by executor-backed stdio MCPs whose config omits `cwd`. - Ok(McpRuntimeEnvironment::new(environment, cwd)) -} - -#[cfg(test)] -mod tests { - use super::runtime_environment_without_thread; - use codex_core::RuntimeCapabilities; - use codex_exec_server::EnvironmentManager; - use codex_exec_server::ExecServerRuntimePaths; - use codex_utils_absolute_path::AbsolutePathBuf; - use pretty_assertions::assert_eq; - - #[test] - fn threadless_mcp_local_fallback_rejects_isolated_runtime() { - let environment_manager = EnvironmentManager::disabled_for_tests(test_runtime_paths()); - let error = runtime_environment_without_thread( - &environment_manager, - &RuntimeCapabilities::isolated(), - test_cwd(), - "list MCP server status without thread", - ) - .expect_err("isolated runtime should reject local MCP fallback"); - - assert_eq!( - error.message, - "list MCP server status without thread requires ambient worker-local environment" - ); - } - - fn test_runtime_paths() -> ExecServerRuntimePaths { - ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe"), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths") - } - - fn test_cwd() -> std::path::PathBuf { - AbsolutePathBuf::current_dir() - .expect("current dir") - .to_path_buf() - } -} - fn with_mcp_tool_call_thread_id_meta( meta: Option, thread_id: &str, diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index 567068b36e..2e7b590afc 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -657,16 +657,10 @@ impl ThreadRequestProcessor { .map(|response| Some(response.into())) } - async fn instruction_sources_from_config( - runtime_capabilities: &RuntimeCapabilities, - config: &Config, - ) -> Result, JSONRPCErrorError> { - let file_system = runtime_capabilities - .require_local_filesystem("load thread instruction sources") - .map_err(|err| internal_error(err.to_string()))?; - Ok(codex_core::AgentsMdManager::new(config) - .instruction_sources(file_system.as_ref()) - .await) + async fn instruction_sources_from_config(config: &Config) -> Vec { + codex_core::AgentsMdManager::new(config) + .instruction_sources(LOCAL_FS.as_ref()) + .await } async fn load_thread( @@ -889,12 +883,14 @@ impl ThreadRequestProcessor { }; let request_trace = request_context.request_trace(); let config_manager = self.config_manager.clone(); + let runtime_capabilities = Arc::clone(&self.runtime_capabilities); let outgoing = Arc::clone(&listener_task_context.outgoing); let error_request_id = request_id.clone(); let thread_start_task = async move { if let Err(error) = Self::thread_start_task( listener_task_context, config_manager, + runtime_capabilities, request_id, app_server_client_name, app_server_client_version, @@ -979,6 +975,7 @@ impl ThreadRequestProcessor { async fn thread_start_task( listener_task_context: ListenerTaskContext, config_manager: ConfigManager, + runtime_capabilities: Arc, request_id: ConnectionRequestId, app_server_client_name: Option, app_server_client_version: Option, @@ -1015,8 +1012,7 @@ impl ThreadRequestProcessor { && config.active_project.trust_level.is_none() && (requested_permissions_trust_project || effective_permissions_trust_project) { - let file_system = self - .runtime_capabilities + let file_system = runtime_capabilities .require_local_filesystem("trust requested thread cwd") .map_err(|err| internal_error(err.to_string()))?; let trust_target = @@ -1069,8 +1065,7 @@ impl ThreadRequestProcessor { .map_err(|err| config_load_error(&err))?; } - let instruction_sources = - Self::instruction_sources_from_config(&self.runtime_capabilities, &config).await?; + let instruction_sources = Self::instruction_sources_from_config(&config).await; let environments = environments.unwrap_or_else(|| { listener_task_context .thread_manager @@ -2454,8 +2449,7 @@ impl ThreadRequestProcessor { } }; - let instruction_sources = - Self::instruction_sources_from_config(&self.runtime_capabilities, &config).await?; + let instruction_sources = Self::instruction_sources_from_config(&config).await; let response_history = thread_history.clone(); match self @@ -2740,11 +2734,8 @@ impl ThreadRequestProcessor { thread_summary.session_id = existing_thread.session_configured().session_id.to_string(); let mut config_for_instruction_sources = self.config.as_ref().clone(); config_for_instruction_sources.cwd = config_snapshot.cwd.clone(); - let instruction_sources = Self::instruction_sources_from_config( - &self.runtime_capabilities, - &config_for_instruction_sources, - ) - .await?; + let instruction_sources = + Self::instruction_sources_from_config(&config_for_instruction_sources).await; let listener_command_tx = { let thread_state = thread_state.lock().await; @@ -3114,8 +3105,7 @@ impl ThreadRequestProcessor { .map_err(|err| config_load_error(&err))?; let fallback_model_provider = config.model_provider_id.clone(); - let instruction_sources = - Self::instruction_sources_from_config(&self.runtime_capabilities, &config).await?; + let instruction_sources = Self::instruction_sources_from_config(&config).await; let NewThread { thread_id, diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index 59833d9741..f62b52040e 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -59,8 +59,6 @@ mod thread_processor_behavior_tests { use codex_config::SessionThreadConfig; use codex_config::StaticThreadConfigLoader; use codex_config::ThreadConfigSource; - use codex_core::RuntimeCapabilities; - use codex_core::config::ConfigBuilder; use codex_model_provider_info::ModelProviderInfo; use codex_model_provider_info::WireApi; use codex_protocol::ThreadId; @@ -90,28 +88,6 @@ mod thread_processor_behavior_tests { use std::sync::Arc; use tempfile::TempDir; - #[tokio::test] - async fn instruction_sources_reject_isolated_runtime_without_local_filesystem() { - let temp_dir = TempDir::new().expect("tempdir"); - let config = ConfigBuilder::without_managed_config_for_tests() - .codex_home(temp_dir.path().to_path_buf()) - .build() - .await - .expect("build config"); - - let error = ThreadRequestProcessor::instruction_sources_from_config( - &RuntimeCapabilities::isolated(), - &config, - ) - .await - .expect_err("isolated runtime should reject instruction source load"); - - assert_eq!( - error.message, - "load thread instruction sources requires ambient worker-local filesystem" - ); - } - #[test] fn validate_dynamic_tools_rejects_unsupported_input_schema() { let tools = vec![ApiDynamicToolSpec {