mirror of
https://github.com/openai/codex.git
synced 2026-11-03 08:22:30 +00:00
Refactor extension tools onto shared ToolExecutor (#22369)
## Why Extension tools were split across two public runtime contracts: `codex-tool-api` exposed `ToolBundle` plus its own call/spec/error types, while core native tools used `codex_tools::ToolExecutor`. That made contributed tool specs and execution behavior easy to drift apart and added another crate boundary for what should be one executable-tool seam. This PR makes `ToolExecutor` the single runtime contract and keeps extension-specific pinning in `codex-extension-api`. ## Remaining todo https://github.com/openai/codex/pull/22369/changes#diff-b935ea8245c3ce568a30cff660175fa6390b66b872ae409e1e2e965738250741R5 Either generic `Invocation` or sub-extract the `ToolCall` and clean `ToolInvocation` ## What changed - Removed the `codex-tool-api` workspace crate and its dependencies from core and `codex-extension-api`. - Made `codex_tools::ToolExecutor` object-safe with `async_trait` so extension contributors can return a dyn executor. - Added the extension-facing aliases under `ext/extension-api/src/contributors/tools.rs`, including `ExtensionToolExecutor = dyn ToolExecutor<ToolCall, Output = ExtensionToolOutput>`. - Changed `ToolContributor::tools` to return extension executors directly instead of `ToolBundle`s. - Updated core’s extension tool handler/registry/router path to adapt those extension executors into the existing native `ToolInvocation` runtime path. - Added focused coverage for extension tools being registered, model-visible, dispatchable, and not replacing built-in tools. ## Verification - `cargo test -p codex-tools` - `cargo test -p codex-extension-api`
This commit is contained in:
1 parent
1824685a00
commit
9c5dfa7b1a
30 files changed
+349
-450
No files matched your search
@@ -543,7 +543,7 @@ fn test_tool_runtime(session: Arc<Session>, turn_context: Arc<TurnContext>) -> T
|
||||
mcp_tools: None,
|
||||
deferred_mcp_tools: None,
|
||||
discoverable_tools: None,
|
||||
extension_tool_bundles: Vec::new(),
|
||||
extension_tool_executors: Vec::new(),
|
||||
dynamic_tools: turn_context.dynamic_tools.as_slice(),
|
||||
},
|
||||
));
|
||||
@@ -8607,7 +8607,7 @@ async fn fatal_tool_error_stops_turn_and_reports_error() {
|
||||
deferred_mcp_tools,
|
||||
mcp_tools: Some(tools),
|
||||
discoverable_tools: None,
|
||||
extension_tool_bundles: Vec::new(),
|
||||
extension_tool_executors: Vec::new(),
|
||||
dynamic_tools: turn_context.dynamic_tools.as_slice(),
|
||||
},
|
||||
);
|
||||
|
||||
@@ -55,7 +55,7 @@ use crate::tools::context::SharedTurnDiffTracker;
|
||||
use crate::tools::parallel::ToolCallRuntime;
|
||||
use crate::tools::registry::ToolArgumentDiffConsumer;
|
||||
use crate::tools::router::ToolRouterParams;
|
||||
use crate::tools::router::extension_tool_bundles;
|
||||
use crate::tools::router::extension_tool_executors;
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use crate::turn_timing::record_turn_ttft_metric;
|
||||
use crate::util::backoff;
|
||||
@@ -1250,7 +1250,7 @@ pub(crate) async fn built_tools(
|
||||
mcp_tools,
|
||||
deferred_mcp_tools,
|
||||
discoverable_tools,
|
||||
extension_tool_bundles: extension_tool_bundles(sess),
|
||||
extension_tool_executors: extension_tool_executors(sess),
|
||||
dynamic_tools: turn_context.dynamic_tools.as_slice(),
|
||||
},
|
||||
)))
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_tool_api::ToolBundle as ExtensionToolBundle;
|
||||
use codex_tool_api::ToolError as ExtensionToolError;
|
||||
use codex_tools::ResponsesApiTool;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_extension_api::ExtensionToolExecutor;
|
||||
use codex_extension_api::ExtensionToolOutput;
|
||||
use codex_tools::ToolCall as ExtensionToolCall;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolSpec;
|
||||
use serde_json::Value;
|
||||
@@ -19,46 +18,13 @@ use crate::tools::registry::PreToolUsePayload;
|
||||
use crate::tools::registry::ToolExecutor;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
|
||||
pub(crate) struct BundledToolOutput {
|
||||
value: Value,
|
||||
pub(crate) struct ExtensionToolHandler {
|
||||
executor: Arc<dyn ExtensionToolExecutor>,
|
||||
}
|
||||
|
||||
impl ToolOutput for BundledToolOutput {
|
||||
fn log_preview(&self) -> String {
|
||||
self.value.to_string()
|
||||
}
|
||||
|
||||
fn success_for_logging(&self) -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
fn to_response_item(&self, call_id: &str, _payload: &ToolPayload) -> ResponseInputItem {
|
||||
ResponseInputItem::FunctionCallOutput {
|
||||
call_id: call_id.to_string(),
|
||||
output: FunctionCallOutputPayload {
|
||||
body: FunctionCallOutputBody::Text(self.value.to_string()),
|
||||
success: Some(true),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option<Value> {
|
||||
Some(self.value.clone())
|
||||
}
|
||||
|
||||
fn code_mode_result(&self, _payload: &ToolPayload) -> Value {
|
||||
self.value.clone()
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) struct BundledToolHandler {
|
||||
bundle: ExtensionToolBundle,
|
||||
spec: ToolSpec,
|
||||
}
|
||||
|
||||
impl BundledToolHandler {
|
||||
pub(crate) fn new(bundle: ExtensionToolBundle, spec: ToolSpec) -> Self {
|
||||
Self { bundle, spec }
|
||||
impl ExtensionToolHandler {
|
||||
pub(crate) fn new(executor: Arc<dyn ExtensionToolExecutor>) -> Self {
|
||||
Self { executor }
|
||||
}
|
||||
|
||||
fn arguments_from_payload<'a>(&self, payload: &'a ToolPayload) -> Option<&'a str> {
|
||||
@@ -69,41 +35,23 @@ impl BundledToolHandler {
|
||||
}
|
||||
}
|
||||
|
||||
impl ToolExecutor<ToolInvocation> for BundledToolHandler {
|
||||
type Output = BundledToolOutput;
|
||||
impl ToolExecutor<ToolInvocation> for ExtensionToolHandler {
|
||||
type Output = ExtensionToolOutput;
|
||||
|
||||
fn tool_name(&self) -> ToolName {
|
||||
ToolName::plain(self.bundle.tool_name())
|
||||
self.executor.tool_name()
|
||||
}
|
||||
|
||||
fn spec(&self) -> Option<ToolSpec> {
|
||||
Some(self.spec.clone())
|
||||
self.executor.spec()
|
||||
}
|
||||
|
||||
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
|
||||
let arguments = self
|
||||
.arguments_from_payload(&invocation.payload)
|
||||
.ok_or_else(|| {
|
||||
FunctionCallError::Fatal(format!(
|
||||
"tool {} invoked with incompatible payload",
|
||||
self.bundle.tool_name()
|
||||
))
|
||||
})?
|
||||
.to_string();
|
||||
let value = self
|
||||
.bundle
|
||||
.executor()
|
||||
.execute(codex_tool_api::ToolCall {
|
||||
call_id: invocation.call_id,
|
||||
arguments,
|
||||
})
|
||||
.await
|
||||
.map_err(map_extension_tool_error)?;
|
||||
Ok(BundledToolOutput { value })
|
||||
self.executor.handle(to_extension_call(&invocation)).await
|
||||
}
|
||||
}
|
||||
|
||||
impl ToolHandler for BundledToolHandler {
|
||||
impl ToolHandler for ExtensionToolHandler {
|
||||
fn matches_kind(&self, payload: &ToolPayload) -> bool {
|
||||
self.arguments_from_payload(payload).is_some()
|
||||
}
|
||||
@@ -132,23 +80,11 @@ impl ToolHandler for BundledToolHandler {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn extension_tool_spec(
|
||||
spec: &codex_tool_api::FunctionToolSpec,
|
||||
) -> Result<ToolSpec, serde_json::Error> {
|
||||
Ok(ToolSpec::Function(ResponsesApiTool {
|
||||
name: spec.name.clone(),
|
||||
description: spec.description.clone(),
|
||||
strict: spec.strict,
|
||||
defer_loading: None,
|
||||
parameters: codex_tools::parse_tool_input_schema(&spec.parameters)?,
|
||||
output_schema: None,
|
||||
}))
|
||||
}
|
||||
|
||||
fn map_extension_tool_error(error: ExtensionToolError) -> FunctionCallError {
|
||||
match error {
|
||||
ExtensionToolError::RespondToModel(message) => FunctionCallError::RespondToModel(message),
|
||||
ExtensionToolError::Fatal(message) => FunctionCallError::Fatal(message),
|
||||
fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
|
||||
ExtensionToolCall {
|
||||
call_id: invocation.call_id.clone(),
|
||||
tool_name: invocation.tool_name.clone(),
|
||||
payload: invocation.payload.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -167,9 +103,7 @@ mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
use super::BundledToolHandler;
|
||||
use super::BundledToolOutput;
|
||||
use super::extension_tool_spec;
|
||||
use super::ExtensionToolHandler;
|
||||
use crate::tools::context::ToolCallSource;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolPayload;
|
||||
@@ -181,32 +115,43 @@ mod tests {
|
||||
|
||||
struct StubExtensionExecutor;
|
||||
|
||||
impl codex_tool_api::ToolExecutor for StubExtensionExecutor {
|
||||
fn execute(&self, _call: codex_tool_api::ToolCall) -> codex_tool_api::ToolFuture<'_> {
|
||||
Box::pin(async { Ok(json!({ "ok": true })) })
|
||||
impl codex_extension_api::ExtensionToolExecutor for StubExtensionExecutor {
|
||||
fn tool_name(&self) -> codex_tools::ToolName {
|
||||
codex_tools::ToolName::plain("extension_echo")
|
||||
}
|
||||
|
||||
fn spec(&self) -> Option<codex_tools::ToolSpec> {
|
||||
Some(codex_tools::ToolSpec::Function(
|
||||
codex_tools::ResponsesApiTool {
|
||||
name: "extension_echo".to_string(),
|
||||
description: "Echoes arguments.".to_string(),
|
||||
strict: true,
|
||||
parameters: codex_tools::parse_tool_input_schema(&json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"message": { "type": "string" },
|
||||
},
|
||||
"required": ["message"],
|
||||
"additionalProperties": false,
|
||||
}))
|
||||
.expect("extension schema should parse"),
|
||||
output_schema: None,
|
||||
defer_loading: None,
|
||||
},
|
||||
))
|
||||
}
|
||||
|
||||
fn handle(
|
||||
&self,
|
||||
_call: codex_tools::ToolCall,
|
||||
) -> codex_extension_api::ExtensionToolFuture<'_> {
|
||||
Box::pin(async { Ok(codex_tools::JsonToolOutput::new(json!({ "ok": true }))) })
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exposes_generic_hook_payloads() {
|
||||
let bundle = codex_tool_api::ToolBundle::new(
|
||||
codex_tool_api::FunctionToolSpec {
|
||||
name: "extension_echo".to_string(),
|
||||
description: "Echoes arguments.".to_string(),
|
||||
strict: true,
|
||||
parameters: json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"message": { "type": "string" },
|
||||
},
|
||||
"required": ["message"],
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
},
|
||||
Arc::new(StubExtensionExecutor),
|
||||
);
|
||||
let spec = extension_tool_spec(bundle.spec()).expect("extension spec should convert");
|
||||
let handler = BundledToolHandler::new(bundle, spec);
|
||||
let handler = ExtensionToolHandler::new(Arc::new(StubExtensionExecutor));
|
||||
let (session, turn) = crate::session::tests::make_session_and_context().await;
|
||||
let invocation = ToolInvocation {
|
||||
session: session.into(),
|
||||
@@ -220,9 +165,7 @@ mod tests {
|
||||
arguments: json!({ "message": "hello" }).to_string(),
|
||||
},
|
||||
};
|
||||
let output = BundledToolOutput {
|
||||
value: json!({ "ok": true }),
|
||||
};
|
||||
let output = codex_tools::JsonToolOutput::new(json!({ "ok": true }));
|
||||
|
||||
assert_eq!(
|
||||
ToolHandler::pre_tool_use_payload(&handler, &invocation),
|
||||
|
||||
@@ -18,15 +18,14 @@ use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::flat_tool_name;
|
||||
use crate::tools::handlers::extension_tools::BundledToolHandler;
|
||||
use crate::tools::handlers::extension_tools::extension_tool_spec;
|
||||
use crate::tools::handlers::extension_tools::ExtensionToolHandler;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::tool_dispatch_trace::ToolDispatchTrace;
|
||||
use crate::tools::tool_search_entry::ToolSearchInfo;
|
||||
use crate::util::error_or_panic;
|
||||
use codex_extension_api::ExtensionToolExecutor;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_tool_api::ToolBundle as ExtensionToolBundle;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolSpec;
|
||||
use futures::future::BoxFuture;
|
||||
@@ -584,25 +583,18 @@ impl ToolRegistryBuilder {
|
||||
self.handlers.insert(name, handler);
|
||||
}
|
||||
|
||||
pub fn register_tool_bundle(&mut self, bundle: ExtensionToolBundle) {
|
||||
let tool_name = ToolName::plain(bundle.tool_name());
|
||||
pub fn register_extension_tool_executor(&mut self, executor: Arc<dyn ExtensionToolExecutor>) {
|
||||
let tool_name = executor.tool_name();
|
||||
if self.handlers.contains_key(&tool_name) {
|
||||
warn!("Skipping extension tool `{tool_name}`: handler already registered");
|
||||
return;
|
||||
}
|
||||
|
||||
let spec = match extension_tool_spec(bundle.spec()) {
|
||||
Ok(spec) => spec,
|
||||
Err(error) => {
|
||||
error_or_panic(format!(
|
||||
"failed to convert extension tool `{tool_name}` to a host spec: {error}"
|
||||
));
|
||||
return;
|
||||
}
|
||||
};
|
||||
self.push_spec(spec.clone());
|
||||
if let Some(spec) = executor.spec() {
|
||||
self.push_spec(spec);
|
||||
}
|
||||
|
||||
let handler: Arc<dyn AnyToolHandler> = Arc::new(BundledToolHandler::new(bundle, spec));
|
||||
let handler: Arc<dyn AnyToolHandler> = Arc::new(ExtensionToolHandler::new(executor));
|
||||
self.handlers.insert(tool_name, handler);
|
||||
}
|
||||
|
||||
|
||||
@@ -9,13 +9,13 @@ use crate::tools::registry::AnyToolResult;
|
||||
use crate::tools::registry::ToolArgumentDiffConsumer;
|
||||
use crate::tools::registry::ToolRegistry;
|
||||
use crate::tools::spec::build_specs_with_discoverable_tools;
|
||||
use codex_extension_api::ExtensionToolExecutor;
|
||||
use codex_mcp::ToolInfo;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_protocol::models::LocalShellAction;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::models::SearchToolCallParams;
|
||||
use codex_protocol::models::ShellToolCallParams;
|
||||
use codex_tool_api::ToolBundle as ExtensionToolBundle;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::ResponsesApiNamespaceTool;
|
||||
use codex_tools::ToolName;
|
||||
@@ -44,7 +44,7 @@ pub(crate) struct ToolRouterParams<'a> {
|
||||
pub(crate) mcp_tools: Option<Vec<ToolInfo>>,
|
||||
pub(crate) deferred_mcp_tools: Option<Vec<ToolInfo>>,
|
||||
pub(crate) discoverable_tools: Option<Vec<DiscoverableTool>>,
|
||||
pub(crate) extension_tool_bundles: Vec<ExtensionToolBundle>,
|
||||
pub(crate) extension_tool_executors: Vec<Arc<dyn ExtensionToolExecutor>>,
|
||||
pub(crate) dynamic_tools: &'a [DynamicToolSpec],
|
||||
}
|
||||
|
||||
@@ -54,7 +54,7 @@ impl ToolRouter {
|
||||
mcp_tools,
|
||||
deferred_mcp_tools,
|
||||
discoverable_tools,
|
||||
extension_tool_bundles,
|
||||
extension_tool_executors,
|
||||
dynamic_tools,
|
||||
} = params;
|
||||
let builder = build_specs_with_discoverable_tools(
|
||||
@@ -62,7 +62,7 @@ impl ToolRouter {
|
||||
mcp_tools,
|
||||
deferred_mcp_tools,
|
||||
discoverable_tools,
|
||||
&extension_tool_bundles,
|
||||
&extension_tool_executors,
|
||||
dynamic_tools,
|
||||
);
|
||||
let (specs, registry) = builder.build();
|
||||
@@ -217,7 +217,7 @@ impl ToolRouter {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn extension_tool_bundles(session: &Session) -> Vec<ExtensionToolBundle> {
|
||||
pub(crate) fn extension_tool_executors(session: &Session) -> Vec<Arc<dyn ExtensionToolExecutor>> {
|
||||
session
|
||||
.services
|
||||
.extensions
|
||||
|
||||
@@ -7,15 +7,14 @@ use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_extension_api::ExtensionRegistry;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
use codex_extension_api::FunctionToolSpec;
|
||||
use codex_extension_api::ToolBundle;
|
||||
use codex_extension_api::ToolExecutor;
|
||||
use codex_extension_api::ToolFuture;
|
||||
use codex_extension_api::ExtensionToolExecutor;
|
||||
use codex_extension_api::ExtensionToolOutput;
|
||||
use codex_extension_api::ResponsesApiTool;
|
||||
use codex_extension_api::ToolCall as ExtensionToolCall;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_tool_api::ToolCall as ExtensionToolCall;
|
||||
use codex_tools::ResponsesApiNamespaceTool;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolSpec;
|
||||
@@ -27,7 +26,7 @@ use super::ToolCall;
|
||||
use super::ToolCallSource;
|
||||
use super::ToolRouter;
|
||||
use super::ToolRouterParams;
|
||||
use super::extension_tool_bundles;
|
||||
use super::extension_tool_executors;
|
||||
|
||||
struct ExtensionEchoContributor;
|
||||
|
||||
@@ -36,38 +35,46 @@ impl codex_extension_api::ToolContributor for ExtensionEchoContributor {
|
||||
&self,
|
||||
_session_store: &ExtensionData,
|
||||
_thread_store: &ExtensionData,
|
||||
) -> Vec<ToolBundle> {
|
||||
vec![ToolBundle::new(
|
||||
FunctionToolSpec {
|
||||
name: "extension_echo".to_string(),
|
||||
description: "Echoes arguments through an extension tool.".to_string(),
|
||||
strict: true,
|
||||
parameters: json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"message": { "type": "string" },
|
||||
},
|
||||
"required": ["message"],
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
},
|
||||
Arc::new(ExtensionEchoExecutor),
|
||||
)]
|
||||
) -> Vec<Arc<dyn ExtensionToolExecutor>> {
|
||||
vec![Arc::new(ExtensionEchoExecutor)]
|
||||
}
|
||||
}
|
||||
|
||||
struct ExtensionEchoExecutor;
|
||||
|
||||
impl ToolExecutor for ExtensionEchoExecutor {
|
||||
fn execute<'a>(&'a self, call: ExtensionToolCall) -> ToolFuture<'a> {
|
||||
impl ExtensionToolExecutor for ExtensionEchoExecutor {
|
||||
fn tool_name(&self) -> ToolName {
|
||||
ToolName::plain("extension_echo")
|
||||
}
|
||||
|
||||
fn spec(&self) -> Option<ToolSpec> {
|
||||
Some(ToolSpec::Function(ResponsesApiTool {
|
||||
name: "extension_echo".to_string(),
|
||||
description: "Echoes arguments through an extension tool.".to_string(),
|
||||
strict: true,
|
||||
parameters: codex_extension_api::parse_tool_input_schema(&json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"message": { "type": "string" },
|
||||
},
|
||||
"required": ["message"],
|
||||
"additionalProperties": false,
|
||||
}))
|
||||
.expect("extension schema should parse"),
|
||||
output_schema: None,
|
||||
defer_loading: None,
|
||||
}))
|
||||
}
|
||||
|
||||
fn handle(&self, call: ExtensionToolCall) -> codex_extension_api::ExtensionToolFuture<'_> {
|
||||
Box::pin(async move {
|
||||
let arguments: serde_json::Value =
|
||||
serde_json::from_str(&call.arguments).expect("test arguments should parse");
|
||||
Ok(json!({
|
||||
let arguments: serde_json::Value = serde_json::from_str(call.function_arguments()?)
|
||||
.expect("test arguments should parse");
|
||||
Ok(ExtensionToolOutput::new(json!({
|
||||
"arguments": arguments,
|
||||
"callId": call.call_id.clone(),
|
||||
"ok": true,
|
||||
}))
|
||||
})))
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -98,7 +105,7 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow
|
||||
deferred_mcp_tools: None,
|
||||
mcp_tools: Some(mcp_tools),
|
||||
discoverable_tools: None,
|
||||
extension_tool_bundles: Vec::new(),
|
||||
extension_tool_executors: Vec::new(),
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
},
|
||||
);
|
||||
@@ -177,7 +184,7 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> {
|
||||
),
|
||||
]),
|
||||
discoverable_tools: None,
|
||||
extension_tool_bundles: Vec::new(),
|
||||
extension_tool_executors: Vec::new(),
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
},
|
||||
);
|
||||
@@ -212,7 +219,7 @@ async fn tools_without_handlers_do_not_support_parallel() -> anyhow::Result<()>
|
||||
deferred_mcp_tools: None,
|
||||
mcp_tools: None,
|
||||
discoverable_tools: None,
|
||||
extension_tool_bundles: Vec::new(),
|
||||
extension_tool_executors: Vec::new(),
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
},
|
||||
);
|
||||
@@ -264,7 +271,7 @@ async fn specs_filter_deferred_dynamic_tools() -> anyhow::Result<()> {
|
||||
deferred_mcp_tools: None,
|
||||
mcp_tools: None,
|
||||
discoverable_tools: None,
|
||||
extension_tool_bundles: Vec::new(),
|
||||
extension_tool_executors: Vec::new(),
|
||||
dynamic_tools: &dynamic_tools,
|
||||
},
|
||||
);
|
||||
@@ -310,7 +317,7 @@ fn mcp_tool_info(
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn extension_tool_bundles_are_model_visible_and_dispatchable() -> anyhow::Result<()> {
|
||||
async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow::Result<()> {
|
||||
let (mut session, turn) = make_session_and_context().await;
|
||||
session.services.extensions = extension_tool_test_registry();
|
||||
|
||||
@@ -320,7 +327,7 @@ async fn extension_tool_bundles_are_model_visible_and_dispatchable() -> anyhow::
|
||||
deferred_mcp_tools: None,
|
||||
mcp_tools: None,
|
||||
discoverable_tools: None,
|
||||
extension_tool_bundles: extension_tool_bundles(&session),
|
||||
extension_tool_executors: extension_tool_executors(&session),
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
},
|
||||
);
|
||||
|
||||
@@ -7,12 +7,13 @@ use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions;
|
||||
use crate::tools::registry::ToolRegistryBuilder;
|
||||
use crate::tools::spec_plan::build_tool_registry_builder;
|
||||
use crate::tools::spec_plan_types::ToolRegistryBuildParams;
|
||||
use codex_extension_api::ExtensionToolExecutor;
|
||||
use codex_mcp::ToolInfo;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_tool_api::ToolBundle as ExtensionToolBundle;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::ToolUserShellType;
|
||||
use codex_tools::ToolsConfig;
|
||||
use std::sync::Arc;
|
||||
|
||||
pub(crate) fn tool_user_shell_type(user_shell: &Shell) -> ToolUserShellType {
|
||||
match user_shell.shell_type {
|
||||
@@ -29,7 +30,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
mcp_tools: Option<Vec<ToolInfo>>,
|
||||
deferred_mcp_tools: Option<Vec<ToolInfo>>,
|
||||
discoverable_tools: Option<Vec<DiscoverableTool>>,
|
||||
extension_tool_bundles: &[ExtensionToolBundle],
|
||||
extension_tool_executors: &[Arc<dyn ExtensionToolExecutor>],
|
||||
dynamic_tools: &[DynamicToolSpec],
|
||||
) -> ToolRegistryBuilder {
|
||||
let default_agent_type_description =
|
||||
@@ -50,7 +51,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
mcp_tools: mcp_tools.as_deref(),
|
||||
deferred_mcp_tools: deferred_mcp_tools.as_deref(),
|
||||
discoverable_tools: discoverable_tools.as_deref(),
|
||||
extension_tool_bundles,
|
||||
extension_tool_executors,
|
||||
dynamic_tools,
|
||||
default_agent_type_description: &default_agent_type_description,
|
||||
wait_agent_timeouts: WaitAgentTimeoutOptions {
|
||||
|
||||
@@ -27,7 +27,6 @@ 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::extension_tool_spec;
|
||||
use crate::tools::handlers::multi_agents::CloseAgentHandler;
|
||||
use crate::tools::handlers::multi_agents::ResumeAgentHandler;
|
||||
use crate::tools::handlers::multi_agents::SendInputHandler;
|
||||
@@ -49,6 +48,7 @@ use crate::tools::registry::AnyToolHandler;
|
||||
use crate::tools::registry::ToolRegistryBuilder;
|
||||
use crate::tools::spec_plan_types::ToolRegistryBuildParams;
|
||||
use crate::tools::spec_plan_types::agent_type_description;
|
||||
use codex_extension_api::ExtensionToolExecutor;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_tools::ResponsesApiNamespaceTool;
|
||||
use codex_tools::ToolEnvironmentMode;
|
||||
@@ -84,7 +84,7 @@ pub fn build_tool_registry_builder(
|
||||
for handler in build_code_mode_handlers(
|
||||
config,
|
||||
&handlers,
|
||||
params.extension_tool_bundles,
|
||||
params.extension_tool_executors,
|
||||
config.search_tool && !all_deferred_tools.is_empty(),
|
||||
) {
|
||||
builder.register_any_handler(handler);
|
||||
@@ -134,8 +134,8 @@ pub fn build_tool_registry_builder(
|
||||
builder.register_handler(Arc::new(ToolSearchHandler::new(deferred_search_infos)));
|
||||
}
|
||||
|
||||
for bundle in params.extension_tool_bundles.iter().cloned() {
|
||||
builder.register_tool_bundle(bundle);
|
||||
for executor in params.extension_tool_executors.iter().cloned() {
|
||||
builder.register_extension_tool_executor(executor);
|
||||
}
|
||||
|
||||
builder
|
||||
@@ -144,7 +144,7 @@ pub fn build_tool_registry_builder(
|
||||
fn build_code_mode_handlers(
|
||||
config: &ToolsConfig,
|
||||
handlers: &[Arc<dyn AnyToolHandler>],
|
||||
extension_tool_bundles: &[codex_tool_api::ToolBundle],
|
||||
extension_tool_executors: &[Arc<dyn ExtensionToolExecutor>],
|
||||
deferred_tools_available: bool,
|
||||
) -> Vec<Arc<dyn AnyToolHandler>> {
|
||||
if !config.code_mode_enabled {
|
||||
@@ -156,9 +156,9 @@ fn build_code_mode_handlers(
|
||||
.filter_map(|handler| handler.spec())
|
||||
.collect::<Vec<_>>();
|
||||
code_mode_nested_tool_specs.extend(
|
||||
extension_tool_bundles
|
||||
extension_tool_executors
|
||||
.iter()
|
||||
.filter_map(|bundle| extension_tool_spec(bundle.spec()).ok()),
|
||||
.filter_map(|executor| executor.spec()),
|
||||
);
|
||||
let namespace_descriptions = code_mode_namespace_descriptions(&code_mode_nested_tool_specs);
|
||||
let mut enabled_tools =
|
||||
|
||||
@@ -26,6 +26,8 @@ use crate::tools::handlers::view_image_spec::ViewImageToolOptions;
|
||||
use crate::tools::handlers::view_image_spec::create_view_image_tool;
|
||||
use crate::tools::registry::ToolRegistry;
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
use codex_extension_api::ExtensionToolExecutor;
|
||||
use codex_extension_api::ToolCall as ExtensionToolCall;
|
||||
use codex_features::Feature;
|
||||
use codex_features::Features;
|
||||
use codex_mcp::ToolInfo;
|
||||
@@ -41,10 +43,6 @@ use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::WebSearchToolType;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_tool_api::FunctionToolSpec;
|
||||
use codex_tool_api::ToolBundle as ExtensionToolBundle;
|
||||
use codex_tool_api::ToolExecutor;
|
||||
use codex_tool_api::ToolFuture;
|
||||
use codex_tools::AdditionalProperties;
|
||||
use codex_tools::DiscoverablePluginInfo;
|
||||
use codex_tools::DiscoverableTool;
|
||||
@@ -67,6 +65,7 @@ use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
const CODEX_APPS_MCP_SERVER_NAME: &str = "codex_apps";
|
||||
const DEFAULT_AGENT_TYPE_DESCRIPTION: &str = "Test agent type description.";
|
||||
@@ -74,32 +73,44 @@ const DEFAULT_WAIT_TIMEOUT_MS: i64 = 30_000;
|
||||
const MIN_WAIT_TIMEOUT_MS: i64 = 10_000;
|
||||
const MAX_WAIT_TIMEOUT_MS: i64 = 3_600_000;
|
||||
|
||||
struct UnusedExtensionExecutor;
|
||||
|
||||
impl ToolExecutor for UnusedExtensionExecutor {
|
||||
fn execute<'a>(&'a self, _call: codex_tool_api::ToolCall) -> ToolFuture<'a> {
|
||||
Box::pin(async { panic!("spec planning should not execute extension tools") })
|
||||
fn extension_tool_executor(name: &str, description: &str) -> Arc<dyn ExtensionToolExecutor> {
|
||||
struct SpecOnlyExtensionExecutor {
|
||||
name: String,
|
||||
description: String,
|
||||
}
|
||||
}
|
||||
|
||||
fn extension_tool_bundle(name: &str, description: &str) -> ExtensionToolBundle {
|
||||
ExtensionToolBundle::new(
|
||||
FunctionToolSpec {
|
||||
name: name.to_string(),
|
||||
description: description.to_string(),
|
||||
strict: true,
|
||||
parameters: serde_json::to_value(JsonSchema::object(
|
||||
BTreeMap::from([(
|
||||
"message".to_string(),
|
||||
JsonSchema::string(/*description*/ None),
|
||||
)]),
|
||||
Some(vec!["message".to_string()]),
|
||||
Some(false.into()),
|
||||
))
|
||||
.expect("extension schema should serialize"),
|
||||
},
|
||||
std::sync::Arc::new(UnusedExtensionExecutor),
|
||||
)
|
||||
impl ExtensionToolExecutor for SpecOnlyExtensionExecutor {
|
||||
fn tool_name(&self) -> ToolName {
|
||||
ToolName::plain(self.name.as_str())
|
||||
}
|
||||
|
||||
fn spec(&self) -> Option<ToolSpec> {
|
||||
Some(ToolSpec::Function(ResponsesApiTool {
|
||||
name: self.name.clone(),
|
||||
description: self.description.clone(),
|
||||
strict: true,
|
||||
parameters: JsonSchema::object(
|
||||
BTreeMap::from([(
|
||||
"message".to_string(),
|
||||
JsonSchema::string(/*description*/ None),
|
||||
)]),
|
||||
Some(vec!["message".to_string()]),
|
||||
Some(false.into()),
|
||||
),
|
||||
output_schema: None,
|
||||
defer_loading: None,
|
||||
}))
|
||||
}
|
||||
|
||||
fn handle(&self, _call: ExtensionToolCall) -> codex_extension_api::ExtensionToolFuture<'_> {
|
||||
Box::pin(async { panic!("spec planning should not execute extension tools") })
|
||||
}
|
||||
}
|
||||
|
||||
Arc::new(SpecOnlyExtensionExecutor {
|
||||
name: name.to_string(),
|
||||
description: description.to_string(),
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -116,7 +127,7 @@ fn extension_tools_do_not_replace_builtin_tools() {
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
let extension_tool_bundles = vec![extension_tool_bundle(
|
||||
let extension_tool_executors = vec![extension_tool_executor(
|
||||
"update_plan",
|
||||
"Extension attempt to replace a built-in tool.",
|
||||
)];
|
||||
@@ -125,7 +136,7 @@ fn extension_tools_do_not_replace_builtin_tools() {
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
/*discoverable_tools*/ None,
|
||||
&extension_tool_bundles,
|
||||
&extension_tool_executors,
|
||||
&[],
|
||||
);
|
||||
|
||||
@@ -1880,7 +1891,7 @@ fn request_plugin_install_is_not_registered_without_feature_flag() {
|
||||
"Google Calendar",
|
||||
"Plan events and schedules.",
|
||||
)]),
|
||||
/*extension_tool_bundles*/ &[],
|
||||
/*extension_tool_executors*/ &[],
|
||||
&[],
|
||||
);
|
||||
|
||||
@@ -1921,7 +1932,7 @@ fn request_plugin_install_can_be_registered_without_search_tool() {
|
||||
"Google Calendar",
|
||||
"Plan events and schedules.",
|
||||
)]),
|
||||
/*extension_tool_bundles*/ &[],
|
||||
/*extension_tool_executors*/ &[],
|
||||
&[],
|
||||
);
|
||||
|
||||
@@ -1986,7 +1997,7 @@ fn request_plugin_install_description_lists_discoverable_tools() {
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
Some(discoverable_tools),
|
||||
/*extension_tool_bundles*/ &[],
|
||||
/*extension_tool_executors*/ &[],
|
||||
&[],
|
||||
);
|
||||
assert!(registry.has_handler(&ToolName::plain(REQUEST_PLUGIN_INSTALL_TOOL_NAME)));
|
||||
@@ -2279,7 +2290,7 @@ fn code_mode_only_exec_description_includes_extension_tool_details() {
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
|
||||
let extension_tool_bundles = vec![extension_tool_bundle(
|
||||
let extension_tool_executors = vec![extension_tool_executor(
|
||||
"extension_echo",
|
||||
"Echoes arguments through an extension tool.",
|
||||
)];
|
||||
@@ -2288,7 +2299,7 @@ fn code_mode_only_exec_description_includes_extension_tool_details() {
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
/*discoverable_tools*/ None,
|
||||
&extension_tool_bundles,
|
||||
&extension_tool_executors,
|
||||
&[],
|
||||
);
|
||||
let ToolSpec::Freeform(FreeformTool { description, .. }) = find_tool(&tools, "exec") else {
|
||||
@@ -2386,7 +2397,7 @@ fn build_specs(
|
||||
mcp_tools,
|
||||
deferred_mcp_tools,
|
||||
/*discoverable_tools*/ None,
|
||||
/*extension_tool_bundles*/ &[],
|
||||
/*extension_tool_executors*/ &[],
|
||||
dynamic_tools,
|
||||
)
|
||||
}
|
||||
@@ -2396,7 +2407,7 @@ fn build_specs_with_discoverable_tools(
|
||||
mcp_tools: Option<HashMap<ToolName, rmcp::model::Tool>>,
|
||||
deferred_mcp_tools: Option<Vec<ToolInfo>>,
|
||||
discoverable_tools: Option<Vec<DiscoverableTool>>,
|
||||
extension_tool_bundles: &[codex_tool_api::ToolBundle],
|
||||
extension_tool_executors: &[Arc<dyn ExtensionToolExecutor>],
|
||||
dynamic_tools: &[DynamicToolSpec],
|
||||
) -> (Vec<ToolSpec>, ToolRegistry) {
|
||||
let mcp_tool_inputs = mcp_tools.as_ref().map(|mcp_tools| {
|
||||
@@ -2411,7 +2422,7 @@ fn build_specs_with_discoverable_tools(
|
||||
mcp_tools: mcp_tool_inputs.as_deref(),
|
||||
deferred_mcp_tools: deferred_mcp_tools.as_deref(),
|
||||
discoverable_tools: discoverable_tools.as_deref(),
|
||||
extension_tool_bundles,
|
||||
extension_tool_executors,
|
||||
dynamic_tools,
|
||||
default_agent_type_description: DEFAULT_AGENT_TYPE_DESCRIPTION,
|
||||
wait_agent_timeouts: wait_agent_timeout_options(),
|
||||
|
||||
@@ -1,16 +1,17 @@
|
||||
use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions;
|
||||
use codex_extension_api::ExtensionToolExecutor;
|
||||
use codex_mcp::ToolInfo;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_tool_api::ToolBundle as ExtensionToolBundle;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::ToolsConfig;
|
||||
use std::sync::Arc;
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
pub struct ToolRegistryBuildParams<'a> {
|
||||
pub mcp_tools: Option<&'a [ToolInfo]>,
|
||||
pub deferred_mcp_tools: Option<&'a [ToolInfo]>,
|
||||
pub discoverable_tools: Option<&'a [DiscoverableTool]>,
|
||||
pub extension_tool_bundles: &'a [ExtensionToolBundle],
|
||||
pub extension_tool_executors: &'a [Arc<dyn ExtensionToolExecutor>],
|
||||
pub dynamic_tools: &'a [DynamicToolSpec],
|
||||
pub default_agent_type_description: &'a str,
|
||||
pub wait_agent_timeouts: WaitAgentTimeoutOptions,
|
||||
|
||||
@@ -275,7 +275,7 @@ fn build_specs(
|
||||
mcp_tools,
|
||||
deferred_mcp_tools,
|
||||
/*discoverable_tools*/ None,
|
||||
/*extension_tool_bundles*/ &[],
|
||||
/*extension_tool_executors*/ &[],
|
||||
dynamic_tools,
|
||||
)
|
||||
}
|
||||
@@ -335,7 +335,7 @@ async fn assert_model_tools(
|
||||
mcp_tools: None,
|
||||
deferred_mcp_tools: None,
|
||||
discoverable_tools: None,
|
||||
extension_tool_bundles: Vec::new(),
|
||||
extension_tool_executors: Vec::new(),
|
||||
dynamic_tools: &[],
|
||||
},
|
||||
);
|
||||
@@ -804,7 +804,7 @@ async fn request_plugin_install_requires_apps_and_plugins_features() {
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
discoverable_tools.clone(),
|
||||
/*extension_tool_bundles*/ &[],
|
||||
/*extension_tool_executors*/ &[],
|
||||
&[],
|
||||
)
|
||||
.build();
|
||||
|
||||
Reference in new issue
Block a user