agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: make skills loader tests hermetic with ~/.agents skills (#12474)

Browse Source
This commit is contained in:
Michael Bolin
2026-02-21 14:40:13 -08:00
committed by GitHub
parent 3586fcb802
commit f33ac830aa
1 changed files with 49 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/skills/loader.rs
+49 -33
View File
@@ -137,7 +137,17 @@ impl fmt::Display for SkillParseError {
impl Error for SkillParseError {} impl Error for SkillParseError {}
pub fn load_skills(config: &Config) -> SkillLoadOutcome { pub fn load_skills(config: &Config) -> SkillLoadOutcome {
load_skills_from_roots(skill_roots(config)) load_skills_with_home_dir(config, home_dir().as_deref())
}
fn load_skills_with_home_dir(config: &Config, home_dir: Option<&Path>) -> SkillLoadOutcome {
let mut roots = skill_roots_from_layer_stack_inner(&config.config_layer_stack, home_dir);
roots.extend(repo_agents_skill_roots(
&config.config_layer_stack,
&config.cwd,
));
dedupe_skill_roots_by_path(&mut roots);
load_skills_from_roots(roots)
} }
pub(crate) struct SkillRoot { pub(crate) struct SkillRoot {
@@ -240,6 +250,7 @@ fn skill_roots_from_layer_stack_inner(
roots roots
} }
#[cfg(test)]
fn skill_roots(config: &Config) -> Vec<SkillRoot> { fn skill_roots(config: &Config) -> Vec<SkillRoot> {
skill_roots_from_layer_stack_with_agents(&config.config_layer_stack, &config.cwd) skill_roots_from_layer_stack_with_agents(&config.config_layer_stack, &config.cwd)
} }
@@ -867,6 +878,11 @@ mod tests {
.expect("defaults for test should always succeed") .expect("defaults for test should always succeed")
} }
fn load_skills_for_test(config: &Config) -> SkillLoadOutcome {
// Keep unit tests hermetic by never scanning the real `$HOME/.agents/skills`.
super::load_skills_with_home_dir(config, None)
}
fn mark_as_git_repo(dir: &Path) { fn mark_as_git_repo(dir: &Path) {
// Config/project-root discovery only checks for the presence of `.git` (file or dir), // Config/project-root discovery only checks for the presence of `.git` (file or dir),
// so we can avoid shelling out to `git init` in tests. // so we can avoid shelling out to `git init` in tests.
@@ -1129,7 +1145,7 @@ mod tests {
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1208,7 +1224,7 @@ interface:
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1258,7 +1274,7 @@ policy:
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1289,7 +1305,7 @@ policy: {}
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1332,7 +1348,7 @@ permissions:
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1394,7 +1410,7 @@ permissions: {}
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1452,7 +1468,7 @@ permissions:
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1502,7 +1518,7 @@ permissions:
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1547,7 +1563,7 @@ permissions:
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1595,7 +1611,7 @@ permissions:
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1642,7 +1658,7 @@ permissions:
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1691,7 +1707,7 @@ permissions:
); );
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1736,7 +1752,7 @@ permissions:
symlink_dir(shared.path(), &codex_home.path().join("skills/shared")); symlink_dir(shared.path(), &codex_home.path().join("skills/shared"));
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1773,7 +1789,7 @@ permissions:
symlink_file(&shared_skill_path, &skill_dir.join(SKILLS_FILENAME)); symlink_file(&shared_skill_path, &skill_dir.join(SKILLS_FILENAME));
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1797,7 +1813,7 @@ permissions:
let skill_path = write_skill_at(&cycle_dir, "demo", "cycle-skill", "still loads"); let skill_path = write_skill_at(&cycle_dir, "demo", "cycle-skill", "still loads");
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1875,7 +1891,7 @@ permissions:
symlink_dir(shared.path(), &repo_skills_root.join("shared")); symlink_dir(shared.path(), &repo_skills_root.join("shared"));
let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await; let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -1972,7 +1988,7 @@ permissions:
let skill_path = write_skill(&codex_home, "demo", "demo-skill", "does things\ncarefully"); let skill_path = write_skill(&codex_home, "demo", "demo-skill", "does things\ncarefully");
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2004,7 +2020,7 @@ permissions:
fs::write(&skill_path, contents).unwrap(); fs::write(&skill_path, contents).unwrap();
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2038,7 +2054,7 @@ permissions:
fs::write(skill_dir.join(SKILLS_FILENAME), contents).unwrap(); fs::write(skill_dir.join(SKILLS_FILENAME), contents).unwrap();
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert_eq!(outcome.skills.len(), 0); assert_eq!(outcome.skills.len(), 0);
assert_eq!(outcome.errors.len(), 1); assert_eq!(outcome.errors.len(), 1);
assert!( assert!(
@@ -2067,7 +2083,7 @@ permissions:
fs::write(invalid_dir.join(SKILLS_FILENAME), "---\nname: bad").unwrap(); fs::write(invalid_dir.join(SKILLS_FILENAME), "---\nname: bad").unwrap();
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert_eq!(outcome.skills.len(), 0); assert_eq!(outcome.skills.len(), 0);
assert_eq!(outcome.errors.len(), 1); assert_eq!(outcome.errors.len(), 1);
assert!( assert!(
@@ -2085,7 +2101,7 @@ permissions:
write_skill(&codex_home, "max-len", "max-len", &max_desc); write_skill(&codex_home, "max-len", "max-len", &max_desc);
let cfg = make_config(&codex_home).await; let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2095,7 +2111,7 @@ permissions:
let too_long_desc = "\u{1F4A1}".repeat(MAX_DESCRIPTION_LEN + 1); let too_long_desc = "\u{1F4A1}".repeat(MAX_DESCRIPTION_LEN + 1);
write_skill(&codex_home, "too-long", "too-long", &too_long_desc); write_skill(&codex_home, "too-long", "too-long", &too_long_desc);
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert_eq!(outcome.skills.len(), 1); assert_eq!(outcome.skills.len(), 1);
assert_eq!(outcome.errors.len(), 1); assert_eq!(outcome.errors.len(), 1);
assert!( assert!(
@@ -2117,7 +2133,7 @@ permissions:
let skill_path = write_skill_at(&skills_root, "repo", "repo-skill", "from repo"); let skill_path = write_skill_at(&skills_root, "repo", "repo-skill", "from repo");
let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await; let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2153,7 +2169,7 @@ permissions:
); );
let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await; let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2206,7 +2222,7 @@ permissions:
let cfg = make_config_for_cwd(&codex_home, nested_dir).await; let cfg = make_config_for_cwd(&codex_home, nested_dir).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2258,7 +2274,7 @@ permissions:
let cfg = make_config_for_cwd(&codex_home, work_dir.path().to_path_buf()).await; let cfg = make_config_for_cwd(&codex_home, work_dir.path().to_path_buf()).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2337,7 +2353,7 @@ permissions:
let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await; let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2402,7 +2418,7 @@ permissions:
); );
let cfg = make_config_for_cwd(&codex_home, nested_dir).await; let cfg = make_config_for_cwd(&codex_home, nested_dir).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -2468,7 +2484,7 @@ permissions:
let cfg = make_config_for_cwd(&codex_home, repo_dir).await; let cfg = make_config_for_cwd(&codex_home, repo_dir).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2497,7 +2513,7 @@ permissions:
let cfg = make_config_for_cwd(&codex_home, file_path).await; let cfg = make_config_for_cwd(&codex_home, file_path).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2538,7 +2554,7 @@ permissions:
let cfg = make_config_for_cwd(&codex_home, nested_dir).await; let cfg = make_config_for_cwd(&codex_home, nested_dir).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",
@@ -2556,7 +2572,7 @@ permissions:
let cfg = make_config_for_cwd(&codex_home, work_dir.path().to_path_buf()).await; let cfg = make_config_for_cwd(&codex_home, work_dir.path().to_path_buf()).await;
let outcome = load_skills(&cfg); let outcome = load_skills_for_test(&cfg);
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
"unexpected errors: {:?}", "unexpected errors: {:?}",