diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 758b59204..c7923beaf 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -862,7 +862,7 @@ pub(crate) struct TurnContext { pub(crate) reasoning_effort: Option, pub(crate) reasoning_summary: ReasoningSummaryConfig, pub(crate) session_source: SessionSource, - pub(crate) environment: Arc, + pub(crate) environment: Option>, /// The session's absolute working directory. All relative paths provided /// by the model as well as sandbox policies are resolved against this path /// instead of `std::env::current_dir()`. @@ -958,6 +958,7 @@ impl TurnContext { .with_unified_exec_shell_mode(self.tools_config.unified_exec_shell_mode.clone()) .with_web_search_config(self.tools_config.web_search_config.clone()) .with_allow_login_shell(self.tools_config.allow_login_shell) + .with_has_environment(self.tools_config.has_environment) .with_agent_type_description(crate::agent::role::spawn_tool_spec::build( &config.agent_roles, )); @@ -977,7 +978,7 @@ impl TurnContext { reasoning_effort, reasoning_summary: self.reasoning_summary, session_source: self.session_source.clone(), - environment: Arc::clone(&self.environment), + environment: self.environment.clone(), cwd: self.cwd.clone(), current_date: self.current_date.clone(), timezone: self.timezone.clone(), @@ -1406,7 +1407,7 @@ impl Session { model_info: ModelInfo, models_manager: &ModelsManager, network: Option, - environment: Arc, + environment: Option>, sub_id: String, js_repl: Arc, skills_outcome: Arc, @@ -1439,6 +1440,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_agent_type_description(crate::agent::role::spawn_tool_spec::build( &per_turn_config.agent_roles, )); @@ -2542,7 +2544,7 @@ impl Session { .network_proxy .as_ref() .map(StartedNetworkProxy::proxy), - Arc::clone(&self.services.environment), + self.services.environment.clone(), sub_id, Arc::clone(&self.js_repl), skills_outcome, @@ -5614,6 +5616,7 @@ 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_agent_type_description(crate::agent::role::spawn_tool_spec::build( &config.agent_roles, )); @@ -5672,7 +5675,7 @@ async fn spawn_review_thread( reasoning_effort, reasoning_summary, session_source, - environment: Arc::clone(&parent_turn_context.environment), + environment: parent_turn_context.environment.clone(), tools_config, features: parent_turn_context.features.clone(), ghost_snapshot: parent_turn_context.ghost_snapshot.clone(), diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 93d3eba02..ab6324efa 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -78,8 +78,8 @@ pub(crate) async fn run_codex_thread_interactive( config, auth_manager, models_manager, - environment_manager: Arc::new(EnvironmentManager::new( - parent_ctx.environment.exec_server_url().map(str::to_owned), + environment_manager: Arc::new(EnvironmentManager::from_environment( + parent_ctx.environment.as_deref(), )), skills_manager: Arc::clone(&parent_session.services.skills_manager), plugins_manager: Arc::clone(&parent_session.services.plugins_manager), diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 3a7ceb6f8..88a8902f8 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -2770,7 +2770,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: Arc::clone(&environment), + environment: Some(Arc::clone(&environment)), }; let js_repl = Arc::new(JsReplHandle::with_node_path( config.js_repl_node_path.clone(), @@ -2797,7 +2797,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { model_info, &models_manager, /*network*/ None, - environment, + Some(environment), "turn_id".to_string(), Arc::clone(&js_repl), skills_outcome, @@ -3610,7 +3610,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: Arc::clone(&environment), + environment: Some(Arc::clone(&environment)), }; let js_repl = Arc::new(JsReplHandle::with_node_path( config.js_repl_node_path.clone(), @@ -3637,7 +3637,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( model_info, &models_manager, /*network*/ None, - environment, + Some(environment), "turn_id".to_string(), Arc::clone(&js_repl), skills_outcome, diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index e0d7741f4..a0600ddce 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -57,5 +57,5 @@ 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: Arc, + pub(crate) environment: Option>, } diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index ec34fafb3..5fa005bac 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -91,9 +91,13 @@ impl ToolHandler for ViewImageHandler { AbsolutePathBuf::try_from(turn.resolve_path(Some(args.path))).map_err(|error| { FunctionCallError::RespondToModel(format!("unable to resolve image path: {error}")) })?; + let Some(environment) = turn.environment.as_ref() else { + return Err(FunctionCallError::RespondToModel( + "view_image is unavailable in this session".to_string(), + )); + }; - let metadata = turn - .environment + let metadata = environment .get_filesystem() .get_metadata(&abs_path) .await @@ -110,8 +114,7 @@ impl ToolHandler for ViewImageHandler { abs_path.display() ))); } - let file_bytes = turn - .environment + let file_bytes = environment .get_filesystem() .read_file(&abs_path) .await diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 86610bbb2..3858150da 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -239,7 +239,12 @@ impl<'a> ToolRuntime for UnifiedExecRunt .await? { Some(prepared) => { - if ctx.turn.environment.exec_server_url().is_some() { + let Some(environment) = ctx.turn.environment.as_ref() else { + return Err(ToolError::Rejected( + "exec_command is unavailable in this session".to_string(), + )); + }; + if environment.is_remote() { return Err(ToolError::Rejected( "unified_exec zsh-fork is not supported when exec_server_url is configured".to_string(), )); @@ -251,7 +256,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt &prepared.exec_request, req.tty, prepared.spawn_lifecycle, - ctx.turn.environment.as_ref(), + environment.as_ref(), ) .await .map_err(|err| match err { @@ -281,13 +286,18 @@ impl<'a> ToolRuntime for UnifiedExecRunt let exec_env = attempt .env_for(command, options, req.network.as_ref()) .map_err(|err| ToolError::Codex(err.into()))?; + let Some(environment) = ctx.turn.environment.as_ref() else { + return Err(ToolError::Rejected( + "exec_command is unavailable in this session".to_string(), + )); + }; self.manager .open_session_with_exec_env( req.process_id, &exec_env, req.tty, Box::new(NoopSpawnLifecycle), - ctx.turn.environment.as_ref(), + environment.as_ref(), ) .await .map_err(|err| match err { diff --git a/codex-rs/core/src/unified_exec/mod_tests.rs b/codex-rs/core/src/unified_exec/mod_tests.rs index 8348614f9..a5db73bf4 100644 --- a/codex-rs/core/src/unified_exec/mod_tests.rs +++ b/codex-rs/core/src/unified_exec/mod_tests.rs @@ -98,7 +98,7 @@ async fn exec_command_with_tty( &request, tty, Box::new(NoopSpawnLifecycle), - turn.environment.as_ref(), + turn.environment.as_ref().expect("turn environment"), ) .await?, ); @@ -593,7 +593,7 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<() let remote_test_env = remote_test_env().await?; let (_, mut turn) = make_session_and_context().await; - turn.environment = Arc::new(remote_test_env.environment().clone()); + turn.environment = Some(Arc::new(remote_test_env.environment().clone())); let request = test_exec_request( &turn, @@ -611,7 +611,7 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<() Box::new(TestSpawnLifecycle { inherited_fds: vec![42], }), - turn.environment.as_ref(), + turn.environment.as_ref().expect("turn environment"), ) .await .expect_err("expected inherited fd rejection"); diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index a3465f7d6..affb72b69 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -593,7 +593,7 @@ impl UnifiedExecProcessManager { .ok_or(UnifiedExecError::MissingCommandLine)?; let inherited_fds = spawn_lifecycle.inherited_fds(); - if environment.exec_server_url().is_some() { + if environment.is_remote() { if !inherited_fds.is_empty() { return Err(UnifiedExecError::create_process( "remote exec-server does not support inherited file descriptors".to_string(), diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 0518612c3..c76ac431e 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -14,44 +14,83 @@ use crate::remote_process::RemoteProcess; pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; -pub trait ExecutorEnvironment: Send + Sync { - fn get_exec_backend(&self) -> Arc; -} - -#[derive(Debug, Default)] +/// Lazily creates and caches the active environment for a session. +/// +/// The manager keeps the session's environment selection stable so subagents +/// and follow-up turns preserve an explicit disabled state. +#[derive(Debug)] pub struct EnvironmentManager { exec_server_url: Option, - current_environment: OnceCell>, + disabled: bool, + current_environment: OnceCell>>, +} + +impl Default for EnvironmentManager { + fn default() -> Self { + Self::new(/*exec_server_url*/ None) + } } impl EnvironmentManager { + /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value. pub fn new(exec_server_url: Option) -> Self { + let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url); Self { - exec_server_url: normalize_exec_server_url(exec_server_url), + exec_server_url, + disabled, current_environment: OnceCell::new(), } } + /// Builds a manager from process environment variables. pub fn from_env() -> Self { Self::new(std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok()) } + /// 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), + disabled: false, + current_environment: OnceCell::new(), + }, + None => Self { + exec_server_url: None, + disabled: true, + current_environment: OnceCell::new(), + }, + } + } + + /// Returns the remote exec-server URL when one is configured. pub fn exec_server_url(&self) -> Option<&str> { self.exec_server_url.as_deref() } - pub async fn current(&self) -> Result, ExecServerError> { + /// Returns the cached environment, creating it on first access. + pub async fn current(&self) -> Result>, ExecServerError> { self.current_environment .get_or_try_init(|| async { - Ok(Arc::new( - Environment::create(self.exec_server_url.clone()).await?, - )) + if self.disabled { + Ok(None) + } else { + Ok(Some(Arc::new( + Environment::create(self.exec_server_url.clone()).await?, + ))) + } }) .await - .map(Arc::clone) + .map(Option::as_ref) + .map(std::option::Option::<&Arc>::cloned) } } +/// Concrete execution/filesystem environment selected for a session. +/// +/// This bundles the selected backend together with the corresponding remote +/// client, if any. #[derive(Clone)] pub struct Environment { exec_server_url: Option, @@ -86,12 +125,19 @@ 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 { - let exec_server_url = normalize_exec_server_url(exec_server_url); - let remote_exec_server_client = if let Some(url) = &exec_server_url { + let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url); + if disabled { + return Err(ExecServerError::Protocol( + "disabled mode does not create an Environment".to_string(), + )); + } + + let remote_exec_server_client = if let Some(exec_server_url) = &exec_server_url { Some( ExecServerClient::connect_websocket(RemoteExecServerConnectArgs { - websocket_url: url.clone(), + 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), @@ -102,10 +148,14 @@ impl Environment { None }; - let exec_backend: Arc = - if let Some(client) = remote_exec_server_client.clone() { - Arc::new(RemoteProcess::new(client)) - } else { + let exec_backend: Arc = match remote_exec_server_client.clone() { + Some(client) => Arc::new(RemoteProcess::new(client)), + None if exec_server_url.is_some() => { + return Err(ExecServerError::Protocol( + "remote mode should have an exec-server client".to_string(), + )); + } + None => { let local_process = LocalProcess::default(); local_process .initialize() @@ -114,7 +164,8 @@ impl Environment { .initialized() .map_err(ExecServerError::Protocol)?; Arc::new(local_process) - }; + } + }; Ok(Self { exec_server_url, @@ -123,6 +174,11 @@ impl Environment { }) } + pub fn is_remote(&self) -> bool { + self.exec_server_url.is_some() + } + + /// Returns the remote exec-server URL when this environment is remote. pub fn exec_server_url(&self) -> Option<&str> { self.exec_server_url.as_deref() } @@ -132,27 +188,20 @@ impl Environment { } pub fn get_filesystem(&self) -> Arc { - if let Some(client) = self.remote_exec_server_client.clone() { - Arc::new(RemoteFileSystem::new(client)) - } else { - Arc::new(LocalFileSystem) + match self.remote_exec_server_client.clone() { + Some(client) => Arc::new(RemoteFileSystem::new(client)), + None => Arc::new(LocalFileSystem), } } } -fn normalize_exec_server_url(exec_server_url: Option) -> Option { - exec_server_url.and_then(|url| { - let url = url.trim(); - (!url.is_empty()).then(|| url.to_string()) - }) -} - -impl ExecutorEnvironment for Environment { - fn get_exec_backend(&self) -> Arc { - Arc::clone(&self.exec_backend) +fn normalize_exec_server_url(exec_server_url: Option) -> (Option, bool) { + match exec_server_url.as_deref().map(str::trim) { + None | Some("") => (None, false), + Some(url) if url.eq_ignore_ascii_case("none") => (None, true), + Some(url) => (Some(url.to_string()), false), } } - #[cfg(test)] mod tests { use std::sync::Arc; @@ -163,7 +212,7 @@ mod tests { use pretty_assertions::assert_eq; #[tokio::test] - async fn create_without_remote_exec_server_url_does_not_connect() { + async fn create_local_environment_does_not_connect() { let environment = Environment::create(/*exec_server_url*/ None) .await .expect("create environment"); @@ -176,6 +225,15 @@ mod tests { fn environment_manager_normalizes_empty_url() { let manager = EnvironmentManager::new(Some(String::new())); + assert!(!manager.disabled); + assert_eq!(manager.exec_server_url(), None); + } + + #[test] + fn environment_manager_treats_none_value_as_disabled() { + let manager = EnvironmentManager::new(Some("none".to_string())); + + assert!(manager.disabled); assert_eq!(manager.exec_server_url(), None); } @@ -186,9 +244,25 @@ mod tests { let first = manager.current().await.expect("get current environment"); let second = manager.current().await.expect("get current environment"); + let first = first.expect("local environment"); + let second = second.expect("local environment"); + assert!(Arc::ptr_eq(&first, &second)); } + #[tokio::test] + async fn disabled_environment_manager_has_no_current_environment() { + let manager = EnvironmentManager::new(Some("none".to_string())); + + assert!( + manager + .current() + .await + .expect("get current environment") + .is_none() + ); + } + #[tokio::test] async fn default_environment_has_ready_local_executor() { let environment = Environment::default(); diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index 77834c614..ab27ab239 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -34,7 +34,6 @@ pub use codex_app_server_protocol::FsWriteFileResponse; pub use environment::CODEX_EXEC_SERVER_URL_ENV_VAR; pub use environment::Environment; pub use environment::EnvironmentManager; -pub use environment::ExecutorEnvironment; pub use file_system::CopyOptions; pub use file_system::CreateDirectoryOptions; pub use file_system::ExecutorFileSystem; diff --git a/codex-rs/tools/src/tool_config.rs b/codex-rs/tools/src/tool_config.rs index f48f1b8fc..6e9e0d1f5 100644 --- a/codex-rs/tools/src/tool_config.rs +++ b/codex-rs/tools/src/tool_config.rs @@ -86,6 +86,7 @@ pub struct ToolsConfig { pub shell_type: ConfigShellToolType, pub shell_command_backend: ShellCommandBackendConfig, pub unified_exec_shell_mode: UnifiedExecShellMode, + pub has_environment: bool, pub allow_login_shell: bool, pub apply_patch_tool_type: Option, pub web_search_mode: Option, @@ -200,6 +201,7 @@ impl ToolsConfig { shell_type, shell_command_backend, unified_exec_shell_mode: UnifiedExecShellMode::Direct, + has_environment: true, allow_login_shell: true, apply_patch_tool_type, web_search_mode: *web_search_mode, @@ -236,6 +238,11 @@ impl ToolsConfig { self } + pub fn with_has_environment(mut self, has_environment: bool) -> Self { + self.has_environment = has_environment; + self + } + pub fn with_unified_exec_shell_mode( mut self, unified_exec_shell_mode: UnifiedExecShellMode, diff --git a/codex-rs/tools/src/tool_registry_plan.rs b/codex-rs/tools/src/tool_registry_plan.rs index cc706f24f..8985e2ee8 100644 --- a/codex-rs/tools/src/tool_registry_plan.rs +++ b/codex-rs/tools/src/tool_registry_plan.rs @@ -107,54 +107,56 @@ pub fn build_tool_registry_plan( ); } - match &config.shell_type { - ConfigShellToolType::Default => { - plan.push_spec( - create_shell_tool(ShellToolOptions { - exec_permission_approvals_enabled, - }), - /*supports_parallel_tool_calls*/ true, - config.code_mode_enabled, - ); - } - ConfigShellToolType::Local => { - plan.push_spec( - create_local_shell_tool(), - /*supports_parallel_tool_calls*/ true, - config.code_mode_enabled, - ); - } - ConfigShellToolType::UnifiedExec => { - plan.push_spec( - create_exec_command_tool(CommandToolOptions { - allow_login_shell: config.allow_login_shell, - exec_permission_approvals_enabled, - }), - /*supports_parallel_tool_calls*/ true, - config.code_mode_enabled, - ); - plan.push_spec( - create_write_stdin_tool(), - /*supports_parallel_tool_calls*/ false, - config.code_mode_enabled, - ); - plan.register_handler("exec_command", ToolHandlerKind::UnifiedExec); - plan.register_handler("write_stdin", ToolHandlerKind::UnifiedExec); - } - ConfigShellToolType::Disabled => {} - ConfigShellToolType::ShellCommand => { - plan.push_spec( - create_shell_command_tool(CommandToolOptions { - allow_login_shell: config.allow_login_shell, - exec_permission_approvals_enabled, - }), - /*supports_parallel_tool_calls*/ true, - config.code_mode_enabled, - ); + if config.has_environment { + match &config.shell_type { + ConfigShellToolType::Default => { + plan.push_spec( + create_shell_tool(ShellToolOptions { + exec_permission_approvals_enabled, + }), + /*supports_parallel_tool_calls*/ true, + config.code_mode_enabled, + ); + } + ConfigShellToolType::Local => { + plan.push_spec( + create_local_shell_tool(), + /*supports_parallel_tool_calls*/ true, + config.code_mode_enabled, + ); + } + ConfigShellToolType::UnifiedExec => { + plan.push_spec( + create_exec_command_tool(CommandToolOptions { + allow_login_shell: config.allow_login_shell, + exec_permission_approvals_enabled, + }), + /*supports_parallel_tool_calls*/ true, + config.code_mode_enabled, + ); + plan.push_spec( + create_write_stdin_tool(), + /*supports_parallel_tool_calls*/ false, + config.code_mode_enabled, + ); + plan.register_handler("exec_command", ToolHandlerKind::UnifiedExec); + plan.register_handler("write_stdin", ToolHandlerKind::UnifiedExec); + } + ConfigShellToolType::Disabled => {} + ConfigShellToolType::ShellCommand => { + plan.push_spec( + create_shell_command_tool(CommandToolOptions { + allow_login_shell: config.allow_login_shell, + exec_permission_approvals_enabled, + }), + /*supports_parallel_tool_calls*/ true, + config.code_mode_enabled, + ); + } } } - if config.shell_type != ConfigShellToolType::Disabled { + if config.has_environment && config.shell_type != ConfigShellToolType::Disabled { plan.register_handler("shell", ToolHandlerKind::Shell); plan.register_handler("container.exec", ToolHandlerKind::Shell); plan.register_handler("local_shell", ToolHandlerKind::Shell); @@ -189,7 +191,7 @@ pub fn build_tool_registry_plan( ); plan.register_handler("update_plan", ToolHandlerKind::Plan); - if config.js_repl_enabled { + if config.has_environment && config.js_repl_enabled { plan.push_spec( create_js_repl_tool(), /*supports_parallel_tool_calls*/ false, @@ -265,7 +267,9 @@ pub fn build_tool_registry_plan( plan.register_handler(TOOL_SUGGEST_TOOL_NAME, ToolHandlerKind::ToolSuggest); } - if let Some(apply_patch_tool_type) = &config.apply_patch_tool_type { + if config.has_environment + && let Some(apply_patch_tool_type) = &config.apply_patch_tool_type + { match apply_patch_tool_type { ApplyPatchToolType::Freeform => { plan.push_spec( @@ -285,10 +289,11 @@ pub fn build_tool_registry_plan( plan.register_handler("apply_patch", ToolHandlerKind::ApplyPatch); } - if config - .experimental_supported_tools - .iter() - .any(|tool| tool == "list_dir") + if config.has_environment + && config + .experimental_supported_tools + .iter() + .any(|tool| tool == "list_dir") { plan.push_spec( create_list_dir_tool(), @@ -331,14 +336,16 @@ pub fn build_tool_registry_plan( ); } - plan.push_spec( - create_view_image_tool(ViewImageToolOptions { - can_request_original_image_detail: config.can_request_original_image_detail, - }), - /*supports_parallel_tool_calls*/ true, - config.code_mode_enabled, - ); - plan.register_handler("view_image", ToolHandlerKind::ViewImage); + if config.has_environment { + plan.push_spec( + create_view_image_tool(ViewImageToolOptions { + can_request_original_image_detail: config.can_request_original_image_detail, + }), + /*supports_parallel_tool_calls*/ true, + config.code_mode_enabled, + ); + plan.register_handler("view_image", ToolHandlerKind::ViewImage); + } if config.collab_tools { if config.multi_agent_v2 { diff --git a/codex-rs/tools/src/tool_registry_plan_tests.rs b/codex-rs/tools/src/tool_registry_plan_tests.rs index fd7ab1007..c73302d1b 100644 --- a/codex-rs/tools/src/tool_registry_plan_tests.rs +++ b/codex-rs/tools/src/tool_registry_plan_tests.rs @@ -453,6 +453,42 @@ fn view_image_tool_includes_detail_with_original_detail_feature() { assert!(description.contains("omit this field for default resized behavior")); } +#[test] +fn disabled_environment_omits_environment_backed_tools() { + let model_info = model_info(); + let mut features = Features::with_defaults(); + features.enable(Feature::UnifiedExec); + features.enable(Feature::JsRepl); + let available_models = Vec::new(); + let mut tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + available_models: &available_models, + features: &features, + web_search_mode: Some(WebSearchMode::Cached), + session_source: SessionSource::Cli, + sandbox_policy: &SandboxPolicy::DangerFullAccess, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + }) + .with_has_environment(/*has_environment*/ false); + tools_config + .experimental_supported_tools + .push("list_dir".to_string()); + let (tools, _) = build_specs( + &tools_config, + /*mcp_tools*/ None, + /*app_tools*/ None, + &[], + ); + + assert_lacks_tool_name(&tools, "exec_command"); + assert_lacks_tool_name(&tools, "write_stdin"); + assert_lacks_tool_name(&tools, "js_repl"); + assert_lacks_tool_name(&tools, "js_repl_reset"); + assert_lacks_tool_name(&tools, "apply_patch"); + assert_lacks_tool_name(&tools, "list_dir"); + assert_lacks_tool_name(&tools, VIEW_IMAGE_TOOL_NAME); +} + #[test] fn test_build_specs_agent_job_worker_tools_enabled() { let model_info = model_info();