Remove obsolete plugin skill discovery helpers (#37832)

## What changed

- Remove the path-only `effective_skill_roots` API and its type-erasing trait in favor of `effective_plugin_skill_roots`, which preserves plugin metadata.
- Remove skill-path ancestor lookup helpers and keep namespace resolution scoped to an explicit plugin root.
- Update plugin manager and namespace tests to exercise the retained APIs directly.

GitOrigin-RevId: 507bf43bc6147bb5ab2deba4fa176bea2535159c
This commit is contained in:
felixxia-oai
2026-08-10 14:25:13 +00:00
committed by copyberry
parent d109393270
commit bfb7790eb3
5 changed files with 24 additions and 66 deletions
+7 -3
View File
@@ -991,7 +991,11 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
}]
);
assert_eq!(
outcome.effective_skill_roots(),
outcome
.effective_plugin_skill_roots()
.into_iter()
.map(|root| root.path)
.collect::<Vec<_>>(),
vec![plugin_root.join("skills").abs()]
);
assert_eq!(outcome.effective_mcp_servers().len(), 1);
@@ -2596,7 +2600,7 @@ async fn load_plugins_preserves_disabled_plugins_without_effective_contributions
error: None,
}]
);
assert!(outcome.effective_skill_roots().is_empty());
assert!(outcome.effective_plugin_skill_roots().is_empty());
assert!(outcome.effective_mcp_servers().is_empty());
}
@@ -3086,7 +3090,7 @@ async fn load_plugins_rejects_invalid_plugin_keys() {
outcome.plugins()[0].error.as_deref(),
Some("invalid plugin key `sample`; expected <plugin>@<marketplace>")
);
assert!(outcome.effective_skill_roots().is_empty());
assert!(outcome.effective_plugin_skill_roots().is_empty());
assert!(outcome.effective_mcp_servers().is_empty());
}
-2
View File
@@ -3,7 +3,6 @@
use std::collections::HashSet;
pub use codex_utils_plugins::mention_syntax;
pub use codex_utils_plugins::plugin_namespace_for_skill_path;
mod load_outcome;
pub mod manifest;
@@ -12,7 +11,6 @@ mod provider;
use codex_config::HookEventsToml;
use codex_utils_absolute_path::AbsolutePathBuf;
pub use load_outcome::EffectiveSkillRoots;
pub use load_outcome::LoadedPlugin;
pub use load_outcome::PluginLoadOutcome;
pub use load_outcome::prompt_safe_plugin_description;
-30
View File
@@ -120,18 +120,6 @@ impl<M: Clone> PluginLoadOutcome<M> {
}
}
pub fn effective_skill_roots(&self) -> Vec<AbsolutePathBuf> {
let mut skill_roots: Vec<AbsolutePathBuf> = self
.plugins
.iter()
.filter(|plugin| plugin.is_active())
.flat_map(|plugin| plugin.skill_roots.iter().cloned())
.collect();
skill_roots.sort_unstable();
skill_roots.dedup();
skill_roots
}
pub fn effective_plugin_skill_roots(&self) -> Vec<PluginSkillRoot> {
let mut skill_roots = Vec::new();
let mut seen_paths = HashSet::new();
@@ -205,24 +193,6 @@ impl<M: Clone> PluginLoadOutcome<M> {
}
}
/// Implemented by [`PluginLoadOutcome`] so callers (e.g. skills) can depend on `codex-plugin`
/// without naming the MCP config type parameter.
pub trait EffectiveSkillRoots {
fn effective_skill_roots(&self) -> Vec<AbsolutePathBuf>;
fn effective_plugin_skill_roots(&self) -> Vec<PluginSkillRoot>;
}
impl<M: Clone> EffectiveSkillRoots for PluginLoadOutcome<M> {
fn effective_skill_roots(&self) -> Vec<AbsolutePathBuf> {
PluginLoadOutcome::effective_skill_roots(self)
}
fn effective_plugin_skill_roots(&self) -> Vec<PluginSkillRoot> {
PluginLoadOutcome::effective_plugin_skill_roots(self)
}
}
#[cfg(test)]
mod tests {
use super::*;
-2
View File
@@ -16,8 +16,6 @@ pub use plugin_namespace::SUPPORTED_AGENT_PLUGIN_SCHEMA_URIS;
pub use plugin_namespace::agent_plugin_schema_status;
pub use plugin_namespace::find_plugin_manifest_path;
pub use plugin_namespace::plugin_namespace_for_root_uri;
pub use plugin_namespace::plugin_namespace_for_skill_path;
pub use plugin_namespace::plugin_namespace_for_skill_uri;
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)]
pub enum SkillDiscoveryMode {
+17 -29
View File
@@ -2,7 +2,6 @@
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server_protocol::DISCOVERABLE_PLUGIN_MANIFEST_PATHS;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use std::path::Path;
use std::path::PathBuf;
@@ -100,38 +99,15 @@ pub async fn plugin_namespace_for_root_uri(
)
}
/// Returns the plugin manifest `name` for the nearest ancestor of `path` that contains a valid
/// plugin manifest (same `name` rules as full manifest loading in codex-core).
pub async fn plugin_namespace_for_skill_path(
fs: &dyn ExecutorFileSystem,
path: &AbsolutePathBuf,
) -> Option<String> {
plugin_namespace_for_skill_uri(fs, &PathUri::from_abs_path(path)).await
}
/// Returns the plugin manifest `name` for the nearest URI ancestor of `path`.
pub async fn plugin_namespace_for_skill_uri(
fs: &dyn ExecutorFileSystem,
path: &PathUri,
) -> Option<String> {
let mut ancestor = Some(path.clone());
while let Some(path) = ancestor {
if let Some(name) = plugin_namespace_for_root_uri(fs, &path).await {
return Some(name);
}
ancestor = path.parent();
}
None
}
#[cfg(test)]
mod tests {
use super::AGENT_PLUGIN_MANIFEST_RELATIVE_PATH;
use super::AGENT_PLUGIN_SCHEMA_URI;
use super::find_plugin_manifest_path;
use super::plugin_namespace_for_skill_path;
use super::plugin_namespace_for_root_uri;
use codex_exec_server::LOCAL_FS;
use codex_utils_absolute_path::test_support::PathBufExt;
use codex_utils_path_uri::PathUri;
use std::fs;
use tempfile::tempdir;
@@ -154,7 +130,11 @@ mod tests {
fs::write(&skill_path, "---\ndescription: search\n---\n").expect("write skill");
assert_eq!(
plugin_namespace_for_skill_path(LOCAL_FS.as_ref(), &skill_path.abs()).await,
plugin_namespace_for_root_uri(
LOCAL_FS.as_ref(),
&PathUri::from_abs_path(&plugin_root.abs()),
)
.await,
Some("sample".to_string())
);
}
@@ -173,7 +153,11 @@ mod tests {
fs::write(&skill_path, "---\ndescription: search\n---\n").expect("write skill");
assert_eq!(
plugin_namespace_for_skill_path(LOCAL_FS.as_ref(), &skill_path.abs()).await,
plugin_namespace_for_root_uri(
LOCAL_FS.as_ref(),
&PathUri::from_abs_path(&plugin_root.abs()),
)
.await,
Some("sample".to_string())
);
assert_eq!(find_plugin_manifest_path(&plugin_root), Some(manifest_path));
@@ -193,7 +177,11 @@ mod tests {
fs::write(&skill_path, "---\ndescription: search\n---\n").expect("write skill");
assert_eq!(
plugin_namespace_for_skill_path(LOCAL_FS.as_ref(), &skill_path.abs()).await,
plugin_namespace_for_root_uri(
LOCAL_FS.as_ref(),
&PathUri::from_abs_path(&plugin_root.abs()),
)
.await,
Some("sample".to_string())
);
assert_eq!(find_plugin_manifest_path(&plugin_root), Some(manifest_path));