mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Split tool handlers by tool name (#20687)
## Why Tool registration used to bind a tool name to a handler externally, which left ownership split between the registry plan and the handler implementation. Some built-in handlers also multiplexed multiple in-core tools by switching on the invoked tool name internally. This moves the registry identity onto the handler itself and makes built-in multi-tool areas use separate concrete handlers, so each registered handler instance owns exactly one tool name and one dispatch path. ## What Changed - Added `ToolHandler::tool_name()` and changed `ToolRegistryBuilder::register_handler` to derive the registry key from the handler. - Split built-in multiplexed handlers into concrete per-tool handlers for unified exec, shell/local shell/container exec, MCP resources, goal tools, and agent job tools. - Kept name-carrying handler instances only where the runtime target is inherently external or dynamic, such as MCP tools, dynamic tools, and unavailable placeholders. - Updated `ToolHandlerKind` and registry-plan construction so plan entries map directly to concrete handler registrations. ## Verification - `cargo test -p codex-tools tool_registry_plan` - `cargo test -p codex-core --lib tools::registry_tests` - `just fix -p codex-tools` - `just fix -p codex-core`
This commit is contained in:
committed by
GitHub
Unverified
parent
9cbef243b5
commit
f593323ef1
@@ -172,8 +172,8 @@ pub fn build_tool_registry_plan(
|
||||
/*supports_parallel_tool_calls*/ false,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
plan.register_handler("exec_command", ToolHandlerKind::UnifiedExec);
|
||||
plan.register_handler("write_stdin", ToolHandlerKind::UnifiedExec);
|
||||
plan.register_handler("exec_command", ToolHandlerKind::ExecCommand);
|
||||
plan.register_handler("write_stdin", ToolHandlerKind::WriteStdin);
|
||||
}
|
||||
ConfigShellToolType::Disabled => {}
|
||||
ConfigShellToolType::ShellCommand => {
|
||||
@@ -193,8 +193,8 @@ pub fn build_tool_registry_plan(
|
||||
&& 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);
|
||||
plan.register_handler("container.exec", ToolHandlerKind::ContainerExec);
|
||||
plan.register_handler("local_shell", ToolHandlerKind::LocalShell);
|
||||
plan.register_handler("shell_command", ToolHandlerKind::ShellCommand);
|
||||
}
|
||||
|
||||
@@ -214,9 +214,12 @@ pub fn build_tool_registry_plan(
|
||||
/*supports_parallel_tool_calls*/ true,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
plan.register_handler("list_mcp_resources", ToolHandlerKind::McpResource);
|
||||
plan.register_handler("list_mcp_resource_templates", ToolHandlerKind::McpResource);
|
||||
plan.register_handler("read_mcp_resource", ToolHandlerKind::McpResource);
|
||||
plan.register_handler("list_mcp_resources", ToolHandlerKind::ListMcpResources);
|
||||
plan.register_handler(
|
||||
"list_mcp_resource_templates",
|
||||
ToolHandlerKind::ListMcpResourceTemplates,
|
||||
);
|
||||
plan.register_handler("read_mcp_resource", ToolHandlerKind::ReadMcpResource);
|
||||
}
|
||||
|
||||
plan.push_spec(
|
||||
@@ -231,19 +234,19 @@ pub fn build_tool_registry_plan(
|
||||
/*supports_parallel_tool_calls*/ false,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
plan.register_handler("get_goal", ToolHandlerKind::Goal);
|
||||
plan.register_handler("get_goal", ToolHandlerKind::GetGoal);
|
||||
plan.push_spec(
|
||||
create_create_goal_tool(),
|
||||
/*supports_parallel_tool_calls*/ false,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
plan.register_handler("create_goal", ToolHandlerKind::Goal);
|
||||
plan.register_handler("create_goal", ToolHandlerKind::CreateGoal);
|
||||
plan.push_spec(
|
||||
create_update_goal_tool(),
|
||||
/*supports_parallel_tool_calls*/ false,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
plan.register_handler("update_goal", ToolHandlerKind::Goal);
|
||||
plan.register_handler("update_goal", ToolHandlerKind::UpdateGoal);
|
||||
}
|
||||
|
||||
plan.push_spec(
|
||||
@@ -493,14 +496,17 @@ pub fn build_tool_registry_plan(
|
||||
/*supports_parallel_tool_calls*/ false,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
plan.register_handler("spawn_agents_on_csv", ToolHandlerKind::AgentJobs);
|
||||
plan.register_handler("spawn_agents_on_csv", ToolHandlerKind::SpawnAgentsOnCsv);
|
||||
if config.agent_jobs_worker_tools {
|
||||
plan.push_spec(
|
||||
create_report_agent_job_result_tool(),
|
||||
/*supports_parallel_tool_calls*/ false,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
plan.register_handler("report_agent_job_result", ToolHandlerKind::AgentJobs);
|
||||
plan.register_handler(
|
||||
"report_agent_job_result",
|
||||
ToolHandlerKind::ReportAgentJobResult,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -10,19 +10,26 @@ use std::collections::HashMap;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum ToolHandlerKind {
|
||||
AgentJobs,
|
||||
ApplyPatch,
|
||||
CloseAgentV1,
|
||||
CloseAgentV2,
|
||||
CodeModeExecute,
|
||||
CodeModeWait,
|
||||
ContainerExec,
|
||||
CreateGoal,
|
||||
DynamicTool,
|
||||
ExecCommand,
|
||||
FollowupTaskV2,
|
||||
Goal,
|
||||
GetGoal,
|
||||
ListAgentsV2,
|
||||
ListMcpResourceTemplates,
|
||||
ListMcpResources,
|
||||
LocalShell,
|
||||
Mcp,
|
||||
McpResource,
|
||||
Plan,
|
||||
ReadMcpResource,
|
||||
ReportAgentJobResult,
|
||||
RequestPluginInstall,
|
||||
RequestPermissions,
|
||||
RequestUserInput,
|
||||
ResumeAgentV1,
|
||||
@@ -30,15 +37,16 @@ pub enum ToolHandlerKind {
|
||||
SendMessageV2,
|
||||
Shell,
|
||||
ShellCommand,
|
||||
SpawnAgentsOnCsv,
|
||||
SpawnAgentV1,
|
||||
SpawnAgentV2,
|
||||
TestSync,
|
||||
ToolSearch,
|
||||
RequestPluginInstall,
|
||||
UnifiedExec,
|
||||
UpdateGoal,
|
||||
ViewImage,
|
||||
WaitAgentV1,
|
||||
WaitAgentV2,
|
||||
WriteStdin,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
|
||||
Reference in New Issue
Block a user