refactor: centralize tool exposure planning (#23876)

## Why

Tool exposure is a planning concern, but the deferred MCP path and
dispatch-only legacy shell path were carrying those decisions in handler
constructors and a shell-only tool-family builder. Keeping those
decisions in `spec_plan` makes the core tool plan easier to follow and
keeps handlers focused on runtime behavior.

## What changed

- add `PlannedTools` helpers for ordinary runtimes, exposure overrides,
dispatch-only runtimes, and hosted specs
- inline shell tool assembly into `core/src/tools/spec_plan.rs` and
remove the shell-only `tool_family` module
- remove exposure state and special exposure constructors from
`McpHandler` and `ShellCommandHandler`
- keep hidden runtime behavior centralized in `ExposureOverride`,
including disabling parallel tool calls for hidden handlers

## Testing

- Not run (refactor only)
This commit is contained in:
jif-oai
2026-05-21 16:21:23 +02:00
committed by GitHub
parent 2a25602783
commit e6c8371e4e
7 changed files with 132 additions and 208 deletions

View File

@@ -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, serde_json::Error> {
Self::with_exposure(tool_info, ToolExposure::Direct)
}
pub fn with_exposure(
tool_info: ToolInfo,
exposure: ToolExposure,
) -> Result<Self, serde_json::Error> {
let spec = create_tool_spec(&tool_info)?;
Ok(Self {
tool_info,
spec,
exposure,
})
Ok(Self { tool_info, spec })
}
}
@@ -61,10 +48,6 @@ impl ToolExecutor<ToolInvocation> 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
}

View File

@@ -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<ShellCommandBackendConfig> 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<ToolInvocation> for ShellCommandHandler {
})
}
fn exposure(&self) -> ToolExposure {
self.exposure
}
fn supports_parallel_tool_calls(&self) -> bool {
self.exposure != ToolExposure::Hidden
true
}
async fn handle(

View File

@@ -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;

View File

@@ -193,7 +193,7 @@ impl ToolExecutor<ToolInvocation> 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(

View File

@@ -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<T>(&mut self, runtime: T)
fn add<T>(&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<T>(&mut self, handler: T, exposure: ToolExposure)
where
T: CoreToolRuntime + 'static,
{
self.runtimes
.push(override_tool_exposure(Arc::new(handler), exposure));
}
fn add_dispatch_only<T>(&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<dyn CoreToolRuntime> {
override_tool_exposure(Arc::new(handler), exposure)
}
fn multi_agent_v2_handler(
handler: impl CoreToolRuntime + 'static,
exposure: ToolExposure,
namespace: Option<&str>,
) -> Arc<dyn CoreToolRuntime> {
let handler: Arc<dyn CoreToolRuntime> = 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 {

View File

@@ -1 +0,0 @@
pub(crate) mod shell;

View File

@@ -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<Arc<dyn CoreToolRuntime>> {
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<T>(runtimes: &mut Vec<Arc<dyn CoreToolRuntime>>, runtime: T)
where
T: CoreToolRuntime + 'static,
{
runtimes.push(Arc::new(runtime));
}