Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions tests/tools/test_skill_symlink_discovery.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
"""Regression test: skill discovery follows symlinked category directories (#35184)."""

import os
import tempfile
from pathlib import Path
from unittest.mock import patch

from tools.skill_manager_tool import _find_skill


def test_find_skill_follows_symlinks():
"""#35184: _find_skill should discover skills under symlinked
category directories, not just direct subdirectories.

pathlib.Path.rglob() does not follow symlinks into directories,
so skills in symlinked categories were invisible. The fix uses
iter_skill_index_files() which calls os.walk(followlinks=True).
"""
with tempfile.TemporaryDirectory() as tmp:
# Create a real skills dir
skills_dir = Path(tmp) / "skills"
skills_dir.mkdir()

# Create a skill in a real subdirectory
real_cat = skills_dir / "real-cat"
real_cat.mkdir()
skill_dir = real_cat / "my-symlinked-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text("# My Skill")

# Create a skill in an external location
alt_location = Path(tmp) / "alt-skills"
alt_location.mkdir()
symlinked_cat = alt_location / "symlinked-cat"
symlinked_cat.mkdir()
symlinked_skill = symlinked_cat / "my-symlinked-skill-2"
symlinked_skill.mkdir()
(symlinked_skill / "SKILL.md").write_text("# Symlinked Skill")

# Symlink the external category into skills dir
symlink = skills_dir / "linked-cat"
os.symlink(symlinked_cat, symlink, target_is_directory=True)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please handle OSError and NotImplementedError here and skip when symlinks are unavailable. The established helper in tests/tools/test_skills_tool.py:47-55 keeps these tests portable to restricted Windows environments.


with patch("agent.skill_utils.get_all_skills_dirs", return_value=[skills_dir]):
# Direct (non-symlinked) skill — always worked
result1 = _find_skill("my-symlinked-skill")
assert result1 is not None, "Direct skill should be found"
assert result1["path"] == skill_dir

# Symlinked skill — was broken with rglob, fixed with iter_skill_index_files
result2 = _find_skill("my-symlinked-skill-2")
assert result2 is not None, (
"Skill under symlinked category should be found "
"(regression: rglob does not follow symlinks)"
)
assert result2["path"].resolve() == symlinked_skill.resolve(), (
f"Expected {symlinked_skill.resolve()}, got {result2['path'].resolve()}"
)
12 changes: 4 additions & 8 deletions tools/skill_manager_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,13 +283,11 @@ def _find_skill(name: str) -> Optional[Dict[str, Any]]:
external dirs configured via skills.external_dirs. Returns
{"path": Path} or None.
"""
from agent.skill_utils import get_all_skills_dirs, is_excluded_skill_path
from agent.skill_utils import get_all_skills_dirs, iter_skill_index_files
for skills_dir in get_all_skills_dirs():
if not skills_dir.exists():
continue
for skill_md in skills_dir.rglob("SKILL.md"):
if is_excluded_skill_path(skill_md):
continue
for skill_md in iter_skill_index_files(skills_dir, "SKILL.md"):
if skill_md.parent.name == name:
return {"path": skill_md.parent}
return None
Expand All @@ -307,7 +305,7 @@ def _find_skill_in_other_profiles(name: str) -> List[Tuple[str, Path]]:
matches: List[Tuple[str, Path]] = []
try:
from hermes_constants import get_default_hermes_root
from agent.skill_utils import is_excluded_skill_path
from agent.skill_utils import iter_skill_index_files
except Exception:
return matches

Expand Down Expand Up @@ -350,9 +348,7 @@ def _find_skill_in_other_profiles(name: str) -> List[Tuple[str, Path]]:
if not skills_dir.is_dir():
continue
try:
for skill_md in skills_dir.rglob("SKILL.md"):
if is_excluded_skill_path(skill_md):
continue
for skill_md in iter_skill_index_files(skills_dir, "SKILL.md"):
if skill_md.parent.name == name:
matches.append((profile_name, skill_md.parent))
break # one match per profile is enough
Expand Down
Loading