diff --git a/gateway/run.py b/gateway/run.py index 198d23f61..2f19edcfa 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -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 diff --git a/tests/tools/test_credential_files.py b/tests/tools/test_credential_files.py index 488badadf..ee3bbd4f3 100644 --- a/tests/tools/test_credential_files.py +++ b/tests/tools/test_credential_files.py @@ -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: diff --git a/tools/credential_files.py b/tools/credential_files.py index c58e0615a..9a30f9bff 100644 --- a/tools/credential_files.py +++ b/tools/credential_files.py @@ -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 ``/skills``, external dirs + at ``/external_skills/``. """ + 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 diff --git a/tools/environments/docker.py b/tools/environments/docker.py index 19889ea35..11deccb02 100644 --- a/tools/environments/docker.py +++ b/tools/environments/docker.py @@ -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", diff --git a/tools/environments/singularity.py b/tools/environments/singularity.py index 2ee525a36..89d9ffb04 100644 --- a/tools/environments/singularity.py +++ b/tools/environments/singularity.py @@ -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", diff --git a/tools/environments/ssh.py b/tools/environments/ssh.py index 94b0a6b3f..387dea34e 100644 --- a/tools/environments/ssh.py +++ b/tools/environments/ssh.py @@ -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}") diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index d6d2f6f78..b8d8d6223 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -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 diff --git a/tools/skills_tool.py b/tools/skills_tool.py index 6c9e2441a..da023a143 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -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()):