From f8723c47818e84df7d4514228381be5b0509b7f3 Mon Sep 17 00:00:00 2001 From: JP Lew <462836+jplew@users.noreply.github.com> Date: Wed, 1 Jul 2026 23:55:16 +0000 Subject: [PATCH] fix(skills): resolve skills dir from active profile --- tests/tools/test_skills_tool_profile_scope.py | 105 ++++++++++++++++++ tools/skills_tool.py | 39 +++++-- 2 files changed, 134 insertions(+), 10 deletions(-) create mode 100644 tests/tools/test_skills_tool_profile_scope.py diff --git a/tests/tools/test_skills_tool_profile_scope.py b/tests/tools/test_skills_tool_profile_scope.py new file mode 100644 index 00000000000..4c8b4cd8a4d --- /dev/null +++ b/tests/tools/test_skills_tool_profile_scope.py @@ -0,0 +1,105 @@ +"""Regression tests for profile-scoped skills_tool path resolution.""" + +import importlib +import json +from pathlib import Path + + +def _write_skill(root: Path, category: str, name: str, description: str) -> Path: + skill_dir = root / "skills" / category / name + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"---\n" + f"name: {name}\n" + f"description: {description}\n" + f"---\n\n" + f"# {name}\n\n" + f"Loaded from {description}.\n", + encoding="utf-8", + ) + return skill_dir + + +def _reload_skills_tool(import_home: Path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(import_home)) + import tools.skills_tool as skills_tool + + return importlib.reload(skills_tool) + + +def test_skill_view_uses_live_profile_home_after_module_import(tmp_path, monkeypatch): + """skill_view should not stay pinned to HERMES_HOME from import time.""" + default_home = tmp_path / "default-home" + profile_home = tmp_path / "profiles" / "orchestrator" + _write_skill(default_home, "autonomous-ai-agents", "default-only", "default home") + profile_skill_dir = _write_skill( + profile_home, + "software-development", + "kanban-orchestrator-operations", + "orchestrator profile", + ) + + skills_tool = _reload_skills_tool(default_home, monkeypatch) + assert skills_tool.SKILLS_DIR == default_home / "skills" + + monkeypatch.setenv("HERMES_HOME", str(profile_home)) + + result = json.loads( + skills_tool.skill_view("kanban-orchestrator-operations", preprocess=False) + ) + + assert result["success"] is True + assert result["name"] == "kanban-orchestrator-operations" + assert Path(result["skill_dir"]) == profile_skill_dir + assert "orchestrator profile" in result["content"] + + +def test_skills_list_uses_live_profile_home_after_module_import(tmp_path, monkeypatch): + """skills_list should list the active profile skills, not the import-time root.""" + default_home = tmp_path / "default-home" + profile_home = tmp_path / "profiles" / "orchestrator" + _write_skill(default_home, "autonomous-ai-agents", "default-only", "default home") + _write_skill( + profile_home, + "software-development", + "kanban-orchestrator-operations", + "orchestrator profile", + ) + + skills_tool = _reload_skills_tool(default_home, monkeypatch) + monkeypatch.setenv("HERMES_HOME", str(profile_home)) + + result = json.loads(skills_tool.skills_list()) + names = {skill["name"] for skill in result["skills"]} + + assert result["success"] is True + assert "kanban-orchestrator-operations" in names + assert "default-only" not in names + + +def test_explicit_skills_dir_monkeypatch_still_wins(tmp_path, monkeypatch): + """Existing tests can still override tools.skills_tool.SKILLS_DIR directly.""" + default_home = tmp_path / "default-home" + profile_home = tmp_path / "profiles" / "orchestrator" + patched_root = tmp_path / "patched" + patched_skill_dir = _write_skill( + patched_root, + "software-development", + "patched-skill", + "patched skills dir", + ) + _write_skill( + profile_home, + "software-development", + "profile-skill", + "orchestrator profile", + ) + + skills_tool = _reload_skills_tool(default_home, monkeypatch) + monkeypatch.setenv("HERMES_HOME", str(profile_home)) + monkeypatch.setattr(skills_tool, "SKILLS_DIR", patched_root / "skills") + + result = json.loads(skills_tool.skill_view("patched-skill", preprocess=False)) + + assert result["success"] is True + assert Path(result["skill_dir"]) == patched_skill_dir diff --git a/tools/skills_tool.py b/tools/skills_tool.py index cd699fae8f6..86a8a5de6cb 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -92,6 +92,22 @@ logger = logging.getLogger(__name__) # skills all coexist here without polluting the git repo. HERMES_HOME = get_hermes_home() SKILLS_DIR = HERMES_HOME / "skills" +_SKILLS_DIR_AT_IMPORT = SKILLS_DIR + + +def _skills_dir() -> Path: + """Return the active profile's skills directory at call time. + + Some long-lived runtimes import this module before the active profile has + set HERMES_HOME. Keep the legacy SKILLS_DIR module attribute for tests and + external patchers, but when it has not been patched, resolve from the live + profile-scoped HERMES_HOME on every call. + """ + configured = Path(SKILLS_DIR) + if configured != _SKILLS_DIR_AT_IMPORT: + return configured + return get_hermes_home() / "skills" + # Anthropic-recommended limits for progressive disclosure efficiency MAX_NAME_LENGTH = 64 @@ -501,9 +517,9 @@ def _get_category_from_path(skill_path: Path) -> Optional[str]: 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), + # Try the active profile skills dir first (respects monkeypatching in tests), # then fall back to external dirs from config. - dirs_to_check = [SKILLS_DIR] + dirs_to_check = [_skills_dir()] try: from agent.skill_utils import get_external_skills_dirs dirs_to_check.extend(get_external_skills_dirs()) @@ -622,8 +638,9 @@ def _find_all_skills(*, skip_disabled: bool = False) -> List[Dict[str, Any]]: # Scan local dir first, then external dirs (local takes precedence) dirs_to_scan = [] - if SKILLS_DIR.exists(): - dirs_to_scan.append(SKILLS_DIR) + active_skills_dir = _skills_dir() + if active_skills_dir.exists(): + dirs_to_scan.append(active_skills_dir) dirs_to_scan.extend(get_external_skills_dirs()) for scan_dir in dirs_to_scan: @@ -701,8 +718,9 @@ def skills_list(category: str = None, task_id: str = None) -> str: JSON string with minimal skill info: name, description, category """ try: - if not SKILLS_DIR.exists(): - SKILLS_DIR.mkdir(parents=True, exist_ok=True) + active_skills_dir = _skills_dir() + if not active_skills_dir.exists(): + active_skills_dir.mkdir(parents=True, exist_ok=True) return json.dumps( { "success": True, @@ -984,8 +1002,9 @@ def skill_view( # Build list of all skill directories to search all_dirs = [] - if SKILLS_DIR.exists(): - all_dirs.append(SKILLS_DIR) + active_skills_dir = _skills_dir() + if active_skills_dir.exists(): + all_dirs.append(active_skills_dir) all_dirs.extend(get_external_skills_dirs()) if not all_dirs: @@ -1135,7 +1154,7 @@ def skill_view( # 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()] + _trusted_dirs = [active_skills_dir.resolve()] try: _trusted_dirs.extend(d.resolve() for d in all_dirs[1:]) except Exception: @@ -1375,7 +1394,7 @@ def skill_view( linked_files["scripts"] = script_files try: - rel_path = str(skill_md.relative_to(SKILLS_DIR)) + rel_path = str(skill_md.relative_to(active_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