From 1669c2403f793d0230065397dfc25f52b844244e Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Tue, 4 Aug 2026 11:14:32 +0000 Subject: [PATCH] Add host skill root loading (#36884) ## What changed - Add a loader that discovers skills from canonical host roots and preserves their `SkillScope`. - Skip hidden directories, follow directory symlinks for user, repo, and admin scopes, and ignore them for system skills. - Load dependencies and policy from optional `agents/openai.yaml` metadata without rejecting a skill when that metadata is missing or invalid. - Resolve plugin and symlink namespaces while retaining host paths and reporting per-skill errors outside system scope. ## Testing Add coverage for frontmatter and metadata loading, invalid optional metadata, hidden directories, plugin namespaces, and scope-specific symlink discovery. GitOrigin-RevId: ac436d93b5943b7eaae32143e43367921dd1c350 --- codex-rs/ext/skills/Cargo.toml | 2 +- codex-rs/ext/skills/src/lib.rs | 2 + codex-rs/ext/skills/src/loader/discovery.rs | 44 ++- codex-rs/ext/skills/src/loader/environment.rs | 13 +- codex-rs/ext/skills/src/loader/host.rs | 251 ++++++++++++++++++ codex-rs/ext/skills/src/loader/host_tests.rs | 193 ++++++++++++++ codex-rs/ext/skills/src/loader/metadata.rs | 72 +++++ codex-rs/ext/skills/src/loader/mod.rs | 1 + codex-rs/ext/skills/src/loader/namespace.rs | 6 +- 9 files changed, 578 insertions(+), 6 deletions(-) create mode 100644 codex-rs/ext/skills/src/loader/host.rs create mode 100644 codex-rs/ext/skills/src/loader/host_tests.rs diff --git a/codex-rs/ext/skills/Cargo.toml b/codex-rs/ext/skills/Cargo.toml index 8c64028ac2..f074d45c88 100644 --- a/codex-rs/ext/skills/Cargo.toml +++ b/codex-rs/ext/skills/Cargo.toml @@ -21,6 +21,7 @@ codex-otel = { workspace = true } codex-protocol = { workspace = true } codex-skills = { workspace = true } codex-tools = { workspace = true } +codex-utils-absolute-path = { workspace = true } codex-utils-path-uri = { workspace = true } codex-utils-plugins = { workspace = true } codex-utils-string = { workspace = true } @@ -35,7 +36,6 @@ url = { workspace = true } [dev-dependencies] codex-models-manager = { workspace = true } -codex-utils-absolute-path = { workspace = true } codex-utils-cargo-bin = { workspace = true } insta = { workspace = true } opentelemetry_sdk = { workspace = true } diff --git a/codex-rs/ext/skills/src/lib.rs b/codex-rs/ext/skills/src/lib.rs index 4b4c4e57ce..39b3611ff5 100644 --- a/codex-rs/ext/skills/src/lib.rs +++ b/codex-rs/ext/skills/src/lib.rs @@ -4,6 +4,8 @@ mod config; mod dynamic_skill_selector; mod extension; mod fragments; +// Host loading is staged before its crate-internal runtime caller in this PR stack. +#[allow(dead_code)] mod loader; pub mod provider; mod render; diff --git a/codex-rs/ext/skills/src/loader/discovery.rs b/codex-rs/ext/skills/src/loader/discovery.rs index 3d1609b237..547400bc4f 100644 --- a/codex-rs/ext/skills/src/loader/discovery.rs +++ b/codex-rs/ext/skills/src/loader/discovery.rs @@ -16,6 +16,21 @@ 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(super) struct SkillDiscovery { pub skills: Vec, pub plugin_roots: HashSet, @@ -37,6 +52,7 @@ pub(super) enum SkillMetadataDiscovery { pub(super) async fn discover_skills( file_system: &dyn ExecutorFileSystem, root: &PathUri, + options: SkillDiscoveryOptions, ) -> SkillDiscovery { let empty_discovery = || SkillDiscovery { skills: Vec::new(), @@ -51,8 +67,14 @@ pub(super) async fn discover_skills( max_depth: MAX_SCAN_DEPTH, max_directories: MAX_SKILLS_DIRS_PER_ROOT, max_entries: MAX_SKILLS_ENTRIES_PER_ROOT, - follow_directory_symlinks: true, - prune_hidden_directories: false, + follow_directory_symlinks: matches!( + options.directory_symlinks, + DirectorySymlinkPolicy::Follow + ), + prune_hidden_directories: matches!( + options.hidden_directories, + HiddenDirectoryPolicy::Skip + ), }, /*sandbox*/ None, ) @@ -86,11 +108,15 @@ pub(super) async fn discover_skills( )); } + 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 @@ -138,6 +164,20 @@ pub(super) async fn discover_skills( } } +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, diff --git a/codex-rs/ext/skills/src/loader/environment.rs b/codex-rs/ext/skills/src/loader/environment.rs index dcf3ac15ce..1100ddc97e 100644 --- a/codex-rs/ext/skills/src/loader/environment.rs +++ b/codex-rs/ext/skills/src/loader/environment.rs @@ -13,8 +13,11 @@ use codex_utils_path_uri::PathUri; use futures::StreamExt; use super::MAX_QUALIFIED_NAME_LEN; +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::metadata::SkillMetadataFile; @@ -111,7 +114,15 @@ pub async fn load_environment_skills_from_root( let mut outcome = EnvironmentSkillLoadOutcome::default(); // Preserve environment discovery behavior by following directory aliases and including // hidden directories exposed by the executor. - let discovery = discover_skills(file_system, root).await; + let discovery = discover_skills( + file_system, + root, + SkillDiscoveryOptions { + directory_symlinks: DirectorySymlinkPolicy::Follow, + hidden_directories: HiddenDirectoryPolicy::Include, + }, + ) + .await; tracing::Span::current().record("skill_count", discovery.skills.len()); outcome.warnings.extend(discovery.warnings); if discovery.skills.is_empty() { diff --git a/codex-rs/ext/skills/src/loader/host.rs b/codex-rs/ext/skills/src/loader/host.rs new file mode 100644 index 0000000000..c76c3a9ecf --- /dev/null +++ b/codex-rs/ext/skills/src/loader/host.rs @@ -0,0 +1,251 @@ +use std::sync::Arc; + +use codex_exec_server::ExecutorFileSystem; +use codex_protocol::protocol::SkillScope; +use codex_skills::ParsedSkillFrontmatter; +use codex_skills::SkillMetadata; +use codex_skills::parse_skill_frontmatter_metadata; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; +use futures::StreamExt; +use tracing::error; + +use super::MAX_QUALIFIED_NAME_LEN; +use super::SKILLS_METADATA_DIR; +use super::SKILLS_METADATA_FILENAME; +use super::discovery::DirectorySymlinkPolicy; +use super::discovery::DiscoveredSkill; +use super::discovery::HiddenDirectoryPolicy; +use super::discovery::MAX_CONCURRENT_SKILL_LOADS; +use super::discovery::SkillDiscovery; +use super::discovery::SkillDiscoveryOptions; +use super::discovery::SkillMetadataDiscovery; +use super::discovery::discover_skills; +use super::metadata::LoadedSkillMetadata; +use super::metadata::load_host_skill_metadata; +use super::metadata::sanitize_single_line; +use super::metadata::validate_len; +use super::namespace::SkillNamespaceResolver; + +/// A resolved host skill root ready for filesystem discovery. +pub struct HostSkillRoot { + pub path: AbsolutePathBuf, + pub scope: SkillScope, + pub file_system: Arc, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct HostSkillError { + pub path: AbsolutePathBuf, + pub message: String, +} + +/// Skills and errors loaded from one canonical host root. +#[derive(Clone)] +pub struct HostSkillRootSnapshot { + pub root: AbsolutePathBuf, + pub skills: Vec, + pub errors: Vec, + pub file_system: Arc, +} + +struct ResolvedDiscoveredSkill { + skill: DiscoveredSkill, + path: AbsolutePathBuf, + path_uri: PathUri, +} + +pub async fn load_host_skill_root(root: HostSkillRoot) -> HostSkillRootSnapshot { + let canonical_root = + canonicalize_for_skill_identity(root.file_system.as_ref(), &root.path).await; + let (skills, errors) = load_skills_under_root(&root, &canonical_root).await; + HostSkillRootSnapshot { + root: canonical_root, + skills, + errors, + file_system: root.file_system, + } +} + +async fn load_skills_under_root( + skill_root: &HostSkillRoot, + root: &AbsolutePathBuf, +) -> (Vec, Vec) { + let file_system = skill_root.file_system.as_ref(); + 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( + file_system, + &PathUri::from_abs_path(root), + SkillDiscoveryOptions { + directory_symlinks, + hidden_directories: HiddenDirectoryPolicy::Skip, + }, + ) + .await; + for warning in warnings { + error!("{warning}"); + } + if skills.is_empty() { + return (Vec::new(), Vec::new()); + } + + let root_uri = PathUri::from_abs_path(root); + let resolved_skills = futures::stream::iter(skills) + .map(|skill| async move { + let path_uri = file_system + .canonicalize(&skill.path, /*sandbox*/ None) + .await + .unwrap_or_else(|_| skill.path.clone()); + let path = match path_uri.to_abs_path() { + Ok(path) => path, + Err(error) => { + error!("failed to convert discovered skill path {path_uri}: {error}"); + return None; + } + }; + Some(ResolvedDiscoveredSkill { + skill, + path, + path_uri, + }) + }) + .buffered(MAX_CONCURRENT_SKILL_LOADS) + .filter_map(futures::future::ready) + .collect::>() + .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::>(); + let namespace_resolver = SkillNamespaceResolver::discover( + file_system, + &root_uri, + &skill_paths, + plugin_roots, + namespace_roots, + ); + let skill_results = futures::stream::iter(resolved_skills) + .map(|skill| async move { + let result = parse_skill_file( + file_system, + &skill.skill, + &skill.path, + &skill.path_uri, + skill_root.scope, + ) + .await; + (skill.path, skill.path_uri, result) + }) + .buffered(MAX_CONCURRENT_SKILL_LOADS) + .collect::>(); + let (namespace_resolver, skill_results) = tokio::join!(namespace_resolver, skill_results); + + let mut loaded_skills = Vec::new(); + let mut errors = Vec::new(); + for (path, path_uri, 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(|error| error.to_string())?; + Ok(skill) + }); + match result { + Ok(skill) => loaded_skills.push(skill), + Err(message) if skill_root.scope != SkillScope::System => { + errors.push(HostSkillError { path, message }); + } + Err(_) => {} + } + } + (loaded_skills, errors) +} + +async fn parse_skill_file( + file_system: &dyn ExecutorFileSystem, + skill: &DiscoveredSkill, + path: &AbsolutePathBuf, + path_uri: &PathUri, + scope: SkillScope, +) -> Result { + 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!( + file_system.read_file_text(path_uri, /*sandbox*/ None), + load_host_skill_metadata(file_system, &metadata), + ); + let contents = contents.map_err(|error| format!("failed to read file: {error}"))?; + let ParsedSkillFrontmatter { + name, + description, + short_description, + } = parse_skill_frontmatter_metadata(&contents, || default_skill_name(path)) + .map_err(|error| error.to_string())?; + let LoadedSkillMetadata { + dependencies, + policy, + } = loaded_metadata; + + Ok(SkillMetadata { + name, + description, + short_description, + interface: None, + dependencies, + policy, + path_to_skills_md: path.clone(), + scope, + plugin_id: None, + remote_plugin_id: None, + }) +} + +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 canonicalize_for_skill_identity( + file_system: &dyn ExecutorFileSystem, + path: &AbsolutePathBuf, +) -> AbsolutePathBuf { + let path_uri = PathUri::from_abs_path(path); + file_system + .canonicalize(&path_uri, /*sandbox*/ None) + .await + .and_then(|path| path.to_abs_path()) + .unwrap_or_else(|_| path.clone()) +} + +#[cfg(test)] +#[path = "host_tests.rs"] +mod tests; diff --git a/codex-rs/ext/skills/src/loader/host_tests.rs b/codex-rs/ext/skills/src/loader/host_tests.rs new file mode 100644 index 0000000000..047a149fc1 --- /dev/null +++ b/codex-rs/ext/skills/src/loader/host_tests.rs @@ -0,0 +1,193 @@ +use std::fs; +use std::sync::Arc; + +use codex_exec_server::LOCAL_FS; +use codex_protocol::protocol::Product; +use codex_protocol::protocol::SkillScope; +use codex_skills::SkillDependencies; +use codex_skills::SkillMetadata; +use codex_skills::SkillPolicy; +use codex_skills::SkillToolDependency; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; +use tempfile::TempDir; + +use super::HostSkillRoot; +use super::load_host_skill_root; + +fn write_skill(root: &TempDir, directory: &str, frontmatter: &str) -> AbsolutePathBuf { + let skill_dir = root.path().join(directory); + fs::create_dir_all(&skill_dir).expect("create skill directory"); + let skill_path = skill_dir.join("SKILL.md"); + fs::write( + &skill_path, + format!("---\n{frontmatter}\n---\n\n# Instructions\n"), + ) + .expect("write skill"); + AbsolutePathBuf::from_absolute_path(fs::canonicalize(skill_path).expect("canonical skill path")) + .expect("absolute skill path") +} + +fn write_metadata(root: &TempDir, directory: &str, contents: &str) { + let metadata_dir = root.path().join(directory).join("agents"); + fs::create_dir_all(&metadata_dir).expect("create metadata directory"); + fs::write(metadata_dir.join("openai.yaml"), contents).expect("write metadata"); +} + +fn root_for(temp_dir: &TempDir, scope: SkillScope) -> HostSkillRoot { + HostSkillRoot { + path: AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute root"), + scope, + file_system: Arc::clone(&LOCAL_FS), + } +} + +#[tokio::test] +async fn loads_host_frontmatter_dependencies_and_policy() { + let root = TempDir::new().expect("temp dir"); + let skill_path = write_skill( + &root, + "demo", + "name: demo\ndescription: Demo skill\nmetadata:\n short-description: Short demo", + ); + write_metadata( + &root, + "demo", + r##"dependencies: + tools: + - type: mcp + value: demo-tool + description: Demo tool +policy: + allow_implicit_invocation: false + products: [codex] +"##, + ); + + let snapshot = load_host_skill_root(root_for(&root, SkillScope::User)).await; + + assert_eq!(snapshot.errors, Vec::new()); + assert_eq!( + snapshot.skills, + vec![SkillMetadata { + name: "demo".to_string(), + description: "Demo skill".to_string(), + short_description: Some("Short demo".to_string()), + interface: None, + dependencies: Some(SkillDependencies { + tools: vec![SkillToolDependency { + r#type: "mcp".to_string(), + value: "demo-tool".to_string(), + description: Some("Demo tool".to_string()), + transport: None, + command: None, + url: None, + }], + }), + policy: Some(SkillPolicy { + allow_implicit_invocation: Some(false), + products: vec![Product::Codex], + }), + path_to_skills_md: skill_path, + scope: SkillScope::User, + plugin_id: None, + remote_plugin_id: None, + }] + ); +} + +#[tokio::test] +async fn invalid_optional_metadata_fails_open() { + let root = TempDir::new().expect("temp dir"); + let skill_path = write_skill(&root, "demo", "name: demo\ndescription: Demo skill"); + write_metadata(&root, "demo", "interface: [not-an-interface]"); + + let snapshot = load_host_skill_root(root_for(&root, SkillScope::Repo)).await; + + assert_eq!(snapshot.errors, Vec::new()); + assert_eq!( + snapshot.skills, + vec![SkillMetadata { + name: "demo".to_string(), + description: "Demo skill".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: skill_path, + scope: SkillScope::Repo, + plugin_id: None, + remote_plugin_id: None, + }] + ); +} + +#[tokio::test] +async fn skips_hidden_host_skills() { + let root = TempDir::new().expect("temp dir"); + let visible_path = write_skill( + &root, + "visible", + "name: visible\ndescription: Visible skill", + ); + write_skill(&root, ".hidden", "name: hidden\ndescription: Hidden skill"); + + let snapshot = load_host_skill_root(root_for(&root, SkillScope::User)).await; + + assert_eq!(snapshot.errors, Vec::new()); + assert_eq!(snapshot.skills.len(), 1); + assert_eq!(snapshot.skills[0].path_to_skills_md, visible_path); +} + +#[tokio::test] +async fn discovers_nested_plugin_namespace_without_plugin_identity() { + let root = TempDir::new().expect("temp dir"); + let plugin_manifest = root.path().join("nested/.codex-plugin/plugin.json"); + fs::create_dir_all(plugin_manifest.parent().expect("plugin manifest parent")) + .expect("create plugin manifest directory"); + fs::write(&plugin_manifest, r#"{"name":"plugin-name"}"#).expect("write plugin manifest"); + let skill_path = write_skill( + &root, + "nested/skills/demo", + "name: demo\ndescription: Demo skill", + ); + + let snapshot = load_host_skill_root(root_for(&root, SkillScope::User)).await; + + assert_eq!(snapshot.errors, Vec::new()); + assert_eq!( + snapshot.skills, + vec![SkillMetadata { + name: "plugin-name:demo".to_string(), + description: "Demo skill".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: skill_path, + scope: SkillScope::User, + plugin_id: None, + remote_plugin_id: None, + }] + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn follows_directory_symlinks_for_user_but_not_system_scope() { + use std::os::unix::fs::symlink; + + let root = TempDir::new().expect("temp dir"); + let target = TempDir::new().expect("target temp dir"); + 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; + + 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!(system_snapshot.errors, Vec::new()); + assert_eq!(system_snapshot.skills, Vec::new()); +} diff --git a/codex-rs/ext/skills/src/loader/metadata.rs b/codex-rs/ext/skills/src/loader/metadata.rs index e3537cfdd1..5c9c15f12f 100644 --- a/codex-rs/ext/skills/src/loader/metadata.rs +++ b/codex-rs/ext/skills/src/loader/metadata.rs @@ -1,5 +1,7 @@ +use std::io; use std::path::PathBuf; +use codex_exec_server::ExecutorFileSystem; use codex_protocol::protocol::Product; use codex_skills::SkillDependencies; use codex_skills::SkillParseError; @@ -13,6 +15,8 @@ use super::MAX_DEPENDENCY_TRANSPORT_LEN; use super::MAX_DEPENDENCY_TYPE_LEN; use super::MAX_DEPENDENCY_URL_LEN; use super::MAX_DEPENDENCY_VALUE_LEN; +use super::SKILLS_METADATA_FILENAME; +use super::discovery::SkillMetadataDiscovery; #[derive(Debug, Default, Deserialize)] pub(super) struct SkillMetadataFile { @@ -42,6 +46,12 @@ struct Interface { _default_prompt: Option, } +#[derive(Default)] +pub(super) struct LoadedSkillMetadata { + pub(super) dependencies: Option, + pub(super) policy: Option, +} + #[derive(Debug, Default, Deserialize)] pub(super) struct Dependencies { #[serde(default)] @@ -67,6 +77,68 @@ struct DependencyTool { url: Option, } +pub(super) async fn load_host_skill_metadata( + file_system: &dyn ExecutorFileSystem, + metadata: &SkillMetadataDiscovery, +) -> LoadedSkillMetadata { + // Fail open: optional metadata should not block loading SKILL.md. + let metadata_path = match metadata { + SkillMetadataDiscovery::Present(path) => path, + SkillMetadataDiscovery::Absent => return LoadedSkillMetadata::default(), + SkillMetadataDiscovery::Probe(path) => { + match file_system.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}", + label = SKILLS_METADATA_FILENAME + ); + return LoadedSkillMetadata::default(); + } + } + path + } + }; + + 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 {label}: {error}", + label = SKILLS_METADATA_FILENAME + ); + return LoadedSkillMetadata::default(); + } + }; + + let parsed: SkillMetadataFile = match serde_yaml::from_str(&contents) { + Ok(parsed) => parsed, + Err(error) => { + tracing::warn!( + "ignoring {metadata_path}: invalid {label}: {error}", + label = SKILLS_METADATA_FILENAME + ); + return LoadedSkillMetadata::default(); + } + }; + + let SkillMetadataFile { + _interface: _, + dependencies, + policy, + } = parsed; + LoadedSkillMetadata { + dependencies: resolve_dependencies(dependencies), + policy: resolve_policy(policy), + } +} pub(super) fn resolve_dependencies( dependencies: Option, ) -> Option { diff --git a/codex-rs/ext/skills/src/loader/mod.rs b/codex-rs/ext/skills/src/loader/mod.rs index f2e7ae7a1c..148fbffd96 100644 --- a/codex-rs/ext/skills/src/loader/mod.rs +++ b/codex-rs/ext/skills/src/loader/mod.rs @@ -1,5 +1,6 @@ mod discovery; mod environment; +mod host; mod metadata; mod namespace; diff --git a/codex-rs/ext/skills/src/loader/namespace.rs b/codex-rs/ext/skills/src/loader/namespace.rs index 3027a235bf..bd323f921d 100644 --- a/codex-rs/ext/skills/src/loader/namespace.rs +++ b/codex-rs/ext/skills/src/loader/namespace.rs @@ -18,8 +18,10 @@ use super::discovery::MAX_CONCURRENT_SKILL_LOADS; /// ancestor manifest probes for sibling skills. This resolver resolves relevant /// roots once per scan, then selects the nearest matching root for each skill. /// -/// The deepest matching canonical symlink root or nested plugin root wins, -/// followed by the namespace inherited from the scanned skills root. +/// Namespace precedence is: +/// +/// 1. the deepest matching canonical symlink root or nested plugin root; +/// 2. the namespace inherited from the scanned skills root. pub(crate) struct SkillNamespaceResolver { inherited_namespace: ResolvedSkillNamespace, nested_namespaces: Vec<(PathUri, ResolvedSkillNamespace)>,