mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Extract tool discovery helpers into codex-tools (#16477)
## Why Follow-up to #16379 and #16471. `codex-rs/core/src/tools/spec.rs` still owned the pure discovery-shaping helpers that turn app metadata and discoverable tool metadata into the inputs used by `tool_search` and `tool_suggest`. Those helpers do not need `codex-core` runtime state, so keeping them in `codex-core` continued to blur the crate boundary this migration is trying to tighten. This change keeps pushing spec-only logic behind the `codex-tools` API so `codex-core` can focus on wiring runtime handlers to the resulting tool definitions. ## What Changed - Added `collect_tool_search_app_infos` and `collect_tool_suggest_entries` to `codex-rs/tools/src/tool_discovery.rs`. - Added a small `ToolSearchAppSource` adapter type in `codex-tools` so `codex-core` can pass app metadata into that shared helper logic without exposing `ToolInfo` across the crate boundary. - Re-exported the new discovery helpers from `codex-rs/tools/src/lib.rs`, which remains exports-only. - Updated `codex-rs/core/src/tools/spec.rs` to use those `codex-tools` helpers instead of maintaining local `tool_search_app_infos` and `tool_suggest_entries` functions. - Removed the now-redundant helper implementations from `codex-core`. ## Testing - `cargo test -p codex-tools` - `cargo test -p codex-core tools::spec::tests`
This commit is contained in:
committed by
GitHub
Unverified
parent
148dbb25f0
commit
1b711a5501
@@ -22,15 +22,15 @@ use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_protocol::openai_models::WebSearchToolType;
|
||||
use codex_tools::CommandToolOptions;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::DiscoverableToolType;
|
||||
use codex_tools::ShellToolOptions;
|
||||
use codex_tools::SpawnAgentToolOptions;
|
||||
use codex_tools::ToolSearchAppInfo;
|
||||
use codex_tools::ToolSuggestEntry;
|
||||
use codex_tools::ToolSearchAppSource;
|
||||
use codex_tools::ToolUserShellType;
|
||||
use codex_tools::ViewImageToolOptions;
|
||||
use codex_tools::WaitAgentTimeoutOptions;
|
||||
use codex_tools::augment_tool_spec_for_code_mode;
|
||||
use codex_tools::collect_tool_search_app_infos;
|
||||
use codex_tools::collect_tool_suggest_entries;
|
||||
use codex_tools::create_apply_patch_freeform_tool;
|
||||
use codex_tools::create_apply_patch_json_tool;
|
||||
use codex_tools::create_assign_task_tool;
|
||||
@@ -367,12 +367,17 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
&& let Some(app_tools) = app_tools
|
||||
{
|
||||
let search_tool_handler = Arc::new(ToolSearchHandler::new(app_tools.clone()));
|
||||
let search_app_infos = collect_tool_search_app_infos(
|
||||
app_tools.values().map(|tool| ToolSearchAppSource {
|
||||
server_name: &tool.server_name,
|
||||
connector_name: tool.connector_name.as_deref(),
|
||||
connector_description: tool.connector_description.as_deref(),
|
||||
}),
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
);
|
||||
push_tool_spec(
|
||||
&mut builder,
|
||||
create_tool_search_tool(
|
||||
&tool_search_app_infos(&app_tools),
|
||||
TOOL_SEARCH_DEFAULT_LIMIT,
|
||||
),
|
||||
create_tool_search_tool(&search_app_infos, TOOL_SEARCH_DEFAULT_LIMIT),
|
||||
/*supports_parallel_tool_calls*/ true,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
@@ -392,7 +397,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
.filter(|tools| !tools.is_empty())
|
||||
{
|
||||
builder.push_spec_with_parallel_support(
|
||||
create_tool_suggest_tool(&tool_suggest_entries(discoverable_tools)),
|
||||
create_tool_suggest_tool(&collect_tool_suggest_entries(discoverable_tools)),
|
||||
/*supports_parallel_tool_calls*/ true,
|
||||
);
|
||||
builder.register_handler(TOOL_SUGGEST_TOOL_NAME, tool_suggest_handler);
|
||||
@@ -675,55 +680,6 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
|
||||
builder
|
||||
}
|
||||
|
||||
fn tool_search_app_infos(app_tools: &HashMap<String, ToolInfo>) -> Vec<ToolSearchAppInfo> {
|
||||
app_tools
|
||||
.values()
|
||||
.filter(|tool| tool.server_name == CODEX_APPS_MCP_SERVER_NAME)
|
||||
.filter_map(|tool| {
|
||||
let name = tool
|
||||
.connector_name
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|connector_name| !connector_name.is_empty())?
|
||||
.to_string();
|
||||
let description = tool
|
||||
.connector_description
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|connector_description| !connector_description.is_empty())
|
||||
.map(str::to_string);
|
||||
Some(ToolSearchAppInfo { name, description })
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn tool_suggest_entries(discoverable_tools: &[DiscoverableTool]) -> Vec<ToolSuggestEntry> {
|
||||
discoverable_tools
|
||||
.iter()
|
||||
.map(|tool| match tool {
|
||||
DiscoverableTool::Connector(connector) => ToolSuggestEntry {
|
||||
id: connector.id.clone(),
|
||||
name: connector.name.clone(),
|
||||
description: connector.description.clone(),
|
||||
tool_type: DiscoverableToolType::Connector,
|
||||
has_skills: false,
|
||||
mcp_server_names: Vec::new(),
|
||||
app_connector_ids: Vec::new(),
|
||||
},
|
||||
DiscoverableTool::Plugin(plugin) => ToolSuggestEntry {
|
||||
id: plugin.id.clone(),
|
||||
name: plugin.name.clone(),
|
||||
description: plugin.description.clone(),
|
||||
tool_type: DiscoverableToolType::Plugin,
|
||||
has_skills: plugin.has_skills,
|
||||
mcp_server_names: plugin.mcp_server_names.clone(),
|
||||
app_connector_ids: plugin.app_connector_ids.clone(),
|
||||
},
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "spec_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -89,7 +89,10 @@ pub use tool_discovery::DiscoverableTool;
|
||||
pub use tool_discovery::DiscoverableToolAction;
|
||||
pub use tool_discovery::DiscoverableToolType;
|
||||
pub use tool_discovery::ToolSearchAppInfo;
|
||||
pub use tool_discovery::ToolSearchAppSource;
|
||||
pub use tool_discovery::ToolSuggestEntry;
|
||||
pub use tool_discovery::collect_tool_search_app_infos;
|
||||
pub use tool_discovery::collect_tool_suggest_entries;
|
||||
pub use tool_discovery::create_tool_search_tool;
|
||||
pub use tool_discovery::create_tool_suggest_tool;
|
||||
pub use tool_discovery::filter_tool_suggest_discoverable_tools_for_client;
|
||||
|
||||
@@ -14,6 +14,13 @@ pub struct ToolSearchAppInfo {
|
||||
pub description: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub struct ToolSearchAppSource<'a> {
|
||||
pub server_name: &'a str,
|
||||
pub connector_name: Option<&'a str>,
|
||||
pub connector_description: Option<&'a str>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum DiscoverableToolType {
|
||||
@@ -178,6 +185,29 @@ pub fn create_tool_search_tool(app_tools: &[ToolSearchAppInfo], default_limit: u
|
||||
}
|
||||
}
|
||||
|
||||
pub fn collect_tool_search_app_infos<'a>(
|
||||
app_tools: impl IntoIterator<Item = ToolSearchAppSource<'a>>,
|
||||
codex_apps_server_name: &str,
|
||||
) -> Vec<ToolSearchAppInfo> {
|
||||
app_tools
|
||||
.into_iter()
|
||||
.filter(|tool| tool.server_name == codex_apps_server_name)
|
||||
.filter_map(|tool| {
|
||||
let name = tool
|
||||
.connector_name
|
||||
.map(str::trim)
|
||||
.filter(|connector_name| !connector_name.is_empty())?
|
||||
.to_string();
|
||||
let description = tool
|
||||
.connector_description
|
||||
.map(str::trim)
|
||||
.filter(|connector_description| !connector_description.is_empty())
|
||||
.map(str::to_string);
|
||||
Some(ToolSearchAppInfo { name, description })
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub fn create_tool_suggest_tool(discoverable_tools: &[ToolSuggestEntry]) -> ToolSpec {
|
||||
let discoverable_tool_ids = discoverable_tools
|
||||
.iter()
|
||||
@@ -245,6 +275,34 @@ pub fn create_tool_suggest_tool(discoverable_tools: &[ToolSuggestEntry]) -> Tool
|
||||
})
|
||||
}
|
||||
|
||||
pub fn collect_tool_suggest_entries(
|
||||
discoverable_tools: &[DiscoverableTool],
|
||||
) -> Vec<ToolSuggestEntry> {
|
||||
discoverable_tools
|
||||
.iter()
|
||||
.map(|tool| match tool {
|
||||
DiscoverableTool::Connector(connector) => ToolSuggestEntry {
|
||||
id: connector.id.clone(),
|
||||
name: connector.name.clone(),
|
||||
description: connector.description.clone(),
|
||||
tool_type: DiscoverableToolType::Connector,
|
||||
has_skills: false,
|
||||
mcp_server_names: Vec::new(),
|
||||
app_connector_ids: Vec::new(),
|
||||
},
|
||||
DiscoverableTool::Plugin(plugin) => ToolSuggestEntry {
|
||||
id: plugin.id.clone(),
|
||||
name: plugin.name.clone(),
|
||||
description: plugin.description.clone(),
|
||||
tool_type: DiscoverableToolType::Plugin,
|
||||
has_skills: plugin.has_skills,
|
||||
mcp_server_names: plugin.mcp_server_names.clone(),
|
||||
app_connector_ids: plugin.app_connector_ids.clone(),
|
||||
},
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn format_discoverable_tools(discoverable_tools: &[ToolSuggestEntry]) -> String {
|
||||
let mut discoverable_tools = discoverable_tools.to_vec();
|
||||
discoverable_tools.sort_by(|left, right| {
|
||||
|
||||
Reference in New Issue
Block a user