diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index a83e0b01e5..a6f3cf4ced 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -14,7 +14,6 @@ use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolExposure; use crate::tools::registry::ToolTelemetryTags; use crate::tools::tool_search_entry::ToolSearchInfo; use codex_mcp::ToolInfo; @@ -30,24 +29,12 @@ use serde_json::Value; pub struct McpHandler { tool_info: ToolInfo, spec: ToolSpec, - exposure: ToolExposure, } impl McpHandler { pub fn new(tool_info: ToolInfo) -> Result { - Self::with_exposure(tool_info, ToolExposure::Direct) - } - - pub fn with_exposure( - tool_info: ToolInfo, - exposure: ToolExposure, - ) -> Result { let spec = create_tool_spec(&tool_info)?; - Ok(Self { - tool_info, - spec, - exposure, - }) + Ok(Self { tool_info, spec }) } } @@ -61,10 +48,6 @@ impl ToolExecutor for McpHandler { self.spec.clone() } - fn exposure(&self) -> ToolExposure { - self.exposure - } - fn supports_parallel_tool_calls(&self) -> bool { self.tool_info.supports_parallel_tool_calls } diff --git a/codex-rs/core/src/tools/handlers/shell/shell_command.rs b/codex-rs/core/src/tools/handlers/shell/shell_command.rs index ab79510add..e9ec246f18 100644 --- a/codex-rs/core/src/tools/handlers/shell/shell_command.rs +++ b/codex-rs/core/src/tools/handlers/shell/shell_command.rs @@ -22,7 +22,6 @@ use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolExposure; use crate::tools::runtimes::shell::ShellRuntimeBackend; use codex_tools::ToolSpec; @@ -41,7 +40,6 @@ enum ShellCommandBackend { pub struct ShellCommandHandler { backend: ShellCommandBackend, options: ShellCommandHandlerOptions, - exposure: ToolExposure, } #[derive(Clone, Copy)] @@ -53,23 +51,11 @@ pub(crate) struct ShellCommandHandlerOptions { impl ShellCommandHandler { pub(crate) fn new(options: ShellCommandHandlerOptions) -> Self { - Self::with_exposure(options, ToolExposure::Direct) - } - - pub(crate) fn hidden(options: ShellCommandHandlerOptions) -> Self { - Self::with_exposure(options, ToolExposure::Hidden) - } - - fn with_exposure(options: ShellCommandHandlerOptions, exposure: ToolExposure) -> Self { let backend = match options.backend_config { ShellCommandBackendConfig::Classic => ShellCommandBackend::Classic, ShellCommandBackendConfig::ZshFork => ShellCommandBackend::ZshFork, }; - Self { - backend, - options, - exposure, - } + Self { backend, options } } fn shell_runtime_backend(&self) -> ShellRuntimeBackend { @@ -130,7 +116,7 @@ impl ShellCommandHandler { impl From for ShellCommandHandler { fn from(backend_config: ShellCommandBackendConfig) -> Self { - Self::hidden(ShellCommandHandlerOptions { + Self::new(ShellCommandHandlerOptions { backend_config, allow_login_shell: false, exec_permission_approvals_enabled: false, @@ -151,12 +137,8 @@ impl ToolExecutor for ShellCommandHandler { }) } - fn exposure(&self) -> ToolExposure { - self.exposure - } - fn supports_parallel_tool_calls(&self) -> bool { - self.exposure != ToolExposure::Hidden + true } async fn handle( diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index 1171596f82..5b7a17f428 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -14,7 +14,6 @@ pub(crate) mod runtimes; pub(crate) mod sandboxing; pub(crate) mod spec_plan; pub(crate) mod tool_dispatch_trace; -pub(crate) mod tool_family; pub(crate) mod tool_search_entry; use std::borrow::Cow; diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 24782a833a..411ce14359 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -193,7 +193,7 @@ impl ToolExecutor for ExposureOverride { } fn supports_parallel_tool_calls(&self) -> bool { - self.handler.supports_parallel_tool_calls() + self.exposure != ToolExposure::Hidden && self.handler.supports_parallel_tool_calls() } async fn handle( diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index fcbefc1eed..c3656f2434 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -6,6 +6,8 @@ use crate::tools::handlers::CodeModeExecuteHandler; use crate::tools::handlers::CodeModeWaitHandler; use crate::tools::handlers::CreateGoalHandler; use crate::tools::handlers::DynamicToolHandler; +use crate::tools::handlers::ExecCommandHandler; +use crate::tools::handlers::ExecCommandHandlerOptions; use crate::tools::handlers::GetGoalHandler; use crate::tools::handlers::ListAvailablePluginsToInstallHandler; use crate::tools::handlers::ListMcpResourceTemplatesHandler; @@ -16,10 +18,13 @@ use crate::tools::handlers::ReadMcpResourceHandler; use crate::tools::handlers::RequestPermissionsHandler; use crate::tools::handlers::RequestPluginInstallHandler; use crate::tools::handlers::RequestUserInputHandler; +use crate::tools::handlers::ShellCommandHandler; +use crate::tools::handlers::ShellCommandHandlerOptions; use crate::tools::handlers::TestSyncHandler; use crate::tools::handlers::ToolSearchHandler; use crate::tools::handlers::UpdateGoalHandler; use crate::tools::handlers::ViewImageHandler; +use crate::tools::handlers::WriteStdinHandler; use crate::tools::handlers::agent_jobs::ReportAgentJobResultHandler; use crate::tools::handlers::agent_jobs::SpawnAgentsOnCsvHandler; use crate::tools::handlers::extension_tools::ExtensionToolAdapter; @@ -49,12 +54,11 @@ use crate::tools::registry::ToolRegistry; use crate::tools::registry::override_tool_exposure; use crate::tools::router::ToolRouter; use crate::tools::router::ToolRouterParams; -use crate::tools::tool_family::shell::ShellToolsOptions; -use crate::tools::tool_family::shell::build_shell_tools; use codex_features::Feature; use codex_login::AuthManager; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; +use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::InputModality; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; @@ -91,15 +95,34 @@ struct PlannedTools { } impl PlannedTools { - fn add_runtime(&mut self, runtime: T) + fn add(&mut self, handler: T) where T: CoreToolRuntime + 'static, { - self.runtimes.push(Arc::new(runtime)); + self.runtimes.push(Arc::new(handler)); } - fn add_runtime_arc(&mut self, runtime: PlannedRuntime) { - self.runtimes.push(runtime); + fn add_arc(&mut self, handler: PlannedRuntime) { + self.runtimes.push(handler); + } + + fn add_with_exposure(&mut self, handler: T, exposure: ToolExposure) + where + T: CoreToolRuntime + 'static, + { + self.runtimes + .push(override_tool_exposure(Arc::new(handler), exposure)); + } + + fn add_dispatch_only(&mut self, handler: T) + where + T: CoreToolRuntime + 'static, + { + self.add_with_exposure(handler, ToolExposure::Hidden); + } + + fn add_hosted_spec(&mut self, spec: ToolSpec) { + self.hosted_specs.push(spec); } fn runtimes(&self) -> &[PlannedRuntime] { @@ -481,30 +504,55 @@ fn add_tool_sources(context: &CoreToolPlanContext<'_>, planned_tools: &mut Plann add_mcp_runtime_tools(context, planned_tools); add_dynamic_tools(context, planned_tools); add_extension_tools(context, planned_tools); - planned_tools - .hosted_specs - .extend(hosted_model_tool_specs(context.turn_context)); + for spec in hosted_model_tool_specs(context.turn_context) { + planned_tools.add_hosted_spec(spec); + } } fn add_shell_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut PlannedTools) { let turn_context = context.turn_context; let features = turn_context.features.get(); - planned_tools - .runtimes - .extend(build_shell_tools(ShellToolsOptions { - shell_type: shell_type_for_model_and_features(&turn_context.model_info, features), - shell_command_backend: shell_command_backend_for_features(features), - environment_mode: turn_context.tool_environment_mode(), - allow_login_shell: turn_context.config.permissions.allow_login_shell, - exec_permission_approvals_enabled: features.enabled(Feature::ExecPermissionApprovals), - })); + let environment_mode = turn_context.tool_environment_mode(); + if !environment_mode.has_environment() { + return; + } + + let allow_login_shell = turn_context.config.permissions.allow_login_shell; + let exec_permission_approvals_enabled = features.enabled(Feature::ExecPermissionApprovals); + let include_environment_id = matches!(environment_mode, ToolEnvironmentMode::Multiple); + let shell_command_options = ShellCommandHandlerOptions { + backend_config: shell_command_backend_for_features(features), + allow_login_shell, + exec_permission_approvals_enabled, + }; + + match shell_type_for_model_and_features(&turn_context.model_info, features) { + ConfigShellToolType::UnifiedExec => { + planned_tools.add(ExecCommandHandler::new(ExecCommandHandlerOptions { + allow_login_shell, + exec_permission_approvals_enabled, + include_environment_id, + })); + planned_tools.add(WriteStdinHandler); + + // Keep the legacy shell tool registered while unified exec is + // model-visible. + planned_tools.add_dispatch_only(ShellCommandHandler::new(shell_command_options)); + } + ConfigShellToolType::Disabled => {} + ConfigShellToolType::Default + | ConfigShellToolType::Local + | ConfigShellToolType::ShellCommand => { + planned_tools.add(ShellCommandHandler::new(shell_command_options)); + } + } } fn add_mcp_resource_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut PlannedTools) { if context.mcp_tools.is_some() { - planned_tools.add_runtime(ListMcpResourcesHandler); - planned_tools.add_runtime(ListMcpResourceTemplatesHandler); - planned_tools.add_runtime(ReadMcpResourceHandler); + planned_tools.add(ListMcpResourcesHandler); + planned_tools.add(ListMcpResourceTemplatesHandler); + planned_tools.add(ReadMcpResourceHandler); } } @@ -513,35 +561,35 @@ fn add_core_utility_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut let features = turn_context.features.get(); let environment_mode = turn_context.tool_environment_mode(); - planned_tools.add_runtime(PlanHandler); + planned_tools.add(PlanHandler); if goal_tools_enabled(turn_context) { - planned_tools.add_runtime(GetGoalHandler); - planned_tools.add_runtime(CreateGoalHandler); - planned_tools.add_runtime(UpdateGoalHandler); + planned_tools.add(GetGoalHandler); + planned_tools.add(CreateGoalHandler); + planned_tools.add(UpdateGoalHandler); } - planned_tools.add_runtime(RequestUserInputHandler { + planned_tools.add(RequestUserInputHandler { available_modes: request_user_input_available_modes(features), }); if features.enabled(Feature::RequestPermissionsTool) { - planned_tools.add_runtime(RequestPermissionsHandler); + planned_tools.add(RequestPermissionsHandler); } if tool_suggest_enabled(turn_context) && let Some(discoverable_tools) = context.discoverable_tools.filter(|tools| !tools.is_empty()) { - planned_tools.add_runtime(ListAvailablePluginsToInstallHandler::new( + planned_tools.add(ListAvailablePluginsToInstallHandler::new( collect_request_plugin_install_entries(discoverable_tools), )); - planned_tools.add_runtime(RequestPluginInstallHandler); + planned_tools.add(RequestPluginInstallHandler); } if environment_mode.has_environment() && turn_context.model_info.apply_patch_tool_type.is_some() { let include_environment_id = matches!(environment_mode, ToolEnvironmentMode::Multiple); - planned_tools.add_runtime(ApplyPatchHandler::new(include_environment_id)); + planned_tools.add(ApplyPatchHandler::new(include_environment_id)); } if turn_context @@ -550,12 +598,12 @@ fn add_core_utility_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut .iter() .any(|tool| tool == "test_sync_tool") { - planned_tools.add_runtime(TestSyncHandler); + planned_tools.add(TestSyncHandler); } if environment_mode.has_environment() { let include_environment_id = matches!(environment_mode, ToolEnvironmentMode::Multiple); - planned_tools.add_runtime(ViewImageHandler::new(ViewImageToolOptions { + planned_tools.add(ViewImageHandler::new(ViewImageToolOptions { can_request_original_image_detail: can_request_original_image_detail( &turn_context.model_info, ), @@ -578,47 +626,47 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mu .flatten(); let agent_type_description = agent_type_description(turn_context, context.default_agent_type_description); - planned_tools.add_runtime_arc(multi_agent_v2_handler( - SpawnAgentHandlerV2::new(SpawnAgentToolOptions { - available_models: turn_context.available_models.clone(), - agent_type_description, - hide_agent_type_model_reasoning: turn_context - .config - .multi_agent_v2 - .hide_spawn_agent_metadata, - include_usage_hint: turn_context.config.multi_agent_v2.usage_hint_enabled, - usage_hint_text: turn_context.config.multi_agent_v2.usage_hint_text.clone(), - max_concurrent_threads_per_session: max_concurrent_threads_per_session( - turn_context, - ), - }), + planned_tools.add_arc(override_tool_exposure( + multi_agent_v2_handler( + SpawnAgentHandlerV2::new(SpawnAgentToolOptions { + available_models: turn_context.available_models.clone(), + agent_type_description, + hide_agent_type_model_reasoning: turn_context + .config + .multi_agent_v2 + .hide_spawn_agent_metadata, + include_usage_hint: turn_context.config.multi_agent_v2.usage_hint_enabled, + usage_hint_text: turn_context.config.multi_agent_v2.usage_hint_text.clone(), + max_concurrent_threads_per_session: max_concurrent_threads_per_session( + turn_context, + ), + }), + tool_namespace, + ), exposure, - tool_namespace, )); - planned_tools.add_runtime_arc(multi_agent_v2_handler( - SendMessageHandlerV2, + planned_tools.add_arc(override_tool_exposure( + multi_agent_v2_handler(SendMessageHandlerV2, tool_namespace), exposure, - tool_namespace, )); - planned_tools.add_runtime_arc(multi_agent_v2_handler( - FollowupTaskHandlerV2, + planned_tools.add_arc(override_tool_exposure( + multi_agent_v2_handler(FollowupTaskHandlerV2, tool_namespace), exposure, - tool_namespace, )); - planned_tools.add_runtime_arc(multi_agent_v2_handler( - WaitAgentHandlerV2::new(context.wait_agent_timeouts), + planned_tools.add_arc(override_tool_exposure( + multi_agent_v2_handler( + WaitAgentHandlerV2::new(context.wait_agent_timeouts), + tool_namespace, + ), exposure, - tool_namespace, )); - planned_tools.add_runtime_arc(multi_agent_v2_handler( - CloseAgentHandlerV2, + planned_tools.add_arc(override_tool_exposure( + multi_agent_v2_handler(CloseAgentHandlerV2, tool_namespace), exposure, - tool_namespace, )); - planned_tools.add_runtime_arc(multi_agent_v2_handler( - ListAgentsHandlerV2, + planned_tools.add_arc(override_tool_exposure( + multi_agent_v2_handler(ListAgentsHandlerV2, tool_namespace), exposure, - tool_namespace, )); } else { let agent_type_description = @@ -629,7 +677,7 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mu } else { ToolExposure::Direct }; - planned_tools.add_runtime_arc(multi_agent_v1_handler( + planned_tools.add_with_exposure( SpawnAgentHandler::new(SpawnAgentToolOptions { available_models: turn_context.available_models.clone(), agent_type_description, @@ -644,21 +692,19 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mu ), }), exposure, - )); - planned_tools.add_runtime_arc(multi_agent_v1_handler(SendInputHandler, exposure)); - planned_tools.add_runtime_arc(multi_agent_v1_handler(ResumeAgentHandler, exposure)); - planned_tools.add_runtime_arc(multi_agent_v1_handler( - WaitAgentHandler::new(context.wait_agent_timeouts), - exposure, - )); - planned_tools.add_runtime_arc(multi_agent_v1_handler(CloseAgentHandler, exposure)); + ); + planned_tools.add_with_exposure(SendInputHandler, exposure); + planned_tools.add_with_exposure(ResumeAgentHandler, exposure); + planned_tools + .add_with_exposure(WaitAgentHandler::new(context.wait_agent_timeouts), exposure); + planned_tools.add_with_exposure(CloseAgentHandler, exposure); } } if agent_jobs_tools_enabled(turn_context) { - planned_tools.add_runtime(SpawnAgentsOnCsvHandler); + planned_tools.add(SpawnAgentsOnCsvHandler); if agent_jobs_worker_tools_enabled(turn_context) { - planned_tools.add_runtime(ReportAgentJobResultHandler); + planned_tools.add(ReportAgentJobResultHandler); } } } @@ -667,7 +713,7 @@ fn add_mcp_runtime_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut if let Some(mcp_tools) = context.mcp_tools { for tool in mcp_tools { match McpHandler::new(tool.clone()) { - Ok(handler) => planned_tools.add_runtime(handler), + Ok(handler) => planned_tools.add(handler), Err(err) => warn!( "Skipping MCP tool `{}`: failed to build tool spec: {err}", tool.canonical_tool_name() @@ -678,8 +724,8 @@ fn add_mcp_runtime_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut if let Some(deferred_mcp_tools) = context.deferred_mcp_tools { for tool in deferred_mcp_tools { - match McpHandler::with_exposure(tool.clone(), ToolExposure::Deferred) { - Ok(handler) => planned_tools.add_runtime(handler), + match McpHandler::new(tool.clone()) { + Ok(handler) => planned_tools.add_with_exposure(handler, ToolExposure::Deferred), Err(err) => warn!( "Skipping deferred MCP tool `{}`: failed to build tool spec: {err}", tool.canonical_tool_name() @@ -699,7 +745,7 @@ fn add_dynamic_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut Plan continue; }; - planned_tools.add_runtime(handler); + planned_tools.add(handler); } } @@ -732,7 +778,7 @@ fn append_tool_search_executor( return; } - planned_tools.add_runtime(ToolSearchHandler::new(search_infos)); + planned_tools.add(ToolSearchHandler::new(search_infos)); } fn prepend_code_mode_executors( @@ -787,30 +833,21 @@ fn append_extension_tool_executors( warn!("Skipping extension tool `{tool_name}`: tool already registered"); continue; } - planned_tools.add_runtime(ExtensionToolAdapter::new(executor)); + planned_tools.add(ExtensionToolAdapter::new(executor)); } } -fn multi_agent_v1_handler( - handler: impl CoreToolRuntime + 'static, - exposure: ToolExposure, -) -> Arc { - override_tool_exposure(Arc::new(handler), exposure) -} - fn multi_agent_v2_handler( handler: impl CoreToolRuntime + 'static, - exposure: ToolExposure, namespace: Option<&str>, ) -> Arc { - let handler: Arc = match namespace { + match namespace { Some(namespace) => Arc::new(MultiAgentV2NamespaceOverride { handler: Arc::new(handler), namespace: namespace.to_string(), }), None => Arc::new(handler), - }; - override_tool_exposure(handler, exposure) + } } struct MultiAgentV2NamespaceOverride { diff --git a/codex-rs/core/src/tools/tool_family/mod.rs b/codex-rs/core/src/tools/tool_family/mod.rs deleted file mode 100644 index d1e1cbd5a9..0000000000 --- a/codex-rs/core/src/tools/tool_family/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub(crate) mod shell; diff --git a/codex-rs/core/src/tools/tool_family/shell.rs b/codex-rs/core/src/tools/tool_family/shell.rs deleted file mode 100644 index 09b6d13a29..0000000000 --- a/codex-rs/core/src/tools/tool_family/shell.rs +++ /dev/null @@ -1,76 +0,0 @@ -use std::sync::Arc; - -use codex_protocol::openai_models::ConfigShellToolType; -use codex_tools::ShellCommandBackendConfig; -use codex_tools::ToolEnvironmentMode; - -use crate::tools::handlers::ExecCommandHandler; -use crate::tools::handlers::ExecCommandHandlerOptions; -use crate::tools::handlers::ShellCommandHandler; -use crate::tools::handlers::ShellCommandHandlerOptions; -use crate::tools::handlers::WriteStdinHandler; -use crate::tools::registry::CoreToolRuntime; - -#[derive(Clone, Copy, Debug)] -pub(crate) struct ShellToolsOptions { - pub(crate) shell_type: ConfigShellToolType, - pub(crate) shell_command_backend: ShellCommandBackendConfig, - pub(crate) environment_mode: ToolEnvironmentMode, - pub(crate) allow_login_shell: bool, - pub(crate) exec_permission_approvals_enabled: bool, -} - -pub(crate) fn build_shell_tools(options: ShellToolsOptions) -> Vec> { - let mut runtimes = Vec::new(); - if !options.environment_mode.has_environment() { - return runtimes; - } - - let include_environment_id = matches!(options.environment_mode, ToolEnvironmentMode::Multiple); - match options.shell_type { - ConfigShellToolType::UnifiedExec => { - add_runtime( - &mut runtimes, - ExecCommandHandler::new(ExecCommandHandlerOptions { - allow_login_shell: options.allow_login_shell, - exec_permission_approvals_enabled: options.exec_permission_approvals_enabled, - include_environment_id, - }), - ); - add_runtime(&mut runtimes, WriteStdinHandler); - - // Keep the legacy shell tool registered as a hidden runtime while - // unified exec is model-visible. - add_runtime( - &mut runtimes, - ShellCommandHandler::hidden(ShellCommandHandlerOptions { - backend_config: options.shell_command_backend, - allow_login_shell: options.allow_login_shell, - exec_permission_approvals_enabled: options.exec_permission_approvals_enabled, - }), - ); - } - ConfigShellToolType::Disabled => {} - ConfigShellToolType::Default - | ConfigShellToolType::Local - | ConfigShellToolType::ShellCommand => { - add_runtime( - &mut runtimes, - ShellCommandHandler::new(ShellCommandHandlerOptions { - backend_config: options.shell_command_backend, - allow_login_shell: options.allow_login_shell, - exec_permission_approvals_enabled: options.exec_permission_approvals_enabled, - }), - ); - } - } - - runtimes -} - -fn add_runtime(runtimes: &mut Vec>, runtime: T) -where - T: CoreToolRuntime + 'static, -{ - runtimes.push(Arc::new(runtime)); -}