mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Add plugin ID to skill analytics (#20923)
## Summary - thread plugin skill roots through the skills loader with their plugin ID - store plugin ID on loaded skill metadata for plugin-provided skills - include plugin ID on skill invocation analytics events ## Test plan - cargo check -p codex-core-skills - cargo check -p codex-core -p codex-core-plugins -p codex-analytics - cargo check -p codex-tui - cargo check -p codex-plugin -p codex-core -p codex-core-plugins -p codex-analytics - cargo check -p codex-app-server - cargo test -p codex-analytics - HOME=/private/tmp/codex-empty-home cargo test -p codex-core-skills - just fix -p codex-core-skills - just fix -p codex-analytics - just fix -p codex-core-plugins - just fix -p codex-core - just fmt - git diff --check
This commit is contained in:
committed by
GitHub
Unverified
parent
707e51bd8b
commit
3ad7cf0993
@@ -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();
|
||||
|
||||
@@ -80,6 +80,7 @@ pub(crate) struct SkillInvocationEventRequest {
|
||||
pub(crate) struct SkillInvocationEventParams {
|
||||
pub(crate) product_client_id: Option<String>,
|
||||
pub(crate) skill_scope: Option<String>,
|
||||
pub(crate) plugin_id: Option<String>,
|
||||
pub(crate) repo_url: Option<String>,
|
||||
pub(crate) thread_id: Option<String>,
|
||||
pub(crate) invoke_type: Option<InvocationType>,
|
||||
|
||||
@@ -173,6 +173,7 @@ pub struct SkillInvocation {
|
||||
pub skill_name: String,
|
||||
pub skill_scope: SkillScope,
|
||||
pub skill_path: PathBuf,
|
||||
pub plugin_id: Option<String>,
|
||||
pub invocation_type: InvocationType,
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
},
|
||||
));
|
||||
|
||||
@@ -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<Product>,
|
||||
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::<Vec<_>>();
|
||||
let outcome = load_skills_from_roots(roots).await;
|
||||
|
||||
@@ -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<AbsolutePathBuf> {
|
||||
) -> Vec<PluginSkillRoot> {
|
||||
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(
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<dyn ExecutorFileSystem>,
|
||||
pub plugin_id: Option<String>,
|
||||
}
|
||||
|
||||
pub async fn load_skills_from_roots<I>(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<Arc<dyn ExecutorFileSystem>>,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
cwd: &AbsolutePathBuf,
|
||||
plugin_skill_roots: Vec<AbsolutePathBuf>,
|
||||
plugin_skill_roots: Vec<PluginSkillRoot>,
|
||||
) -> Vec<SkillRoot> {
|
||||
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<AbsolutePathBuf>,
|
||||
plugin_skill_roots: Vec<PluginSkillRoot>,
|
||||
) -> Vec<SkillRoot> {
|
||||
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<SkillMetadata, SkillParseError> {
|
||||
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),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<AbsolutePathBuf>,
|
||||
pub effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
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<AbsolutePathBuf>,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
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<String>)>,
|
||||
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(),
|
||||
|
||||
@@ -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),
|
||||
);
|
||||
|
||||
@@ -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<String>,
|
||||
}
|
||||
|
||||
impl SkillMetadata {
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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());
|
||||
|
||||
@@ -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<AbsolutePathBuf>,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
) -> 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 {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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<M: Clone> PluginLoadOutcome<M> {
|
||||
skill_roots
|
||||
}
|
||||
|
||||
pub fn effective_plugin_skill_roots(&self) -> Vec<PluginSkillRoot> {
|
||||
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<String, M> {
|
||||
let mut mcp_servers = HashMap::new();
|
||||
for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) {
|
||||
@@ -172,10 +191,61 @@ impl<M: Clone> PluginLoadOutcome<M> {
|
||||
/// without naming the MCP config type parameter.
|
||||
pub trait EffectiveSkillRoots {
|
||||
fn effective_skill_roots(&self) -> Vec<AbsolutePathBuf>;
|
||||
|
||||
fn effective_plugin_skill_roots(&self) -> Vec<PluginSkillRoot>;
|
||||
}
|
||||
|
||||
impl<M: Clone> EffectiveSkillRoots for PluginLoadOutcome<M> {
|
||||
fn effective_skill_roots(&self) -> Vec<AbsolutePathBuf> {
|
||||
PluginLoadOutcome::effective_skill_roots(self)
|
||||
}
|
||||
|
||||
fn effective_plugin_skill_roots(&self) -> Vec<PluginSkillRoot> {
|
||||
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<AbsolutePathBuf>) -> 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(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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,
|
||||
}]),
|
||||
});
|
||||
|
||||
|
||||
@@ -234,6 +234,7 @@ fn protocol_skill_to_core(skill: &ProtocolSkillMetadata) -> Option<SkillMetadata
|
||||
policy: None,
|
||||
path_to_skills_md: skill.path.clone(),
|
||||
scope,
|
||||
plugin_id: None,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -523,6 +523,7 @@ async fn submission_prefers_selected_duplicate_skill_path() {
|
||||
policy: None,
|
||||
path_to_skills_md: repo_skill_path,
|
||||
scope: crate::test_support::skill_scope_repo(),
|
||||
plugin_id: None,
|
||||
},
|
||||
SkillMetadata {
|
||||
name: "figma".to_string(),
|
||||
@@ -533,6 +534,7 @@ async fn submission_prefers_selected_duplicate_skill_path() {
|
||||
policy: None,
|
||||
path_to_skills_md: user_skill_path.clone(),
|
||||
scope: crate::test_support::skill_scope_user(),
|
||||
plugin_id: None,
|
||||
},
|
||||
]));
|
||||
|
||||
|
||||
@@ -1,9 +1,17 @@
|
||||
//! Plugin path resolution, plaintext mention sigils, and MCP connector helpers shared across Codex
|
||||
//! crates.
|
||||
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
pub mod mcp_connector;
|
||||
pub mod mention_syntax;
|
||||
pub mod plugin_namespace;
|
||||
|
||||
pub use plugin_namespace::find_plugin_manifest_path;
|
||||
pub use plugin_namespace::plugin_namespace_for_skill_path;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
pub struct PluginSkillRoot {
|
||||
pub path: AbsolutePathBuf,
|
||||
pub plugin_id: String,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user