mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Remove the legacy core skill loader (#37457)
## What changed - Use `HostSkillRoot` throughout host root resolution and loading, including plugin roots. - Remove the duplicate loader, root snapshot, and product-filtering implementation from `core-skills` now that loading is owned by the skills extension. - Move and expand loader coverage for discovery, namespaces, filesystem routing, root merging, symlinks, and frontmatter parsing. ## Testing - Added focused unit and integration tests under `ext/skills` and `skills` for the consolidated loader behavior. GitOrigin-RevId: 214d06d59bf3033ee0f220ea5959ee4feff66782
This commit is contained in:
felixxia-oai
Generated
-5
@@ -2919,18 +2919,13 @@ dependencies = [
|
||||
"codex-login",
|
||||
"codex-model-provider",
|
||||
"codex-otel",
|
||||
"codex-protocol",
|
||||
"codex-skills",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-path-uri",
|
||||
"codex-utils-plugins",
|
||||
"codex-utils-string",
|
||||
"dunce",
|
||||
"futures",
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"serde_yaml",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
"toml 0.9.11+spec-1.1.0",
|
||||
|
||||
@@ -126,7 +126,7 @@ impl SkillsWatcher {
|
||||
.into_iter()
|
||||
// Plugin roots have explicit lifecycle invalidation; generated system skills are
|
||||
// installed before this watcher starts.
|
||||
.filter(|root| root.plugin_identity.is_none() && root.scope != SkillScope::System)
|
||||
.filter(|root| root.plugin_identity().is_none() && root.scope != SkillScope::System)
|
||||
.map(|root| WatchPath {
|
||||
path: root.path.into_path_buf(),
|
||||
recursive: true,
|
||||
|
||||
@@ -20,7 +20,7 @@ use crate::app_mcp_routing::apply_app_mcp_routing_policy;
|
||||
use crate::loader::PluginSkillInventory;
|
||||
use crate::loader::load_plugin_apps;
|
||||
use crate::loader::load_plugin_mcp_servers;
|
||||
use crate::loader::load_plugin_skill_inventory_with_loader;
|
||||
use crate::loader::load_plugin_skill_inventory;
|
||||
use crate::manager::ConfiguredMarketplacePlugin;
|
||||
use crate::manager::remote_plugin_install_required_description;
|
||||
use crate::manifest::PluginManifestFormat;
|
||||
@@ -230,7 +230,7 @@ async fn load_plugin_metadata(
|
||||
remote_plugin_id: None,
|
||||
};
|
||||
let manifest = loaded_manifest.manifest;
|
||||
let skill_inventory = load_plugin_skill_inventory_with_loader(
|
||||
let skill_inventory = load_plugin_skill_inventory(
|
||||
plugin_root,
|
||||
&plugin_identity,
|
||||
&manifest,
|
||||
|
||||
@@ -21,17 +21,12 @@ codex-exec-server = { workspace = true }
|
||||
codex-login = { workspace = true }
|
||||
codex-model-provider = { workspace = true }
|
||||
codex-otel = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-skills = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
codex-utils-path-uri = { workspace = true }
|
||||
codex-utils-plugins = { workspace = true }
|
||||
codex-utils-string = { workspace = true }
|
||||
dunce = { workspace = true }
|
||||
futures = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
serde_yaml = { workspace = true }
|
||||
tokio = { workspace = true, features = ["fs", "macros", "rt"] }
|
||||
tracing = { workspace = true }
|
||||
zip = { workspace = true }
|
||||
|
||||
@@ -1,10 +1,8 @@
|
||||
pub mod config_rules;
|
||||
pub mod injection;
|
||||
pub(crate) mod invocation_utils;
|
||||
pub mod loader;
|
||||
pub mod model;
|
||||
pub mod remote;
|
||||
mod root_loader;
|
||||
mod skill_instructions;
|
||||
|
||||
/// Hard byte limit for one model-visible skill instruction body.
|
||||
@@ -21,6 +19,4 @@ pub use model::SkillError;
|
||||
pub use model::SkillLoadOutcome;
|
||||
pub use model::SkillMetadata;
|
||||
pub use model::SkillPolicy;
|
||||
pub use model::filter_skill_load_outcome_for_product;
|
||||
pub use root_loader::PluginSkillSnapshots;
|
||||
pub use skill_instructions::SkillInstructions;
|
||||
|
||||
@@ -1,645 +0,0 @@
|
||||
mod discovery;
|
||||
mod environment;
|
||||
mod namespace;
|
||||
|
||||
pub use crate::root_loader::load_skill_root_snapshot;
|
||||
pub use environment::EnvironmentSkillLoadOutcome;
|
||||
pub use environment::EnvironmentSkillMetadata;
|
||||
pub use environment::load_environment_skills_from_root;
|
||||
|
||||
use crate::model::SkillDependencies;
|
||||
use crate::model::SkillError;
|
||||
use crate::model::SkillInterface;
|
||||
use crate::model::SkillLoadOutcome;
|
||||
use crate::model::SkillMetadata;
|
||||
use crate::model::SkillPolicy;
|
||||
use crate::model::SkillToolDependency;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_skills::ParsedSkillFrontmatter;
|
||||
use codex_skills::SkillInterfaceAssetPolicy;
|
||||
use codex_skills::SkillInterfaceFile;
|
||||
use codex_skills::SkillParseError;
|
||||
use codex_skills::parse_skill_frontmatter_metadata;
|
||||
use codex_skills::resolve_skill_interface;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::AbsolutePathBufGuard;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginIdentity;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
use discovery::DirectorySymlinkPolicy;
|
||||
use discovery::DiscoveredSkill;
|
||||
use discovery::HiddenDirectoryPolicy;
|
||||
use discovery::MAX_CONCURRENT_SKILL_LOADS;
|
||||
use discovery::SkillDiscovery;
|
||||
use discovery::SkillDiscoveryOptions;
|
||||
use discovery::SkillMetadataDiscovery;
|
||||
use discovery::discover_skills;
|
||||
use futures::FutureExt;
|
||||
use futures::StreamExt;
|
||||
use namespace::SkillNamespaceResolver;
|
||||
use serde::Deserialize;
|
||||
use std::collections::HashMap;
|
||||
use std::error::Error;
|
||||
use std::fmt;
|
||||
use std::io;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::Semaphore;
|
||||
use tracing::error;
|
||||
|
||||
// TODO(anp): Tune this eight-scan limit after revisiting byte-based backpressure.
|
||||
pub const MAX_CONCURRENT_ROOT_SCANS: usize = 8;
|
||||
|
||||
#[derive(Debug, Default, Deserialize)]
|
||||
struct SkillMetadataFile {
|
||||
#[serde(default)]
|
||||
interface: Option<SkillInterfaceFile>,
|
||||
#[serde(default)]
|
||||
dependencies: Option<Dependencies>,
|
||||
#[serde(default)]
|
||||
policy: Option<Policy>,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct LoadedSkillMetadata {
|
||||
interface: Option<SkillInterface>,
|
||||
dependencies: Option<SkillDependencies>,
|
||||
policy: Option<SkillPolicy>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize)]
|
||||
struct Dependencies {
|
||||
#[serde(default)]
|
||||
tools: Vec<DependencyTool>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct Policy {
|
||||
#[serde(default)]
|
||||
allow_implicit_invocation: Option<bool>,
|
||||
#[serde(default)]
|
||||
products: Vec<Product>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize)]
|
||||
struct DependencyTool {
|
||||
#[serde(rename = "type")]
|
||||
kind: Option<String>,
|
||||
value: Option<String>,
|
||||
description: Option<String>,
|
||||
transport: Option<String>,
|
||||
command: Option<String>,
|
||||
url: Option<String>,
|
||||
}
|
||||
|
||||
const SKILLS_FILENAME: &str = "SKILL.md";
|
||||
const SKILLS_METADATA_DIR: &str = "agents";
|
||||
const SKILLS_METADATA_FILENAME: &str = "openai.yaml";
|
||||
const MAX_NAME_LEN: usize = 64;
|
||||
const MAX_QUALIFIED_NAME_LEN: usize = MAX_NAME_LEN * 2 + 1;
|
||||
const MAX_DESCRIPTION_LEN: usize = 1024;
|
||||
const MAX_DEPENDENCY_TYPE_LEN: usize = MAX_NAME_LEN;
|
||||
const MAX_DEPENDENCY_TRANSPORT_LEN: usize = MAX_NAME_LEN;
|
||||
const MAX_DEPENDENCY_VALUE_LEN: usize = MAX_DESCRIPTION_LEN;
|
||||
const MAX_DEPENDENCY_DESCRIPTION_LEN: usize = MAX_DESCRIPTION_LEN;
|
||||
const MAX_DEPENDENCY_COMMAND_LEN: usize = MAX_DESCRIPTION_LEN;
|
||||
const MAX_DEPENDENCY_URL_LEN: usize = MAX_DESCRIPTION_LEN;
|
||||
// Traversal depth from the skills root.
|
||||
const MAX_SCAN_DEPTH: usize = 6;
|
||||
const MAX_SKILLS_DIRS_PER_ROOT: usize = 2000;
|
||||
|
||||
struct ResolvedDiscoveredSkill {
|
||||
skill: DiscoveredSkill,
|
||||
path: AbsolutePathBuf,
|
||||
path_uri: PathUri,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
enum SkillLoadError {
|
||||
Read(std::io::Error),
|
||||
Parse(SkillParseError),
|
||||
}
|
||||
|
||||
impl fmt::Display for SkillLoadError {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
Self::Read(error) => write!(f, "failed to read file: {error}"),
|
||||
Self::Parse(error) => write!(f, "{error}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Error for SkillLoadError {}
|
||||
|
||||
impl From<SkillParseError> for SkillLoadError {
|
||||
fn from(error: SkillParseError) -> Self {
|
||||
Self::Parse(error)
|
||||
}
|
||||
}
|
||||
|
||||
pub struct SkillRoot {
|
||||
pub path: AbsolutePathBuf,
|
||||
pub scope: SkillScope,
|
||||
pub file_system: Arc<dyn ExecutorFileSystem>,
|
||||
pub plugin_identity: Option<PluginIdentity>,
|
||||
pub plugin_namespace: Option<String>,
|
||||
pub plugin_root: Option<AbsolutePathBuf>,
|
||||
pub discovery_mode: SkillDiscoveryMode,
|
||||
}
|
||||
|
||||
pub async fn load_skills_from_roots<I>(
|
||||
roots: I,
|
||||
plugin_skill_snapshots: Option<&crate::PluginSkillSnapshots>,
|
||||
root_scan_slots: Arc<Semaphore>,
|
||||
) -> SkillLoadOutcome
|
||||
where
|
||||
I: IntoIterator<Item = SkillRoot> + Send,
|
||||
I::IntoIter: Send,
|
||||
{
|
||||
crate::root_loader::load_and_merge_skill_roots(roots, plugin_skill_snapshots, &root_scan_slots)
|
||||
.boxed()
|
||||
.await
|
||||
}
|
||||
|
||||
/// One loaded skill root and the filesystem that supplied it.
|
||||
///
|
||||
/// This is exposed temporarily while host discovery moves out of `core-skills`; callers should
|
||||
/// combine snapshots through [`SkillLoadOutcome::from_root_snapshots`].
|
||||
#[derive(Clone)]
|
||||
pub struct SkillRootSnapshot {
|
||||
pub(crate) root: AbsolutePathBuf,
|
||||
pub(crate) is_agent_plugin: bool,
|
||||
pub(crate) skills: Vec<SkillMetadata>,
|
||||
pub(crate) skill_discovery_path_by_path: Arc<HashMap<AbsolutePathBuf, AbsolutePathBuf>>,
|
||||
pub(crate) errors: Vec<SkillError>,
|
||||
pub(crate) file_system: Arc<dyn ExecutorFileSystem>,
|
||||
}
|
||||
|
||||
impl SkillRootSnapshot {
|
||||
pub fn new(
|
||||
root: AbsolutePathBuf,
|
||||
skills: Vec<SkillMetadata>,
|
||||
skill_discovery_path_by_path: Arc<HashMap<AbsolutePathBuf, AbsolutePathBuf>>,
|
||||
errors: Vec<SkillError>,
|
||||
file_system: Arc<dyn ExecutorFileSystem>,
|
||||
) -> Self {
|
||||
Self {
|
||||
root,
|
||||
is_agent_plugin: false,
|
||||
skills,
|
||||
skill_discovery_path_by_path,
|
||||
errors,
|
||||
file_system,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn load_skill_root(root: SkillRoot) -> SkillRootSnapshot {
|
||||
let canonical_root =
|
||||
canonicalize_for_skill_identity(root.file_system.as_ref(), &root.path).await;
|
||||
let mut outcome = SkillLoadOutcome::default();
|
||||
load_skills_under_root(&root, &canonical_root, &mut outcome).await;
|
||||
SkillRootSnapshot {
|
||||
root: canonical_root,
|
||||
is_agent_plugin: root.discovery_mode == SkillDiscoveryMode::DirectChildren,
|
||||
skills: outcome.skills,
|
||||
skill_discovery_path_by_path: outcome.skill_discovery_path_by_path,
|
||||
errors: outcome.errors,
|
||||
file_system: root.file_system,
|
||||
}
|
||||
}
|
||||
|
||||
async fn canonicalize_for_skill_identity(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
path: &AbsolutePathBuf,
|
||||
) -> AbsolutePathBuf {
|
||||
let path_uri = PathUri::from_abs_path(path);
|
||||
fs.canonicalize(&path_uri, /*sandbox*/ None)
|
||||
.await
|
||||
.and_then(|path| path.to_abs_path())
|
||||
.unwrap_or_else(|_| path.clone())
|
||||
}
|
||||
|
||||
async fn load_skills_under_root(
|
||||
skill_root: &SkillRoot,
|
||||
root: &AbsolutePathBuf,
|
||||
outcome: &mut SkillLoadOutcome,
|
||||
) {
|
||||
let fs = skill_root.file_system.as_ref();
|
||||
let plugin_identity = skill_root.plugin_identity.as_ref();
|
||||
let plugin_root = match skill_root.plugin_root.as_ref() {
|
||||
Some(plugin_root) => Some(canonicalize_for_skill_identity(fs, plugin_root).await),
|
||||
None => None,
|
||||
};
|
||||
let directory_symlinks = match skill_root.scope {
|
||||
SkillScope::User | SkillScope::Repo | SkillScope::Admin => DirectorySymlinkPolicy::Follow,
|
||||
SkillScope::System => DirectorySymlinkPolicy::Ignore,
|
||||
};
|
||||
let SkillDiscovery {
|
||||
skills,
|
||||
plugin_roots,
|
||||
mut namespace_roots,
|
||||
warnings,
|
||||
} = discover_skills(
|
||||
fs,
|
||||
&PathUri::from_abs_path(root),
|
||||
// Preserve host discovery behavior: directory aliases are scope-dependent, while hidden
|
||||
// directories are skipped unless reached through a visible alias.
|
||||
SkillDiscoveryOptions {
|
||||
directory_symlinks,
|
||||
hidden_directories: HiddenDirectoryPolicy::Skip,
|
||||
mode: skill_root.discovery_mode,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
for warning in warnings {
|
||||
error!("{warning}");
|
||||
}
|
||||
// With no skills, there is nothing to canonicalize, parse, or namespace-qualify.
|
||||
if skills.is_empty() {
|
||||
return;
|
||||
}
|
||||
let root_uri = PathUri::from_abs_path(root);
|
||||
let resolved_plugin_root = plugin_root.as_ref();
|
||||
let resolved_skills = futures::stream::iter(skills)
|
||||
.map(|skill| async move {
|
||||
let path_uri = match fs.canonicalize(&skill.path, /*sandbox*/ None).await {
|
||||
Ok(path) => path,
|
||||
Err(err) if skill_root.discovery_mode == SkillDiscoveryMode::DirectChildren => {
|
||||
error!(
|
||||
"failed to resolve Agent Plugin skill path {}: {err}",
|
||||
skill.path
|
||||
);
|
||||
return None;
|
||||
}
|
||||
Err(_) => skill.path.clone(),
|
||||
};
|
||||
let path = match path_uri.to_abs_path() {
|
||||
Ok(path) => path,
|
||||
Err(err) => {
|
||||
error!("failed to convert discovered skill path {path_uri}: {err}");
|
||||
return None;
|
||||
}
|
||||
};
|
||||
if skill_root.discovery_mode == SkillDiscoveryMode::DirectChildren {
|
||||
let Some(plugin_root) = resolved_plugin_root else {
|
||||
error!("Agent Plugin skill root is missing its plugin root");
|
||||
return None;
|
||||
};
|
||||
if !path.as_path().starts_with(plugin_root.as_path()) {
|
||||
error!(
|
||||
"Agent Plugin skill path {} resolves outside plugin root {}",
|
||||
path.display(),
|
||||
plugin_root.display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
match fs.get_metadata(&path_uri, /*sandbox*/ None).await {
|
||||
Ok(metadata) if metadata.is_file => {}
|
||||
Ok(_) => {
|
||||
error!(
|
||||
"Agent Plugin skill path {} is not a regular file",
|
||||
path.display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
Err(err) => {
|
||||
error!(
|
||||
"failed to inspect Agent Plugin skill path {}: {err}",
|
||||
path.display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
}
|
||||
}
|
||||
Some(ResolvedDiscoveredSkill {
|
||||
skill,
|
||||
path,
|
||||
path_uri,
|
||||
})
|
||||
})
|
||||
.buffered(MAX_CONCURRENT_SKILL_LOADS)
|
||||
.filter_map(futures::future::ready)
|
||||
.collect::<Vec<_>>()
|
||||
.await;
|
||||
namespace_roots.extend(resolved_skills.iter().filter_map(|skill| {
|
||||
(skill.path_uri != skill.skill.path)
|
||||
.then(|| skill.path_uri.parent())
|
||||
.flatten()
|
||||
}));
|
||||
let skill_paths = resolved_skills
|
||||
.iter()
|
||||
.map(|skill| skill.path_uri.clone())
|
||||
.collect::<Vec<_>>();
|
||||
let namespace_resolver = async {
|
||||
match skill_root.plugin_namespace.as_deref() {
|
||||
Some(namespace) => SkillNamespaceResolver::with_provided_namespace(namespace),
|
||||
None => {
|
||||
SkillNamespaceResolver::discover(
|
||||
fs,
|
||||
&root_uri,
|
||||
&skill_paths,
|
||||
plugin_roots,
|
||||
namespace_roots,
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
};
|
||||
let skill_results = futures::stream::iter(resolved_skills)
|
||||
.map(|skill| {
|
||||
let plugin_root = plugin_root.as_ref();
|
||||
async move {
|
||||
let discovery_path = skill
|
||||
.skill
|
||||
.path
|
||||
.to_abs_path()
|
||||
.unwrap_or_else(|_| skill.path.clone());
|
||||
let result = parse_skill_file(
|
||||
fs,
|
||||
&skill.skill,
|
||||
&skill.path,
|
||||
&skill.path_uri,
|
||||
skill_root.scope,
|
||||
plugin_identity,
|
||||
plugin_root,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| err.to_string());
|
||||
(skill.path, skill.path_uri, discovery_path, result)
|
||||
}
|
||||
})
|
||||
.buffered(MAX_CONCURRENT_SKILL_LOADS)
|
||||
.collect::<Vec<_>>()
|
||||
.boxed();
|
||||
let (namespace_resolver, skill_results) = tokio::join!(namespace_resolver, skill_results);
|
||||
for (path, path_uri, discovery_path, result) in skill_results {
|
||||
let result = result.and_then(|mut skill| {
|
||||
skill.name = namespace_resolver
|
||||
.for_skill(&root_uri, &path_uri)
|
||||
.qualify(&skill.name);
|
||||
validate_len(&skill.name, MAX_QUALIFIED_NAME_LEN, "qualified name")
|
||||
.map_err(|err| err.to_string())?;
|
||||
Ok(skill)
|
||||
});
|
||||
match result {
|
||||
Ok(skill) => {
|
||||
Arc::make_mut(&mut outcome.skill_discovery_path_by_path)
|
||||
.insert(skill.path_to_skills_md.clone(), discovery_path);
|
||||
outcome.skills.push(skill);
|
||||
}
|
||||
Err(err) if skill_root.scope != SkillScope::System => {
|
||||
outcome.errors.push(SkillError { path, message: err })
|
||||
}
|
||||
Err(_) => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn parse_skill_file(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
skill: &DiscoveredSkill,
|
||||
path: &AbsolutePathBuf,
|
||||
path_uri: &PathUri,
|
||||
scope: SkillScope,
|
||||
plugin_identity: Option<&PluginIdentity>,
|
||||
plugin_root: Option<&AbsolutePathBuf>,
|
||||
) -> Result<SkillMetadata, SkillLoadError> {
|
||||
let metadata_path = path_uri
|
||||
.parent()
|
||||
.and_then(|parent| parent.join(SKILLS_METADATA_DIR).ok())
|
||||
.and_then(|directory| directory.join(SKILLS_METADATA_FILENAME).ok());
|
||||
let metadata = match &skill.metadata {
|
||||
SkillMetadataDiscovery::Present(_) => metadata_path.map(SkillMetadataDiscovery::Present),
|
||||
SkillMetadataDiscovery::Probe(_) => metadata_path.map(SkillMetadataDiscovery::Probe),
|
||||
SkillMetadataDiscovery::Absent => None,
|
||||
}
|
||||
.unwrap_or(SkillMetadataDiscovery::Absent);
|
||||
let (contents, loaded_metadata) = tokio::join!(
|
||||
fs.read_file_text(path_uri, /*sandbox*/ None),
|
||||
load_skill_metadata(fs, path, &metadata, plugin_root),
|
||||
);
|
||||
let contents = contents.map_err(SkillLoadError::Read)?;
|
||||
let ParsedSkillFrontmatter {
|
||||
name: base_name,
|
||||
description,
|
||||
short_description,
|
||||
} = parse_skill_frontmatter_metadata(&contents, || default_skill_name(path))?;
|
||||
let LoadedSkillMetadata {
|
||||
interface,
|
||||
dependencies,
|
||||
policy,
|
||||
} = loaded_metadata;
|
||||
|
||||
Ok(SkillMetadata {
|
||||
name: base_name,
|
||||
description,
|
||||
short_description,
|
||||
interface,
|
||||
dependencies,
|
||||
policy,
|
||||
path_to_skills_md: path.clone(),
|
||||
scope,
|
||||
plugin_id: plugin_identity.map(|identity| identity.plugin_id.clone()),
|
||||
remote_plugin_id: plugin_identity.and_then(|identity| identity.remote_plugin_id.clone()),
|
||||
})
|
||||
}
|
||||
|
||||
fn default_skill_name(path: &AbsolutePathBuf) -> String {
|
||||
path.parent()
|
||||
.and_then(|parent| {
|
||||
parent
|
||||
.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.map(sanitize_single_line)
|
||||
})
|
||||
.filter(|value| !value.is_empty())
|
||||
.unwrap_or_else(|| "skill".to_string())
|
||||
}
|
||||
|
||||
async fn load_skill_metadata(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
skill_path: &AbsolutePathBuf,
|
||||
metadata: &SkillMetadataDiscovery,
|
||||
plugin_root: Option<&AbsolutePathBuf>,
|
||||
) -> LoadedSkillMetadata {
|
||||
// Fail open: optional metadata should not block loading SKILL.md.
|
||||
let Some(skill_dir) = skill_path.parent() else {
|
||||
return LoadedSkillMetadata::default();
|
||||
};
|
||||
let metadata_path_uri = match metadata {
|
||||
SkillMetadataDiscovery::Present(path) => path,
|
||||
SkillMetadataDiscovery::Absent => return LoadedSkillMetadata::default(),
|
||||
SkillMetadataDiscovery::Probe(path) => {
|
||||
match fs.get_metadata(path, /*sandbox*/ None).await {
|
||||
Ok(metadata) if metadata.is_file => {}
|
||||
Ok(_) => return LoadedSkillMetadata::default(),
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => {
|
||||
return LoadedSkillMetadata::default();
|
||||
}
|
||||
Err(error) => {
|
||||
tracing::warn!(
|
||||
"ignoring {path}: failed to stat {label}: {error}",
|
||||
path = path,
|
||||
label = SKILLS_METADATA_FILENAME
|
||||
);
|
||||
return LoadedSkillMetadata::default();
|
||||
}
|
||||
}
|
||||
path
|
||||
}
|
||||
};
|
||||
|
||||
let contents = match fs.read_file_text(metadata_path_uri, /*sandbox*/ None).await {
|
||||
Ok(contents) => contents,
|
||||
Err(error) => {
|
||||
tracing::warn!(
|
||||
"ignoring {path}: failed to read {label}: {error}",
|
||||
path = metadata_path_uri,
|
||||
label = SKILLS_METADATA_FILENAME
|
||||
);
|
||||
return LoadedSkillMetadata::default();
|
||||
}
|
||||
};
|
||||
|
||||
let parsed: SkillMetadataFile = {
|
||||
let _guard = AbsolutePathBufGuard::new(skill_dir.as_path());
|
||||
match serde_yaml::from_str(&contents) {
|
||||
Ok(parsed) => parsed,
|
||||
Err(error) => {
|
||||
tracing::warn!(
|
||||
"ignoring {path}: invalid {label}: {error}",
|
||||
path = metadata_path_uri,
|
||||
label = SKILLS_METADATA_FILENAME
|
||||
);
|
||||
return LoadedSkillMetadata::default();
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let SkillMetadataFile {
|
||||
interface,
|
||||
dependencies,
|
||||
policy,
|
||||
} = parsed;
|
||||
let asset_policy = match plugin_root {
|
||||
Some(plugin_root) => SkillInterfaceAssetPolicy::PluginShared { plugin_root },
|
||||
None => SkillInterfaceAssetPolicy::LocalOnly,
|
||||
};
|
||||
LoadedSkillMetadata {
|
||||
interface: resolve_skill_interface(interface, &skill_dir, asset_policy),
|
||||
dependencies: resolve_dependencies(dependencies),
|
||||
policy: resolve_policy(policy),
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_dependencies(dependencies: Option<Dependencies>) -> Option<SkillDependencies> {
|
||||
let dependencies = dependencies?;
|
||||
let tools: Vec<SkillToolDependency> = dependencies
|
||||
.tools
|
||||
.into_iter()
|
||||
.filter_map(resolve_dependency_tool)
|
||||
.collect();
|
||||
if tools.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(SkillDependencies { tools })
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_policy(policy: Option<Policy>) -> Option<SkillPolicy> {
|
||||
policy.map(|policy| SkillPolicy {
|
||||
allow_implicit_invocation: policy.allow_implicit_invocation,
|
||||
products: policy.products,
|
||||
})
|
||||
}
|
||||
|
||||
fn resolve_dependency_tool(tool: DependencyTool) -> Option<SkillToolDependency> {
|
||||
let r#type = resolve_required_str(
|
||||
tool.kind,
|
||||
MAX_DEPENDENCY_TYPE_LEN,
|
||||
"dependencies.tools.type",
|
||||
)?;
|
||||
let value = resolve_required_str(
|
||||
tool.value,
|
||||
MAX_DEPENDENCY_VALUE_LEN,
|
||||
"dependencies.tools.value",
|
||||
)?;
|
||||
let description = resolve_str(
|
||||
tool.description,
|
||||
MAX_DEPENDENCY_DESCRIPTION_LEN,
|
||||
"dependencies.tools.description",
|
||||
);
|
||||
let transport = resolve_str(
|
||||
tool.transport,
|
||||
MAX_DEPENDENCY_TRANSPORT_LEN,
|
||||
"dependencies.tools.transport",
|
||||
);
|
||||
let command = resolve_str(
|
||||
tool.command,
|
||||
MAX_DEPENDENCY_COMMAND_LEN,
|
||||
"dependencies.tools.command",
|
||||
);
|
||||
let url = resolve_str(tool.url, MAX_DEPENDENCY_URL_LEN, "dependencies.tools.url");
|
||||
|
||||
Some(SkillToolDependency {
|
||||
r#type,
|
||||
value,
|
||||
description,
|
||||
transport,
|
||||
command,
|
||||
url,
|
||||
})
|
||||
}
|
||||
|
||||
fn sanitize_single_line(raw: &str) -> String {
|
||||
raw.split_whitespace().collect::<Vec<_>>().join(" ")
|
||||
}
|
||||
|
||||
fn validate_len(
|
||||
value: &str,
|
||||
max_len: usize,
|
||||
field_name: &'static str,
|
||||
) -> Result<(), SkillParseError> {
|
||||
if value.is_empty() {
|
||||
return Err(SkillParseError::MissingField(field_name));
|
||||
}
|
||||
if value.chars().count() > max_len {
|
||||
return Err(SkillParseError::InvalidField {
|
||||
field: field_name,
|
||||
reason: format!("exceeds maximum length of {max_len} characters"),
|
||||
});
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn resolve_str(value: Option<String>, max_len: usize, field: &'static str) -> Option<String> {
|
||||
let value = value?;
|
||||
let value = sanitize_single_line(&value);
|
||||
if value.is_empty() {
|
||||
tracing::warn!("ignoring {field}: value is empty");
|
||||
return None;
|
||||
}
|
||||
if value.chars().count() > max_len {
|
||||
tracing::warn!("ignoring {field}: exceeds maximum length of {max_len} characters");
|
||||
return None;
|
||||
}
|
||||
Some(value)
|
||||
}
|
||||
|
||||
fn resolve_required_str(
|
||||
value: Option<String>,
|
||||
max_len: usize,
|
||||
field: &'static str,
|
||||
) -> Option<String> {
|
||||
let Some(value) = value else {
|
||||
tracing::warn!("ignoring {field}: value is missing");
|
||||
return None;
|
||||
};
|
||||
resolve_str(Some(value), max_len, field)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "loader_tests.rs"]
|
||||
mod tests;
|
||||
@@ -1,219 +0,0 @@
|
||||
use std::collections::HashSet;
|
||||
use std::io;
|
||||
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::WalkEntryKind;
|
||||
use codex_exec_server::WalkOptions;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::DISCOVERABLE_PLUGIN_MANIFEST_PATHS;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
|
||||
use super::MAX_SCAN_DEPTH;
|
||||
use super::MAX_SKILLS_DIRS_PER_ROOT;
|
||||
use super::SKILLS_FILENAME;
|
||||
use super::SKILLS_METADATA_DIR;
|
||||
use super::SKILLS_METADATA_FILENAME;
|
||||
|
||||
const MAX_SKILLS_ENTRIES_PER_ROOT: usize = 20_000;
|
||||
pub(super) const MAX_CONCURRENT_SKILL_LOADS: usize = 64;
|
||||
|
||||
pub(super) enum DirectorySymlinkPolicy {
|
||||
Follow,
|
||||
Ignore,
|
||||
}
|
||||
|
||||
pub(super) enum HiddenDirectoryPolicy {
|
||||
Include,
|
||||
Skip,
|
||||
}
|
||||
|
||||
pub(super) struct SkillDiscoveryOptions {
|
||||
pub directory_symlinks: DirectorySymlinkPolicy,
|
||||
pub hidden_directories: HiddenDirectoryPolicy,
|
||||
pub mode: SkillDiscoveryMode,
|
||||
}
|
||||
|
||||
pub(super) struct SkillDiscovery {
|
||||
pub skills: Vec<DiscoveredSkill>,
|
||||
pub plugin_roots: HashSet<PathUri>,
|
||||
pub namespace_roots: HashSet<PathUri>,
|
||||
pub warnings: Vec<String>,
|
||||
}
|
||||
|
||||
pub(super) struct DiscoveredSkill {
|
||||
pub path: PathUri,
|
||||
pub metadata: SkillMetadataDiscovery,
|
||||
}
|
||||
|
||||
pub(super) enum SkillMetadataDiscovery {
|
||||
Present(PathUri),
|
||||
Absent,
|
||||
Probe(PathUri),
|
||||
}
|
||||
|
||||
pub(super) async fn discover_skills(
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
root: &PathUri,
|
||||
options: SkillDiscoveryOptions,
|
||||
) -> SkillDiscovery {
|
||||
let empty_discovery = || SkillDiscovery {
|
||||
skills: Vec::new(),
|
||||
plugin_roots: HashSet::new(),
|
||||
namespace_roots: HashSet::new(),
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
let walk = match file_system
|
||||
.walk(
|
||||
root,
|
||||
WalkOptions {
|
||||
max_depth: match options.mode {
|
||||
SkillDiscoveryMode::Recursive => MAX_SCAN_DEPTH,
|
||||
SkillDiscoveryMode::DirectChildren => 2,
|
||||
},
|
||||
max_directories: MAX_SKILLS_DIRS_PER_ROOT,
|
||||
max_entries: MAX_SKILLS_ENTRIES_PER_ROOT,
|
||||
follow_directory_symlinks: matches!(
|
||||
options.directory_symlinks,
|
||||
DirectorySymlinkPolicy::Follow
|
||||
),
|
||||
prune_hidden_directories: matches!(
|
||||
options.hidden_directories,
|
||||
HiddenDirectoryPolicy::Skip
|
||||
),
|
||||
},
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(walk) => walk,
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => return empty_discovery(),
|
||||
Err(error) => {
|
||||
let mut discovery = empty_discovery();
|
||||
discovery
|
||||
.warnings
|
||||
.push(format!("failed to walk skills root {root}: {error:#}"));
|
||||
return discovery;
|
||||
}
|
||||
};
|
||||
|
||||
let inventory_complete = !walk.truncated && walk.errors.is_empty();
|
||||
let mut warnings = walk
|
||||
.errors
|
||||
.into_iter()
|
||||
.map(|error| {
|
||||
format!(
|
||||
"failed to scan skill path {}: {}",
|
||||
error.path, error.message
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
if walk.truncated {
|
||||
warnings.push(format!(
|
||||
"skills scan reached its traversal limit (root: {root})"
|
||||
));
|
||||
}
|
||||
|
||||
let skip_hidden = matches!(options.hidden_directories, HiddenDirectoryPolicy::Skip);
|
||||
let mut skill_files = Vec::new();
|
||||
let mut file_paths = HashSet::new();
|
||||
let mut metadata_directory_parents = HashSet::new();
|
||||
let mut plugin_roots = HashSet::new();
|
||||
for entry in walk.entries {
|
||||
if skip_hidden && has_hidden_ancestor_below_root(&entry.path, root) {
|
||||
continue;
|
||||
}
|
||||
match entry.kind {
|
||||
WalkEntryKind::Directory => {
|
||||
if entry
|
||||
.path
|
||||
.basename()
|
||||
.is_some_and(|name| name.eq_ignore_ascii_case(SKILLS_METADATA_DIR))
|
||||
&& let Some(skill_dir) = entry.path.parent()
|
||||
{
|
||||
metadata_directory_parents.insert(skill_dir);
|
||||
}
|
||||
if DISCOVERABLE_PLUGIN_MANIFEST_PATHS
|
||||
.iter()
|
||||
.any(|path| path.split('/').next() == entry.path.basename().as_deref())
|
||||
&& let Some(plugin_root) = entry.path.parent()
|
||||
{
|
||||
plugin_roots.insert(plugin_root);
|
||||
}
|
||||
}
|
||||
WalkEntryKind::File => {
|
||||
file_paths.insert(entry.path.clone());
|
||||
if entry.path.basename().as_deref() == Some(SKILLS_FILENAME)
|
||||
&& (options.mode == SkillDiscoveryMode::Recursive
|
||||
|| is_direct_child_skill_path(&entry.path, root))
|
||||
{
|
||||
skill_files.push(entry.path);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
let skills = skill_files
|
||||
.into_iter()
|
||||
.map(|path| DiscoveredSkill {
|
||||
metadata: discover_skill_metadata(
|
||||
&path,
|
||||
&file_paths,
|
||||
&metadata_directory_parents,
|
||||
inventory_complete,
|
||||
),
|
||||
path,
|
||||
})
|
||||
.collect();
|
||||
|
||||
SkillDiscovery {
|
||||
skills,
|
||||
plugin_roots,
|
||||
namespace_roots: HashSet::from([root.clone()]),
|
||||
warnings,
|
||||
}
|
||||
}
|
||||
|
||||
fn is_direct_child_skill_path(path: &PathUri, root: &PathUri) -> bool {
|
||||
path.parent().and_then(|parent| parent.parent()).as_ref() == Some(root)
|
||||
}
|
||||
|
||||
fn has_hidden_ancestor_below_root(path: &PathUri, root: &PathUri) -> bool {
|
||||
let mut ancestor = path.parent();
|
||||
while let Some(current) = ancestor {
|
||||
if ¤t == root {
|
||||
return false;
|
||||
}
|
||||
if current.basename().is_some_and(|name| name.starts_with('.')) {
|
||||
return true;
|
||||
}
|
||||
ancestor = current.parent();
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
fn discover_skill_metadata(
|
||||
skill_path: &PathUri,
|
||||
file_paths: &HashSet<PathUri>,
|
||||
metadata_directory_parents: &HashSet<PathUri>,
|
||||
inventory_complete: bool,
|
||||
) -> SkillMetadataDiscovery {
|
||||
let Some(skill_dir) = skill_path.parent() else {
|
||||
return SkillMetadataDiscovery::Absent;
|
||||
};
|
||||
let Ok(metadata_dir) = skill_dir.join(SKILLS_METADATA_DIR) else {
|
||||
return SkillMetadataDiscovery::Absent;
|
||||
};
|
||||
let Ok(metadata_path) = metadata_dir.join(SKILLS_METADATA_FILENAME) else {
|
||||
return SkillMetadataDiscovery::Absent;
|
||||
};
|
||||
if file_paths.contains(&metadata_path) {
|
||||
return SkillMetadataDiscovery::Present(metadata_path);
|
||||
}
|
||||
|
||||
if inventory_complete && !metadata_directory_parents.contains(&skill_dir) {
|
||||
SkillMetadataDiscovery::Absent
|
||||
} else {
|
||||
// A complete walk proves ordinary absence, but keep a filesystem probe for case aliases,
|
||||
// file symlinks omitted by the walk, and incomplete inventories.
|
||||
SkillMetadataDiscovery::Probe(metadata_path)
|
||||
}
|
||||
}
|
||||
@@ -1,252 +0,0 @@
|
||||
use std::io;
|
||||
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_protocol::protocol::Product;
|
||||
pub use codex_skills::EnvironmentSkillMetadata;
|
||||
use codex_skills::ParsedSkillFrontmatter;
|
||||
use codex_skills::parse_skill_frontmatter_metadata;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use futures::StreamExt;
|
||||
|
||||
use crate::model::SkillDependencies;
|
||||
use crate::model::SkillPolicy;
|
||||
|
||||
use super::MAX_QUALIFIED_NAME_LEN;
|
||||
use super::SkillMetadataFile;
|
||||
use super::discovery::DirectorySymlinkPolicy;
|
||||
use super::discovery::DiscoveredSkill;
|
||||
use super::discovery::HiddenDirectoryPolicy;
|
||||
use super::discovery::MAX_CONCURRENT_SKILL_LOADS;
|
||||
use super::discovery::SkillDiscoveryOptions;
|
||||
use super::discovery::SkillMetadataDiscovery;
|
||||
use super::discovery::discover_skills;
|
||||
use super::namespace::SkillNamespaceResolver;
|
||||
use super::resolve_dependencies;
|
||||
use super::resolve_policy;
|
||||
use super::sanitize_single_line;
|
||||
use super::validate_len;
|
||||
|
||||
struct ParsedEnvironmentSkill {
|
||||
path_to_skills_md: PathUri,
|
||||
base_name: String,
|
||||
description: String,
|
||||
short_description: Option<String>,
|
||||
dependencies: Option<SkillDependencies>,
|
||||
policy: Option<SkillPolicy>,
|
||||
}
|
||||
|
||||
impl ParsedEnvironmentSkill {
|
||||
async fn load(
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
skill: &DiscoveredSkill,
|
||||
) -> Result<Self, String> {
|
||||
let (contents, discovered_metadata) = match &skill.metadata {
|
||||
SkillMetadataDiscovery::Present(metadata_path) => {
|
||||
let (contents, metadata) = tokio::join!(
|
||||
read_skill_contents(file_system, &skill.path),
|
||||
read_skill_metadata(file_system, metadata_path),
|
||||
);
|
||||
(contents?, metadata)
|
||||
}
|
||||
SkillMetadataDiscovery::Absent | SkillMetadataDiscovery::Probe(_) => (
|
||||
read_skill_contents(file_system, &skill.path).await?,
|
||||
(None, None),
|
||||
),
|
||||
};
|
||||
let ParsedSkillFrontmatter {
|
||||
name: base_name,
|
||||
description,
|
||||
short_description,
|
||||
} = parse_skill_frontmatter_metadata(&contents, || default_skill_name(&skill.path))
|
||||
.map_err(|err| err.to_string())?;
|
||||
let (dependencies, policy) = match &skill.metadata {
|
||||
SkillMetadataDiscovery::Present(_) | SkillMetadataDiscovery::Absent => {
|
||||
discovered_metadata
|
||||
}
|
||||
SkillMetadataDiscovery::Probe(metadata_path) => {
|
||||
probe_skill_metadata(file_system, metadata_path).await
|
||||
}
|
||||
};
|
||||
|
||||
Ok(Self {
|
||||
path_to_skills_md: skill.path.clone(),
|
||||
base_name,
|
||||
description,
|
||||
short_description,
|
||||
dependencies,
|
||||
policy,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
pub struct EnvironmentSkillLoadOutcome {
|
||||
pub skills: Vec<EnvironmentSkillMetadata>,
|
||||
pub warnings: Vec<String>,
|
||||
}
|
||||
|
||||
/// Discovers skills without converting environment-owned paths to host paths.
|
||||
#[tracing::instrument(
|
||||
name = "skills.environment.load",
|
||||
level = "info",
|
||||
skip_all,
|
||||
fields(skill_count = tracing::field::Empty)
|
||||
)]
|
||||
pub async fn load_environment_skills_from_root(
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
root: &PathUri,
|
||||
restriction_product: Option<Product>,
|
||||
) -> EnvironmentSkillLoadOutcome {
|
||||
let mut outcome = EnvironmentSkillLoadOutcome::default();
|
||||
let discovery = discover_skills(
|
||||
file_system,
|
||||
root,
|
||||
// Preserve environment discovery behavior by following directory aliases and including
|
||||
// hidden directories exposed by the executor.
|
||||
SkillDiscoveryOptions {
|
||||
directory_symlinks: DirectorySymlinkPolicy::Follow,
|
||||
hidden_directories: HiddenDirectoryPolicy::Include,
|
||||
mode: codex_utils_plugins::SkillDiscoveryMode::Recursive,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
tracing::Span::current().record("skill_count", discovery.skills.len());
|
||||
outcome.warnings.extend(discovery.warnings);
|
||||
if discovery.skills.is_empty() {
|
||||
return outcome;
|
||||
}
|
||||
|
||||
let skill_paths = discovery
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| skill.path.clone())
|
||||
.collect::<Vec<_>>();
|
||||
let namespace_resolver = SkillNamespaceResolver::discover(
|
||||
file_system,
|
||||
root,
|
||||
&skill_paths,
|
||||
discovery.plugin_roots,
|
||||
discovery.namespace_roots,
|
||||
);
|
||||
|
||||
// Remote executors can multiplex these independent per-skill reads, so polling a bounded
|
||||
// number together allows the I/O for each skill and its metadata to happen concurrently.
|
||||
let skill_results = futures::stream::iter(discovery.skills)
|
||||
.map(|skill| {
|
||||
let path = skill.path.clone();
|
||||
async move {
|
||||
(
|
||||
path,
|
||||
ParsedEnvironmentSkill::load(file_system, &skill).await,
|
||||
)
|
||||
}
|
||||
})
|
||||
.buffered(MAX_CONCURRENT_SKILL_LOADS)
|
||||
.collect::<Vec<_>>();
|
||||
let (namespace_resolver, skill_results) = tokio::join!(namespace_resolver, skill_results);
|
||||
|
||||
for (path, result) in skill_results {
|
||||
let result = result.and_then(|skill| {
|
||||
let name = namespace_resolver
|
||||
.for_skill(root, &skill.path_to_skills_md)
|
||||
.qualify(&skill.base_name);
|
||||
validate_len(&name, MAX_QUALIFIED_NAME_LEN, "qualified name")
|
||||
.map_err(|err| err.to_string())?;
|
||||
|
||||
Ok(EnvironmentSkillMetadata {
|
||||
path_to_skills_md: skill.path_to_skills_md,
|
||||
name,
|
||||
description: skill.description,
|
||||
short_description: skill.short_description,
|
||||
dependencies: skill.dependencies,
|
||||
policy: skill.policy,
|
||||
})
|
||||
});
|
||||
match result {
|
||||
Ok(skill) if skill.matches_product_restriction(restriction_product) => {
|
||||
outcome.skills.push(skill);
|
||||
}
|
||||
Ok(_) => {}
|
||||
Err(message) => outcome.warnings.push(format!(
|
||||
"Failed to load environment skill at {path}: {message}"
|
||||
)),
|
||||
}
|
||||
}
|
||||
outcome.skills.sort_by(|left, right| {
|
||||
left.name.cmp(&right.name).then_with(|| {
|
||||
left.path_to_skills_md
|
||||
.to_string()
|
||||
.cmp(&right.path_to_skills_md.to_string())
|
||||
})
|
||||
});
|
||||
outcome
|
||||
}
|
||||
|
||||
async fn read_skill_contents(
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
skill_path: &PathUri,
|
||||
) -> Result<String, String> {
|
||||
file_system
|
||||
.read_file_text(skill_path, /*sandbox*/ None)
|
||||
.await
|
||||
.map_err(|err| format!("failed to read file: {err}"))
|
||||
}
|
||||
|
||||
async fn probe_skill_metadata(
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
metadata_path: &PathUri,
|
||||
) -> (Option<SkillDependencies>, Option<SkillPolicy>) {
|
||||
match file_system
|
||||
.get_metadata(metadata_path, /*sandbox*/ None)
|
||||
.await
|
||||
{
|
||||
Ok(metadata) if metadata.is_file => {}
|
||||
Ok(_) => return (None, None),
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => return (None, None),
|
||||
Err(error) => {
|
||||
tracing::warn!("ignoring {metadata_path}: failed to stat metadata: {error}");
|
||||
return (None, None);
|
||||
}
|
||||
}
|
||||
read_skill_metadata(file_system, metadata_path).await
|
||||
}
|
||||
|
||||
async fn read_skill_metadata(
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
metadata_path: &PathUri,
|
||||
) -> (Option<SkillDependencies>, Option<SkillPolicy>) {
|
||||
let contents = match file_system
|
||||
.read_file_text(metadata_path, /*sandbox*/ None)
|
||||
.await
|
||||
{
|
||||
Ok(contents) => contents,
|
||||
Err(error) => {
|
||||
tracing::warn!("ignoring {metadata_path}: failed to read metadata: {error}");
|
||||
return (None, None);
|
||||
}
|
||||
};
|
||||
let parsed: SkillMetadataFile = match serde_yaml::from_str(&contents) {
|
||||
Ok(parsed) => parsed,
|
||||
Err(error) => {
|
||||
tracing::warn!("ignoring {metadata_path}: invalid metadata: {error}");
|
||||
return (None, None);
|
||||
}
|
||||
};
|
||||
|
||||
(
|
||||
resolve_dependencies(parsed.dependencies),
|
||||
resolve_policy(parsed.policy),
|
||||
)
|
||||
}
|
||||
|
||||
fn default_skill_name(path: &PathUri) -> String {
|
||||
path.parent()
|
||||
.and_then(|parent| parent.basename())
|
||||
.map(|name| sanitize_single_line(&name))
|
||||
.filter(|name| !name.is_empty())
|
||||
.unwrap_or_else(|| "skill".to_string())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "environment_tests.rs"]
|
||||
mod tests;
|
||||
@@ -1,78 +0,0 @@
|
||||
use std::fs;
|
||||
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
use crate::model::SkillDependencies;
|
||||
use crate::model::SkillPolicy;
|
||||
use crate::model::SkillToolDependency;
|
||||
|
||||
use super::EnvironmentSkillMetadata;
|
||||
use super::load_environment_skills_from_root;
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_plugin_namespace_dependencies_and_policy() {
|
||||
let root = tempdir().expect("tempdir");
|
||||
let skill_dir = root.path().join("skills/deploy");
|
||||
fs::create_dir_all(root.path().join(".codex-plugin")).expect("manifest dir");
|
||||
fs::create_dir_all(skill_dir.join("agents")).expect("metadata dir");
|
||||
fs::write(
|
||||
root.path().join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"demo-plugin"}"#,
|
||||
)
|
||||
.expect("manifest");
|
||||
fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
"---\nname: deploy\ndescription: Deploy the service.\n---\n",
|
||||
)
|
||||
.expect("skill");
|
||||
fs::write(
|
||||
skill_dir.join("agents/openai.yaml"),
|
||||
r#"
|
||||
dependencies:
|
||||
tools:
|
||||
- type: mcp
|
||||
value: deploy-server
|
||||
description: Deploy MCP
|
||||
policy:
|
||||
allow_implicit_invocation: false
|
||||
products: [codex]
|
||||
"#,
|
||||
)
|
||||
.expect("metadata");
|
||||
|
||||
let root_uri = PathUri::from_host_native_path(root.path()).expect("root URI");
|
||||
let outcome =
|
||||
load_environment_skills_from_root(LOCAL_FS.as_ref(), &root_uri, Some(Product::Codex)).await;
|
||||
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![EnvironmentSkillMetadata {
|
||||
path_to_skills_md: PathUri::from_host_native_path(skill_dir.join("SKILL.md"),).unwrap(),
|
||||
name: "demo-plugin:deploy".to_string(),
|
||||
description: "Deploy the service.".to_string(),
|
||||
short_description: None,
|
||||
dependencies: Some(SkillDependencies {
|
||||
tools: vec![SkillToolDependency {
|
||||
r#type: "mcp".to_string(),
|
||||
value: "deploy-server".to_string(),
|
||||
description: Some("Deploy MCP".to_string()),
|
||||
transport: None,
|
||||
command: None,
|
||||
url: None,
|
||||
}],
|
||||
}),
|
||||
policy: Some(SkillPolicy {
|
||||
allow_implicit_invocation: Some(false),
|
||||
products: vec![Product::Codex],
|
||||
}),
|
||||
}]
|
||||
);
|
||||
let filtered =
|
||||
load_environment_skills_from_root(LOCAL_FS.as_ref(), &root_uri, Some(Product::Chatgpt))
|
||||
.await;
|
||||
assert!(filtered.skills.is_empty());
|
||||
}
|
||||
@@ -1,182 +0,0 @@
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::plugin_namespace_for_root_uri;
|
||||
use futures::StreamExt;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
use super::discovery::MAX_CONCURRENT_SKILL_LOADS;
|
||||
|
||||
/// Resolves the namespace prefix applied to skill names during one skills scan.
|
||||
///
|
||||
/// A plugin namespace is the plugin name from the nearest valid plugin manifest
|
||||
/// above a skill path. For example, a skill named `search` beneath a plugin named
|
||||
/// `sample` is exposed as `sample:search`.
|
||||
///
|
||||
/// Resolving the namespace separately for every `SKILL.md` repeats the same
|
||||
/// ancestor manifest probes for sibling skills. This resolver resolves relevant
|
||||
/// roots once per scan, then selects the nearest matching root for each skill.
|
||||
///
|
||||
/// Namespace precedence is:
|
||||
///
|
||||
/// 1. an explicitly provided plugin namespace;
|
||||
/// 2. the deepest matching canonical symlink root or nested plugin root;
|
||||
/// 3. the namespace inherited from the scanned skills root.
|
||||
pub(crate) struct SkillNamespaceResolver {
|
||||
inherited_namespace: ResolvedSkillNamespace,
|
||||
nested_namespaces: Vec<(PathUri, ResolvedSkillNamespace)>,
|
||||
}
|
||||
|
||||
impl SkillNamespaceResolver {
|
||||
/// Builds a resolver whose explicit plugin-owned namespace overrides discovery.
|
||||
pub(crate) fn with_provided_namespace(namespace: &str) -> Self {
|
||||
Self {
|
||||
inherited_namespace: ResolvedSkillNamespace::Plugin(namespace.to_string()),
|
||||
nested_namespaces: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn discover(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
root: &PathUri,
|
||||
skill_paths: &[PathUri],
|
||||
plugin_roots: HashSet<PathUri>,
|
||||
namespace_roots: HashSet<PathUri>,
|
||||
) -> Self {
|
||||
// Only probe plugin roots above loaded skills; unused siblings cannot affect names.
|
||||
let mut skill_ancestors = HashSet::new();
|
||||
for skill_path in skill_paths {
|
||||
let mut ancestor = skill_path.parent();
|
||||
while let Some(path) = ancestor {
|
||||
skill_ancestors.insert(path.clone());
|
||||
ancestor = path.parent();
|
||||
}
|
||||
}
|
||||
let plugin_roots = plugin_roots
|
||||
.into_iter()
|
||||
.filter(|plugin_root| skill_ancestors.contains(plugin_root))
|
||||
.collect::<HashSet<_>>();
|
||||
|
||||
// The scan root is already the fallback above if nothing else matches, exclude from the search.
|
||||
let namespace_roots = namespace_roots
|
||||
.into_iter()
|
||||
.filter(|namespace_root| namespace_root != root)
|
||||
.collect::<Vec<_>>();
|
||||
let namespace_root_set = namespace_roots.iter().cloned().collect::<HashSet<_>>();
|
||||
let plugin_roots = plugin_roots
|
||||
.into_iter()
|
||||
.filter(|plugin_root| plugin_root != root && !namespace_root_set.contains(plugin_root))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let lookup_roots = std::iter::once(root.clone())
|
||||
.chain(namespace_roots.iter().cloned())
|
||||
.collect::<Vec<_>>();
|
||||
let mut pending_lookups = lookup_roots
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|lookup_root| (lookup_root.clone(), lookup_root))
|
||||
.collect::<Vec<_>>();
|
||||
let mut direct_plugin_roots = plugin_roots.iter().cloned().collect::<HashSet<_>>();
|
||||
let mut namespaces_by_root = HashMap::new();
|
||||
let mut namespaces_by_lookup_root = HashMap::new();
|
||||
while !pending_lookups.is_empty() {
|
||||
let probe_roots = pending_lookups
|
||||
.iter()
|
||||
.map(|(_, ancestor)| ancestor.clone())
|
||||
.chain(direct_plugin_roots.drain())
|
||||
.filter(|ancestor| !namespaces_by_root.contains_key(ancestor))
|
||||
.collect::<HashSet<_>>();
|
||||
namespaces_by_root.extend(
|
||||
futures::stream::iter(probe_roots)
|
||||
.map(|manifest_root| async move {
|
||||
let namespace = plugin_namespace_for_root_uri(fs, &manifest_root).await;
|
||||
(manifest_root, namespace)
|
||||
})
|
||||
.buffered(MAX_CONCURRENT_SKILL_LOADS)
|
||||
.collect::<HashMap<_, _>>()
|
||||
.await,
|
||||
);
|
||||
|
||||
let mut next_lookups = Vec::new();
|
||||
for (lookup_root, ancestor) in pending_lookups {
|
||||
match namespaces_by_root.get(&ancestor) {
|
||||
Some(Some(namespace)) => {
|
||||
namespaces_by_lookup_root.insert(lookup_root, Some(namespace.clone()));
|
||||
}
|
||||
Some(None) => match ancestor.parent() {
|
||||
Some(parent) => next_lookups.push((lookup_root, parent)),
|
||||
None => {
|
||||
namespaces_by_lookup_root.insert(lookup_root, None);
|
||||
}
|
||||
},
|
||||
None => unreachable!("pending namespace ancestor was not probed"),
|
||||
}
|
||||
}
|
||||
pending_lookups = next_lookups;
|
||||
}
|
||||
|
||||
// Ordinary descendants fall back to the nearest valid manifest at or above the scan root.
|
||||
let inherited_namespace = namespaces_by_lookup_root
|
||||
.get(root)
|
||||
.and_then(Option::as_ref)
|
||||
.cloned()
|
||||
.map(ResolvedSkillNamespace::Plugin)
|
||||
.unwrap_or(ResolvedSkillNamespace::Plain);
|
||||
let namespace_lookups = namespace_roots.into_iter().map(|namespace_root| {
|
||||
let namespace = namespaces_by_lookup_root
|
||||
.get(&namespace_root)
|
||||
.and_then(Option::as_ref)
|
||||
.cloned()
|
||||
.map(ResolvedSkillNamespace::Plugin)
|
||||
.unwrap_or(ResolvedSkillNamespace::Plain);
|
||||
(namespace_root, namespace)
|
||||
});
|
||||
// Invalid nested manifests are omitted, so the deepest remaining match wins.
|
||||
let plugin_lookups = plugin_roots.into_iter().filter_map(|plugin_root| {
|
||||
namespaces_by_root
|
||||
.get(&plugin_root)
|
||||
.and_then(Option::as_ref)
|
||||
.cloned()
|
||||
.map(|namespace| (plugin_root, ResolvedSkillNamespace::Plugin(namespace)))
|
||||
});
|
||||
let nested_namespaces = namespace_lookups.chain(plugin_lookups).collect();
|
||||
|
||||
Self {
|
||||
inherited_namespace,
|
||||
nested_namespaces,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn for_skill(&self, root: &PathUri, path: &PathUri) -> &ResolvedSkillNamespace {
|
||||
// Ancestor symlink targets cannot override skills still owned by the scan root.
|
||||
let path_is_under_root = path.starts_with(root);
|
||||
// The deepest matching path prefix is the nearest applicable namespace.
|
||||
self.nested_namespaces
|
||||
.iter()
|
||||
.filter(|(namespace_root, _)| {
|
||||
path.starts_with(namespace_root)
|
||||
&& (!path_is_under_root || !root.starts_with(namespace_root))
|
||||
})
|
||||
.max_by_key(|(namespace_root, _)| namespace_root.ancestors().count())
|
||||
.map(|(_, namespace)| namespace)
|
||||
.unwrap_or(&self.inherited_namespace)
|
||||
}
|
||||
}
|
||||
|
||||
/// The completed namespace resolution for a skill root.
|
||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||
pub(crate) enum ResolvedSkillNamespace {
|
||||
/// No plugin namespace applies to matching skills.
|
||||
Plain,
|
||||
/// Qualify matching skill names with this plugin namespace.
|
||||
Plugin(String),
|
||||
}
|
||||
|
||||
impl ResolvedSkillNamespace {
|
||||
pub(crate) fn qualify(&self, base_name: &str) -> String {
|
||||
match self {
|
||||
Self::Plain => base_name.to_string(),
|
||||
Self::Plugin(namespace) => format!("{namespace}:{base_name}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
File diff suppressed because it is too large
Load Diff
@@ -6,7 +6,6 @@ use std::sync::Arc;
|
||||
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_protocol::protocol::Product;
|
||||
pub use codex_skills::SkillDependencies;
|
||||
pub use codex_skills::SkillError;
|
||||
pub use codex_skills::SkillInterface;
|
||||
@@ -120,14 +119,6 @@ impl SkillLoadOutcome {
|
||||
.get(&skill.path_to_skills_md)
|
||||
}
|
||||
|
||||
/// Builds the legacy aggregate from independently loaded roots.
|
||||
///
|
||||
/// This is a temporary migration boundary while host root loading moves to the skills
|
||||
/// extension. It deliberately reuses the existing merge, precedence, and deduplication logic.
|
||||
pub fn from_root_snapshots(snapshots: Vec<crate::loader::SkillRootSnapshot>) -> Self {
|
||||
crate::root_loader::merge_skill_root_snapshots(snapshots)
|
||||
}
|
||||
|
||||
/// Reads one loaded skill through the filesystem that discovered it.
|
||||
pub async fn read_skill_text(&self, skill: &SkillMetadata) -> io::Result<String> {
|
||||
let fs = self
|
||||
@@ -181,16 +172,6 @@ impl SkillFileSystemsByPath {
|
||||
fn get(&self, path: &AbsolutePathBuf) -> Option<Arc<dyn ExecutorFileSystem>> {
|
||||
self.values.get(path).map(Arc::clone)
|
||||
}
|
||||
|
||||
fn retain_paths(&mut self, paths: &HashSet<AbsolutePathBuf>) {
|
||||
self.values = Arc::new(
|
||||
self.values
|
||||
.iter()
|
||||
.filter(|(path, _)| paths.contains(*path))
|
||||
.map(|(path, fs)| (path.clone(), Arc::clone(fs)))
|
||||
.collect(),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Debug for SkillFileSystemsByPath {
|
||||
@@ -200,61 +181,3 @@ impl fmt::Debug for SkillFileSystemsByPath {
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
pub fn filter_skill_load_outcome_for_product(
|
||||
mut outcome: SkillLoadOutcome,
|
||||
restriction_product: Option<Product>,
|
||||
) -> SkillLoadOutcome {
|
||||
outcome
|
||||
.skills
|
||||
.retain(|skill| skill.matches_product_restriction_for_product(restriction_product));
|
||||
let retained_paths: HashSet<AbsolutePathBuf> = outcome
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| skill.path_to_skills_md.clone())
|
||||
.collect();
|
||||
outcome
|
||||
.file_systems_by_skill_path
|
||||
.retain_paths(&retained_paths);
|
||||
outcome.skill_root_by_path = Arc::new(
|
||||
outcome
|
||||
.skill_root_by_path
|
||||
.iter()
|
||||
.filter(|(path, _)| retained_paths.contains(*path))
|
||||
.map(|(path, root)| (path.clone(), root.clone()))
|
||||
.collect(),
|
||||
);
|
||||
outcome.skill_discovery_path_by_path = Arc::new(
|
||||
outcome
|
||||
.skill_discovery_path_by_path
|
||||
.iter()
|
||||
.filter(|(path, _)| retained_paths.contains(*path))
|
||||
.map(|(path, discovery_path)| (path.clone(), discovery_path.clone()))
|
||||
.collect(),
|
||||
);
|
||||
outcome
|
||||
.agent_plugin_skill_paths
|
||||
.retain(|path| retained_paths.contains(path));
|
||||
let retained_roots: HashSet<AbsolutePathBuf> =
|
||||
outcome.skill_root_by_path.values().cloned().collect();
|
||||
outcome
|
||||
.skill_roots
|
||||
.retain(|root| retained_roots.contains(root));
|
||||
outcome.implicit_skills_by_scripts_dir = Arc::new(
|
||||
outcome
|
||||
.implicit_skills_by_scripts_dir
|
||||
.iter()
|
||||
.filter(|(_, skill)| skill.matches_product_restriction_for_product(restriction_product))
|
||||
.map(|(path, skill)| (path.clone(), skill.clone()))
|
||||
.collect(),
|
||||
);
|
||||
outcome.implicit_skills_by_doc_path = Arc::new(
|
||||
outcome
|
||||
.implicit_skills_by_doc_path
|
||||
.iter()
|
||||
.filter(|(_, skill)| skill.matches_product_restriction_for_product(restriction_product))
|
||||
.map(|(path, skill)| (path.clone(), skill.clone()))
|
||||
.collect(),
|
||||
);
|
||||
outcome
|
||||
}
|
||||
|
||||
@@ -1,221 +0,0 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::fmt;
|
||||
use std::hash::Hash;
|
||||
use std::hash::Hasher;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use futures::StreamExt;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use crate::SkillLoadOutcome;
|
||||
use crate::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use crate::loader::SkillRoot;
|
||||
use crate::loader::SkillRootSnapshot;
|
||||
use crate::loader::load_skill_root;
|
||||
use crate::model::SkillFileSystemsByPath;
|
||||
|
||||
/// Parsed plugin skill-root snapshots produced by one plugin load.
|
||||
///
|
||||
/// Clones share the same snapshots. The plugins manager stores them with the corresponding loaded
|
||||
/// plugins and passes a clone to skill loading as an optional preload.
|
||||
#[derive(Clone)]
|
||||
pub struct PluginSkillSnapshots {
|
||||
snapshots_by_root: Arc<Mutex<HashMap<PluginSkillRoot, SkillRootSnapshot>>>,
|
||||
}
|
||||
|
||||
impl PluginSkillSnapshots {
|
||||
/// Creates an empty snapshot collection for a plugin load to populate.
|
||||
pub fn for_plugin_load() -> Self {
|
||||
Self {
|
||||
snapshots_by_root: Arc::new(Mutex::new(HashMap::new())),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl PartialEq for PluginSkillSnapshots {
|
||||
fn eq(&self, other: &Self) -> bool {
|
||||
Arc::ptr_eq(&self.snapshots_by_root, &other.snapshots_by_root)
|
||||
}
|
||||
}
|
||||
|
||||
impl Eq for PluginSkillSnapshots {}
|
||||
|
||||
impl Hash for PluginSkillSnapshots {
|
||||
fn hash<H: Hasher>(&self, state: &mut H) {
|
||||
Arc::as_ptr(&self.snapshots_by_root).hash(state);
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Debug for PluginSkillSnapshots {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.debug_struct("PluginSkillSnapshots")
|
||||
.finish_non_exhaustive()
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn load_and_merge_skill_roots<I>(
|
||||
roots: I,
|
||||
plugin_skill_snapshots: Option<&PluginSkillSnapshots>,
|
||||
root_scan_slots: &Semaphore,
|
||||
) -> SkillLoadOutcome
|
||||
where
|
||||
I: IntoIterator<Item = SkillRoot>,
|
||||
{
|
||||
let mut indexed_root_snapshots = futures::stream::iter(roots.into_iter().enumerate())
|
||||
.map(|(root_index, root)| async move {
|
||||
// Bound root scans across all concurrent loads sharing this pool.
|
||||
let _root_scan_slot = root_scan_slots
|
||||
.acquire()
|
||||
.await
|
||||
.unwrap_or_else(|_| unreachable!());
|
||||
let snapshot = load_skill_root_snapshot(root, plugin_skill_snapshots).await;
|
||||
(root_index, snapshot)
|
||||
})
|
||||
// Keep each load's scan queue bounded while avoiding head-of-line blocking.
|
||||
.buffer_unordered(MAX_CONCURRENT_ROOT_SCANS)
|
||||
.collect::<Vec<_>>()
|
||||
.await;
|
||||
// Keep every scan slot productive, then restore root precedence for deterministic merging.
|
||||
indexed_root_snapshots.sort_unstable_by_key(|(root_index, _)| *root_index);
|
||||
let root_snapshots = indexed_root_snapshots
|
||||
.into_iter()
|
||||
.map(|(_, snapshot)| snapshot)
|
||||
.collect();
|
||||
|
||||
merge_skill_root_snapshots(root_snapshots)
|
||||
}
|
||||
|
||||
/// Loads one root while preserving the existing plugin-loader snapshot reuse.
|
||||
///
|
||||
/// This is a temporary migration boundary for callers that load non-plugin roots elsewhere but
|
||||
/// still need plugin roots to use the legacy preload cache.
|
||||
pub async fn load_skill_root_snapshot(
|
||||
root: SkillRoot,
|
||||
plugin_skill_snapshots: Option<&PluginSkillSnapshots>,
|
||||
) -> SkillRootSnapshot {
|
||||
let cache_key = match (
|
||||
root.plugin_identity.clone(),
|
||||
root.plugin_namespace.clone(),
|
||||
root.plugin_root.clone(),
|
||||
) {
|
||||
(Some(plugin_identity), Some(plugin_namespace), Some(plugin_root)) => {
|
||||
Some(PluginSkillRoot {
|
||||
path: root.path.clone(),
|
||||
plugin_identity,
|
||||
plugin_namespace,
|
||||
plugin_root,
|
||||
discovery_mode: root.discovery_mode,
|
||||
})
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
let cached_snapshot = cache_key.as_ref().and_then(|cache_key| {
|
||||
let plugin_skill_snapshots = plugin_skill_snapshots?;
|
||||
plugin_skill_snapshots
|
||||
.snapshots_by_root
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.get(cache_key)
|
||||
.cloned()
|
||||
});
|
||||
|
||||
match cached_snapshot {
|
||||
Some(snapshot) => snapshot,
|
||||
None => {
|
||||
let snapshot = load_skill_root(root).await;
|
||||
if let Some(plugin_skill_snapshots) = plugin_skill_snapshots
|
||||
&& let Some(cache_key) = cache_key
|
||||
{
|
||||
plugin_skill_snapshots
|
||||
.snapshots_by_root
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.insert(cache_key, snapshot.clone());
|
||||
}
|
||||
snapshot
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn merge_skill_root_snapshots(snapshots: Vec<SkillRootSnapshot>) -> SkillLoadOutcome {
|
||||
fn scope_rank(scope: codex_protocol::protocol::SkillScope) -> u8 {
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
|
||||
// Higher-priority scopes first (matches root scan order for dedupe).
|
||||
match scope {
|
||||
SkillScope::Repo => 0,
|
||||
SkillScope::User => 1,
|
||||
SkillScope::System => 2,
|
||||
SkillScope::Admin => 3,
|
||||
}
|
||||
}
|
||||
|
||||
let mut outcome = SkillLoadOutcome::default();
|
||||
let mut skill_roots = Vec::new();
|
||||
let mut skill_root_by_path = HashMap::new();
|
||||
let mut skill_discovery_path_by_path = HashMap::new();
|
||||
let mut file_systems_by_skill_path = HashMap::new();
|
||||
|
||||
for snapshot in snapshots {
|
||||
let SkillRootSnapshot {
|
||||
root,
|
||||
is_agent_plugin,
|
||||
skills,
|
||||
skill_discovery_path_by_path: discovery_paths,
|
||||
errors,
|
||||
file_system,
|
||||
} = snapshot;
|
||||
if !skills.is_empty() && !skill_roots.contains(&root) {
|
||||
skill_roots.push(root.clone());
|
||||
}
|
||||
for skill in &skills {
|
||||
let path = skill.path_to_skills_md.clone();
|
||||
if !skill_root_by_path.contains_key(&path) {
|
||||
skill_root_by_path.insert(path.clone(), root.clone());
|
||||
if let Some(discovery_path) = discovery_paths.get(&path) {
|
||||
skill_discovery_path_by_path.insert(path.clone(), discovery_path.clone());
|
||||
}
|
||||
file_systems_by_skill_path.insert(path.clone(), Arc::clone(&file_system));
|
||||
if is_agent_plugin {
|
||||
outcome.agent_plugin_skill_paths.insert(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
outcome.skills.extend(skills);
|
||||
outcome.errors.extend(errors);
|
||||
}
|
||||
|
||||
let mut seen = HashSet::new();
|
||||
outcome
|
||||
.skills
|
||||
.retain(|skill| seen.insert(skill.path_to_skills_md.clone()));
|
||||
let retained_skill_paths = outcome
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| skill.path_to_skills_md.clone())
|
||||
.collect::<HashSet<_>>();
|
||||
skill_root_by_path.retain(|path, _| retained_skill_paths.contains(path));
|
||||
skill_discovery_path_by_path.retain(|path, _| retained_skill_paths.contains(path));
|
||||
let used_roots = skill_root_by_path.values().cloned().collect::<HashSet<_>>();
|
||||
skill_roots.retain(|root| used_roots.contains(root));
|
||||
file_systems_by_skill_path.retain(|path, _| retained_skill_paths.contains(path));
|
||||
outcome
|
||||
.agent_plugin_skill_paths
|
||||
.retain(|path| retained_skill_paths.contains(path));
|
||||
outcome.skill_roots = skill_roots;
|
||||
outcome.skill_root_by_path = Arc::new(skill_root_by_path);
|
||||
outcome.skill_discovery_path_by_path = Arc::new(skill_discovery_path_by_path);
|
||||
outcome.file_systems_by_skill_path = SkillFileSystemsByPath::new(file_systems_by_skill_path);
|
||||
|
||||
outcome.skills.sort_by(|a, b| {
|
||||
scope_rank(a.scope)
|
||||
.cmp(&scope_rank(b.scope))
|
||||
.then_with(|| a.name.cmp(&b.name))
|
||||
.then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md))
|
||||
});
|
||||
|
||||
outcome
|
||||
}
|
||||
@@ -1,294 +0,0 @@
|
||||
#![cfg(unix)]
|
||||
|
||||
use std::fs;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
use codex_exec_server::CopyOptions;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::ExecutorFileSystemFuture;
|
||||
use codex_exec_server::FileMetadata;
|
||||
use codex_exec_server::FileSystemReadStream;
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_exec_server::ReadDirectoryEntry;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_exec_server::WalkOptions;
|
||||
use codex_exec_server::WalkOutcome;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
struct RecordingFileSystem<'a> {
|
||||
inner: &'a dyn ExecutorFileSystem,
|
||||
read_files: Mutex<Vec<PathUri>>,
|
||||
metadata_files: Mutex<Vec<PathUri>>,
|
||||
walks: AtomicUsize,
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
struct FileSystemCalls {
|
||||
walks: usize,
|
||||
read_files: Vec<PathUri>,
|
||||
metadata_files: Vec<PathUri>,
|
||||
}
|
||||
|
||||
impl<'a> RecordingFileSystem<'a> {
|
||||
fn new(inner: &'a dyn ExecutorFileSystem) -> Self {
|
||||
Self {
|
||||
inner,
|
||||
read_files: Mutex::new(Vec::new()),
|
||||
metadata_files: Mutex::new(Vec::new()),
|
||||
walks: AtomicUsize::new(0),
|
||||
}
|
||||
}
|
||||
|
||||
fn calls(&self) -> FileSystemCalls {
|
||||
let mut read_files = self
|
||||
.read_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone();
|
||||
read_files.sort_by_key(ToString::to_string);
|
||||
let mut metadata_files = self
|
||||
.metadata_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone();
|
||||
metadata_files.sort_by_key(ToString::to_string);
|
||||
FileSystemCalls {
|
||||
walks: self.walks.load(Ordering::Relaxed),
|
||||
read_files,
|
||||
metadata_files,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ExecutorFileSystem for RecordingFileSystem<'_> {
|
||||
fn canonicalize<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, PathUri> {
|
||||
self.inner.canonicalize(path, sandbox)
|
||||
}
|
||||
|
||||
fn read_file<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, Vec<u8>> {
|
||||
self.read_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push(path.clone());
|
||||
self.inner.read_file(path, sandbox)
|
||||
}
|
||||
|
||||
fn read_file_stream<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, FileSystemReadStream> {
|
||||
self.inner.read_file_stream(path, sandbox)
|
||||
}
|
||||
|
||||
fn write_file<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
contents: Vec<u8>,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner.write_file(path, contents, sandbox)
|
||||
}
|
||||
|
||||
fn create_directory<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
options: CreateDirectoryOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner.create_directory(path, options, sandbox)
|
||||
}
|
||||
|
||||
fn get_metadata<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, FileMetadata> {
|
||||
self.metadata_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push(path.clone());
|
||||
self.inner.get_metadata(path, sandbox)
|
||||
}
|
||||
|
||||
fn read_directory<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, Vec<ReadDirectoryEntry>> {
|
||||
self.inner.read_directory(path, sandbox)
|
||||
}
|
||||
|
||||
fn walk<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
options: WalkOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, WalkOutcome> {
|
||||
self.walks.fetch_add(1, Ordering::Relaxed);
|
||||
self.inner.walk(path, options, sandbox)
|
||||
}
|
||||
|
||||
fn remove<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
options: RemoveOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner.remove(path, options, sandbox)
|
||||
}
|
||||
|
||||
fn copy<'a>(
|
||||
&'a self,
|
||||
source_path: &'a PathUri,
|
||||
destination_path: &'a PathUri,
|
||||
options: CopyOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner
|
||||
.copy(source_path, destination_path, options, sandbox)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
|
||||
use std::os::unix::fs::symlink;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core_skills::SkillMetadata;
|
||||
use codex_core_skills::SkillPolicy;
|
||||
use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use codex_core_skills::loader::SkillRoot;
|
||||
use codex_core_skills::loader::load_skills_from_roots;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
|
||||
let root = tempdir().expect("tempdir");
|
||||
let shared_plugin_root = tempdir().expect("tempdir");
|
||||
let manifest_path = shared_plugin_root.path().join(".codex-plugin/plugin.json");
|
||||
fs::create_dir_all(manifest_path.parent().expect("manifest parent")).expect("manifest dir");
|
||||
fs::write(&manifest_path, r#"{"name":"linked"}"#).expect("manifest");
|
||||
|
||||
let skills_root = shared_plugin_root.path().join("skills");
|
||||
for name in ["first", "second"] {
|
||||
let skill_path = skills_root.join(name).join("SKILL.md");
|
||||
fs::create_dir_all(skill_path.parent().expect("skill parent")).expect("skill dir");
|
||||
fs::write(
|
||||
&skill_path,
|
||||
format!("---\nname: {name}\ndescription: {name} skill.\n---\n"),
|
||||
)
|
||||
.expect("skill");
|
||||
}
|
||||
let metadata_path = skills_root.join("first/agents/openai.yaml");
|
||||
fs::create_dir_all(metadata_path.parent().expect("metadata parent")).expect("metadata dir");
|
||||
fs::write(
|
||||
&metadata_path,
|
||||
"policy:\n allow_implicit_invocation: false\n",
|
||||
)
|
||||
.expect("metadata");
|
||||
|
||||
let host_root = root.path().join("skills");
|
||||
fs::create_dir_all(&host_root).expect("host skills dir");
|
||||
let linked_root = host_root.join("linked-plugin");
|
||||
symlink(&skills_root, &linked_root).expect("skill pack symlink");
|
||||
|
||||
let recording = Arc::new(RecordingFileSystem::new(LOCAL_FS.as_ref()));
|
||||
let file_system: Arc<dyn ExecutorFileSystem> = recording.clone();
|
||||
let future = load_skills_from_roots(
|
||||
[SkillRoot {
|
||||
path: host_root.abs(),
|
||||
scope: SkillScope::User,
|
||||
file_system,
|
||||
plugin_identity: None,
|
||||
plugin_namespace: None,
|
||||
plugin_root: None,
|
||||
discovery_mode: codex_utils_plugins::SkillDiscoveryMode::Recursive,
|
||||
}],
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(tokio::sync::Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)),
|
||||
);
|
||||
fn assert_send<T: Send>(_: &T) {}
|
||||
assert_send(&future);
|
||||
let outcome = future.await;
|
||||
|
||||
assert_eq!(outcome.errors, Vec::new());
|
||||
let first_skill_path = dunce::canonicalize(skills_root.join("first/SKILL.md"))
|
||||
.unwrap()
|
||||
.abs();
|
||||
let second_skill_path = dunce::canonicalize(skills_root.join("second/SKILL.md"))
|
||||
.unwrap()
|
||||
.abs();
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![
|
||||
SkillMetadata {
|
||||
name: "linked:first".to_string(),
|
||||
description: "first skill.".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: Some(SkillPolicy {
|
||||
allow_implicit_invocation: Some(false),
|
||||
products: Vec::new(),
|
||||
}),
|
||||
path_to_skills_md: first_skill_path,
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
},
|
||||
SkillMetadata {
|
||||
name: "linked:second".to_string(),
|
||||
description: "second skill.".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: second_skill_path,
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
},
|
||||
]
|
||||
);
|
||||
|
||||
let calls = recording.calls();
|
||||
assert_eq!(calls.walks, 1);
|
||||
let linked_root = PathUri::from_host_native_path(linked_root).unwrap();
|
||||
assert!(
|
||||
calls
|
||||
.read_files
|
||||
.iter()
|
||||
.all(|path| !path.starts_with(&linked_root))
|
||||
);
|
||||
assert!(
|
||||
calls
|
||||
.metadata_files
|
||||
.iter()
|
||||
.all(|path| path.basename().as_deref() != Some("openai.yaml"))
|
||||
);
|
||||
let manifest_uri =
|
||||
PathUri::from_host_native_path(dunce::canonicalize(manifest_path).unwrap()).unwrap();
|
||||
assert_eq!(
|
||||
calls
|
||||
.metadata_files
|
||||
.iter()
|
||||
.filter(|path| **path == manifest_uri)
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
}
|
||||
@@ -18,11 +18,9 @@ pub use codex_core_skills::SkillLoadOutcome;
|
||||
pub use codex_core_skills::build_skill_name_counts;
|
||||
pub use codex_core_skills::config_rules;
|
||||
pub use codex_core_skills::detect_implicit_skill_invocation_for_command;
|
||||
pub use codex_core_skills::filter_skill_load_outcome_for_product;
|
||||
pub use codex_core_skills::injection;
|
||||
pub use codex_core_skills::injection::SkillInjections;
|
||||
pub use codex_core_skills::injection::build_skill_injections;
|
||||
pub use codex_core_skills::loader;
|
||||
pub use codex_core_skills::model;
|
||||
pub use codex_core_skills::remote;
|
||||
pub use codex_skills::SkillMetadata;
|
||||
|
||||
@@ -7,7 +7,6 @@ use codex_config::ConfigLayerStack;
|
||||
use codex_config::default_project_root_markers;
|
||||
use codex_config::merge_toml_values;
|
||||
use codex_config::project_root_markers_from_config;
|
||||
use codex_core_skills::loader::SkillRoot;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
@@ -15,7 +14,6 @@ use codex_skills::system_cache_root_dir;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
use dirs::home_dir;
|
||||
use futures::StreamExt;
|
||||
use toml::Value as TomlValue;
|
||||
@@ -32,7 +30,7 @@ pub(crate) async fn resolve_skill_roots(
|
||||
cwd: &AbsolutePathBuf,
|
||||
plugin_skill_roots: Vec<PluginSkillRoot>,
|
||||
extra_skill_roots: Vec<AbsolutePathBuf>,
|
||||
) -> Vec<SkillRoot> {
|
||||
) -> Vec<HostSkillRoot> {
|
||||
let home_dir =
|
||||
home_dir().and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok());
|
||||
resolve_skill_roots_with_home_dir(
|
||||
@@ -53,33 +51,20 @@ async fn resolve_skill_roots_with_home_dir(
|
||||
home_dir: Option<&AbsolutePathBuf>,
|
||||
plugin_skill_roots: Vec<PluginSkillRoot>,
|
||||
extra_skill_roots: Vec<AbsolutePathBuf>,
|
||||
) -> Vec<SkillRoot> {
|
||||
) -> Vec<HostSkillRoot> {
|
||||
let mut roots =
|
||||
roots_from_layer_stack(config_layer_stack, home_dir, repository_file_system.clone())
|
||||
roots_from_layer_stack(config_layer_stack, home_dir, repository_file_system.clone());
|
||||
roots.extend(
|
||||
plugin_skill_roots
|
||||
.into_iter()
|
||||
.map(host_root_to_skill_root)
|
||||
.collect::<Vec<_>>();
|
||||
roots.extend(plugin_skill_roots.into_iter().map(|root| SkillRoot {
|
||||
path: root.path,
|
||||
scope: SkillScope::User,
|
||||
file_system: Arc::clone(&LOCAL_FS),
|
||||
plugin_identity: Some(root.plugin_identity),
|
||||
plugin_namespace: Some(root.plugin_namespace),
|
||||
plugin_root: Some(root.plugin_root),
|
||||
discovery_mode: root.discovery_mode,
|
||||
}));
|
||||
.map(|root| HostSkillRoot::plugin(root, Arc::clone(&LOCAL_FS))),
|
||||
);
|
||||
roots.extend(
|
||||
extra_skill_roots
|
||||
.into_iter()
|
||||
.map(|path| local_root(path, SkillScope::User))
|
||||
.map(host_root_to_skill_root),
|
||||
);
|
||||
roots.extend(
|
||||
repo_agents_skill_roots(repository_file_system, config_layer_stack, cwd)
|
||||
.await
|
||||
.into_iter()
|
||||
.map(host_root_to_skill_root),
|
||||
.map(|path| local_root(path, SkillScope::User)),
|
||||
);
|
||||
roots.extend(repo_agents_skill_roots(repository_file_system, config_layer_stack, cwd).await);
|
||||
dedupe_skill_roots_by_path(&mut roots);
|
||||
roots
|
||||
}
|
||||
@@ -147,18 +132,6 @@ fn local_root(path: AbsolutePathBuf, scope: SkillScope) -> HostSkillRoot {
|
||||
HostSkillRoot::host(path, scope, Arc::clone(&LOCAL_FS))
|
||||
}
|
||||
|
||||
fn host_root_to_skill_root(root: HostSkillRoot) -> SkillRoot {
|
||||
SkillRoot {
|
||||
path: root.path,
|
||||
scope: root.scope,
|
||||
file_system: root.file_system,
|
||||
plugin_identity: None,
|
||||
plugin_namespace: None,
|
||||
plugin_root: None,
|
||||
discovery_mode: SkillDiscoveryMode::Recursive,
|
||||
}
|
||||
}
|
||||
|
||||
async fn repo_agents_skill_roots(
|
||||
repository_file_system: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
@@ -285,7 +258,7 @@ fn dirs_between_project_root_and_cwd(
|
||||
directories
|
||||
}
|
||||
|
||||
fn dedupe_skill_roots_by_path(roots: &mut Vec<SkillRoot>) {
|
||||
fn dedupe_skill_roots_by_path(roots: &mut Vec<HostSkillRoot>) {
|
||||
let mut seen = HashSet::new();
|
||||
roots.retain(|root| seen.insert(root.path.clone()));
|
||||
}
|
||||
|
||||
@@ -8,8 +8,6 @@ use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_core_skills::SkillMetadata;
|
||||
use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use codex_core_skills::loader::load_skills_from_roots;
|
||||
use codex_exec_server::CopyOptions;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
@@ -36,6 +34,8 @@ use tokio::sync::Semaphore;
|
||||
use super::repo_agents_skill_roots;
|
||||
use super::resolve_skill_roots_with_home_dir;
|
||||
use super::roots_from_layer_stack;
|
||||
use crate::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use crate::loader::load_and_merge_host_skill_roots;
|
||||
|
||||
struct BlockingMetadataFileSystem {
|
||||
inner: Arc<dyn ExecutorFileSystem>,
|
||||
@@ -317,10 +317,10 @@ async fn plugin_roots_preserve_plugin_resolution_metadata() {
|
||||
(
|
||||
root.path.clone(),
|
||||
root.scope,
|
||||
root.plugin_identity.clone(),
|
||||
root.plugin_namespace.clone(),
|
||||
root.plugin_root.clone(),
|
||||
root.discovery_mode,
|
||||
root.plugin_identity().cloned(),
|
||||
root.plugin_namespace().map(str::to_string),
|
||||
root.plugin_root().cloned(),
|
||||
root.discovery_mode(),
|
||||
),
|
||||
(
|
||||
skills_root,
|
||||
@@ -357,10 +357,10 @@ async fn unique_extra_root_loads_as_recursive_user_root() {
|
||||
(
|
||||
root.path.clone(),
|
||||
root.scope,
|
||||
root.plugin_identity.clone(),
|
||||
root.plugin_namespace.clone(),
|
||||
root.plugin_root.clone(),
|
||||
root.discovery_mode,
|
||||
root.plugin_identity().cloned(),
|
||||
root.plugin_namespace().map(str::to_string),
|
||||
root.plugin_root().cloned(),
|
||||
root.discovery_mode(),
|
||||
),
|
||||
(
|
||||
extra_root,
|
||||
@@ -373,10 +373,11 @@ async fn unique_extra_root_loads_as_recursive_user_root() {
|
||||
);
|
||||
assert!(Arc::ptr_eq(&root.file_system, &LOCAL_FS));
|
||||
|
||||
let outcome = load_skills_from_roots(
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
roots,
|
||||
&Semaphore::new(MAX_CONCURRENT_ROOT_SCANS),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -455,10 +456,11 @@ async fn resolved_project_layer_loads_skill_without_git_marker() {
|
||||
Vec::new(),
|
||||
)
|
||||
.await;
|
||||
let outcome = load_skills_from_roots(
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
roots,
|
||||
&Semaphore::new(MAX_CONCURRENT_ROOT_SCANS),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -491,10 +493,11 @@ async fn resolved_project_layer_loads_skill_when_cwd_is_file() {
|
||||
Vec::new(),
|
||||
)
|
||||
.await;
|
||||
let outcome = load_skills_from_roots(
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
roots,
|
||||
&Semaphore::new(MAX_CONCURRENT_ROOT_SCANS),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -645,10 +648,11 @@ async fn resolved_config_and_repo_roots_preserve_order_and_dedupe_paths_not_name
|
||||
)
|
||||
.await;
|
||||
assert_eq!(roots.len(), 8);
|
||||
let outcome = load_skills_from_roots(
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
roots,
|
||||
&Semaphore::new(MAX_CONCURRENT_ROOT_SCANS),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)),
|
||||
)
|
||||
.await;
|
||||
assert!(outcome.errors.is_empty());
|
||||
|
||||
@@ -24,8 +24,6 @@ use codex_core_skills::SkillLoadOutcome;
|
||||
use codex_core_skills::config_rules::SkillConfigRules;
|
||||
use codex_core_skills::config_rules::resolve_disabled_skill_paths;
|
||||
use codex_core_skills::config_rules::skill_config_rules_from_stack;
|
||||
use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use codex_core_skills::loader::SkillRoot;
|
||||
use codex_skills::LoadedSkills;
|
||||
use codex_skills::SkillLoadFuture;
|
||||
use codex_skills::SkillRootLoadRequest;
|
||||
@@ -36,6 +34,7 @@ use codex_skills::install_system_skills;
|
||||
use crate::HostSkillsSnapshot;
|
||||
use crate::host_roots::resolve_skill_roots;
|
||||
use crate::loader::HostSkillRoot;
|
||||
use crate::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use crate::loader::load_and_merge_host_skill_roots;
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
@@ -165,7 +164,7 @@ impl HostSkillsService {
|
||||
&self,
|
||||
input: &HostSkillsLoadInput,
|
||||
fs: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
) -> Vec<SkillRoot> {
|
||||
) -> Vec<HostSkillRoot> {
|
||||
if input.bundled_skills_enabled {
|
||||
self.ensure_system_skills_installed();
|
||||
}
|
||||
@@ -247,7 +246,7 @@ impl HostSkillsService {
|
||||
async fn snapshot_for_skill_roots(
|
||||
&self,
|
||||
input: &HostSkillsLoadInput,
|
||||
roots: Vec<SkillRoot>,
|
||||
roots: Vec<HostSkillRoot>,
|
||||
skill_config_rules: &SkillConfigRules,
|
||||
cache_key: ConfigSkillsCacheKey,
|
||||
force_reload: bool,
|
||||
@@ -285,33 +284,9 @@ impl HostSkillsService {
|
||||
async fn build_skill_outcome(
|
||||
&self,
|
||||
input: &HostSkillsLoadInput,
|
||||
roots: Vec<SkillRoot>,
|
||||
roots: Vec<HostSkillRoot>,
|
||||
skill_config_rules: &SkillConfigRules,
|
||||
) -> SkillLoadOutcome {
|
||||
let roots = roots
|
||||
.into_iter()
|
||||
.map(|root| {
|
||||
match (
|
||||
root.plugin_identity,
|
||||
root.plugin_namespace,
|
||||
root.plugin_root,
|
||||
) {
|
||||
(Some(plugin_identity), Some(plugin_namespace), Some(plugin_root)) => {
|
||||
HostSkillRoot::plugin(
|
||||
PluginSkillRoot {
|
||||
path: root.path,
|
||||
plugin_identity,
|
||||
plugin_namespace,
|
||||
plugin_root,
|
||||
discovery_mode: root.discovery_mode,
|
||||
},
|
||||
root.file_system,
|
||||
)
|
||||
}
|
||||
_ => HostSkillRoot::host(root.path, root.scope, root.file_system),
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
roots,
|
||||
&self.root_scan_slots,
|
||||
@@ -466,7 +441,7 @@ pub fn bundled_skills_enabled_from_stack(
|
||||
}
|
||||
|
||||
fn config_skills_cache_key(
|
||||
roots: &[SkillRoot],
|
||||
roots: &[HostSkillRoot],
|
||||
skill_config_rules: &SkillConfigRules,
|
||||
plugin_skill_snapshots: Option<&SkillRootSnapshots<PluginSkillRoot>>,
|
||||
) -> ConfigSkillsCacheKey {
|
||||
@@ -483,15 +458,15 @@ fn config_skills_cache_key(
|
||||
ConfigSkillRootCacheKey {
|
||||
path: root.path.clone(),
|
||||
scope_rank,
|
||||
plugin_identity: root.plugin_identity.clone(),
|
||||
plugin_namespace: root.plugin_namespace.clone(),
|
||||
plugin_identity: root.plugin_identity().cloned(),
|
||||
plugin_namespace: root.plugin_namespace().map(str::to_string),
|
||||
file_system: FileSystemIdentity(Arc::downgrade(&root.file_system)),
|
||||
}
|
||||
})
|
||||
.collect(),
|
||||
skill_config_rules: skill_config_rules.clone(),
|
||||
plugin_skill_snapshots: plugin_skill_snapshots
|
||||
.filter(|_| roots.iter().any(|root| root.plugin_identity.is_some()))
|
||||
.filter(|_| roots.iter().any(|root| root.plugin_identity().is_some()))
|
||||
.cloned(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -28,6 +28,7 @@ pub use host_service::HostSkillsLoadInput;
|
||||
pub use host_service::HostSkillsService;
|
||||
pub use host_service::bundled_skills_enabled_from_stack;
|
||||
pub use host_snapshot::HostSkillsSnapshot;
|
||||
pub use loader::HostSkillRoot;
|
||||
pub use provider::ExecutorSkillProvider;
|
||||
pub use provider::HostSkillProvider;
|
||||
pub use provider::OrchestratorSkillProvider;
|
||||
|
||||
@@ -218,3 +218,7 @@ fn discover_skill_metadata(
|
||||
SkillMetadataDiscovery::Probe(metadata_path)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "discovery_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
use super::DirectorySymlinkPolicy;
|
||||
use super::HiddenDirectoryPolicy;
|
||||
use super::SkillDiscovery;
|
||||
use super::SkillDiscoveryOptions;
|
||||
use super::discover_skills;
|
||||
|
||||
fn write_skill(root: &Path, directory: &str) {
|
||||
let path = root.join(directory).join("SKILL.md");
|
||||
fs::create_dir_all(path.parent().expect("skill parent")).expect("create skill directory");
|
||||
fs::write(path, "---\nname: demo\ndescription: Demo skill\n---\n")
|
||||
.expect("write skill frontmatter");
|
||||
}
|
||||
|
||||
fn canonical_uri(path: &Path) -> PathUri {
|
||||
PathUri::from_host_native_path(fs::canonicalize(path).expect("canonical path"))
|
||||
.expect("absolute path URI")
|
||||
}
|
||||
|
||||
async fn discover(root: &Path) -> SkillDiscovery {
|
||||
discover_skills(
|
||||
LOCAL_FS.as_ref(),
|
||||
&canonical_uri(root),
|
||||
SkillDiscoveryOptions {
|
||||
directory_symlinks: DirectorySymlinkPolicy::Follow,
|
||||
hidden_directories: HiddenDirectoryPolicy::Skip,
|
||||
mode: SkillDiscoveryMode::Recursive,
|
||||
},
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn discovers_hidden_directory_through_visible_symlink() {
|
||||
let root = TempDir::new().expect("root temp dir");
|
||||
write_skill(root.path(), ".hidden/search");
|
||||
std::os::unix::fs::symlink(root.path().join(".hidden"), root.path().join("visible"))
|
||||
.expect("create visible skill directory symlink");
|
||||
|
||||
let discovery = discover(root.path()).await;
|
||||
|
||||
assert_eq!(discovery.warnings, Vec::<String>::new());
|
||||
assert_eq!(
|
||||
discovery
|
||||
.skills
|
||||
.into_iter()
|
||||
.map(|skill| skill.path)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
canonical_uri(root.path())
|
||||
.join("visible/search/SKILL.md")
|
||||
.expect("visible skill URI"),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn ignores_symlinked_skill_files_and_directory_cycles() {
|
||||
let root = TempDir::new().expect("root temp dir");
|
||||
let external = TempDir::new().expect("external temp dir");
|
||||
write_skill(external.path(), "external");
|
||||
write_skill(root.path(), "cycle/demo");
|
||||
fs::create_dir_all(root.path().join("linked-file")).expect("create linked skill directory");
|
||||
std::os::unix::fs::symlink(
|
||||
external.path().join("external/SKILL.md"),
|
||||
root.path().join("linked-file/SKILL.md"),
|
||||
)
|
||||
.expect("create skill file symlink");
|
||||
std::os::unix::fs::symlink(root.path().join("cycle"), root.path().join("cycle/loop"))
|
||||
.expect("create directory symlink cycle");
|
||||
|
||||
let discovery = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(/*secs*/ 5),
|
||||
discover(root.path()),
|
||||
)
|
||||
.await
|
||||
.expect("directory symlink cycles must not prevent discovery from completing");
|
||||
|
||||
assert_eq!(
|
||||
discovery
|
||||
.skills
|
||||
.into_iter()
|
||||
.map(|skill| skill.path)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
canonical_uri(root.path())
|
||||
.join("cycle/demo/SKILL.md")
|
||||
.expect("valid skill URI"),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn respects_maximum_skill_scan_depth() {
|
||||
let root = TempDir::new().expect("root temp dir");
|
||||
let within_depth = "d0/d1/d2/d3/d4/d5";
|
||||
let beyond_depth = "d0/d1/d2/d3/d4/d5/d6";
|
||||
write_skill(root.path(), within_depth);
|
||||
write_skill(root.path(), beyond_depth);
|
||||
|
||||
let discovery = discover(root.path()).await;
|
||||
|
||||
assert_eq!(
|
||||
discovery
|
||||
.skills
|
||||
.into_iter()
|
||||
.map(|skill| skill.path)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
canonical_uri(root.path())
|
||||
.join(&format!("{within_depth}/SKILL.md"))
|
||||
.expect("reachable skill URI"),
|
||||
]
|
||||
);
|
||||
}
|
||||
@@ -1,208 +1,18 @@
|
||||
use std::fs;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::Duration;
|
||||
|
||||
use codex_exec_server::CopyOptions;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::ExecutorFileSystemFuture;
|
||||
use codex_exec_server::FileMetadata;
|
||||
use codex_exec_server::FileSystemReadStream;
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_exec_server::ReadDirectoryEntry;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_exec_server::WalkOptions;
|
||||
use codex_exec_server::WalkOutcome;
|
||||
use codex_skills::EnvironmentSkillMetadata;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
use tokio::sync::Notify;
|
||||
|
||||
use crate::loader::io_test_support::FileSystemCalls;
|
||||
use crate::loader::io_test_support::ManifestMetadataBehavior;
|
||||
use crate::loader::io_test_support::RecordingFileSystem;
|
||||
|
||||
use super::load_environment_skills_from_root;
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum ManifestMetadataBehavior {
|
||||
Immediate,
|
||||
WaitForSkillRead,
|
||||
}
|
||||
|
||||
struct RecordingFileSystem<'a> {
|
||||
inner: &'a dyn ExecutorFileSystem,
|
||||
read_files: Mutex<Vec<PathUri>>,
|
||||
metadata_files: Mutex<Vec<PathUri>>,
|
||||
walks: AtomicUsize,
|
||||
manifest_metadata_behavior: ManifestMetadataBehavior,
|
||||
skill_read_started: AtomicBool,
|
||||
skill_read_started_notify: Notify,
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
struct FileSystemCalls {
|
||||
walks: usize,
|
||||
read_files: Vec<PathUri>,
|
||||
metadata_files: Vec<PathUri>,
|
||||
}
|
||||
|
||||
impl<'a> RecordingFileSystem<'a> {
|
||||
fn new(
|
||||
inner: &'a dyn ExecutorFileSystem,
|
||||
manifest_metadata_behavior: ManifestMetadataBehavior,
|
||||
) -> Self {
|
||||
Self {
|
||||
inner,
|
||||
read_files: Mutex::new(Vec::new()),
|
||||
metadata_files: Mutex::new(Vec::new()),
|
||||
walks: AtomicUsize::new(0),
|
||||
manifest_metadata_behavior,
|
||||
skill_read_started: AtomicBool::new(false),
|
||||
skill_read_started_notify: Notify::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn calls(&self) -> FileSystemCalls {
|
||||
let mut read_files = self
|
||||
.read_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone();
|
||||
read_files.sort_by_key(ToString::to_string);
|
||||
let mut metadata_files = self
|
||||
.metadata_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone();
|
||||
metadata_files.sort_by_key(ToString::to_string);
|
||||
FileSystemCalls {
|
||||
walks: self.walks.load(Ordering::Relaxed),
|
||||
read_files,
|
||||
metadata_files,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ExecutorFileSystem for RecordingFileSystem<'_> {
|
||||
fn canonicalize<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, PathUri> {
|
||||
self.inner.canonicalize(path, sandbox)
|
||||
}
|
||||
|
||||
fn read_file<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, Vec<u8>> {
|
||||
self.read_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push(path.clone());
|
||||
if path.basename().as_deref() == Some("SKILL.md") {
|
||||
self.skill_read_started.store(true, Ordering::Release);
|
||||
self.skill_read_started_notify.notify_waiters();
|
||||
}
|
||||
self.inner.read_file(path, sandbox)
|
||||
}
|
||||
|
||||
fn read_file_stream<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, FileSystemReadStream> {
|
||||
self.inner.read_file_stream(path, sandbox)
|
||||
}
|
||||
|
||||
fn write_file<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
contents: Vec<u8>,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner.write_file(path, contents, sandbox)
|
||||
}
|
||||
|
||||
fn create_directory<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
options: CreateDirectoryOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner.create_directory(path, options, sandbox)
|
||||
}
|
||||
|
||||
fn get_metadata<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, FileMetadata> {
|
||||
self.metadata_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push(path.clone());
|
||||
if matches!(
|
||||
self.manifest_metadata_behavior,
|
||||
ManifestMetadataBehavior::WaitForSkillRead
|
||||
) && path.basename().as_deref() == Some("plugin.json")
|
||||
{
|
||||
return Box::pin(async move {
|
||||
loop {
|
||||
let notified = self.skill_read_started_notify.notified();
|
||||
if self.skill_read_started.load(Ordering::Acquire) {
|
||||
break;
|
||||
}
|
||||
notified.await;
|
||||
}
|
||||
self.inner.get_metadata(path, sandbox).await
|
||||
});
|
||||
}
|
||||
self.inner.get_metadata(path, sandbox)
|
||||
}
|
||||
|
||||
fn read_directory<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, Vec<ReadDirectoryEntry>> {
|
||||
self.inner.read_directory(path, sandbox)
|
||||
}
|
||||
|
||||
fn walk<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
options: WalkOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, WalkOutcome> {
|
||||
self.walks.fetch_add(1, Ordering::Relaxed);
|
||||
self.inner.walk(path, options, sandbox)
|
||||
}
|
||||
|
||||
fn remove<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
options: RemoveOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner.remove(path, options, sandbox)
|
||||
}
|
||||
|
||||
fn copy<'a>(
|
||||
&'a self,
|
||||
source_path: &'a PathUri,
|
||||
destination_path: &'a PathUri,
|
||||
options: CopyOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner
|
||||
.copy(source_path, destination_path, options, sandbox)
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_nearest_plugin_namespaces_without_reading_unused_sibling_manifests() {
|
||||
let root = tempdir().expect("tempdir");
|
||||
|
||||
@@ -1,18 +1,92 @@
|
||||
use std::fs;
|
||||
|
||||
use codex_core_skills::loader::load_environment_skills_from_root;
|
||||
use codex_exec_server::CapabilityRootDiscoverRequest;
|
||||
use codex_exec_server::CapabilityRootsDiscoverParams;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_exec_server::discover_capability_roots;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_skills::EnvironmentSkillMetadata;
|
||||
use codex_skills::SkillDependencies;
|
||||
use codex_skills::SkillPolicy;
|
||||
use codex_skills::SkillToolDependency;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
use super::load_environment_skills_from_discovery;
|
||||
use super::load_environment_skills_from_root;
|
||||
|
||||
#[tokio::test]
|
||||
async fn executor_bundle_parser_matches_the_existing_environment_loader() {
|
||||
async fn direct_environment_loader_preserves_plugin_dependencies_and_product_policy() {
|
||||
let root = tempdir().expect("tempdir");
|
||||
let skill_dir = root.path().join("skills/deploy");
|
||||
fs::create_dir_all(root.path().join(".codex-plugin")).expect("manifest dir");
|
||||
fs::create_dir_all(skill_dir.join("agents")).expect("metadata dir");
|
||||
fs::write(
|
||||
root.path().join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"demo-plugin"}"#,
|
||||
)
|
||||
.expect("manifest");
|
||||
fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
"---\nname: deploy\ndescription: Deploy the service.\n---\n",
|
||||
)
|
||||
.expect("skill");
|
||||
fs::write(
|
||||
skill_dir.join("agents/openai.yaml"),
|
||||
r#"
|
||||
dependencies:
|
||||
tools:
|
||||
- type: mcp
|
||||
value: deploy-server
|
||||
description: Deploy MCP
|
||||
transport: stdio
|
||||
command: deploy-mcp
|
||||
policy:
|
||||
allow_implicit_invocation: false
|
||||
products: [codex, atlas]
|
||||
"#,
|
||||
)
|
||||
.expect("metadata");
|
||||
|
||||
let root_uri = PathUri::from_host_native_path(root.path()).expect("root URI");
|
||||
let outcome =
|
||||
load_environment_skills_from_root(LOCAL_FS.as_ref(), &root_uri, Some(Product::Codex)).await;
|
||||
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![EnvironmentSkillMetadata {
|
||||
path_to_skills_md: PathUri::from_host_native_path(skill_dir.join("SKILL.md")).unwrap(),
|
||||
name: "demo-plugin:deploy".to_string(),
|
||||
description: "Deploy the service.".to_string(),
|
||||
short_description: None,
|
||||
dependencies: Some(SkillDependencies {
|
||||
tools: vec![SkillToolDependency {
|
||||
r#type: "mcp".to_string(),
|
||||
value: "deploy-server".to_string(),
|
||||
description: Some("Deploy MCP".to_string()),
|
||||
transport: Some("stdio".to_string()),
|
||||
command: Some("deploy-mcp".to_string()),
|
||||
url: None,
|
||||
}],
|
||||
}),
|
||||
policy: Some(SkillPolicy {
|
||||
allow_implicit_invocation: Some(false),
|
||||
products: vec![Product::Codex, Product::Atlas],
|
||||
}),
|
||||
}]
|
||||
);
|
||||
let atlas =
|
||||
load_environment_skills_from_root(LOCAL_FS.as_ref(), &root_uri, Some(Product::Atlas)).await;
|
||||
assert_eq!(atlas.skills, outcome.skills);
|
||||
let filtered =
|
||||
load_environment_skills_from_root(LOCAL_FS.as_ref(), &root_uri, Some(Product::Chatgpt))
|
||||
.await;
|
||||
assert!(filtered.skills.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn executor_bundle_parser_matches_direct_environment_loader() {
|
||||
let root = tempdir().expect("tempdir");
|
||||
let plugin_manifest = root.path().join(".codex-plugin/plugin.json");
|
||||
let nested_manifest = root.path().join("nested/.claude-plugin/plugin.json");
|
||||
|
||||
@@ -75,7 +75,8 @@ impl HostSkillRoot {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn plugin_identity(&self) -> Option<&PluginIdentity> {
|
||||
/// Returns the owning plugin identity when this root belongs to a plugin.
|
||||
pub fn plugin_identity(&self) -> Option<&PluginIdentity> {
|
||||
self.plugin.as_ref().map(|plugin| &plugin.identity)
|
||||
}
|
||||
|
||||
@@ -404,3 +405,7 @@ async fn canonicalize_for_skill_identity(
|
||||
#[cfg(test)]
|
||||
#[path = "host_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
#[cfg(all(test, unix))]
|
||||
#[path = "host_io_tests.rs"]
|
||||
mod io_tests;
|
||||
|
||||
@@ -0,0 +1,138 @@
|
||||
use std::fs;
|
||||
use std::os::unix::fs::symlink;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_skills::SkillMetadata;
|
||||
use codex_skills::SkillPolicy;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use crate::loader::HostSkillRoot;
|
||||
use crate::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use crate::loader::io_test_support::ManifestMetadataBehavior;
|
||||
use crate::loader::io_test_support::RecordingFileSystem;
|
||||
use crate::loader::load_and_merge_host_skill_roots;
|
||||
|
||||
#[tokio::test]
|
||||
async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
|
||||
let root = tempdir().expect("tempdir");
|
||||
let shared_plugin_root = tempdir().expect("tempdir");
|
||||
let manifest_path = shared_plugin_root.path().join(".codex-plugin/plugin.json");
|
||||
fs::create_dir_all(manifest_path.parent().expect("manifest parent")).expect("manifest dir");
|
||||
fs::write(&manifest_path, r#"{"name":"linked"}"#).expect("manifest");
|
||||
|
||||
let skills_root = shared_plugin_root.path().join("skills");
|
||||
for name in ["first", "second"] {
|
||||
let skill_path = skills_root.join(name).join("SKILL.md");
|
||||
fs::create_dir_all(skill_path.parent().expect("skill parent")).expect("skill dir");
|
||||
fs::write(
|
||||
&skill_path,
|
||||
format!("---\nname: {name}\ndescription: {name} skill.\n---\n"),
|
||||
)
|
||||
.expect("skill");
|
||||
}
|
||||
let metadata_path = skills_root.join("first/agents/openai.yaml");
|
||||
fs::create_dir_all(metadata_path.parent().expect("metadata parent")).expect("metadata dir");
|
||||
fs::write(
|
||||
&metadata_path,
|
||||
"policy:\n allow_implicit_invocation: false\n",
|
||||
)
|
||||
.expect("metadata");
|
||||
|
||||
let host_root = root.path().join("skills");
|
||||
fs::create_dir_all(&host_root).expect("host skills dir");
|
||||
let linked_root = host_root.join("linked-plugin");
|
||||
symlink(&skills_root, &linked_root).expect("skill pack symlink");
|
||||
|
||||
let recording = Arc::new(RecordingFileSystem::new(
|
||||
LOCAL_FS.as_ref(),
|
||||
ManifestMetadataBehavior::Immediate,
|
||||
));
|
||||
let file_system: Arc<dyn ExecutorFileSystem> = recording.clone();
|
||||
let root_scan_slots = Semaphore::new(MAX_CONCURRENT_ROOT_SCANS);
|
||||
let future = load_and_merge_host_skill_roots(
|
||||
vec![HostSkillRoot::host(
|
||||
host_root.abs(),
|
||||
SkillScope::User,
|
||||
file_system,
|
||||
)],
|
||||
&root_scan_slots,
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
);
|
||||
fn assert_send<T: Send>(_: &T) {}
|
||||
assert_send(&future);
|
||||
let outcome = future.await;
|
||||
|
||||
assert_eq!(outcome.errors, Vec::new());
|
||||
let first_skill_path = dunce::canonicalize(skills_root.join("first/SKILL.md"))
|
||||
.unwrap()
|
||||
.abs();
|
||||
let second_skill_path = dunce::canonicalize(skills_root.join("second/SKILL.md"))
|
||||
.unwrap()
|
||||
.abs();
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![
|
||||
SkillMetadata {
|
||||
name: "linked:first".to_string(),
|
||||
description: "first skill.".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: Some(SkillPolicy {
|
||||
allow_implicit_invocation: Some(false),
|
||||
products: Vec::new(),
|
||||
}),
|
||||
path_to_skills_md: first_skill_path,
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
},
|
||||
SkillMetadata {
|
||||
name: "linked:second".to_string(),
|
||||
description: "second skill.".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: second_skill_path,
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
},
|
||||
]
|
||||
);
|
||||
|
||||
let calls = recording.calls();
|
||||
assert_eq!(calls.walks, 1);
|
||||
let linked_root = PathUri::from_host_native_path(linked_root).unwrap();
|
||||
assert!(
|
||||
calls
|
||||
.read_files
|
||||
.iter()
|
||||
.all(|path| !path.starts_with(&linked_root))
|
||||
);
|
||||
assert!(
|
||||
calls
|
||||
.metadata_files
|
||||
.iter()
|
||||
.all(|path| path.basename().as_deref() != Some("openai.yaml"))
|
||||
);
|
||||
let manifest_uri =
|
||||
PathUri::from_host_native_path(dunce::canonicalize(manifest_path).unwrap()).unwrap();
|
||||
assert_eq!(
|
||||
calls
|
||||
.metadata_files
|
||||
.iter()
|
||||
.filter(|path| **path == manifest_uri)
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
}
|
||||
@@ -3,7 +3,6 @@ use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core_skills::SkillLoadOutcome;
|
||||
use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
@@ -18,6 +17,7 @@ use tokio::sync::OnceCell;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use super::HostSkillRoot;
|
||||
use super::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use super::host::HostSkillRootSnapshot;
|
||||
use super::load_host_skill_root;
|
||||
|
||||
|
||||
@@ -2,13 +2,19 @@ use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_skills::LoadedSkillRoot;
|
||||
use codex_skills::SkillError;
|
||||
use codex_skills::SkillRootSnapshotCache;
|
||||
use codex_skills::SkillRootSnapshots;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginIdentity;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
@@ -17,6 +23,10 @@ use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use crate::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use crate::loader::io_test_support::ManifestMetadataBehavior;
|
||||
use crate::loader::io_test_support::RecordingFileSystem;
|
||||
|
||||
use super::HostSkillRoot;
|
||||
use super::load_and_merge_host_skill_roots;
|
||||
|
||||
@@ -311,3 +321,188 @@ async fn recognizes_symlinked_migrated_command_roots() {
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn host_root_scans_wait_for_shared_capacity() {
|
||||
let temp = TempDir::new().expect("temp dir");
|
||||
let root = AbsolutePathBuf::from_absolute_path(temp.path()).expect("absolute skill root");
|
||||
let root_scan_slots = Semaphore::new(/*permits*/ 1);
|
||||
let held_slot = root_scan_slots
|
||||
.try_acquire()
|
||||
.expect("root scan slot should be available");
|
||||
let load = load_and_merge_host_skill_roots(
|
||||
vec![HostSkillRoot::host(
|
||||
root,
|
||||
SkillScope::Repo,
|
||||
Arc::clone(&LOCAL_FS),
|
||||
)],
|
||||
&root_scan_slots,
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
);
|
||||
tokio::pin!(load);
|
||||
|
||||
assert!(futures::poll!(load.as_mut()).is_pending());
|
||||
drop(held_slot);
|
||||
let outcome = load.await;
|
||||
|
||||
assert_eq!(outcome.skills, Vec::new());
|
||||
assert_eq!(outcome.errors, Vec::new());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn duplicate_host_root_paths_preserve_first_root_scope() {
|
||||
let temp = TempDir::new().expect("temp dir");
|
||||
let root = AbsolutePathBuf::from_absolute_path(temp.path()).expect("absolute skill root");
|
||||
write_skill(&root, "demo", "demo", "Shared skill");
|
||||
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
vec![
|
||||
HostSkillRoot::host(root.clone(), SkillScope::Repo, Arc::clone(&LOCAL_FS)),
|
||||
HostSkillRoot::host(root.clone(), SkillScope::User, Arc::clone(&LOCAL_FS)),
|
||||
],
|
||||
&Semaphore::new(/*permits*/ 2),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(outcome.errors, Vec::new());
|
||||
assert_eq!(outcome.skills.len(), 1);
|
||||
assert_eq!(outcome.skills[0].scope, SkillScope::Repo);
|
||||
let canonical_root = AbsolutePathBuf::from_absolute_path(
|
||||
fs::canonicalize(root.as_path()).expect("canonicalize skill root"),
|
||||
)
|
||||
.expect("absolute canonical skill root");
|
||||
assert_eq!(
|
||||
outcome.skill_root_for_path(&outcome.skills[0].path_to_skills_md),
|
||||
Some(&canonical_root)
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn deduplicated_symlinked_skill_preserves_first_discovery_path() {
|
||||
let source = TempDir::new().expect("source temp dir");
|
||||
let first = TempDir::new().expect("first root temp dir");
|
||||
let second = TempDir::new().expect("second root temp dir");
|
||||
let source_root =
|
||||
AbsolutePathBuf::from_absolute_path(source.path()).expect("absolute source root");
|
||||
let first_root =
|
||||
AbsolutePathBuf::from_absolute_path(first.path()).expect("absolute first root");
|
||||
let second_root =
|
||||
AbsolutePathBuf::from_absolute_path(second.path()).expect("absolute second root");
|
||||
write_skill(&source_root, "demo", "demo", "Shared skill");
|
||||
std::os::unix::fs::symlink(source.path().join("demo"), first.path().join("first-link"))
|
||||
.expect("create first skill directory symlink");
|
||||
std::os::unix::fs::symlink(
|
||||
source.path().join("demo"),
|
||||
second.path().join("second-link"),
|
||||
)
|
||||
.expect("create second skill directory symlink");
|
||||
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
vec![
|
||||
HostSkillRoot::host(first_root.clone(), SkillScope::Repo, Arc::clone(&LOCAL_FS)),
|
||||
HostSkillRoot::host(second_root, SkillScope::User, Arc::clone(&LOCAL_FS)),
|
||||
],
|
||||
&Semaphore::new(/*permits*/ 2),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(outcome.errors, Vec::new());
|
||||
assert_eq!(outcome.skills.len(), 1);
|
||||
let skill_path = &outcome.skills[0].path_to_skills_md;
|
||||
let canonical_first_root = AbsolutePathBuf::from_absolute_path(
|
||||
fs::canonicalize(first_root.as_path()).expect("canonicalize first skill root"),
|
||||
)
|
||||
.expect("absolute first skill root");
|
||||
assert_eq!(
|
||||
(
|
||||
outcome.skill_root_for_path(skill_path),
|
||||
outcome.skill_discovery_path_for_path(skill_path),
|
||||
),
|
||||
(
|
||||
Some(&canonical_first_root),
|
||||
Some(&canonical_first_root.join("first-link/SKILL.md")),
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn merges_host_root_results_in_input_order_when_scans_finish_out_of_order() {
|
||||
const ROOT_COUNT: usize = MAX_CONCURRENT_ROOT_SCANS + 1;
|
||||
|
||||
let temp = TempDir::new().expect("temp dir");
|
||||
let roots = (0..ROOT_COUNT)
|
||||
.map(|index| temp.path().join(format!("root-{index}")))
|
||||
.collect::<Vec<_>>();
|
||||
for root in &roots {
|
||||
fs::create_dir_all(root).expect("create skill root");
|
||||
}
|
||||
let first_skill = roots[0].join("broken/SKILL.md");
|
||||
let second_skill = roots[1].join("broken/SKILL.md");
|
||||
for path in [&first_skill, &second_skill] {
|
||||
fs::create_dir_all(path.parent().expect("skill parent"))
|
||||
.expect("create invalid skill directory");
|
||||
fs::write(path, "missing frontmatter").expect("write invalid skill");
|
||||
}
|
||||
|
||||
let mut recording =
|
||||
RecordingFileSystem::new(LOCAL_FS.as_ref(), ManifestMetadataBehavior::Immediate);
|
||||
recording.blocked_walk_root = Some(PathUri::from_abs_path(
|
||||
&dunce::canonicalize(&roots[0])
|
||||
.expect("canonical blocked skill root")
|
||||
.abs(),
|
||||
));
|
||||
let recording = Arc::new(recording);
|
||||
let file_system: Arc<dyn ExecutorFileSystem> = recording.clone();
|
||||
let host_roots = roots
|
||||
.iter()
|
||||
.map(|root| HostSkillRoot::host(root.clone().abs(), SkillScope::User, file_system.clone()))
|
||||
.collect::<Vec<_>>();
|
||||
let load = tokio::spawn(async move {
|
||||
load_and_merge_host_skill_roots(
|
||||
host_roots,
|
||||
&Semaphore::new(MAX_CONCURRENT_ROOT_SCANS),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
)
|
||||
.await
|
||||
});
|
||||
|
||||
tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 5), async {
|
||||
loop {
|
||||
let walk_started = recording.walk_started.notified();
|
||||
if recording.walks.load(Ordering::Acquire) == ROOT_COUNT {
|
||||
break;
|
||||
}
|
||||
walk_started.await;
|
||||
}
|
||||
})
|
||||
.await
|
||||
.expect("all host root scans should start despite the blocked first root");
|
||||
recording.blocked_walk_gate.add_permits(/*n*/ 1);
|
||||
let outcome = load.await.expect("host skill root loading should complete");
|
||||
|
||||
assert_eq!(outcome.skills, Vec::new());
|
||||
assert_eq!(
|
||||
outcome.errors,
|
||||
vec![
|
||||
SkillError {
|
||||
path: dunce::canonicalize(first_skill)
|
||||
.expect("canonical first invalid skill path")
|
||||
.abs(),
|
||||
message: "missing YAML frontmatter delimited by ---".to_string(),
|
||||
},
|
||||
SkillError {
|
||||
path: dunce::canonicalize(second_skill)
|
||||
.expect("canonical second invalid skill path")
|
||||
.abs(),
|
||||
message: "missing YAML frontmatter delimited by ---".to_string(),
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -119,7 +119,7 @@ async fn loads_host_frontmatter_dependencies_and_policy() {
|
||||
description: Demo tool
|
||||
policy:
|
||||
allow_implicit_invocation: false
|
||||
products: [codex]
|
||||
products: [codex, CHATGPT, atlas]
|
||||
"##,
|
||||
);
|
||||
|
||||
@@ -145,7 +145,7 @@ policy:
|
||||
}),
|
||||
policy: Some(SkillPolicy {
|
||||
allow_implicit_invocation: Some(false),
|
||||
products: vec![Product::Codex],
|
||||
products: vec![Product::Codex, Product::Chatgpt, Product::Atlas],
|
||||
}),
|
||||
path_to_skills_md: skill_path,
|
||||
scope: SkillScope::User,
|
||||
@@ -433,6 +433,41 @@ async fn plugin_root_accepts_maximum_length_qualified_skill_name() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_root_rejects_overlong_qualified_skill_name() {
|
||||
let root = TempDir::new().expect("temp dir");
|
||||
let skill_name = "s".repeat(MAX_NAME_LEN);
|
||||
write_skill(
|
||||
&root,
|
||||
"skills/search",
|
||||
&format!("name: {skill_name}\ndescription: Search skill"),
|
||||
);
|
||||
let plugin_root = AbsolutePathBuf::from_absolute_path(root.path()).expect("plugin root");
|
||||
|
||||
let snapshot = load_host_skill_root(HostSkillRoot::plugin(
|
||||
PluginSkillRoot {
|
||||
path: plugin_root.join("skills"),
|
||||
plugin_identity: PluginIdentity {
|
||||
plugin_id: "demo@test".to_string(),
|
||||
remote_plugin_id: None,
|
||||
},
|
||||
plugin_namespace: "p".repeat(MAX_NAME_LEN + 1),
|
||||
plugin_root,
|
||||
discovery_mode: SkillDiscoveryMode::Recursive,
|
||||
},
|
||||
Arc::clone(&LOCAL_FS),
|
||||
))
|
||||
.await;
|
||||
|
||||
assert_eq!(snapshot.skills, Vec::new());
|
||||
assert_eq!(snapshot.errors.len(), 1);
|
||||
assert!(
|
||||
snapshot.errors[0]
|
||||
.message
|
||||
.contains("invalid qualified name")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn recursive_plugin_root_preserves_owner_namespace_and_shared_asset_policy() {
|
||||
let root = TempDir::new().expect("temp dir");
|
||||
@@ -638,7 +673,7 @@ async fn recursive_plugin_root_preserves_symlinked_skill_discovery_path() {
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn follows_directory_symlinks_for_user_but_not_system_scope() {
|
||||
async fn follows_directory_symlinks_except_for_system_scope() {
|
||||
use std::os::unix::fs::symlink;
|
||||
|
||||
let root = TempDir::new().expect("temp dir");
|
||||
@@ -646,18 +681,25 @@ async fn follows_directory_symlinks_for_user_but_not_system_scope() {
|
||||
let target_skill = write_skill(&target, "demo", "name: demo\ndescription: Symlinked skill");
|
||||
symlink(target.path().join("demo"), root.path().join("alias")).expect("create symlink");
|
||||
|
||||
let user_snapshot = load_host_skill_root(root_for(&root, SkillScope::User)).await;
|
||||
let system_snapshot = load_host_skill_root(root_for(&root, SkillScope::System)).await;
|
||||
for scope in [SkillScope::User, SkillScope::Repo, SkillScope::Admin] {
|
||||
let snapshot = load_host_skill_root(root_for(&root, scope)).await;
|
||||
|
||||
assert_eq!(user_snapshot.errors, Vec::new());
|
||||
assert_eq!(user_snapshot.skills.len(), 1);
|
||||
assert_eq!(user_snapshot.skills[0].path_to_skills_md, target_skill);
|
||||
assert_eq!(
|
||||
user_snapshot
|
||||
.skill_discovery_path_by_path
|
||||
.get(&target_skill),
|
||||
Some(&user_snapshot.root.join("alias/SKILL.md"))
|
||||
);
|
||||
assert_eq!(snapshot.errors, Vec::new());
|
||||
assert_eq!(snapshot.skills.len(), 1);
|
||||
assert_eq!(
|
||||
(
|
||||
&snapshot.skills[0].path_to_skills_md,
|
||||
snapshot.skills[0].scope
|
||||
),
|
||||
(&target_skill, scope)
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.skill_discovery_path_by_path.get(&target_skill),
|
||||
Some(&snapshot.root.join("alias/SKILL.md"))
|
||||
);
|
||||
}
|
||||
|
||||
let system_snapshot = load_host_skill_root(root_for(&root, SkillScope::System)).await;
|
||||
assert_eq!(system_snapshot.errors, Vec::new());
|
||||
assert_eq!(system_snapshot.skills, Vec::new());
|
||||
}
|
||||
|
||||
@@ -0,0 +1,214 @@
|
||||
use std::sync::Mutex;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
use codex_exec_server::CopyOptions;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::ExecutorFileSystemFuture;
|
||||
use codex_exec_server::FileMetadata;
|
||||
use codex_exec_server::FileSystemReadStream;
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_exec_server::ReadDirectoryEntry;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_exec_server::WalkOptions;
|
||||
use codex_exec_server::WalkOutcome;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use tokio::sync::Notify;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
pub(super) enum ManifestMetadataBehavior {
|
||||
Immediate,
|
||||
WaitForSkillRead,
|
||||
}
|
||||
|
||||
pub(super) struct RecordingFileSystem<'a> {
|
||||
inner: &'a dyn ExecutorFileSystem,
|
||||
read_files: Mutex<Vec<PathUri>>,
|
||||
metadata_files: Mutex<Vec<PathUri>>,
|
||||
pub(super) walks: AtomicUsize,
|
||||
manifest_metadata_behavior: ManifestMetadataBehavior,
|
||||
skill_read_started: AtomicBool,
|
||||
skill_read_started_notify: Notify,
|
||||
pub(super) blocked_walk_root: Option<PathUri>,
|
||||
pub(super) blocked_walk_gate: Semaphore,
|
||||
pub(super) walk_started: Notify,
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub(super) struct FileSystemCalls {
|
||||
pub(super) walks: usize,
|
||||
pub(super) read_files: Vec<PathUri>,
|
||||
pub(super) metadata_files: Vec<PathUri>,
|
||||
}
|
||||
|
||||
impl<'a> RecordingFileSystem<'a> {
|
||||
pub(super) fn new(
|
||||
inner: &'a dyn ExecutorFileSystem,
|
||||
manifest_metadata_behavior: ManifestMetadataBehavior,
|
||||
) -> Self {
|
||||
Self {
|
||||
inner,
|
||||
read_files: Mutex::new(Vec::new()),
|
||||
metadata_files: Mutex::new(Vec::new()),
|
||||
walks: AtomicUsize::new(/*v*/ 0),
|
||||
manifest_metadata_behavior,
|
||||
skill_read_started: AtomicBool::new(/*v*/ false),
|
||||
skill_read_started_notify: Notify::new(),
|
||||
blocked_walk_root: None,
|
||||
blocked_walk_gate: Semaphore::new(/*permits*/ 0),
|
||||
walk_started: Notify::new(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn calls(&self) -> FileSystemCalls {
|
||||
let mut read_files = self
|
||||
.read_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone();
|
||||
read_files.sort_by_key(ToString::to_string);
|
||||
let mut metadata_files = self
|
||||
.metadata_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone();
|
||||
metadata_files.sort_by_key(ToString::to_string);
|
||||
FileSystemCalls {
|
||||
walks: self.walks.load(Ordering::Relaxed),
|
||||
read_files,
|
||||
metadata_files,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ExecutorFileSystem for RecordingFileSystem<'_> {
|
||||
fn canonicalize<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, PathUri> {
|
||||
self.inner.canonicalize(path, sandbox)
|
||||
}
|
||||
|
||||
fn read_file<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, Vec<u8>> {
|
||||
self.read_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push(path.clone());
|
||||
if path.basename().as_deref() == Some("SKILL.md") {
|
||||
self.skill_read_started.store(true, Ordering::Release);
|
||||
self.skill_read_started_notify.notify_waiters();
|
||||
}
|
||||
self.inner.read_file(path, sandbox)
|
||||
}
|
||||
|
||||
fn read_file_stream<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, FileSystemReadStream> {
|
||||
self.inner.read_file_stream(path, sandbox)
|
||||
}
|
||||
|
||||
fn write_file<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
contents: Vec<u8>,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner.write_file(path, contents, sandbox)
|
||||
}
|
||||
|
||||
fn create_directory<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
options: CreateDirectoryOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner.create_directory(path, options, sandbox)
|
||||
}
|
||||
|
||||
fn get_metadata<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, FileMetadata> {
|
||||
self.metadata_files
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push(path.clone());
|
||||
if matches!(
|
||||
self.manifest_metadata_behavior,
|
||||
ManifestMetadataBehavior::WaitForSkillRead
|
||||
) && path.basename().as_deref() == Some("plugin.json")
|
||||
{
|
||||
return Box::pin(async move {
|
||||
loop {
|
||||
let notified = self.skill_read_started_notify.notified();
|
||||
if self.skill_read_started.load(Ordering::Acquire) {
|
||||
break;
|
||||
}
|
||||
notified.await;
|
||||
}
|
||||
self.inner.get_metadata(path, sandbox).await
|
||||
});
|
||||
}
|
||||
self.inner.get_metadata(path, sandbox)
|
||||
}
|
||||
|
||||
fn read_directory<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, Vec<ReadDirectoryEntry>> {
|
||||
self.inner.read_directory(path, sandbox)
|
||||
}
|
||||
|
||||
fn walk<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
options: WalkOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, WalkOutcome> {
|
||||
self.walks.fetch_add(/*val*/ 1, Ordering::AcqRel);
|
||||
self.walk_started.notify_waiters();
|
||||
if self.blocked_walk_root.as_ref() != Some(path) {
|
||||
return self.inner.walk(path, options, sandbox);
|
||||
}
|
||||
Box::pin(async move {
|
||||
self.blocked_walk_gate
|
||||
.acquire()
|
||||
.await
|
||||
.expect("blocked walk gate should remain open")
|
||||
.forget();
|
||||
self.inner.walk(path, options, sandbox).await
|
||||
})
|
||||
}
|
||||
|
||||
fn remove<'a>(
|
||||
&'a self,
|
||||
path: &'a PathUri,
|
||||
options: RemoveOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner.remove(path, options, sandbox)
|
||||
}
|
||||
|
||||
fn copy<'a>(
|
||||
&'a self,
|
||||
source_path: &'a PathUri,
|
||||
destination_path: &'a PathUri,
|
||||
options: CopyOptions,
|
||||
sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, ()> {
|
||||
self.inner
|
||||
.copy(source_path, destination_path, options, sandbox)
|
||||
}
|
||||
}
|
||||
@@ -2,15 +2,18 @@ mod discovery;
|
||||
mod environment;
|
||||
mod host;
|
||||
mod host_merge;
|
||||
#[cfg(test)]
|
||||
mod io_test_support;
|
||||
mod metadata;
|
||||
mod namespace;
|
||||
|
||||
pub(crate) use environment::load_environment_skills_from_discovery;
|
||||
pub(crate) use environment::load_environment_skills_from_root;
|
||||
pub(crate) use host::HostSkillRoot;
|
||||
pub use host::HostSkillRoot;
|
||||
pub(crate) use host::load_host_skill_root;
|
||||
pub(crate) use host_merge::load_and_merge_host_skill_roots;
|
||||
|
||||
pub(crate) const MAX_CONCURRENT_ROOT_SCANS: usize = 8;
|
||||
pub(super) const SKILLS_FILENAME: &str = "SKILL.md";
|
||||
pub(super) const SKILLS_METADATA_DIR: &str = "agents";
|
||||
pub(super) const SKILLS_METADATA_FILENAME: &str = "openai.yaml";
|
||||
|
||||
@@ -180,3 +180,7 @@ impl ResolvedSkillNamespace {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "namespace_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,172 @@
|
||||
use std::collections::HashSet;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
#[cfg(unix)]
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
#[cfg(unix)]
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[cfg(unix)]
|
||||
use crate::loader::HostSkillRoot;
|
||||
#[cfg(unix)]
|
||||
use crate::loader::load_host_skill_root;
|
||||
|
||||
use super::SkillNamespaceResolver;
|
||||
|
||||
fn canonical_uri(path: &Path) -> PathUri {
|
||||
PathUri::from_host_native_path(fs::canonicalize(path).expect("canonical path"))
|
||||
.expect("absolute path URI")
|
||||
}
|
||||
|
||||
fn write_skill(root: &Path, directory: &str) -> PathUri {
|
||||
let path = root.join(directory).join("SKILL.md");
|
||||
fs::create_dir_all(path.parent().expect("skill parent")).expect("create skill directory");
|
||||
fs::write(&path, "---\nname: skill\ndescription: Demo skill\n---\n")
|
||||
.expect("write skill frontmatter");
|
||||
canonical_uri(&path)
|
||||
}
|
||||
|
||||
fn write_manifest(root: &Path, contents: &str) -> PathUri {
|
||||
let path = root.join(".codex-plugin/plugin.json");
|
||||
fs::create_dir_all(path.parent().expect("manifest parent"))
|
||||
.expect("create plugin manifest directory");
|
||||
fs::write(path, contents).expect("write plugin manifest");
|
||||
canonical_uri(root)
|
||||
}
|
||||
|
||||
async fn namespaces_for(
|
||||
root: &Path,
|
||||
skill_paths: &[PathUri],
|
||||
plugin_roots: HashSet<PathUri>,
|
||||
mut namespace_roots: HashSet<PathUri>,
|
||||
) -> Vec<String> {
|
||||
let root = canonical_uri(root);
|
||||
namespace_roots.insert(root.clone());
|
||||
let resolver = SkillNamespaceResolver::discover(
|
||||
LOCAL_FS.as_ref(),
|
||||
&root,
|
||||
skill_paths,
|
||||
plugin_roots,
|
||||
namespace_roots,
|
||||
)
|
||||
.await;
|
||||
|
||||
skill_paths
|
||||
.iter()
|
||||
.map(|path| resolver.for_skill(&root, path).qualify("skill"))
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn nested_plugin_namespaces_do_not_apply_to_plain_siblings() {
|
||||
let root = TempDir::new().expect("root temp dir");
|
||||
let nested_plugin = root.path().join("nested-plugin");
|
||||
let plugin_root = write_manifest(&nested_plugin, r#"{"name":"nested"}"#);
|
||||
let plain_skill = write_skill(root.path(), "plain");
|
||||
let nested_skill = write_skill(&nested_plugin, "skills/search");
|
||||
|
||||
let namespaces = namespaces_for(
|
||||
root.path(),
|
||||
&[plain_skill, nested_skill],
|
||||
HashSet::from([plugin_root]),
|
||||
HashSet::new(),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(namespaces, vec!["skill", "nested:skill"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn nearest_valid_manifest_overrides_or_falls_back_to_outer_namespace() {
|
||||
let root = TempDir::new().expect("root temp dir");
|
||||
let plugin_root = root.path().join("outer");
|
||||
let skills_root = plugin_root.join("skills");
|
||||
let nested_plugin = skills_root.join("nested");
|
||||
let invalid_plugin = skills_root.join("invalid");
|
||||
write_manifest(&plugin_root, r#"{"name":"outer"}"#);
|
||||
let nested_root = write_manifest(&nested_plugin, r#"{"name":"nested"}"#);
|
||||
let invalid_root = write_manifest(&invalid_plugin, "not json");
|
||||
let direct_skill = write_skill(&skills_root, "direct");
|
||||
let nested_skill = write_skill(&nested_plugin, "skills/search");
|
||||
let fallback_skill = write_skill(&invalid_plugin, "skills/fallback");
|
||||
|
||||
let namespaces = namespaces_for(
|
||||
&skills_root,
|
||||
&[direct_skill, nested_skill, fallback_skill],
|
||||
HashSet::from([nested_root, invalid_root]),
|
||||
HashSet::new(),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
namespaces,
|
||||
vec!["outer:skill", "nested:skill", "outer:skill"]
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn symlinked_external_directories_do_not_inherit_plugin_namespace() {
|
||||
let root = TempDir::new().expect("root temp dir");
|
||||
let external = TempDir::new().expect("external temp dir");
|
||||
let plugin_root = root.path().join("outer");
|
||||
let skills_root = plugin_root.join("skills");
|
||||
write_manifest(&plugin_root, r#"{"name":"outer"}"#);
|
||||
write_skill(external.path(), "plain");
|
||||
fs::create_dir_all(&skills_root).expect("create skill root");
|
||||
std::os::unix::fs::symlink(external.path(), skills_root.join("linked"))
|
||||
.expect("create external directory symlink");
|
||||
|
||||
let snapshot = load_host_skill_root(HostSkillRoot::host(
|
||||
AbsolutePathBuf::from_absolute_path(&skills_root).expect("absolute skills root"),
|
||||
SkillScope::User,
|
||||
Arc::clone(&LOCAL_FS),
|
||||
))
|
||||
.await;
|
||||
|
||||
assert_eq!(snapshot.errors, Vec::new());
|
||||
assert_eq!(
|
||||
snapshot
|
||||
.skills
|
||||
.into_iter()
|
||||
.map(|skill| skill.name)
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["skill"]
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn ancestor_symlink_roots_preserve_inherited_plugin_namespace() {
|
||||
let root = TempDir::new().expect("root temp dir");
|
||||
let plugin_root = root.path().join("a/b/c/d/e/f/outer");
|
||||
let skills_root = plugin_root.join("skills");
|
||||
write_manifest(&plugin_root, r#"{"name":"outer"}"#);
|
||||
write_skill(&skills_root, "direct");
|
||||
std::os::unix::fs::symlink(root.path(), skills_root.join("ancestor"))
|
||||
.expect("create ancestor directory symlink");
|
||||
|
||||
let snapshot = load_host_skill_root(HostSkillRoot::host(
|
||||
AbsolutePathBuf::from_absolute_path(&skills_root).expect("absolute skills root"),
|
||||
SkillScope::User,
|
||||
Arc::clone(&LOCAL_FS),
|
||||
))
|
||||
.await;
|
||||
|
||||
assert_eq!(snapshot.errors, Vec::new());
|
||||
assert_eq!(
|
||||
snapshot
|
||||
.skills
|
||||
.into_iter()
|
||||
.map(|skill| skill.name)
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["outer:skill"]
|
||||
);
|
||||
}
|
||||
@@ -2,8 +2,6 @@ use std::sync::Arc;
|
||||
use std::time::SystemTime;
|
||||
use std::time::UNIX_EPOCH;
|
||||
|
||||
use codex_core_skills::loader::SkillRoot;
|
||||
use codex_core_skills::loader::load_skills_from_roots;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -11,6 +9,8 @@ use pretty_assertions::assert_eq;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use super::catalog_from_outcome;
|
||||
use crate::loader::HostSkillRoot;
|
||||
use crate::loader::load_and_merge_host_skill_roots;
|
||||
|
||||
#[tokio::test]
|
||||
async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box<dyn std::error::Error>>
|
||||
@@ -31,18 +31,15 @@ async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box<dy
|
||||
"---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n",
|
||||
)?;
|
||||
let root = AbsolutePathBuf::try_from(std::fs::canonicalize(root)?)?;
|
||||
let outcome = load_skills_from_roots(
|
||||
[SkillRoot {
|
||||
path: root.clone(),
|
||||
scope: SkillScope::User,
|
||||
file_system: Arc::clone(&LOCAL_FS),
|
||||
plugin_identity: None,
|
||||
plugin_namespace: None,
|
||||
plugin_root: None,
|
||||
discovery_mode: Default::default(),
|
||||
}],
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
vec![HostSkillRoot::host(
|
||||
root.clone(),
|
||||
SkillScope::User,
|
||||
Arc::clone(&LOCAL_FS),
|
||||
)],
|
||||
&Semaphore::new(/*permits*/ 1),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(Semaphore::new(1)),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -79,18 +76,15 @@ async fn host_catalog_preserves_symlinked_skill_discovery_paths()
|
||||
std::os::unix::fs::symlink(&source_skill_dir, root.path().join("linked-skill"))?;
|
||||
|
||||
let root = AbsolutePathBuf::try_from(std::fs::canonicalize(root.path())?)?;
|
||||
let outcome = load_skills_from_roots(
|
||||
[SkillRoot {
|
||||
path: root.clone(),
|
||||
scope: SkillScope::User,
|
||||
file_system: Arc::clone(&LOCAL_FS),
|
||||
plugin_identity: None,
|
||||
plugin_namespace: None,
|
||||
plugin_root: None,
|
||||
discovery_mode: Default::default(),
|
||||
}],
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
vec![HostSkillRoot::host(
|
||||
root.clone(),
|
||||
SkillScope::User,
|
||||
Arc::clone(&LOCAL_FS),
|
||||
)],
|
||||
&Semaphore::new(/*permits*/ 1),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(Semaphore::new(1)),
|
||||
)
|
||||
.await;
|
||||
let catalog = catalog_from_outcome(&outcome);
|
||||
|
||||
@@ -6,8 +6,6 @@ use crate::catalog::SkillResourceId;
|
||||
use crate::provider::HostSkillProvider;
|
||||
use crate::provider::SkillListQuery;
|
||||
use crate::provider::SkillProvider;
|
||||
use codex_core_skills::loader::SkillRoot;
|
||||
use codex_core_skills::loader::load_skills_from_roots;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_extension_api::ContextualUserFragment;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
@@ -19,6 +17,8 @@ use std::time::UNIX_EPOCH;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use crate::catalog_prompt::render_available_skills_body;
|
||||
use crate::loader::HostSkillRoot;
|
||||
use crate::loader::load_and_merge_host_skill_roots;
|
||||
|
||||
fn entry(name: &str, description: &str, short_description: Option<&str>) -> SkillCatalogEntry {
|
||||
entry_with_path(
|
||||
@@ -301,29 +301,18 @@ async fn host_alias_roots_follow_core_discovery_order() -> Result<(), Box<dyn st
|
||||
}
|
||||
let user_root = AbsolutePathBuf::try_from(std::fs::canonicalize(&user_root_path)?)?;
|
||||
let system_root = AbsolutePathBuf::try_from(std::fs::canonicalize(&system_root_path)?)?;
|
||||
let outcome = load_skills_from_roots(
|
||||
[
|
||||
SkillRoot {
|
||||
path: user_root.clone(),
|
||||
scope: SkillScope::User,
|
||||
file_system: Arc::clone(&LOCAL_FS),
|
||||
plugin_identity: None,
|
||||
plugin_namespace: None,
|
||||
plugin_root: None,
|
||||
discovery_mode: Default::default(),
|
||||
},
|
||||
SkillRoot {
|
||||
path: system_root.clone(),
|
||||
scope: SkillScope::System,
|
||||
file_system: Arc::clone(&LOCAL_FS),
|
||||
plugin_identity: None,
|
||||
plugin_namespace: None,
|
||||
plugin_root: None,
|
||||
discovery_mode: Default::default(),
|
||||
},
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
vec![
|
||||
HostSkillRoot::host(user_root.clone(), SkillScope::User, Arc::clone(&LOCAL_FS)),
|
||||
HostSkillRoot::host(
|
||||
system_root.clone(),
|
||||
SkillScope::System,
|
||||
Arc::clone(&LOCAL_FS),
|
||||
),
|
||||
],
|
||||
&Semaphore::new(/*permits*/ 2),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(Semaphore::new(2)),
|
||||
)
|
||||
.await;
|
||||
let catalog = HostSkillProvider::new()
|
||||
|
||||
@@ -4,10 +4,10 @@ use std::sync::Arc;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use codex_core_skills::loader::SkillRoot;
|
||||
use codex_core_skills::loader::load_environment_skills_from_root as load_legacy_environment_skills_from_root;
|
||||
use codex_core_skills::loader::load_skills_from_roots;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_exec_server::CopyOptions;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
@@ -25,9 +25,9 @@ use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_skills_extension::ExecutorSkillProvider;
|
||||
use codex_skills_extension::HostSkillsSnapshot;
|
||||
use codex_skills_extension::HostSkillsLoadInput;
|
||||
use codex_skills_extension::HostSkillsService;
|
||||
use codex_skills_extension::catalog::SkillAuthority;
|
||||
use codex_skills_extension::catalog::SkillPackageId;
|
||||
use codex_skills_extension::catalog::SkillResourceId;
|
||||
@@ -203,31 +203,47 @@ impl ExecutorFileSystem for SyntheticFileSystem {
|
||||
async fn skill_loading_and_reads_use_the_supplied_executor_file_system() {
|
||||
let test_root =
|
||||
std::env::temp_dir().join(format!("codex-executor-skill-fs-{}", std::process::id()));
|
||||
let alias_root = AbsolutePathBuf::from_absolute_path_checked(test_root.join("alias"))
|
||||
.expect("absolute path");
|
||||
let project_folder = AbsolutePathBuf::from_absolute_path_checked(test_root.join(".codex"))
|
||||
.expect("absolute project folder");
|
||||
let alias_root = project_folder.join("skills");
|
||||
let canonical_root = AbsolutePathBuf::from_absolute_path_checked(test_root.join("canonical"))
|
||||
.expect("absolute path");
|
||||
assert!(!alias_root.as_path().exists());
|
||||
assert!(!canonical_root.as_path().exists());
|
||||
|
||||
let outcome = load_skills_from_roots(
|
||||
[SkillRoot {
|
||||
path: alias_root.clone(),
|
||||
scope: SkillScope::User,
|
||||
file_system: Arc::new(SyntheticFileSystem {
|
||||
let config_layer_stack = ConfigLayerStack::new(
|
||||
vec![ConfigLayerEntry::new(
|
||||
ConfigLayerSource::Project {
|
||||
dot_codex_folder: project_folder,
|
||||
},
|
||||
toml::Value::Table(Default::default()),
|
||||
)],
|
||||
Default::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("valid project config stack");
|
||||
let cwd = AbsolutePathBuf::from_absolute_path_checked(test_root).expect("absolute test root");
|
||||
let service = HostSkillsService::new_with_restriction_product(
|
||||
cwd.clone(),
|
||||
/*bundled_skills_enabled*/ false,
|
||||
/*restriction_product*/ None,
|
||||
);
|
||||
let snapshot = service
|
||||
.snapshot_for_config(
|
||||
&HostSkillsLoadInput::new(
|
||||
cwd,
|
||||
Vec::new(),
|
||||
config_layer_stack,
|
||||
/*bundled_skills_enabled*/ false,
|
||||
),
|
||||
Some(Arc::new(SyntheticFileSystem {
|
||||
alias_root: PathUri::from_abs_path(&alias_root),
|
||||
canonical_root: PathUri::from_abs_path(&canonical_root),
|
||||
has_plugin_manifest: false,
|
||||
}),
|
||||
plugin_identity: None,
|
||||
plugin_namespace: None,
|
||||
plugin_root: None,
|
||||
discovery_mode: Default::default(),
|
||||
}],
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(tokio::sync::Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)),
|
||||
)
|
||||
.await;
|
||||
})),
|
||||
)
|
||||
.await;
|
||||
let outcome = snapshot.outcome();
|
||||
assert_eq!(outcome.errors, Vec::new());
|
||||
assert_eq!(outcome.skills.len(), 1);
|
||||
|
||||
@@ -237,9 +253,8 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() {
|
||||
skill.path_to_skills_md,
|
||||
canonical_root.join("skill/SKILL.md")
|
||||
);
|
||||
let loaded = HostSkillsSnapshot::new(Arc::new(outcome));
|
||||
assert_eq!(
|
||||
loaded.read_skill_text(&skill).await.expect("skill body"),
|
||||
snapshot.read_skill_text(&skill).await.expect("skill body"),
|
||||
SKILL_CONTENTS
|
||||
);
|
||||
}
|
||||
@@ -441,16 +456,6 @@ async fn executor_discovery_routes_produce_equivalent_catalog_metadata() {
|
||||
executor_capability_discovery,
|
||||
};
|
||||
|
||||
let legacy_file_system = manager
|
||||
.get_environment("local")
|
||||
.expect("local environment")
|
||||
.get_filesystem();
|
||||
let legacy = load_legacy_environment_skills_from_root(
|
||||
legacy_file_system.as_ref(),
|
||||
&PathUri::from_host_native_path(&test_root).expect("skill root URI"),
|
||||
Some(Product::Codex),
|
||||
)
|
||||
.await;
|
||||
let direct = provider
|
||||
.list(query(None))
|
||||
.await
|
||||
@@ -484,36 +489,8 @@ async fn executor_discovery_routes_produce_equivalent_catalog_metadata() {
|
||||
.collect::<Vec<_>>()
|
||||
};
|
||||
assert_eq!(comparable_entries(&bundled), comparable_entries(&direct));
|
||||
assert_eq!(legacy.warnings, direct.warnings);
|
||||
assert_eq!(direct.warnings.len(), 1);
|
||||
assert!(direct.warnings[0].contains("missing field `description`"));
|
||||
let legacy_metadata = legacy
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| {
|
||||
(
|
||||
skill.name.clone(),
|
||||
skill.description.clone(),
|
||||
skill.short_description.clone(),
|
||||
skill.dependencies.clone(),
|
||||
skill.allows_implicit_invocation(),
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let catalog_metadata = direct
|
||||
.entries
|
||||
.iter()
|
||||
.map(|entry| {
|
||||
(
|
||||
entry.name.clone(),
|
||||
entry.description.clone(),
|
||||
entry.short_description.clone(),
|
||||
entry.dependencies.clone(),
|
||||
entry.prompt_visible,
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(catalog_metadata, legacy_metadata);
|
||||
let entry = direct
|
||||
.entries
|
||||
.iter()
|
||||
@@ -692,16 +669,6 @@ async fn direct_executor_discovery_preserves_hidden_nested_and_probed_metadata()
|
||||
},
|
||||
};
|
||||
let manager = Arc::new(EnvironmentManager::default_for_tests());
|
||||
let legacy_file_system = manager
|
||||
.get_environment("local")
|
||||
.expect("local environment")
|
||||
.get_filesystem();
|
||||
let legacy = load_legacy_environment_skills_from_root(
|
||||
legacy_file_system.as_ref(),
|
||||
&root_uri,
|
||||
/*restriction_product*/ None,
|
||||
)
|
||||
.await;
|
||||
let provider = ExecutorSkillProvider::new_with_restriction_product(
|
||||
manager, /*restriction_product*/ None,
|
||||
);
|
||||
@@ -720,18 +687,7 @@ async fn direct_executor_discovery_preserves_hidden_nested_and_probed_metadata()
|
||||
.await
|
||||
.expect("list directly discovered executor skills");
|
||||
|
||||
assert_eq!(direct.warnings, legacy.warnings);
|
||||
let legacy_metadata = legacy
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| {
|
||||
(
|
||||
skill.name.clone(),
|
||||
skill.description.clone(),
|
||||
skill.allows_implicit_invocation(),
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(direct.warnings, Vec::<String>::new());
|
||||
let catalog_metadata = direct
|
||||
.entries
|
||||
.iter()
|
||||
@@ -743,7 +699,6 @@ async fn direct_executor_discovery_preserves_hidden_nested_and_probed_metadata()
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(catalog_metadata, legacy_metadata);
|
||||
assert_eq!(
|
||||
catalog_metadata,
|
||||
vec![
|
||||
|
||||
@@ -4,11 +4,10 @@ use std::sync::Mutex;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_core_skills::SkillLoadOutcome;
|
||||
use codex_core_skills::injection::InjectedHostSkillPrompts;
|
||||
use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use codex_core_skills::loader::SkillRoot;
|
||||
use codex_core_skills::loader::load_skills_from_roots;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_extension_api::ConversationHistory;
|
||||
use codex_extension_api::ExtensionData;
|
||||
@@ -44,6 +43,8 @@ use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_skills::SkillMetadata;
|
||||
use codex_skills_extension::HostSkillProvider;
|
||||
use codex_skills_extension::HostSkillsLoadInput;
|
||||
use codex_skills_extension::HostSkillsService;
|
||||
use codex_skills_extension::HostSkillsSnapshot;
|
||||
use codex_skills_extension::SkillProviders;
|
||||
use codex_skills_extension::SkillsExtensionConfig;
|
||||
@@ -70,7 +71,6 @@ use opentelemetry_sdk::metrics::InMemoryMetricExporter;
|
||||
use opentelemetry_sdk::metrics::data::AggregatedMetrics;
|
||||
use opentelemetry_sdk::metrics::data::MetricData;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
type TestResult = Result<(), Box<dyn std::error::Error>>;
|
||||
|
||||
@@ -1876,22 +1876,34 @@ async fn host_catalog_compacts_shared_paths_under_budget_pressure() -> TestResul
|
||||
}
|
||||
let root = AbsolutePathBuf::try_from(std::fs::canonicalize(root)?)?;
|
||||
let rendered_root = root.to_string_lossy().replace('\\', "/");
|
||||
let outcome = load_skills_from_roots(
|
||||
[SkillRoot {
|
||||
path: root,
|
||||
scope: SkillScope::User,
|
||||
file_system: Arc::clone(&LOCAL_FS),
|
||||
plugin_identity: None,
|
||||
plugin_namespace: None,
|
||||
plugin_root: None,
|
||||
discovery_mode: Default::default(),
|
||||
}],
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(outcome.errors, Vec::new());
|
||||
assert_eq!(outcome.skills.len(), 12 + usize::from(cfg!(unix)));
|
||||
let config_layer_stack = ConfigLayerStack::new(
|
||||
Vec::new(),
|
||||
Default::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)?;
|
||||
let codex_home = AbsolutePathBuf::try_from(test_root.clone())?;
|
||||
let service = HostSkillsService::new_with_restriction_product(
|
||||
codex_home.clone(),
|
||||
/*bundled_skills_enabled*/ false,
|
||||
/*restriction_product*/ None,
|
||||
);
|
||||
service.set_extra_roots(vec![root]);
|
||||
let snapshot = service
|
||||
.snapshot_for_config(
|
||||
&HostSkillsLoadInput::new(
|
||||
codex_home,
|
||||
Vec::new(),
|
||||
config_layer_stack,
|
||||
/*bundled_skills_enabled*/ false,
|
||||
),
|
||||
Some(Arc::clone(&LOCAL_FS)),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(snapshot.outcome().errors, Vec::new());
|
||||
assert_eq!(
|
||||
snapshot.outcome().skills.len(),
|
||||
12 + usize::from(cfg!(unix))
|
||||
);
|
||||
|
||||
let mut builder = ExtensionRegistryBuilder::new();
|
||||
install(&mut builder, skills_extension_config);
|
||||
@@ -1916,7 +1928,7 @@ async fn host_catalog_compacts_shared_paths_under_budget_pressure() -> TestResul
|
||||
model_info.context_window = Some(10_000);
|
||||
thread_store.insert(model_info);
|
||||
let turn_store = ExtensionData::new("turn-1");
|
||||
turn_store.insert(HostSkillsSnapshot::new(Arc::new(outcome)));
|
||||
turn_store.insert(snapshot);
|
||||
|
||||
let fragments = registry.turn_input_contributors()[0]
|
||||
.contribute(
|
||||
|
||||
@@ -41,3 +41,79 @@ fn uses_default_name_and_requires_description() {
|
||||
.expect_err("description should be required");
|
||||
assert_eq!(error.to_string(), "missing field `description`");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn repairs_short_descriptions_containing_colons_and_apostrophes() {
|
||||
let parsed = parse_skill_frontmatter_metadata(
|
||||
"---\nname: short\ndescription: Short skill\nmetadata:\n short-description: What's included: builds and tests\n---\n",
|
||||
|| "fallback".to_string(),
|
||||
)
|
||||
.expect("frontmatter with an unquoted short description should be repaired");
|
||||
|
||||
assert_eq!(
|
||||
parsed,
|
||||
ParsedSkillFrontmatter {
|
||||
name: "short".to_string(),
|
||||
description: "Short skill".to_string(),
|
||||
short_description: Some("What's included: builds and tests".to_string()),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn repairs_unrecognized_frontmatter_fields_that_need_quotes() {
|
||||
let parsed = parse_skill_frontmatter_metadata(
|
||||
"---\nname: unknown\ndescription: Unknown fields\nargument-hint: <duration: e.g. 7d, 2w>\ntags: [next,@supabase/ssr]\n---\n",
|
||||
|| "fallback".to_string(),
|
||||
)
|
||||
.expect("frontmatter with unrecognized fields should be repaired");
|
||||
|
||||
assert_eq!(
|
||||
parsed,
|
||||
ParsedSkillFrontmatter {
|
||||
name: "unknown".to_string(),
|
||||
description: "Unknown fields".to_string(),
|
||||
short_description: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_block_scalar_bodies_while_repairing_other_fields() {
|
||||
let parsed = parse_skill_frontmatter_metadata(
|
||||
"---\nname: block\ndescription: |-\n Build for AWS: ECS\nargument-hint: <duration: e.g. 7d>\n---\n",
|
||||
|| "fallback".to_string(),
|
||||
)
|
||||
.expect("frontmatter repair should preserve block scalar bodies");
|
||||
|
||||
assert_eq!(
|
||||
parsed,
|
||||
ParsedSkillFrontmatter {
|
||||
name: "block".to_string(),
|
||||
description: "Build for AWS: ECS".to_string(),
|
||||
short_description: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_overlong_descriptions_and_short_descriptions() {
|
||||
let description = "💡".repeat(/*n*/ 1_025);
|
||||
let short_description = "x".repeat(/*n*/ 1_025);
|
||||
let parsed = parse_skill_frontmatter_metadata(
|
||||
&format!(
|
||||
"---\nname: long\ndescription: {description}\nmetadata:\n short-description: {short_description}\n---\n"
|
||||
),
|
||||
|| "fallback".to_string(),
|
||||
)
|
||||
.expect("descriptions should not be truncated during frontmatter parsing");
|
||||
|
||||
assert_eq!(
|
||||
parsed,
|
||||
ParsedSkillFrontmatter {
|
||||
name: "long".to_string(),
|
||||
description,
|
||||
short_description: Some(short_description),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user