Skip to content

Commit c9e46ed

Browse files
authored
[codex] Make handlers own parallel tool support (openai#22254)
## Why `ToolRouter::tool_supports_parallel()` was still consulting configured specs when a handler lookup missed, even though parallel schedulability is really a property of the executable handler. Keeping that metadata on `ConfiguredToolSpec` duplicated state between the model-visible spec layer and the runtime handler layer. This change makes handlers the sole source of truth for parallel tool support and removes the extra spec wrapper that only existed to carry duplicated metadata. ## What changed - removed `ConfiguredToolSpec` and store plain `ToolSpec` values in the registry/router builder path - changed `ToolRouter::tool_supports_parallel()` to consult only the handler registry and fall back to `false` - simplified spec collection and test helpers to operate directly on `ToolSpec` - updated router/spec tests to cover handler-owned parallel behavior and the no-handler fallback ## Validation - `cargo test -p codex-tools` - `cargo test -p codex-core mcp_parallel_support_uses_handler_data` - `cargo test -p codex-core deferred_responses_api_tool_serializes_with_defer_loading` - `cargo test -p codex-core tools_without_handlers_do_not_support_parallel` - `cargo test -p codex-core request_plugin_install_can_be_registered_without_search_tool` ## Docs No documentation updates needed.
1 parent 79c65f8 commit c9e46ed

10 files changed

Lines changed: 115 additions & 185 deletions

File tree

codex-rs/core/src/tools/registry.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ use crate::util::error_or_panic;
2525
use codex_protocol::models::ResponseInputItem;
2626
use codex_protocol::protocol::EventMsg;
2727
use codex_tool_api::ToolBundle as ExtensionToolBundle;
28-
use codex_tools::ConfiguredToolSpec;
2928
use codex_tools::ToolName;
3029
use codex_tools::ToolSpec;
3130
use codex_utils_readiness::Readiness;
@@ -554,7 +553,7 @@ impl ToolRegistry {
554553

555554
pub struct ToolRegistryBuilder {
556555
handlers: HashMap<ToolName, Arc<dyn AnyToolHandler>>,
557-
specs: Vec<ConfiguredToolSpec>,
556+
specs: Vec<ToolSpec>,
558557
code_mode_enabled: bool,
559558
}
560559

@@ -567,14 +566,13 @@ impl ToolRegistryBuilder {
567566
}
568567
}
569568

570-
pub(crate) fn push_spec(&mut self, spec: ToolSpec, supports_parallel_tool_calls: bool) {
569+
pub(crate) fn push_spec(&mut self, spec: ToolSpec) {
571570
let spec = if self.code_mode_enabled {
572571
codex_tools::augment_tool_spec_for_code_mode(spec)
573572
} else {
574573
spec
575574
};
576-
self.specs
577-
.push(ConfiguredToolSpec::new(spec, supports_parallel_tool_calls));
575+
self.specs.push(spec);
578576
}
579577

580578
pub fn register_handler<H>(&mut self, handler: Arc<H>)
@@ -588,8 +586,7 @@ impl ToolRegistryBuilder {
588586
}
589587

590588
if let Some(spec) = handler.spec() {
591-
let supports_parallel_tool_calls = handler.supports_parallel_tool_calls();
592-
self.push_spec(spec, supports_parallel_tool_calls);
589+
self.push_spec(spec);
593590
}
594591

595592
let handler: Arc<dyn AnyToolHandler> = handler;
@@ -612,17 +609,17 @@ impl ToolRegistryBuilder {
612609
return;
613610
}
614611
};
615-
self.push_spec(spec.clone(), /*supports_parallel_tool_calls*/ false);
612+
self.push_spec(spec.clone());
616613

617614
let handler: Arc<dyn AnyToolHandler> = Arc::new(BundledToolHandler::new(bundle, spec));
618615
self.handlers.insert(tool_name, handler);
619616
}
620617

