diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 846487aba6f8..029be960b693 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1,6 +1,7 @@ use crate::app_mcp_routing::apply_app_mcp_routing_policy; use crate::app_mcp_routing::apps_route_available; use crate::is_openai_curated_marketplace_name; +use crate::manifest::PluginManifest; use crate::manifest::PluginManifestHooks; use crate::manifest::PluginManifestMcpServers; use crate::manifest::PluginManifestPaths; @@ -664,6 +665,7 @@ async fn load_plugin( let mut loaded_plugin = LoadedPlugin { config_name, manifest_name: None, + plugin_namespace: None, manifest_description: None, root, enabled: plugin.enabled, @@ -706,6 +708,7 @@ async fn load_plugin( }; let manifest_paths = &manifest.paths; + loaded_plugin.plugin_namespace = Some(manifest.name.clone()); match scope { PluginLoadScope::AllCapabilities { restriction_product, @@ -717,7 +720,7 @@ async fn load_plugin( let resolved_skills = load_plugin_skills( &plugin_root, &loaded_plugin_id, - manifest_paths, + &manifest, *restriction_product, skill_config_rules, ) @@ -785,17 +788,18 @@ impl ResolvedPluginSkills { pub async fn load_plugin_skills( plugin_root: &AbsolutePathBuf, plugin_id: &PluginId, - manifest_paths: &PluginManifestPaths, + manifest: &PluginManifest, restriction_product: Option, skill_config_rules: &SkillConfigRules, ) -> ResolvedPluginSkills { - let roots = plugin_skill_roots(plugin_root, manifest_paths) + let roots = plugin_skill_roots(plugin_root, &manifest.paths) .into_iter() .map(|path| SkillRoot { path, scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: Some(plugin_id.as_key()), + plugin_namespace: Some(manifest.name.clone()), plugin_root: Some(plugin_root.clone()), }) .collect::>(); diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 04f2ff613246..b1bb508eebd7 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -1591,7 +1591,7 @@ impl PluginsManager { let resolved_skills = load_plugin_skills( &source_path, &plugin_id, - &manifest.paths, + &manifest, self.restriction_product, &codex_core_skills::config_rules::skill_config_rules_from_stack( &config.config_layer_stack, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index b5d22911f9db..f5073f68b04a 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -623,6 +623,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { vec![LoadedPlugin { config_name: "sample@test".to_string(), manifest_name: Some("sample".to_string()), + plugin_namespace: Some("sample".to_string()), manifest_description: Some( "Plugin that includes the sample MCP server and Skills".to_string(), ), @@ -1446,23 +1447,30 @@ async fn load_plugin_skills_dedupes_overlapping_manifest_roots() { &plugin_root.join("skills/edk/SKILL.md"), "---\nname: edk\ndescription: edk skill\n---\n", ); - let manifest_paths = crate::manifest::PluginManifestPaths { - skills: vec![ - plugin_root.join("skills"), - plugin_root.join("skills/abc"), - plugin_root.join("skills/edk"), - plugin_root.join("skills/abc"), - ], - mcp_servers: None, - apps: None, - hooks: None, + let manifest = crate::manifest::PluginManifest { + name: "sample".to_string(), + version: None, + description: None, + keywords: Vec::new(), + paths: crate::manifest::PluginManifestPaths { + skills: vec![ + plugin_root.join("skills"), + plugin_root.join("skills/abc"), + plugin_root.join("skills/edk"), + plugin_root.join("skills/abc"), + ], + mcp_servers: None, + apps: None, + hooks: None, + }, + interface: None, }; let plugin_id = PluginId::parse("sample@test").expect("plugin id should parse"); let resolved = load_plugin_skills( &plugin_root, &plugin_id, - &manifest_paths, + &manifest, /*restriction_product*/ None, &SkillConfigRules::default(), ) @@ -1677,6 +1685,7 @@ async fn load_plugins_preserves_disabled_plugins_without_effective_contributions vec![LoadedPlugin { config_name: "sample@test".to_string(), manifest_name: None, + plugin_namespace: None, manifest_description: None, root: AbsolutePathBuf::try_from(plugin_root).unwrap(), enabled: false, @@ -1845,6 +1854,12 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { let plugin = |config_name: &str, dir_name: &str, manifest_name: &str| LoadedPlugin { config_name: config_name.to_string(), manifest_name: Some(manifest_name.to_string()), + plugin_namespace: Some( + config_name + .split_once('@') + .map_or(config_name, |(name, _)| name) + .to_string(), + ), manifest_description: None, root: AbsolutePathBuf::try_from(codex_home.path().join(dir_name)).unwrap(), enabled: true, diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index e2877349db70..64861c45ebd0 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -157,6 +157,7 @@ pub struct SkillRoot { pub scope: SkillScope, pub file_system: Arc, pub plugin_id: Option, + pub plugin_namespace: Option, pub plugin_root: Option, } @@ -178,6 +179,7 @@ where &root_path, root.scope, root.plugin_id.as_deref(), + root.plugin_namespace.as_deref(), root.plugin_root.as_ref(), &mut outcome, ) @@ -266,6 +268,7 @@ async fn skill_roots_with_home_dir( scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: Some(root.plugin_id), + plugin_namespace: Some(root.plugin_namespace), plugin_root: Some(root.plugin_root), })); roots.extend(extra_skill_roots.into_iter().map(|path| SkillRoot { @@ -273,6 +276,7 @@ async fn skill_roots_with_home_dir( scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: None, + plugin_namespace: None, plugin_root: None, })); roots.extend(repo_agents_skill_roots(fs, config_layer_stack, cwd).await); @@ -303,6 +307,7 @@ fn skill_roots_from_layer_stack_inner( scope: SkillScope::Repo, file_system: Arc::clone(repo_fs), plugin_id: None, + plugin_namespace: None, plugin_root: None, }); } @@ -315,6 +320,7 @@ fn skill_roots_from_layer_stack_inner( scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: None, + plugin_namespace: None, plugin_root: None, }); @@ -325,6 +331,7 @@ fn skill_roots_from_layer_stack_inner( scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: None, + plugin_namespace: None, plugin_root: None, }); } @@ -336,6 +343,7 @@ fn skill_roots_from_layer_stack_inner( scope: SkillScope::System, file_system: Arc::clone(&LOCAL_FS), plugin_id: None, + plugin_namespace: None, plugin_root: None, }); } @@ -347,6 +355,7 @@ fn skill_roots_from_layer_stack_inner( scope: SkillScope::Admin, file_system: Arc::clone(&LOCAL_FS), plugin_id: None, + plugin_namespace: None, plugin_root: None, }); } @@ -382,6 +391,7 @@ async fn repo_agents_skill_roots( scope: SkillScope::Repo, file_system: Arc::clone(&fs), plugin_id: None, + plugin_namespace: None, plugin_root: None, }), Ok(_) => {} @@ -490,6 +500,7 @@ async fn discover_skills_under_root( root: &AbsolutePathBuf, scope: SkillScope, plugin_id: Option<&str>, + plugin_namespace: Option<&str>, plugin_root: Option<&AbsolutePathBuf>, outcome: &mut SkillLoadOutcome, ) { @@ -610,7 +621,16 @@ async fn discover_skills_under_root( } if metadata.is_file && file_name == SKILLS_FILENAME { - match parse_skill_file(fs, &path, scope, plugin_id, plugin_root.as_ref()).await { + match parse_skill_file( + fs, + &path, + scope, + plugin_id, + plugin_namespace, + plugin_root.as_ref(), + ) + .await + { Ok(skill) => { outcome.skills.push(skill); } @@ -641,6 +661,7 @@ async fn parse_skill_file( path: &AbsolutePathBuf, scope: SkillScope, plugin_id: Option<&str>, + plugin_namespace: Option<&str>, plugin_root: Option<&AbsolutePathBuf>, ) -> Result { let path_uri = PathUri::from_abs_path(path); @@ -671,7 +692,7 @@ async fn parse_skill_file( .map(sanitize_single_line) .filter(|value| !value.is_empty()) .unwrap_or_else(|| default_skill_name(path)); - let name = namespaced_skill_name(fs, path, &base_name).await; + let name = namespaced_skill_name(fs, path, &base_name, plugin_namespace).await; let description = parsed .description .as_deref() @@ -731,7 +752,11 @@ async fn namespaced_skill_name( fs: &dyn ExecutorFileSystem, path: &AbsolutePathBuf, base_name: &str, + plugin_namespace: Option<&str>, ) -> String { + if let Some(plugin_namespace) = plugin_namespace { + return format!("{plugin_namespace}:{base_name}"); + } plugin_namespace_for_skill_path(fs, path) .await .map(|namespace| format!("{namespace}:{base_name}")) diff --git a/codex-rs/core-skills/src/loader_tests.rs b/codex-rs/core-skills/src/loader_tests.rs index cc1d8c8e049a..3a6ae9b1c4e9 100644 --- a/codex-rs/core-skills/src/loader_tests.rs +++ b/codex-rs/core-skills/src/loader_tests.rs @@ -850,6 +850,7 @@ interface: scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: Some("twilio-developer-kit@test".to_string()), + plugin_namespace: None, plugin_root: Some(plugin_root_abs.clone()), }]) .await; @@ -907,6 +908,7 @@ interface: scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: Some("twilio-developer-kit@test".to_string()), + plugin_namespace: None, plugin_root: Some(plugin_root.abs()), }]) .await; @@ -1053,6 +1055,7 @@ async fn loads_skills_via_symlinked_subdir_for_admin_scope() { scope: SkillScope::Admin, file_system: Arc::clone(&LOCAL_FS), plugin_id: None, + plugin_namespace: None, plugin_root: None, }]) .await; @@ -1135,6 +1138,7 @@ async fn system_scope_ignores_symlinked_subdir() { scope: SkillScope::System, file_system: Arc::clone(&LOCAL_FS), plugin_id: None, + plugin_namespace: None, plugin_root: None, }]) .await; @@ -1169,6 +1173,7 @@ async fn respects_max_scan_depth_for_user_scope() { scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: None, + plugin_namespace: None, plugin_root: None, }]) .await; @@ -1256,7 +1261,7 @@ async fn falls_back_to_directory_name_when_skill_name_is_missing() { } #[tokio::test] -async fn namespaces_plugin_skills_using_plugin_name() { +async fn namespaces_plugin_skills_using_provided_namespace() { let root = tempfile::tempdir().expect("tempdir"); let plugin_root = root.path().join("plugins/sample"); let skill_path = write_raw_skill_at( @@ -1267,7 +1272,7 @@ async fn namespaces_plugin_skills_using_plugin_name() { fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap(); fs::write( plugin_root.join(".codex-plugin/plugin.json"), - r#"{"name":"sample"}"#, + r#"{"name":"should-not-be-read"}"#, ) .unwrap(); @@ -1276,6 +1281,7 @@ async fn namespaces_plugin_skills_using_plugin_name() { scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: Some("sample@test".to_string()), + plugin_namespace: Some("sample".to_string()), plugin_root: Some(plugin_root.abs()), }]) .await; @@ -1321,6 +1327,7 @@ async fn plugin_skill_name_length_limit_allows_max_qualified_name() { scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: Some("sample@test".to_string()), + plugin_namespace: Some(plugin_name.clone()), plugin_root: Some(plugin_root.abs()), }]) .await; @@ -1366,6 +1373,7 @@ async fn plugin_skill_name_length_limit_rejects_overlong_qualified_name() { scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: Some("sample@test".to_string()), + plugin_namespace: Some(plugin_name.clone()), plugin_root: Some(plugin_root.abs()), }]) .await; @@ -1805,6 +1813,7 @@ async fn deduplicates_by_path_preferring_first_root() { scope: SkillScope::Repo, file_system: Arc::clone(&LOCAL_FS), plugin_id: None, + plugin_namespace: None, plugin_root: None, }, SkillRoot { @@ -1812,6 +1821,7 @@ async fn deduplicates_by_path_preferring_first_root() { scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), plugin_id: None, + plugin_namespace: None, plugin_root: None, }, ]) diff --git a/codex-rs/core-skills/src/service.rs b/codex-rs/core-skills/src/service.rs index d20f5e39e4de..a6a1e30f57fd 100644 --- a/codex-rs/core-skills/src/service.rs +++ b/codex-rs/core-skills/src/service.rs @@ -256,7 +256,7 @@ impl SkillsService { #[derive(Debug, Clone, PartialEq, Eq, Hash)] struct ConfigSkillsCacheKey { - roots: Vec<(AbsolutePathBuf, u8, Option)>, + roots: Vec<(AbsolutePathBuf, u8, Option, Option)>, skill_config_rules: SkillConfigRules, } @@ -296,7 +296,12 @@ fn config_skills_cache_key( SkillScope::System => 2, SkillScope::Admin => 3, }; - (root.path.clone(), scope_rank, root.plugin_id.clone()) + ( + root.path.clone(), + scope_rank, + root.plugin_id.clone(), + root.plugin_namespace.clone(), + ) }) .collect(), skill_config_rules: skill_config_rules.clone(), diff --git a/codex-rs/core-skills/src/service_tests.rs b/codex-rs/core-skills/src/service_tests.rs index bc05ebf35084..23a15f79550b 100644 --- a/codex-rs/core-skills/src/service_tests.rs +++ b/codex-rs/core-skills/src/service_tests.rs @@ -55,7 +55,11 @@ fn write_plugin_skill( skill_path } -fn plugin_skill_root_for_skill_path(skill_path: &Path, plugin_id: &str) -> PluginSkillRoot { +fn plugin_skill_root_for_skill_path( + skill_path: &Path, + plugin_id: &str, + plugin_namespace: &str, +) -> PluginSkillRoot { let skills_root = skill_path .parent() .and_then(Path::parent) @@ -66,6 +70,7 @@ fn plugin_skill_root_for_skill_path(skill_path: &Path, plugin_id: &str) -> Plugi PluginSkillRoot { path: skills_root.abs(), plugin_id: plugin_id.to_string(), + plugin_namespace: plugin_namespace.to_string(), plugin_root: plugin_root.abs(), } } @@ -364,7 +369,8 @@ async fn skills_for_config_disables_plugin_skills_by_name() { &codex_home, &name_toggle_config("sample:sample-search", /*enabled*/ false), ); - let plugin_skill_root = plugin_skill_root_for_skill_path(&skill_path, "test-plugin@test"); + let plugin_skill_root = + plugin_skill_root_for_skill_path(&skill_path, "test-plugin@test", "sample"); let skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, diff --git a/codex-rs/ext/skills/src/provider/executor.rs b/codex-rs/ext/skills/src/provider/executor.rs index 2d26ae8bed00..8da0265d0925 100644 --- a/codex-rs/ext/skills/src/provider/executor.rs +++ b/codex-rs/ext/skills/src/provider/executor.rs @@ -81,6 +81,7 @@ impl SkillProvider for ExecutorSkillProvider { scope: SkillScope::User, file_system: Arc::clone(&file_system), plugin_id: None, + plugin_namespace: None, plugin_root: None, }]) .await, diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index 1eff53c3b93c..bc2a87a74016 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -196,6 +196,7 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() { canonical_root: canonical_root.clone(), }), plugin_id: None, + plugin_namespace: None, plugin_root: None, }]) .await; diff --git a/codex-rs/plugin/src/load_outcome.rs b/codex-rs/plugin/src/load_outcome.rs index 95b98d863a4c..ad83655463bc 100644 --- a/codex-rs/plugin/src/load_outcome.rs +++ b/codex-rs/plugin/src/load_outcome.rs @@ -17,6 +17,7 @@ const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024; pub struct LoadedPlugin { pub config_name: String, pub manifest_name: Option, + pub plugin_namespace: Option, pub manifest_description: Option, pub root: AbsolutePathBuf, pub enabled: bool, @@ -126,11 +127,15 @@ impl PluginLoadOutcome { let mut skill_roots = Vec::new(); let mut seen_paths = HashSet::new(); for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) { + let Some(plugin_namespace) = &plugin.plugin_namespace else { + continue; + }; for path in &plugin.skill_roots { if seen_paths.insert(path.clone()) { skill_roots.push(PluginSkillRoot { path: path.clone(), plugin_id: plugin.config_name.clone(), + plugin_namespace: plugin_namespace.clone(), plugin_root: plugin.root.clone(), }); } @@ -218,6 +223,12 @@ mod tests { LoadedPlugin { config_name: config_name.to_string(), manifest_name: None, + plugin_namespace: Some( + config_name + .split_once('@') + .map_or(config_name, |(name, _)| name) + .to_string(), + ), manifest_description: None, root: test_path(config_name), enabled: true, @@ -245,6 +256,7 @@ mod tests { vec![PluginSkillRoot { path: shared_root, plugin_id: "zeta@test".to_string(), + plugin_namespace: "zeta".to_string(), plugin_root: test_path("zeta@test"), }] ); diff --git a/codex-rs/utils/plugins/src/lib.rs b/codex-rs/utils/plugins/src/lib.rs index fe178e679c3b..203b1f22e41d 100644 --- a/codex-rs/utils/plugins/src/lib.rs +++ b/codex-rs/utils/plugins/src/lib.rs @@ -15,5 +15,6 @@ pub use plugin_namespace::plugin_namespace_for_skill_path; pub struct PluginSkillRoot { pub path: AbsolutePathBuf, pub plugin_id: String, + pub plugin_namespace: String, pub plugin_root: AbsolutePathBuf, }