From bfb6a6ea226b4f2d710e09736bb08fd5d366db31 Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Thu, 6 Aug 2026 12:39:11 +0000 Subject: [PATCH] Support plugin roots in the host skill loader (#37267) ## What changed - Carry plugin identity, namespace, root, and discovery mode through host skill loading. - Apply the owning plugin namespace and IDs to loaded skill metadata. - Respect direct-child discovery for Agent Plugins and reject skills that resolve outside the plugin root or are not regular files. - Preserve recursive discovery and symlink behavior for legacy plugin roots, and allow the full 64-character namespace plus 64-character skill name. ## Testing Add host-loader coverage for plugin metadata, namespace ownership, direct-child filtering, path containment, recursive symlinks, shared assets, and maximum-length qualified names. GitOrigin-RevId: cdde821643ce39bd030d0c3753b3304b75161690 --- codex-rs/ext/skills/src/host_roots.rs | 29 +- codex-rs/ext/skills/src/host_service.rs | 11 +- codex-rs/ext/skills/src/loader/discovery.rs | 17 +- codex-rs/ext/skills/src/loader/environment.rs | 2 + codex-rs/ext/skills/src/loader/host.rs | 142 ++++++++- codex-rs/ext/skills/src/loader/host_tests.rs | 289 +++++++++++++++++- codex-rs/ext/skills/src/loader/mod.rs | 2 +- codex-rs/ext/skills/src/loader/namespace.rs | 8 + 8 files changed, 445 insertions(+), 55 deletions(-) diff --git a/codex-rs/ext/skills/src/host_roots.rs b/codex-rs/ext/skills/src/host_roots.rs index dfa4652c82..1bb0b21658 100644 --- a/codex-rs/ext/skills/src/host_roots.rs +++ b/codex-rs/ext/skills/src/host_roots.rs @@ -99,12 +99,11 @@ fn roots_from_layer_stack( match &layer.name { ConfigLayerSource::Project { .. } => { if let Some(repository_file_system) = &repository_file_system { - roots.push(HostSkillRoot { - path: config_folder.join(SKILLS_DIR_NAME), - scope: SkillScope::Repo, - file_system: Arc::clone(repository_file_system), - plugin_root: None, - }); + roots.push(HostSkillRoot::host( + config_folder.join(SKILLS_DIR_NAME), + SkillScope::Repo, + Arc::clone(repository_file_system), + )); } } ConfigLayerSource::User { .. } => { @@ -145,12 +144,7 @@ fn roots_from_layer_stack( } fn local_root(path: AbsolutePathBuf, scope: SkillScope) -> HostSkillRoot { - HostSkillRoot { - path, - scope, - file_system: Arc::clone(&LOCAL_FS), - plugin_root: None, - } + HostSkillRoot::host(path, scope, Arc::clone(&LOCAL_FS)) } fn host_root_to_skill_root(root: HostSkillRoot) -> SkillRoot { @@ -193,12 +187,11 @@ async fn repo_agents_skill_roots( .buffered(MAX_CONCURRENT_ANCESTOR_PROBES); while let Some((agents_skills, result)) = results.next().await { match result { - Ok(metadata) if metadata.is_directory => roots.push(HostSkillRoot { - path: agents_skills, - scope: SkillScope::Repo, - file_system: Arc::clone(&repository_file_system), - plugin_root: None, - }), + Ok(metadata) if metadata.is_directory => roots.push(HostSkillRoot::host( + agents_skills, + SkillScope::Repo, + Arc::clone(&repository_file_system), + )), Ok(_) => {} Err(error) if error.kind() == io::ErrorKind::NotFound => {} Err(error) => { diff --git a/codex-rs/ext/skills/src/host_service.rs b/codex-rs/ext/skills/src/host_service.rs index b759e99d33..e13f2a2389 100644 --- a/codex-rs/ext/skills/src/host_service.rs +++ b/codex-rs/ext/skills/src/host_service.rs @@ -303,12 +303,11 @@ impl HostSkillsService { let snapshot = if use_legacy_loader { load_skill_root_snapshot(root, plugin_skill_snapshots).await } else { - let snapshot = load_host_skill_root(HostSkillRoot { - path: root.path, - scope: root.scope, - file_system: root.file_system, - plugin_root: root.plugin_root, - }) + let snapshot = load_host_skill_root(HostSkillRoot::host( + root.path, + root.scope, + root.file_system, + )) .await; SkillRootSnapshot::new( snapshot.root, diff --git a/codex-rs/ext/skills/src/loader/discovery.rs b/codex-rs/ext/skills/src/loader/discovery.rs index 547400bc4f..50e06533a6 100644 --- a/codex-rs/ext/skills/src/loader/discovery.rs +++ b/codex-rs/ext/skills/src/loader/discovery.rs @@ -6,6 +6,7 @@ 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; @@ -29,6 +30,7 @@ pub(super) enum HiddenDirectoryPolicy { pub(super) struct SkillDiscoveryOptions { pub directory_symlinks: DirectorySymlinkPolicy, pub hidden_directories: HiddenDirectoryPolicy, + pub mode: SkillDiscoveryMode, } pub(super) struct SkillDiscovery { @@ -64,7 +66,10 @@ pub(super) async fn discover_skills( .walk( root, WalkOptions { - max_depth: MAX_SCAN_DEPTH, + 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!( @@ -137,7 +142,15 @@ pub(super) async fn discover_skills( } WalkEntryKind::File => { file_paths.insert(entry.path.clone()); - if entry.path.basename().as_deref() == Some(SKILLS_FILENAME) { + if entry.path.basename().as_deref() == Some(SKILLS_FILENAME) + && (options.mode == SkillDiscoveryMode::Recursive + || entry + .path + .parent() + .and_then(|parent| parent.parent()) + .as_ref() + == Some(root)) + { skill_files.push(entry.path); } } diff --git a/codex-rs/ext/skills/src/loader/environment.rs b/codex-rs/ext/skills/src/loader/environment.rs index 1100ddc97e..89f8c49b9c 100644 --- a/codex-rs/ext/skills/src/loader/environment.rs +++ b/codex-rs/ext/skills/src/loader/environment.rs @@ -10,6 +10,7 @@ use codex_skills::SkillDependencies; use codex_skills::SkillPolicy; use codex_skills::parse_skill_frontmatter_metadata; use codex_utils_path_uri::PathUri; +use codex_utils_plugins::SkillDiscoveryMode; use futures::StreamExt; use super::MAX_QUALIFIED_NAME_LEN; @@ -120,6 +121,7 @@ pub async fn load_environment_skills_from_root( SkillDiscoveryOptions { directory_symlinks: DirectorySymlinkPolicy::Follow, hidden_directories: HiddenDirectoryPolicy::Include, + mode: SkillDiscoveryMode::Recursive, }, ) .await; diff --git a/codex-rs/ext/skills/src/loader/host.rs b/codex-rs/ext/skills/src/loader/host.rs index a6c6d0c4f2..71161b970c 100644 --- a/codex-rs/ext/skills/src/loader/host.rs +++ b/codex-rs/ext/skills/src/loader/host.rs @@ -8,6 +8,10 @@ use codex_skills::SkillMetadata; use codex_skills::parse_skill_frontmatter_metadata; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; +use codex_utils_plugins::PluginIdentity; +#[cfg(test)] +use codex_utils_plugins::PluginSkillRoot; +use codex_utils_plugins::SkillDiscoveryMode; use futures::StreamExt; use tracing::error; @@ -33,7 +37,64 @@ pub struct HostSkillRoot { pub path: AbsolutePathBuf, pub scope: SkillScope, pub file_system: Arc, - pub plugin_root: Option, + plugin: Option, +} + +struct PluginSkillRootContext { + identity: PluginIdentity, + namespace: String, + root: AbsolutePathBuf, + discovery_mode: SkillDiscoveryMode, +} + +impl HostSkillRoot { + pub(crate) fn host( + path: AbsolutePathBuf, + scope: SkillScope, + file_system: Arc, + ) -> Self { + Self { + path, + scope, + file_system, + plugin: None, + } + } + + #[cfg(test)] + pub(crate) fn plugin(root: PluginSkillRoot, file_system: Arc) -> Self { + Self { + path: root.path, + scope: SkillScope::User, + file_system, + plugin: Some(PluginSkillRootContext { + identity: root.plugin_identity, + namespace: root.plugin_namespace, + root: root.plugin_root, + discovery_mode: root.discovery_mode, + }), + } + } + + pub(crate) fn plugin_identity(&self) -> Option<&PluginIdentity> { + self.plugin.as_ref().map(|plugin| &plugin.identity) + } + + pub(crate) fn plugin_namespace(&self) -> Option<&str> { + self.plugin.as_ref().map(|plugin| plugin.namespace.as_str()) + } + + pub(crate) fn plugin_root(&self) -> Option<&AbsolutePathBuf> { + self.plugin.as_ref().map(|plugin| &plugin.root) + } + + pub(crate) fn discovery_mode(&self) -> SkillDiscoveryMode { + self.plugin + .as_ref() + .map_or(SkillDiscoveryMode::Recursive, |plugin| { + plugin.discovery_mode + }) + } } #[derive(Clone, Debug, PartialEq, Eq)] @@ -81,10 +142,12 @@ async fn load_skills_under_root( Vec, ) { let file_system = skill_root.file_system.as_ref(); - let plugin_root = match skill_root.plugin_root.as_ref() { + let plugin_identity = skill_root.plugin_identity(); + let plugin_root = match skill_root.plugin_root() { Some(plugin_root) => Some(canonicalize_for_skill_identity(file_system, plugin_root).await), None => None, }; + let discovery_mode = skill_root.discovery_mode(); let directory_symlinks = match skill_root.scope { SkillScope::User | SkillScope::Repo | SkillScope::Admin => DirectorySymlinkPolicy::Follow, SkillScope::System => DirectorySymlinkPolicy::Ignore, @@ -100,6 +163,7 @@ async fn load_skills_under_root( SkillDiscoveryOptions { directory_symlinks, hidden_directories: HiddenDirectoryPolicy::Skip, + mode: discovery_mode, }, ) .await; @@ -111,12 +175,23 @@ async fn load_skills_under_root( } 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 = file_system + let path_uri = match file_system .canonicalize(&skill.path, /*sandbox*/ None) .await - .unwrap_or_else(|_| skill.path.clone()); + { + Ok(path) => path, + Err(error) if discovery_mode == SkillDiscoveryMode::DirectChildren => { + error!( + "failed to resolve Agent Plugin skill path {}: {error}", + skill.path + ); + return None; + } + Err(_) => skill.path.clone(), + }; let path = match path_uri.to_abs_path() { Ok(path) => path, Err(error) => { @@ -124,6 +199,37 @@ async fn load_skills_under_root( return None; } }; + if 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 file_system.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(error) => { + error!( + "failed to inspect Agent Plugin skill path {}: {error}", + path.display() + ); + return None; + } + } + } Some(ResolvedDiscoveredSkill { skill, path, @@ -143,13 +249,21 @@ async fn load_skills_under_root( .iter() .map(|skill| skill.path_uri.clone()) .collect::>(); - let namespace_resolver = SkillNamespaceResolver::discover( - file_system, - &root_uri, - &skill_paths, - plugin_roots, - namespace_roots, - ); + let namespace_resolver = async { + match skill_root.plugin_namespace() { + Some(namespace) => SkillNamespaceResolver::with_provided_namespace(namespace), + None => { + SkillNamespaceResolver::discover( + file_system, + &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(); @@ -165,6 +279,7 @@ async fn load_skills_under_root( &skill.path, &skill.path_uri, skill_root.scope, + plugin_identity, plugin_root, ) .await; @@ -212,6 +327,7 @@ async fn parse_skill_file( path: &AbsolutePathBuf, path_uri: &PathUri, scope: SkillScope, + plugin_identity: Option<&PluginIdentity>, plugin_root: Option<&AbsolutePathBuf>, ) -> Result { let metadata_path = path_uri @@ -250,8 +366,8 @@ async fn parse_skill_file( policy, path_to_skills_md: path.clone(), scope, - plugin_id: None, - remote_plugin_id: None, + plugin_id: plugin_identity.map(|identity| identity.plugin_id.clone()), + remote_plugin_id: plugin_identity.and_then(|identity| identity.remote_plugin_id.clone()), }) } diff --git a/codex-rs/ext/skills/src/loader/host_tests.rs b/codex-rs/ext/skills/src/loader/host_tests.rs index b382a6191f..c5d85750e4 100644 --- a/codex-rs/ext/skills/src/loader/host_tests.rs +++ b/codex-rs/ext/skills/src/loader/host_tests.rs @@ -10,9 +10,14 @@ use codex_skills::SkillMetadata; use codex_skills::SkillPolicy; use codex_skills::SkillToolDependency; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PluginIdentity; +use codex_utils_plugins::PluginSkillRoot; +use codex_utils_plugins::SkillDiscoveryMode; use pretty_assertions::assert_eq; use tempfile::TempDir; +use crate::loader::MAX_NAME_LEN; + use super::HostSkillRoot; use super::load_host_skill_root; @@ -36,12 +41,11 @@ fn write_metadata(root: &TempDir, directory: &str, contents: &str) { } fn root_for(temp_dir: &TempDir, scope: SkillScope) -> HostSkillRoot { - HostSkillRoot { - path: AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute root"), + HostSkillRoot::host( + AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute root"), scope, - file_system: Arc::clone(&LOCAL_FS), - plugin_root: None, - } + Arc::clone(&LOCAL_FS), + ) } struct PluginSkillFixture { @@ -81,12 +85,19 @@ impl PluginSkillFixture { } fn host_root(&self) -> HostSkillRoot { - HostSkillRoot { - path: self.plugin_root.join("skills"), - scope: SkillScope::User, - file_system: Arc::clone(&LOCAL_FS), - plugin_root: Some(self.plugin_root.clone()), - } + HostSkillRoot::plugin( + PluginSkillRoot { + path: self.plugin_root.join("skills"), + plugin_identity: PluginIdentity { + plugin_id: "fixture@test".to_string(), + remote_plugin_id: None, + }, + plugin_namespace: "plugin".to_string(), + plugin_root: self.plugin_root.clone(), + discovery_mode: SkillDiscoveryMode::Recursive, + }, + Arc::clone(&LOCAL_FS), + ) } } @@ -234,7 +245,7 @@ async fn loads_plugin_skill_interface_icons_from_local_and_shared_assets() { assert_eq!( snapshot.skills, vec![SkillMetadata { - name: "send-message".to_string(), + name: "plugin:send-message".to_string(), description: "Send messages".to_string(), short_description: None, interface: Some(SkillInterface { @@ -253,7 +264,7 @@ async fn loads_plugin_skill_interface_icons_from_local_and_shared_assets() { policy: None, path_to_skills_md: fixture.skill_path, scope: SkillScope::User, - plugin_id: None, + plugin_id: Some("fixture@test".to_string()), remote_plugin_id: None, }] ); @@ -276,7 +287,7 @@ async fn rejects_plugin_skill_interface_icons_outside_shared_assets() { assert_eq!( snapshot.skills, vec![SkillMetadata { - name: "send-message".to_string(), + name: "plugin:send-message".to_string(), description: "Send messages".to_string(), short_description: None, interface: Some(SkillInterface { @@ -291,7 +302,7 @@ async fn rejects_plugin_skill_interface_icons_outside_shared_assets() { policy: None, path_to_skills_md: fixture.skill_path, scope: SkillScope::User, - plugin_id: None, + plugin_id: Some("fixture@test".to_string()), remote_plugin_id: None, }] ); @@ -377,6 +388,254 @@ async fn discovers_nested_plugin_namespace_without_plugin_identity() { ); } +#[tokio::test] +async fn plugin_root_accepts_maximum_length_qualified_skill_name() { + let root = TempDir::new().expect("temp dir"); + let plugin_namespace = "p".repeat(MAX_NAME_LEN); + let skill_name = "s".repeat(MAX_NAME_LEN); + let skill_path = 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: plugin_namespace.clone(), + plugin_root, + discovery_mode: SkillDiscoveryMode::Recursive, + }, + Arc::clone(&LOCAL_FS), + )) + .await; + + assert_eq!(snapshot.errors, Vec::new()); + assert_eq!( + snapshot.skills, + vec![SkillMetadata { + name: format!("{plugin_namespace}:{skill_name}"), + description: "Search skill".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: skill_path, + scope: SkillScope::User, + plugin_id: Some("demo@test".to_string()), + remote_plugin_id: None, + }] + ); +} + +#[tokio::test] +async fn recursive_plugin_root_preserves_owner_namespace_and_shared_asset_policy() { + let root = TempDir::new().expect("temp dir"); + let skill_path = write_skill( + &root, + "skills/group/demo", + "name: demo\ndescription: Demo skill", + ); + let nested_manifest = root.path().join("skills/group/.codex-plugin/plugin.json"); + fs::create_dir_all(nested_manifest.parent().expect("nested manifest parent")) + .expect("create nested plugin manifest directory"); + fs::write(nested_manifest, r#"{"name":"conflicting-plugin"}"#) + .expect("write nested plugin manifest"); + write_metadata( + &root, + "skills/group/demo", + "interface:\n icon_small: ../../../assets/logo.svg\n", + ); + fs::create_dir_all(root.path().join("assets")).expect("create assets directory"); + fs::write(root.path().join("assets/logo.svg"), "").expect("write asset"); + let plugin_root = AbsolutePathBuf::from_absolute_path(root.path()).expect("plugin root"); + let canonical_plugin_root = AbsolutePathBuf::from_absolute_path( + fs::canonicalize(root.path()).expect("canonicalize plugin root"), + ) + .expect("canonical 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: Some("remote-demo".to_string()), + }, + plugin_namespace: "plugin".to_string(), + plugin_root, + discovery_mode: SkillDiscoveryMode::Recursive, + }, + Arc::clone(&LOCAL_FS), + )) + .await; + + assert_eq!(snapshot.errors, Vec::new()); + assert_eq!( + snapshot.skills, + vec![SkillMetadata { + name: "plugin:demo".to_string(), + description: "Demo skill".to_string(), + short_description: None, + interface: Some(SkillInterface { + display_name: None, + short_description: None, + icon_small: Some(canonical_plugin_root.join("assets/logo.svg")), + icon_large: None, + brand_color: None, + default_prompt: None, + }), + dependencies: None, + policy: None, + path_to_skills_md: skill_path.clone(), + scope: SkillScope::User, + plugin_id: Some("demo@test".to_string()), + remote_plugin_id: Some("remote-demo".to_string()), + }] + ); + assert_eq!( + snapshot.skill_discovery_path_by_path.get(&skill_path), + Some(&skill_path) + ); +} + +#[tokio::test] +async fn direct_child_plugin_root_ignores_nested_skills() { + let root = TempDir::new().expect("temp dir"); + let direct_path = write_skill( + &root, + "skills/direct", + "name: direct\ndescription: Direct skill", + ); + write_skill( + &root, + "skills/nested/too-deep", + "name: nested\ndescription: Nested 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: "plugin".to_string(), + plugin_root, + discovery_mode: SkillDiscoveryMode::DirectChildren, + }, + Arc::clone(&LOCAL_FS), + )) + .await; + + assert_eq!(snapshot.errors, Vec::new()); + assert_eq!( + snapshot.skills, + vec![SkillMetadata { + name: "plugin:direct".to_string(), + description: "Direct skill".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: direct_path, + scope: SkillScope::User, + plugin_id: Some("demo@test".to_string()), + remote_plugin_id: None, + }] + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn direct_child_plugin_root_skips_skills_resolving_outside_plugin_root() { + let root = TempDir::new().expect("temp dir"); + let outside_root = TempDir::new().expect("outside temp dir"); + write_skill( + &outside_root, + "escaped", + "name: escaped\ndescription: Escaped skill", + ); + fs::create_dir_all(root.path().join("skills")).expect("create skills root"); + std::os::unix::fs::symlink( + outside_root.path().join("escaped"), + root.path().join("skills/escaped"), + ) + .expect("create skill symlink"); + 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: "plugin".to_string(), + plugin_root, + discovery_mode: SkillDiscoveryMode::DirectChildren, + }, + Arc::clone(&LOCAL_FS), + )) + .await; + + assert_eq!(snapshot.errors, Vec::new()); + assert_eq!(snapshot.skills, Vec::new()); +} + +#[cfg(unix)] +#[tokio::test] +async fn recursive_plugin_root_preserves_symlinked_skill_discovery_path() { + 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"); + fs::create_dir_all(root.path().join("skills")).expect("create skills root"); + std::os::unix::fs::symlink(target.path().join("demo"), root.path().join("skills/alias")) + .expect("create skill symlink"); + 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: "plugin".to_string(), + plugin_root, + discovery_mode: SkillDiscoveryMode::Recursive, + }, + Arc::clone(&LOCAL_FS), + )) + .await; + + assert_eq!(snapshot.errors, Vec::new()); + assert_eq!( + snapshot.skills, + vec![SkillMetadata { + name: "plugin:demo".to_string(), + description: "Symlinked skill".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: target_skill.clone(), + scope: SkillScope::User, + plugin_id: Some("demo@test".to_string()), + remote_plugin_id: None, + }] + ); + assert_eq!( + snapshot.skill_discovery_path_by_path.get(&target_skill), + Some(&snapshot.root.join("alias/SKILL.md")) + ); +} + #[cfg(unix)] #[tokio::test] async fn follows_directory_symlinks_for_user_but_not_system_scope() { diff --git a/codex-rs/ext/skills/src/loader/mod.rs b/codex-rs/ext/skills/src/loader/mod.rs index 83f2f8d699..5083c5db28 100644 --- a/codex-rs/ext/skills/src/loader/mod.rs +++ b/codex-rs/ext/skills/src/loader/mod.rs @@ -13,7 +13,7 @@ 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"; pub(super) const MAX_NAME_LEN: usize = 64; -pub(super) const MAX_QUALIFIED_NAME_LEN: usize = 128; +pub(super) const MAX_QUALIFIED_NAME_LEN: usize = MAX_NAME_LEN * 2 + 1; pub(super) const MAX_DESCRIPTION_LEN: usize = 1024; pub(super) const MAX_DEPENDENCY_TYPE_LEN: usize = MAX_NAME_LEN; pub(super) const MAX_DEPENDENCY_TRANSPORT_LEN: usize = MAX_NAME_LEN; diff --git a/codex-rs/ext/skills/src/loader/namespace.rs b/codex-rs/ext/skills/src/loader/namespace.rs index bd323f921d..687238138b 100644 --- a/codex-rs/ext/skills/src/loader/namespace.rs +++ b/codex-rs/ext/skills/src/loader/namespace.rs @@ -28,6 +28,14 @@ pub(crate) struct SkillNamespaceResolver { } impl SkillNamespaceResolver { + /// Uses the authoritative namespace supplied by an owning plugin. + 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,