mirror of
https://github.com/openai/codex.git
synced 2026-11-03 08:22:30 +00:00
jif-oai
1824685a00
## Why Codex still models model-visible tools and executable behavior largely inside `codex-core`, which makes it harder to evolve the tool system toward a single reusable abstraction for built-ins, MCP-backed tools, dynamic tools, and later tools injected from outside core. This PR takes the next incremental step in that direction by moving the common execution-facing pieces out of core and separating them from core-only orchestration. The intent is to let shared tool abstractions improve in one place, while `codex-core` keeps the parts that are still inherently host-specific today, such as `ToolInvocation`, dispatch wiring, and hook integration. This PR is mostly moving things around. The only interesting piece is this abstraction: https://github.com/openai/codex/pull/22359/changes#diff-81af519002548ba51ed102bdaaf77e081d40a1e73a6e5f9b104bbbc96a6f1b3dR13 ## What changed - Added `codex_tools::ToolExecutor<Invocation>` as the shared execution trait for model-visible tools. - Moved the reusable execution support types from `codex-core` into `codex-tools`: - `FunctionCallError` - `ToolPayload` - `ToolOutput` - Refactored core tool implementations so that execution behavior lives on `ToolExecutor<ToolInvocation>`, while `ToolHandler` remains the core-local extension point for hook payloads, telemetry tags, diff consumers, and other orchestration concerns. - Kept the registry and dispatch flow behaviorally unchanged while making the shared/extracted boundary explicit across built-in, MCP, dynamic, extension-backed, shell, and multi-agent tool handlers. ## Verification - `cargo test -p codex-tools` - `just fix -p codex-tools` - `just fix -p codex-core` - `cargo test -p codex-core` progressed through the updated tool surfaces and then hit the existing unrelated multi-agent stack overflow in `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed`.
78 lines
2.5 KiB
Rust
78 lines
2.5 KiB
Rust
use super::*;
|
|
use crate::tools::handlers::GetGoalHandler;
|
|
use crate::tools::handlers::goal_spec::GET_GOAL_TOOL_NAME;
|
|
use crate::tools::handlers::goal_spec::create_get_goal_tool;
|
|
use pretty_assertions::assert_eq;
|
|
|
|
struct TestHandler {
|
|
tool_name: codex_tools::ToolName,
|
|
}
|
|
|
|
impl ToolExecutor<ToolInvocation> for TestHandler {
|
|
type Output = crate::tools::context::FunctionToolOutput;
|
|
|
|
fn tool_name(&self) -> codex_tools::ToolName {
|
|
self.tool_name.clone()
|
|
}
|
|
|
|
async fn handle(&self, _invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
|
|
Ok(crate::tools::context::FunctionToolOutput::from_text(
|
|
"ok".to_string(),
|
|
Some(true),
|
|
))
|
|
}
|
|
}
|
|
|
|
impl ToolHandler for TestHandler {}
|
|
|
|
#[test]
|
|
fn handler_looks_up_namespaced_aliases_explicitly() {
|
|
let namespace = "mcp__codex_apps__gmail";
|
|
let tool_name = "gmail_get_recent_emails";
|
|
let plain_name = codex_tools::ToolName::plain(tool_name);
|
|
let namespaced_name = codex_tools::ToolName::namespaced(namespace, tool_name);
|
|
let plain_handler = Arc::new(TestHandler {
|
|
tool_name: plain_name.clone(),
|
|
}) as Arc<dyn AnyToolHandler>;
|
|
let namespaced_handler = Arc::new(TestHandler {
|
|
tool_name: namespaced_name.clone(),
|
|
}) as Arc<dyn AnyToolHandler>;
|
|
let registry = ToolRegistry::new(HashMap::from([
|
|
(plain_name.clone(), Arc::clone(&plain_handler)),
|
|
(namespaced_name.clone(), Arc::clone(&namespaced_handler)),
|
|
]));
|
|
|
|
let plain = registry.handler(&plain_name);
|
|
let namespaced = registry.handler(&namespaced_name);
|
|
let missing_namespaced = registry.handler(&codex_tools::ToolName::namespaced(
|
|
"mcp__codex_apps__calendar",
|
|
tool_name,
|
|
));
|
|
|
|
assert_eq!(plain.is_some(), true);
|
|
assert_eq!(namespaced.is_some(), true);
|
|
assert_eq!(missing_namespaced.is_none(), true);
|
|
assert!(
|
|
plain
|
|
.as_ref()
|
|
.is_some_and(|handler| Arc::ptr_eq(handler, &plain_handler))
|
|
);
|
|
assert!(
|
|
namespaced
|
|
.as_ref()
|
|
.is_some_and(|handler| Arc::ptr_eq(handler, &namespaced_handler))
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn register_handler_adds_handler_and_spec() {
|
|
let mut builder = ToolRegistryBuilder::new();
|
|
builder.register_handler(Arc::new(GetGoalHandler));
|
|
|
|
let (specs, registry) = builder.build();
|
|
|
|
assert_eq!(specs.len(), 1);
|
|
assert_eq!(specs[0], create_get_goal_tool());
|
|
assert!(registry.has_handler(&codex_tools::ToolName::plain(GET_GOAL_TOOL_NAME)));
|
|
}
|