From 8a517dd5310797584a8d9c3649cb16aff9c82fd8 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 15:02:29 -0700 Subject: [PATCH 01/20] Support multiple managed environments Refactor EnvironmentManager to own a keyed environment registry with explicit default and local lookups. Keep remote exec-server connections lazy at environment use sites and preserve disabled agent environment access separately from internal local environment access. Co-authored-by: Codex --- codex-rs/app-server-client/src/lib.rs | 21 +- .../app-server/src/bespoke_event_handling.rs | 4 +- .../app-server/src/codex_message_processor.rs | 70 +--- codex-rs/app-server/src/in_process.rs | 4 +- .../src/message_processor/tracing_tests.rs | 4 +- .../app-server/tests/suite/v2/mcp_resource.rs | 4 +- codex-rs/core/src/agent/control_tests.rs | 28 +- codex-rs/core/src/codex_delegate.rs | 5 +- codex-rs/core/src/memories/tests.rs | 4 +- codex-rs/core/src/session/mod.rs | 7 +- codex-rs/core/src/session/review.rs | 2 +- codex-rs/core/src/session/session.rs | 8 +- codex-rs/core/src/session/tests.rs | 20 +- .../core/src/session/tests/guardian_tests.rs | 4 +- codex-rs/core/src/session/turn_context.rs | 4 +- codex-rs/core/src/state/service.rs | 3 + codex-rs/core/src/thread_manager.rs | 10 +- codex-rs/core/src/thread_manager_tests.rs | 20 +- codex-rs/core/tests/common/test_codex.rs | 14 +- codex-rs/core/tests/suite/client.rs | 4 +- codex-rs/core/tests/suite/skills.rs | 7 +- codex-rs/exec-server/src/environment.rs | 332 +++++++++++------- .../exec-server/src/remote_file_system.rs | 34 +- codex-rs/exec-server/src/remote_process.rs | 11 +- codex-rs/exec-server/tests/exec_process.rs | 4 +- codex-rs/exec-server/tests/file_system.rs | 4 +- codex-rs/tui/src/app/test_support.rs | 4 +- codex-rs/tui/src/app/tests.rs | 8 +- codex-rs/tui/src/lib.rs | 34 +- codex-rs/tui/src/onboarding/auth.rs | 8 +- 30 files changed, 394 insertions(+), 292 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index c6f678c2aa22..31184063e921 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -46,6 +46,7 @@ use codex_core::config::Config; use codex_core::config_loader::CloudRequirementsLoader; use codex_core::config_loader::LoaderOverrides; pub use codex_exec_server::EnvironmentManager; +pub use codex_exec_server::EnvironmentManagerArgs; pub use codex_exec_server::ExecServerRuntimePaths; use codex_feedback::CodexFeedback; use codex_protocol::protocol::SessionSource; @@ -968,7 +969,9 @@ mod tests { cloud_requirements: CloudRequirementsLoader::default(), feedback: CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), config_warnings: Vec::new(), session_source, enable_codex_api_key_env: false, @@ -1969,9 +1972,12 @@ mod tests { #[tokio::test] async fn runtime_start_args_forward_environment_manager() { let config = Arc::new(build_test_config().await); - let environment_manager = Arc::new(EnvironmentManager::new(Some( - "ws://127.0.0.1:8765".to_string(), - ))); + let environment_manager = Arc::new(EnvironmentManager::from_exec_server_url( + EnvironmentManagerArgs { + exec_server_url: Some("ws://127.0.0.1:8765".to_string()), + local_runtime_paths: None, + }, + )); let runtime_args = InProcessClientStartArgs { arg0_paths: Arg0DispatchPaths::default(), @@ -1998,7 +2004,12 @@ mod tests { &runtime_args.environment_manager, &environment_manager )); - assert!(runtime_args.environment_manager.is_remote()); + assert!( + runtime_args + .environment_manager + .default_environment() + .is_some_and(|environment| environment.is_remote()) + ); } #[tokio::test] diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index a2ea77900aa9..43a75df6780b 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -3497,8 +3497,8 @@ mod tests { CodexAuth::create_dummy_chatgpt_auth_for_testing(), config.model_provider.clone(), config.codex_home.to_path_buf(), - Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), ), ); diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 4669c0cb1a31..15c0782d660b 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -5677,27 +5677,17 @@ impl CodexMessageProcessor { .to_mcp_config(self.thread_manager.plugins_manager().as_ref()) .await; let auth = self.auth_manager.auth().await; - let runtime_environment = match self.thread_manager.environment_manager().current().await { - Ok(Some(environment)) => { + 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()) } - Ok(None) => McpRuntimeEnvironment::new( - Arc::new(codex_exec_server::Environment::default()), + None => McpRuntimeEnvironment::new( + environment_manager.local_environment(), config.cwd.to_path_buf(), ), - Err(err) => { - // TODO(aibrahim): Investigate degrading MCP status listing when - // executor environment creation fails. - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to create environment: {err}"), - data: None, - }; - self.outgoing.send_error(request, error).await; - return; - } }; tokio::spawn(async move { @@ -5856,25 +5846,15 @@ impl CodexMessageProcessor { .to_mcp_config(self.thread_manager.plugins_manager().as_ref()) .await; let auth = self.auth_manager.auth().await; - let runtime_environment = match self.thread_manager.environment_manager().current().await { - Ok(Some(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()) - } - Ok(None) => McpRuntimeEnvironment::new( - Arc::new(codex_exec_server::Environment::default()), - config.cwd.to_path_buf(), - ), - Err(err) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to create environment: {err}"), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; - } + let runtime_environment = { + let environment = self + .thread_manager + .environment_manager() + .default_environment() + .unwrap_or_else(|| Arc::new(codex_exec_server::Environment::default())); + // 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()) }; tokio::spawn(async move { @@ -6461,23 +6441,11 @@ impl CodexMessageProcessor { }; let skills_manager = self.thread_manager.skills_manager(); let plugins_manager = self.thread_manager.plugins_manager(); - let fs = match self.thread_manager.environment_manager().current().await { - Ok(Some(environment)) => Some(environment.get_filesystem()), - Ok(None) => None, - Err(err) => { - self.outgoing - .send_error( - request_id, - JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to create environment: {err}"), - data: None, - }, - ) - .await; - return; - } - }; + let fs = self + .thread_manager + .environment_manager() + .default_environment() + .map(|environment| environment.get_filesystem()); let mut data = Vec::new(); for cwd in cwds { let extra_roots = extra_roots_by_cwd diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 4fe88fa277ac..c586d3af103f 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -750,7 +750,9 @@ mod tests { thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), config_warnings: Vec::new(), session_source, enable_codex_api_key_env: false, diff --git a/codex-rs/app-server/src/message_processor/tracing_tests.rs b/codex-rs/app-server/src/message_processor/tracing_tests.rs index bf5986f6de69..cb697ae3883e 100644 --- a/codex-rs/app-server/src/message_processor/tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor/tracing_tests.rs @@ -279,7 +279,9 @@ fn build_test_processor( arg0_paths: Arg0DispatchPaths::default(), config, config_manager, - environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), feedback: CodexFeedback::new(), log_db: None, config_warnings: Vec::new(), diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs index c26b456fa91c..34b7a55d6aa4 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -204,7 +204,9 @@ async fn mcp_resource_read_returns_error_for_unknown_thread() -> Result<()> { thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), config_warnings: Vec::new(), session_source: SessionSource::Cli, enable_codex_api_key_env: false, diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 10850ef8c74e..473180caa3e0 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -95,8 +95,8 @@ impl AgentControlHarness { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), ); let control = manager.agent_control(); @@ -911,8 +911,8 @@ async fn spawn_agent_respects_max_threads_limit() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), ); let control = manager.agent_control(); @@ -965,8 +965,8 @@ async fn spawn_agent_releases_slot_after_shutdown() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), ); let control = manager.agent_control(); @@ -1010,8 +1010,8 @@ async fn spawn_agent_limit_shared_across_clones() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), ); let control = manager.agent_control(); @@ -1057,8 +1057,8 @@ async fn resume_agent_respects_max_threads_limit() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), ); let control = manager.agent_control(); @@ -1115,8 +1115,8 @@ async fn resume_agent_releases_slot_after_resume_failure() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), ); let control = manager.agent_control(); @@ -1512,8 +1512,8 @@ async fn resume_thread_subagent_restores_stored_nickname_and_role() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), ); let control = manager.agent_control(); diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 7247c601f46e..4f4ced4101f4 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -4,7 +4,6 @@ use std::sync::Arc; use async_channel::Receiver; use async_channel::Sender; use codex_async_utils::OrCancelExt; -use codex_exec_server::EnvironmentManager; use codex_protocol::protocol::ApplyPatchApprovalRequestEvent; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; @@ -78,9 +77,7 @@ pub(crate) async fn run_codex_thread_interactive( config, auth_manager, models_manager, - environment_manager: Arc::new(EnvironmentManager::from_environment( - parent_ctx.environment.as_deref(), - )), + environment_manager: Arc::clone(&parent_session.services.environment_manager), skills_manager: Arc::clone(&parent_session.services.skills_manager), plugins_manager: Arc::clone(&parent_session.services.plugins_manager), mcp_manager: Arc::clone(&parent_session.services.mcp_manager), diff --git a/codex-rs/core/src/memories/tests.rs b/codex-rs/core/src/memories/tests.rs index af698e1eaaa3..2b68596fd424 100644 --- a/codex-rs/core/src/memories/tests.rs +++ b/codex-rs/core/src/memories/tests.rs @@ -491,8 +491,8 @@ mod phase2 { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), ); let (mut session, _turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 80084f197930..1aa15a2cad8c 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -458,10 +458,7 @@ impl Codex { let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let (tx_event, rx_event) = async_channel::unbounded(); - let environment = environment_manager - .current() - .await - .map_err(|err| CodexErr::Fatal(format!("failed to create environment: {err}")))?; + let environment = environment_manager.default_environment(); let fs = environment .as_ref() .map(|environment| environment.get_filesystem()); @@ -648,7 +645,7 @@ impl Codex { mcp_manager.clone(), skills_watcher, agent_control, - environment, + environment_manager, analytics_events_client, ) .await diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index 94de4617d5a4..af1028686d2f 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -46,7 +46,7 @@ pub(super) async fn spawn_review_thread( ) .with_web_search_config(/*web_search_config*/ None) .with_allow_login_shell(config.permissions.allow_login_shell) - .with_has_environment(parent_turn_context.environment.is_some()) + .with_has_environment(parent_turn_context.tools_config.has_environment) .with_spawn_agent_usage_hint(config.multi_agent_v2.usage_hint_enabled) .with_spawn_agent_usage_hint_text(config.multi_agent_v2.usage_hint_text.clone()) .with_hide_spawn_agent_metadata(config.multi_agent_v2.hide_spawn_agent_metadata) diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 640e59b5b712..232bfaf09add 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -227,7 +227,7 @@ impl Session { mcp_manager: Arc, skills_watcher: Arc, agent_control: AgentControl, - environment: Option>, + environment_manager: Arc, analytics_events_client: Option, ) -> anyhow::Result> { debug!( @@ -623,6 +623,8 @@ impl Session { config.analytics_enabled, ) }); + let environment = environment_manager.default_environment(); + let allows_agent_environment_access = environment_manager.allows_agent_environment_access(); let services = SessionServices { // Initialize the MCP connection manager with an uninitialized // instance. It will be replaced with one created via @@ -676,7 +678,9 @@ impl Session { code_mode_service: crate::tools::code_mode::CodeModeService::new( config.js_repl_node_path.clone(), ), - environment: environment.clone(), + environment_manager, + environment, + allows_agent_environment_access, }; services .model_client diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index ee72769bda6a..7378dd15ec2d 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -2945,11 +2945,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() { mcp_manager, Arc::new(SkillsWatcher::noop()), AgentControl::default(), - Some(Arc::new( - codex_exec_server::Environment::create(/*exec_server_url*/ None) - .await - .expect("create environment"), - )), + Arc::new(codex_exec_server::EnvironmentManager::default()), /*analytics_events_client*/ None, ) .await; @@ -3047,7 +3043,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { let network_approval = Arc::new(NetworkApprovalService::default()); let environment = Arc::new( codex_exec_server::Environment::create(/*exec_server_url*/ None) - .await .expect("create environment"), ); @@ -3107,7 +3102,9 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { code_mode_service: crate::tools::code_mode::CodeModeService::new( config.js_repl_node_path.clone(), ), + environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default()), environment: Some(Arc::clone(&environment)), + allows_agent_environment_access: true, }; let js_repl = Arc::new(JsReplHandle::with_node_path( config.js_repl_node_path.clone(), @@ -3141,6 +3138,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { model_info, &models_manager, /*network*/ None, + /*allows_agent_environment_access*/ true, Some(environment), "turn_id".to_string(), Arc::clone(&js_repl), @@ -3262,11 +3260,7 @@ async fn make_session_with_config_and_rx( mcp_manager, Arc::new(SkillsWatcher::noop()), AgentControl::default(), - Some(Arc::new( - codex_exec_server::Environment::create(/*exec_server_url*/ None) - .await - .expect("create environment"), - )), + Arc::new(codex_exec_server::EnvironmentManager::default()), /*analytics_events_client*/ None, ) .await?; @@ -4138,7 +4132,6 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( let network_approval = Arc::new(NetworkApprovalService::default()); let environment = Arc::new( codex_exec_server::Environment::create(/*exec_server_url*/ None) - .await .expect("create environment"), ); @@ -4198,7 +4191,9 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( code_mode_service: crate::tools::code_mode::CodeModeService::new( config.js_repl_node_path.clone(), ), + environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default()), environment: Some(Arc::clone(&environment)), + allows_agent_environment_access: true, }; let js_repl = Arc::new(JsReplHandle::with_node_path( config.js_repl_node_path.clone(), @@ -4232,6 +4227,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( model_info, &models_manager, /*network*/ None, + /*allows_agent_environment_access*/ true, Some(environment), "turn_id".to_string(), Arc::clone(&js_repl), diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 73a0de285a0f..e59faee713d7 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -635,7 +635,9 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { config, auth_manager, models_manager, - environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), skills_manager, plugins_manager, mcp_manager, diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index dd86804ee5d6..58fcfc73b8c9 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -345,6 +345,7 @@ impl Session { model_info: ModelInfo, models_manager: &ModelsManager, network: Option, + allows_agent_environment_access: bool, environment: Option>, sub_id: String, js_repl: Arc, @@ -381,7 +382,7 @@ impl Session { ) .with_web_search_config(per_turn_config.web_search_config.clone()) .with_allow_login_shell(per_turn_config.permissions.allow_login_shell) - .with_has_environment(environment.is_some()) + .with_has_environment(allows_agent_environment_access) .with_spawn_agent_usage_hint(per_turn_config.multi_agent_v2.usage_hint_enabled) .with_spawn_agent_usage_hint_text(per_turn_config.multi_agent_v2.usage_hint_text.clone()) .with_hide_spawn_agent_metadata(per_turn_config.multi_agent_v2.hide_spawn_agent_metadata) @@ -576,6 +577,7 @@ impl Session { ) .then(|| started_proxy.proxy()) }), + self.services.allows_agent_environment_access, self.services.environment.clone(), sub_id, Arc::clone(&self.js_repl), diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index aae02d61bdc6..74d55afe2add 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -17,6 +17,7 @@ use crate::tools::sandboxing::ApprovalStore; use crate::unified_exec::UnifiedExecProcessManager; use codex_analytics::AnalyticsEventsClient; use codex_exec_server::Environment; +use codex_exec_server::EnvironmentManager; use codex_hooks::Hooks; use codex_login::AuthManager; use codex_mcp::McpConnectionManager; @@ -62,5 +63,7 @@ pub(crate) struct SessionServices { /// Session-scoped model client shared across turns. pub(crate) model_client: ModelClient, pub(crate) code_mode_service: CodeModeService, + pub(crate) environment_manager: Arc, pub(crate) environment: Option>, + pub(crate) allows_agent_environment_access: bool, } diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index e4da99bb55a0..293423defe91 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -301,7 +301,9 @@ impl ThreadManager { auth, provider, codex_home.clone(), - Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), ); manager._test_codex_home_guard = Some(TempCodexHomeGuard { path: codex_home }); manager @@ -920,11 +922,7 @@ impl ThreadManagerState { parent_trace: Option, user_shell_override: Option, ) -> CodexResult { - let environment = self - .environment_manager - .current() - .await - .map_err(|err| CodexErr::Fatal(format!("failed to create environment: {err}")))?; + let environment = self.environment_manager.default_environment(); let watch_registration = match environment.as_ref() { Some(environment) if !environment.is_remote() => { self.skills_watcher diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index fe2039e89bc4..8c0fbe5e3805 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -246,8 +246,8 @@ async fn shutdown_all_threads_bounded_submits_shutdown_to_every_thread() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), ); let thread_1 = manager @@ -297,8 +297,8 @@ async fn new_uses_configured_openai_provider_for_model_refresh() { auth_manager, SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), /*analytics_events_client*/ None, ); @@ -434,8 +434,8 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor auth_manager.clone(), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), /*analytics_events_client*/ None, ); @@ -537,8 +537,8 @@ async fn interrupted_fork_snapshot_preserves_explicit_turn_id() { auth_manager.clone(), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), /*analytics_events_client*/ None, ); @@ -630,8 +630,8 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ auth_manager.clone(), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), /*analytics_events_client*/ None, ); diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 5075f91620e5..32f5657b3498 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -76,7 +76,7 @@ impl TestEnv { pub async fn local() -> Result { let local_cwd_temp_dir = Arc::new(TempDir::new()?); let cwd = local_cwd_temp_dir.abs(); - let environment = codex_exec_server::Environment::create(/*exec_server_url*/ None).await?; + let environment = codex_exec_server::Environment::create(/*exec_server_url*/ None)?; Ok(Self { environment, cwd, @@ -115,7 +115,7 @@ pub async fn test_env() -> Result { match get_remote_test_env() { Some(remote_env) => { let websocket_url = remote_exec_server_url()?; - let environment = codex_exec_server::Environment::create(Some(websocket_url)).await?; + let environment = codex_exec_server::Environment::create(Some(websocket_url))?; let cwd = remote_aware_cwd_path(); environment .get_filesystem() @@ -350,9 +350,13 @@ impl TestCodexBuilder { let (config, fallback_cwd) = self .prepare_config(base_url, &home, test_env.cwd().clone()) .await?; - let environment_manager = Arc::new(codex_exec_server::EnvironmentManager::new( - test_env.exec_server_url().map(str::to_owned), - )); + let environment_manager = + Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs { + exec_server_url: test_env.exec_server_url().map(str::to_owned), + local_runtime_paths: None, + }, + )); let file_system = test_env.environment().get_filesystem(); let mut workspace_setups = vec![]; swap(&mut self.workspace_setups, &mut workspace_setups); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 2086367b21e7..3bebd2192c70 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1103,8 +1103,8 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() { .features .enabled(Feature::DefaultModeRequestUserInput), }, - Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, + Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), )), /*analytics_events_client*/ None, ); diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 1b2ac71643b2..4ff6da00d131 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -234,7 +234,12 @@ async fn list_skills_skips_cwd_roots_when_environment_disabled() -> Result<()> { codex_core::test_support::auth_manager_from_auth(CodexAuth::from_api_key("dummy")), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(EnvironmentManager::new(Some("none".to_string()))), + Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs { + exec_server_url: Some("none".to_string()), + local_runtime_paths: None, + }, + )), /*analytics_events_client*/ None, ); let new_thread = thread_manager.start_thread(config.clone()).await?; diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index afe072019600..d43d2ba2c45e 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -1,4 +1,6 @@ +use std::collections::HashMap; use std::sync::Arc; +use std::time::Duration; use tokio::sync::OnceCell; @@ -15,45 +17,64 @@ use crate::remote_process::RemoteProcess; pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; -/// Lazily creates and caches the active environment for a session. +/// Owns the execution/filesystem environments available to a session. /// -/// The manager keeps the session's environment selection stable so subagents -/// and follow-up turns preserve an explicit disabled state. +/// The manager keeps the session's default environment selection stable while +/// separately tracking whether model-facing tools may access environments. #[derive(Debug)] pub struct EnvironmentManager { - exec_server_url: Option, - local_runtime_paths: Option, - disabled: bool, - current_environment: OnceCell>>, + default_environment: Option, + environment_disabled_for_agent: bool, + environments: HashMap>, } -impl Default for EnvironmentManager { - fn default() -> Self { - Self::new(/*exec_server_url*/ None) - } +pub const LOCAL_ENVIRONMENT_ID: &str = "local"; +pub const REMOTE_ENVIRONMENT_ID: &str = "remote"; + +#[derive(Clone, Debug, Default)] +pub struct EnvironmentManagerArgs { + pub exec_server_url: Option, + pub local_runtime_paths: Option, } -impl EnvironmentManager { - /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value. - pub fn new(exec_server_url: Option) -> Self { - Self::new_with_runtime_paths(exec_server_url, /*local_runtime_paths*/ None) - } +#[derive(Clone, Debug)] +pub(crate) struct LazyRemoteExecServerClient { + websocket_url: String, + client: Arc>, +} - /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value and local - /// runtime paths used when creating local filesystem helpers. - pub fn new_with_runtime_paths( - exec_server_url: Option, - local_runtime_paths: Option, - ) -> Self { - let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url); +impl LazyRemoteExecServerClient { + fn new(websocket_url: String) -> Self { Self { - exec_server_url, - local_runtime_paths, - disabled, - current_environment: OnceCell::new(), + websocket_url, + client: Arc::new(OnceCell::new()), } } + pub(crate) async fn get(&self) -> Result { + self.client + .get_or_try_init(|| async { + ExecServerClient::connect_websocket(RemoteExecServerConnectArgs { + websocket_url: self.websocket_url.clone(), + client_name: "codex-environment".to_string(), + connect_timeout: Duration::from_secs(5), + initialize_timeout: Duration::from_secs(5), + resume_session_id: None, + }) + .await + }) + .await + .cloned() + } +} + +impl Default for EnvironmentManager { + fn default() -> Self { + Self::from_exec_server_url(EnvironmentManagerArgs::default()) + } +} + +impl EnvironmentManager { /// Builds a manager from process environment variables. pub fn from_env() -> Self { Self::from_env_with_runtime_paths(/*local_runtime_paths*/ None) @@ -64,60 +85,81 @@ impl EnvironmentManager { pub fn from_env_with_runtime_paths( local_runtime_paths: Option, ) -> Self { - Self::new_with_runtime_paths( - std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), + Self::from_exec_server_url(EnvironmentManagerArgs { + exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), local_runtime_paths, - ) + }) } - /// Builds a manager from the currently selected environment, or from the - /// disabled mode when no environment is available. - pub fn from_environment(environment: Option<&Environment>) -> Self { - match environment { - Some(environment) => Self { - exec_server_url: environment.exec_server_url().map(str::to_owned), - local_runtime_paths: environment.local_runtime_paths().cloned(), - disabled: false, - current_environment: OnceCell::new(), - }, - None => Self { - exec_server_url: None, - local_runtime_paths: None, - disabled: true, - current_environment: OnceCell::new(), - }, + /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value and local + /// runtime paths used when creating local filesystem helpers. + pub fn from_exec_server_url(args: EnvironmentManagerArgs) -> Self { + let EnvironmentManagerArgs { + exec_server_url, + local_runtime_paths, + } = args; + let (exec_server_url, environment_disabled_for_agent) = + normalize_exec_server_url(exec_server_url); + let mut environments = HashMap::new(); + environments.insert( + LOCAL_ENVIRONMENT_ID.to_string(), + Arc::new( + Environment::create_with_runtime_paths( + /*exec_server_url*/ None, + local_runtime_paths.clone(), + ) + .expect("valid local environment"), + ), + ); + + let default_environment = match exec_server_url { + Some(exec_server_url) => { + environments.insert( + REMOTE_ENVIRONMENT_ID.to_string(), + Arc::new( + Environment::create_with_runtime_paths( + Some(exec_server_url), + local_runtime_paths, + ) + .expect("valid remote environment"), + ), + ); + Some(REMOTE_ENVIRONMENT_ID.to_string()) + } + None => Some(LOCAL_ENVIRONMENT_ID.to_string()), + }; + + Self { + default_environment, + environment_disabled_for_agent, + environments, } } - /// Returns the remote exec-server URL when one is configured. - pub fn exec_server_url(&self) -> Option<&str> { - self.exec_server_url.as_deref() + /// Returns true when model-facing tools may access an environment. + pub fn allows_agent_environment_access(&self) -> bool { + !self.environment_disabled_for_agent + && self + .default_environment + .as_deref() + .is_some_and(|environment_id| self.environments.contains_key(environment_id)) } - /// Returns true when this manager is configured to use a remote exec server. - pub fn is_remote(&self) -> bool { - self.exec_server_url.is_some() + /// Returns the default environment instance. + pub fn default_environment(&self) -> Option> { + self.default_environment + .as_deref() + .and_then(|environment_id| self.get_environment(environment_id)) } - /// Returns the cached environment, creating it on first access. - pub async fn current(&self) -> Result>, ExecServerError> { - self.current_environment - .get_or_try_init(|| async { - if self.disabled { - Ok(None) - } else { - Ok(Some(Arc::new( - Environment::create_with_runtime_paths( - self.exec_server_url.clone(), - self.local_runtime_paths.clone(), - ) - .await?, - ))) - } - }) - .await - .map(Option::as_ref) - .map(std::option::Option::<&Arc>::cloned) + /// Returns the local environment instance. + pub fn local_environment(&self) -> Option> { + self.get_environment(LOCAL_ENVIRONMENT_ID) + } + + /// Returns a named environment instance. + pub fn get_environment(&self, environment_id: &str) -> Option> { + self.environments.get(environment_id).cloned() } } @@ -128,7 +170,7 @@ impl EnvironmentManager { #[derive(Clone)] pub struct Environment { exec_server_url: Option, - remote_exec_server_client: Option, + remote_exec_server_client: Option, exec_backend: Arc, local_runtime_paths: Option, } @@ -154,13 +196,13 @@ impl std::fmt::Debug for Environment { impl Environment { /// Builds an environment from the raw `CODEX_EXEC_SERVER_URL` value. - pub async fn create(exec_server_url: Option) -> Result { - Self::create_with_runtime_paths(exec_server_url, /*local_runtime_paths*/ None).await + pub fn create(exec_server_url: Option) -> Result { + Self::create_with_runtime_paths(exec_server_url, /*local_runtime_paths*/ None) } /// Builds an environment from the raw `CODEX_EXEC_SERVER_URL` value and /// local runtime paths used when creating local filesystem helpers. - pub async fn create_with_runtime_paths( + pub(crate) fn create_with_runtime_paths( exec_server_url: Option, local_runtime_paths: Option, ) -> Result { @@ -171,17 +213,8 @@ impl Environment { )); } - let remote_exec_server_client = if let Some(exec_server_url) = &exec_server_url { - Some( - ExecServerClient::connect_websocket(RemoteExecServerConnectArgs { - websocket_url: exec_server_url.clone(), - client_name: "codex-environment".to_string(), - connect_timeout: std::time::Duration::from_secs(5), - initialize_timeout: std::time::Duration::from_secs(5), - resume_session_id: None, - }) - .await?, - ) + let remote_exec_server_client = if let Some(exec_server_url) = exec_server_url.clone() { + Some(LazyRemoteExecServerClient::new(exec_server_url)) } else { None }; @@ -242,15 +275,16 @@ mod tests { use super::Environment; use super::EnvironmentManager; + use super::EnvironmentManagerArgs; + use super::REMOTE_ENVIRONMENT_ID; use crate::ExecServerRuntimePaths; use crate::ProcessId; use pretty_assertions::assert_eq; #[tokio::test] async fn create_local_environment_does_not_connect() { - let environment = Environment::create(/*exec_server_url*/ None) - .await - .expect("create environment"); + let environment = + Environment::create(/*exec_server_url*/ None).expect("create environment"); assert_eq!(environment.exec_server_url(), None); assert!(environment.remote_exec_server_client.is_none()); @@ -258,39 +292,63 @@ mod tests { #[test] fn environment_manager_normalizes_empty_url() { - let manager = EnvironmentManager::new(Some(String::new())); + let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + exec_server_url: Some(String::new()), + local_runtime_paths: None, + }); - assert!(!manager.disabled); - assert_eq!(manager.exec_server_url(), None); - assert!(!manager.is_remote()); + let environment = manager.default_environment().expect("local environment"); + assert!(!environment.is_remote()); + assert!(manager.allows_agent_environment_access()); + assert!(manager.local_environment().is_some()); + assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } #[test] fn environment_manager_treats_none_value_as_disabled() { - let manager = EnvironmentManager::new(Some("none".to_string())); + let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + exec_server_url: Some("none".to_string()), + local_runtime_paths: None, + }); - assert!(manager.disabled); - assert_eq!(manager.exec_server_url(), None); - assert!(!manager.is_remote()); + assert!(!manager.allows_agent_environment_access()); + assert!(manager.default_environment().is_some()); + assert!(manager.local_environment().is_some()); + assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } #[test] fn environment_manager_reports_remote_url() { - let manager = EnvironmentManager::new(Some("ws://127.0.0.1:8765".to_string())); + let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + exec_server_url: Some("ws://127.0.0.1:8765".to_string()), + local_runtime_paths: None, + }); - assert!(manager.is_remote()); - assert_eq!(manager.exec_server_url(), Some("ws://127.0.0.1:8765")); + let environment = manager.default_environment().expect("remote environment"); + assert!(environment.is_remote()); + assert!(manager.allows_agent_environment_access()); + assert_eq!(environment.exec_server_url(), Some("ws://127.0.0.1:8765")); + assert!( + !manager + .local_environment() + .expect("local environment") + .is_remote() + ); + assert_eq!( + manager + .get_environment(REMOTE_ENVIRONMENT_ID) + .expect("remote environment") + .exec_server_url(), + Some("ws://127.0.0.1:8765") + ); } #[tokio::test] - async fn environment_manager_current_caches_environment() { - let manager = EnvironmentManager::new(/*exec_server_url*/ None); - - let first = manager.current().await.expect("get current environment"); - let second = manager.current().await.expect("get current environment"); + async fn environment_manager_default_environment_caches_environment() { + let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs::default()); - let first = first.expect("local environment"); - let second = second.expect("local environment"); + let first = manager.default_environment().expect("local environment"); + let second = manager.default_environment().expect("local environment"); assert!(Arc::ptr_eq(&first, &second)); } @@ -302,35 +360,51 @@ mod tests { /*codex_linux_sandbox_exe*/ None, ) .expect("runtime paths"); - let manager = EnvironmentManager::new_with_runtime_paths( - /*exec_server_url*/ None, - Some(runtime_paths.clone()), - ); + let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + exec_server_url: None, + local_runtime_paths: Some(runtime_paths.clone()), + }); - let environment = manager - .current() - .await - .expect("get current environment") - .expect("local environment"); + let environment = manager.default_environment().expect("local environment"); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); - assert_eq!( - EnvironmentManager::from_environment(Some(&environment)).local_runtime_paths, - Some(runtime_paths) - ); + let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + exec_server_url: environment.exec_server_url().map(str::to_owned), + local_runtime_paths: environment.local_runtime_paths().cloned(), + }); + let environment = manager.default_environment().expect("local environment"); + assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); } #[tokio::test] - async fn disabled_environment_manager_has_no_current_environment() { - let manager = EnvironmentManager::new(Some("none".to_string())); + async fn disabled_environment_manager_has_default_environment_but_no_tool_environment() { + let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + exec_server_url: Some("none".to_string()), + local_runtime_paths: None, + }); - assert!( - manager - .current() - .await - .expect("get current environment") - .is_none() - ); + assert!(manager.default_environment().is_some()); + assert!(!manager.allows_agent_environment_access()); + } + + #[tokio::test] + async fn environment_manager_allows_local_lookup_when_disabled() { + let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + exec_server_url: Some("none".to_string()), + local_runtime_paths: None, + }); + + assert!(manager.default_environment().is_some()); + assert!(!manager.allows_agent_environment_access()); + assert!(manager.local_environment().is_some()); + assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); + } + + #[tokio::test] + async fn get_environment_returns_none_for_unknown_id() { + let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs::default()); + + assert!(manager.get_environment("does-not-exist").is_none()); } #[tokio::test] diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index d6a32ba4d532..1b4150fe5ef6 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -7,7 +7,6 @@ use tracing::trace; use crate::CopyOptions; use crate::CreateDirectoryOptions; -use crate::ExecServerClient; use crate::ExecServerError; use crate::ExecutorFileSystem; use crate::FileMetadata; @@ -15,6 +14,7 @@ use crate::FileSystemResult; use crate::FileSystemSandboxContext; use crate::ReadDirectoryEntry; use crate::RemoveOptions; +use crate::environment::LazyRemoteExecServerClient; use crate::protocol::FsCopyParams; use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsGetMetadataParams; @@ -28,14 +28,18 @@ const NOT_FOUND_ERROR_CODE: i64 = -32004; #[derive(Clone)] pub(crate) struct RemoteFileSystem { - client: ExecServerClient, + client: LazyRemoteExecServerClient, } impl RemoteFileSystem { - pub(crate) fn new(client: ExecServerClient) -> Self { + pub(crate) fn new(client: LazyRemoteExecServerClient) -> Self { trace!("remote fs new"); Self { client } } + + async fn client(&self) -> FileSystemResult { + self.client.get().await.map_err(map_remote_error) + } } #[async_trait] @@ -46,8 +50,8 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { trace!("remote fs read_file"); - let response = self - .client + let client = self.client().await?; + let response = client .fs_read_file(FsReadFileParams { path: path.clone(), sandbox: sandbox.cloned(), @@ -69,7 +73,8 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs write_file"); - self.client + let client = self.client().await?; + client .fs_write_file(FsWriteFileParams { path: path.clone(), data_base64: STANDARD.encode(contents), @@ -87,7 +92,8 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs create_directory"); - self.client + let client = self.client().await?; + client .fs_create_directory(FsCreateDirectoryParams { path: path.clone(), recursive: Some(options.recursive), @@ -104,8 +110,8 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { trace!("remote fs get_metadata"); - let response = self - .client + let client = self.client().await?; + let response = client .fs_get_metadata(FsGetMetadataParams { path: path.clone(), sandbox: sandbox.cloned(), @@ -127,8 +133,8 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { trace!("remote fs read_directory"); - let response = self - .client + let client = self.client().await?; + let response = client .fs_read_directory(FsReadDirectoryParams { path: path.clone(), sandbox: sandbox.cloned(), @@ -153,7 +159,8 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs remove"); - self.client + let client = self.client().await?; + client .fs_remove(FsRemoveParams { path: path.clone(), recursive: Some(options.recursive), @@ -173,7 +180,8 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs copy"); - self.client + let client = self.client().await?; + client .fs_copy(FsCopyParams { source_path: source_path.clone(), destination_path: destination_path.clone(), diff --git a/codex-rs/exec-server/src/remote_process.rs b/codex-rs/exec-server/src/remote_process.rs index 86786a54f743..19828d9d691d 100644 --- a/codex-rs/exec-server/src/remote_process.rs +++ b/codex-rs/exec-server/src/remote_process.rs @@ -9,15 +9,15 @@ use crate::ExecProcess; use crate::ExecProcessEventReceiver; use crate::ExecServerError; use crate::StartedExecProcess; -use crate::client::ExecServerClient; use crate::client::Session; +use crate::environment::LazyRemoteExecServerClient; use crate::protocol::ExecParams; use crate::protocol::ReadResponse; use crate::protocol::WriteResponse; #[derive(Clone)] pub(crate) struct RemoteProcess { - client: ExecServerClient, + client: LazyRemoteExecServerClient, } struct RemoteExecProcess { @@ -25,7 +25,7 @@ struct RemoteExecProcess { } impl RemoteProcess { - pub(crate) fn new(client: ExecServerClient) -> Self { + pub(crate) fn new(client: LazyRemoteExecServerClient) -> Self { trace!("remote process new"); Self { client } } @@ -35,8 +35,9 @@ impl RemoteProcess { impl ExecBackend for RemoteProcess { async fn start(&self, params: ExecParams) -> Result { let process_id = params.process_id.clone(); - let session = self.client.register_session(&process_id).await?; - if let Err(err) = self.client.exec(params).await { + let client = self.client.get().await?; + let session = client.register_session(&process_id).await?; + if let Err(err) = client.exec(params).await { session.unregister().await; return Err(err); } diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index d449315c8d6e..4887a0be4de1 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -49,13 +49,13 @@ enum ProcessEventSnapshot { async fn create_process_context(use_remote: bool) -> Result { if use_remote { let server = exec_server().await?; - let environment = Environment::create(Some(server.websocket_url().to_string())).await?; + let environment = Environment::create(Some(server.websocket_url().to_string()))?; Ok(ProcessContext { backend: environment.get_exec_backend(), server: Some(server), }) } else { - let environment = Environment::create(/*exec_server_url*/ None).await?; + let environment = Environment::create(/*exec_server_url*/ None)?; Ok(ProcessContext { backend: environment.get_exec_backend(), server: None, diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index d4f94c7e44c1..4bb654198f91 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -46,7 +46,7 @@ struct FileSystemContext { async fn create_file_system_context(use_remote: bool) -> Result { if use_remote { let server = exec_server().await?; - let environment = Environment::create(Some(server.websocket_url().to_string())).await?; + let environment = Environment::create(Some(server.websocket_url().to_string()))?; Ok(FileSystemContext { file_system: environment.get_filesystem(), _helper_paths: None, @@ -214,7 +214,7 @@ async fn sandboxed_file_system_helper_finds_bwrap_on_preserved_path() -> Result< let helper_path = std::env::join_paths(path_entries)?; let server = exec_server_with_env([("PATH", helper_path.as_os_str())]).await?; - let environment = Environment::create(Some(server.websocket_url().to_string())).await?; + let environment = Environment::create(Some(server.websocket_url().to_string()))?; let file_system = environment.get_filesystem(); let workspace = tmp.path().join("workspace"); std::fs::create_dir_all(&workspace)?; diff --git a/codex-rs/tui/src/app/test_support.rs b/codex-rs/tui/src/app/test_support.rs index 8b2dfe8d47f8..254fc4cfa7cb 100644 --- a/codex-rs/tui/src/app/test_support.rs +++ b/codex-rs/tui/src/app/test_support.rs @@ -38,7 +38,9 @@ pub(super) async fn make_test_app() -> App { backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, - environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), remote_app_server_url: None, remote_app_server_auth_token: None, pending_update_action: None, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 1749ee9f1eb1..6f4ff6d1aa4e 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -3576,7 +3576,9 @@ async fn make_test_app() -> App { backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, - environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), remote_app_server_url: None, remote_app_server_auth_token: None, pending_update_action: None, @@ -3633,7 +3635,9 @@ async fn make_test_app_with_channels() -> ( backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, - environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), remote_app_server_url: None, remote_app_server_auth_token: None, pending_update_action: None, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 7c60b2e38a28..2fb6ace80a45 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -425,7 +425,9 @@ pub(crate) async fn start_embedded_app_server_for_picker( start_app_server_for_picker( config, &AppServerTarget::Embedded, - Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), ) .await } @@ -623,7 +625,9 @@ fn config_cwd_for_app_server_target( app_server_target: &AppServerTarget, environment_manager: &EnvironmentManager, ) -> std::io::Result> { - if environment_manager.is_remote() + if environment_manager + .default_environment() + .is_some_and(|environment| environment.is_remote()) || matches!(app_server_target, AppServerTarget::Remote { .. }) { return Ok(None); @@ -1771,7 +1775,9 @@ mod tests { CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), /*log_db*/ None, - Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), ) .await } @@ -1930,7 +1936,9 @@ mod tests { websocket_url: "ws://127.0.0.1:1234/".to_string(), auth_token: None, }; - let environment_manager = EnvironmentManager::new(/*exec_server_url*/ None); + let environment_manager = EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + ); let config_cwd = config_cwd_for_app_server_target(Some(remote_only_cwd), &target, &environment_manager)?; @@ -1943,7 +1951,9 @@ mod tests { fn config_cwd_for_app_server_target_canonicalizes_embedded_cli_cwd() -> std::io::Result<()> { let temp_dir = TempDir::new()?; let target = AppServerTarget::Embedded; - let environment_manager = EnvironmentManager::new(/*exec_server_url*/ None); + let environment_manager = EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + ); let config_cwd = config_cwd_for_app_server_target(Some(temp_dir.path()), &target, &environment_manager)?; @@ -1963,7 +1973,9 @@ mod tests { let temp_dir = TempDir::new()?; let missing = temp_dir.path().join("missing"); let target = AppServerTarget::Embedded; - let environment_manager = EnvironmentManager::new(/*exec_server_url*/ None); + let environment_manager = EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + ); let err = config_cwd_for_app_server_target(Some(&missing), &target, &environment_manager) .expect_err("missing embedded cwd should fail"); @@ -1980,7 +1992,11 @@ mod tests { Path::new("/definitely/not/local/to/this/test") }; let target = AppServerTarget::Embedded; - let environment_manager = EnvironmentManager::new(Some("ws://127.0.0.1:8765".to_string())); + let environment_manager = + EnvironmentManager::from_exec_server_url(codex_exec_server::EnvironmentManagerArgs { + exec_server_url: Some("ws://127.0.0.1:8765".to_string()), + local_runtime_paths: None, + }); let config_cwd = config_cwd_for_app_server_target(Some(remote_only_cwd), &target, &environment_manager)?; @@ -2107,7 +2123,9 @@ mod tests { CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), /*log_db*/ None, - Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + Arc::new(EnvironmentManager::from_exec_server_url( + codex_exec_server::EnvironmentManagerArgs::default(), + )), |_args| async { Err(std::io::Error::other("boom")) }, ) .await; diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index aee909cea1fc..ee6f843c8cb2 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -989,9 +989,11 @@ mod tests { ), feedback: codex_feedback::CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(codex_app_server_client::EnvironmentManager::new( - /*exec_server_url*/ None, - )), + environment_manager: Arc::new( + codex_app_server_client::EnvironmentManager::from_exec_server_url( + codex_app_server_client::EnvironmentManagerArgs::default(), + ), + ), config_warnings: Vec::new(), session_source: SessionSource::Cli, enable_codex_api_key_env: false, From b39ac6891d190599294808335822a7d3f72b91e7 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 15:06:09 -0700 Subject: [PATCH 02/20] Rename environment manager args constructor Use EnvironmentManager::new for args-struct construction and keep from_env methods as the env-var factory entrypoints. Co-authored-by: Codex --- codex-rs/app-server-client/src/lib.rs | 12 ++++------ .../app-server/src/bespoke_event_handling.rs | 2 +- codex-rs/app-server/src/in_process.rs | 2 +- .../src/message_processor/tracing_tests.rs | 2 +- .../app-server/tests/suite/v2/mcp_resource.rs | 2 +- codex-rs/core/src/agent/control_tests.rs | 14 +++++------ codex-rs/core/src/memories/tests.rs | 2 +- .../core/src/session/tests/guardian_tests.rs | 2 +- codex-rs/core/src/thread_manager.rs | 2 +- codex-rs/core/src/thread_manager_tests.rs | 10 ++++---- codex-rs/core/tests/common/test_codex.rs | 13 +++++----- codex-rs/core/tests/suite/client.rs | 2 +- codex-rs/core/tests/suite/skills.rs | 2 +- codex-rs/exec-server/src/environment.rs | 24 +++++++++---------- codex-rs/tui/src/app/test_support.rs | 2 +- codex-rs/tui/src/app/tests.rs | 4 ++-- codex-rs/tui/src/lib.rs | 23 ++++++++---------- codex-rs/tui/src/onboarding/auth.rs | 8 +++---- 18 files changed, 60 insertions(+), 68 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 31184063e921..49ed5256f08e 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -969,7 +969,7 @@ mod tests { cloud_requirements: CloudRequirementsLoader::default(), feedback: CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + environment_manager: Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), config_warnings: Vec::new(), @@ -1972,12 +1972,10 @@ mod tests { #[tokio::test] async fn runtime_start_args_forward_environment_manager() { let config = Arc::new(build_test_config().await); - let environment_manager = Arc::new(EnvironmentManager::from_exec_server_url( - EnvironmentManagerArgs { - exec_server_url: Some("ws://127.0.0.1:8765".to_string()), - local_runtime_paths: None, - }, - )); + let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { + exec_server_url: Some("ws://127.0.0.1:8765".to_string()), + local_runtime_paths: None, + })); let runtime_args = InProcessClientStartArgs { arg0_paths: Arg0DispatchPaths::default(), diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 43a75df6780b..806652a7d6e5 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -3497,7 +3497,7 @@ mod tests { CodexAuth::create_dummy_chatgpt_auth_for_testing(), config.model_provider.clone(), config.codex_home.to_path_buf(), - Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ), diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index c586d3af103f..1cd6e16ca054 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -750,7 +750,7 @@ mod tests { thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + environment_manager: Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), config_warnings: Vec::new(), diff --git a/codex-rs/app-server/src/message_processor/tracing_tests.rs b/codex-rs/app-server/src/message_processor/tracing_tests.rs index cb697ae3883e..f615aa429bbe 100644 --- a/codex-rs/app-server/src/message_processor/tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor/tracing_tests.rs @@ -279,7 +279,7 @@ fn build_test_processor( arg0_paths: Arg0DispatchPaths::default(), config, config_manager, - environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + environment_manager: Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), feedback: CodexFeedback::new(), diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs index 34b7a55d6aa4..4bc412d54e8d 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -204,7 +204,7 @@ async fn mcp_resource_read_returns_error_for_unknown_thread() -> Result<()> { thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + environment_manager: Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), config_warnings: Vec::new(), diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 473180caa3e0..15eb23d0cdbb 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -95,7 +95,7 @@ impl AgentControlHarness { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ); @@ -911,7 +911,7 @@ async fn spawn_agent_respects_max_threads_limit() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ); @@ -965,7 +965,7 @@ async fn spawn_agent_releases_slot_after_shutdown() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ); @@ -1010,7 +1010,7 @@ async fn spawn_agent_limit_shared_across_clones() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ); @@ -1057,7 +1057,7 @@ async fn resume_agent_respects_max_threads_limit() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ); @@ -1115,7 +1115,7 @@ async fn resume_agent_releases_slot_after_resume_failure() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ); @@ -1512,7 +1512,7 @@ async fn resume_thread_subagent_restores_stored_nickname_and_role() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ); diff --git a/codex-rs/core/src/memories/tests.rs b/codex-rs/core/src/memories/tests.rs index 2b68596fd424..1048b3da869a 100644 --- a/codex-rs/core/src/memories/tests.rs +++ b/codex-rs/core/src/memories/tests.rs @@ -491,7 +491,7 @@ mod phase2 { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ); diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index e59faee713d7..7a81c171fcd6 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -635,7 +635,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { config, auth_manager, models_manager, - environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + environment_manager: Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), skills_manager, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 293423defe91..c5da648bf503 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -301,7 +301,7 @@ impl ThreadManager { auth, provider, codex_home.clone(), - Arc::new(EnvironmentManager::from_exec_server_url( + Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ); diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 8c0fbe5e3805..76dd62bb9a56 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -246,7 +246,7 @@ async fn shutdown_all_threads_bounded_submits_shutdown_to_every_thread() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ); @@ -297,7 +297,7 @@ async fn new_uses_configured_openai_provider_for_model_refresh() { auth_manager, SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), /*analytics_events_client*/ None, @@ -434,7 +434,7 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor auth_manager.clone(), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), /*analytics_events_client*/ None, @@ -537,7 +537,7 @@ async fn interrupted_fork_snapshot_preserves_explicit_turn_id() { auth_manager.clone(), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), /*analytics_events_client*/ None, @@ -630,7 +630,7 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ auth_manager.clone(), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), /*analytics_events_client*/ None, diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 32f5657b3498..0993b5b0e42f 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -350,13 +350,12 @@ impl TestCodexBuilder { let (config, fallback_cwd) = self .prepare_config(base_url, &home, test_env.cwd().clone()) .await?; - let environment_manager = - Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( - codex_exec_server::EnvironmentManagerArgs { - exec_server_url: test_env.exec_server_url().map(str::to_owned), - local_runtime_paths: None, - }, - )); + let environment_manager = Arc::new(codex_exec_server::EnvironmentManager::new( + codex_exec_server::EnvironmentManagerArgs { + exec_server_url: test_env.exec_server_url().map(str::to_owned), + local_runtime_paths: None, + }, + )); let file_system = test_env.environment().get_filesystem(); let mut workspace_setups = vec![]; swap(&mut self.workspace_setups, &mut workspace_setups); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 3bebd2192c70..3f0a7762ba23 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1103,7 +1103,7 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() { .features .enabled(Feature::DefaultModeRequestUserInput), }, - Arc::new(codex_exec_server::EnvironmentManager::from_exec_server_url( + Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), /*analytics_events_client*/ None, diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 4ff6da00d131..090f7a57903c 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -234,7 +234,7 @@ async fn list_skills_skips_cwd_roots_when_environment_disabled() -> Result<()> { codex_core::test_support::auth_manager_from_auth(CodexAuth::from_api_key("dummy")), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(EnvironmentManager::from_exec_server_url( + Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), local_runtime_paths: None, diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index d43d2ba2c45e..4cfcd6f9c009 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -70,7 +70,7 @@ impl LazyRemoteExecServerClient { impl Default for EnvironmentManager { fn default() -> Self { - Self::from_exec_server_url(EnvironmentManagerArgs::default()) + Self::new(EnvironmentManagerArgs::default()) } } @@ -85,7 +85,7 @@ impl EnvironmentManager { pub fn from_env_with_runtime_paths( local_runtime_paths: Option, ) -> Self { - Self::from_exec_server_url(EnvironmentManagerArgs { + Self::new(EnvironmentManagerArgs { exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), local_runtime_paths, }) @@ -93,7 +93,7 @@ impl EnvironmentManager { /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value and local /// runtime paths used when creating local filesystem helpers. - pub fn from_exec_server_url(args: EnvironmentManagerArgs) -> Self { + pub fn new(args: EnvironmentManagerArgs) -> Self { let EnvironmentManagerArgs { exec_server_url, local_runtime_paths, @@ -292,7 +292,7 @@ mod tests { #[test] fn environment_manager_normalizes_empty_url() { - let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some(String::new()), local_runtime_paths: None, }); @@ -306,7 +306,7 @@ mod tests { #[test] fn environment_manager_treats_none_value_as_disabled() { - let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), local_runtime_paths: None, }); @@ -319,7 +319,7 @@ mod tests { #[test] fn environment_manager_reports_remote_url() { - let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("ws://127.0.0.1:8765".to_string()), local_runtime_paths: None, }); @@ -345,7 +345,7 @@ mod tests { #[tokio::test] async fn environment_manager_default_environment_caches_environment() { - let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs::default()); + let manager = EnvironmentManager::new(EnvironmentManagerArgs::default()); let first = manager.default_environment().expect("local environment"); let second = manager.default_environment().expect("local environment"); @@ -360,7 +360,7 @@ mod tests { /*codex_linux_sandbox_exe*/ None, ) .expect("runtime paths"); - let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: None, local_runtime_paths: Some(runtime_paths.clone()), }); @@ -368,7 +368,7 @@ mod tests { let environment = manager.default_environment().expect("local environment"); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); - let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: environment.exec_server_url().map(str::to_owned), local_runtime_paths: environment.local_runtime_paths().cloned(), }); @@ -378,7 +378,7 @@ mod tests { #[tokio::test] async fn disabled_environment_manager_has_default_environment_but_no_tool_environment() { - let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), local_runtime_paths: None, }); @@ -389,7 +389,7 @@ mod tests { #[tokio::test] async fn environment_manager_allows_local_lookup_when_disabled() { - let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs { + let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), local_runtime_paths: None, }); @@ -402,7 +402,7 @@ mod tests { #[tokio::test] async fn get_environment_returns_none_for_unknown_id() { - let manager = EnvironmentManager::from_exec_server_url(EnvironmentManagerArgs::default()); + let manager = EnvironmentManager::new(EnvironmentManagerArgs::default()); assert!(manager.get_environment("does-not-exist").is_none()); } diff --git a/codex-rs/tui/src/app/test_support.rs b/codex-rs/tui/src/app/test_support.rs index 254fc4cfa7cb..8b2c22512ef8 100644 --- a/codex-rs/tui/src/app/test_support.rs +++ b/codex-rs/tui/src/app/test_support.rs @@ -38,7 +38,7 @@ pub(super) async fn make_test_app() -> App { backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, - environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + environment_manager: Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), remote_app_server_url: None, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 6f4ff6d1aa4e..56d093598fac 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -3576,7 +3576,7 @@ async fn make_test_app() -> App { backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, - environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + environment_manager: Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), remote_app_server_url: None, @@ -3635,7 +3635,7 @@ async fn make_test_app_with_channels() -> ( backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, - environment_manager: Arc::new(EnvironmentManager::from_exec_server_url( + environment_manager: Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), remote_app_server_url: None, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 2fb6ace80a45..bbc6df14bb14 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -425,7 +425,7 @@ pub(crate) async fn start_embedded_app_server_for_picker( start_app_server_for_picker( config, &AppServerTarget::Embedded, - Arc::new(EnvironmentManager::from_exec_server_url( + Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ) @@ -1775,7 +1775,7 @@ mod tests { CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), /*log_db*/ None, - Arc::new(EnvironmentManager::from_exec_server_url( + Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), ) @@ -1936,9 +1936,8 @@ mod tests { websocket_url: "ws://127.0.0.1:1234/".to_string(), auth_token: None, }; - let environment_manager = EnvironmentManager::from_exec_server_url( - codex_exec_server::EnvironmentManagerArgs::default(), - ); + let environment_manager = + EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default()); let config_cwd = config_cwd_for_app_server_target(Some(remote_only_cwd), &target, &environment_manager)?; @@ -1951,9 +1950,8 @@ mod tests { fn config_cwd_for_app_server_target_canonicalizes_embedded_cli_cwd() -> std::io::Result<()> { let temp_dir = TempDir::new()?; let target = AppServerTarget::Embedded; - let environment_manager = EnvironmentManager::from_exec_server_url( - codex_exec_server::EnvironmentManagerArgs::default(), - ); + let environment_manager = + EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default()); let config_cwd = config_cwd_for_app_server_target(Some(temp_dir.path()), &target, &environment_manager)?; @@ -1973,9 +1971,8 @@ mod tests { let temp_dir = TempDir::new()?; let missing = temp_dir.path().join("missing"); let target = AppServerTarget::Embedded; - let environment_manager = EnvironmentManager::from_exec_server_url( - codex_exec_server::EnvironmentManagerArgs::default(), - ); + let environment_manager = + EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default()); let err = config_cwd_for_app_server_target(Some(&missing), &target, &environment_manager) .expect_err("missing embedded cwd should fail"); @@ -1993,7 +1990,7 @@ mod tests { }; let target = AppServerTarget::Embedded; let environment_manager = - EnvironmentManager::from_exec_server_url(codex_exec_server::EnvironmentManagerArgs { + EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs { exec_server_url: Some("ws://127.0.0.1:8765".to_string()), local_runtime_paths: None, }); @@ -2123,7 +2120,7 @@ mod tests { CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), /*log_db*/ None, - Arc::new(EnvironmentManager::from_exec_server_url( + Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs::default(), )), |_args| async { Err(std::io::Error::other("boom")) }, diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index ee6f843c8cb2..2379c2d55dbb 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -989,11 +989,9 @@ mod tests { ), feedback: codex_feedback::CodexFeedback::new(), log_db: None, - environment_manager: Arc::new( - codex_app_server_client::EnvironmentManager::from_exec_server_url( - codex_app_server_client::EnvironmentManagerArgs::default(), - ), - ), + environment_manager: Arc::new(codex_app_server_client::EnvironmentManager::new( + codex_app_server_client::EnvironmentManagerArgs::default(), + )), config_warnings: Vec::new(), session_source: SessionSource::Cli, enable_codex_api_key_env: false, From 4b927b9a7303f6907a5146b1ef6dc033f783d907 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 15:09:41 -0700 Subject: [PATCH 03/20] Make default environment lookup infallible Return concrete default and local environments from EnvironmentManager now that the manager always installs local and default entries. Keep arbitrary ID lookup optional. Co-authored-by: Codex --- codex-rs/app-server-client/src/lib.rs | 2 +- .../app-server/src/codex_message_processor.rs | 25 ++++----- codex-rs/core/src/session/mod.rs | 6 +-- codex-rs/core/src/session/session.rs | 2 +- codex-rs/core/src/thread_manager.rs | 6 +-- codex-rs/exec-server/src/environment.rs | 52 ++++++++----------- codex-rs/tui/src/lib.rs | 4 +- 7 files changed, 41 insertions(+), 56 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 49ed5256f08e..b767a3c50d5f 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -2006,7 +2006,7 @@ mod tests { runtime_args .environment_manager .default_environment() - .is_some_and(|environment| environment.is_remote()) + .is_remote() ); } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 15c0782d660b..f51722b3abf9 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -5677,17 +5677,14 @@ impl CodexMessageProcessor { .to_mcp_config(self.thread_manager.plugins_manager().as_ref()) .await; let auth = self.auth_manager.auth().await; - 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(), - ), + let runtime_environment = { + let environment = self + .thread_manager + .environment_manager() + .default_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()) }; tokio::spawn(async move { @@ -5850,8 +5847,7 @@ impl CodexMessageProcessor { let environment = self .thread_manager .environment_manager() - .default_environment() - .unwrap_or_else(|| Arc::new(codex_exec_server::Environment::default())); + .default_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()) @@ -6445,7 +6441,8 @@ impl CodexMessageProcessor { .thread_manager .environment_manager() .default_environment() - .map(|environment| environment.get_filesystem()); + .get_filesystem(); + let fs = Some(fs); let mut data = Vec::new(); for cwd in cwds { let extra_roots = extra_roots_by_cwd diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 1aa15a2cad8c..7b9e205eedfa 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -459,9 +459,7 @@ impl Codex { let (tx_event, rx_event) = async_channel::unbounded(); let environment = environment_manager.default_environment(); - let fs = environment - .as_ref() - .map(|environment| environment.get_filesystem()); + let fs = Some(environment.get_filesystem()); let plugin_outcome = plugins_manager.plugins_for_config(&config).await; let effective_skill_roots = plugin_outcome.effective_skill_roots(); let skills_input = skills_load_input_from_config(&config, effective_skill_roots); @@ -511,7 +509,7 @@ impl Codex { } let user_instructions = AgentsMdManager::new(&config) - .user_instructions(environment.as_deref()) + .user_instructions(Some(environment.as_ref())) .await; let exec_policy = if crate::guardian::is_guardian_reviewer_source(&session_source) { diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 232bfaf09add..2459b0926dd5 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -679,7 +679,7 @@ impl Session { config.js_repl_node_path.clone(), ), environment_manager, - environment, + environment: Some(environment), allows_agent_environment_access, }; services diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index c5da648bf503..da109bbcc7be 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -923,8 +923,8 @@ impl ThreadManagerState { user_shell_override: Option, ) -> CodexResult { let environment = self.environment_manager.default_environment(); - let watch_registration = match environment.as_ref() { - Some(environment) if !environment.is_remote() => { + let watch_registration = match environment.is_remote() { + false => { self.skills_watcher .register_config( &config, @@ -934,7 +934,7 @@ impl ThreadManagerState { ) .await } - Some(_) | None => crate::file_watcher::WatchRegistration::default(), + true => crate::file_watcher::WatchRegistration::default(), }; let CodexSpawnOk { codex, thread_id, .. diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 4cfcd6f9c009..634b2e7c79ef 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -23,7 +23,7 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; /// separately tracking whether model-facing tools may access environments. #[derive(Debug)] pub struct EnvironmentManager { - default_environment: Option, + default_environment: String, environment_disabled_for_agent: bool, environments: HashMap>, } @@ -124,9 +124,9 @@ impl EnvironmentManager { .expect("valid remote environment"), ), ); - Some(REMOTE_ENVIRONMENT_ID.to_string()) + REMOTE_ENVIRONMENT_ID.to_string() } - None => Some(LOCAL_ENVIRONMENT_ID.to_string()), + None => LOCAL_ENVIRONMENT_ID.to_string(), }; Self { @@ -139,22 +139,19 @@ impl EnvironmentManager { /// Returns true when model-facing tools may access an environment. pub fn allows_agent_environment_access(&self) -> bool { !self.environment_disabled_for_agent - && self - .default_environment - .as_deref() - .is_some_and(|environment_id| self.environments.contains_key(environment_id)) + && self.environments.contains_key(&self.default_environment) } /// Returns the default environment instance. - pub fn default_environment(&self) -> Option> { - self.default_environment - .as_deref() - .and_then(|environment_id| self.get_environment(environment_id)) + pub fn default_environment(&self) -> Arc { + self.get_environment(&self.default_environment) + .expect("default environment exists") } /// Returns the local environment instance. - pub fn local_environment(&self) -> Option> { + pub fn local_environment(&self) -> Arc { self.get_environment(LOCAL_ENVIRONMENT_ID) + .expect("local environment exists") } /// Returns a named environment instance. @@ -297,10 +294,10 @@ mod tests { local_runtime_paths: None, }); - let environment = manager.default_environment().expect("local environment"); + let environment = manager.default_environment(); assert!(!environment.is_remote()); assert!(manager.allows_agent_environment_access()); - assert!(manager.local_environment().is_some()); + assert!(!manager.local_environment().is_remote()); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } @@ -312,8 +309,8 @@ mod tests { }); assert!(!manager.allows_agent_environment_access()); - assert!(manager.default_environment().is_some()); - assert!(manager.local_environment().is_some()); + assert!(!manager.default_environment().is_remote()); + assert!(!manager.local_environment().is_remote()); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } @@ -324,16 +321,11 @@ mod tests { local_runtime_paths: None, }); - let environment = manager.default_environment().expect("remote environment"); + let environment = manager.default_environment(); assert!(environment.is_remote()); assert!(manager.allows_agent_environment_access()); assert_eq!(environment.exec_server_url(), Some("ws://127.0.0.1:8765")); - assert!( - !manager - .local_environment() - .expect("local environment") - .is_remote() - ); + assert!(!manager.local_environment().is_remote()); assert_eq!( manager .get_environment(REMOTE_ENVIRONMENT_ID) @@ -347,8 +339,8 @@ mod tests { async fn environment_manager_default_environment_caches_environment() { let manager = EnvironmentManager::new(EnvironmentManagerArgs::default()); - let first = manager.default_environment().expect("local environment"); - let second = manager.default_environment().expect("local environment"); + let first = manager.default_environment(); + let second = manager.default_environment(); assert!(Arc::ptr_eq(&first, &second)); } @@ -365,14 +357,14 @@ mod tests { local_runtime_paths: Some(runtime_paths.clone()), }); - let environment = manager.default_environment().expect("local environment"); + let environment = manager.default_environment(); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: environment.exec_server_url().map(str::to_owned), local_runtime_paths: environment.local_runtime_paths().cloned(), }); - let environment = manager.default_environment().expect("local environment"); + let environment = manager.default_environment(); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); } @@ -383,7 +375,7 @@ mod tests { local_runtime_paths: None, }); - assert!(manager.default_environment().is_some()); + assert!(!manager.default_environment().is_remote()); assert!(!manager.allows_agent_environment_access()); } @@ -394,9 +386,9 @@ mod tests { local_runtime_paths: None, }); - assert!(manager.default_environment().is_some()); + assert!(!manager.default_environment().is_remote()); assert!(!manager.allows_agent_environment_access()); - assert!(manager.local_environment().is_some()); + assert!(!manager.local_environment().is_remote()); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index bbc6df14bb14..10ba55de8037 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -625,9 +625,7 @@ fn config_cwd_for_app_server_target( app_server_target: &AppServerTarget, environment_manager: &EnvironmentManager, ) -> std::io::Result> { - if environment_manager - .default_environment() - .is_some_and(|environment| environment.is_remote()) + if environment_manager.default_environment().is_remote() || matches!(app_server_target, AppServerTarget::Remote { .. }) { return Ok(None); From a3bc33f8014d1d703c94787b8e64bed80e0e08c4 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 15:10:30 -0700 Subject: [PATCH 04/20] Move lazy exec-server client handle Keep the lazy remote exec-server client wrapper alongside ExecServerClient and import it from the client module for environment-backed exec and filesystem use. Co-authored-by: Codex --- codex-rs/exec-server/src/client.rs | 32 ++++++++++++++++ codex-rs/exec-server/src/environment.rs | 37 +------------------ .../exec-server/src/remote_file_system.rs | 2 +- codex-rs/exec-server/src/remote_process.rs | 2 +- 4 files changed, 35 insertions(+), 38 deletions(-) diff --git a/codex-rs/exec-server/src/client.rs b/codex-rs/exec-server/src/client.rs index 4e282c8fd3fb..c4526e13bd4f 100644 --- a/codex-rs/exec-server/src/client.rs +++ b/codex-rs/exec-server/src/client.rs @@ -10,6 +10,7 @@ use arc_swap::ArcSwap; use codex_app_server_protocol::JSONRPCNotification; use serde_json::Value; use tokio::sync::Mutex; +use tokio::sync::OnceCell; use tokio::sync::mpsc; use tokio::sync::watch; @@ -174,6 +175,37 @@ pub struct ExecServerClient { inner: Arc, } +#[derive(Clone, Debug)] +pub(crate) struct LazyRemoteExecServerClient { + websocket_url: String, + client: Arc>, +} + +impl LazyRemoteExecServerClient { + pub(crate) fn new(websocket_url: String) -> Self { + Self { + websocket_url, + client: Arc::new(OnceCell::new()), + } + } + + pub(crate) async fn get(&self) -> Result { + self.client + .get_or_try_init(|| async { + ExecServerClient::connect_websocket(RemoteExecServerConnectArgs { + websocket_url: self.websocket_url.clone(), + client_name: "codex-environment".to_string(), + connect_timeout: Duration::from_secs(5), + initialize_timeout: Duration::from_secs(5), + resume_session_id: None, + }) + .await + }) + .await + .cloned() + } +} + #[derive(Debug, thiserror::Error)] pub enum ExecServerError { #[error("failed to spawn exec-server: {0}")] diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 634b2e7c79ef..2452144001ba 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -1,13 +1,9 @@ use std::collections::HashMap; use std::sync::Arc; -use std::time::Duration; -use tokio::sync::OnceCell; - -use crate::ExecServerClient; use crate::ExecServerError; use crate::ExecServerRuntimePaths; -use crate::RemoteExecServerConnectArgs; +use crate::client::LazyRemoteExecServerClient; use crate::file_system::ExecutorFileSystem; use crate::local_file_system::LocalFileSystem; use crate::local_process::LocalProcess; @@ -37,37 +33,6 @@ pub struct EnvironmentManagerArgs { pub local_runtime_paths: Option, } -#[derive(Clone, Debug)] -pub(crate) struct LazyRemoteExecServerClient { - websocket_url: String, - client: Arc>, -} - -impl LazyRemoteExecServerClient { - fn new(websocket_url: String) -> Self { - Self { - websocket_url, - client: Arc::new(OnceCell::new()), - } - } - - pub(crate) async fn get(&self) -> Result { - self.client - .get_or_try_init(|| async { - ExecServerClient::connect_websocket(RemoteExecServerConnectArgs { - websocket_url: self.websocket_url.clone(), - client_name: "codex-environment".to_string(), - connect_timeout: Duration::from_secs(5), - initialize_timeout: Duration::from_secs(5), - resume_session_id: None, - }) - .await - }) - .await - .cloned() - } -} - impl Default for EnvironmentManager { fn default() -> Self { Self::new(EnvironmentManagerArgs::default()) diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index 1b4150fe5ef6..02a5e2883686 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -14,7 +14,7 @@ use crate::FileSystemResult; use crate::FileSystemSandboxContext; use crate::ReadDirectoryEntry; use crate::RemoveOptions; -use crate::environment::LazyRemoteExecServerClient; +use crate::client::LazyRemoteExecServerClient; use crate::protocol::FsCopyParams; use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsGetMetadataParams; diff --git a/codex-rs/exec-server/src/remote_process.rs b/codex-rs/exec-server/src/remote_process.rs index 19828d9d691d..d8d06735cdb9 100644 --- a/codex-rs/exec-server/src/remote_process.rs +++ b/codex-rs/exec-server/src/remote_process.rs @@ -9,8 +9,8 @@ use crate::ExecProcess; use crate::ExecProcessEventReceiver; use crate::ExecServerError; use crate::StartedExecProcess; +use crate::client::LazyRemoteExecServerClient; use crate::client::Session; -use crate::environment::LazyRemoteExecServerClient; use crate::protocol::ExecParams; use crate::protocol::ReadResponse; use crate::protocol::WriteResponse; From a482fd89e1a651343e3d18c75faccc50dbfb7f5a Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 15:13:20 -0700 Subject: [PATCH 05/20] Remove path-specific environment factory Use EnvironmentManager::new with EnvironmentManagerArgs for runtime-path-aware construction and keep from_env only for the no-args env-var factory. Co-authored-by: Codex --- codex-rs/app-server-client/src/lib.rs | 1 + codex-rs/app-server/src/lib.rs | 11 +++++++---- codex-rs/exec-server/src/environment.rs | 10 +--------- codex-rs/exec/src/lib.rs | 9 ++++++--- codex-rs/mcp-server/src/lib.rs | 11 +++++++---- codex-rs/tui/src/lib.rs | 11 +++++++---- 6 files changed, 29 insertions(+), 24 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index b767a3c50d5f..726939d292fe 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -45,6 +45,7 @@ use codex_config::NoopThreadConfigLoader; use codex_core::config::Config; use codex_core::config_loader::CloudRequirementsLoader; use codex_core::config_loader::LoaderOverrides; +pub use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; pub use codex_exec_server::EnvironmentManager; pub use codex_exec_server::EnvironmentManagerArgs; pub use codex_exec_server::ExecServerRuntimePaths; diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index ca80ad2c99dc..a69015bff9df 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -6,6 +6,8 @@ use codex_config::ThreadConfigLoader; use codex_core::config::Config; use codex_core::config_loader::ConfigLayerStackOrdering; use codex_core::config_loader::LoaderOverrides; +use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; +use codex_exec_server::EnvironmentManagerArgs; use codex_features::Feature; use codex_login::AuthManager; use codex_utils_cli::CliConfigOverrides; @@ -360,12 +362,13 @@ pub async fn run_main_with_transport( session_source: SessionSource, auth: AppServerWebsocketAuthSettings, ) -> IoResult<()> { - let environment_manager = Arc::new(EnvironmentManager::from_env_with_runtime_paths(Some( - ExecServerRuntimePaths::from_optional_paths( + let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { + exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), + local_runtime_paths: Some(ExecServerRuntimePaths::from_optional_paths( arg0_paths.codex_self_exe.clone(), arg0_paths.codex_linux_sandbox_exe.clone(), - )?, - ))); + )?), + })); let (transport_event_tx, mut transport_event_rx) = mpsc::channel::(CHANNEL_CAPACITY); let (outgoing_tx, mut outgoing_rx) = mpsc::channel::(CHANNEL_CAPACITY); diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 2452144001ba..e77fb564c616 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -42,17 +42,9 @@ impl Default for EnvironmentManager { impl EnvironmentManager { /// Builds a manager from process environment variables. pub fn from_env() -> Self { - Self::from_env_with_runtime_paths(/*local_runtime_paths*/ None) - } - - /// Builds a manager from process environment variables and local runtime - /// paths used when creating local filesystem helpers. - pub fn from_env_with_runtime_paths( - local_runtime_paths: Option, - ) -> Self { Self::new(EnvironmentManagerArgs { exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), - local_runtime_paths, + local_runtime_paths: None, }) } diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 781e423fde45..4cb80a1543e6 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -13,8 +13,10 @@ pub(crate) mod exec_events; pub use cli::Cli; pub use cli::Command; pub use cli::ReviewArgs; +use codex_app_server_client::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_app_server_client::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY; use codex_app_server_client::EnvironmentManager; +use codex_app_server_client::EnvironmentManagerArgs; use codex_app_server_client::ExecServerRuntimePaths; use codex_app_server_client::InProcessAppServerClient; use codex_app_server_client::InProcessClientStartArgs; @@ -497,9 +499,10 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result cloud_requirements: run_cloud_requirements, feedback: CodexFeedback::new(), log_db: None, - environment_manager: std::sync::Arc::new(EnvironmentManager::from_env_with_runtime_paths( - Some(local_runtime_paths), - )), + environment_manager: std::sync::Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { + exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), + local_runtime_paths: Some(local_runtime_paths), + })), config_warnings, session_source: SessionSource::Exec, enable_codex_api_key_env: true, diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index 1320fd1b67e2..2eb93130f135 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -7,7 +7,9 @@ use std::sync::Arc; use codex_arg0::Arg0DispatchPaths; use codex_core::config::Config; +use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_exec_server::EnvironmentManager; +use codex_exec_server::EnvironmentManagerArgs; use codex_exec_server::ExecServerRuntimePaths; use codex_login::default_client::set_default_client_residency_requirement; use codex_utils_cli::CliConfigOverrides; @@ -59,12 +61,13 @@ pub async fn run_main( arg0_paths: Arg0DispatchPaths, cli_config_overrides: CliConfigOverrides, ) -> IoResult<()> { - let environment_manager = Arc::new(EnvironmentManager::from_env_with_runtime_paths(Some( - ExecServerRuntimePaths::from_optional_paths( + let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { + exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), + local_runtime_paths: Some(ExecServerRuntimePaths::from_optional_paths( arg0_paths.codex_self_exe.clone(), arg0_paths.codex_linux_sandbox_exe.clone(), - )?, - ))); + )?), + })); // Parse CLI overrides once and derive the base Config eagerly so later // components do not need to work with raw TOML values. let cli_kv_overrides = cli_config_overrides.parse_overrides().map_err(|e| { diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 10ba55de8037..98369a0bbda8 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -35,7 +35,9 @@ use codex_config::CloudRequirementsLoader; use codex_config::ConfigLoadError; use codex_config::LoaderOverrides; use codex_config::format_config_error_with_source; +use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_exec_server::EnvironmentManager; +use codex_exec_server::EnvironmentManagerArgs; use codex_exec_server::ExecServerRuntimePaths; use codex_login::AuthConfig; use codex_login::default_client::set_default_client_residency_requirement; @@ -728,12 +730,13 @@ pub async fn run_main( } }; - let environment_manager = Arc::new(EnvironmentManager::from_env_with_runtime_paths(Some( - ExecServerRuntimePaths::from_optional_paths( + let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { + exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), + local_runtime_paths: Some(ExecServerRuntimePaths::from_optional_paths( arg0_paths.codex_self_exe.clone(), arg0_paths.codex_linux_sandbox_exe.clone(), - )?, - ))); + )?), + })); let cwd = cli.cwd.clone(); let config_cwd = config_cwd_for_app_server_target(cwd.as_deref(), &app_server_target, &environment_manager)?; From 8bce278d79678b16fda795fbdbb8dccddeff8b5c Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 15:15:03 -0700 Subject: [PATCH 06/20] Document environment manager behavior Add high-level EnvironmentManager docs for local/remote initialization, default environment selection, disabled agent access, and lazy remote connection behavior. Co-authored-by: Codex --- codex-rs/exec-server/src/environment.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index e77fb564c616..86fa6b650794 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -15,8 +15,21 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; /// Owns the execution/filesystem environments available to a session. /// -/// The manager keeps the session's default environment selection stable while -/// separately tracking whether model-facing tools may access environments. +/// `EnvironmentManager` is the session-scoped registry for concrete +/// environments. It always creates a local environment under [`LOCAL_ENVIRONMENT_ID`]. +/// When `CODEX_EXEC_SERVER_URL` is set to a websocket URL, it also creates a +/// remote environment under [`REMOTE_ENVIRONMENT_ID`] and makes that the default +/// environment. Otherwise the local environment is the default. +/// +/// Setting `CODEX_EXEC_SERVER_URL=none` does not remove the local environment: +/// Codex internals may still use `local_environment()` or `default_environment()`. +/// Instead it disables agent/tool access as reported by +/// `allows_agent_environment_access()`, so shell/filesystem tools can be hidden +/// from the model while internal local filesystem access still works. +/// +/// Remote environments hold a lazy exec-server client handle. The websocket is +/// not opened when the manager or environment is constructed; it connects on the +/// first remote exec or filesystem operation. #[derive(Debug)] pub struct EnvironmentManager { default_environment: String, From 372aef7a350a607aca9cfb60b57d3e40077cf77b Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 15:15:56 -0700 Subject: [PATCH 07/20] Remove local environment convenience method Drop the unused local_environment helper and keep local lookups on the generic get_environment API. Co-authored-by: Codex --- codex-rs/exec-server/src/environment.rs | 38 ++++++++++++++++++------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 86fa6b650794..53a737dc5708 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -22,7 +22,8 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; /// environment. Otherwise the local environment is the default. /// /// Setting `CODEX_EXEC_SERVER_URL=none` does not remove the local environment: -/// Codex internals may still use `local_environment()` or `default_environment()`. +/// Codex internals may still use `default_environment()` or explicit +/// `get_environment()` lookups. /// Instead it disables agent/tool access as reported by /// `allows_agent_environment_access()`, so shell/filesystem tools can be hidden /// from the model while internal local filesystem access still works. @@ -118,12 +119,6 @@ impl EnvironmentManager { .expect("default environment exists") } - /// Returns the local environment instance. - pub fn local_environment(&self) -> Arc { - self.get_environment(LOCAL_ENVIRONMENT_ID) - .expect("local environment exists") - } - /// Returns a named environment instance. pub fn get_environment(&self, environment_id: &str) -> Option> { self.environments.get(environment_id).cloned() @@ -243,6 +238,7 @@ mod tests { use super::Environment; use super::EnvironmentManager; use super::EnvironmentManagerArgs; + use super::LOCAL_ENVIRONMENT_ID; use super::REMOTE_ENVIRONMENT_ID; use crate::ExecServerRuntimePaths; use crate::ProcessId; @@ -267,7 +263,12 @@ mod tests { let environment = manager.default_environment(); assert!(!environment.is_remote()); assert!(manager.allows_agent_environment_access()); - assert!(!manager.local_environment().is_remote()); + assert!( + !manager + .get_environment(LOCAL_ENVIRONMENT_ID) + .expect("local environment") + .is_remote() + ); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } @@ -280,7 +281,12 @@ mod tests { assert!(!manager.allows_agent_environment_access()); assert!(!manager.default_environment().is_remote()); - assert!(!manager.local_environment().is_remote()); + assert!( + !manager + .get_environment(LOCAL_ENVIRONMENT_ID) + .expect("local environment") + .is_remote() + ); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } @@ -295,7 +301,12 @@ mod tests { assert!(environment.is_remote()); assert!(manager.allows_agent_environment_access()); assert_eq!(environment.exec_server_url(), Some("ws://127.0.0.1:8765")); - assert!(!manager.local_environment().is_remote()); + assert!( + !manager + .get_environment(LOCAL_ENVIRONMENT_ID) + .expect("local environment") + .is_remote() + ); assert_eq!( manager .get_environment(REMOTE_ENVIRONMENT_ID) @@ -358,7 +369,12 @@ mod tests { assert!(!manager.default_environment().is_remote()); assert!(!manager.allows_agent_environment_access()); - assert!(!manager.local_environment().is_remote()); + assert!( + !manager + .get_environment(LOCAL_ENVIRONMENT_ID) + .expect("local environment") + .is_remote() + ); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } From fa6cc40cdb0c986371782176d4ec637cb6cee0a1 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 15:27:17 -0700 Subject: [PATCH 08/20] Document shared environment manager handle Clarify that SessionServices carries an Arc handle to the process-level EnvironmentManager rather than owning a session-specific manager. Co-authored-by: Codex --- codex-rs/core/src/state/service.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 74d55afe2add..792d5c41d7df 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -63,6 +63,8 @@ pub(crate) struct SessionServices { /// Session-scoped model client shared across turns. pub(crate) model_client: ModelClient, pub(crate) code_mode_service: CodeModeService, + /// Shared process-level environment registry. Sessions carry an `Arc` handle so they can pass + /// the same manager through child-thread spawn paths without reconstructing it. pub(crate) environment_manager: Arc, pub(crate) environment: Option>, pub(crate) allows_agent_environment_access: bool, From 775f15ff4f28444d05fd66e4e8fcff9ffed4307f Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 15:57:03 -0700 Subject: [PATCH 09/20] Use optional default environment for disabled mode Restore CODEX_EXEC_SERVER_URL=none semantics by making EnvironmentManager::default_environment return None when environment access is disabled. Remove the separate disabled-for-agent flag and derive tool availability from the optional default environment. Add an end-to-end tool exposure test for CODEX_EXEC_SERVER_URL=none. Co-authored-by: Codex --- .../app-server/src/codex_message_processor.rs | 9 +- codex-rs/core/src/session/mod.rs | 4 +- codex-rs/core/src/session/session.rs | 4 - codex-rs/core/src/session/tests.rs | 6 - codex-rs/core/src/session/turn_context.rs | 11 +- codex-rs/core/src/state/service.rs | 3 - codex-rs/core/src/thread_manager.rs | 6 +- codex-rs/core/tests/common/test_codex.rs | 13 +- codex-rs/core/tests/suite/tools.rs | 60 ++++++++ codex-rs/exec-server/src/environment.rs | 132 ++++++++---------- codex-rs/tui/src/lib.rs | 4 +- 11 files changed, 145 insertions(+), 107 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index f51722b3abf9..1e90ded15ff2 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -5681,7 +5681,8 @@ impl CodexMessageProcessor { let environment = self .thread_manager .environment_manager() - .default_environment(); + .default_environment() + .unwrap_or_else(|| Arc::new(codex_exec_server::Environment::default())); // 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()) @@ -5847,7 +5848,8 @@ impl CodexMessageProcessor { let environment = self .thread_manager .environment_manager() - .default_environment(); + .default_environment() + .unwrap_or_else(|| Arc::new(codex_exec_server::Environment::default())); // 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()) @@ -6441,8 +6443,7 @@ impl CodexMessageProcessor { .thread_manager .environment_manager() .default_environment() - .get_filesystem(); - let fs = Some(fs); + .map(|environment| environment.get_filesystem()); let mut data = Vec::new(); for cwd in cwds { let extra_roots = extra_roots_by_cwd diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 7b9e205eedfa..43abf12059d1 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -459,7 +459,9 @@ impl Codex { let (tx_event, rx_event) = async_channel::unbounded(); let environment = environment_manager.default_environment(); - let fs = Some(environment.get_filesystem()); + let fs = environment + .as_ref() + .map(|environment| environment.get_filesystem()); let plugin_outcome = plugins_manager.plugins_for_config(&config).await; let effective_skill_roots = plugin_outcome.effective_skill_roots(); let skills_input = skills_load_input_from_config(&config, effective_skill_roots); diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 2459b0926dd5..c3054eec30e1 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -623,8 +623,6 @@ impl Session { config.analytics_enabled, ) }); - let environment = environment_manager.default_environment(); - let allows_agent_environment_access = environment_manager.allows_agent_environment_access(); let services = SessionServices { // Initialize the MCP connection manager with an uninitialized // instance. It will be replaced with one created via @@ -679,8 +677,6 @@ impl Session { config.js_repl_node_path.clone(), ), environment_manager, - environment: Some(environment), - allows_agent_environment_access, }; services .model_client diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 7378dd15ec2d..48364c922967 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -3103,8 +3103,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { config.js_repl_node_path.clone(), ), environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default()), - environment: Some(Arc::clone(&environment)), - allows_agent_environment_access: true, }; let js_repl = Arc::new(JsReplHandle::with_node_path( config.js_repl_node_path.clone(), @@ -3138,7 +3136,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { model_info, &models_manager, /*network*/ None, - /*allows_agent_environment_access*/ true, Some(environment), "turn_id".to_string(), Arc::clone(&js_repl), @@ -4192,8 +4189,6 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( config.js_repl_node_path.clone(), ), environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default()), - environment: Some(Arc::clone(&environment)), - allows_agent_environment_access: true, }; let js_repl = Arc::new(JsReplHandle::with_node_path( config.js_repl_node_path.clone(), @@ -4227,7 +4222,6 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( model_info, &models_manager, /*network*/ None, - /*allows_agent_environment_access*/ true, Some(environment), "turn_id".to_string(), Arc::clone(&js_repl), diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 58fcfc73b8c9..ce6758c442ba 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -345,7 +345,6 @@ impl Session { model_info: ModelInfo, models_manager: &ModelsManager, network: Option, - allows_agent_environment_access: bool, environment: Option>, sub_id: String, js_repl: Arc, @@ -382,7 +381,7 @@ impl Session { ) .with_web_search_config(per_turn_config.web_search_config.clone()) .with_allow_login_shell(per_turn_config.permissions.allow_login_shell) - .with_has_environment(allows_agent_environment_access) + .with_has_environment(environment.is_some()) .with_spawn_agent_usage_hint(per_turn_config.multi_agent_v2.usage_hint_enabled) .with_spawn_agent_usage_hint_text(per_turn_config.multi_agent_v2.usage_hint_text.clone()) .with_hide_spawn_agent_metadata(per_turn_config.multi_agent_v2.hide_spawn_agent_metadata) @@ -545,9 +544,8 @@ impl Session { .await; let effective_skill_roots = plugin_outcome.effective_skill_roots(); let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots); - let fs = self - .services - .environment + let environment = self.services.environment_manager.default_environment(); + let fs = environment .as_ref() .map(|environment| environment.get_filesystem()); let skills_outcome = Arc::new( @@ -577,8 +575,7 @@ impl Session { ) .then(|| started_proxy.proxy()) }), - self.services.allows_agent_environment_access, - self.services.environment.clone(), + environment, sub_id, Arc::clone(&self.js_repl), skills_outcome, diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 792d5c41d7df..3fbc0361e42f 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -16,7 +16,6 @@ use crate::tools::network_approval::NetworkApprovalService; use crate::tools::sandboxing::ApprovalStore; use crate::unified_exec::UnifiedExecProcessManager; use codex_analytics::AnalyticsEventsClient; -use codex_exec_server::Environment; use codex_exec_server::EnvironmentManager; use codex_hooks::Hooks; use codex_login::AuthManager; @@ -66,6 +65,4 @@ pub(crate) struct SessionServices { /// Shared process-level environment registry. Sessions carry an `Arc` handle so they can pass /// the same manager through child-thread spawn paths without reconstructing it. pub(crate) environment_manager: Arc, - pub(crate) environment: Option>, - pub(crate) allows_agent_environment_access: bool, } diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index da109bbcc7be..c5da648bf503 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -923,8 +923,8 @@ impl ThreadManagerState { user_shell_override: Option, ) -> CodexResult { let environment = self.environment_manager.default_environment(); - let watch_registration = match environment.is_remote() { - false => { + let watch_registration = match environment.as_ref() { + Some(environment) if !environment.is_remote() => { self.skills_watcher .register_config( &config, @@ -934,7 +934,7 @@ impl ThreadManagerState { ) .await } - true => crate::file_watcher::WatchRegistration::default(), + Some(_) | None => crate::file_watcher::WatchRegistration::default(), }; let CodexSpawnOk { codex, thread_id, .. diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 0993b5b0e42f..67d1a49968fd 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -204,6 +204,7 @@ pub struct TestCodexBuilder { workspace_setups: Vec>, home: Option>, user_shell_override: Option, + exec_server_url: Option, } impl TestCodexBuilder { @@ -255,6 +256,11 @@ impl TestCodexBuilder { self } + pub fn with_exec_server_url(mut self, exec_server_url: impl Into) -> Self { + self.exec_server_url = Some(exec_server_url.into()); + self + } + pub fn with_windows_cmd_shell(self) -> Self { if cfg!(windows) { self.with_user_shell(get_shell_by_model_provided_path(&PathBuf::from("cmd.exe"))) @@ -350,9 +356,13 @@ impl TestCodexBuilder { let (config, fallback_cwd) = self .prepare_config(base_url, &home, test_env.cwd().clone()) .await?; + let exec_server_url = self + .exec_server_url + .clone() + .or_else(|| test_env.exec_server_url().map(str::to_owned)); let environment_manager = Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs { - exec_server_url: test_env.exec_server_url().map(str::to_owned), + exec_server_url, local_runtime_paths: None, }, )); @@ -888,6 +898,7 @@ pub fn test_codex() -> TestCodexBuilder { workspace_setups: vec![], home: None, user_shell_override: None, + exec_server_url: None, } } diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index a995e54431c4..a6166f8403bf 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -75,6 +75,66 @@ fn ev_namespaced_function_call( }) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn exec_server_none_omits_environment_backed_tools() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let response_mock = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-1"), + ]), + ) + .await; + + let mut builder = test_codex() + .with_exec_server_url("none") + .with_config(|config| { + config + .features + .enable(Feature::UnifiedExec) + .expect("unified exec should enable for test"); + config + .features + .enable(Feature::JsRepl) + .expect("js repl should enable for test"); + config.include_apply_patch_tool = true; + }); + let test = builder.build(&server).await?; + + test.submit_turn("which tools are available?").await?; + + let tools = tool_names(&response_mock.single_request().body_json()); + assert!( + tools.contains(&"update_plan".to_string()), + "non-environment tool should remain available; got {tools:?}" + ); + for environment_tool in [ + "exec_command", + "write_stdin", + "js_repl", + "js_repl_reset", + "apply_patch", + "view_image", + ] { + assert!( + !tools.contains(&environment_tool.to_string()), + "{environment_tool} should be omitted when CODEX_EXEC_SERVER_URL=none; got {tools:?}" + ); + } + assert!( + test.thread_manager + .environment_manager() + .default_environment() + .is_none() + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn custom_tool_unknown_returns_custom_output_error() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 53a737dc5708..d8e532e197e4 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -16,25 +16,22 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; /// Owns the execution/filesystem environments available to a session. /// /// `EnvironmentManager` is the session-scoped registry for concrete -/// environments. It always creates a local environment under [`LOCAL_ENVIRONMENT_ID`]. -/// When `CODEX_EXEC_SERVER_URL` is set to a websocket URL, it also creates a -/// remote environment under [`REMOTE_ENVIRONMENT_ID`] and makes that the default -/// environment. Otherwise the local environment is the default. +/// environments. It creates a local environment under [`LOCAL_ENVIRONMENT_ID`] +/// unless environment access is disabled. When `CODEX_EXEC_SERVER_URL` is set to +/// a websocket URL, it also creates a remote environment under +/// [`REMOTE_ENVIRONMENT_ID`] and makes that the default environment. Otherwise +/// the local environment is the default. /// -/// Setting `CODEX_EXEC_SERVER_URL=none` does not remove the local environment: -/// Codex internals may still use `default_environment()` or explicit -/// `get_environment()` lookups. -/// Instead it disables agent/tool access as reported by -/// `allows_agent_environment_access()`, so shell/filesystem tools can be hidden -/// from the model while internal local filesystem access still works. +/// Setting `CODEX_EXEC_SERVER_URL=none` disables environment access by leaving +/// the default environment unset. Callers use `default_environment().is_some()` +/// as the signal for model-facing shell/filesystem tool availability. /// /// Remote environments hold a lazy exec-server client handle. The websocket is /// not opened when the manager or environment is constructed; it connects on the /// first remote exec or filesystem operation. #[derive(Debug)] pub struct EnvironmentManager { - default_environment: String, - environment_disabled_for_agent: bool, + default_environment: Option, environments: HashMap>, } @@ -69,54 +66,50 @@ impl EnvironmentManager { exec_server_url, local_runtime_paths, } = args; - let (exec_server_url, environment_disabled_for_agent) = - normalize_exec_server_url(exec_server_url); + let (exec_server_url, environment_disabled) = normalize_exec_server_url(exec_server_url); let mut environments = HashMap::new(); - environments.insert( - LOCAL_ENVIRONMENT_ID.to_string(), - Arc::new( - Environment::create_with_runtime_paths( - /*exec_server_url*/ None, - local_runtime_paths.clone(), - ) - .expect("valid local environment"), - ), - ); - - let default_environment = match exec_server_url { - Some(exec_server_url) => { - environments.insert( - REMOTE_ENVIRONMENT_ID.to_string(), - Arc::new( - Environment::create_with_runtime_paths( - Some(exec_server_url), - local_runtime_paths, - ) - .expect("valid remote environment"), - ), - ); - REMOTE_ENVIRONMENT_ID.to_string() + let default_environment = if environment_disabled { + None + } else { + environments.insert( + LOCAL_ENVIRONMENT_ID.to_string(), + Arc::new( + Environment::create_with_runtime_paths( + /*exec_server_url*/ None, + local_runtime_paths.clone(), + ) + .expect("valid local environment"), + ), + ); + match exec_server_url { + Some(exec_server_url) => { + environments.insert( + REMOTE_ENVIRONMENT_ID.to_string(), + Arc::new( + Environment::create_with_runtime_paths( + Some(exec_server_url), + local_runtime_paths, + ) + .expect("valid remote environment"), + ), + ); + Some(REMOTE_ENVIRONMENT_ID.to_string()) + } + None => Some(LOCAL_ENVIRONMENT_ID.to_string()), } - None => LOCAL_ENVIRONMENT_ID.to_string(), }; Self { default_environment, - environment_disabled_for_agent, environments, } } - /// Returns true when model-facing tools may access an environment. - pub fn allows_agent_environment_access(&self) -> bool { - !self.environment_disabled_for_agent - && self.environments.contains_key(&self.default_environment) - } - /// Returns the default environment instance. - pub fn default_environment(&self) -> Arc { - self.get_environment(&self.default_environment) - .expect("default environment exists") + pub fn default_environment(&self) -> Option> { + self.default_environment + .as_deref() + .and_then(|environment_id| self.get_environment(environment_id)) } /// Returns a named environment instance. @@ -260,9 +253,8 @@ mod tests { local_runtime_paths: None, }); - let environment = manager.default_environment(); + let environment = manager.default_environment().expect("default environment"); assert!(!environment.is_remote()); - assert!(manager.allows_agent_environment_access()); assert!( !manager .get_environment(LOCAL_ENVIRONMENT_ID) @@ -279,14 +271,8 @@ mod tests { local_runtime_paths: None, }); - assert!(!manager.allows_agent_environment_access()); - assert!(!manager.default_environment().is_remote()); - assert!( - !manager - .get_environment(LOCAL_ENVIRONMENT_ID) - .expect("local environment") - .is_remote() - ); + assert!(manager.default_environment().is_none()); + assert!(manager.get_environment(LOCAL_ENVIRONMENT_ID).is_none()); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } @@ -297,9 +283,8 @@ mod tests { local_runtime_paths: None, }); - let environment = manager.default_environment(); + let environment = manager.default_environment().expect("default environment"); assert!(environment.is_remote()); - assert!(manager.allows_agent_environment_access()); assert_eq!(environment.exec_server_url(), Some("ws://127.0.0.1:8765")); assert!( !manager @@ -320,8 +305,8 @@ mod tests { async fn environment_manager_default_environment_caches_environment() { let manager = EnvironmentManager::new(EnvironmentManagerArgs::default()); - let first = manager.default_environment(); - let second = manager.default_environment(); + let first = manager.default_environment().expect("default environment"); + let second = manager.default_environment().expect("default environment"); assert!(Arc::ptr_eq(&first, &second)); } @@ -338,43 +323,36 @@ mod tests { local_runtime_paths: Some(runtime_paths.clone()), }); - let environment = manager.default_environment(); + let environment = manager.default_environment().expect("default environment"); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: environment.exec_server_url().map(str::to_owned), local_runtime_paths: environment.local_runtime_paths().cloned(), }); - let environment = manager.default_environment(); + let environment = manager.default_environment().expect("default environment"); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); } #[tokio::test] - async fn disabled_environment_manager_has_default_environment_but_no_tool_environment() { + async fn disabled_environment_manager_has_no_default_environment() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), local_runtime_paths: None, }); - assert!(!manager.default_environment().is_remote()); - assert!(!manager.allows_agent_environment_access()); + assert!(manager.default_environment().is_none()); } #[tokio::test] - async fn environment_manager_allows_local_lookup_when_disabled() { + async fn environment_manager_omits_environment_lookup_when_disabled() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), local_runtime_paths: None, }); - assert!(!manager.default_environment().is_remote()); - assert!(!manager.allows_agent_environment_access()); - assert!( - !manager - .get_environment(LOCAL_ENVIRONMENT_ID) - .expect("local environment") - .is_remote() - ); + assert!(manager.default_environment().is_none()); + assert!(manager.get_environment(LOCAL_ENVIRONMENT_ID).is_none()); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 98369a0bbda8..74d692ebb691 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -627,7 +627,9 @@ fn config_cwd_for_app_server_target( app_server_target: &AppServerTarget, environment_manager: &EnvironmentManager, ) -> std::io::Result> { - if environment_manager.default_environment().is_remote() + if environment_manager + .default_environment() + .is_some_and(|environment| environment.is_remote()) || matches!(app_server_target, AppServerTarget::Remote { .. }) { return Ok(None); From 5f504abb4ce93b5145df32d5e61ea025608157a2 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 16:25:55 -0700 Subject: [PATCH 10/20] Fix environment manager follow-up compile errors Co-authored-by: Codex --- codex-rs/core/src/session/handlers.rs | 4 ++-- codex-rs/core/src/session/mod.rs | 2 +- codex-rs/core/src/session/tests.rs | 4 ++-- codex-rs/exec-server/src/client.rs | 2 +- codex-rs/exec-server/src/environment.rs | 8 ++++---- codex-rs/exec-server/src/lib.rs | 1 + codex-rs/tui/src/lib.rs | 21 ++++++++++++--------- 7 files changed, 23 insertions(+), 19 deletions(-) diff --git a/codex-rs/core/src/session/handlers.rs b/codex-rs/core/src/session/handlers.rs index 958b98b2f168..921d528cd3ae 100644 --- a/codex-rs/core/src/session/handlers.rs +++ b/codex-rs/core/src/session/handlers.rs @@ -507,8 +507,8 @@ pub async fn list_skills(sess: &Session, sub_id: String, cwds: Vec, for let plugins_manager = &sess.services.plugins_manager; let fs = sess .services - .environment - .as_ref() + .environment_manager + .default_environment() .map(|environment| environment.get_filesystem()); let config = sess.get_config().await; let codex_home = sess.codex_home().await; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 43abf12059d1..1aa15a2cad8c 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -511,7 +511,7 @@ impl Codex { } let user_instructions = AgentsMdManager::new(&config) - .user_instructions(Some(environment.as_ref())) + .user_instructions(environment.as_deref()) .await; let exec_policy = if crate::guardian::is_guardian_reviewer_source(&session_source) { diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 48364c922967..e11786145de7 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -2718,8 +2718,8 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() { let skill_fs = session .services - .environment - .as_ref() + .environment_manager + .default_environment() .map(|environment| environment.get_filesystem()) .unwrap_or_else(|| std::sync::Arc::clone(&codex_exec_server::LOCAL_FS)); let parent_outcome = session diff --git a/codex-rs/exec-server/src/client.rs b/codex-rs/exec-server/src/client.rs index c4526e13bd4f..375571b77930 100644 --- a/codex-rs/exec-server/src/client.rs +++ b/codex-rs/exec-server/src/client.rs @@ -175,7 +175,7 @@ pub struct ExecServerClient { inner: Arc, } -#[derive(Clone, Debug)] +#[derive(Clone)] pub(crate) struct LazyRemoteExecServerClient { websocket_url: String, client: Arc>, diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index d8e532e197e4..f852148eb52c 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -246,8 +246,8 @@ mod tests { assert!(environment.remote_exec_server_client.is_none()); } - #[test] - fn environment_manager_normalizes_empty_url() { + #[tokio::test] + async fn environment_manager_normalizes_empty_url() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some(String::new()), local_runtime_paths: None, @@ -276,8 +276,8 @@ mod tests { assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } - #[test] - fn environment_manager_reports_remote_url() { + #[tokio::test] + async fn environment_manager_reports_remote_url() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("ws://127.0.0.1:8765".to_string()), local_runtime_paths: None, diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index 067fa0a7c147..fc6a86f50836 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -25,6 +25,7 @@ pub use client_api::RemoteExecServerConnectArgs; pub use environment::CODEX_EXEC_SERVER_URL_ENV_VAR; pub use environment::Environment; pub use environment::EnvironmentManager; +pub use environment::EnvironmentManagerArgs; pub use file_system::CopyOptions; pub use file_system::CreateDirectoryOptions; pub use file_system::ExecutorFileSystem; diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 74d692ebb691..a021f11f236c 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -1928,8 +1928,9 @@ mod tests { Ok(()) } - #[test] - fn config_cwd_for_app_server_target_omits_cwd_for_remote_sessions() -> std::io::Result<()> { + #[tokio::test] + async fn config_cwd_for_app_server_target_omits_cwd_for_remote_sessions() -> std::io::Result<()> + { let remote_only_cwd = if cfg!(windows) { Path::new(r"C:\definitely\not\local\to\this\test") } else { @@ -1949,8 +1950,9 @@ mod tests { Ok(()) } - #[test] - fn config_cwd_for_app_server_target_canonicalizes_embedded_cli_cwd() -> std::io::Result<()> { + #[tokio::test] + async fn config_cwd_for_app_server_target_canonicalizes_embedded_cli_cwd() -> std::io::Result<()> + { let temp_dir = TempDir::new()?; let target = AppServerTarget::Embedded; let environment_manager = @@ -1968,9 +1970,9 @@ mod tests { Ok(()) } - #[test] - fn config_cwd_for_app_server_target_errors_for_missing_embedded_cli_cwd() -> std::io::Result<()> - { + #[tokio::test] + async fn config_cwd_for_app_server_target_errors_for_missing_embedded_cli_cwd() + -> std::io::Result<()> { let temp_dir = TempDir::new()?; let missing = temp_dir.path().join("missing"); let target = AppServerTarget::Embedded; @@ -1984,8 +1986,9 @@ mod tests { Ok(()) } - #[test] - fn config_cwd_for_app_server_target_omits_cwd_for_remote_exec_server() -> std::io::Result<()> { + #[tokio::test] + async fn config_cwd_for_app_server_target_omits_cwd_for_remote_exec_server() + -> std::io::Result<()> { let remote_only_cwd = if cfg!(windows) { Path::new(r"C:\definitely\not\local\to\this\test") } else { From bb3600779c6ad1fc8f4cc46943ba8b023dbdd4ca Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 17 Apr 2026 18:27:18 -0700 Subject: [PATCH 11/20] Fix environment manager hardening issues Co-authored-by: Codex --- codex-rs/app-server-client/src/lib.rs | 1 + codex-rs/core/src/session/session.rs | 5 +- codex-rs/exec-server/src/environment.rs | 174 +++++++++++------- .../exec-server/src/remote_file_system.rs | 24 ++- codex-rs/exec-server/src/remote_process.rs | 6 +- 5 files changed, 124 insertions(+), 86 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 726939d292fe..c1072e566938 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -2007,6 +2007,7 @@ mod tests { runtime_args .environment_manager .default_environment() + .expect("default environment") .is_remote() ); } diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index c3054eec30e1..8a87e43da180 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -770,8 +770,9 @@ impl Session { tx_event.clone(), session_configuration.sandbox_policy.get().clone(), McpRuntimeEnvironment::new( - environment - .clone() + sess.services + .environment_manager + .default_environment() .unwrap_or_else(|| Arc::new(Environment::default())), session_configuration.cwd.to_path_buf(), ), diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index f852148eb52c..660880f9ae56 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -3,7 +3,6 @@ use std::sync::Arc; use crate::ExecServerError; use crate::ExecServerRuntimePaths; -use crate::client::LazyRemoteExecServerClient; use crate::file_system::ExecutorFileSystem; use crate::local_file_system::LocalFileSystem; use crate::local_process::LocalProcess; @@ -13,22 +12,23 @@ use crate::remote_process::RemoteProcess; pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; -/// Owns the execution/filesystem environments available to a session. +/// Owns the execution/filesystem environments available to the Codex runtime. /// -/// `EnvironmentManager` is the session-scoped registry for concrete -/// environments. It creates a local environment under [`LOCAL_ENVIRONMENT_ID`] -/// unless environment access is disabled. When `CODEX_EXEC_SERVER_URL` is set to -/// a websocket URL, it also creates a remote environment under -/// [`REMOTE_ENVIRONMENT_ID`] and makes that the default environment. Otherwise -/// the local environment is the default. +/// `EnvironmentManager` is a shared registry for concrete environments. It +/// always creates a local environment under [`LOCAL_ENVIRONMENT_ID`]. When +/// `CODEX_EXEC_SERVER_URL` is set to a websocket URL, it also creates a remote +/// environment under [`REMOTE_ENVIRONMENT_ID`] and makes that the default +/// environment. Otherwise the local environment is the default. /// /// Setting `CODEX_EXEC_SERVER_URL=none` disables environment access by leaving -/// the default environment unset. Callers use `default_environment().is_some()` -/// as the signal for model-facing shell/filesystem tool availability. +/// the default environment unset while still keeping the local environment +/// available for internal callers by id. Callers use +/// `default_environment().is_some()` as the signal for model-facing +/// shell/filesystem tool availability. /// -/// Remote environments hold a lazy exec-server client handle. The websocket is -/// not opened when the manager or environment is constructed; it connects on the -/// first remote exec or filesystem operation. +/// Remote environments create remote filesystem and execution backends that +/// lazy-connect to the configured exec-server on first use. The websocket is +/// not opened when the manager or environment is constructed. #[derive(Debug)] pub struct EnvironmentManager { default_environment: Option, @@ -44,6 +44,15 @@ pub struct EnvironmentManagerArgs { pub local_runtime_paths: Option, } +impl From> for EnvironmentManagerArgs { + fn from(exec_server_url: Option) -> Self { + Self { + exec_server_url, + local_runtime_paths: None, + } + } +} + impl Default for EnvironmentManager { fn default() -> Self { Self::new(EnvironmentManagerArgs::default()) @@ -61,37 +70,30 @@ impl EnvironmentManager { /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value and local /// runtime paths used when creating local filesystem helpers. - pub fn new(args: EnvironmentManagerArgs) -> Self { + pub fn new(exec_server_url: impl Into) -> Self { + let args = exec_server_url.into(); let EnvironmentManagerArgs { exec_server_url, local_runtime_paths, } = args; let (exec_server_url, environment_disabled) = normalize_exec_server_url(exec_server_url); - let mut environments = HashMap::new(); + let mut environments = HashMap::from([( + LOCAL_ENVIRONMENT_ID.to_string(), + Arc::new(Environment::local_with_runtime_paths( + local_runtime_paths.clone(), + )), + )]); let default_environment = if environment_disabled { None } else { - environments.insert( - LOCAL_ENVIRONMENT_ID.to_string(), - Arc::new( - Environment::create_with_runtime_paths( - /*exec_server_url*/ None, - local_runtime_paths.clone(), - ) - .expect("valid local environment"), - ), - ); match exec_server_url { Some(exec_server_url) => { environments.insert( REMOTE_ENVIRONMENT_ID.to_string(), - Arc::new( - Environment::create_with_runtime_paths( - Some(exec_server_url), - local_runtime_paths, - ) - .expect("valid remote environment"), - ), + Arc::new(Environment::remote_with_runtime_paths( + exec_server_url, + local_runtime_paths, + )), ); Some(REMOTE_ENVIRONMENT_ID.to_string()) } @@ -120,13 +122,13 @@ impl EnvironmentManager { /// Concrete execution/filesystem environment selected for a session. /// -/// This bundles the selected backend together with the corresponding remote -/// client, if any. +/// This bundles the selected backend metadata together with the local runtime +/// paths used by filesystem helpers. #[derive(Clone)] pub struct Environment { exec_server_url: Option, - remote_exec_server_client: Option, exec_backend: Arc, + filesystem: Arc, local_runtime_paths: Option, } @@ -134,8 +136,8 @@ impl Default for Environment { fn default() -> Self { Self { exec_server_url: None, - remote_exec_server_client: None, exec_backend: Arc::new(LocalProcess::default()), + filesystem: Arc::new(LocalFileSystem::unsandboxed()), local_runtime_paths: None, } } @@ -168,25 +170,43 @@ impl Environment { )); } - let remote_exec_server_client = if let Some(exec_server_url) = exec_server_url.clone() { - Some(LazyRemoteExecServerClient::new(exec_server_url)) - } else { - None + Ok(match exec_server_url { + Some(exec_server_url) => { + Self::remote_with_runtime_paths(exec_server_url, local_runtime_paths) + } + None => Self::local_with_runtime_paths(local_runtime_paths), + }) + } + + fn local_with_runtime_paths(local_runtime_paths: Option) -> Self { + let filesystem: Arc = match local_runtime_paths.clone() { + Some(runtime_paths) => Arc::new(LocalFileSystem::with_runtime_paths(runtime_paths)), + None => Arc::new(LocalFileSystem::unsandboxed()), }; + Self { + exec_server_url: None, + exec_backend: Arc::new(LocalProcess::default()), + filesystem, + local_runtime_paths, + } + } + + fn remote_with_runtime_paths( + exec_server_url: String, + local_runtime_paths: Option, + ) -> Self { let exec_backend: Arc = - if let Some(client) = remote_exec_server_client.clone() { - Arc::new(RemoteProcess::new(client)) - } else { - Arc::new(LocalProcess::default()) - }; + Arc::new(RemoteProcess::new(exec_server_url.clone())); + let filesystem: Arc = + Arc::new(RemoteFileSystem::new(exec_server_url.clone())); - Ok(Self { - exec_server_url, - remote_exec_server_client, + Self { + exec_server_url: Some(exec_server_url), exec_backend, + filesystem, local_runtime_paths, - }) + } } pub fn is_remote(&self) -> bool { @@ -207,13 +227,7 @@ impl Environment { } pub fn get_filesystem(&self) -> Arc { - match self.remote_exec_server_client.clone() { - Some(client) => Arc::new(RemoteFileSystem::new(client)), - None => match self.local_runtime_paths.clone() { - Some(runtime_paths) => Arc::new(LocalFileSystem::with_runtime_paths(runtime_paths)), - None => Arc::new(LocalFileSystem::unsandboxed()), - }, - } + Arc::clone(&self.filesystem) } } @@ -243,7 +257,7 @@ mod tests { Environment::create(/*exec_server_url*/ None).expect("create environment"); assert_eq!(environment.exec_server_url(), None); - assert!(environment.remote_exec_server_client.is_none()); + assert!(!environment.is_remote()); } #[tokio::test] @@ -264,15 +278,20 @@ mod tests { assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } - #[test] - fn environment_manager_treats_none_value_as_disabled() { + #[tokio::test] + async fn environment_manager_treats_none_value_as_disabled() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), local_runtime_paths: None, }); assert!(manager.default_environment().is_none()); - assert!(manager.get_environment(LOCAL_ENVIRONMENT_ID).is_none()); + assert!( + !manager + .get_environment(LOCAL_ENVIRONMENT_ID) + .expect("local environment") + .is_remote() + ); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } @@ -286,19 +305,27 @@ mod tests { let environment = manager.default_environment().expect("default environment"); assert!(environment.is_remote()); assert_eq!(environment.exec_server_url(), Some("ws://127.0.0.1:8765")); + assert!(Arc::ptr_eq( + &environment, + &manager + .get_environment(REMOTE_ENVIRONMENT_ID) + .expect("remote environment") + )); assert!( !manager .get_environment(LOCAL_ENVIRONMENT_ID) .expect("local environment") .is_remote() ); - assert_eq!( - manager - .get_environment(REMOTE_ENVIRONMENT_ID) - .expect("remote environment") - .exec_server_url(), - Some("ws://127.0.0.1:8765") - ); + } + + #[test] + fn create_remote_environment_does_not_connect() { + let environment = + Environment::create(Some("ws://127.0.0.1:9".to_string())).expect("create environment"); + + assert!(environment.is_remote()); + assert_eq!(environment.exec_server_url(), Some("ws://127.0.0.1:9")); } #[tokio::test] @@ -309,6 +336,10 @@ mod tests { let second = manager.default_environment().expect("default environment"); assert!(Arc::ptr_eq(&first, &second)); + assert!(Arc::ptr_eq( + &first.get_filesystem(), + &second.get_filesystem() + )); } #[tokio::test] @@ -345,14 +376,19 @@ mod tests { } #[tokio::test] - async fn environment_manager_omits_environment_lookup_when_disabled() { + async fn environment_manager_keeps_local_lookup_when_default_disabled() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), local_runtime_paths: None, }); assert!(manager.default_environment().is_none()); - assert!(manager.get_environment(LOCAL_ENVIRONMENT_ID).is_none()); + assert!( + !manager + .get_environment(LOCAL_ENVIRONMENT_ID) + .expect("local environment") + .is_remote() + ); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index 02a5e2883686..d06f9acdd1ba 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -32,13 +32,11 @@ pub(crate) struct RemoteFileSystem { } impl RemoteFileSystem { - pub(crate) fn new(client: LazyRemoteExecServerClient) -> Self { + pub(crate) fn new(websocket_url: String) -> Self { trace!("remote fs new"); - Self { client } - } - - async fn client(&self) -> FileSystemResult { - self.client.get().await.map_err(map_remote_error) + Self { + client: LazyRemoteExecServerClient::new(websocket_url), + } } } @@ -50,7 +48,7 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { trace!("remote fs read_file"); - let client = self.client().await?; + let client = self.client.get().await.map_err(map_remote_error)?; let response = client .fs_read_file(FsReadFileParams { path: path.clone(), @@ -73,7 +71,7 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs write_file"); - let client = self.client().await?; + let client = self.client.get().await.map_err(map_remote_error)?; client .fs_write_file(FsWriteFileParams { path: path.clone(), @@ -92,7 +90,7 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs create_directory"); - let client = self.client().await?; + let client = self.client.get().await.map_err(map_remote_error)?; client .fs_create_directory(FsCreateDirectoryParams { path: path.clone(), @@ -110,7 +108,7 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { trace!("remote fs get_metadata"); - let client = self.client().await?; + let client = self.client.get().await.map_err(map_remote_error)?; let response = client .fs_get_metadata(FsGetMetadataParams { path: path.clone(), @@ -133,7 +131,7 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { trace!("remote fs read_directory"); - let client = self.client().await?; + let client = self.client.get().await.map_err(map_remote_error)?; let response = client .fs_read_directory(FsReadDirectoryParams { path: path.clone(), @@ -159,7 +157,7 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs remove"); - let client = self.client().await?; + let client = self.client.get().await.map_err(map_remote_error)?; client .fs_remove(FsRemoveParams { path: path.clone(), @@ -180,7 +178,7 @@ impl ExecutorFileSystem for RemoteFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs copy"); - let client = self.client().await?; + let client = self.client.get().await.map_err(map_remote_error)?; client .fs_copy(FsCopyParams { source_path: source_path.clone(), diff --git a/codex-rs/exec-server/src/remote_process.rs b/codex-rs/exec-server/src/remote_process.rs index d8d06735cdb9..9de649a274ef 100644 --- a/codex-rs/exec-server/src/remote_process.rs +++ b/codex-rs/exec-server/src/remote_process.rs @@ -25,9 +25,11 @@ struct RemoteExecProcess { } impl RemoteProcess { - pub(crate) fn new(client: LazyRemoteExecServerClient) -> Self { + pub(crate) fn new(websocket_url: String) -> Self { trace!("remote process new"); - Self { client } + Self { + client: LazyRemoteExecServerClient::new(websocket_url), + } } } From e329e992aae7f60651b7b65d2b8e6361ea032145 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Mon, 20 Apr 2026 15:50:46 -0700 Subject: [PATCH 12/20] codex: remove low-value environment test Co-authored-by: Codex --- codex-rs/exec-server/src/environment.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 660880f9ae56..d7e09d7da9f0 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -319,15 +319,6 @@ mod tests { ); } - #[test] - fn create_remote_environment_does_not_connect() { - let environment = - Environment::create(Some("ws://127.0.0.1:9".to_string())).expect("create environment"); - - assert!(environment.is_remote()); - assert_eq!(environment.exec_server_url(), Some("ws://127.0.0.1:9")); - } - #[tokio::test] async fn environment_manager_default_environment_caches_environment() { let manager = EnvironmentManager::new(EnvironmentManagerArgs::default()); From 34d7a7afdbd8b6a16c9185767698d513a31c7945 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 21 Apr 2026 11:13:27 -0700 Subject: [PATCH 13/20] Share remote environment exec-server client Create one lazy exec-server client per remote environment and pass clones into the remote process and filesystem backends. This keeps ExecServerClient as the connected-client type while avoiding duplicate websocket clients for one environment. Co-authored-by: Codex --- codex-rs/exec-server/src/environment.rs | 8 ++++---- codex-rs/exec-server/src/remote_file_system.rs | 6 ++---- codex-rs/exec-server/src/remote_process.rs | 6 ++---- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index d7e09d7da9f0..9fe2eb530090 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use crate::ExecServerError; use crate::ExecServerRuntimePaths; +use crate::client::LazyRemoteExecServerClient; use crate::file_system::ExecutorFileSystem; use crate::local_file_system::LocalFileSystem; use crate::local_process::LocalProcess; @@ -196,10 +197,9 @@ impl Environment { exec_server_url: String, local_runtime_paths: Option, ) -> Self { - let exec_backend: Arc = - Arc::new(RemoteProcess::new(exec_server_url.clone())); - let filesystem: Arc = - Arc::new(RemoteFileSystem::new(exec_server_url.clone())); + let client = LazyRemoteExecServerClient::new(exec_server_url.clone()); + let exec_backend: Arc = Arc::new(RemoteProcess::new(client.clone())); + let filesystem: Arc = Arc::new(RemoteFileSystem::new(client)); Self { exec_server_url: Some(exec_server_url), diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index d06f9acdd1ba..dc269505a1d4 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -32,11 +32,9 @@ pub(crate) struct RemoteFileSystem { } impl RemoteFileSystem { - pub(crate) fn new(websocket_url: String) -> Self { + pub(crate) fn new(client: LazyRemoteExecServerClient) -> Self { trace!("remote fs new"); - Self { - client: LazyRemoteExecServerClient::new(websocket_url), - } + Self { client } } } diff --git a/codex-rs/exec-server/src/remote_process.rs b/codex-rs/exec-server/src/remote_process.rs index 9de649a274ef..d8d06735cdb9 100644 --- a/codex-rs/exec-server/src/remote_process.rs +++ b/codex-rs/exec-server/src/remote_process.rs @@ -25,11 +25,9 @@ struct RemoteExecProcess { } impl RemoteProcess { - pub(crate) fn new(websocket_url: String) -> Self { + pub(crate) fn new(client: LazyRemoteExecServerClient) -> Self { trace!("remote process new"); - Self { - client: LazyRemoteExecServerClient::new(websocket_url), - } + Self { client } } } From e461c3ad8d368c511b2d29511d5386d2130bec54 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 21 Apr 2026 11:21:04 -0700 Subject: [PATCH 14/20] Hide environment manager env parsing Make EnvironmentManagerArgs::default() own CODEX_EXEC_SERVER_URL parsing so production entrypoints can keep using EnvironmentManager::new with struct update syntax for runtime paths. Add explicit test defaults so test managers do not depend on the process environment. Co-authored-by: Codex --- codex-rs/app-server-client/src/lib.rs | 2 +- .../app-server/src/bespoke_event_handling.rs | 2 +- codex-rs/app-server/src/in_process.rs | 2 +- codex-rs/app-server/src/lib.rs | 3 +- .../src/message_processor/tracing_tests.rs | 2 +- .../app-server/tests/suite/v2/mcp_resource.rs | 2 +- codex-rs/core/src/agent/control_tests.rs | 14 ++++---- codex-rs/core/src/memories/tests.rs | 2 +- codex-rs/core/src/prompt_debug.rs | 3 +- codex-rs/core/src/session/tests.rs | 8 ++--- .../core/src/session/tests/guardian_tests.rs | 2 +- codex-rs/core/src/thread_manager.rs | 2 +- codex-rs/core/src/thread_manager_tests.rs | 10 +++--- codex-rs/core/tests/suite/client.rs | 2 +- codex-rs/exec-server/src/environment.rs | 32 +++++++++++++------ codex-rs/exec/src/lib.rs | 3 +- codex-rs/mcp-server/src/lib.rs | 3 +- codex-rs/tui/src/app/test_support.rs | 2 +- codex-rs/tui/src/app/tests.rs | 4 +-- codex-rs/tui/src/lib.rs | 15 ++++----- codex-rs/tui/src/onboarding/auth.rs | 2 +- 21 files changed, 64 insertions(+), 53 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index c1072e566938..7bc5ccca93e1 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -971,7 +971,7 @@ mod tests { feedback: CodexFeedback::new(), log_db: None, environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), config_warnings: Vec::new(), session_source, diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 806652a7d6e5..8d85fa7000b9 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -3498,7 +3498,7 @@ mod tests { config.model_provider.clone(), config.codex_home.to_path_buf(), Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ), ); diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 1cd6e16ca054..e447c270b3c1 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -751,7 +751,7 @@ mod tests { feedback: CodexFeedback::new(), log_db: None, environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), config_warnings: Vec::new(), session_source, diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index a69015bff9df..f6b0f77ce047 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -6,7 +6,6 @@ use codex_config::ThreadConfigLoader; use codex_core::config::Config; use codex_core::config_loader::ConfigLayerStackOrdering; use codex_core::config_loader::LoaderOverrides; -use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_exec_server::EnvironmentManagerArgs; use codex_features::Feature; use codex_login::AuthManager; @@ -363,11 +362,11 @@ pub async fn run_main_with_transport( auth: AppServerWebsocketAuthSettings, ) -> IoResult<()> { let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), local_runtime_paths: Some(ExecServerRuntimePaths::from_optional_paths( arg0_paths.codex_self_exe.clone(), arg0_paths.codex_linux_sandbox_exe.clone(), )?), + ..EnvironmentManagerArgs::default() })); let (transport_event_tx, mut transport_event_rx) = mpsc::channel::(CHANNEL_CAPACITY); diff --git a/codex-rs/app-server/src/message_processor/tracing_tests.rs b/codex-rs/app-server/src/message_processor/tracing_tests.rs index f615aa429bbe..99a2c918e0e3 100644 --- a/codex-rs/app-server/src/message_processor/tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor/tracing_tests.rs @@ -280,7 +280,7 @@ fn build_test_processor( config, config_manager, environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), feedback: CodexFeedback::new(), log_db: None, diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs index 4bc412d54e8d..7af4682cadb2 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -205,7 +205,7 @@ async fn mcp_resource_read_returns_error_for_unknown_thread() -> Result<()> { feedback: CodexFeedback::new(), log_db: None, environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), config_warnings: Vec::new(), session_source: SessionSource::Cli, diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 15eb23d0cdbb..567ee1a029ab 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -96,7 +96,7 @@ impl AgentControlHarness { config.model_provider.clone(), config.codex_home.to_path_buf(), std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ); let control = manager.agent_control(); @@ -912,7 +912,7 @@ async fn spawn_agent_respects_max_threads_limit() { config.model_provider.clone(), config.codex_home.to_path_buf(), std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ); let control = manager.agent_control(); @@ -966,7 +966,7 @@ async fn spawn_agent_releases_slot_after_shutdown() { config.model_provider.clone(), config.codex_home.to_path_buf(), std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ); let control = manager.agent_control(); @@ -1011,7 +1011,7 @@ async fn spawn_agent_limit_shared_across_clones() { config.model_provider.clone(), config.codex_home.to_path_buf(), std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ); let control = manager.agent_control(); @@ -1058,7 +1058,7 @@ async fn resume_agent_respects_max_threads_limit() { config.model_provider.clone(), config.codex_home.to_path_buf(), std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ); let control = manager.agent_control(); @@ -1116,7 +1116,7 @@ async fn resume_agent_releases_slot_after_resume_failure() { config.model_provider.clone(), config.codex_home.to_path_buf(), std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ); let control = manager.agent_control(); @@ -1513,7 +1513,7 @@ async fn resume_thread_subagent_restores_stored_nickname_and_role() { config.model_provider.clone(), config.codex_home.to_path_buf(), std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ); let control = manager.agent_control(); diff --git a/codex-rs/core/src/memories/tests.rs b/codex-rs/core/src/memories/tests.rs index 1048b3da869a..18cec7c15714 100644 --- a/codex-rs/core/src/memories/tests.rs +++ b/codex-rs/core/src/memories/tests.rs @@ -492,7 +492,7 @@ mod phase2 { config.model_provider.clone(), config.codex_home.to_path_buf(), std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ); let (mut session, _turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index 9717163df2db..73d8c2ae14a9 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -2,6 +2,7 @@ use std::collections::HashSet; use std::sync::Arc; use codex_exec_server::EnvironmentManager; +use codex_exec_server::EnvironmentManagerArgs; use codex_features::Feature; use codex_login::AuthManager; use codex_models_manager::collaboration_mode_presets::CollaborationModesConfig; @@ -38,7 +39,7 @@ pub async fn build_prompt_input( .features .enabled(Feature::DefaultModeRequestUserInput), }, - Arc::new(EnvironmentManager::from_env()), + Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::default())), /*analytics_events_client*/ None, ); let thread = thread_manager.start_thread(config).await?; diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index e11786145de7..f0d21245e18b 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -2945,7 +2945,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() { mcp_manager, Arc::new(SkillsWatcher::noop()), AgentControl::default(), - Arc::new(codex_exec_server::EnvironmentManager::default()), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, ) .await; @@ -3102,7 +3102,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { code_mode_service: crate::tools::code_mode::CodeModeService::new( config.js_repl_node_path.clone(), ), - environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default()), + environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), }; let js_repl = Arc::new(JsReplHandle::with_node_path( config.js_repl_node_path.clone(), @@ -3257,7 +3257,7 @@ async fn make_session_with_config_and_rx( mcp_manager, Arc::new(SkillsWatcher::noop()), AgentControl::default(), - Arc::new(codex_exec_server::EnvironmentManager::default()), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, ) .await?; @@ -4188,7 +4188,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( code_mode_service: crate::tools::code_mode::CodeModeService::new( config.js_repl_node_path.clone(), ), - environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default()), + environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), }; let js_repl = Arc::new(JsReplHandle::with_node_path( config.js_repl_node_path.clone(), diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 7a81c171fcd6..28a4d0d33795 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -636,7 +636,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { auth_manager, models_manager, environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), skills_manager, plugins_manager, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index c5da648bf503..5dc8a2b79ec8 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -302,7 +302,7 @@ impl ThreadManager { provider, codex_home.clone(), Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ); manager._test_codex_home_guard = Some(TempCodexHomeGuard { path: codex_home }); diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 76dd62bb9a56..8b6f2c3048b0 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -247,7 +247,7 @@ async fn shutdown_all_threads_bounded_submits_shutdown_to_every_thread() { config.model_provider.clone(), config.codex_home.to_path_buf(), Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ); let thread_1 = manager @@ -298,7 +298,7 @@ async fn new_uses_configured_openai_provider_for_model_refresh() { SessionSource::Exec, CollaborationModesConfig::default(), Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), /*analytics_events_client*/ None, ); @@ -435,7 +435,7 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor SessionSource::Exec, CollaborationModesConfig::default(), Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), /*analytics_events_client*/ None, ); @@ -538,7 +538,7 @@ async fn interrupted_fork_snapshot_preserves_explicit_turn_id() { SessionSource::Exec, CollaborationModesConfig::default(), Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), /*analytics_events_client*/ None, ); @@ -631,7 +631,7 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ SessionSource::Exec, CollaborationModesConfig::default(), Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), /*analytics_events_client*/ None, ); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 3f0a7762ba23..ae8fbc102444 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1104,7 +1104,7 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() { .enabled(Feature::DefaultModeRequestUserInput), }, Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), /*analytics_events_client*/ None, ); diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 9fe2eb530090..2182ad9197b9 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -39,12 +39,30 @@ pub struct EnvironmentManager { pub const LOCAL_ENVIRONMENT_ID: &str = "local"; pub const REMOTE_ENVIRONMENT_ID: &str = "remote"; -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub struct EnvironmentManagerArgs { pub exec_server_url: Option, pub local_runtime_paths: Option, } +impl Default for EnvironmentManagerArgs { + fn default() -> Self { + Self { + exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), + local_runtime_paths: None, + } + } +} + +impl EnvironmentManagerArgs { + pub fn default_for_tests() -> Self { + Self { + exec_server_url: None, + local_runtime_paths: None, + } + } +} + impl From> for EnvironmentManagerArgs { fn from(exec_server_url: Option) -> Self { Self { @@ -61,12 +79,8 @@ impl Default for EnvironmentManager { } impl EnvironmentManager { - /// Builds a manager from process environment variables. - pub fn from_env() -> Self { - Self::new(EnvironmentManagerArgs { - exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), - local_runtime_paths: None, - }) + pub fn default_for_tests() -> Self { + Self::new(EnvironmentManagerArgs::default_for_tests()) } /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value and local @@ -321,7 +335,7 @@ mod tests { #[tokio::test] async fn environment_manager_default_environment_caches_environment() { - let manager = EnvironmentManager::new(EnvironmentManagerArgs::default()); + let manager = EnvironmentManager::new(EnvironmentManagerArgs::default_for_tests()); let first = manager.default_environment().expect("default environment"); let second = manager.default_environment().expect("default environment"); @@ -385,7 +399,7 @@ mod tests { #[tokio::test] async fn get_environment_returns_none_for_unknown_id() { - let manager = EnvironmentManager::new(EnvironmentManagerArgs::default()); + let manager = EnvironmentManager::new(EnvironmentManagerArgs::default_for_tests()); assert!(manager.get_environment("does-not-exist").is_none()); } diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 4cb80a1543e6..1404348af193 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -13,7 +13,6 @@ pub(crate) mod exec_events; pub use cli::Cli; pub use cli::Command; pub use cli::ReviewArgs; -use codex_app_server_client::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_app_server_client::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY; use codex_app_server_client::EnvironmentManager; use codex_app_server_client::EnvironmentManagerArgs; @@ -500,8 +499,8 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result feedback: CodexFeedback::new(), log_db: None, environment_manager: std::sync::Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), local_runtime_paths: Some(local_runtime_paths), + ..EnvironmentManagerArgs::default() })), config_warnings, session_source: SessionSource::Exec, diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index 2eb93130f135..7577818fc4af 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -7,7 +7,6 @@ use std::sync::Arc; use codex_arg0::Arg0DispatchPaths; use codex_core::config::Config; -use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_exec_server::EnvironmentManager; use codex_exec_server::EnvironmentManagerArgs; use codex_exec_server::ExecServerRuntimePaths; @@ -62,11 +61,11 @@ pub async fn run_main( cli_config_overrides: CliConfigOverrides, ) -> IoResult<()> { let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), local_runtime_paths: Some(ExecServerRuntimePaths::from_optional_paths( arg0_paths.codex_self_exe.clone(), arg0_paths.codex_linux_sandbox_exe.clone(), )?), + ..EnvironmentManagerArgs::default() })); // Parse CLI overrides once and derive the base Config eagerly so later // components do not need to work with raw TOML values. diff --git a/codex-rs/tui/src/app/test_support.rs b/codex-rs/tui/src/app/test_support.rs index 8b2c22512ef8..a48739dedc43 100644 --- a/codex-rs/tui/src/app/test_support.rs +++ b/codex-rs/tui/src/app/test_support.rs @@ -39,7 +39,7 @@ pub(super) async fn make_test_app() -> App { feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), remote_app_server_url: None, remote_app_server_auth_token: None, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 56d093598fac..5237ce7d9325 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -3577,7 +3577,7 @@ async fn make_test_app() -> App { feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), remote_app_server_url: None, remote_app_server_auth_token: None, @@ -3636,7 +3636,7 @@ async fn make_test_app_with_channels() -> ( feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), remote_app_server_url: None, remote_app_server_auth_token: None, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index a021f11f236c..930adeb57dba 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -35,7 +35,6 @@ use codex_config::CloudRequirementsLoader; use codex_config::ConfigLoadError; use codex_config::LoaderOverrides; use codex_config::format_config_error_with_source; -use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_exec_server::EnvironmentManager; use codex_exec_server::EnvironmentManagerArgs; use codex_exec_server::ExecServerRuntimePaths; @@ -428,7 +427,7 @@ pub(crate) async fn start_embedded_app_server_for_picker( config, &AppServerTarget::Embedded, Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ) .await @@ -733,11 +732,11 @@ pub async fn run_main( }; let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), local_runtime_paths: Some(ExecServerRuntimePaths::from_optional_paths( arg0_paths.codex_self_exe.clone(), arg0_paths.codex_linux_sandbox_exe.clone(), )?), + ..EnvironmentManagerArgs::default() })); let cwd = cli.cwd.clone(); let config_cwd = @@ -1779,7 +1778,7 @@ mod tests { codex_feedback::CodexFeedback::new(), /*log_db*/ None, Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), ) .await @@ -1941,7 +1940,7 @@ mod tests { auth_token: None, }; let environment_manager = - EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default()); + EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default_for_tests()); let config_cwd = config_cwd_for_app_server_target(Some(remote_only_cwd), &target, &environment_manager)?; @@ -1956,7 +1955,7 @@ mod tests { let temp_dir = TempDir::new()?; let target = AppServerTarget::Embedded; let environment_manager = - EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default()); + EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default_for_tests()); let config_cwd = config_cwd_for_app_server_target(Some(temp_dir.path()), &target, &environment_manager)?; @@ -1977,7 +1976,7 @@ mod tests { let missing = temp_dir.path().join("missing"); let target = AppServerTarget::Embedded; let environment_manager = - EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default()); + EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default_for_tests()); let err = config_cwd_for_app_server_target(Some(&missing), &target, &environment_manager) .expect_err("missing embedded cwd should fail"); @@ -2127,7 +2126,7 @@ mod tests { codex_feedback::CodexFeedback::new(), /*log_db*/ None, Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default(), + codex_exec_server::EnvironmentManagerArgs::default_for_tests(), )), |_args| async { Err(std::io::Error::other("boom")) }, ) diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 2379c2d55dbb..380f13580a85 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -990,7 +990,7 @@ mod tests { feedback: codex_feedback::CodexFeedback::new(), log_db: None, environment_manager: Arc::new(codex_app_server_client::EnvironmentManager::new( - codex_app_server_client::EnvironmentManagerArgs::default(), + codex_app_server_client::EnvironmentManagerArgs::default_for_tests(), )), config_warnings: Vec::new(), session_source: SessionSource::Cli, From 8351ce34800f1b227fe2d333768329809e7459f8 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 21 Apr 2026 11:47:30 -0700 Subject: [PATCH 15/20] Remove redundant environment-backed tools test Drop the networked integration test for CODEX_EXEC_SERVER_URL=none omitting environment-backed tools. Lower-level coverage already verifies disabled environments omit those tools. Co-authored-by: Codex --- codex-rs/core/tests/suite/tools.rs | 60 ------------------------------ 1 file changed, 60 deletions(-) diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index a6166f8403bf..a995e54431c4 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -75,66 +75,6 @@ fn ev_namespaced_function_call( }) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn exec_server_none_omits_environment_backed_tools() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let response_mock = mount_sse_once( - &server, - sse(vec![ - ev_response_created("resp-1"), - ev_assistant_message("msg-1", "done"), - ev_completed("resp-1"), - ]), - ) - .await; - - let mut builder = test_codex() - .with_exec_server_url("none") - .with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("unified exec should enable for test"); - config - .features - .enable(Feature::JsRepl) - .expect("js repl should enable for test"); - config.include_apply_patch_tool = true; - }); - let test = builder.build(&server).await?; - - test.submit_turn("which tools are available?").await?; - - let tools = tool_names(&response_mock.single_request().body_json()); - assert!( - tools.contains(&"update_plan".to_string()), - "non-environment tool should remain available; got {tools:?}" - ); - for environment_tool in [ - "exec_command", - "write_stdin", - "js_repl", - "js_repl_reset", - "apply_patch", - "view_image", - ] { - assert!( - !tools.contains(&environment_tool.to_string()), - "{environment_tool} should be omitted when CODEX_EXEC_SERVER_URL=none; got {tools:?}" - ); - } - assert!( - test.thread_manager - .environment_manager() - .default_environment() - .is_none() - ); - - Ok(()) -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn custom_tool_unknown_returns_custom_output_error() -> Result<()> { skip_if_no_network!(Ok(())); From 4565b991aba8a2af387f44cdbbb699a1218c9f02 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 21 Apr 2026 12:44:35 -0700 Subject: [PATCH 16/20] Require runtime paths for environments Make EnvironmentManagerArgs carry ExecServerRuntimePaths for production construction and route test-only unsandboxed setup through explicit _for_tests helpers. Use the manager local environment for MCP and app-server filesystem fallbacks instead of constructing a fresh default environment. Co-authored-by: Codex --- codex-rs/app-server-client/src/lib.rs | 10 +- .../app-server/src/bespoke_event_handling.rs | 4 +- .../app-server/src/codex_message_processor.rs | 27 +-- codex-rs/app-server/src/fs_api.rs | 11 +- codex-rs/app-server/src/in_process.rs | 4 +- codex-rs/app-server/src/lib.rs | 9 +- codex-rs/app-server/src/message_processor.rs | 7 +- .../src/message_processor/tracing_tests.rs | 4 +- .../app-server/tests/suite/v2/mcp_resource.rs | 4 +- codex-rs/core/src/agent/control_tests.rs | 28 +-- codex-rs/core/src/connectors.rs | 16 +- codex-rs/core/src/memories/tests.rs | 4 +- codex-rs/core/src/prompt_debug.rs | 10 +- codex-rs/core/src/session/mcp.rs | 2 +- codex-rs/core/src/session/session.rs | 2 +- codex-rs/core/src/session/tests.rs | 4 +- .../core/src/session/tests/guardian_tests.rs | 4 +- codex-rs/core/src/thread_manager.rs | 4 +- codex-rs/core/src/thread_manager_tests.rs | 20 +-- codex-rs/core/src/unified_exec/mod_tests.rs | 2 +- codex-rs/core/tests/common/test_codex.rs | 11 +- codex-rs/core/tests/suite/client.rs | 4 +- codex-rs/core/tests/suite/skills.rs | 6 +- codex-rs/exec-server/src/environment.rs | 167 +++++++++++------- codex-rs/exec-server/tests/exec_process.rs | 4 +- codex-rs/exec-server/tests/file_system.rs | 4 +- codex-rs/exec/src/lib.rs | 7 +- codex-rs/mcp-server/src/lib.rs | 9 +- codex-rs/tui/src/app/test_support.rs | 4 +- codex-rs/tui/src/app/tests.rs | 8 +- codex-rs/tui/src/lib.rs | 35 ++-- codex-rs/tui/src/onboarding/auth.rs | 6 +- 32 files changed, 224 insertions(+), 217 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 7bc5ccca93e1..fdd90f9281c1 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -970,9 +970,7 @@ mod tests { cloud_requirements: CloudRequirementsLoader::default(), feedback: CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + environment_manager: Arc::new(EnvironmentManager::default_for_tests()), config_warnings: Vec::new(), session_source, enable_codex_api_key_env: false, @@ -1975,7 +1973,11 @@ mod tests { let config = Arc::new(build_test_config().await); let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("ws://127.0.0.1:8765".to_string()), - local_runtime_paths: None, + local_runtime_paths: ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths"), })); let runtime_args = InProcessClientStartArgs { diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 8d85fa7000b9..56cd688d2a4e 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -3497,9 +3497,7 @@ mod tests { CodexAuth::create_dummy_chatgpt_auth_for_testing(), config.model_provider.clone(), config.codex_home.to_path_buf(), - Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ), ); let codex_core::NewThread { diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 1e90ded15ff2..59189dc07abb 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -5677,15 +5677,17 @@ impl CodexMessageProcessor { .to_mcp_config(self.thread_manager.plugins_manager().as_ref()) .await; let auth = self.auth_manager.auth().await; - let runtime_environment = { - let environment = self - .thread_manager - .environment_manager() - .default_environment() - .unwrap_or_else(|| Arc::new(codex_exec_server::Environment::default())); - // 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()) + 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 { @@ -5845,11 +5847,10 @@ impl CodexMessageProcessor { .await; let auth = self.auth_manager.auth().await; let runtime_environment = { - let environment = self - .thread_manager - .environment_manager() + let environment_manager = self.thread_manager.environment_manager(); + let environment = environment_manager .default_environment() - .unwrap_or_else(|| Arc::new(codex_exec_server::Environment::default())); + .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()) diff --git a/codex-rs/app-server/src/fs_api.rs b/codex-rs/app-server/src/fs_api.rs index a2c71871db70..93b4f21c2b3b 100644 --- a/codex-rs/app-server/src/fs_api.rs +++ b/codex-rs/app-server/src/fs_api.rs @@ -20,7 +20,6 @@ use codex_app_server_protocol::FsWriteFileResponse; use codex_app_server_protocol::JSONRPCErrorError; use codex_exec_server::CopyOptions; use codex_exec_server::CreateDirectoryOptions; -use codex_exec_server::Environment; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::RemoveOptions; use std::io; @@ -31,15 +30,11 @@ pub(crate) struct FsApi { file_system: Arc, } -impl Default for FsApi { - fn default() -> Self { - Self { - file_system: Environment::default().get_filesystem(), - } +impl FsApi { + pub(crate) fn new(file_system: Arc) -> Self { + Self { file_system } } -} -impl FsApi { pub(crate) async fn read_file( &self, params: FsReadFileParams, diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index e447c270b3c1..729f6d04af0b 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -750,9 +750,7 @@ mod tests { thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + environment_manager: Arc::new(EnvironmentManager::default_for_tests()), config_warnings: Vec::new(), session_source, enable_codex_api_key_env: false, diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index f6b0f77ce047..a2f35305ae75 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -361,13 +361,12 @@ pub async fn run_main_with_transport( session_source: SessionSource, auth: AppServerWebsocketAuthSettings, ) -> IoResult<()> { - let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { - local_runtime_paths: Some(ExecServerRuntimePaths::from_optional_paths( + let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::from_env( + ExecServerRuntimePaths::from_optional_paths( arg0_paths.codex_self_exe.clone(), arg0_paths.codex_linux_sandbox_exe.clone(), - )?), - ..EnvironmentManagerArgs::default() - })); + )?, + ))); let (transport_event_tx, mut transport_event_rx) = mpsc::channel::(CHANNEL_CAPACITY); let (outgoing_tx, mut outgoing_rx) = mpsc::channel::(CHANNEL_CAPACITY); diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 2b7097c26238..54e6dbe160dd 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -328,7 +328,12 @@ impl MessageProcessor { let device_key_api = DeviceKeyApi::default(); let external_agent_config_api = ExternalAgentConfigApi::new(config.codex_home.to_path_buf()); - let fs_api = FsApi::default(); + let fs_api = FsApi::new( + thread_manager + .environment_manager() + .local_environment() + .get_filesystem(), + ); let fs_watch_manager = FsWatchManager::new(outgoing.clone()); Self { diff --git a/codex-rs/app-server/src/message_processor/tracing_tests.rs b/codex-rs/app-server/src/message_processor/tracing_tests.rs index 99a2c918e0e3..e42dd5afbc0a 100644 --- a/codex-rs/app-server/src/message_processor/tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor/tracing_tests.rs @@ -279,9 +279,7 @@ fn build_test_processor( arg0_paths: Arg0DispatchPaths::default(), config, config_manager, - environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + environment_manager: Arc::new(EnvironmentManager::default_for_tests()), feedback: CodexFeedback::new(), log_db: None, config_warnings: Vec::new(), diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs index 7af4682cadb2..a347d87fc763 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -204,9 +204,7 @@ async fn mcp_resource_read_returns_error_for_unknown_thread() -> Result<()> { thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + environment_manager: Arc::new(EnvironmentManager::default_for_tests()), config_warnings: Vec::new(), session_source: SessionSource::Cli, enable_codex_api_key_env: false, diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 567ee1a029ab..1a5c38723f8b 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -95,9 +95,7 @@ impl AgentControlHarness { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + std::sync::Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); let control = manager.agent_control(); Self { @@ -911,9 +909,7 @@ async fn spawn_agent_respects_max_threads_limit() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + std::sync::Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); let control = manager.agent_control(); @@ -965,9 +961,7 @@ async fn spawn_agent_releases_slot_after_shutdown() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + std::sync::Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); let control = manager.agent_control(); @@ -1010,9 +1004,7 @@ async fn spawn_agent_limit_shared_across_clones() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + std::sync::Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); let control = manager.agent_control(); let cloned = control.clone(); @@ -1057,9 +1049,7 @@ async fn resume_agent_respects_max_threads_limit() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + std::sync::Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); let control = manager.agent_control(); @@ -1115,9 +1105,7 @@ async fn resume_agent_releases_slot_after_resume_failure() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + std::sync::Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); let control = manager.agent_control(); @@ -1512,9 +1500,7 @@ async fn resume_thread_subagent_restores_stored_nickname_and_role() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + std::sync::Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); let control = manager.agent_control(); let harness = AgentControlHarness { diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 933ce0ac764e..befb7fe561b2 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -13,7 +13,9 @@ pub use codex_app_server_protocol::AppInfo; pub use codex_app_server_protocol::AppMetadata; use codex_connectors::AllConnectorsCacheKey; use codex_connectors::DirectoryListResponse; -use codex_exec_server::Environment; +use codex_exec_server::EnvironmentManager; +use codex_exec_server::EnvironmentManagerArgs; +use codex_exec_server::ExecServerRuntimePaths; use codex_login::token_data::TokenData; use codex_protocol::protocol::SandboxPolicy; use codex_tools::DiscoverableTool; @@ -235,6 +237,16 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( let (tx_event, rx_event) = unbounded(); drop(rx_event); + let local_runtime_paths = ExecServerRuntimePaths::from_optional_paths( + config.codex_self_exe.clone(), + config.codex_linux_sandbox_exe.clone(), + )?; + let environment_manager = + EnvironmentManager::new(EnvironmentManagerArgs::from_env(local_runtime_paths)); + let environment = environment_manager + .default_environment() + .unwrap_or_else(|| environment_manager.local_environment()); + let (mcp_connection_manager, cancel_token) = McpConnectionManager::new( &mcp_servers, config.mcp_oauth_credentials_store_mode, @@ -243,7 +255,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( INITIAL_SUBMIT_ID.to_owned(), tx_event, SandboxPolicy::new_read_only_policy(), - McpRuntimeEnvironment::new(Arc::new(Environment::default()), config.cwd.to_path_buf()), + McpRuntimeEnvironment::new(environment, config.cwd.to_path_buf()), config.codex_home.to_path_buf(), codex_apps_tools_cache_key(auth.as_ref()), ToolPluginProvenance::default(), diff --git a/codex-rs/core/src/memories/tests.rs b/codex-rs/core/src/memories/tests.rs index 18cec7c15714..1b5614d31451 100644 --- a/codex-rs/core/src/memories/tests.rs +++ b/codex-rs/core/src/memories/tests.rs @@ -491,9 +491,7 @@ mod phase2 { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - std::sync::Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + std::sync::Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); let (mut session, _turn_context) = make_session_and_context().await; session.services.state_db = Some(Arc::clone(&state_db)); diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index 73d8c2ae14a9..1f62c2b08845 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use codex_exec_server::EnvironmentManager; use codex_exec_server::EnvironmentManagerArgs; +use codex_exec_server::ExecServerRuntimePaths; use codex_features::Feature; use codex_login::AuthManager; use codex_models_manager::collaboration_mode_presets::CollaborationModesConfig; @@ -30,6 +31,11 @@ pub async fn build_prompt_input( let auth_manager = AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false); + let local_runtime_paths = ExecServerRuntimePaths::from_optional_paths( + config.codex_self_exe.clone(), + config.codex_linux_sandbox_exe.clone(), + )?; + let thread_manager = ThreadManager::new( &config, Arc::clone(&auth_manager), @@ -39,7 +45,9 @@ pub async fn build_prompt_input( .features .enabled(Feature::DefaultModeRequestUserInput), }, - Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::default())), + Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::from_env( + local_runtime_paths, + ))), /*analytics_events_client*/ None, ); let thread = thread_manager.start_thread(config).await?; diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 6fcc5336754a..2e4a3301eae7 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -237,7 +237,7 @@ impl Session { turn_context .environment .clone() - .unwrap_or_else(|| Arc::new(Environment::default())), + .unwrap_or_else(|| self.services.environment_manager.local_environment()), turn_context.cwd.to_path_buf(), ), config.codex_home.to_path_buf(), diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 8a87e43da180..b3c9b8d6a693 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -773,7 +773,7 @@ impl Session { sess.services .environment_manager .default_environment() - .unwrap_or_else(|| Arc::new(Environment::default())), + .unwrap_or_else(|| sess.services.environment_manager.local_environment()), session_configuration.cwd.to_path_buf(), ), config.codex_home.to_path_buf(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index f0d21245e18b..0b936195fd32 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -3042,7 +3042,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { )); let network_approval = Arc::new(NetworkApprovalService::default()); let environment = Arc::new( - codex_exec_server::Environment::create(/*exec_server_url*/ None) + codex_exec_server::Environment::create_for_tests(/*exec_server_url*/ None) .expect("create environment"), ); @@ -4128,7 +4128,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( )); let network_approval = Arc::new(NetworkApprovalService::default()); let environment = Arc::new( - codex_exec_server::Environment::create(/*exec_server_url*/ None) + codex_exec_server::Environment::create_for_tests(/*exec_server_url*/ None) .expect("create environment"), ); diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 28a4d0d33795..d655802ed174 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -635,9 +635,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { config, auth_manager, models_manager, - environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + environment_manager: Arc::new(EnvironmentManager::default_for_tests()), skills_manager, plugins_manager, mcp_manager, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 5dc8a2b79ec8..066bfe316525 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -301,9 +301,7 @@ impl ThreadManager { auth, provider, codex_home.clone(), - Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(EnvironmentManager::default_for_tests()), ); manager._test_codex_home_guard = Some(TempCodexHomeGuard { path: codex_home }); manager diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 8b6f2c3048b0..4dcc29f562fd 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -246,9 +246,7 @@ async fn shutdown_all_threads_bounded_submits_shutdown_to_every_thread() { CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); let thread_1 = manager .start_thread(config.clone()) @@ -297,9 +295,7 @@ async fn new_uses_configured_openai_provider_for_model_refresh() { auth_manager, SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, ); @@ -434,9 +430,7 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor auth_manager.clone(), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, ); @@ -537,9 +531,7 @@ async fn interrupted_fork_snapshot_preserves_explicit_turn_id() { auth_manager.clone(), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, ); @@ -630,9 +622,7 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ auth_manager.clone(), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, ); diff --git a/codex-rs/core/src/unified_exec/mod_tests.rs b/codex-rs/core/src/unified_exec/mod_tests.rs index 9877b2cb9fcd..1865188d3e7d 100644 --- a/codex-rs/core/src/unified_exec/mod_tests.rs +++ b/codex-rs/core/src/unified_exec/mod_tests.rs @@ -508,7 +508,7 @@ async fn completed_pipe_commands_preserve_exit_code() -> anyhow::Result<()> { shell_env(), ); - let environment = codex_exec_server::Environment::default(); + let environment = codex_exec_server::Environment::default_for_tests(); let process = UnifiedExecProcessManager::default() .open_session_with_exec_env( /*process_id*/ 1234, diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 67d1a49968fd..73219423b5a5 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -76,7 +76,8 @@ impl TestEnv { pub async fn local() -> Result { let local_cwd_temp_dir = Arc::new(TempDir::new()?); let cwd = local_cwd_temp_dir.abs(); - let environment = codex_exec_server::Environment::create(/*exec_server_url*/ None)?; + let environment = + codex_exec_server::Environment::create_for_tests(/*exec_server_url*/ None)?; Ok(Self { environment, cwd, @@ -115,7 +116,8 @@ pub async fn test_env() -> Result { match get_remote_test_env() { Some(remote_env) => { let websocket_url = remote_exec_server_url()?; - let environment = codex_exec_server::Environment::create(Some(websocket_url))?; + let environment = + codex_exec_server::Environment::create_for_tests(Some(websocket_url))?; let cwd = remote_aware_cwd_path(); environment .get_filesystem() @@ -363,7 +365,10 @@ impl TestCodexBuilder { let environment_manager = Arc::new(codex_exec_server::EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs { exec_server_url, - local_runtime_paths: None, + local_runtime_paths: codex_exec_server::ExecServerRuntimePaths::new( + std::env::current_exe()?, + /*codex_linux_sandbox_exe*/ None, + )?, }, )); let file_system = test_env.environment().get_filesystem(); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index ae8fbc102444..2ebd49d53e11 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1103,9 +1103,7 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() { .features .enabled(Feature::DefaultModeRequestUserInput), }, - Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, ); let NewThread { thread: codex, .. } = thread_manager diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 090f7a57903c..59d28b61fc62 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -5,6 +5,7 @@ use anyhow::Result; use codex_core::ThreadManager; use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::EnvironmentManager; +use codex_exec_server::ExecServerRuntimePaths; use codex_exec_server::ExecutorFileSystem; use codex_login::CodexAuth; use codex_models_manager::collaboration_mode_presets::CollaborationModesConfig; @@ -237,7 +238,10 @@ async fn list_skills_skips_cwd_roots_when_environment_disabled() -> Result<()> { Arc::new(EnvironmentManager::new( codex_exec_server::EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), - local_runtime_paths: None, + local_runtime_paths: ExecServerRuntimePaths::new( + std::env::current_exe()?, + /*codex_linux_sandbox_exe*/ None, + )?, }, )), /*analytics_events_client*/ None, diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 2182ad9197b9..9cc18ea7be22 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -42,51 +42,40 @@ pub const REMOTE_ENVIRONMENT_ID: &str = "remote"; #[derive(Clone, Debug)] pub struct EnvironmentManagerArgs { pub exec_server_url: Option, - pub local_runtime_paths: Option, -} - -impl Default for EnvironmentManagerArgs { - fn default() -> Self { - Self { - exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), - local_runtime_paths: None, - } - } + pub local_runtime_paths: ExecServerRuntimePaths, } impl EnvironmentManagerArgs { - pub fn default_for_tests() -> Self { + pub fn new(local_runtime_paths: ExecServerRuntimePaths) -> Self { Self { exec_server_url: None, - local_runtime_paths: None, + local_runtime_paths, } } -} -impl From> for EnvironmentManagerArgs { - fn from(exec_server_url: Option) -> Self { + pub fn from_env(local_runtime_paths: ExecServerRuntimePaths) -> Self { Self { - exec_server_url, - local_runtime_paths: None, + exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), + local_runtime_paths, } } } -impl Default for EnvironmentManager { - fn default() -> Self { - Self::new(EnvironmentManagerArgs::default()) - } -} - impl EnvironmentManager { + /// Builds a test-only manager without configured sandbox helper paths. pub fn default_for_tests() -> Self { - Self::new(EnvironmentManagerArgs::default_for_tests()) + Self { + default_environment: Some(LOCAL_ENVIRONMENT_ID.to_string()), + environments: HashMap::from([( + LOCAL_ENVIRONMENT_ID.to_string(), + Arc::new(Environment::default_for_tests()), + )]), + } } /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value and local /// runtime paths used when creating local filesystem helpers. - pub fn new(exec_server_url: impl Into) -> Self { - let args = exec_server_url.into(); + pub fn new(args: EnvironmentManagerArgs) -> Self { let EnvironmentManagerArgs { exec_server_url, local_runtime_paths, @@ -94,9 +83,7 @@ impl EnvironmentManager { let (exec_server_url, environment_disabled) = normalize_exec_server_url(exec_server_url); let mut environments = HashMap::from([( LOCAL_ENVIRONMENT_ID.to_string(), - Arc::new(Environment::local_with_runtime_paths( - local_runtime_paths.clone(), - )), + Arc::new(Environment::local(local_runtime_paths.clone())), )]); let default_environment = if environment_disabled { None @@ -105,10 +92,7 @@ impl EnvironmentManager { Some(exec_server_url) => { environments.insert( REMOTE_ENVIRONMENT_ID.to_string(), - Arc::new(Environment::remote_with_runtime_paths( - exec_server_url, - local_runtime_paths, - )), + Arc::new(Environment::remote(exec_server_url, local_runtime_paths)), ); Some(REMOTE_ENVIRONMENT_ID.to_string()) } @@ -129,6 +113,12 @@ impl EnvironmentManager { .and_then(|environment_id| self.get_environment(environment_id)) } + /// Returns the local environment instance used for internal runtime work. + pub fn local_environment(&self) -> Arc { + self.get_environment(LOCAL_ENVIRONMENT_ID) + .expect("EnvironmentManager always has a local environment") + } + /// Returns a named environment instance. pub fn get_environment(&self, environment_id: &str) -> Option> { self.environments.get(environment_id).cloned() @@ -147,8 +137,9 @@ pub struct Environment { local_runtime_paths: Option, } -impl Default for Environment { - fn default() -> Self { +impl Environment { + /// Builds a test-only local environment without configured sandbox helper paths. + pub fn default_for_tests() -> Self { Self { exec_server_url: None, exec_backend: Arc::new(LocalProcess::default()), @@ -168,13 +159,21 @@ impl std::fmt::Debug for Environment { impl Environment { /// Builds an environment from the raw `CODEX_EXEC_SERVER_URL` value. - pub fn create(exec_server_url: Option) -> Result { - Self::create_with_runtime_paths(exec_server_url, /*local_runtime_paths*/ None) + pub fn create( + exec_server_url: Option, + local_runtime_paths: ExecServerRuntimePaths, + ) -> Result { + Self::create_inner(exec_server_url, Some(local_runtime_paths)) + } + + /// Builds a test-only environment without configured sandbox helper paths. + pub fn create_for_tests(exec_server_url: Option) -> Result { + Self::create_inner(exec_server_url, /*local_runtime_paths*/ None) } /// Builds an environment from the raw `CODEX_EXEC_SERVER_URL` value and /// local runtime paths used when creating local filesystem helpers. - pub(crate) fn create_with_runtime_paths( + fn create_inner( exec_server_url: Option, local_runtime_paths: Option, ) -> Result { @@ -186,28 +185,30 @@ impl Environment { } Ok(match exec_server_url { - Some(exec_server_url) => { - Self::remote_with_runtime_paths(exec_server_url, local_runtime_paths) - } - None => Self::local_with_runtime_paths(local_runtime_paths), + Some(exec_server_url) => Self::remote_inner(exec_server_url, local_runtime_paths), + None => match local_runtime_paths { + Some(local_runtime_paths) => Self::local(local_runtime_paths), + None => Self::default_for_tests(), + }, }) } - fn local_with_runtime_paths(local_runtime_paths: Option) -> Self { - let filesystem: Arc = match local_runtime_paths.clone() { - Some(runtime_paths) => Arc::new(LocalFileSystem::with_runtime_paths(runtime_paths)), - None => Arc::new(LocalFileSystem::unsandboxed()), - }; - + fn local(local_runtime_paths: ExecServerRuntimePaths) -> Self { Self { exec_server_url: None, exec_backend: Arc::new(LocalProcess::default()), - filesystem, - local_runtime_paths, + filesystem: Arc::new(LocalFileSystem::with_runtime_paths( + local_runtime_paths.clone(), + )), + local_runtime_paths: Some(local_runtime_paths), } } - fn remote_with_runtime_paths( + fn remote(exec_server_url: String, local_runtime_paths: ExecServerRuntimePaths) -> Self { + Self::remote_inner(exec_server_url, Some(local_runtime_paths)) + } + + fn remote_inner( exec_server_url: String, local_runtime_paths: Option, ) -> Self { @@ -265,10 +266,18 @@ mod tests { use crate::ProcessId; use pretty_assertions::assert_eq; + fn test_runtime_paths() -> ExecServerRuntimePaths { + ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths") + } + #[tokio::test] async fn create_local_environment_does_not_connect() { - let environment = - Environment::create(/*exec_server_url*/ None).expect("create environment"); + let environment = Environment::create(/*exec_server_url*/ None, test_runtime_paths()) + .expect("create environment"); assert_eq!(environment.exec_server_url(), None); assert!(!environment.is_remote()); @@ -278,7 +287,7 @@ mod tests { async fn environment_manager_normalizes_empty_url() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some(String::new()), - local_runtime_paths: None, + local_runtime_paths: test_runtime_paths(), }); let environment = manager.default_environment().expect("default environment"); @@ -296,7 +305,7 @@ mod tests { async fn environment_manager_treats_none_value_as_disabled() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), - local_runtime_paths: None, + local_runtime_paths: test_runtime_paths(), }); assert!(manager.default_environment().is_none()); @@ -313,7 +322,7 @@ mod tests { async fn environment_manager_reports_remote_url() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("ws://127.0.0.1:8765".to_string()), - local_runtime_paths: None, + local_runtime_paths: test_runtime_paths(), }); let environment = manager.default_environment().expect("default environment"); @@ -335,7 +344,7 @@ mod tests { #[tokio::test] async fn environment_manager_default_environment_caches_environment() { - let manager = EnvironmentManager::new(EnvironmentManagerArgs::default_for_tests()); + let manager = EnvironmentManager::default_for_tests(); let first = manager.default_environment().expect("default environment"); let second = manager.default_environment().expect("default environment"); @@ -349,14 +358,10 @@ mod tests { #[tokio::test] async fn environment_manager_carries_local_runtime_paths() { - let runtime_paths = ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe"), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths"); + let runtime_paths = test_runtime_paths(); let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: None, - local_runtime_paths: Some(runtime_paths.clone()), + local_runtime_paths: runtime_paths.clone(), }); let environment = manager.default_environment().expect("default environment"); @@ -364,7 +369,10 @@ mod tests { assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: environment.exec_server_url().map(str::to_owned), - local_runtime_paths: environment.local_runtime_paths().cloned(), + local_runtime_paths: environment + .local_runtime_paths() + .expect("local runtime paths") + .clone(), }); let environment = manager.default_environment().expect("default environment"); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); @@ -374,7 +382,7 @@ mod tests { async fn disabled_environment_manager_has_no_default_environment() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), - local_runtime_paths: None, + local_runtime_paths: test_runtime_paths(), }); assert!(manager.default_environment().is_none()); @@ -384,7 +392,7 @@ mod tests { async fn environment_manager_keeps_local_lookup_when_default_disabled() { let manager = EnvironmentManager::new(EnvironmentManagerArgs { exec_server_url: Some("none".to_string()), - local_runtime_paths: None, + local_runtime_paths: test_runtime_paths(), }); assert!(manager.default_environment().is_none()); @@ -399,14 +407,14 @@ mod tests { #[tokio::test] async fn get_environment_returns_none_for_unknown_id() { - let manager = EnvironmentManager::new(EnvironmentManagerArgs::default_for_tests()); + let manager = EnvironmentManager::default_for_tests(); assert!(manager.get_environment("does-not-exist").is_none()); } #[tokio::test] async fn default_environment_has_ready_local_executor() { - let environment = Environment::default(); + let environment = Environment::default_for_tests(); let response = environment .get_exec_backend() @@ -425,4 +433,27 @@ mod tests { assert_eq!(response.process.process_id().as_str(), "default-env-proc"); } + + #[tokio::test] + async fn test_environment_rejects_sandboxed_filesystem_without_runtime_paths() { + let environment = Environment::default_for_tests(); + let path = codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path( + std::env::current_exe().expect("current exe").as_path(), + ) + .expect("absolute current exe"); + let sandbox = crate::FileSystemSandboxContext::new( + codex_protocol::protocol::SandboxPolicy::new_read_only_policy(), + ); + + let err = environment + .get_filesystem() + .read_file(&path, Some(&sandbox)) + .await + .expect_err("sandboxed read should require runtime paths"); + + assert_eq!( + err.to_string(), + "sandboxed filesystem operations require configured runtime paths" + ); + } } diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index 4887a0be4de1..9972cc004a78 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -49,13 +49,13 @@ enum ProcessEventSnapshot { async fn create_process_context(use_remote: bool) -> Result { if use_remote { let server = exec_server().await?; - let environment = Environment::create(Some(server.websocket_url().to_string()))?; + let environment = Environment::create_for_tests(Some(server.websocket_url().to_string()))?; Ok(ProcessContext { backend: environment.get_exec_backend(), server: Some(server), }) } else { - let environment = Environment::create(/*exec_server_url*/ None)?; + let environment = Environment::create_for_tests(/*exec_server_url*/ None)?; Ok(ProcessContext { backend: environment.get_exec_backend(), server: None, diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index 4bb654198f91..f137be969580 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -46,7 +46,7 @@ struct FileSystemContext { async fn create_file_system_context(use_remote: bool) -> Result { if use_remote { let server = exec_server().await?; - let environment = Environment::create(Some(server.websocket_url().to_string()))?; + let environment = Environment::create_for_tests(Some(server.websocket_url().to_string()))?; Ok(FileSystemContext { file_system: environment.get_filesystem(), _helper_paths: None, @@ -214,7 +214,7 @@ async fn sandboxed_file_system_helper_finds_bwrap_on_preserved_path() -> Result< let helper_path = std::env::join_paths(path_entries)?; let server = exec_server_with_env([("PATH", helper_path.as_os_str())]).await?; - let environment = Environment::create(Some(server.websocket_url().to_string()))?; + let environment = Environment::create_for_tests(Some(server.websocket_url().to_string()))?; let file_system = environment.get_filesystem(); let workspace = tmp.path().join("workspace"); std::fs::create_dir_all(&workspace)?; diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 1404348af193..1279532daf04 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -498,10 +498,9 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result cloud_requirements: run_cloud_requirements, feedback: CodexFeedback::new(), log_db: None, - environment_manager: std::sync::Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { - local_runtime_paths: Some(local_runtime_paths), - ..EnvironmentManagerArgs::default() - })), + environment_manager: std::sync::Arc::new(EnvironmentManager::new( + EnvironmentManagerArgs::from_env(local_runtime_paths), + )), config_warnings, session_source: SessionSource::Exec, enable_codex_api_key_env: true, diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index 7577818fc4af..1d904e4577a0 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -60,13 +60,12 @@ pub async fn run_main( arg0_paths: Arg0DispatchPaths, cli_config_overrides: CliConfigOverrides, ) -> IoResult<()> { - let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { - local_runtime_paths: Some(ExecServerRuntimePaths::from_optional_paths( + let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::from_env( + ExecServerRuntimePaths::from_optional_paths( arg0_paths.codex_self_exe.clone(), arg0_paths.codex_linux_sandbox_exe.clone(), - )?), - ..EnvironmentManagerArgs::default() - })); + )?, + ))); // Parse CLI overrides once and derive the base Config eagerly so later // components do not need to work with raw TOML values. let cli_kv_overrides = cli_config_overrides.parse_overrides().map_err(|e| { diff --git a/codex-rs/tui/src/app/test_support.rs b/codex-rs/tui/src/app/test_support.rs index a48739dedc43..4dc724ee5e1f 100644 --- a/codex-rs/tui/src/app/test_support.rs +++ b/codex-rs/tui/src/app/test_support.rs @@ -38,9 +38,7 @@ pub(super) async fn make_test_app() -> App { backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, - environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + environment_manager: Arc::new(EnvironmentManager::default_for_tests()), remote_app_server_url: None, remote_app_server_auth_token: None, pending_update_action: None, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 5237ce7d9325..c498d3d41d78 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -3576,9 +3576,7 @@ async fn make_test_app() -> App { backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, - environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + environment_manager: Arc::new(EnvironmentManager::default_for_tests()), remote_app_server_url: None, remote_app_server_auth_token: None, pending_update_action: None, @@ -3635,9 +3633,7 @@ async fn make_test_app_with_channels() -> ( backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), feedback_audience: FeedbackAudience::External, - environment_manager: Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + environment_manager: Arc::new(EnvironmentManager::default_for_tests()), remote_app_server_url: None, remote_app_server_auth_token: None, pending_update_action: None, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 930adeb57dba..7e33f2e8a3f6 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -426,9 +426,7 @@ pub(crate) async fn start_embedded_app_server_for_picker( start_app_server_for_picker( config, &AppServerTarget::Embedded, - Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(EnvironmentManager::default_for_tests()), ) .await } @@ -731,13 +729,12 @@ pub async fn run_main( } }; - let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { - local_runtime_paths: Some(ExecServerRuntimePaths::from_optional_paths( + let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::from_env( + ExecServerRuntimePaths::from_optional_paths( arg0_paths.codex_self_exe.clone(), arg0_paths.codex_linux_sandbox_exe.clone(), - )?), - ..EnvironmentManagerArgs::default() - })); + )?, + ))); let cwd = cli.cwd.clone(); let config_cwd = config_cwd_for_app_server_target(cwd.as_deref(), &app_server_target, &environment_manager)?; @@ -1777,9 +1774,7 @@ mod tests { CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), /*log_db*/ None, - Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(EnvironmentManager::default_for_tests()), ) .await } @@ -1939,8 +1934,7 @@ mod tests { websocket_url: "ws://127.0.0.1:1234/".to_string(), auth_token: None, }; - let environment_manager = - EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default_for_tests()); + let environment_manager = EnvironmentManager::default_for_tests(); let config_cwd = config_cwd_for_app_server_target(Some(remote_only_cwd), &target, &environment_manager)?; @@ -1954,8 +1948,7 @@ mod tests { { let temp_dir = TempDir::new()?; let target = AppServerTarget::Embedded; - let environment_manager = - EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default_for_tests()); + let environment_manager = EnvironmentManager::default_for_tests(); let config_cwd = config_cwd_for_app_server_target(Some(temp_dir.path()), &target, &environment_manager)?; @@ -1975,8 +1968,7 @@ mod tests { let temp_dir = TempDir::new()?; let missing = temp_dir.path().join("missing"); let target = AppServerTarget::Embedded; - let environment_manager = - EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs::default_for_tests()); + let environment_manager = EnvironmentManager::default_for_tests(); let err = config_cwd_for_app_server_target(Some(&missing), &target, &environment_manager) .expect_err("missing embedded cwd should fail"); @@ -1997,7 +1989,10 @@ mod tests { let environment_manager = EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs { exec_server_url: Some("ws://127.0.0.1:8765".to_string()), - local_runtime_paths: None, + local_runtime_paths: ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + )?, }); let config_cwd = @@ -2125,9 +2120,7 @@ mod tests { CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), /*log_db*/ None, - Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs::default_for_tests(), - )), + Arc::new(EnvironmentManager::default_for_tests()), |_args| async { Err(std::io::Error::other("boom")) }, ) .await; diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 380f13580a85..1e55b5c5d2e1 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -989,9 +989,9 @@ mod tests { ), feedback: codex_feedback::CodexFeedback::new(), log_db: None, - environment_manager: Arc::new(codex_app_server_client::EnvironmentManager::new( - codex_app_server_client::EnvironmentManagerArgs::default_for_tests(), - )), + environment_manager: Arc::new( + codex_app_server_client::EnvironmentManager::default_for_tests(), + ), config_warnings: Vec::new(), session_source: SessionSource::Cli, enable_codex_api_key_env: false, From 17a7058f829fb833296b742f55bcf06f3df7af25 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 21 Apr 2026 12:47:17 -0700 Subject: [PATCH 17/20] Drop unused exec-server env var re-export Remove the app-server-client re-export now that environment-manager construction owns CODEX_EXEC_SERVER_URL reading directly in exec-server. Co-authored-by: Codex --- codex-rs/app-server-client/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index fdd90f9281c1..1c3de1208b15 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -45,7 +45,6 @@ use codex_config::NoopThreadConfigLoader; use codex_core::config::Config; use codex_core::config_loader::CloudRequirementsLoader; use codex_core::config_loader::LoaderOverrides; -pub use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; pub use codex_exec_server::EnvironmentManager; pub use codex_exec_server::EnvironmentManagerArgs; pub use codex_exec_server::ExecServerRuntimePaths; From 2d20ced4c705d01856ffb734d8da9fdcc39e71d7 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 21 Apr 2026 13:00:40 -0700 Subject: [PATCH 18/20] Reuse EnvironmentManager for app-server connectors Add a connector loading helper that accepts the existing EnvironmentManager and switch app-server paths to use it. Keep the config-only helper as a temporary fallback for callers such as TUI that do not yet pass the manager through. Co-authored-by: Codex --- .../app-server/src/codex_message_processor.rs | 30 ++++++++++++------- .../plugin_app_helpers.rs | 8 +++-- codex-rs/app-server/src/message_processor.rs | 9 ++++-- codex-rs/chatgpt/src/connectors.rs | 1 + codex-rs/core/src/connectors.rs | 28 +++++++++++++---- 5 files changed, 55 insertions(+), 21 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 59189dc07abb..3316cb94154c 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -6244,13 +6244,17 @@ impl CodexMessageProcessor { let accessible_config = config.clone(); let accessible_tx = tx.clone(); + let environment_manager = self.thread_manager.environment_manager(); tokio::spawn(async move { - let result = connectors::list_accessible_connectors_from_mcp_tools_with_options( - &accessible_config, - force_refetch, - ) - .await - .map_err(|err| format!("failed to load accessible apps: {err}")); + let result = + connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( + &accessible_config, + force_refetch, + &environment_manager, + ) + .await + .map(|status| status.connectors) + .map_err(|err| format!("failed to load accessible apps: {err}")); let _ = accessible_tx.send(AppListLoadResult::Accessible(result)); }); @@ -6760,8 +6764,13 @@ impl CodexMessageProcessor { return; } }; - let app_summaries = - plugin_app_helpers::load_plugin_app_summaries(&config, &outcome.plugin.apps).await; + let environment_manager = self.thread_manager.environment_manager(); + let app_summaries = plugin_app_helpers::load_plugin_app_summaries( + &config, + &outcome.plugin.apps, + &environment_manager, + ) + .await; let visible_skills = outcome .plugin .skills @@ -6938,10 +6947,11 @@ impl CodexMessageProcessor { ) { Vec::new() } else { + let environment_manager = self.thread_manager.environment_manager(); let (all_connectors_result, accessible_connectors_result) = tokio::join!( connectors::list_all_connectors_with_options(&config, /*force_refetch*/ true), - connectors::list_accessible_connectors_from_mcp_tools_with_options_and_status( - &config, /*force_refetch*/ true + connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( + &config, /*force_refetch*/ true, &environment_manager ), ); diff --git a/codex-rs/app-server/src/codex_message_processor/plugin_app_helpers.rs b/codex-rs/app-server/src/codex_message_processor/plugin_app_helpers.rs index f2ba96d43acf..ad5875608b0f 100644 --- a/codex-rs/app-server/src/codex_message_processor/plugin_app_helpers.rs +++ b/codex-rs/app-server/src/codex_message_processor/plugin_app_helpers.rs @@ -5,11 +5,13 @@ use codex_app_server_protocol::AppSummary; use codex_chatgpt::connectors; use codex_core::config::Config; use codex_core::plugins::AppConnectorId; +use codex_exec_server::EnvironmentManager; use tracing::warn; pub(super) async fn load_plugin_app_summaries( config: &Config, plugin_apps: &[AppConnectorId], + environment_manager: &EnvironmentManager, ) -> Vec { if plugin_apps.is_empty() { return Vec::new(); @@ -29,8 +31,10 @@ pub(super) async fn load_plugin_app_summaries( let plugin_connectors = connectors::connectors_for_plugin_apps(connectors, plugin_apps); let accessible_connectors = - match connectors::list_accessible_connectors_from_mcp_tools_with_options_and_status( - config, /*force_refetch*/ false, + match connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( + config, + /*force_refetch*/ false, + environment_manager, ) .await { diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 54e6dbe160dd..57fa6e21e0c7 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -1084,11 +1084,14 @@ impl MessageProcessor { } let outgoing = Arc::clone(&self.outgoing); + let environment_manager = self.thread_manager.environment_manager(); tokio::spawn(async move { let (all_connectors_result, accessible_connectors_result) = tokio::join!( connectors::list_all_connectors_with_options(&config, /*force_refetch*/ true), - connectors::list_accessible_connectors_from_mcp_tools_with_options( - &config, /*force_refetch*/ true, + connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( + &config, + /*force_refetch*/ true, + &environment_manager, ), ); let all_connectors = match all_connectors_result { @@ -1101,7 +1104,7 @@ impl MessageProcessor { } }; let accessible_connectors = match accessible_connectors_result { - Ok(connectors) => connectors, + Ok(status) => status.connectors, Err(err) => { tracing::warn!( "failed to force-refresh accessible apps after experimental feature enablement: {err:#}" diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index 5f6efbc124c1..c054d1b8df82 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -13,6 +13,7 @@ use codex_connectors::merge::merge_connectors; use codex_connectors::merge::merge_plugin_connectors; use codex_core::config::Config; pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools; +pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager; pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_options; pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_options_and_status; pub use codex_core::connectors::list_cached_accessible_connectors_from_mcp_tools; diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index befb7fe561b2..7641b4cb620e 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -192,6 +192,28 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options( pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( config: &Config, force_refetch: bool, +) -> anyhow::Result { + // TODO: Wire callers that already own an EnvironmentManager into + // list_accessible_connectors_from_mcp_tools_with_environment_manager instead + // of constructing a temporary manager here. + let local_runtime_paths = ExecServerRuntimePaths::from_optional_paths( + config.codex_self_exe.clone(), + config.codex_linux_sandbox_exe.clone(), + )?; + let environment_manager = + EnvironmentManager::new(EnvironmentManagerArgs::from_env(local_runtime_paths)); + list_accessible_connectors_from_mcp_tools_with_environment_manager( + config, + force_refetch, + &environment_manager, + ) + .await +} + +pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager( + config: &Config, + force_refetch: bool, + environment_manager: &EnvironmentManager, ) -> anyhow::Result { let auth_manager = AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); @@ -237,12 +259,6 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( let (tx_event, rx_event) = unbounded(); drop(rx_event); - let local_runtime_paths = ExecServerRuntimePaths::from_optional_paths( - config.codex_self_exe.clone(), - config.codex_linux_sandbox_exe.clone(), - )?; - let environment_manager = - EnvironmentManager::new(EnvironmentManagerArgs::from_env(local_runtime_paths)); let environment = environment_manager .default_environment() .unwrap_or_else(|| environment_manager.local_environment()); From 933e64424d7db3ffaed959bd8d4b350ccae3ec98 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 21 Apr 2026 13:11:32 -0700 Subject: [PATCH 19/20] Pass environment manager to app list task Co-authored-by: Codex --- codex-rs/app-server/src/codex_message_processor.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 3316cb94154c..8f31288cd54d 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -260,6 +260,7 @@ use codex_core_plugins::loader::load_plugin_mcp_servers; use codex_core_plugins::manifest::PluginManifestInterface; use codex_core_plugins::marketplace::MarketplaceError; use codex_core_plugins::marketplace::MarketplacePluginSource; +use codex_exec_server::EnvironmentManager; use codex_exec_server::LOCAL_FS; use codex_features::FEATURES; use codex_features::Feature; @@ -6201,8 +6202,9 @@ impl CodexMessageProcessor { let request = request_id.clone(); let outgoing = Arc::clone(&self.outgoing); + let environment_manager = self.thread_manager.environment_manager(); tokio::spawn(async move { - Self::apps_list_task(outgoing, request, params, config).await; + Self::apps_list_task(outgoing, request, params, config, environment_manager).await; }); } @@ -6211,6 +6213,7 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: AppsListParams, config: Config, + environment_manager: Arc, ) { let AppsListParams { cursor, @@ -6244,7 +6247,6 @@ impl CodexMessageProcessor { let accessible_config = config.clone(); let accessible_tx = tx.clone(); - let environment_manager = self.thread_manager.environment_manager(); tokio::spawn(async move { let result = connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( From 3ec48a2375c9d8a95c9927ac47830347da294f97 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 21 Apr 2026 13:52:20 -0700 Subject: [PATCH 20/20] Avoid expect in local environment lookup Co-authored-by: Codex --- codex-rs/exec-server/src/environment.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 9cc18ea7be22..8e10c4a34ebb 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -115,8 +115,10 @@ impl EnvironmentManager { /// Returns the local environment instance used for internal runtime work. pub fn local_environment(&self) -> Arc { - self.get_environment(LOCAL_ENVIRONMENT_ID) - .expect("EnvironmentManager always has a local environment") + match self.get_environment(LOCAL_ENVIRONMENT_ID) { + Some(environment) => environment, + None => unreachable!("EnvironmentManager always has a local environment"), + } } /// Returns a named environment instance.