mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 06:51:16 +08:00
feat: wire skills.external_dirs into all remaining discovery paths
The config key skills.external_dirs and core resolution (get_all_skills_dirs,
get_external_skills_dirs in agent/skill_utils.py) already existed but several
code paths still only scanned SKILLS_DIR. Now external dirs are respected
everywhere:
- skills_categories(): scan all dirs for category discovery
- _get_category_from_path(): resolve categories against any skills root
- skill_manager_tool._find_skill(): search all dirs for edit/patch/delete
- credential_files.get_skills_directory_mount(): mount all dirs into
Docker/Singularity containers (external dirs at external_skills/<idx>)
- credential_files.iter_skills_files(): list files from all dirs for
Modal/Daytona upload
- tools/environments/ssh.py: rsync all skill dirs to remote hosts
- gateway _check_unavailable_skill(): check disabled skills across all dirs
Usage in config.yaml:
skills:
external_dirs:
- ~/repos/agent-skills/hermes
- /shared/team-skills
This commit is contained in:
@@ -349,19 +349,23 @@ def _check_unavailable_skill(command_name: str) -> str | None:
|
||||
# Normalize: command uses hyphens, skill names may use hyphens or underscores
|
||||
normalized = command_name.lower().replace("_", "-")
|
||||
try:
|
||||
from tools.skills_tool import SKILLS_DIR, _get_disabled_skill_names
|
||||
from tools.skills_tool import _get_disabled_skill_names
|
||||
from agent.skill_utils import get_all_skills_dirs
|
||||
disabled = _get_disabled_skill_names()
|
||||
|
||||
# Check disabled built-in skills
|
||||
for skill_md in SKILLS_DIR.rglob("SKILL.md"):
|
||||
if any(part in ('.git', '.github', '.hub') for part in skill_md.parts):
|
||||
# Check disabled skills across all dirs (local + external)
|
||||
for skills_dir in get_all_skills_dirs():
|
||||
if not skills_dir.exists():
|
||||
continue
|
||||
name = skill_md.parent.name.lower().replace("_", "-")
|
||||
if name == normalized and name in disabled:
|
||||
return (
|
||||
f"The **{command_name}** skill is installed but disabled.\n"
|
||||
f"Enable it with: `hermes skills config`"
|
||||
)
|
||||
for skill_md in skills_dir.rglob("SKILL.md"):
|
||||
if any(part in ('.git', '.github', '.hub') for part in skill_md.parts):
|
||||
continue
|
||||
name = skill_md.parent.name.lower().replace("_", "-")
|
||||
if name == normalized and name in disabled:
|
||||
return (
|
||||
f"The **{command_name}** skill is installed but disabled.\n"
|
||||
f"Enable it with: `hermes skills config`"
|
||||
)
|
||||
|
||||
# Check optional skills (shipped with repo but not installed)
|
||||
from hermes_constants import get_hermes_home, get_optional_skills_dir
|
||||
|
||||
@@ -110,29 +110,31 @@ class TestSkillsDirectoryMount:
|
||||
(skills_dir / "test-skill" / "SKILL.md").write_text("# test")
|
||||
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}):
|
||||
mount = get_skills_directory_mount()
|
||||
mounts = get_skills_directory_mount()
|
||||
|
||||
assert mount is not None
|
||||
assert mount["host_path"] == str(skills_dir)
|
||||
assert mount["container_path"] == "/root/.hermes/skills"
|
||||
assert len(mounts) >= 1
|
||||
assert mounts[0]["host_path"] == str(skills_dir)
|
||||
assert mounts[0]["container_path"] == "/root/.hermes/skills"
|
||||
|
||||
def test_returns_none_when_no_skills_dir(self, tmp_path):
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}):
|
||||
mount = get_skills_directory_mount()
|
||||
mounts = get_skills_directory_mount()
|
||||
|
||||
assert mount is None
|
||||
# No local skills dir → no local mount (external dirs may still appear)
|
||||
local_mounts = [m for m in mounts if m["container_path"].endswith("/skills")]
|
||||
assert local_mounts == []
|
||||
|
||||
def test_custom_container_base(self, tmp_path):
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
(hermes_home / "skills").mkdir(parents=True)
|
||||
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}):
|
||||
mount = get_skills_directory_mount(container_base="/home/user/.hermes")
|
||||
mounts = get_skills_directory_mount(container_base="/home/user/.hermes")
|
||||
|
||||
assert mount["container_path"] == "/home/user/.hermes/skills"
|
||||
assert mounts[0]["container_path"] == "/home/user/.hermes/skills"
|
||||
|
||||
def test_symlinks_are_sanitized(self, tmp_path):
|
||||
"""Symlinks in skills dir should be excluded from the mount."""
|
||||
@@ -146,9 +148,10 @@ class TestSkillsDirectoryMount:
|
||||
(skills_dir / "evil_link").symlink_to(secret)
|
||||
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}):
|
||||
mount = get_skills_directory_mount()
|
||||
mounts = get_skills_directory_mount()
|
||||
|
||||
assert mount is not None
|
||||
assert len(mounts) >= 1
|
||||
mount = mounts[0]
|
||||
# The mount path should be a sanitized copy, not the original
|
||||
safe_path = Path(mount["host_path"])
|
||||
assert safe_path != skills_dir
|
||||
@@ -166,9 +169,9 @@ class TestSkillsDirectoryMount:
|
||||
(skills_dir / "skill.md").write_text("ok")
|
||||
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}):
|
||||
mount = get_skills_directory_mount()
|
||||
mounts = get_skills_directory_mount()
|
||||
|
||||
assert mount["host_path"] == str(skills_dir)
|
||||
assert mounts[0]["host_path"] == str(skills_dir)
|
||||
|
||||
|
||||
class TestIterSkillsFiles:
|
||||
|
||||
@@ -193,8 +193,8 @@ def get_credential_file_mounts() -> List[Dict[str, str]]:
|
||||
|
||||
def get_skills_directory_mount(
|
||||
container_base: str = "/root/.hermes",
|
||||
) -> Dict[str, str] | None:
|
||||
"""Return mount info for a symlink-safe copy of the skills directory.
|
||||
) -> list[Dict[str, str]]:
|
||||
"""Return mount info for all skill directories (local + external).
|
||||
|
||||
Skills may include ``scripts/``, ``templates/``, and ``references/``
|
||||
subdirectories that the agent needs to execute inside remote sandboxes.
|
||||
@@ -206,18 +206,34 @@ def get_skills_directory_mount(
|
||||
symlinks are present (the common case), the original directory is returned
|
||||
directly with zero overhead.
|
||||
|
||||
Returns a dict with ``host_path`` and ``container_path`` keys, or None.
|
||||
Returns a list of dicts with ``host_path`` and ``container_path`` keys.
|
||||
The local skills dir mounts at ``<container_base>/skills``, external dirs
|
||||
at ``<container_base>/external_skills/<index>``.
|
||||
"""
|
||||
mounts = []
|
||||
hermes_home = _resolve_hermes_home()
|
||||
skills_dir = hermes_home / "skills"
|
||||
if not skills_dir.is_dir():
|
||||
return None
|
||||
if skills_dir.is_dir():
|
||||
host_path = _safe_skills_path(skills_dir)
|
||||
mounts.append({
|
||||
"host_path": host_path,
|
||||
"container_path": f"{container_base.rstrip('/')}/skills",
|
||||
})
|
||||
|
||||
host_path = _safe_skills_path(skills_dir)
|
||||
return {
|
||||
"host_path": host_path,
|
||||
"container_path": f"{container_base.rstrip('/')}/skills",
|
||||
}
|
||||
# Mount external skill dirs
|
||||
try:
|
||||
from agent.skill_utils import get_external_skills_dirs
|
||||
for idx, ext_dir in enumerate(get_external_skills_dirs()):
|
||||
if ext_dir.is_dir():
|
||||
host_path = _safe_skills_path(ext_dir)
|
||||
mounts.append({
|
||||
"host_path": host_path,
|
||||
"container_path": f"{container_base.rstrip('/')}/external_skills/{idx}",
|
||||
})
|
||||
except ImportError:
|
||||
pass
|
||||
|
||||
return mounts
|
||||
|
||||
|
||||
_safe_skills_tempdir: Path | None = None
|
||||
@@ -271,24 +287,44 @@ def iter_skills_files(
|
||||
) -> List[Dict[str, str]]:
|
||||
"""Yield individual (host_path, container_path) entries for skills files.
|
||||
|
||||
Skips symlinks entirely. Preferred for backends that upload files
|
||||
individually (Daytona, Modal) rather than mounting a directory.
|
||||
Includes both the local skills dir and any external dirs configured via
|
||||
skills.external_dirs. Skips symlinks entirely. Preferred for backends
|
||||
that upload files individually (Daytona, Modal) rather than mounting a
|
||||
directory.
|
||||
"""
|
||||
result: List[Dict[str, str]] = []
|
||||
|
||||
hermes_home = _resolve_hermes_home()
|
||||
skills_dir = hermes_home / "skills"
|
||||
if not skills_dir.is_dir():
|
||||
return []
|
||||
if skills_dir.is_dir():
|
||||
container_root = f"{container_base.rstrip('/')}/skills"
|
||||
for item in skills_dir.rglob("*"):
|
||||
if item.is_symlink() or not item.is_file():
|
||||
continue
|
||||
rel = item.relative_to(skills_dir)
|
||||
result.append({
|
||||
"host_path": str(item),
|
||||
"container_path": f"{container_root}/{rel}",
|
||||
})
|
||||
|
||||
# Include external skill dirs
|
||||
try:
|
||||
from agent.skill_utils import get_external_skills_dirs
|
||||
for idx, ext_dir in enumerate(get_external_skills_dirs()):
|
||||
if not ext_dir.is_dir():
|
||||
continue
|
||||
container_root = f"{container_base.rstrip('/')}/external_skills/{idx}"
|
||||
for item in ext_dir.rglob("*"):
|
||||
if item.is_symlink() or not item.is_file():
|
||||
continue
|
||||
rel = item.relative_to(ext_dir)
|
||||
result.append({
|
||||
"host_path": str(item),
|
||||
"container_path": f"{container_root}/{rel}",
|
||||
})
|
||||
except ImportError:
|
||||
pass
|
||||
|
||||
container_root = f"{container_base.rstrip('/')}/skills"
|
||||
result: List[Dict[str, str]] = []
|
||||
for item in skills_dir.rglob("*"):
|
||||
if item.is_symlink() or not item.is_file():
|
||||
continue
|
||||
rel = item.relative_to(skills_dir)
|
||||
result.append({
|
||||
"host_path": str(item),
|
||||
"container_path": f"{container_root}/{rel}",
|
||||
})
|
||||
return result
|
||||
|
||||
|
||||
|
||||
@@ -332,10 +332,9 @@ class DockerEnvironment(BaseEnvironment):
|
||||
mount_entry["container_path"],
|
||||
)
|
||||
|
||||
# Mount the skills directory so skill scripts/templates are
|
||||
# available inside the container at the same relative path.
|
||||
skills_mount = get_skills_directory_mount()
|
||||
if skills_mount:
|
||||
# Mount skill directories (local + external) so skill
|
||||
# scripts/templates are available inside the container.
|
||||
for skills_mount in get_skills_directory_mount():
|
||||
volume_args.extend([
|
||||
"-v",
|
||||
f"{skills_mount['host_path']}:{skills_mount['container_path']}:ro",
|
||||
|
||||
@@ -265,8 +265,7 @@ class SingularityEnvironment(BaseEnvironment):
|
||||
mount_entry["host_path"],
|
||||
mount_entry["container_path"],
|
||||
)
|
||||
skills_mount = get_skills_directory_mount()
|
||||
if skills_mount:
|
||||
for skills_mount in get_skills_directory_mount():
|
||||
cmd.extend(["--bind", f"{skills_mount['host_path']}:{skills_mount['container_path']}:ro"])
|
||||
logger.info(
|
||||
"Singularity: binding skills dir %s -> %s",
|
||||
|
||||
@@ -135,9 +135,8 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment):
|
||||
else:
|
||||
logger.debug("SSH: rsync credential failed: %s", result.stderr.strip())
|
||||
|
||||
# Sync skills directory (remap to detected home)
|
||||
skills_mount = get_skills_directory_mount(container_base=container_base)
|
||||
if skills_mount:
|
||||
# Sync skill directories (local + external, remap to detected home)
|
||||
for skills_mount in get_skills_directory_mount(container_base=container_base):
|
||||
remote_path = skills_mount["container_path"]
|
||||
mkdir_cmd = self._build_ssh_command()
|
||||
mkdir_cmd.append(f"mkdir -p {remote_path}")
|
||||
|
||||
@@ -203,14 +203,19 @@ def _resolve_skill_dir(name: str, category: str = None) -> Path:
|
||||
|
||||
def _find_skill(name: str) -> Optional[Dict[str, Any]]:
|
||||
"""
|
||||
Find a skill by name in ~/.hermes/skills/.
|
||||
Returns {"path": Path} or None.
|
||||
Find a skill by name across all skill directories.
|
||||
|
||||
Searches the local skills dir (~/.hermes/skills/) first, then any
|
||||
external dirs configured via skills.external_dirs. Returns
|
||||
{"path": Path} or None.
|
||||
"""
|
||||
if not SKILLS_DIR.exists():
|
||||
return None
|
||||
for skill_md in SKILLS_DIR.rglob("SKILL.md"):
|
||||
if skill_md.parent.name == name:
|
||||
return {"path": skill_md.parent}
|
||||
from agent.skill_utils import get_all_skills_dirs
|
||||
for skills_dir in get_all_skills_dirs():
|
||||
if not skills_dir.exists():
|
||||
continue
|
||||
for skill_md in skills_dir.rglob("SKILL.md"):
|
||||
if skill_md.parent.name == name:
|
||||
return {"path": skill_md.parent}
|
||||
return None
|
||||
|
||||
|
||||
|
||||
@@ -427,15 +427,25 @@ def _get_category_from_path(skill_path: Path) -> Optional[str]:
|
||||
Extract category from skill path based on directory structure.
|
||||
|
||||
For paths like: ~/.hermes/skills/mlops/axolotl/SKILL.md -> "mlops"
|
||||
Also works for external skill dirs configured via skills.external_dirs.
|
||||
"""
|
||||
# Try the module-level SKILLS_DIR first (respects monkeypatching in tests),
|
||||
# then fall back to external dirs from config.
|
||||
dirs_to_check = [SKILLS_DIR]
|
||||
try:
|
||||
rel_path = skill_path.relative_to(SKILLS_DIR)
|
||||
parts = rel_path.parts
|
||||
if len(parts) >= 3:
|
||||
return parts[0]
|
||||
return None
|
||||
except ValueError:
|
||||
return None
|
||||
from agent.skill_utils import get_external_skills_dirs
|
||||
dirs_to_check.extend(get_external_skills_dirs())
|
||||
except Exception:
|
||||
pass
|
||||
for skills_dir in dirs_to_check:
|
||||
try:
|
||||
rel_path = skill_path.relative_to(skills_dir)
|
||||
parts = rel_path.parts
|
||||
if len(parts) >= 3:
|
||||
return parts[0]
|
||||
except ValueError:
|
||||
continue
|
||||
return None
|
||||
|
||||
|
||||
def _estimate_tokens(content: str) -> int:
|
||||
@@ -645,7 +655,14 @@ def skills_categories(verbose: bool = False, task_id: str = None) -> str:
|
||||
JSON string with list of categories and their descriptions
|
||||
"""
|
||||
try:
|
||||
if not SKILLS_DIR.exists():
|
||||
# Use module-level SKILLS_DIR (respects monkeypatching) + external dirs
|
||||
all_dirs = [SKILLS_DIR] if SKILLS_DIR.exists() else []
|
||||
try:
|
||||
from agent.skill_utils import get_external_skills_dirs
|
||||
all_dirs.extend(d for d in get_external_skills_dirs() if d.exists())
|
||||
except Exception:
|
||||
pass
|
||||
if not all_dirs:
|
||||
return json.dumps(
|
||||
{
|
||||
"success": True,
|
||||
@@ -657,25 +674,26 @@ def skills_categories(verbose: bool = False, task_id: str = None) -> str:
|
||||
|
||||
category_dirs = {}
|
||||
category_counts: Dict[str, int] = {}
|
||||
for skill_md in SKILLS_DIR.rglob("SKILL.md"):
|
||||
if any(part in _EXCLUDED_SKILL_DIRS for part in skill_md.parts):
|
||||
continue
|
||||
for scan_dir in all_dirs:
|
||||
for skill_md in scan_dir.rglob("SKILL.md"):
|
||||
if any(part in _EXCLUDED_SKILL_DIRS for part in skill_md.parts):
|
||||
continue
|
||||
|
||||
try:
|
||||
frontmatter, _ = _parse_frontmatter(
|
||||
skill_md.read_text(encoding="utf-8")[:4000]
|
||||
)
|
||||
except Exception:
|
||||
frontmatter = {}
|
||||
try:
|
||||
frontmatter, _ = _parse_frontmatter(
|
||||
skill_md.read_text(encoding="utf-8")[:4000]
|
||||
)
|
||||
except Exception:
|
||||
frontmatter = {}
|
||||
|
||||
if not skill_matches_platform(frontmatter):
|
||||
continue
|
||||
if not skill_matches_platform(frontmatter):
|
||||
continue
|
||||
|
||||
category = _get_category_from_path(skill_md)
|
||||
if category:
|
||||
category_counts[category] = category_counts.get(category, 0) + 1
|
||||
if category not in category_dirs:
|
||||
category_dirs[category] = SKILLS_DIR / category
|
||||
category = _get_category_from_path(skill_md)
|
||||
if category:
|
||||
category_counts[category] = category_counts.get(category, 0) + 1
|
||||
if category not in category_dirs:
|
||||
category_dirs[category] = skill_md.parent.parent
|
||||
|
||||
categories = []
|
||||
for name in sorted(category_dirs.keys()):
|
||||
|
||||
Reference in New Issue
Block a user