Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,4 @@ mini-swe-agent/
result
website/static/api/skills-index.json
models-dev-upstream/
.claude/scheduled_tasks.lock
69 changes: 69 additions & 0 deletions tests/tools/test_skill_manager_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -943,3 +943,72 @@ def test_broken_sidecar_fails_open(self, tmp_path):
side_effect=RuntimeError("sidecar broken")):
result = _delete_skill("my-skill")
assert result["success"] is True


# ---------------------------------------------------------------------------
# external_dirs honored on create — #21810
# ---------------------------------------------------------------------------


class TestCreateSkillRespectsExternalDirs:
"""skill_manage(action='create') must drop the new skill into the first
configured ``skills.external_dirs`` entry instead of the local
~/.hermes/skills/ when one is set. Closes #21810.
"""

def test_no_external_dirs_uses_local_skills_dir(self, tmp_path):
"""Regression guard: with no external_dirs, behavior is unchanged."""
with patch("tools.skill_manager_tool.SKILLS_DIR", tmp_path), \
patch("agent.skill_utils.get_all_skills_dirs", return_value=[tmp_path]), \
patch("agent.skill_utils.get_external_skills_dirs", return_value=[]):
result = _create_skill("local-skill", VALID_SKILL_CONTENT)
assert result["success"] is True
assert (tmp_path / "local-skill" / "SKILL.md").exists()

def test_external_dir_used_when_configured(self, tmp_path):
"""First external_dirs entry wins over SKILLS_DIR."""
local = tmp_path / "local"
external = tmp_path / "external"
local.mkdir()
external.mkdir()
with patch("tools.skill_manager_tool.SKILLS_DIR", local), \
patch("agent.skill_utils.get_all_skills_dirs",
return_value=[local, external]), \
patch("agent.skill_utils.get_external_skills_dirs",
return_value=[external]):
result = _create_skill("ext-skill", VALID_SKILL_CONTENT)
assert result["success"] is True
assert (external / "ext-skill" / "SKILL.md").exists()
assert not (local / "ext-skill").exists()

def test_external_dir_used_with_category(self, tmp_path):
"""Category nesting still applies under the external root."""
local = tmp_path / "local"
external = tmp_path / "external"
local.mkdir()
external.mkdir()
with patch("tools.skill_manager_tool.SKILLS_DIR", local), \
patch("agent.skill_utils.get_all_skills_dirs",
return_value=[local, external]), \
patch("agent.skill_utils.get_external_skills_dirs",
return_value=[external]):
result = _create_skill("cat-skill", VALID_SKILL_CONTENT, category="ops")
assert result["success"] is True
assert (external / "ops" / "cat-skill" / "SKILL.md").exists()
assert result["path"] == str(Path("ops") / "cat-skill")

def test_first_external_dir_wins(self, tmp_path):
"""When multiple external dirs configured, the first is preferred."""
first = tmp_path / "first"
second = tmp_path / "second"
first.mkdir()
second.mkdir()
with patch("tools.skill_manager_tool.SKILLS_DIR", tmp_path / "local"), \
patch("agent.skill_utils.get_all_skills_dirs",
return_value=[first, second]), \
patch("agent.skill_utils.get_external_skills_dirs",
return_value=[first, second]):
result = _create_skill("multi-skill", VALID_SKILL_CONTENT)
assert result["success"] is True
assert (first / "multi-skill" / "SKILL.md").exists()
assert not (second / "multi-skill").exists()
34 changes: 31 additions & 3 deletions tools/skill_manager_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,34 @@ def _validate_content_size(content: str, label: str = "SKILL.md") -> Optional[st
return None


def _default_creation_dir() -> Path:
"""Return the directory new skills should be created in.

When ``skills.external_dirs`` is configured in ``config.yaml``, prefer the
first existing entry so contributors editing an external skills repo can
create skills in-place. Falls back to local ``~/.hermes/skills/`` when no
external dirs are configured.

Closes #21810 — ``skill_manage(action='create')`` previously hardcoded
``SKILLS_DIR``, dropping new skills into the user-local store even when an
external dir was set as the canonical authoring location.
"""
try:
from agent.skill_utils import get_external_skills_dirs
external = get_external_skills_dirs()
except Exception:
external = []
if external:
return external[0]
return SKILLS_DIR


def _resolve_skill_dir(name: str, category: str = None) -> Path:
"""Build the directory path for a new skill, optionally under a category."""
base = _default_creation_dir()
if category:
return SKILLS_DIR / category / name
return SKILLS_DIR / name
return base / category / name
return base / name


def _find_skill(name: str) -> Optional[Dict[str, Any]]:
Expand Down Expand Up @@ -412,10 +435,15 @@ def _create_skill(name: str, content: str, category: str = None) -> Dict[str, An
shutil.rmtree(skill_dir, ignore_errors=True)
return {"success": False, "error": scan_error}

creation_root = _containing_skills_root(skill_dir)
try:
rel_path = skill_dir.relative_to(creation_root)
except ValueError:
rel_path = skill_dir
result = {
"success": True,
"message": f"Skill '{name}' created.",
"path": str(skill_dir.relative_to(SKILLS_DIR)),
"path": str(rel_path),
"skill_md": str(skill_md),
}
if category:
Expand Down
Loading