fix(skills): follow symlinks when discovering skills in external dirs - #54200
liuhao1024 wants to merge 4 commits into
Conversation
`_find_skill` and `_find_skill_in_other_profiles` used
`Path.rglob("SKILL.md")` which does not follow symlinks. Skills
installed as directory symlinks in `skills.external_dirs` were
visible to `skills_list` and `skill_view` (which use
`os.walk(followlinks=True)` via `iter_skill_index_files`) but
invisible to `skill_manage` operations.
Replace both `rglob` calls with `iter_skill_index_files` from
`agent.skill_utils`, which already handles symlink traversal and
directory exclusion.
Fixes NousResearch#54195
alt-glitch
added
type/bug
Duplicate of #35244 — same fix (swap |
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Fixes skill discovery to follow symlinks in external dirs (57 additions). Uses iter_skill_index_files instead of rglob to match the behavior of skills_list and skill_view. Includes tests for both symlinked and direct directories.
Reviewed by Hermes Agent
…in_other_profiles The iter_skill_index_files migration removed the hermes_constants import but the function still calls get_default_hermes_root() to locate profile directories. Without the import, NameError is caught by the bare except and _find_skill_in_other_profiles always returns empty, breaking the cross_profile=True hint in _skill_not_found_error.
|
This appears to duplicate #35244 (by Linux2010, opened 2026-05-30) — same fix ( #35244 has slightly broader test coverage (4 tests including nested symlink scenario). Both PRs touch Recommend closing this one in favor of #35244 since it was opened first and has more comprehensive tests. |
|
Duplicate of #35244 by @Linux2010 — same fix (replace Closing this PR in favor of #35244. |
3 participants
What does this PR do?
_find_skilland_find_skill_in_other_profilesusedPath.rglob("SKILL.md")which does not follow symlinks. Skills installed as directory symlinks inskills.external_dirswere visible toskills_listandskill_view(which useos.walk(followlinks=True)viaiter_skill_index_files) but invisible toskill_manageoperations likepatch,edit,delete, andwrite_file.This PR replaces both
rglobcalls withiter_skill_index_filesfromagent.skill_utils, which already handles symlink traversal and directory exclusion consistently with the rest of the skill system.Related Issue
Fixes #54195
Type of Change
Changes Made
tools/skill_manager_tool.py: Replaceskills_dir.rglob("SKILL.md")withiter_skill_index_files(skills_dir, "SKILL.md")in_find_skill()and_find_skill_in_other_profiles()to follow symlinks during skill discoverytests/tools/test_skill_manager_tool.py: AddTestFindSkillSymlinkedExternalDirwith 2 tests — symlink discovery and direct-dir regression guardHow to Test
mkdir -p /tmp/project/skills/my-skill && echo -e '---\nname: my-skill\ndescription: test\n---\n\nBody.' > /tmp/project/skills/my-skill/SKILL.mdmkdir -p ~/.agents/skills && ln -s /tmp/project/skills/my-skill ~/.agents/skills/my-skillskills:\n external_dirs:\n - ~/.agents/skillsskills_list()includesmy-skill— should pass (already worked before)skill_manage(action="patch", name="my-skill", old_string="__DOES_NOT_EXIST__", new_string="x")finds the skill and reports "Could not find a match" instead of "Skill 'my-skill' not found"python -m pytest tests/tools/test_skill_manager_tool.py::TestFindSkillSymlinkedExternalDir -xvs— should passChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A