Skip to content

[codex] Split tool handlers by tool name#20687

Merged
pakrym-oai merged 6 commits intomainfrom
pakrym/split-tool-handlers
May 5, 2026
Merged

[codex] Split tool handlers by tool name#20687
pakrym-oai merged 6 commits intomainfrom
pakrym/split-tool-handlers

Conversation

@pakrym-oai
Copy link
Copy Markdown
Collaborator

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

@pakrym-oai pakrym-oai force-pushed the pakrym/split-tool-handlers branch from 423e867 to e422ce8 Compare May 1, 2026 22:01
Comment thread codex-rs/core/src/tools/handlers/mcp_resource.rs
Comment thread codex-rs/core/src/tools/handlers/shell_tests.rs Outdated
Comment thread codex-rs/core/src/tools/spec.rs
pakrym-oai added 3 commits May 5, 2026 10:08
…dlers

# Conflicts:
#	codex-rs/core/src/tools/handlers/list_dir.rs
#	codex-rs/core/src/tools/handlers/mcp_resource.rs
#	codex-rs/core/src/tools/handlers/mod.rs
#	codex-rs/core/src/tools/handlers/request_plugin_install.rs
#	codex-rs/core/src/tools/spec.rs
#	codex-rs/tools/src/tool_registry_plan_types.rs
…dlers

# Conflicts:
#	codex-rs/core/src/tools/handlers/unified_exec.rs
@pakrym-oai pakrym-oai merged commit f593323 into main May 5, 2026
36 of 38 checks passed
@pakrym-oai pakrym-oai deleted the pakrym/split-tool-handlers branch May 5, 2026 20:46
@github-actions github-actions Bot locked and limited conversation to collaborators May 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants