diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index 52ece67a13..80b8e667f4 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -1816,6 +1816,7 @@ async fn reducer_ingests_skill_invoked_fact() { skill_name: "doc".to_string(), skill_scope: codex_protocol::protocol::SkillScope::User, skill_path, + plugin_id: None, invocation_type: InvocationType::Explicit, }], })), @@ -1833,6 +1834,7 @@ async fn reducer_ingests_skill_invoked_fact() { "event_params": { "product_client_id": originator().value, "skill_scope": "user", + "plugin_id": null, "repo_url": null, "thread_id": "thread-1", "invoke_type": "explicit", @@ -1842,6 +1844,41 @@ async fn reducer_ingests_skill_invoked_fact() { ); } +#[tokio::test] +async fn reducer_includes_plugin_id_for_plugin_skill_invocations() { + let mut reducer = AnalyticsReducer::default(); + let mut events = Vec::new(); + let tracking = TrackEventsContext { + model_slug: "gpt-5".to_string(), + thread_id: "thread-1".to_string(), + turn_id: "turn-1".to_string(), + }; + let skill_path = + PathBuf::from("/Users/abc/.codex/plugins/cache/test/sample/skills/doc/SKILL.md"); + + reducer + .ingest( + AnalyticsFact::Custom(CustomAnalyticsFact::SkillInvoked(SkillInvokedInput { + tracking, + invocations: vec![SkillInvocation { + skill_name: "sample:doc".to_string(), + skill_scope: codex_protocol::protocol::SkillScope::User, + skill_path, + plugin_id: Some("sample@test".to_string()), + invocation_type: InvocationType::Explicit, + }], + })), + &mut events, + ) + .await; + + let payload = serde_json::to_value(&events).expect("serialize events"); + assert_eq!( + payload[0]["event_params"]["plugin_id"], + json!("sample@test") + ); +} + #[tokio::test] async fn reducer_ingests_hook_run_fact() { let mut reducer = AnalyticsReducer::default(); diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index 8bd9440299..09c55e7844 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -80,6 +80,7 @@ pub(crate) struct SkillInvocationEventRequest { pub(crate) struct SkillInvocationEventParams { pub(crate) product_client_id: Option, pub(crate) skill_scope: Option, + pub(crate) plugin_id: Option, pub(crate) repo_url: Option, pub(crate) thread_id: Option, pub(crate) invoke_type: Option, diff --git a/codex-rs/analytics/src/facts.rs b/codex-rs/analytics/src/facts.rs index 424dd523b2..861e6534a2 100644 --- a/codex-rs/analytics/src/facts.rs +++ b/codex-rs/analytics/src/facts.rs @@ -173,6 +173,7 @@ pub struct SkillInvocation { pub skill_name: String, pub skill_scope: SkillScope, pub skill_path: PathBuf, + pub plugin_id: Option, pub invocation_type: InvocationType, } diff --git a/codex-rs/analytics/src/reducer.rs b/codex-rs/analytics/src/reducer.rs index b1dc822d43..49245c2180 100644 --- a/codex-rs/analytics/src/reducer.rs +++ b/codex-rs/analytics/src/reducer.rs @@ -501,6 +501,7 @@ impl AnalyticsReducer { product_client_id: Some(originator().value), repo_url, skill_scope: Some(skill_scope.to_string()), + plugin_id: invocation.plugin_id, }, }, )); diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index b07b7da3e8..f348a3414d 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -569,6 +569,7 @@ async fn load_plugin( loaded_plugin.skill_roots = plugin_skill_roots(&plugin_root, manifest_paths); let resolved_skills = load_plugin_skills( &plugin_root, + &loaded_plugin_id, manifest_paths, restriction_product, skill_config_rules, @@ -647,6 +648,7 @@ impl ResolvedPluginSkills { pub async fn load_plugin_skills( plugin_root: &AbsolutePathBuf, + plugin_id: &PluginId, manifest_paths: &PluginManifestPaths, restriction_product: Option, skill_config_rules: &SkillConfigRules, @@ -657,6 +659,7 @@ pub async fn load_plugin_skills( path, scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), + plugin_id: Some(plugin_id.as_key()), }) .collect::>(); let outcome = load_skills_from_roots(roots).await; diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index aecbd76e5c..ba56646649 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -63,6 +63,7 @@ use codex_plugin::PluginIdError; use codex_plugin::prompt_safe_plugin_description; use codex_protocol::protocol::Product; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PluginSkillRoot; use std::collections::HashMap; use std::collections::HashSet; use std::path::PathBuf; @@ -540,10 +541,10 @@ impl PluginsManager { &self, config_layer_stack: &ConfigLayerStack, config: &PluginsConfigInput, - ) -> Vec { + ) -> Vec { self.plugins_for_layer_stack(config_layer_stack, config, config.plugin_hooks_enabled) .await - .effective_skill_roots() + .effective_plugin_skill_roots() } fn cached_enabled_outcome( @@ -1339,6 +1340,7 @@ impl PluginsManager { ); let resolved_skills = load_plugin_skills( &source_path, + &plugin_id, &manifest.paths, self.restriction_product, &codex_core_skills::config_rules::skill_config_rules_from_stack( diff --git a/codex-rs/core-skills/src/injection.rs b/codex-rs/core-skills/src/injection.rs index ed06cc578e..df62f42e85 100644 --- a/codex-rs/core-skills/src/injection.rs +++ b/codex-rs/core-skills/src/injection.rs @@ -59,6 +59,7 @@ pub async fn build_skill_injections( skill_name: skill.name.clone(), skill_scope: skill.scope, skill_path: skill.path_to_skills_md.to_path_buf(), + plugin_id: skill.plugin_id.clone(), invocation_type: InvocationType::Explicit, }); result.items.push(SkillInjection { diff --git a/codex-rs/core-skills/src/injection_tests.rs b/codex-rs/core-skills/src/injection_tests.rs index 9627318653..78aa195895 100644 --- a/codex-rs/core-skills/src/injection_tests.rs +++ b/codex-rs/core-skills/src/injection_tests.rs @@ -16,6 +16,7 @@ fn make_skill(name: &str, path: &str) -> SkillMetadata { policy: None, path_to_skills_md: test_path_buf(path).abs(), scope: codex_protocol::protocol::SkillScope::User, + plugin_id: None, } } diff --git a/codex-rs/core-skills/src/invocation_utils_tests.rs b/codex-rs/core-skills/src/invocation_utils_tests.rs index ab3a3e8dc0..f6e3883c16 100644 --- a/codex-rs/core-skills/src/invocation_utils_tests.rs +++ b/codex-rs/core-skills/src/invocation_utils_tests.rs @@ -21,6 +21,7 @@ fn test_skill_metadata(skill_doc_path: AbsolutePathBuf) -> SkillMetadata { policy: None, path_to_skills_md: skill_doc_path, scope: codex_protocol::protocol::SkillScope::User, + plugin_id: None, } } diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index d7a69e8a25..2473f7108c 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -19,6 +19,7 @@ use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_plugins::PluginSkillRoot; use codex_utils_plugins::plugin_namespace_for_skill_path; use dirs::home_dir; use serde::Deserialize; @@ -152,6 +153,7 @@ pub struct SkillRoot { pub path: AbsolutePathBuf, pub scope: SkillScope, pub file_system: Arc, + pub plugin_id: Option, } pub async fn load_skills_from_roots(roots: I) -> SkillLoadOutcome @@ -167,7 +169,14 @@ where let root_path = canonicalize_for_skill_identity(&root.path); let fs = root.file_system; let skills_before_root = outcome.skills.len(); - discover_skills_under_root(fs.as_ref(), &root_path, root.scope, &mut outcome).await; + discover_skills_under_root( + fs.as_ref(), + &root_path, + root.scope, + root.plugin_id.as_deref(), + &mut outcome, + ) + .await; for skill in &outcome.skills[skills_before_root..] { if !skill_roots.contains(&root_path) { skill_roots.push(root_path.clone()); @@ -222,7 +231,7 @@ pub(crate) async fn skill_roots( fs: Option>, config_layer_stack: &ConfigLayerStack, cwd: &AbsolutePathBuf, - plugin_skill_roots: Vec, + plugin_skill_roots: Vec, ) -> Vec { let home_dir = home_dir().and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok()); @@ -241,13 +250,14 @@ async fn skill_roots_with_home_dir( config_layer_stack: &ConfigLayerStack, cwd: &AbsolutePathBuf, home_dir: Option<&AbsolutePathBuf>, - plugin_skill_roots: Vec, + plugin_skill_roots: Vec, ) -> Vec { let mut roots = skill_roots_from_layer_stack_inner(config_layer_stack, home_dir, fs.clone()); - roots.extend(plugin_skill_roots.into_iter().map(|path| SkillRoot { - path, + roots.extend(plugin_skill_roots.into_iter().map(|root| SkillRoot { + path: root.path, scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), + plugin_id: Some(root.plugin_id), })); roots.extend(repo_agents_skill_roots(fs, config_layer_stack, cwd).await); dedupe_skill_roots_by_path(&mut roots); @@ -276,6 +286,7 @@ fn skill_roots_from_layer_stack_inner( path: config_folder.join(SKILLS_DIR_NAME), scope: SkillScope::Repo, file_system: Arc::clone(repo_fs), + plugin_id: None, }); } } @@ -286,6 +297,7 @@ fn skill_roots_from_layer_stack_inner( path: config_folder.join(SKILLS_DIR_NAME), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), + plugin_id: None, }); // `$HOME/.agents/skills` (user-installed skills). @@ -294,6 +306,7 @@ fn skill_roots_from_layer_stack_inner( path: home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), + plugin_id: None, }); } @@ -303,6 +316,7 @@ fn skill_roots_from_layer_stack_inner( path: system_cache_root_dir(&config_folder), scope: SkillScope::System, file_system: Arc::clone(&LOCAL_FS), + plugin_id: None, }); } ConfigLayerSource::System { .. } => { @@ -312,6 +326,7 @@ fn skill_roots_from_layer_stack_inner( path: config_folder.join(SKILLS_DIR_NAME), scope: SkillScope::Admin, file_system: Arc::clone(&LOCAL_FS), + plugin_id: None, }); } ConfigLayerSource::Mdm { .. } @@ -343,6 +358,7 @@ async fn repo_agents_skill_roots( path: agents_skills, scope: SkillScope::Repo, file_system: Arc::clone(&fs), + plugin_id: None, }), Ok(_) => {} Err(err) if err.kind() == io::ErrorKind::NotFound => {} @@ -441,6 +457,7 @@ async fn discover_skills_under_root( fs: &dyn ExecutorFileSystem, root: &AbsolutePathBuf, scope: SkillScope, + plugin_id: Option<&str>, outcome: &mut SkillLoadOutcome, ) { let root = canonicalize_for_skill_identity(root); @@ -553,7 +570,7 @@ async fn discover_skills_under_root( } if metadata.is_file && file_name == SKILLS_FILENAME { - match parse_skill_file(fs, &path, scope).await { + match parse_skill_file(fs, &path, scope, plugin_id).await { Ok(skill) => { outcome.skills.push(skill); } @@ -583,6 +600,7 @@ async fn parse_skill_file( fs: &dyn ExecutorFileSystem, path: &AbsolutePathBuf, scope: SkillScope, + plugin_id: Option<&str>, ) -> Result { let contents = fs .read_file_text(path, /*sandbox*/ None) @@ -639,6 +657,7 @@ async fn parse_skill_file( policy, path_to_skills_md: resolved_path, scope, + plugin_id: plugin_id.map(str::to_string), }) } diff --git a/codex-rs/core-skills/src/loader_tests.rs b/codex-rs/core-skills/src/loader_tests.rs index a12f09f80f..c80585871e 100644 --- a/codex-rs/core-skills/src/loader_tests.rs +++ b/codex-rs/core-skills/src/loader_tests.rs @@ -322,6 +322,7 @@ async fn loads_skills_from_home_agents_dir_for_user_scope() -> anyhow::Result<() policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: None, }] ); @@ -472,6 +473,7 @@ async fn loads_skill_dependencies_metadata_from_yaml() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -527,6 +529,7 @@ interface: policy: None, path_to_skills_md: normalized(skill_path.as_path()), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -680,6 +683,7 @@ async fn accepts_icon_paths_under_assets_dir() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -720,6 +724,7 @@ async fn ignores_invalid_brand_color() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -773,6 +778,7 @@ async fn ignores_default_prompt_over_max_length() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -814,6 +820,7 @@ async fn drops_interface_when_icons_are_invalid() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -858,6 +865,7 @@ async fn loads_skills_via_symlinked_subdir_for_user_scope() { policy: None, path_to_skills_md: normalized(&shared_skill_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -917,6 +925,7 @@ async fn does_not_loop_on_symlink_cycle_for_user_scope() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -936,6 +945,7 @@ async fn loads_skills_via_symlinked_subdir_for_admin_scope() { path: admin_root.path().abs(), scope: SkillScope::Admin, file_system: Arc::clone(&LOCAL_FS), + plugin_id: None, }]) .await; @@ -955,6 +965,7 @@ async fn loads_skills_via_symlinked_subdir_for_admin_scope() { policy: None, path_to_skills_md: normalized(&shared_skill_path), scope: SkillScope::Admin, + plugin_id: None, }] ); } @@ -994,6 +1005,7 @@ async fn loads_skills_via_symlinked_subdir_for_repo_scope() { policy: None, path_to_skills_md: normalized(&linked_skill_path), scope: SkillScope::Repo, + plugin_id: None, }] ); } @@ -1014,6 +1026,7 @@ async fn system_scope_ignores_symlinked_subdir() { path: system_root.abs(), scope: SkillScope::System, file_system: Arc::clone(&LOCAL_FS), + plugin_id: None, }]) .await; assert!( @@ -1046,6 +1059,7 @@ async fn respects_max_scan_depth_for_user_scope() { path: skills_root.abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), + plugin_id: None, }]) .await; @@ -1065,6 +1079,7 @@ async fn respects_max_scan_depth_for_user_scope() { policy: None, path_to_skills_md: normalized(&within_depth_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -1092,6 +1107,7 @@ async fn loads_valid_skill() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -1124,6 +1140,7 @@ async fn falls_back_to_directory_name_when_skill_name_is_missing() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -1148,6 +1165,7 @@ async fn namespaces_plugin_skills_using_plugin_name() { path: plugin_root.join("skills").abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), + plugin_id: Some("sample@test".to_string()), }]) .await; @@ -1167,6 +1185,7 @@ async fn namespaces_plugin_skills_using_plugin_name() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: Some("sample@test".to_string()), }] ); } @@ -1198,6 +1217,7 @@ async fn loads_short_description_from_metadata() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, + plugin_id: None, }] ); } @@ -1310,6 +1330,7 @@ async fn loads_skills_from_repo_root() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, + plugin_id: None, }] ); } @@ -1345,6 +1366,7 @@ async fn loads_skills_from_agents_dir_without_codex_dir() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, + plugin_id: None, }] ); } @@ -1398,6 +1420,7 @@ async fn loads_skills_from_all_codex_dirs_under_project_root() { policy: None, path_to_skills_md: normalized(&nested_skill_path), scope: SkillScope::Repo, + plugin_id: None, }, SkillMetadata { name: "root-skill".to_string(), @@ -1408,6 +1431,7 @@ async fn loads_skills_from_all_codex_dirs_under_project_root() { policy: None, path_to_skills_md: normalized(&root_skill_path), scope: SkillScope::Repo, + plugin_id: None, }, ] ); @@ -1447,6 +1471,7 @@ async fn loads_skills_from_codex_dir_when_not_git_repo() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, + plugin_id: None, }] ); } @@ -1462,11 +1487,13 @@ async fn deduplicates_by_path_preferring_first_root() { path: root.path().abs(), scope: SkillScope::Repo, file_system: Arc::clone(&LOCAL_FS), + plugin_id: None, }, SkillRoot { path: root.path().abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), + plugin_id: None, }, ]) .await; @@ -1487,6 +1514,7 @@ async fn deduplicates_by_path_preferring_first_root() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, + plugin_id: None, }] ); } @@ -1528,6 +1556,7 @@ async fn keeps_duplicate_names_from_repo_and_user() { policy: None, path_to_skills_md: normalized(&repo_skill_path), scope: SkillScope::Repo, + plugin_id: None, }, SkillMetadata { name: "dupe-skill".to_string(), @@ -1538,6 +1567,7 @@ async fn keeps_duplicate_names_from_repo_and_user() { policy: None, path_to_skills_md: normalized(&user_skill_path), scope: SkillScope::User, + plugin_id: None, }, ] ); @@ -1600,6 +1630,7 @@ async fn keeps_duplicate_names_from_nested_codex_dirs() { policy: None, path_to_skills_md: first_path, scope: SkillScope::Repo, + plugin_id: None, }, SkillMetadata { name: "dupe-skill".to_string(), @@ -1610,6 +1641,7 @@ async fn keeps_duplicate_names_from_nested_codex_dirs() { policy: None, path_to_skills_md: second_path, scope: SkillScope::Repo, + plugin_id: None, }, ] ); @@ -1681,6 +1713,7 @@ async fn loads_skills_when_cwd_is_file_in_repo() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, + plugin_id: None, }] ); } @@ -1739,6 +1772,7 @@ async fn loads_skills_from_system_cache_when_present() { policy: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::System, + plugin_id: None, }] ); } diff --git a/codex-rs/core-skills/src/manager.rs b/codex-rs/core-skills/src/manager.rs index b7b7a4b64d..73b1f14807 100644 --- a/codex-rs/core-skills/src/manager.rs +++ b/codex-rs/core-skills/src/manager.rs @@ -8,6 +8,7 @@ use codex_exec_server::ExecutorFileSystem; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PluginSkillRoot; use tracing::info; use tracing::warn; @@ -26,7 +27,7 @@ use codex_config::SkillsConfig; #[derive(Debug, Clone)] pub struct SkillsLoadInput { pub cwd: AbsolutePathBuf, - pub effective_skill_roots: Vec, + pub effective_skill_roots: Vec, pub config_layer_stack: ConfigLayerStack, pub bundled_skills_enabled: bool, } @@ -34,7 +35,7 @@ pub struct SkillsLoadInput { impl SkillsLoadInput { pub fn new( cwd: AbsolutePathBuf, - effective_skill_roots: Vec, + effective_skill_roots: Vec, config_layer_stack: ConfigLayerStack, bundled_skills_enabled: bool, ) -> Self { @@ -168,6 +169,7 @@ impl SkillsManager { path, scope: SkillScope::User, file_system: Arc::clone(&fs), + plugin_id: None, }), ); } @@ -239,7 +241,7 @@ impl SkillsManager { #[derive(Debug, Clone, PartialEq, Eq, Hash)] struct ConfigSkillsCacheKey { - roots: Vec<(AbsolutePathBuf, u8)>, + roots: Vec<(AbsolutePathBuf, u8, Option)>, skill_config_rules: SkillConfigRules, } @@ -279,7 +281,7 @@ fn config_skills_cache_key( SkillScope::System => 2, SkillScope::Admin => 3, }; - (root.path.clone(), scope_rank) + (root.path.clone(), scope_rank, root.plugin_id.clone()) }) .collect(), skill_config_rules: skill_config_rules.clone(), diff --git a/codex-rs/core-skills/src/manager_tests.rs b/codex-rs/core-skills/src/manager_tests.rs index 73800a51d0..532d6951ec 100644 --- a/codex-rs/core-skills/src/manager_tests.rs +++ b/codex-rs/core-skills/src/manager_tests.rs @@ -12,6 +12,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_absolute_path::test_support::PathExt; use codex_utils_absolute_path::test_support::test_path_buf; +use codex_utils_plugins::PluginSkillRoot; use pretty_assertions::assert_eq; use std::collections::HashSet; use std::fs; @@ -67,6 +68,7 @@ fn test_skill(name: &str, path: PathBuf) -> SkillMetadata { .canonicalize() .expect("skill path should canonicalize"), scope: SkillScope::User, + plugin_id: None, } } @@ -146,7 +148,14 @@ async fn skills_for_config_with_stack( ) -> SkillLoadOutcome { let skills_input = SkillsLoadInput::new( cwd.path().abs(), - effective_skill_roots.to_vec(), + effective_skill_roots + .iter() + .cloned() + .map(|path| PluginSkillRoot { + path, + plugin_id: "test-plugin@test".to_string(), + }) + .collect(), config_layer_stack.clone(), bundled_skills_enabled_from_stack(config_layer_stack), ); diff --git a/codex-rs/core-skills/src/model.rs b/codex-rs/core-skills/src/model.rs index 0a72c24fe8..fc8e9f5917 100644 --- a/codex-rs/core-skills/src/model.rs +++ b/codex-rs/core-skills/src/model.rs @@ -19,6 +19,7 @@ pub struct SkillMetadata { /// Path to the SKILLS.md file that declares this skill. pub path_to_skills_md: AbsolutePathBuf, pub scope: SkillScope, + pub plugin_id: Option, } impl SkillMetadata { diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 613ed9cbe5..28617fb6c4 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -922,6 +922,7 @@ mod tests { policy: None, path_to_skills_md: test_path_buf(&format!("/tmp/{name}/SKILL.md")).abs(), scope, + plugin_id: None, } } diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 2550d58f82..1c99fb5950 100644 --- a/codex-rs/core/src/agent/role_tests.rs +++ b/codex-rs/core/src/agent/role_tests.rs @@ -657,7 +657,7 @@ enabled = false SkillsManager::new(home.path().abs(), /*bundled_skills_enabled*/ true); let plugins_input = config.plugins_config_input(); let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; - let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); let skills_input = skills_load_input_from_config(&config, effective_skill_roots); let outcome = skills_manager .skills_for_config( diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 46d942f710..0da9968149 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -476,7 +476,7 @@ impl Codex { let fs = environment_selections.primary_filesystem(); let plugins_input = config.plugins_config_input(); let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; - let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); let skills_input = skills_load_input_from_config(&config, effective_skill_roots); let loaded_skills = skills_manager.skills_for_config(&skills_input, fs).await; diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index fe38cd7f64..0a8ea46f81 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -3661,7 +3661,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { .plugins_manager .plugins_for_config(&per_turn_config.plugins_config_input()) .await; - let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); let skills_input = crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots); let skill_fs = environment.get_filesystem(); @@ -5189,7 +5189,7 @@ where .plugins_manager .plugins_for_config(&per_turn_config.plugins_config_input()) .await; - let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); let skills_input = crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots); let skill_fs = environment.get_filesystem(); @@ -5829,6 +5829,7 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() policy: None, path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), scope: SkillScope::Admin, + plugin_id: None, }, SkillMetadata { name: "repo-skill".to_string(), @@ -5839,6 +5840,7 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() policy: None, path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), scope: SkillScope::Repo, + plugin_id: None, }, ]; turn_context.model_info.context_window = Some(100); @@ -5874,6 +5876,7 @@ fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() { policy: None, path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), scope: SkillScope::Repo, + plugin_id: None, }]; let rendered = build_available_skills( &outcome, @@ -5918,6 +5921,7 @@ fn emit_thread_start_skill_metrics_records_description_truncated_chars_without_o policy: None, path_to_skills_md: test_path_buf("/tmp/alpha-skill/SKILL.md").abs(), scope: SkillScope::Repo, + plugin_id: None, }; let beta = SkillMetadata { name: "beta-skill".to_string(), @@ -5928,6 +5932,7 @@ fn emit_thread_start_skill_metrics_records_description_truncated_chars_without_o policy: None, path_to_skills_md: test_path_buf("/tmp/beta-skill/SKILL.md").abs(), scope: SkillScope::Repo, + plugin_id: None, }; let minimum_skill_line_cost = |skill: &SkillMetadata| { let path = skill.path_to_skills_md.to_string_lossy().replace('\\', "/"); @@ -5975,6 +5980,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil policy: None, path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), scope: SkillScope::Admin, + plugin_id: None, }, SkillMetadata { name: "repo-skill".to_string(), @@ -5985,6 +5991,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil policy: None, path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), scope: SkillScope::Repo, + plugin_id: None, }, ]; turn_context.model_info.context_window = Some(100); diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index faa8d693a9..fe54d61546 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -699,7 +699,7 @@ impl Session { .plugins_manager .plugins_for_config(&per_turn_config.plugins_config_input()) .await; - let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots); let fs = primary_turn_environment .map(|turn_environment| turn_environment.environment.get_filesystem()); diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index 0681d09624..2cceb2be36 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -14,6 +14,7 @@ use codex_protocol::request_user_input::RequestUserInputArgs; use codex_protocol::request_user_input::RequestUserInputQuestion; use codex_protocol::request_user_input::RequestUserInputResponse; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PluginSkillRoot; use tracing::warn; pub use codex_core_skills::SkillDependencyInfo; @@ -45,7 +46,7 @@ pub use codex_core_skills::system; pub(crate) fn skills_load_input_from_config( config: &Config, - effective_skill_roots: Vec, + effective_skill_roots: Vec, ) -> SkillsLoadInput { SkillsLoadInput::new( config.cwd.clone(), @@ -187,6 +188,7 @@ pub(crate) async fn maybe_emit_implicit_skill_invocation( skill_name: candidate.name, skill_scope: candidate.scope, skill_path: candidate.path_to_skills_md.to_path_buf(), + plugin_id: candidate.plugin_id, invocation_type: InvocationType::Implicit, }; let skill_scope = match invocation.skill_scope { diff --git a/codex-rs/core/src/skills_watcher.rs b/codex-rs/core/src/skills_watcher.rs index d97b41f1d5..fb271ca876 100644 --- a/codex-rs/core/src/skills_watcher.rs +++ b/codex-rs/core/src/skills_watcher.rs @@ -63,7 +63,7 @@ impl SkillsWatcher { ) -> WatchRegistration { let plugins_input = config.plugins_config_input(); let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; - let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); let skills_input = skills_load_input_from_config(config, effective_skill_roots); let roots = skills_manager .skill_roots_for_config(&skills_input, fs) diff --git a/codex-rs/plugin/src/load_outcome.rs b/codex-rs/plugin/src/load_outcome.rs index 0865b9020f..c76697366f 100644 --- a/codex-rs/plugin/src/load_outcome.rs +++ b/codex-rs/plugin/src/load_outcome.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::collections::HashSet; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PluginSkillRoot; use crate::AppConnectorId; use crate::PluginCapabilitySummary; @@ -116,6 +117,24 @@ impl PluginLoadOutcome { skill_roots } + pub fn effective_plugin_skill_roots(&self) -> Vec { + let mut skill_roots = Vec::new(); + let mut seen_paths = HashSet::new(); + for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) { + 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(), + }); + } + } + } + + skill_roots.sort_unstable_by(|a, b| a.path.cmp(&b.path)); + skill_roots + } + pub fn effective_mcp_servers(&self) -> HashMap { let mut mcp_servers = HashMap::new(); for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) { @@ -172,10 +191,61 @@ impl PluginLoadOutcome { /// without naming the MCP config type parameter. pub trait EffectiveSkillRoots { fn effective_skill_roots(&self) -> Vec; + + fn effective_plugin_skill_roots(&self) -> Vec; } impl EffectiveSkillRoots for PluginLoadOutcome { fn effective_skill_roots(&self) -> Vec { PluginLoadOutcome::effective_skill_roots(self) } + + fn effective_plugin_skill_roots(&self) -> Vec { + PluginLoadOutcome::effective_plugin_skill_roots(self) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn test_path(name: &str) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path_checked(std::env::temp_dir().join(name)) + .expect("absolute temp path") + } + + fn loaded_plugin(config_name: &str, skill_roots: Vec) -> LoadedPlugin<()> { + LoadedPlugin { + config_name: config_name.to_string(), + manifest_name: None, + manifest_description: None, + root: test_path(config_name), + enabled: true, + skill_roots, + disabled_skill_paths: HashSet::new(), + has_enabled_skills: true, + mcp_servers: HashMap::new(), + apps: Vec::new(), + hook_sources: Vec::new(), + hook_load_warnings: Vec::new(), + error: None, + } + } + + #[test] + fn effective_plugin_skill_roots_preserves_first_plugin_for_shared_root() { + let shared_root = test_path("shared-skills"); + let outcome = PluginLoadOutcome::from_plugins(vec![ + loaded_plugin("zeta@test", vec![shared_root.clone()]), + loaded_plugin("alpha@test", vec![shared_root.clone()]), + ]); + + assert_eq!( + outcome.effective_plugin_skill_roots(), + vec![PluginSkillRoot { + path: shared_root, + plugin_id: "zeta@test".to_string(), + }] + ); + } } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 4433d79cf4..5b90feab32 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -6258,6 +6258,7 @@ mod tests { policy: None, path_to_skills_md: skill_path.clone(), scope: crate::test_support::skill_scope_user(), + plugin_id: None, }])); let ActivePopup::Skill(popup) = &composer.active_popup else { @@ -6300,6 +6301,7 @@ mod tests { policy: None, path_to_skills_md: skill_path.clone(), scope: crate::test_support::skill_scope_repo(), + plugin_id: None, }])); composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary { config_name: "google-calendar@debug".to_string(), @@ -6391,6 +6393,7 @@ mod tests { policy: None, path_to_skills_md: test_path_buf("/tmp/repo/google-calendar/SKILL.md").abs(), scope: crate::test_support::skill_scope_repo(), + plugin_id: None, }])); composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary { config_name: "google-calendar@debug".to_string(), diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index c839ddd4a7..7b0694e0b3 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -2408,6 +2408,7 @@ mod tests { policy: None, path_to_skills_md: test_path_buf("/tmp/test-skill/SKILL.md").abs(), scope: crate::test_support::skill_scope_user(), + plugin_id: None, }]), }); diff --git a/codex-rs/tui/src/chatwidget/skills.rs b/codex-rs/tui/src/chatwidget/skills.rs index 71ef567e3d..9c11151f0f 100644 --- a/codex-rs/tui/src/chatwidget/skills.rs +++ b/codex-rs/tui/src/chatwidget/skills.rs @@ -234,6 +234,7 @@ fn protocol_skill_to_core(skill: &ProtocolSkillMetadata) -> Option