mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
test(skills): cover plugin namespace loading (#31369)
## Why Before changing plugin namespace loading performance, lock down the existing behavior so the same cases can be validated before and after the optimization. ## What - cover mixed plain and nested plugin skills under one scan root - cover inherited, nested, and invalid-manifest namespace precedence - cover symlinked plugin skill directories, symlinked plain directories, and scan-root ancestor symlinks ## Validation - just test -p codex-core-skills namespace on unmodified main: 12 passed
This commit is contained in:
committed by
GitHub
parent
f659eb12bc
commit
42156ba007
@@ -388,6 +388,41 @@ fn write_skill_interface_at(skill_dir: &Path, contents: &str) -> PathBuf {
|
||||
write_skill_metadata_at(skill_dir, contents)
|
||||
}
|
||||
|
||||
fn write_plugin_manifest(plugin_root: &Path, contents: &str) {
|
||||
let manifest_path = plugin_root.join(".codex-plugin/plugin.json");
|
||||
fs::create_dir_all(manifest_path.parent().expect("manifest parent")).unwrap();
|
||||
fs::write(manifest_path, contents).unwrap();
|
||||
}
|
||||
|
||||
async fn load_user_skills_root(root: &Path) -> SkillLoadOutcome {
|
||||
load_skills_from_roots(
|
||||
[SkillRoot {
|
||||
path: root.abs(),
|
||||
scope: SkillScope::User,
|
||||
file_system: Arc::clone(&LOCAL_FS),
|
||||
plugin_id: None,
|
||||
plugin_namespace: None,
|
||||
plugin_root: None,
|
||||
}],
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
fn expected_user_skill(path: &Path, name: &str, description: &str) -> SkillMetadata {
|
||||
SkillMetadata {
|
||||
name: name.to_string(),
|
||||
description: description.to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: normalized(path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_skill_dependencies_metadata_from_yaml() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
@@ -1326,6 +1361,246 @@ async fn namespaces_plugin_skills_using_provided_namespace() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn namespaces_nested_plugin_skills_without_namespacing_plain_siblings() {
|
||||
let root = tempfile::tempdir().expect("tempdir");
|
||||
let skills_root = root.path().join("skills");
|
||||
let plain_skill_path =
|
||||
write_skill_at(&skills_root, "plain", "plain-skill", "plain description");
|
||||
let plugin_root = skills_root.join("nested-plugin");
|
||||
write_plugin_manifest(&plugin_root, r#"{"name":"nested"}"#);
|
||||
let plugin_skill_path = write_skill_at(
|
||||
&plugin_root.join("skills"),
|
||||
"search",
|
||||
"plugin-skill",
|
||||
"plugin description",
|
||||
);
|
||||
|
||||
let outcome = load_user_skills_root(&skills_root).await;
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![
|
||||
expected_user_skill(
|
||||
&plugin_skill_path,
|
||||
"nested:plugin-skill",
|
||||
"plugin description"
|
||||
),
|
||||
expected_user_skill(&plain_skill_path, "plain-skill", "plain description"),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn inherits_plugin_namespace_from_above_scanned_skills_root() {
|
||||
let root = tempfile::tempdir().expect("tempdir");
|
||||
let plugin_root = root.path().join("plugin");
|
||||
write_plugin_manifest(&plugin_root, r#"{"name":"outer"}"#);
|
||||
let skills_root = plugin_root.join("skills");
|
||||
let skill_path = write_skill_at(&skills_root, "search", "search-skill", "search description");
|
||||
|
||||
let outcome = load_user_skills_root(&skills_root).await;
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![expected_user_skill(
|
||||
&skill_path,
|
||||
"outer:search-skill",
|
||||
"search description",
|
||||
)]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn nearest_valid_nested_plugin_namespace_overrides_outer_namespace() {
|
||||
let root = tempfile::tempdir().expect("tempdir");
|
||||
let outer_plugin_root = root.path().join("outer-plugin");
|
||||
write_plugin_manifest(&outer_plugin_root, r#"{"name":"outer"}"#);
|
||||
let skills_root = outer_plugin_root.join("skills");
|
||||
let nested_plugin_root = skills_root.join("nested-plugin");
|
||||
write_plugin_manifest(&nested_plugin_root, r#"{"name":"nested"}"#);
|
||||
let skill_path = write_skill_at(
|
||||
&nested_plugin_root.join("skills"),
|
||||
"search",
|
||||
"search-skill",
|
||||
"search description",
|
||||
);
|
||||
|
||||
let outcome = load_user_skills_root(&skills_root).await;
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![expected_user_skill(
|
||||
&skill_path,
|
||||
"nested:search-skill",
|
||||
"search description",
|
||||
)]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn invalid_nested_plugin_manifest_falls_back_to_outer_namespace() {
|
||||
let root = tempfile::tempdir().expect("tempdir");
|
||||
let outer_plugin_root = root.path().join("outer-plugin");
|
||||
write_plugin_manifest(&outer_plugin_root, r#"{"name":"outer"}"#);
|
||||
let skills_root = outer_plugin_root.join("skills");
|
||||
let nested_plugin_root = skills_root.join("nested-plugin");
|
||||
write_plugin_manifest(&nested_plugin_root, "not json");
|
||||
let skill_path = write_skill_at(
|
||||
&nested_plugin_root.join("skills"),
|
||||
"search",
|
||||
"search-skill",
|
||||
"search description",
|
||||
);
|
||||
|
||||
let outcome = load_user_skills_root(&skills_root).await;
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![expected_user_skill(
|
||||
&skill_path,
|
||||
"outer:search-skill",
|
||||
"search description",
|
||||
)]
|
||||
);
|
||||
}
|
||||
|
||||
// Directory symlinks on Windows can require Developer Mode or administrator privileges.
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn namespaces_skills_in_symlinked_plugin_skills_dir() {
|
||||
// skills/
|
||||
// └── linked-plugin -> shared-plugin/skills/
|
||||
// shared-plugin/
|
||||
// ├── .codex-plugin/plugin.json
|
||||
// └── skills/search/SKILL.md
|
||||
let root = tempfile::tempdir().expect("tempdir");
|
||||
let shared_plugin_root = tempfile::tempdir().expect("tempdir");
|
||||
write_plugin_manifest(shared_plugin_root.path(), r#"{"name":"linked"}"#);
|
||||
let skill_path = write_skill_at(
|
||||
&shared_plugin_root.path().join("skills"),
|
||||
"search",
|
||||
"search-skill",
|
||||
"search description",
|
||||
);
|
||||
let skills_root = root.path().join("skills");
|
||||
fs::create_dir_all(&skills_root).unwrap();
|
||||
symlink_dir(
|
||||
&shared_plugin_root.path().join("skills"),
|
||||
&skills_root.join("linked-plugin"),
|
||||
);
|
||||
|
||||
let outcome = load_user_skills_root(&skills_root).await;
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![expected_user_skill(
|
||||
&skill_path,
|
||||
"linked:search-skill",
|
||||
"search description",
|
||||
)]
|
||||
);
|
||||
}
|
||||
|
||||
// Directory symlinks on Windows can require Developer Mode or administrator privileges.
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn does_not_inherit_namespace_for_skills_in_symlinked_plain_dir() {
|
||||
// outer-plugin/
|
||||
// ├── .codex-plugin/plugin.json
|
||||
// └── skills/linked-plain -> plain-root/
|
||||
// plain-root/
|
||||
// └── search/SKILL.md
|
||||
let root = tempfile::tempdir().expect("tempdir");
|
||||
let plugin_root = root.path().join("outer-plugin");
|
||||
write_plugin_manifest(&plugin_root, r#"{"name":"outer"}"#);
|
||||
let skills_root = plugin_root.join("skills");
|
||||
let plain_root = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_skill_at(
|
||||
plain_root.path(),
|
||||
"search",
|
||||
"plain-skill",
|
||||
"plain description",
|
||||
);
|
||||
fs::create_dir_all(&skills_root).unwrap();
|
||||
symlink_dir(plain_root.path(), &skills_root.join("linked-plain"));
|
||||
|
||||
let outcome = load_user_skills_root(&skills_root).await;
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![expected_user_skill(
|
||||
&skill_path,
|
||||
"plain-skill",
|
||||
"plain description",
|
||||
)]
|
||||
);
|
||||
}
|
||||
|
||||
// Directory symlinks on Windows can require Developer Mode or administrator privileges.
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn keeps_inherited_namespace_when_symlink_target_is_scan_root_ancestor() {
|
||||
// temp-root/
|
||||
// └── a/b/c/d/e/f/outer-plugin/
|
||||
// ├── .codex-plugin/plugin.json
|
||||
// └── skills/
|
||||
// ├── root/SKILL.md
|
||||
// └── link -> temp-root/
|
||||
let root = tempfile::tempdir().expect("tempdir");
|
||||
let plugin_root = root.path().join("a/b/c/d/e/f/outer-plugin");
|
||||
write_plugin_manifest(&plugin_root, r#"{"name":"outer"}"#);
|
||||
let skills_root = plugin_root.join("skills");
|
||||
let skill_path = write_skill_at(&skills_root, "root", "root-skill", "root description");
|
||||
symlink_dir(root.path(), &skills_root.join("link"));
|
||||
|
||||
let outcome = load_user_skills_root(&skills_root).await;
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![expected_user_skill(
|
||||
&skill_path,
|
||||
"outer:root-skill",
|
||||
"root description",
|
||||
)]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_skill_name_length_limit_allows_max_qualified_name() {
|
||||
let root = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
Reference in New Issue
Block a user