diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 66547e1a9c..08fb559d28 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -5698,7 +5698,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()) @@ -5855,7 +5856,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()) @@ -6449,8 +6451,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 fcc66da7f4..9031e776eb 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -461,7 +461,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 1c5c1c9567..70cbcb2fd2 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -641,8 +641,6 @@ impl Session { Arc::clone(&auth_manager), session_configuration.session_source.clone(), )); - 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 @@ -698,8 +696,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 1da7ad13e7..48814319eb 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -3245,8 +3245,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(), @@ -3280,7 +3278,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), @@ -4346,8 +4343,6 @@ where 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(), @@ -4381,7 +4376,6 @@ where 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 58fcfc73b8..ce6758c442 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 29fb5eb1d2..94e17eb157 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -17,7 +17,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; @@ -68,6 +67,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 da109bbcc7..c5da648bf5 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 0993b5b0e4..67d1a49968 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 a995e54431..a6166f8403 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 53a737dc57..d8e532e197 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 98369a0bbd..74d692ebb6 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);