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
2 changes: 1 addition & 1 deletion skillclaw/nacos_skill_hub.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ def pull_skills(
}

local_dirs_by_name = {}
for skill_md in Path(skills_dir).glob("**/SKILL.md" if _is_hermes_skill_root(skills_dir) else "*/SKILL.md"):
for skill_md in Path(skills_dir).glob("**/SKILL.md"):
local_dirs_by_name.setdefault(skill_md.parent.name, []).append(str(skill_md.parent))

downloaded = skipped = deleted = failed = 0
Expand Down
22 changes: 21 additions & 1 deletion skillclaw/skill_hub.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,27 @@


def _is_hermes_skill_root(skills_dir: str) -> bool:
return os.path.realpath(skills_dir) == os.path.realpath(os.path.join(os.path.expanduser("~"), ".hermes", "skills"))
skills = os.path.realpath(skills_dir)
# Match any .hermes/skills or .hermes/profiles/*/skills path
parts = skills.split(os.sep)
try:
hermes_idx = parts.index(".hermes")
except ValueError:
return False
if hermes_idx + 2 > len(parts):
return False
# .hermes/skills
if parts[hermes_idx + 1] == "skills" and hermes_idx + 2 == len(parts):
return True
# .hermes/profiles/<name>/skills
if (
hermes_idx + 4 <= len(parts)
and parts[hermes_idx + 1] == "profiles"
and parts[hermes_idx + 3] == "skills"
and hermes_idx + 4 == len(parts)
):
return True
return False


def _skill_dir_for_root(skills_dir: str, skill_name: str, category: str = "general") -> str:
Expand Down
31 changes: 23 additions & 8 deletions skillclaw/skill_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,8 @@ def _load_skills(self) -> Dict[str, Any]:
return result

def _skill_md_paths(self) -> list[str]:
if self._is_hermes_skill_root():
pattern = os.path.join(self._skills_dir, "**", "SKILL.md")
return sorted(glob.glob(pattern, recursive=True))
pattern = os.path.join(self._skills_dir, "*", "SKILL.md")
return sorted(glob.glob(pattern))
pattern = os.path.join(self._skills_dir, "**", "SKILL.md")
return sorted(glob.glob(pattern, recursive=True))

def _compute_skills_fingerprint(self) -> tuple[tuple[str, int, int], ...]:
fingerprint: list[tuple[str, int, int]] = []
Expand All @@ -332,9 +329,27 @@ def _compute_skills_fingerprint(self) -> tuple[tuple[str, int, int], ...]:
return tuple(fingerprint)

def _is_hermes_skill_root(self) -> bool:
return os.path.realpath(self._skills_dir) == os.path.realpath(
os.path.join(os.path.expanduser("~"), ".hermes", "skills")
)
skills = os.path.realpath(self._skills_dir)
# Match any .hermes/skills or .hermes/profiles/*/skills path
parts = skills.split(os.sep)
try:
hermes_idx = parts.index(".hermes")
except ValueError:
return False
if hermes_idx + 2 > len(parts):
return False
# .hermes/skills
if parts[hermes_idx + 1] == "skills" and hermes_idx + 2 == len(parts):
return True
# .hermes/profiles/<name>/skills
if (
hermes_idx + 4 <= len(parts)
and parts[hermes_idx + 1] == "profiles"
and parts[hermes_idx + 3] == "skills"
and hermes_idx + 4 == len(parts)
):
return True
return False

def _skill_dir_path(self, skill: dict) -> str:
name = str(skill.get("name", "unknown") or "unknown").strip()
Expand Down
147 changes: 147 additions & 0 deletions tests/test_recursive_skill_discovery.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
"""
Regression tests for recursive SKILL.md discovery.

Covers:
- ~/.hermes/skills/category/skill/SKILL.md
- ~/.hermes/profiles/lightning-node/skills/category/skill/SKILL.md
- depth-3 skills
- flat skills
- unrelated directories excluded
- symlink duplicates not double-counted
- real-world: all 91 production skills detected
"""

import os
import shutil
import tempfile
from pathlib import Path

import pytest

from skillclaw.skill_manager import SkillManager


def _write_skill(skill_dir: str, name: str, description: str, category: str = "general") -> str:
"""Write a minimal SKILL.md and return its path."""
skill_path = os.path.join(skill_dir, name)
os.makedirs(skill_path, exist_ok=True)
md_path = os.path.join(skill_path, "SKILL.md")
with open(md_path, "w") as f:
f.write(f"---\nname: {name}\ndescription: \"{description}\"\n")
if category != "general":
f.write(f"category: {category}\n")
f.write("---\n\n# {name}\n")
return md_path


class TestRecursiveSkillDiscovery:
"""Prove that _skill_md_paths finds all skills regardless of directory layout."""

def test_hermes_default_skills_root(self):
"""~/.hermes/skills/category/skill/SKILL.md is found."""
with tempfile.TemporaryDirectory() as td:
hermes_skills = os.path.join(td, ".hermes", "skills")
_write_skill(hermes_skills, "my-skill", "A test skill", category="coding")
sm = SkillManager(hermes_skills)
names = {s["name"] for s in sm.skills["all_skills"]}
assert "my-skill" in names

