Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions codex-rs/core-plugins/src/loader.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
)
Expand Down Expand Up @@ -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<Product>,
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::<Vec<_>>();
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core-plugins/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
37 changes: 26 additions & 11 deletions codex-rs/core-plugins/src/manager_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
),
Expand Down Expand Up @@ -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(),
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
29 changes: 27 additions & 2 deletions codex-rs/core-skills/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ pub struct SkillRoot {
pub scope: SkillScope,
pub file_system: Arc<dyn ExecutorFileSystem>,
pub plugin_id: Option<String>,
pub plugin_namespace: Option<String>,
pub plugin_root: Option<AbsolutePathBuf>,
}

Expand All @@ -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,
)
Expand Down Expand Up @@ -266,13 +268,15 @@ 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 {
path,
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);
Expand Down Expand Up @@ -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,
});
}
Expand All @@ -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,
});

Expand All @@ -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,
});
}
Expand All @@ -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,
});
}
Expand All @@ -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,
});
}
Expand Down Expand Up @@ -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(_) => {}
Expand Down Expand Up @@ -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,
) {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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<SkillMetadata, SkillParseError> {
let path_uri = PathUri::from_abs_path(path);
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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}"))
Expand Down
14 changes: 12 additions & 2 deletions codex-rs/core-skills/src/loader_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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();

Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1805,13 +1813,15 @@ 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 {
path: root.path().abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_namespace: None,
plugin_root: None,
},
])
Expand Down
9 changes: 7 additions & 2 deletions codex-rs/core-skills/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ impl SkillsService {

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct ConfigSkillsCacheKey {
roots: Vec<(AbsolutePathBuf, u8, Option<String>)>,
roots: Vec<(AbsolutePathBuf, u8, Option<String>, Option<String>)>,
skill_config_rules: SkillConfigRules,
}

Expand Down Expand Up @@ -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(),
Expand Down
Loading
Loading