mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Simplify tool executor and registry plumbing (#22636)
## Why The tool runtime path still had a typed output associated type on `ToolExecutor`, plus a core-only `RegisteredTool` adapter and extension-only executor aliases. That made every new shared tool runtime carry extra adapter plumbing before it could participate in core dispatch, extension tools, hook payloads, telemetry, and model-visible spec generation. This PR moves output erasure to the shared executor boundary so core and extension tools can use the same execution contract directly. ## What Changed - Changed `codex_tools::ToolExecutor` to return `Box<dyn ToolOutput>` instead of an associated `Output` type. - Removed the extension-specific `ExtensionToolExecutor` / `ExtensionToolOutput` aliases and exposed `ToolExecutor<ToolCall>` plus `ToolOutput` through `codex-extension-api`. - Reworked core tool registration around `CoreToolRuntime` and `ToolRegistry::from_tools`, removing the extra `RegisteredTool` / `ToolRegistryBuilder` layer. - Consolidated model-visible spec planning and registry construction in `core/src/tools/spec_plan.rs`, including deferred tool search and code-mode-only filtering. - Added `ToolOutput` helpers for post-tool-use hook ids and inputs so MCP, unified exec, extension, and other boxed outputs preserve the same hook payload behavior. - Updated core handlers, memories tools, and the related registry/spec/router tests to use the simplified contract. ## Test Coverage - Updated coverage for tool spec planning, registry lookup, deferred tool search registration, extension tool routing, post-tool-use hook payloads, dispatch tracing, guardian output extraction, and memories extension tool execution.
This commit is contained in:
@@ -4,12 +4,13 @@ use std::sync::Arc;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::TokenUsageInfo;
|
||||
use codex_tools::ToolCall;
|
||||
use codex_tools::ToolExecutor;
|
||||
|
||||
use crate::ExtensionData;
|
||||
|
||||
mod prompt;
|
||||
mod thread_lifecycle;
|
||||
mod tools;
|
||||
mod turn_lifecycle;
|
||||
|
||||
pub use prompt::PromptFragment;
|
||||
@@ -17,8 +18,6 @@ pub use prompt::PromptSlot;
|
||||
pub use thread_lifecycle::ThreadResumeInput;
|
||||
pub use thread_lifecycle::ThreadStartInput;
|
||||
pub use thread_lifecycle::ThreadStopInput;
|
||||
pub use tools::ExtensionToolExecutor;
|
||||
pub use tools::ExtensionToolOutput;
|
||||
pub use turn_lifecycle::TurnAbortInput;
|
||||
pub use turn_lifecycle::TurnStartInput;
|
||||
pub use turn_lifecycle::TurnStopInput;
|
||||
@@ -106,7 +105,7 @@ pub trait ToolContributor: Send + Sync {
|
||||
&self,
|
||||
session_store: &ExtensionData,
|
||||
thread_store: &ExtensionData,
|
||||
) -> Vec<Arc<dyn ExtensionToolExecutor>>;
|
||||
) -> Vec<Arc<dyn ToolExecutor<ToolCall>>>;
|
||||
}
|
||||
|
||||
/// Future returned by one claimed approval-review contribution.
|
||||
|
||||
@@ -1,15 +0,0 @@
|
||||
use codex_tools::JsonToolOutput;
|
||||
use codex_tools::ToolCall;
|
||||
use codex_tools::ToolExecutor;
|
||||
|
||||
/// Model-facing output returned by extension-owned tools.
|
||||
pub type ExtensionToolOutput = JsonToolOutput;
|
||||
|
||||
/// Thin alias for extension-owned executable tools.
|
||||
///
|
||||
/// Extensions implement the shared `ToolExecutor<ToolCall>` contract directly;
|
||||
/// the marker keeps contributor signatures readable while preserving one
|
||||
/// executable-tool abstraction across host and extension tools.
|
||||
pub trait ExtensionToolExecutor: ToolExecutor<ToolCall, Output = ExtensionToolOutput> {}
|
||||
|
||||
impl<T> ExtensionToolExecutor for T where T: ToolExecutor<ToolCall, Output = ExtensionToolOutput> {}
|
||||
@@ -11,6 +11,7 @@ pub use codex_tools::ResponsesApiTool;
|
||||
pub use codex_tools::ToolCall;
|
||||
pub use codex_tools::ToolExecutor;
|
||||
pub use codex_tools::ToolName;
|
||||
pub use codex_tools::ToolOutput;
|
||||
pub use codex_tools::ToolPayload;
|
||||
pub use codex_tools::ToolSpec;
|
||||
pub use codex_tools::parse_tool_input_schema;
|
||||
@@ -18,8 +19,6 @@ pub use contributors::ApprovalReviewContributor;
|
||||
pub use contributors::ApprovalReviewFuture;
|
||||
pub use contributors::ConfigContributor;
|
||||
pub use contributors::ContextContributor;
|
||||
pub use contributors::ExtensionToolExecutor;
|
||||
pub use contributors::ExtensionToolOutput;
|
||||
pub use contributors::PromptFragment;
|
||||
pub use contributors::PromptSlot;
|
||||
pub use contributors::ThreadLifecycleContributor;
|
||||
|
||||
@@ -63,7 +63,7 @@ impl ToolContributor for MemoriesExtension {
|
||||
&self,
|
||||
_session_store: &ExtensionData,
|
||||
thread_store: &ExtensionData,
|
||||
) -> Vec<Arc<dyn codex_extension_api::ExtensionToolExecutor>> {
|
||||
) -> Vec<Arc<dyn codex_extension_api::ToolExecutor<codex_extension_api::ToolCall>>> {
|
||||
let Some(config) = thread_store.get::<MemoriesExtensionConfig>() else {
|
||||
return Vec::new();
|
||||
};
|
||||
|
||||
@@ -3,10 +3,10 @@ use std::sync::Arc;
|
||||
|
||||
use codex_extension_api::ContextContributor;
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_extension_api::ExtensionToolExecutor;
|
||||
use codex_extension_api::PromptSlot;
|
||||
use codex_extension_api::ToolCall;
|
||||
use codex_extension_api::ToolContributor;
|
||||
use codex_extension_api::ToolExecutor;
|
||||
use codex_extension_api::ToolName;
|
||||
use codex_extension_api::ToolPayload;
|
||||
use codex_tools::ToolOutput;
|
||||
@@ -290,20 +290,23 @@ async fn search_tool_rejects_legacy_single_query() {
|
||||
.to_string(),
|
||||
};
|
||||
|
||||
let err = tool
|
||||
let result = tool
|
||||
.handle(ToolCall {
|
||||
call_id: "call-1".to_string(),
|
||||
tool_name: memory_tool_name(crate::SEARCH_TOOL_NAME),
|
||||
payload,
|
||||
})
|
||||
.await
|
||||
.expect_err("legacy query field should be rejected");
|
||||
.await;
|
||||
let err = match result {
|
||||
Ok(_) => panic!("legacy query field should be rejected"),
|
||||
Err(err) => err,
|
||||
};
|
||||
|
||||
assert!(err.to_string().contains("unknown field"));
|
||||
assert!(err.to_string().contains("query"));
|
||||
}
|
||||
|
||||
fn memory_tool(memory_root: &Path, tool_name: &str) -> Arc<dyn ExtensionToolExecutor> {
|
||||
fn memory_tool(memory_root: &Path, tool_name: &str) -> Arc<dyn ToolExecutor<ToolCall>> {
|
||||
let expected_tool_name = memory_tool_name(tool_name);
|
||||
crate::tools::memory_tools(LocalMemoriesBackend::from_memory_root(memory_root))
|
||||
.into_iter()
|
||||
|
||||
@@ -39,8 +39,6 @@ impl<B> ToolExecutor<ToolCall> for ListTool<B>
|
||||
where
|
||||
B: MemoriesBackend,
|
||||
{
|
||||
type Output = JsonToolOutput;
|
||||
|
||||
fn tool_name(&self) -> ToolName {
|
||||
memory_tool_name(LIST_TOOL_NAME)
|
||||
}
|
||||
@@ -55,7 +53,8 @@ where
|
||||
async fn handle(
|
||||
&self,
|
||||
call: ToolCall,
|
||||
) -> Result<Self::Output, codex_extension_api::FunctionCallError> {
|
||||
) -> Result<Box<dyn codex_extension_api::ToolOutput>, codex_extension_api::FunctionCallError>
|
||||
{
|
||||
let backend = self.backend.clone();
|
||||
let args: ListArgs = parse_args(&call)?;
|
||||
let response = backend
|
||||
@@ -70,6 +69,6 @@ where
|
||||
})
|
||||
.await
|
||||
.map_err(backend_error_to_function_call)?;
|
||||
Ok(JsonToolOutput::new(json!(response)))
|
||||
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_extension_api::ExtensionToolExecutor;
|
||||
use codex_extension_api::FunctionCallError;
|
||||
use codex_extension_api::ResponsesApiTool;
|
||||
use codex_extension_api::ToolCall;
|
||||
use codex_extension_api::ToolExecutor;
|
||||
use codex_extension_api::ToolName;
|
||||
use codex_extension_api::ToolSpec;
|
||||
use codex_extension_api::parse_tool_input_schema;
|
||||
@@ -23,7 +23,7 @@ mod list;
|
||||
mod read;
|
||||
mod search;
|
||||
|
||||
pub(crate) fn memory_tools<B>(backend: B) -> Vec<Arc<dyn ExtensionToolExecutor>>
|
||||
pub(crate) fn memory_tools<B>(backend: B) -> Vec<Arc<dyn ToolExecutor<ToolCall>>>
|
||||
where
|
||||
B: MemoriesBackend,
|
||||
{
|
||||
|
||||
@@ -38,8 +38,6 @@ impl<B> ToolExecutor<ToolCall> for ReadTool<B>
|
||||
where
|
||||
B: MemoriesBackend,
|
||||
{
|
||||
type Output = JsonToolOutput;
|
||||
|
||||
fn tool_name(&self) -> ToolName {
|
||||
memory_tool_name(READ_TOOL_NAME)
|
||||
}
|
||||
@@ -54,7 +52,8 @@ where
|
||||
async fn handle(
|
||||
&self,
|
||||
call: ToolCall,
|
||||
) -> Result<Self::Output, codex_extension_api::FunctionCallError> {
|
||||
) -> Result<Box<dyn codex_extension_api::ToolOutput>, codex_extension_api::FunctionCallError>
|
||||
{
|
||||
let backend = self.backend.clone();
|
||||
let args: ReadArgs = parse_args(&call)?;
|
||||
let response = backend
|
||||
@@ -66,6 +65,6 @@ where
|
||||
})
|
||||
.await
|
||||
.map_err(backend_error_to_function_call)?;
|
||||
Ok(JsonToolOutput::new(json!(response)))
|
||||
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,8 +47,6 @@ impl<B> ToolExecutor<ToolCall> for SearchTool<B>
|
||||
where
|
||||
B: MemoriesBackend,
|
||||
{
|
||||
type Output = JsonToolOutput;
|
||||
|
||||
fn tool_name(&self) -> ToolName {
|
||||
memory_tool_name(SEARCH_TOOL_NAME)
|
||||
}
|
||||
@@ -63,14 +61,15 @@ where
|
||||
async fn handle(
|
||||
&self,
|
||||
call: ToolCall,
|
||||
) -> Result<Self::Output, codex_extension_api::FunctionCallError> {
|
||||
) -> Result<Box<dyn codex_extension_api::ToolOutput>, codex_extension_api::FunctionCallError>
|
||||
{
|
||||
let backend = self.backend.clone();
|
||||
let args: SearchArgs = parse_args(&call)?;
|
||||
let response = backend
|
||||
.search(args.into_request())
|
||||
.await
|
||||
.map_err(backend_error_to_function_call)?;
|
||||
Ok(JsonToolOutput::new(json!(response)))
|
||||
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user