feat(skills): support external skill directories via config (#3678)
Add skills.external_dirs config option — a list of additional directories to scan for skills alongside ~/.hermes/skills/. External dirs are read-only: skill creation/editing always writes to the local dir. Local skills take precedence when names collide. This lets users share skills across tools/agents without copying them into Hermes's own directory (e.g. ~/.agents/skills, /shared/team-skills). Changes: - agent/skill_utils.py: add get_external_skills_dirs() and get_all_skills_dirs() - agent/prompt_builder.py: scan external dirs in build_skills_system_prompt() - tools/skills_tool.py: _find_all_skills() and skill_view() search external dirs; security check recognizes configured external dirs as trusted - agent/skill_commands.py: /skill slash commands discover external skills - hermes_cli/config.py: add skills.external_dirs to DEFAULT_CONFIG - cli-config.yaml.example: document the option - tests/agent/test_external_skills.py: 11 tests covering discovery, precedence, deduplication, and skill_view for external skills Requested by community member primco.
This commit is contained in:
@@ -494,7 +494,7 @@ def _is_skill_disabled(name: str, platform: str = None) -> bool:
|
||||
|
||||
|
||||
def _find_all_skills(*, skip_disabled: bool = False) -> List[Dict[str, Any]]:
|
||||
"""Recursively find all skills in ~/.hermes/skills/.
|
||||
"""Recursively find all skills in ~/.hermes/skills/ and external dirs.
|
||||
|
||||
Args:
|
||||
skip_disabled: If True, return ALL skills regardless of disabled
|
||||
@@ -504,59 +504,68 @@ def _find_all_skills(*, skip_disabled: bool = False) -> List[Dict[str, Any]]:
|
||||
Returns:
|
||||
List of skill metadata dicts (name, description, category).
|
||||
"""
|
||||
skills = []
|
||||
from agent.skill_utils import get_external_skills_dirs
|
||||
|
||||
if not SKILLS_DIR.exists():
|
||||
return skills
|
||||
skills = []
|
||||
seen_names: set = set()
|
||||
|
||||
# Load disabled set once (not per-skill)
|
||||
disabled = set() if skip_disabled else _get_disabled_skill_names()
|
||||
|
||||
# Scan local dir first, then external dirs (local takes precedence)
|
||||
dirs_to_scan = []
|
||||
if SKILLS_DIR.exists():
|
||||
dirs_to_scan.append(SKILLS_DIR)
|
||||
dirs_to_scan.extend(get_external_skills_dirs())
|
||||
|
||||
for skill_md in SKILLS_DIR.rglob("SKILL.md"):
|
||||
if any(part in _EXCLUDED_SKILL_DIRS for part in skill_md.parts):
|
||||
continue
|
||||
|
||||
skill_dir = skill_md.parent
|
||||
|
||||
try:
|
||||
content = skill_md.read_text(encoding="utf-8")[:4000]
|
||||
frontmatter, body = _parse_frontmatter(content)
|
||||
|
||||
if not skill_matches_platform(frontmatter):
|
||||
for scan_dir in dirs_to_scan:
|
||||
for skill_md in scan_dir.rglob("SKILL.md"):
|
||||
if any(part in _EXCLUDED_SKILL_DIRS for part in skill_md.parts):
|
||||
continue
|
||||
|
||||
name = frontmatter.get("name", skill_dir.name)[:MAX_NAME_LENGTH]
|
||||
if name in disabled:
|
||||
skill_dir = skill_md.parent
|
||||
|
||||
try:
|
||||
content = skill_md.read_text(encoding="utf-8")[:4000]
|
||||
frontmatter, body = _parse_frontmatter(content)
|
||||
|
||||
if not skill_matches_platform(frontmatter):
|
||||
continue
|
||||
|
||||
name = frontmatter.get("name", skill_dir.name)[:MAX_NAME_LENGTH]
|
||||
if name in seen_names:
|
||||
continue
|
||||
if name in disabled:
|
||||
continue
|
||||
|
||||
description = frontmatter.get("description", "")
|
||||
if not description:
|
||||
for line in body.strip().split("\n"):
|
||||
line = line.strip()
|
||||
if line and not line.startswith("#"):
|
||||
description = line
|
||||
break
|
||||
|
||||
if len(description) > MAX_DESCRIPTION_LENGTH:
|
||||
description = description[:MAX_DESCRIPTION_LENGTH - 3] + "..."
|
||||
|
||||
category = _get_category_from_path(skill_md)
|
||||
|
||||
seen_names.add(name)
|
||||
skills.append({
|
||||
"name": name,
|
||||
"description": description,
|
||||
"category": category,
|
||||
})
|
||||
|
||||
except (UnicodeDecodeError, PermissionError) as e:
|
||||
logger.debug("Failed to read skill file %s: %s", skill_md, e)
|
||||
continue
|
||||
except Exception as e:
|
||||
logger.debug(
|
||||
"Skipping skill at %s: failed to parse: %s", skill_md, e, exc_info=True
|
||||
)
|
||||
continue
|
||||
|
||||
description = frontmatter.get("description", "")
|
||||
if not description:
|
||||
for line in body.strip().split("\n"):
|
||||
line = line.strip()
|
||||
if line and not line.startswith("#"):
|
||||
description = line
|
||||
break
|
||||
|
||||
if len(description) > MAX_DESCRIPTION_LENGTH:
|
||||
description = description[:MAX_DESCRIPTION_LENGTH - 3] + "..."
|
||||
|
||||
category = _get_category_from_path(skill_md)
|
||||
|
||||
skills.append({
|
||||
"name": name,
|
||||
"description": description,
|
||||
"category": category,
|
||||
})
|
||||
|
||||
except (UnicodeDecodeError, PermissionError) as e:
|
||||
logger.debug("Failed to read skill file %s: %s", skill_md, e)
|
||||
continue
|
||||
except Exception as e:
|
||||
logger.debug(
|
||||
"Skipping skill at %s: failed to parse: %s", skill_md, e, exc_info=True
|
||||
)
|
||||
continue
|
||||
|
||||
return skills
|
||||
|
||||
@@ -756,7 +765,15 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str:
|
||||
JSON string with skill content or error message
|
||||
"""
|
||||
try:
|
||||
if not SKILLS_DIR.exists():
|
||||
from agent.skill_utils import get_external_skills_dirs
|
||||
|
||||
# Build list of all skill directories to search
|
||||
all_dirs = []
|
||||
if SKILLS_DIR.exists():
|
||||
all_dirs.append(SKILLS_DIR)
|
||||
all_dirs.extend(get_external_skills_dirs())
|
||||
|
||||
if not all_dirs:
|
||||
return json.dumps(
|
||||
{
|
||||
"success": False,
|
||||
@@ -768,27 +785,37 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str:
|
||||
skill_dir = None
|
||||
skill_md = None
|
||||
|
||||
# Try direct path first (e.g., "mlops/axolotl")
|
||||
direct_path = SKILLS_DIR / name
|
||||
if direct_path.is_dir() and (direct_path / "SKILL.md").exists():
|
||||
skill_dir = direct_path
|
||||
skill_md = direct_path / "SKILL.md"
|
||||
elif direct_path.with_suffix(".md").exists():
|
||||
skill_md = direct_path.with_suffix(".md")
|
||||
# Search all dirs: local first, then external (first match wins)
|
||||
for search_dir in all_dirs:
|
||||
# Try direct path first (e.g., "mlops/axolotl")
|
||||
direct_path = search_dir / name
|
||||
if direct_path.is_dir() and (direct_path / "SKILL.md").exists():
|
||||
skill_dir = direct_path
|
||||
skill_md = direct_path / "SKILL.md"
|
||||
break
|
||||
elif direct_path.with_suffix(".md").exists():
|
||||
skill_md = direct_path.with_suffix(".md")
|
||||
break
|
||||
|
||||
# Search by directory name
|
||||
# Search by directory name across all dirs
|
||||
if not skill_md:
|
||||
for found_skill_md in SKILLS_DIR.rglob("SKILL.md"):
|
||||
if found_skill_md.parent.name == name:
|
||||
skill_dir = found_skill_md.parent
|
||||
skill_md = found_skill_md
|
||||
for search_dir in all_dirs:
|
||||
for found_skill_md in search_dir.rglob("SKILL.md"):
|
||||
if found_skill_md.parent.name == name:
|
||||
skill_dir = found_skill_md.parent
|
||||
skill_md = found_skill_md
|
||||
break
|
||||
if skill_md:
|
||||
break
|
||||
|
||||
# Legacy: flat .md files
|
||||
if not skill_md:
|
||||
for found_md in SKILLS_DIR.rglob(f"{name}.md"):
|
||||
if found_md.name != "SKILL.md":
|
||||
skill_md = found_md
|
||||
for search_dir in all_dirs:
|
||||
for found_md in search_dir.rglob(f"{name}.md"):
|
||||
if found_md.name != "SKILL.md":
|
||||
skill_md = found_md
|
||||
break
|
||||
if skill_md:
|
||||
break
|
||||
|
||||
if not skill_md or not skill_md.exists():
|
||||
@@ -815,12 +842,21 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str:
|
||||
ensure_ascii=False,
|
||||
)
|
||||
|
||||
# Security: warn if skill is loaded from outside the trusted skills directory
|
||||
# Security: warn if skill is loaded from outside trusted directories
|
||||
# (local skills dir + configured external_dirs are all trusted)
|
||||
_outside_skills_dir = True
|
||||
_trusted_dirs = [SKILLS_DIR.resolve()]
|
||||
try:
|
||||
skill_md.resolve().relative_to(SKILLS_DIR.resolve())
|
||||
_outside_skills_dir = False
|
||||
except ValueError:
|
||||
_outside_skills_dir = True
|
||||
_trusted_dirs.extend(d.resolve() for d in all_dirs[1:])
|
||||
except Exception:
|
||||
pass
|
||||
for _td in _trusted_dirs:
|
||||
try:
|
||||
skill_md.resolve().relative_to(_td)
|
||||
_outside_skills_dir = False
|
||||
break
|
||||
except ValueError:
|
||||
continue
|
||||
|
||||
# Security: detect common prompt injection patterns
|
||||
_INJECTION_PATTERNS = [
|
||||
@@ -1058,7 +1094,11 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str:
|
||||
if script_files:
|
||||
linked_files["scripts"] = script_files
|
||||
|
||||
rel_path = str(skill_md.relative_to(SKILLS_DIR))
|
||||
try:
|
||||
rel_path = str(skill_md.relative_to(SKILLS_DIR))
|
||||
except ValueError:
|
||||
# External skill — use path relative to the skill's own parent dir
|
||||
rel_path = str(skill_md.relative_to(skill_md.parent.parent)) if skill_md.parent.parent else skill_md.name
|
||||
skill_name = frontmatter.get(
|
||||
"name", skill_md.stem if not skill_dir else skill_dir.name
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user