mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-09 13:21:42 +00:00
fix(skills): resolve skills dir from active profile
This commit is contained in:
parent
491689784e
commit
f8723c4781
2 changed files with 134 additions and 10 deletions
105
tests/tools/test_skills_tool_profile_scope.py
Normal file
105
tests/tools/test_skills_tool_profile_scope.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue