mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
Track plugin ids for skill invocations
This commit is contained in:
@@ -709,6 +709,7 @@ async fn reducer_ingests_skill_invoked_fact() {
|
||||
skill_name: "doc".to_string(),
|
||||
skill_scope: codex_protocol::protocol::SkillScope::User,
|
||||
skill_path,
|
||||
plugin_id: Some("docs@openai".to_string()),
|
||||
invocation_type: InvocationType::Explicit,
|
||||
}],
|
||||
})),
|
||||
@@ -725,6 +726,7 @@ async fn reducer_ingests_skill_invoked_fact() {
|
||||
"skill_name": "doc",
|
||||
"event_params": {
|
||||
"product_client_id": originator().value,
|
||||
"plugin_id": "docs@openai",
|
||||
"skill_scope": "user",
|
||||
"repo_url": null,
|
||||
"thread_id": "thread-1",
|
||||
|
||||
@@ -55,6 +55,7 @@ pub(crate) struct SkillInvocationEventRequest {
|
||||
#[derive(Serialize)]
|
||||
pub(crate) struct SkillInvocationEventParams {
|
||||
pub(crate) product_client_id: Option<String>,
|
||||
pub(crate) plugin_id: Option<String>,
|
||||
pub(crate) skill_scope: Option<String>,
|
||||
pub(crate) repo_url: Option<String>,
|
||||
pub(crate) thread_id: Option<String>,
|
||||
|
||||
@@ -35,6 +35,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,
|
||||
}
|
||||
|
||||
|
||||
@@ -175,6 +175,7 @@ impl AnalyticsReducer {
|
||||
invoke_type: Some(invocation.invocation_type),
|
||||
model_slug: Some(tracking.model_slug.clone()),
|
||||
product_client_id: Some(originator().value),
|
||||
plugin_id: invocation.plugin_id,
|
||||
repo_url,
|
||||
skill_scope: Some(skill_scope.to_string()),
|
||||
},
|
||||
|
||||
@@ -11744,6 +11744,12 @@
|
||||
"path": {
|
||||
"type": "string"
|
||||
},
|
||||
"pluginId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"scope": {
|
||||
"$ref": "#/definitions/v2/SkillScope"
|
||||
},
|
||||
|
||||
@@ -9599,6 +9599,12 @@
|
||||
"path": {
|
||||
"type": "string"
|
||||
},
|
||||
"pluginId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"scope": {
|
||||
"$ref": "#/definitions/SkillScope"
|
||||
},
|
||||
|
||||
@@ -105,6 +105,12 @@
|
||||
"path": {
|
||||
"type": "string"
|
||||
},
|
||||
"pluginId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"scope": {
|
||||
"$ref": "#/definitions/SkillScope"
|
||||
},
|
||||
|
||||
@@ -9,4 +9,4 @@ export type SkillMetadata = { name: string, description: string,
|
||||
/**
|
||||
* Legacy short_description from SKILL.md. Prefer SKILL.json interface.short_description.
|
||||
*/
|
||||
shortDescription?: string, interface?: SkillInterface, dependencies?: SkillDependencies, path: string, scope: SkillScope, enabled: boolean, };
|
||||
shortDescription?: string, interface?: SkillInterface, dependencies?: SkillDependencies, pluginId?: string, path: string, scope: SkillScope, enabled: boolean, };
|
||||
|
||||
@@ -3262,6 +3262,9 @@ pub struct SkillMetadata {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub dependencies: Option<SkillDependencies>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub plugin_id: Option<String>,
|
||||
pub path: PathBuf,
|
||||
pub scope: SkillScope,
|
||||
pub enabled: bool,
|
||||
@@ -3506,6 +3509,7 @@ impl From<CoreSkillMetadata> for SkillMetadata {
|
||||
short_description: value.short_description,
|
||||
interface: value.interface.map(SkillInterface::from),
|
||||
dependencies: value.dependencies.map(SkillDependencies::from),
|
||||
plugin_id: value.plugin_id,
|
||||
path: value.path,
|
||||
scope: value.scope.into(),
|
||||
enabled: true,
|
||||
|
||||
@@ -8205,6 +8205,7 @@ fn skills_to_info(
|
||||
.collect(),
|
||||
}
|
||||
}),
|
||||
plugin_id: skill.plugin_id.clone(),
|
||||
path: skill.path_to_skills_md.clone(),
|
||||
scope: skill.scope.into(),
|
||||
enabled,
|
||||
|
||||
@@ -45,6 +45,7 @@ pub async fn build_skill_injections(
|
||||
skill_name: skill.name.clone(),
|
||||
skill_scope: skill.scope,
|
||||
skill_path: skill.path_to_skills_md.clone(),
|
||||
plugin_id: skill.plugin_id.clone(),
|
||||
invocation_type: InvocationType::Explicit,
|
||||
});
|
||||
result.items.push(ResponseItem::from(SkillInstructions {
|
||||
|
||||
@@ -11,6 +11,7 @@ fn make_skill(name: &str, path: &str) -> SkillMetadata {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: PathBuf::from(path),
|
||||
scope: codex_protocol::protocol::SkillScope::User,
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@ fn test_skill_metadata(skill_doc_path: PathBuf) -> SkillMetadata {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: skill_doc_path,
|
||||
scope: codex_protocol::protocol::SkillScope::User,
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@ pub use env_var_dependencies::SkillDependencyInfo;
|
||||
pub use env_var_dependencies::collect_env_var_dependencies;
|
||||
pub(crate) use invocation_utils::build_implicit_skill_path_indexes;
|
||||
pub use invocation_utils::detect_implicit_skill_invocation_for_command;
|
||||
pub use loader::PluginSkillRoot;
|
||||
pub use manager::SkillsLoadInput;
|
||||
pub use manager::SkillsManager;
|
||||
pub use mention_counts::build_skill_name_counts;
|
||||
|
||||
@@ -147,6 +147,13 @@ impl Error for SkillParseError {}
|
||||
pub struct SkillRoot {
|
||||
pub path: PathBuf,
|
||||
pub scope: SkillScope,
|
||||
pub plugin_id: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct PluginSkillRoot {
|
||||
pub path: PathBuf,
|
||||
pub plugin_id: String,
|
||||
}
|
||||
|
||||
pub fn load_skills_from_roots<I>(roots: I) -> SkillLoadOutcome
|
||||
@@ -155,7 +162,12 @@ where
|
||||
{
|
||||
let mut outcome = SkillLoadOutcome::default();
|
||||
for root in roots {
|
||||
discover_skills_under_root(&root.path, root.scope, &mut outcome);
|
||||
discover_skills_under_root(
|
||||
&root.path,
|
||||
root.scope,
|
||||
root.plugin_id.as_deref(),
|
||||
&mut outcome,
|
||||
);
|
||||
}
|
||||
|
||||
let mut seen: HashSet<PathBuf> = HashSet::new();
|
||||
@@ -186,7 +198,7 @@ where
|
||||
pub(crate) fn skill_roots(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
cwd: &Path,
|
||||
plugin_skill_roots: Vec<PathBuf>,
|
||||
plugin_skill_roots: Vec<PluginSkillRoot>,
|
||||
) -> Vec<SkillRoot> {
|
||||
skill_roots_with_home_dir(
|
||||
config_layer_stack,
|
||||
@@ -200,12 +212,13 @@ fn skill_roots_with_home_dir(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
cwd: &Path,
|
||||
home_dir: Option<&Path>,
|
||||
plugin_skill_roots: Vec<PathBuf>,
|
||||
plugin_skill_roots: Vec<PluginSkillRoot>,
|
||||
) -> Vec<SkillRoot> {
|
||||
let mut roots = skill_roots_from_layer_stack_inner(config_layer_stack, home_dir);
|
||||
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,
|
||||
plugin_id: Some(root.plugin_id),
|
||||
}));
|
||||
roots.extend(repo_agents_skill_roots(config_layer_stack, cwd));
|
||||
dedupe_skill_roots_by_path(&mut roots);
|
||||
@@ -231,6 +244,7 @@ fn skill_roots_from_layer_stack_inner(
|
||||
roots.push(SkillRoot {
|
||||
path: config_folder.as_path().join(SKILLS_DIR_NAME),
|
||||
scope: SkillScope::Repo,
|
||||
plugin_id: None,
|
||||
});
|
||||
}
|
||||
ConfigLayerSource::User { .. } => {
|
||||
@@ -239,6 +253,7 @@ fn skill_roots_from_layer_stack_inner(
|
||||
roots.push(SkillRoot {
|
||||
path: config_folder.as_path().join(SKILLS_DIR_NAME),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
});
|
||||
|
||||
// `$HOME/.agents/skills` (user-installed skills).
|
||||
@@ -246,6 +261,7 @@ fn skill_roots_from_layer_stack_inner(
|
||||
roots.push(SkillRoot {
|
||||
path: home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -254,6 +270,7 @@ fn skill_roots_from_layer_stack_inner(
|
||||
roots.push(SkillRoot {
|
||||
path: system_cache_root_dir(config_folder.as_path()),
|
||||
scope: SkillScope::System,
|
||||
plugin_id: None,
|
||||
});
|
||||
}
|
||||
ConfigLayerSource::System { .. } => {
|
||||
@@ -262,6 +279,7 @@ fn skill_roots_from_layer_stack_inner(
|
||||
roots.push(SkillRoot {
|
||||
path: config_folder.as_path().join(SKILLS_DIR_NAME),
|
||||
scope: SkillScope::Admin,
|
||||
plugin_id: None,
|
||||
});
|
||||
}
|
||||
ConfigLayerSource::Mdm { .. }
|
||||
@@ -285,6 +303,7 @@ fn repo_agents_skill_roots(config_layer_stack: &ConfigLayerStack, cwd: &Path) ->
|
||||
roots.push(SkillRoot {
|
||||
path: agents_skills,
|
||||
scope: SkillScope::Repo,
|
||||
plugin_id: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -353,7 +372,12 @@ fn dedupe_skill_roots_by_path(roots: &mut Vec<SkillRoot>) {
|
||||
roots.retain(|root| seen.insert(root.path.clone()));
|
||||
}
|
||||
|
||||
fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut SkillLoadOutcome) {
|
||||
fn discover_skills_under_root(
|
||||
root: &Path,
|
||||
scope: SkillScope,
|
||||
plugin_id: Option<&str>,
|
||||
outcome: &mut SkillLoadOutcome,
|
||||
) {
|
||||
let Ok(root) = canonicalize_path(root) else {
|
||||
return;
|
||||
};
|
||||
@@ -466,7 +490,7 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil
|
||||
}
|
||||
|
||||
if file_type.is_file() && file_name == SKILLS_FILENAME {
|
||||
match parse_skill_file(&path, scope) {
|
||||
match parse_skill_file(&path, scope, plugin_id) {
|
||||
Ok(skill) => {
|
||||
outcome.skills.push(skill);
|
||||
}
|
||||
@@ -492,7 +516,11 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, SkillParseError> {
|
||||
fn parse_skill_file(
|
||||
path: &Path,
|
||||
scope: SkillScope,
|
||||
plugin_id: Option<&str>,
|
||||
) -> Result<SkillMetadata, SkillParseError> {
|
||||
let contents = fs::read_to_string(path).map_err(SkillParseError::Read)?;
|
||||
|
||||
let frontmatter = extract_frontmatter(&contents).ok_or(SkillParseError::MissingFrontmatter)?;
|
||||
@@ -543,6 +571,7 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, Ski
|
||||
interface,
|
||||
dependencies,
|
||||
policy,
|
||||
plugin_id: plugin_id.map(str::to_string),
|
||||
path_to_skills_md: resolved_path,
|
||||
scope,
|
||||
})
|
||||
|
||||
@@ -284,6 +284,7 @@ fn loads_skills_from_home_agents_dir_for_user_scope() -> anyhow::Result<()> {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -434,6 +435,7 @@ async fn loads_skill_dependencies_metadata_from_yaml() {
|
||||
],
|
||||
}),
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -489,6 +491,7 @@ interface:
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(skill_path.as_path()),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -642,6 +645,7 @@ async fn accepts_icon_paths_under_assets_dir() {
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -682,6 +686,7 @@ async fn ignores_invalid_brand_color() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -735,6 +740,7 @@ async fn ignores_default_prompt_over_max_length() {
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -776,6 +782,7 @@ async fn drops_interface_when_icons_are_invalid() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -820,6 +827,7 @@ async fn loads_skills_via_symlinked_subdir_for_user_scope() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&shared_skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -879,6 +887,7 @@ async fn does_not_loop_on_symlink_cycle_for_user_scope() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -899,6 +908,7 @@ fn loads_skills_via_symlinked_subdir_for_admin_scope() {
|
||||
let outcome = load_skills_from_roots([SkillRoot {
|
||||
path: admin_root.path().to_path_buf(),
|
||||
scope: SkillScope::Admin,
|
||||
plugin_id: None,
|
||||
}]);
|
||||
|
||||
assert!(
|
||||
@@ -915,6 +925,7 @@ fn loads_skills_via_symlinked_subdir_for_admin_scope() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&shared_skill_path),
|
||||
scope: SkillScope::Admin,
|
||||
}]
|
||||
@@ -954,6 +965,7 @@ async fn loads_skills_via_symlinked_subdir_for_repo_scope() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&linked_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -975,6 +987,7 @@ async fn system_scope_ignores_symlinked_subdir() {
|
||||
let outcome = load_skills_from_roots([SkillRoot {
|
||||
path: system_root,
|
||||
scope: SkillScope::System,
|
||||
plugin_id: None,
|
||||
}]);
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
@@ -1005,6 +1018,7 @@ async fn respects_max_scan_depth_for_user_scope() {
|
||||
let outcome = load_skills_from_roots([SkillRoot {
|
||||
path: skills_root,
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
}]);
|
||||
|
||||
assert!(
|
||||
@@ -1021,6 +1035,7 @@ async fn respects_max_scan_depth_for_user_scope() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&within_depth_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -1048,6 +1063,7 @@ async fn loads_valid_skill() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -1080,6 +1096,7 @@ async fn falls_back_to_directory_name_when_skill_name_is_missing() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -1105,6 +1122,7 @@ async fn namespaces_plugin_skills_using_plugin_name() {
|
||||
let outcome = load_skills_from_roots([SkillRoot {
|
||||
path: plugin_root.join("skills"),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some("sample@marketplace".to_string()),
|
||||
}]);
|
||||
|
||||
assert!(
|
||||
@@ -1121,6 +1139,7 @@ async fn namespaces_plugin_skills_using_plugin_name() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: Some("sample@marketplace".to_string()),
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -1152,6 +1171,7 @@ async fn loads_short_description_from_metadata() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -1264,6 +1284,7 @@ async fn loads_skills_from_repo_root() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -1299,6 +1320,7 @@ async fn loads_skills_from_agents_dir_without_codex_dir() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -1352,6 +1374,7 @@ async fn loads_skills_from_all_codex_dirs_under_project_root() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&nested_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
},
|
||||
@@ -1362,6 +1385,7 @@ async fn loads_skills_from_all_codex_dirs_under_project_root() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&root_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
},
|
||||
@@ -1401,6 +1425,7 @@ async fn loads_skills_from_codex_dir_when_not_git_repo() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -1417,10 +1442,12 @@ async fn deduplicates_by_path_preferring_first_root() {
|
||||
SkillRoot {
|
||||
path: root.path().to_path_buf(),
|
||||
scope: SkillScope::Repo,
|
||||
plugin_id: None,
|
||||
},
|
||||
SkillRoot {
|
||||
path: root.path().to_path_buf(),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
},
|
||||
]);
|
||||
|
||||
@@ -1438,6 +1465,7 @@ async fn deduplicates_by_path_preferring_first_root() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -1479,6 +1507,7 @@ async fn keeps_duplicate_names_from_repo_and_user() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&repo_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
},
|
||||
@@ -1489,6 +1518,7 @@ async fn keeps_duplicate_names_from_repo_and_user() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&user_skill_path),
|
||||
scope: SkillScope::User,
|
||||
},
|
||||
@@ -1552,6 +1582,7 @@ async fn keeps_duplicate_names_from_nested_codex_dirs() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: first_path,
|
||||
scope: SkillScope::Repo,
|
||||
},
|
||||
@@ -1562,6 +1593,7 @@ async fn keeps_duplicate_names_from_nested_codex_dirs() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: second_path,
|
||||
scope: SkillScope::Repo,
|
||||
},
|
||||
@@ -1633,6 +1665,7 @@ async fn loads_skills_when_cwd_is_file_in_repo() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -1691,6 +1724,7 @@ async fn loads_skills_from_system_cache_when_present() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::System,
|
||||
}]
|
||||
|
||||
@@ -16,6 +16,7 @@ use crate::build_implicit_skill_path_indexes;
|
||||
use crate::config_rules::SkillConfigRules;
|
||||
use crate::config_rules::resolve_disabled_skill_paths;
|
||||
use crate::config_rules::skill_config_rules_from_stack;
|
||||
use crate::loader::PluginSkillRoot;
|
||||
use crate::loader::SkillRoot;
|
||||
use crate::loader::load_skills_from_roots;
|
||||
use crate::loader::skill_roots;
|
||||
@@ -26,7 +27,7 @@ use codex_config::SkillsConfig;
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct SkillsLoadInput {
|
||||
pub cwd: PathBuf,
|
||||
pub effective_skill_roots: Vec<PathBuf>,
|
||||
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: PathBuf,
|
||||
effective_skill_roots: Vec<PathBuf>,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
config_layer_stack: ConfigLayerStack,
|
||||
bundled_skills_enabled: bool,
|
||||
) -> Self {
|
||||
@@ -154,6 +155,7 @@ impl SkillsManager {
|
||||
.map(|path| SkillRoot {
|
||||
path,
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
}),
|
||||
);
|
||||
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
|
||||
|
||||
@@ -57,6 +57,7 @@ fn test_skill(name: &str, path: PathBuf) -> SkillMetadata {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: path,
|
||||
scope: SkillScope::User,
|
||||
}
|
||||
@@ -122,7 +123,7 @@ fn skills_for_config_with_stack(
|
||||
skills_manager: &SkillsManager,
|
||||
cwd: &TempDir,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
effective_skill_roots: &[PathBuf],
|
||||
effective_skill_roots: &[PluginSkillRoot],
|
||||
) -> SkillLoadOutcome {
|
||||
let skills_input = SkillsLoadInput::new(
|
||||
cwd.path().to_path_buf(),
|
||||
@@ -198,6 +199,10 @@ async fn skills_for_config_disables_plugin_skills_by_name() {
|
||||
.and_then(std::path::Path::parent)
|
||||
.expect("plugin skill should live under a skills root")
|
||||
.to_path_buf();
|
||||
let plugin_skill_root = PluginSkillRoot {
|
||||
path: plugin_skill_root,
|
||||
plugin_id: "sample@test".to_string(),
|
||||
};
|
||||
let skills_manager = SkillsManager::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
@@ -217,6 +222,7 @@ async fn skills_for_config_disables_plugin_skills_by_name() {
|
||||
let skill_path = dunce::canonicalize(skill_path).expect("skill path should canonicalize");
|
||||
|
||||
assert_eq!(skill.path_to_skills_md, skill_path);
|
||||
assert_eq!(skill.plugin_id, Some("sample@test".to_string()));
|
||||
assert!(outcome.disabled_paths.contains(&skill.path_to_skills_md));
|
||||
assert!(
|
||||
!outcome
|
||||
|
||||
@@ -14,6 +14,7 @@ pub struct SkillMetadata {
|
||||
pub interface: Option<SkillInterface>,
|
||||
pub dependencies: Option<SkillDependencies>,
|
||||
pub policy: Option<SkillPolicy>,
|
||||
pub plugin_id: Option<String>,
|
||||
/// Path to the SKILLS.md file that declares this skill.
|
||||
pub path_to_skills_md: PathBuf,
|
||||
pub scope: SkillScope,
|
||||
|
||||
@@ -641,7 +641,7 @@ enabled = false
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
let plugin_outcome = plugins_manager.plugins_for_config(&config);
|
||||
let effective_skill_roots = plugin_outcome.effective_skill_roots();
|
||||
let effective_skill_roots = crate::plugins::effective_plugin_skill_roots(&plugin_outcome);
|
||||
let skills_input = skills_load_input_from_config(&config, effective_skill_roots);
|
||||
let outcome = skills_manager.skills_for_config(&skills_input);
|
||||
let skill = outcome
|
||||
|
||||
@@ -26,6 +26,7 @@ use crate::connectors;
|
||||
use crate::exec_policy::ExecPolicyManager;
|
||||
use crate::parse_turn_item;
|
||||
use crate::path_utils::normalize_for_native_workdir;
|
||||
use crate::plugins::effective_plugin_skill_roots;
|
||||
use crate::realtime_conversation::RealtimeConversationManager;
|
||||
use crate::realtime_conversation::handle_audio as handle_realtime_conversation_audio;
|
||||
use crate::realtime_conversation::handle_close as handle_realtime_conversation_close;
|
||||
@@ -489,7 +490,7 @@ impl Codex {
|
||||
let (tx_event, rx_event) = async_channel::unbounded();
|
||||
|
||||
let plugin_outcome = plugins_manager.plugins_for_config(&config);
|
||||
let effective_skill_roots = plugin_outcome.effective_skill_roots();
|
||||
let effective_skill_roots = effective_plugin_skill_roots(&plugin_outcome);
|
||||
let skills_input = skills_load_input_from_config(&config, effective_skill_roots);
|
||||
let loaded_skills = skills_manager.skills_for_config(&skills_input);
|
||||
|
||||
@@ -2519,7 +2520,7 @@ impl Session {
|
||||
.services
|
||||
.plugins_manager
|
||||
.plugins_for_config(&per_turn_config);
|
||||
let effective_skill_roots = plugin_outcome.effective_skill_roots();
|
||||
let effective_skill_roots = effective_plugin_skill_roots(&plugin_outcome);
|
||||
let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots);
|
||||
let skills_outcome = Arc::new(
|
||||
self.services
|
||||
@@ -5763,6 +5764,7 @@ fn skills_to_info(
|
||||
.collect(),
|
||||
}
|
||||
}),
|
||||
plugin_id: skill.plugin_id.clone(),
|
||||
path: skill.path_to_skills_md.clone(),
|
||||
scope: skill.scope,
|
||||
enabled: !disabled_paths.contains(&skill.path_to_skills_md),
|
||||
|
||||
@@ -2747,7 +2747,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
let plugin_outcome = services
|
||||
.plugins_manager
|
||||
.plugins_for_config(&per_turn_config);
|
||||
let effective_skill_roots = plugin_outcome.effective_skill_roots();
|
||||
let effective_skill_roots = crate::plugins::effective_plugin_skill_roots(&plugin_outcome);
|
||||
let skills_input =
|
||||
crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots);
|
||||
let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&skills_input));
|
||||
@@ -3587,7 +3587,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
|
||||
let plugin_outcome = services
|
||||
.plugins_manager
|
||||
.plugins_for_config(&per_turn_config);
|
||||
let effective_skill_roots = plugin_outcome.effective_skill_roots();
|
||||
let effective_skill_roots = crate::plugins::effective_plugin_skill_roots(&plugin_outcome);
|
||||
let skills_input =
|
||||
crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots);
|
||||
let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&skills_input));
|
||||
|
||||
@@ -37,6 +37,7 @@ use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_rules::SkillConfigRules;
|
||||
use crate::config_rules::resolve_disabled_skill_paths;
|
||||
use crate::config_rules::skill_config_rules_from_stack;
|
||||
use crate::loader::PluginSkillRoot;
|
||||
use crate::loader::SkillRoot;
|
||||
use crate::loader::load_skills_from_roots;
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
@@ -189,6 +190,32 @@ pub struct ConfiguredMarketplaceListOutcome {
|
||||
pub errors: Vec<MarketplaceListError>,
|
||||
}
|
||||
|
||||
pub fn effective_plugin_skill_roots(outcome: &PluginLoadOutcome) -> Vec<PluginSkillRoot> {
|
||||
let mut roots = outcome
|
||||
.plugins()
|
||||
.iter()
|
||||
.filter(|plugin| plugin.is_active())
|
||||
.flat_map(|plugin| {
|
||||
plugin
|
||||
.skill_roots
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|path| PluginSkillRoot {
|
||||
path,
|
||||
plugin_id: plugin.config_name.clone(),
|
||||
})
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
roots.sort_unstable_by(|left, right| {
|
||||
left.path
|
||||
.cmp(&right.path)
|
||||
.then_with(|| left.plugin_id.cmp(&right.plugin_id))
|
||||
});
|
||||
let mut seen_paths = HashSet::new();
|
||||
roots.retain(|root| seen_paths.insert(root.path.clone()));
|
||||
roots
|
||||
}
|
||||
|
||||
impl From<PluginDetail> for PluginCapabilitySummary {
|
||||
fn from(value: PluginDetail) -> Self {
|
||||
let has_skills = value.skills.iter().any(|skill| {
|
||||
@@ -412,12 +439,16 @@ impl PluginsManager {
|
||||
&self,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
plugins_feature_enabled: bool,
|
||||
) -> Vec<PathBuf> {
|
||||
) -> Vec<PluginSkillRoot> {
|
||||
if !plugins_feature_enabled {
|
||||
return Vec::new();
|
||||
}
|
||||
load_plugins_from_layer_stack(config_layer_stack, &self.store, self.restriction_product)
|
||||
.effective_skill_roots()
|
||||
let outcome = load_plugins_from_layer_stack(
|
||||
config_layer_stack,
|
||||
&self.store,
|
||||
self.restriction_product,
|
||||
);
|
||||
effective_plugin_skill_roots(&outcome)
|
||||
}
|
||||
|
||||
fn cached_enabled_outcome(&self) -> Option<PluginLoadOutcome> {
|
||||
@@ -963,6 +994,7 @@ impl PluginsManager {
|
||||
let resolved_skills = load_plugin_skills(
|
||||
source_path.as_path(),
|
||||
manifest_paths,
|
||||
plugin_key.as_str(),
|
||||
self.restriction_product,
|
||||
&skill_config_rules,
|
||||
);
|
||||
@@ -1461,6 +1493,7 @@ fn load_plugin(
|
||||
let resolved_skills = load_plugin_skills(
|
||||
plugin_root.as_path(),
|
||||
manifest_paths,
|
||||
loaded_plugin.config_name.as_str(),
|
||||
restriction_product,
|
||||
skill_config_rules,
|
||||
);
|
||||
@@ -1506,6 +1539,7 @@ impl ResolvedPluginSkills {
|
||||
fn load_plugin_skills(
|
||||
plugin_root: &Path,
|
||||
manifest_paths: &PluginManifestPaths,
|
||||
plugin_id: &str,
|
||||
restriction_product: Option<Product>,
|
||||
skill_config_rules: &SkillConfigRules,
|
||||
) -> ResolvedPluginSkills {
|
||||
@@ -1515,6 +1549,7 @@ fn load_plugin_skills(
|
||||
.map(|path| SkillRoot {
|
||||
path,
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some(plugin_id.to_string()),
|
||||
}),
|
||||
);
|
||||
let had_errors = !outcome.errors.is_empty();
|
||||
|
||||
@@ -189,6 +189,13 @@ fn load_plugins_loads_default_skills_and_mcp_servers() {
|
||||
outcome.effective_skill_roots(),
|
||||
vec![plugin_root.join("skills")]
|
||||
);
|
||||
assert_eq!(
|
||||
effective_plugin_skill_roots(&outcome),
|
||||
vec![crate::loader::PluginSkillRoot {
|
||||
path: plugin_root.join("skills"),
|
||||
plugin_id: "sample@test".to_string(),
|
||||
}]
|
||||
);
|
||||
assert_eq!(outcome.effective_mcp_servers().len(), 1);
|
||||
assert_eq!(
|
||||
outcome.effective_apps(),
|
||||
|
||||
@@ -40,6 +40,7 @@ pub use manager::PluginRemoteSyncError;
|
||||
pub use manager::PluginUninstallError;
|
||||
pub use manager::PluginsManager;
|
||||
pub use manager::RemotePluginSyncResult;
|
||||
pub use manager::effective_plugin_skill_roots;
|
||||
pub use manager::installed_plugin_telemetry_metadata;
|
||||
pub use manager::load_plugin_apps;
|
||||
pub use manager::load_plugin_mcp_servers;
|
||||
|
||||
@@ -2,7 +2,6 @@ use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::env;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::codex::Session;
|
||||
@@ -43,7 +42,7 @@ pub use codex_core_skills::system;
|
||||
|
||||
pub(crate) fn skills_load_input_from_config(
|
||||
config: &Config,
|
||||
effective_skill_roots: Vec<PathBuf>,
|
||||
effective_skill_roots: Vec<codex_core_skills::PluginSkillRoot>,
|
||||
) -> SkillsLoadInput {
|
||||
SkillsLoadInput::new(
|
||||
config.cwd.clone().to_path_buf(),
|
||||
@@ -185,6 +184,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,
|
||||
plugin_id: candidate.plugin_id,
|
||||
invocation_type: InvocationType::Implicit,
|
||||
};
|
||||
let skill_scope = match invocation.skill_scope {
|
||||
|
||||
@@ -17,6 +17,7 @@ use crate::file_watcher::ThrottledWatchReceiver;
|
||||
use crate::file_watcher::WatchPath;
|
||||
use crate::file_watcher::WatchRegistration;
|
||||
use crate::plugins::PluginsManager;
|
||||
use crate::plugins::effective_plugin_skill_roots;
|
||||
use crate::skills_load_input_from_config;
|
||||
|
||||
#[cfg(not(test))]
|
||||
@@ -61,7 +62,7 @@ impl SkillsWatcher {
|
||||
plugins_manager: &PluginsManager,
|
||||
) -> WatchRegistration {
|
||||
let plugin_outcome = plugins_manager.plugins_for_config(config);
|
||||
let effective_skill_roots = plugin_outcome.effective_skill_roots();
|
||||
let effective_skill_roots = effective_plugin_skill_roots(&plugin_outcome);
|
||||
let skills_input = skills_load_input_from_config(config, effective_skill_roots);
|
||||
let roots = skills_manager
|
||||
.skill_roots_for_config(&skills_input)
|
||||
|
||||
@@ -3160,6 +3160,9 @@ pub struct SkillMetadata {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub dependencies: Option<SkillDependencies>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub plugin_id: Option<String>,
|
||||
pub path: PathBuf,
|
||||
pub scope: SkillScope,
|
||||
pub enabled: bool,
|
||||
|
||||
@@ -375,6 +375,7 @@ fn list_skills_response_to_core(response: SkillsListResponse) -> ListSkillsRespo
|
||||
.collect(),
|
||||
}
|
||||
}),
|
||||
plugin_id: skill.plugin_id,
|
||||
path: skill.path,
|
||||
scope: match skill.scope {
|
||||
codex_app_server_protocol::SkillScope::User => {
|
||||
|
||||
@@ -4837,6 +4837,7 @@ mod tests {
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: PathBuf::from("/tmp/repo/google-calendar/SKILL.md"),
|
||||
scope: codex_protocol::protocol::SkillScope::Repo,
|
||||
}]));
|
||||
@@ -4935,6 +4936,7 @@ mod tests {
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: PathBuf::from("/tmp/repo/google-calendar/SKILL.md"),
|
||||
scope: codex_protocol::protocol::SkillScope::Repo,
|
||||
}]));
|
||||
|
||||
@@ -1682,6 +1682,7 @@ mod tests {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: PathBuf::from("test-skill"),
|
||||
scope: SkillScope::User,
|
||||
}]),
|
||||
|
||||
@@ -217,6 +217,7 @@ fn protocol_skill_to_core(skill: &ProtocolSkillMetadata) -> SkillMetadata {
|
||||
.collect(),
|
||||
}),
|
||||
policy: None,
|
||||
plugin_id: skill.plugin_id.clone(),
|
||||
path_to_skills_md: skill.path.clone(),
|
||||
scope: skill.scope,
|
||||
}
|
||||
|
||||
@@ -408,6 +408,7 @@ async fn submission_prefers_selected_duplicate_skill_path() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: repo_skill_path,
|
||||
scope: SkillScope::Repo,
|
||||
},
|
||||
@@ -418,6 +419,7 @@ async fn submission_prefers_selected_duplicate_skill_path() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
plugin_id: None,
|
||||
path_to_skills_md: user_skill_path.clone(),
|
||||
scope: SkillScope::User,
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user