621-
pub(crate) fn specs(&self) -> &[ConfiguredToolSpec] {
618+
pub(crate) fn specs(&self) -> &[ToolSpec] {
622619
&self.specs
623620
}
624621

625-
pub fn build(self) -> (Vec<ConfiguredToolSpec>, ToolRegistry) {
622+
pub fn build(self) -> (Vec<ToolSpec>, ToolRegistry) {
626623
let registry = ToolRegistry::new(self.handlers);
627624
(self.specs, registry)
628625
}

codex-rs/core/src/tools/registry_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ fn register_handler_adds_handler_and_augments_specs_for_code_mode() {
7171

7272
assert_eq!(specs.len(), 1);
7373
assert_eq!(
74-
specs[0].spec,
74+
specs[0],
7575
codex_tools::augment_tool_spec_for_code_mode(create_get_goal_tool())
7676
);
7777
assert!(registry.has_handler(&codex_tools::ToolName::plain(GET_GOAL_TOOL_NAME)));

codex-rs/core/src/tools/router.rs

Lines changed: 9 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ use codex_protocol::models::ResponseItem;
1616
use codex_protocol::models::SearchToolCallParams;
1717
use codex_protocol::models::ShellToolCallParams;
1818
use codex_tool_api::ToolBundle as ExtensionToolBundle;
19-
use codex_tools::ConfiguredToolSpec;
2019
use codex_tools::DiscoverableTool;
2120
use codex_tools::ResponsesApiNamespaceTool;
2221
use codex_tools::ToolName;
@@ -38,7 +37,7 @@ pub struct ToolCall {
3837

3938
pub struct ToolRouter {
4039
registry: ToolRegistry,
41-
specs: Vec<ConfiguredToolSpec>,
40+
specs: Vec<ToolSpec>,
4241
model_visible_specs: Vec<ToolSpec>,
4342
}
4443

@@ -78,17 +77,14 @@ impl ToolRouter {
7877
.collect::<HashSet<_>>();
7978
let model_visible_specs = specs
8079
.iter()
81-
.filter_map(|configured_tool| {
80+
.filter_map(|spec| {
8281
if config.code_mode_only_enabled
83-
&& codex_code_mode::is_code_mode_nested_tool(configured_tool.name())
82+
&& codex_code_mode::is_code_mode_nested_tool(spec.name())
8483
{
8584
return None;
8685
}
8786

88-
filter_deferred_dynamic_tool_spec(
89-
configured_tool.spec.clone(),
90-
&deferred_dynamic_tools,
91-
)
87+
filter_deferred_dynamic_tool_spec(spec.clone(), &deferred_dynamic_tools)
9288
})
9389
.collect();
9490

@@ -100,27 +96,24 @@ impl ToolRouter {
10096
}
10197

10298
pub fn specs(&self) -> Vec<ToolSpec> {
103-
self.specs
104-
.iter()
105-
.map(|config| config.spec.clone())
106-
.collect()
99+
self.specs.clone()
107100
}
108101

109102
pub fn model_visible_specs(&self) -> Vec<ToolSpec> {
110103
self.model_visible_specs.clone()
111104
}
112105

113106
pub fn find_spec(&self, tool_name: &ToolName) -> Option<ToolSpec> {
114-
self.specs.iter().find_map(|config| match &config.spec {
107+
self.specs.iter().find_map(|spec| match spec {
115108
ToolSpec::Function(tool)
116109
if tool_name.namespace.is_none() && tool.name == tool_name.name =>
117110
{
118-
Some(config.spec.clone())
111+
Some(spec.clone())
119112
}
120113
ToolSpec::Freeform(tool)
121114
if tool_name.namespace.is_none() && tool.name == tool_name.name =>
122115
{
123-
Some(config.spec.clone())
116+
Some(spec.clone())
124117
}
125118
ToolSpec::Namespace(namespace) => namespace.tools.iter().find_map(|tool| match tool {
126119
ResponsesApiNamespaceTool::Function(tool)
@@ -142,29 +135,10 @@ impl ToolRouter {
142135
self.registry.create_diff_consumer(tool_name)
143136
}
144137

145-
fn configured_tool_supports_parallel(&self, tool_name: &ToolName) -> bool {
146-
if tool_name.namespace.is_some() {
147-
return false;
148-
}
149-
150-
self.specs
151-
.iter()
152-
.filter(|config| config.supports_parallel_tool_calls)
153-
.any(|config| match &config.spec {
154-
ToolSpec::Function(tool) => tool.name == tool_name.name.as_str(),
155-
ToolSpec::Freeform(tool) => tool.name == tool_name.name.as_str(),
156-
ToolSpec::Namespace(_)
157-
| ToolSpec::ToolSearch { .. }
158-
| ToolSpec::LocalShell {}
159-
| ToolSpec::ImageGeneration { .. }
160-
| ToolSpec::WebSearch { .. } => false,
161-
})
162-
}
163-
164138
pub fn tool_supports_parallel(&self, call: &ToolCall) -> bool {
165139
self.registry
166140
.supports_parallel_tool_calls(&call.tool_name)
167-
.unwrap_or_else(|| self.configured_tool_supports_parallel(&call.tool_name))
141+
.unwrap_or(false)
168142
}
169143

170144
#[instrument(level = "trace", skip_all, err)]

codex-rs/core/src/tools/router_tests.rs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ async fn build_tool_call_uses_namespace_for_registry_name() -> anyhow::Result<()
157157
}
158158

159159
#[tokio::test]
160-
async fn mcp_parallel_support_uses_exact_payload_server() -> anyhow::Result<()> {
160+
async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> {
161161
let (_, turn) = make_session_and_context().await;
162162
let router = ToolRouter::from_config(
163163
&turn.tools_config,
@@ -184,14 +184,14 @@ async fn mcp_parallel_support_uses_exact_payload_server() -> anyhow::Result<()>
184184
},
185185
);
186186

187-
let deferred_call = ToolCall {
187+
let call = ToolCall {
188188
tool_name: ToolName::namespaced("mcp__echo__", "query_with_delay"),
189-
call_id: "call-deferred".to_string(),
189+
call_id: "call-handler".to_string(),
190190
payload: ToolPayload::Function {
191191
arguments: "{}".to_string(),
192192
},
193193
};
194-
assert!(router.tool_supports_parallel(&deferred_call));
194+
assert!(router.tool_supports_parallel(&call));
195195

196196
let different_server_call = ToolCall {
197197
tool_name: ToolName::namespaced("mcp__hello_echo__", "query_with_delay"),
@@ -205,6 +205,32 @@ async fn mcp_parallel_support_uses_exact_payload_server() -> anyhow::Result<()>
205205
Ok(())
206206
}
207207

208+
#[tokio::test]
209+
async fn tools_without_handlers_do_not_support_parallel() -> anyhow::Result<()> {
210+
let (_, turn) = make_session_and_context().await;
211+
let router = ToolRouter::from_config(
212+
&turn.tools_config,
213+
ToolRouterParams {
214+
deferred_mcp_tools: None,
215+
mcp_tools: None,
216+
unavailable_called_tools: Vec::new(),
217+
discoverable_tools: None,
218+
extension_tool_bundles: Vec::new(),
219+
dynamic_tools: turn.dynamic_tools.as_slice(),
220+
},
221+
);
222+
223+
assert!(!router.tool_supports_parallel(&ToolCall {
224+
tool_name: ToolName::plain("web_search"),
225+
call_id: "call-web-search".to_string(),
226+
payload: ToolPayload::Function {
227+
arguments: "{}".to_string(),
228+
},
229+
}));
230+
231+
Ok(())
232+
}
233+
208234
#[tokio::test]
209235
async fn model_visible_specs_filter_deferred_dynamic_tools() -> anyhow::Result<()> {
210236
let (_, turn) = make_session_and_context().await;

codex-rs/core/src/tools/spec_plan.rs

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,8 @@ pub fn build_tool_registry_builder(
9797
..params
9898
},
9999
);
100-
let mut enabled_tools = collect_code_mode_exec_prompt_tool_definitions(
101-
nested_builder
102-
.specs()
103-
.iter()
104-
.map(|configured_tool| &configured_tool.spec),
105-
);
100+
let mut enabled_tools =
101+
collect_code_mode_exec_prompt_tool_definitions(nested_builder.specs().iter());
106102
enabled_tools
107103
.sort_by(|left, right| compare_code_mode_tools(left, right, &namespace_descriptions));
108104
builder.register_handler(Arc::new(CodeModeExecuteHandler::new(
@@ -277,14 +273,11 @@ pub fn build_tool_registry_builder(
277273
web_search_config: config.web_search_config.as_ref(),
278274
web_search_tool_type: config.web_search_tool_type,
279275
}) {
280-
builder.push_spec(web_search_tool, /*supports_parallel_tool_calls*/ false);
276+
builder.push_spec(web_search_tool);
281277
}
282278

283279
if config.image_gen_tool {
284-
builder.push_spec(
285-
create_image_generation_tool("png"),
286-
/*supports_parallel_tool_calls*/ false,
287-
);
280+
builder.push_spec(create_image_generation_tool("png"));
288281
}
289282

290283
if config.environment_mode.has_environment() {
@@ -391,14 +384,11 @@ pub fn build_tool_registry_builder(
391384
}
392385

393386
if config.namespace_tools && !tools.is_empty() {
394-
builder.push_spec(
395-
ToolSpec::Namespace(ResponsesApiNamespace {
396-
name: namespace,
397-
description,
398-
tools,
399-
}),
400-
/*supports_parallel_tool_calls*/ false,
401-
);
387+
builder.push_spec(ToolSpec::Namespace(ResponsesApiNamespace {
388+
name: namespace,
389+
description,
390+
tools,
391+
}));
402392
}
403393
}
404394
}
@@ -422,7 +412,7 @@ pub fn build_tool_registry_builder(
422412
for spec in coalesce_loadable_tool_specs(dynamic_tool_specs) {
423413
let spec = spec.into();
424414
if config.namespace_tools || !matches!(spec, ToolSpec::Namespace(_)) {
425-
builder.push_spec(spec, /*supports_parallel_tool_calls*/ false);
415+
builder.push_spec(spec);
426416
}
427417
}
428418

0 commit comments

Comments
 (0)