fix(skills): follow symlinked category dirs consistently
This commit is contained in:
@@ -345,7 +345,7 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]:
|
|||||||
_skill_commands = {}
|
_skill_commands = {}
|
||||||
try:
|
try:
|
||||||
from tools.skills_tool import SKILLS_DIR, _parse_frontmatter, skill_matches_platform, _get_disabled_skill_names
|
from tools.skills_tool import SKILLS_DIR, _parse_frontmatter, skill_matches_platform, _get_disabled_skill_names
|
||||||
from agent.skill_utils import get_external_skills_dirs
|
from agent.skill_utils import get_external_skills_dirs, iter_skill_index_files
|
||||||
disabled = _get_disabled_skill_names()
|
disabled = _get_disabled_skill_names()
|
||||||
seen_names: set = set()
|
seen_names: set = set()
|
||||||
|
|
||||||
@@ -356,7 +356,7 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]:
|
|||||||
dirs_to_scan.extend(get_external_skills_dirs())
|
dirs_to_scan.extend(get_external_skills_dirs())
|
||||||
|
|
||||||
for scan_dir in dirs_to_scan:
|
for scan_dir in dirs_to_scan:
|
||||||
for skill_md in scan_dir.rglob("SKILL.md"):
|
for skill_md in iter_skill_index_files(scan_dir, "SKILL.md"):
|
||||||
if any(part in ('.git', '.github', '.hub') for part in skill_md.parts):
|
if any(part in ('.git', '.github', '.hub') for part in skill_md.parts):
|
||||||
continue
|
continue
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -38,6 +38,18 @@ description: Description for {name}.
|
|||||||
return skill_dir
|
return skill_dir
|
||||||
|
|
||||||
|
|
||||||
|
def _symlink_category(skills_dir: Path, linked_root: Path, category: str) -> Path:
|
||||||
|
"""Create a category symlink under skills_dir pointing outside the tree."""
|
||||||
|
external_category = linked_root / category
|
||||||
|
external_category.mkdir(parents=True, exist_ok=True)
|
||||||
|
symlink_path = skills_dir / category
|
||||||
|
try:
|
||||||
|
symlink_path.symlink_to(external_category, target_is_directory=True)
|
||||||
|
except (OSError, NotImplementedError) as exc:
|
||||||
|
pytest.skip(f"symlinks unavailable in test environment: {exc}")
|
||||||
|
return external_category
|
||||||
|
|
||||||
|
|
||||||
class TestScanSkillCommands:
|
class TestScanSkillCommands:
|
||||||
def test_finds_skills(self, tmp_path):
|
def test_finds_skills(self, tmp_path):
|
||||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||||
@@ -101,6 +113,20 @@ class TestScanSkillCommands:
|
|||||||
assert "/enabled-skill" in result
|
assert "/enabled-skill" in result
|
||||||
assert "/disabled-skill" not in result
|
assert "/disabled-skill" not in result
|
||||||
|
|
||||||
|
def test_finds_skills_in_symlinked_category_dir(self, tmp_path):
|
||||||
|
external_root = tmp_path / "repo"
|
||||||
|
skills_root = tmp_path / "skills"
|
||||||
|
skills_root.mkdir()
|
||||||
|
|
||||||
|
external_category = _symlink_category(skills_root, external_root, "linked")
|
||||||
|
_make_skill(external_category.parent, "knowledge-brain", category="linked")
|
||||||
|
|
||||||
|
with patch("tools.skills_tool.SKILLS_DIR", skills_root):
|
||||||
|
result = scan_skill_commands()
|
||||||
|
|
||||||
|
assert "/knowledge-brain" in result
|
||||||
|
assert result["/knowledge-brain"]["name"] == "knowledge-brain"
|
||||||
|
|
||||||
|
|
||||||
def test_special_chars_stripped_from_cmd_key(self, tmp_path):
|
def test_special_chars_stripped_from_cmd_key(self, tmp_path):
|
||||||
"""Skill names with +, /, or other special chars produce clean cmd keys."""
|
"""Skill names with +, /, or other special chars produce clean cmd keys."""
|
||||||
|
|||||||
@@ -44,6 +44,18 @@ description: Description for {name}.
|
|||||||
return skill_dir
|
return skill_dir
|
||||||
|
|
||||||
|
|
||||||
|
def _symlink_category(skills_dir: Path, linked_root: Path, category: str) -> Path:
|
||||||
|
"""Create a category symlink under skills_dir pointing outside the tree."""
|
||||||
|
external_category = linked_root / category
|
||||||
|
external_category.mkdir(parents=True, exist_ok=True)
|
||||||
|
symlink_path = skills_dir / category
|
||||||
|
try:
|
||||||
|
symlink_path.symlink_to(external_category, target_is_directory=True)
|
||||||
|
except (OSError, NotImplementedError) as exc:
|
||||||
|
pytest.skip(f"symlinks unavailable in test environment: {exc}")
|
||||||
|
return external_category
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# _parse_frontmatter
|
# _parse_frontmatter
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -255,6 +267,20 @@ class TestFindAllSkills:
|
|||||||
assert len(skills) == 1
|
assert len(skills) == 1
|
||||||
assert skills[0]["name"] == "real-skill"
|
assert skills[0]["name"] == "real-skill"
|
||||||
|
|
||||||
|
def test_finds_skills_in_symlinked_category_dir(self, tmp_path):
|
||||||
|
external_root = tmp_path / "repo"
|
||||||
|
skills_root = tmp_path / "skills"
|
||||||
|
skills_root.mkdir()
|
||||||
|
|
||||||
|
external_category = _symlink_category(skills_root, external_root, "linked")
|
||||||
|
_make_skill(external_category.parent, "knowledge-brain", category="linked")
|
||||||
|
|
||||||
|
with patch("tools.skills_tool.SKILLS_DIR", skills_root):
|
||||||
|
skills = _find_all_skills()
|
||||||
|
|
||||||
|
assert [s["name"] for s in skills] == ["knowledge-brain"]
|
||||||
|
assert skills[0]["category"] == "linked"
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# skills_list
|
# skills_list
|
||||||
@@ -288,6 +314,23 @@ class TestSkillsList:
|
|||||||
assert result["count"] == 1
|
assert result["count"] == 1
|
||||||
assert result["skills"][0]["name"] == "skill-a"
|
assert result["skills"][0]["name"] == "skill-a"
|
||||||
|
|
||||||
|
def test_category_filter_finds_symlinked_category(self, tmp_path):
|
||||||
|
external_root = tmp_path / "repo"
|
||||||
|
skills_root = tmp_path / "skills"
|
||||||
|
skills_root.mkdir()
|
||||||
|
|
||||||
|
external_category = _symlink_category(skills_root, external_root, "linked")
|
||||||
|
_make_skill(external_category.parent, "knowledge-brain", category="linked")
|
||||||
|
|
||||||
|
with patch("tools.skills_tool.SKILLS_DIR", skills_root):
|
||||||
|
raw = skills_list(category="linked")
|
||||||
|
|
||||||
|
result = json.loads(raw)
|
||||||
|
assert result["success"] is True
|
||||||
|
assert result["count"] == 1
|
||||||
|
assert result["categories"] == ["linked"]
|
||||||
|
assert result["skills"][0]["name"] == "knowledge-brain"
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# skill_view
|
# skill_view
|
||||||
@@ -389,6 +432,35 @@ class TestSkillView:
|
|||||||
result = json.loads(raw)
|
result = json.loads(raw)
|
||||||
assert result["success"] is True
|
assert result["success"] is True
|
||||||
|
|
||||||
|
def test_view_finds_skill_in_symlinked_category_dir(self, tmp_path):
|
||||||
|
external_root = tmp_path / "repo"
|
||||||
|
skills_root = tmp_path / "skills"
|
||||||
|
skills_root.mkdir()
|
||||||
|
|
||||||
|
external_category = _symlink_category(skills_root, external_root, "linked")
|
||||||
|
_make_skill(external_category.parent, "knowledge-brain", category="linked")
|
||||||
|
|
||||||
|
with patch("tools.skills_tool.SKILLS_DIR", skills_root):
|
||||||
|
raw = skill_view("knowledge-brain")
|
||||||
|
|
||||||
|
result = json.loads(raw)
|
||||||
|
assert result["success"] is True
|
||||||
|
assert result["name"] == "knowledge-brain"
|
||||||
|
|
||||||
|
def test_not_found_hint_uses_same_order_as_skills_list(self, tmp_path):
|
||||||
|
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||||
|
_make_skill(tmp_path, "zeta", category="z-cat")
|
||||||
|
_make_skill(tmp_path, "alpha", category="a-cat")
|
||||||
|
_make_skill(tmp_path, "beta", category="a-cat")
|
||||||
|
|
||||||
|
list_result = json.loads(skills_list())
|
||||||
|
view_result = json.loads(skill_view("missing-skill"))
|
||||||
|
|
||||||
|
assert view_result["success"] is False
|
||||||
|
assert view_result["available_skills"] == [
|
||||||
|
skill["name"] for skill in list_result["skills"]
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
class TestSkillViewSecureSetupOnLoad:
|
class TestSkillViewSecureSetupOnLoad:
|
||||||
def test_requests_missing_required_env_and_continues(self, tmp_path, monkeypatch):
|
def test_requests_missing_required_env_and_continues(self, tmp_path, monkeypatch):
|
||||||
|
|||||||
@@ -554,7 +554,7 @@ def _find_all_skills(*, skip_disabled: bool = False) -> List[Dict[str, Any]]:
|
|||||||
Returns:
|
Returns:
|
||||||
List of skill metadata dicts (name, description, category).
|
List of skill metadata dicts (name, description, category).
|
||||||
"""
|
"""
|
||||||
from agent.skill_utils import get_external_skills_dirs
|
from agent.skill_utils import get_external_skills_dirs, iter_skill_index_files
|
||||||
|
|
||||||
skills = []
|
skills = []
|
||||||
seen_names: set = set()
|
seen_names: set = set()
|
||||||
@@ -569,7 +569,7 @@ def _find_all_skills(*, skip_disabled: bool = False) -> List[Dict[str, Any]]:
|
|||||||
dirs_to_scan.extend(get_external_skills_dirs())
|
dirs_to_scan.extend(get_external_skills_dirs())
|
||||||
|
|
||||||
for scan_dir in dirs_to_scan:
|
for scan_dir in dirs_to_scan:
|
||||||
for skill_md in scan_dir.rglob("SKILL.md"):
|
for skill_md in iter_skill_index_files(scan_dir, "SKILL.md"):
|
||||||
if any(part in _EXCLUDED_SKILL_DIRS for part in skill_md.parts):
|
if any(part in _EXCLUDED_SKILL_DIRS for part in skill_md.parts):
|
||||||
continue
|
continue
|
||||||
|
|
||||||
@@ -620,6 +620,11 @@ def _find_all_skills(*, skip_disabled: bool = False) -> List[Dict[str, Any]]:
|
|||||||
return skills
|
return skills
|
||||||
|
|
||||||
|
|
||||||
|
def _sort_skills(skills: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
|
||||||
|
"""Keep every skill listing path ordered the same way."""
|
||||||
|
return sorted(skills, key=lambda s: (s.get("category") or "", s["name"]))
|
||||||
|
|
||||||
|
|
||||||
def _load_category_description(category_dir: Path) -> Optional[str]:
|
def _load_category_description(category_dir: Path) -> Optional[str]:
|
||||||
"""
|
"""
|
||||||
Load category description from DESCRIPTION.md if it exists.
|
Load category description from DESCRIPTION.md if it exists.
|
||||||
@@ -709,7 +714,7 @@ def skills_list(category: str = None, task_id: str = None) -> str:
|
|||||||
all_skills = [s for s in all_skills if s.get("category") == category]
|
all_skills = [s for s in all_skills if s.get("category") == category]
|
||||||
|
|
||||||
# Sort by category then name
|
# Sort by category then name
|
||||||
all_skills.sort(key=lambda s: (s.get("category") or "", s["name"]))
|
all_skills = _sort_skills(all_skills)
|
||||||
|
|
||||||
# Extract unique categories
|
# Extract unique categories
|
||||||
categories = sorted(
|
categories = sorted(
|
||||||
@@ -926,7 +931,9 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str:
|
|||||||
# Search by directory name across all dirs
|
# Search by directory name across all dirs
|
||||||
if not skill_md:
|
if not skill_md:
|
||||||
for search_dir in all_dirs:
|
for search_dir in all_dirs:
|
||||||
for found_skill_md in search_dir.rglob("SKILL.md"):
|
from agent.skill_utils import iter_skill_index_files
|
||||||
|
|
||||||
|
for found_skill_md in iter_skill_index_files(search_dir, "SKILL.md"):
|
||||||
if found_skill_md.parent.name == name:
|
if found_skill_md.parent.name == name:
|
||||||
skill_dir = found_skill_md.parent
|
skill_dir = found_skill_md.parent
|
||||||
skill_md = found_skill_md
|
skill_md = found_skill_md
|
||||||
@@ -945,7 +952,7 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str:
|
|||||||
break
|
break
|
||||||
|
|
||||||
if not skill_md or not skill_md.exists():
|
if not skill_md or not skill_md.exists():
|
||||||
available = [s["name"] for s in _find_all_skills()[:20]]
|
available = [s["name"] for s in _sort_skills(_find_all_skills())[:20]]
|
||||||
return json.dumps(
|
return json.dumps(
|
||||||
{
|
{
|
||||||
"success": False,
|
"success": False,
|
||||||
|
|||||||
Reference in New Issue
Block a user