From ed9eb9b7ff7144b83a001a208cdaf5d452ac1b40 Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Sat, 20 Dec 2025 23:50:41 +0100 Subject: [PATCH] Follow symlinked skills Closes #8369 --- codex-rs/core/src/skills/loader.rs | 68 +++++++++++++++++++++++++++++- docs/skills.md | 2 +- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index bce13fbb057..832dcdcaf1c 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -179,7 +179,8 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil return; } - let mut queue: VecDeque = VecDeque::from([root]); + let mut queue: VecDeque = VecDeque::from([root.clone()]); + let mut visited: HashSet = HashSet::from([root]); while let Some(dir) = queue.pop_front() { let entries = match fs::read_dir(&dir) { Ok(entries) => entries, @@ -205,11 +206,45 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil }; if file_type.is_symlink() { + let Ok(target_metadata) = fs::metadata(&path) else { + continue; + }; + + if target_metadata.is_dir() { + let Ok(target_path) = normalize_path(&path) else { + continue; + }; + if visited.insert(target_path.clone()) { + queue.push_back(target_path); + } + continue; + } + + if target_metadata.is_file() && file_name == SKILLS_FILENAME { + match parse_skill_file(&path, scope) { + Ok(skill) => { + outcome.skills.push(skill); + } + Err(err) => { + if scope != SkillScope::System { + outcome.errors.push(SkillError { + path, + message: err.to_string(), + }); + } + } + } + } continue; } if file_type.is_dir() { - queue.push_back(path); + let Ok(dir_path) = normalize_path(&path) else { + continue; + }; + if visited.insert(dir_path.clone()) { + queue.push_back(dir_path); + } continue; } @@ -463,6 +498,35 @@ mod tests { ); } + #[cfg(unix)] + #[tokio::test] + async fn follows_symlinked_skill_dirs() { + use std::os::unix::fs::symlink; + + let codex_home = tempfile::tempdir().expect("tempdir"); + let external_root = codex_home.path().join("external-skill"); + let external_skill = + write_skill_at(&external_root, "linked", "linked-skill", "via symlink"); + let link_root = codex_home.path().join("skills/link"); + fs::create_dir_all(link_root.parent().unwrap()).unwrap(); + symlink(external_skill.parent().unwrap(), &link_root).unwrap(); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills(&cfg); + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!(outcome.skills.len(), 1); + assert_eq!(outcome.skills[0].name, "linked-skill"); + let path_str = outcome.skills[0].path.to_string_lossy().replace('\\', "/"); + assert!( + path_str.ends_with("external-skill/linked/SKILL.md"), + "unexpected path {path_str}" + ); + } + #[tokio::test] async fn enforces_length_limits() { let codex_home = tempfile::tempdir().expect("tempdir"); diff --git a/docs/skills.md b/docs/skills.md index 5a9f17f0b07..276d954da1b 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -19,7 +19,7 @@ Skills are behind the experimental `skills` feature flag and are disabled by def ## Where skills live -- Location (v1): `~/.codex/skills/**/SKILL.md` (recursive). Hidden entries and symlinks are skipped. Only files named exactly `SKILL.md` count. +- Location (v1): `~/.codex/skills/**/SKILL.md` (recursive). Hidden entries are skipped; symlinks are followed. Only files named exactly `SKILL.md` count. - Sorting: rendered by name, then path for stability. ## File format