mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
codex-tools: extract discoverable tool models (#16254)
## Why `#16193` moved the pure `tool_search` and `tool_suggest` spec builders into `codex-tools`, but `codex-core` still owned the shared discoverable-tool model that those builders and the `tool_suggest` runtime both depend on. This change continues the migration by moving that reusable model boundary out of `codex-core` as well, so the discovery/suggestion stack uses one shared set of types and `core/src/tools` no longer needs its own `discoverable.rs` module. ## What changed - Moved `DiscoverableTool`, `DiscoverablePluginInfo`, and `filter_tool_suggest_discoverable_tools_for_client()` into `codex-rs/tools/src/tool_discovery.rs` alongside the extracted discovery/suggestion spec builders. - Added `codex-app-server-protocol` as a `codex-tools` dependency so the shared discoverable-tool model can own the connector-side `AppInfo` variant directly. - Updated `core/src/tools/handlers/tool_suggest.rs`, `core/src/tools/spec.rs`, `core/src/tools/router.rs`, `core/src/connectors.rs`, and `core/src/codex.rs` to consume the shared `codex-tools` model instead of the old core-local declarations. - Changed `core/src/plugins/discoverable.rs` to return `DiscoverablePluginInfo` directly, moved the pure client-filter coverage into `tool_discovery_tests.rs`, and deleted the old `core/src/tools/discoverable.rs` module. - Updated `codex-rs/tools/README.md` so the crate boundary documents that `codex-tools` now owns the discoverable-tool models in addition to the discovery/suggestion spec builders. ## Test plan - `cargo test -p codex-tools` - `CARGO_TARGET_DIR=/tmp/codex-core-discoverable-model cargo test -p codex-core --lib tools::handlers::tool_suggest::` - `CARGO_TARGET_DIR=/tmp/codex-core-discoverable-model cargo test -p codex-core --lib tools::spec::` - `CARGO_TARGET_DIR=/tmp/codex-core-discoverable-model cargo test -p codex-core --lib plugins::discoverable::` - `just bazel-lock-check` - `just argument-comment-lint` ## References - #16193 - #16154 - #15923 - #15928 - #15944 - #15953 - #16031 - #16047 - #16129 - #16132 - #16138 - #16141
This commit is contained in:
committed by
GitHub
Unverified
parent
716f7b0428
commit
258ba436f1
@@ -9,6 +9,7 @@ use super::PluginsManager;
|
||||
use crate::config::Config;
|
||||
use crate::config::types::ToolSuggestDiscoverableType;
|
||||
use codex_features::Feature;
|
||||
use codex_tools::DiscoverablePluginInfo;
|
||||
|
||||
const TOOL_SUGGEST_DISCOVERABLE_PLUGIN_ALLOWLIST: &[&str] = &[
|
||||
"github@openai-curated",
|
||||
@@ -23,7 +24,7 @@ const TOOL_SUGGEST_DISCOVERABLE_PLUGIN_ALLOWLIST: &[&str] = &[
|
||||
|
||||
pub(crate) fn list_tool_suggest_discoverable_plugins(
|
||||
config: &Config,
|
||||
) -> anyhow::Result<Vec<PluginCapabilitySummary>> {
|
||||
) -> anyhow::Result<Vec<DiscoverablePluginInfo>> {
|
||||
if !config.features.enabled(Feature::Plugins) {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
@@ -47,7 +48,7 @@ pub(crate) fn list_tool_suggest_discoverable_plugins(
|
||||
return Ok(Vec::new());
|
||||
};
|
||||
|
||||
let mut discoverable_plugins = Vec::<PluginCapabilitySummary>::new();
|
||||
let mut discoverable_plugins = Vec::<DiscoverablePluginInfo>::new();
|
||||
for plugin in curated_marketplace.plugins {
|
||||
if plugin.installed
|
||||
|| (!TOOL_SUGGEST_DISCOVERABLE_PLUGIN_ALLOWLIST.contains(&plugin.id.as_str())
|
||||
@@ -66,14 +67,28 @@ pub(crate) fn list_tool_suggest_discoverable_plugins(
|
||||
marketplace_path: curated_marketplace.path.clone(),
|
||||
},
|
||||
) {
|
||||
Ok(plugin) => discoverable_plugins.push(plugin.plugin.into()),
|
||||
Ok(plugin) => {
|
||||
let plugin: PluginCapabilitySummary = plugin.plugin.into();
|
||||
discoverable_plugins.push(DiscoverablePluginInfo {
|
||||
id: plugin.config_name,
|
||||
name: plugin.display_name,
|
||||
description: plugin.description,
|
||||
has_skills: plugin.has_skills,
|
||||
mcp_server_names: plugin.mcp_server_names,
|
||||
app_connector_ids: plugin
|
||||
.app_connector_ids
|
||||
.into_iter()
|
||||
.map(|connector_id| connector_id.0)
|
||||
.collect(),
|
||||
});
|
||||
}
|
||||
Err(err) => warn!("failed to load discoverable plugin suggestion {plugin_id}: {err:#}"),
|
||||
}
|
||||
}
|
||||
discoverable_plugins.sort_by(|left, right| {
|
||||
left.display_name
|
||||
.cmp(&right.display_name)
|
||||
.then_with(|| left.config_name.cmp(&right.config_name))
|
||||
left.name
|
||||
.cmp(&right.name)
|
||||
.then_with(|| left.id.cmp(&right.id))
|
||||
});
|
||||
Ok(discoverable_plugins)
|
||||
}
|
||||
|
||||
@@ -5,7 +5,7 @@ use crate::plugins::test_support::write_curated_plugin_sha;
|
||||
use crate::plugins::test_support::write_file;
|
||||
use crate::plugins::test_support::write_openai_curated_marketplace;
|
||||
use crate::plugins::test_support::write_plugins_feature_config;
|
||||
use crate::tools::discoverable::DiscoverablePluginInfo;
|
||||
use codex_tools::DiscoverablePluginInfo;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
@@ -18,11 +18,7 @@ async fn list_tool_suggest_discoverable_plugins_returns_uninstalled_curated_plug
|
||||
write_plugins_feature_config(codex_home.path());
|
||||
|
||||
let config = load_plugins_config(codex_home.path()).await;
|
||||
let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config)
|
||||
.unwrap()
|
||||
.into_iter()
|
||||
.map(DiscoverablePluginInfo::from)
|
||||
.collect::<Vec<_>>();
|
||||
let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
discoverable_plugins,
|
||||
@@ -52,11 +48,7 @@ plugins = false
|
||||
);
|
||||
|
||||
let config = load_plugins_config(codex_home.path()).await;
|
||||
let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config)
|
||||
.unwrap()
|
||||
.into_iter()
|
||||
.map(DiscoverablePluginInfo::from)
|
||||
.collect::<Vec<_>>();
|
||||
let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config).unwrap();
|
||||
|
||||
assert_eq!(discoverable_plugins, Vec::<DiscoverablePluginInfo>::new());
|
||||
}
|
||||
@@ -76,11 +68,7 @@ async fn list_tool_suggest_discoverable_plugins_normalizes_description() {
|
||||
);
|
||||
|
||||
let config = load_plugins_config(codex_home.path()).await;
|
||||
let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config)
|
||||
.unwrap()
|
||||
.into_iter()
|
||||
.map(DiscoverablePluginInfo::from)
|
||||
.collect::<Vec<_>>();
|
||||
let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
discoverable_plugins,
|
||||
@@ -115,11 +103,7 @@ async fn list_tool_suggest_discoverable_plugins_omits_installed_curated_plugins(
|
||||
.expect("plugin should install");
|
||||
|
||||
let refreshed_config = load_plugins_config(codex_home.path()).await;
|
||||
let discoverable_plugins = list_tool_suggest_discoverable_plugins(&refreshed_config)
|
||||
.unwrap()
|
||||
.into_iter()
|
||||
.map(DiscoverablePluginInfo::from)
|
||||
.collect::<Vec<_>>();
|
||||
let discoverable_plugins = list_tool_suggest_discoverable_plugins(&refreshed_config).unwrap();
|
||||
|
||||
assert_eq!(discoverable_plugins, Vec::<DiscoverablePluginInfo>::new());
|
||||
}
|
||||
@@ -140,11 +124,7 @@ discoverables = [{ type = "plugin", id = "sample@openai-curated" }]
|
||||
);
|
||||
|
||||
let config = load_plugins_config(codex_home.path()).await;
|
||||
let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config)
|
||||
.unwrap()
|
||||
.into_iter()
|
||||
.map(DiscoverablePluginInfo::from)
|
||||
.collect::<Vec<_>>();
|
||||
let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
discoverable_plugins,
|
||||
|
||||
Reference in New Issue
Block a user