mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Move tool specs into core handlers (#21416)
## Why This is the first mechanical slice of moving tool spec ownership toward the handlers. `codex-tools` should keep shared primitives and conversion helpers, while builtin tool specs and registration planning live in `codex-core` with the handlers that own those tools. Keeping this PR to relocation and import updates isolates the copy/move review from the later logic change that wires specs through registered handlers. ## What changed - Moved builtin tool spec constructors from `codex-rs/tools/src` into `codex-rs/core/src/tools/handlers/*_spec.rs` or nearby core tool modules. - Moved the registry planning code into `codex-rs/core/src/tools/spec_plan.rs` and its associated types/tests into core. - Kept shared primitives in `codex-tools`, including `ToolSpec`, schema/types, discovery/config primitives, dynamic/MCP conversion helpers, and code-mode collection helpers. - Updated handlers that referenced moved argument types or tool-name constants to use the core spec modules. - Moved spec tests next to the moved spec modules. ## Verification - `cargo check -p codex-tools` - `cargo check -p codex-core` - `cargo test -p codex-tools` - `cargo test -p codex-core _spec::tests` - `cargo test -p codex-core tools::spec_plan::tests` - `just fix -p codex-tools` - `just fix -p codex-core` Note: I also tried the broader `cargo test -p codex-core tools::`; it reached the moved spec-plan/spec tests successfully, then aborted with a stack overflow in `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed`, which is outside this spec relocation.
This commit is contained in:
committed by
GitHub
Unverified
parent
d5eea229cc
commit
9417cf9696
@@ -1,16 +1,12 @@
|
||||
use crate::JsonSchema;
|
||||
use crate::LoadableToolSpec;
|
||||
use crate::ResponsesApiNamespace;
|
||||
use crate::ResponsesApiNamespaceTool;
|
||||
use crate::ResponsesApiTool;
|
||||
use crate::ToolName;
|
||||
use crate::ToolSpec;
|
||||
use crate::default_namespace_description;
|
||||
use crate::mcp_tool_to_deferred_responses_api_tool;
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
const TUI_CLIENT_NAME: &str = "codex-tui";
|
||||
pub const TOOL_SEARCH_TOOL_NAME: &str = "tool_search";
|
||||
@@ -47,15 +43,6 @@ pub enum DiscoverableToolType {
|
||||
Plugin,
|
||||
}
|
||||
|
||||
impl DiscoverableToolType {
|
||||
fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::Connector => "connector",
|
||||
Self::Plugin => "plugin",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum DiscoverableToolAction {
|
||||
@@ -146,63 +133,6 @@ pub struct RequestPluginInstallEntry {
|
||||
pub app_connector_ids: Vec<String>,
|
||||
}
|
||||
|
||||
pub fn create_tool_search_tool(
|
||||
searchable_sources: &[ToolSearchSourceInfo],
|
||||
default_limit: usize,
|
||||
) -> ToolSpec {
|
||||
let properties = BTreeMap::from([
|
||||
(
|
||||
"query".to_string(),
|
||||
JsonSchema::string(Some("Search query for deferred tools.".to_string())),
|
||||
),
|
||||
(
|
||||
"limit".to_string(),
|
||||
JsonSchema::number(Some(format!(
|
||||
"Maximum number of tools to return (defaults to {default_limit})."
|
||||
))),
|
||||
),
|
||||
]);
|
||||
|
||||
let mut source_descriptions = BTreeMap::new();
|
||||
for source in searchable_sources {
|
||||
source_descriptions
|
||||
.entry(source.name.clone())
|
||||
.and_modify(|existing: &mut Option<String>| {
|
||||
if existing.is_none() {
|
||||
*existing = source.description.clone();
|
||||
}
|
||||
})
|
||||
.or_insert(source.description.clone());
|
||||
}
|
||||
|
||||
let source_descriptions = if source_descriptions.is_empty() {
|
||||
"None currently enabled.".to_string()
|
||||
} else {
|
||||
source_descriptions
|
||||
.into_iter()
|
||||
.map(|(name, description)| match description {
|
||||
Some(description) => format!("- {name}: {description}"),
|
||||
None => format!("- {name}"),
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
};
|
||||
|
||||
let description = format!(
|
||||
"# Tool discovery\n\nSearches over deferred tool metadata with BM25 and exposes matching tools for the next model call.\n\nYou have access to tools from the following sources:\n{source_descriptions}\nSome of the tools may not have been provided to you upfront, and you should use this tool (`{TOOL_SEARCH_TOOL_NAME}`) to search for the required tools. For MCP tool discovery, always use `{TOOL_SEARCH_TOOL_NAME}` instead of `list_mcp_resources` or `list_mcp_resource_templates`."
|
||||
);
|
||||
|
||||
ToolSpec::ToolSearch {
|
||||
execution: "client".to_string(),
|
||||
description,
|
||||
parameters: JsonSchema::object(
|
||||
properties,
|
||||
Some(vec!["query".to_string()]),
|
||||
Some(false.into()),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn tool_search_result_source_to_loadable_tool_spec(
|
||||
source: ToolSearchResultSource<'_>,
|
||||
) -> Result<LoadableToolSpec, serde_json::Error> {
|
||||
@@ -275,58 +205,6 @@ pub fn collect_tool_search_source_infos<'a>(
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub fn create_request_plugin_install_tool(
|
||||
discoverable_tools: &[RequestPluginInstallEntry],
|
||||
) -> ToolSpec {
|
||||
let properties = BTreeMap::from([
|
||||
(
|
||||
"tool_type".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Type of discoverable tool to suggest. Use \"connector\" or \"plugin\"."
|
||||
.to_string(),
|
||||
)),
|
||||
),
|
||||
(
|
||||
"action_type".to_string(),
|
||||
JsonSchema::string(Some("Suggested action for the tool. Use \"install\".".to_string())),
|
||||
),
|
||||
(
|
||||
"tool_id".to_string(),
|
||||
JsonSchema::string(Some("Connector or plugin id to suggest.".to_string())),
|
||||
),
|
||||
(
|
||||
"suggest_reason".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Concise one-line user-facing reason why this plugin or connector can help with the current request."
|
||||
.to_string(),
|
||||
)),
|
||||
),
|
||||
]);
|
||||
|
||||
let discoverable_tools = format_discoverable_tools(discoverable_tools);
|
||||
let description = format!(
|
||||
"# Request plugin/connector install\n\nUse this tool only to ask the user to install one known plugin or connector from the list below. The list contains known candidates that are not currently installed.\n\nUse this ONLY when all of the following are true:\n- The user explicitly asks to use a specific plugin or connector that is not already available in the current context or active `tools` list.\n- `{TOOL_SEARCH_TOOL_NAME}` is not available, or it has already been called and did not find or make the requested tool callable.\n- The plugin or connector is one of the known installable plugins or connectors listed below. Only ask to install plugins or connectors from this list.\n\nDo not use this tool for adjacent capabilities, broad recommendations, or tools that merely seem useful. Only use when the user explicitly asks to use that exact listed plugin or connector.\n\nKnown plugins/connectors available to install:\n{discoverable_tools}\n\nWorkflow:\n\n1. Check the current context and active `tools` list first. If current active tools aren't relevant and `{TOOL_SEARCH_TOOL_NAME}` is available, only call this tool after `{TOOL_SEARCH_TOOL_NAME}` has already been tried and found no relevant tool.\n2. Match the user's explicit request against the known plugin/connector list above. Only proceed when one listed plugin or connector exactly fits.\n3. If we found both connectors and plugins to install, use plugins first, only use connectors if the corresponding plugin is installed but the connector is not.\n4. If one plugin or connector clearly fits, call `{REQUEST_PLUGIN_INSTALL_TOOL_NAME}` with:\n - `tool_type`: `connector` or `plugin`\n - `action_type`: `install`\n - `tool_id`: exact id from the known plugin/connector list above\n - `suggest_reason`: concise one-line user-facing reason this plugin or connector can help with the current request\n5. After the request flow completes:\n - if the user finished the install flow, continue by searching again or using the newly available plugin or connector\n - if the user did not finish, continue without that plugin or connector, and don't request it again unless the user explicitly asks for it.\n\nIMPORTANT: DO NOT call this tool in parallel with other tools."
|
||||
);
|
||||
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: REQUEST_PLUGIN_INSTALL_TOOL_NAME.to_string(),
|
||||
description,
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(
|
||||
properties,
|
||||
Some(vec![
|
||||
"tool_type".to_string(),
|
||||
"action_type".to_string(),
|
||||
"tool_id".to_string(),
|
||||
"suggest_reason".to_string(),
|
||||
]),
|
||||
Some(false.into()),
|
||||
),
|
||||
output_schema: None,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn collect_request_plugin_install_entries(
|
||||
discoverable_tools: &[DiscoverableTool],
|
||||
) -> Vec<RequestPluginInstallEntry> {
|
||||
@@ -355,68 +233,6 @@ pub fn collect_request_plugin_install_entries(
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn format_discoverable_tools(discoverable_tools: &[RequestPluginInstallEntry]) -> String {
|
||||
let mut discoverable_tools = discoverable_tools.to_vec();
|
||||
discoverable_tools.sort_by(|left, right| {
|
||||
left.name
|
||||
.cmp(&right.name)
|
||||
.then_with(|| left.id.cmp(&right.id))
|
||||
});
|
||||
|
||||
discoverable_tools
|
||||
.into_iter()
|
||||
.map(|tool| {
|
||||
let description = tool_description_or_fallback(&tool);
|
||||
format!(
|
||||
"- {} (id: `{}`, type: {}, action: install): {}",
|
||||
tool.name,
|
||||
tool.id,
|
||||
tool.tool_type.as_str(),
|
||||
description
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
}
|
||||
|
||||
fn tool_description_or_fallback(tool: &RequestPluginInstallEntry) -> String {
|
||||
if let Some(description) = tool
|
||||
.description
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|description| !description.is_empty())
|
||||
{
|
||||
return description.to_string();
|
||||
}
|
||||
|
||||
match tool.tool_type {
|
||||
DiscoverableToolType::Connector => "No description provided.".to_string(),
|
||||
DiscoverableToolType::Plugin => plugin_summary(tool),
|
||||
}
|
||||
}
|
||||
|
||||
fn plugin_summary(tool: &RequestPluginInstallEntry) -> String {
|
||||
let mut details = Vec::new();
|
||||
if tool.has_skills {
|
||||
details.push("skills".to_string());
|
||||
}
|
||||
if !tool.mcp_server_names.is_empty() {
|
||||
details.push(format!("MCP servers: {}", tool.mcp_server_names.join(", ")));
|
||||
}
|
||||
if !tool.app_connector_ids.is_empty() {
|
||||
details.push(format!(
|
||||
"app connectors: {}",
|
||||
tool.app_connector_ids.join(", ")
|
||||
));
|
||||
}
|
||||
|
||||
if details.is_empty() {
|
||||
"No description provided.".to_string()
|
||||
} else {
|
||||
details.join("; ")
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "tool_discovery_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
Reference in New Issue
Block a user