def test_hermes_profile_skills_root(self):
"""~/.hermes/profiles/lightning-node/skills/category/skill/SKILL.md is found."""
with tempfile.TemporaryDirectory() as td:
profile_skills = os.path.join(td, ".hermes", "profiles", "lightning-node", "skills")
_write_skill(profile_skills, "lightning-ops", "Lightning node operations", category="infrastructure")
sm = SkillManager(profile_skills)
names = {s["name"] for s in sm.skills["all_skills"]}
assert "lightning-ops" in names
# Verify _is_hermes_skill_root recognises profile dirs
assert sm._is_hermes_skill_root()

def test_depth_3_skills_found(self):
"""depth-3 skills (category/subcategory/skill/SKILL.md) are found."""
with tempfile.TemporaryDirectory() as td:
skills_dir = os.path.join(td, "skills")
deep_path = os.path.join(skills_dir, "mlops", "evaluation", "my-eval-skill")
os.makedirs(deep_path, exist_ok=True)
with open(os.path.join(deep_path, "SKILL.md"), "w") as f:
f.write("---\nname: my-eval-skill\ndescription: \"Eval skill at depth 3\"\n---\n\n# Eval Skill\n")
sm = SkillManager(skills_dir)
names = {s["name"] for s in sm.skills["all_skills"]}
assert "my-eval-skill" in names

def test_flat_skills_remain_discoverable(self):
"""Flat skills (skills_dir/skill/SKILL.md) are still found."""
with tempfile.TemporaryDirectory() as td:
skills_dir = os.path.join(td, "skills")
_write_skill(skills_dir, "flat-skill", "A flat skill")
sm = SkillManager(skills_dir)
names = {s["name"] for s in sm.skills["all_skills"]}
assert "flat-skill" in names

def test_unrelated_dirs_excluded(self):
"""Unrelated directories outside the configured skill root are excluded."""
with tempfile.TemporaryDirectory() as td:
skills_dir = os.path.join(td, "skills")
unrelated = os.path.join(td, "not-skills", "intruder")
os.makedirs(unrelated, exist_ok=True)
with open(os.path.join(unrelated, "SKILL.md"), "w") as f:
f.write("---\nname: intruder\ndescription: \"Should not be found\"\n---\n\n# Intruder\n")
_write_skill(skills_dir, "real-skill", "A real skill")
sm = SkillManager(skills_dir)
names = {s["name"] for s in sm.skills["all_skills"]}
assert "real-skill" in names
assert "intruder" not in names

def test_symlink_duplicates_not_double_counted(self):
"""Symlink duplicates are not returned twice (realpath dedup)."""
with tempfile.TemporaryDirectory() as td:
skills_dir = os.path.join(td, "skills")
os.makedirs(skills_dir, exist_ok=True)
real_skill_dir = os.path.join(td, "real-skills", "linked-skill")
os.makedirs(real_skill_dir, exist_ok=True)
with open(os.path.join(real_skill_dir, "SKILL.md"), "w") as f:
f.write("---\nname: linked-skill\ndescription: \"Linked skill\"\n---\n\n# Linked\n")
# Create symlink inside skills_dir
os.symlink(real_skill_dir, os.path.join(skills_dir, "linked-skill"))
sm = SkillManager(skills_dir)
linked = [s for s in sm.skills["all_skills"] if s["name"] == "linked-skill"]
assert len(linked) == 1, f"Expected 1, got {len(linked)}"

def test_real_world_91_skills_detected(self):
"""All 91 current skills are detected in the user's production profile."""
skills_dir = os.path.expanduser("~/.hermes/profiles/lightning-node/skills")
if not os.path.isdir(skills_dir):
pytest.skip("Production skills directory not available")
sm = SkillManager(skills_dir)
count = len(sm.skills["all_skills"])
assert count >= 91, f"Expected at least 91 skills, found {count}"
# Verify key Lightning skills are present
names = {s["name"] for s in sm.skills["all_skills"]}
expected = {
"lightning-node-ops",
"core-lightning-ops",
"lndg-remote-db-ops",
"loopd-cost-reconciliation",
"intervention-ledger-reconciliation",
"hermes-agent",
"obsidian",
"github-workflow",
}
missing = expected - names
assert not missing, f"Missing critical skills: {missing}"

def test_is_hermes_skill_root_profile(self):
"""_is_hermes_skill_root returns True for profile skills directories."""
with tempfile.TemporaryDirectory() as td:
profile_skills = os.path.join(td, ".hermes", "profiles", "my-profile", "skills")
os.makedirs(profile_skills, exist_ok=True)
sm = SkillManager(profile_skills)
assert sm._is_hermes_skill_root()

def test_is_hermes_skill_root_not_hermes(self):
"""_is_hermes_skill_root returns False for non-Hermes directories."""
with tempfile.TemporaryDirectory() as td:
skills_dir = os.path.join(td, "random-skills")
os.makedirs(skills_dir, exist_ok=True)
sm = SkillManager(skills_dir)
assert not sm._is_hermes_skill_root()