mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Refactor namespaced tool spec registration (#22256)
## Summary This refactor makes tool handlers the owner of the specs they can publish, so registry construction can register handlers once and separately publish only the specs that should be model-visible. The main motivation is deferred tools: MCP and dynamic tools still need handlers registered up front, but deferred tools should be discoverable through `tool_search` rather than emitted in the initial tool spec list. ## What changed - `McpHandler` and `DynamicToolHandler` can return their own `ToolSpec`. - `build_tool_registry_builder` now collects handlers, registers them through the no-spec path, and publishes only non-deferred handler specs. - Deferred MCP and dynamic tool names are combined into one `all_deferred_tools` set that drives spec filtering, code-mode deferred-tool signaling, and `tool_search` registration. - `tool_search` registration now requires both deferred tools and `namespace_tools`. - Namespace specs are merged in `spec_plan`, preserving top-level spec order, sorting tools within each namespace, and backfilling empty namespace descriptions. - Hosted web search and image-generation specs are included in the collected spec vector before namespace merge/publication, and tool-name tests that should not care about hosted relative order now compare sets. ## Testing - `cargo test -p codex-core tools::spec::tests:: -- --nocapture` - `cargo test -p codex-core tools::spec_plan::tests:: -- --nocapture` - `cargo test -p codex-core tools::router::tests::specs_filter_deferred_dynamic_tools -- --nocapture` - `cargo test -p codex-core suite::prompt_caching::prompt_tools_are_consistent_across_requests -- --nocapture` - `just fmt` - `just fix -p codex-core` - `cargo test -p codex-core -- --skip tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed` passed the library suite after skipping the known stack-overflowing unit test. Full `cargo test -p codex-core` currently hits a stack overflow in `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed`; the same focused test reproduces on `origin/main`.
This commit is contained in:
1 parent
b6e718591b
commit
0173f71143
14 files changed
+342
-358
No files matched your search
@@ -365,7 +365,7 @@ async fn assert_model_tools(
|
||||
.iter()
|
||||
.map(ToolSpec::name)
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(&tool_names, &expected_tools,);
|
||||
assert_eq!(&tool_names, &expected_tools);
|
||||
}
|
||||
|
||||
async fn assert_default_model_tools(
|
||||
@@ -396,14 +396,14 @@ async fn test_build_specs_gpt5_codex_default() {
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
"view_image",
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
@@ -421,14 +421,14 @@ async fn test_build_specs_gpt51_codex_default() {
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
"view_image",
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
@@ -448,14 +448,14 @@ async fn test_build_specs_gpt5_codex_unified_exec_web_search() {
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
"view_image",
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
@@ -475,14 +475,14 @@ async fn test_build_specs_gpt51_codex_unified_exec_web_search() {
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
"view_image",
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
@@ -500,14 +500,14 @@ async fn test_gpt_5_1_codex_max_defaults() {
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
"view_image",
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
@@ -525,14 +525,14 @@ async fn test_codex_5_1_mini_defaults() {
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
"view_image",
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
@@ -550,14 +550,14 @@ async fn test_gpt_5_defaults() {
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
"view_image",
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
@@ -575,14 +575,14 @@ async fn test_gpt_5_1_defaults() {
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
"view_image",
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
@@ -602,14 +602,14 @@ async fn test_gpt_5_1_codex_max_unified_exec_web_search() {
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
"view_image",
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"web_search",
|
||||
"image_generation",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
@@ -841,7 +841,7 @@ async fn request_plugin_install_requires_apps_and_plugins_features() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn search_tool_description_handles_no_enabled_mcp_tools() {
|
||||
async fn search_tool_is_hidden_without_deferred_tools() {
|
||||
let model_info = search_capable_model_info().await;
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::Apps);
|
||||
@@ -865,13 +865,11 @@ async fn search_tool_description_handles_no_enabled_mcp_tools() {
|
||||
&[],
|
||||
)
|
||||
.build();
|
||||
let search_tool = find_tool(&tools, TOOL_SEARCH_TOOL_NAME);
|
||||
let ToolSpec::ToolSearch { description, .. } = search_tool else {
|
||||
panic!("expected tool_search tool");
|
||||
};
|
||||
|
||||
assert!(description.contains("None currently enabled."));
|
||||
assert!(!description.contains("{{source_descriptions}}"));
|
||||
assert!(
|
||||
!tools
|
||||
.iter()
|
||||
.any(|tool| tool.name() == TOOL_SEARCH_TOOL_NAME)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -1003,7 +1001,7 @@ async fn search_tool_registers_namespaced_mcp_tool_aliases() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn tool_search_entries_skip_namespace_outputs_when_namespace_tools_are_disabled() {
|
||||
async fn tool_search_entries_skip_mcp_outputs_when_namespace_tools_are_disabled() {
|
||||
let model_info = search_capable_model_info().await;
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
@@ -1048,11 +1046,15 @@ async fn tool_search_entries_skip_namespace_outputs_when_namespace_tools_are_dis
|
||||
.map(|entry| entry.output)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(outputs.len(), 1);
|
||||
match &outputs[0] {
|
||||
LoadableToolSpec::Function(tool) => assert_eq!(tool.name, "plain_dynamic"),
|
||||
LoadableToolSpec::Namespace(_) => panic!("namespace tool_search output should be hidden"),
|
||||
}
|
||||
assert_eq!(outputs.len(), 2);
|
||||
assert!(outputs.iter().any(|output| match output {
|
||||
LoadableToolSpec::Namespace(namespace) => namespace.name == "codex_app",
|
||||
LoadableToolSpec::Function(_) => false,
|
||||
}));
|
||||
assert!(outputs.iter().any(|output| match output {
|
||||
LoadableToolSpec::Function(tool) => tool.name == "plain_dynamic",
|
||||
LoadableToolSpec::Namespace(_) => false,
|
||||
}));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in new issue
Block a user