mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-27 01:11:40 +00:00
fix: restore all removed bundled skills + fix skills sync system
- Restored 21 skills removed in commits757d012and740dd92: accelerate, audiocraft, code-review, faiss, flash-attention, gguf, grpo-rl-training, guidance, llava, nemo-curator, obliteratus, peft, pytorch-fsdp, pytorch-lightning, simpo, slime, stable-diffusion, tensorrt-llm, torchtitan, trl-fine-tuning, whisper - Rewrote sync_skills() with proper update semantics: * New skills (not in manifest): copied to user dir * Existing skills (in manifest + on disk): updated via hash comparison * User-deleted skills (in manifest, not on disk): respected, not re-added * Stale manifest entries (removed from bundled): cleaned from manifest - Added sync_skills() to CLI startup (cmd_chat) and gateway startup (start_gateway) — previously only ran during 'hermes update' - Updated cmd_update output to show new/updated/cleaned counts - Rewrote tests: 20 tests covering manifest CRUD, dir hashing, fresh install, user deletion respect, update detection, stale cleanup, and name collision handling 75 bundled skills total. 2002 tests pass.
This commit is contained in:
teknium1
parent
68fbae5692
commit
ab0f4126cf
74 changed files with 27881 additions and 44 deletions
|
|
@ -1,4 +1,4 @@
|
|||
"""Tests for tools/skills_sync.py — manifest-based skill seeding."""
|
||||
"""Tests for tools/skills_sync.py — manifest-based skill seeding and updating."""
|
||||
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
|
@ -8,6 +8,7 @@ from tools.skills_sync import (
|
|||
_write_manifest,
|
||||
_discover_bundled_skills,
|
||||
_compute_relative_dest,
|
||||
_dir_hash,
|
||||
sync_skills,
|
||||
MANIFEST_FILE,
|
||||
SKILLS_DIR,
|
||||
|
|
@ -54,6 +55,36 @@ class TestReadWriteManifest:
|
|||
assert result == {"skill-a", "skill-b"}
|
||||
|
||||
|
||||
class TestDirHash:
|
||||
def test_same_content_same_hash(self, tmp_path):
|
||||
dir_a = tmp_path / "a"
|
||||
dir_b = tmp_path / "b"
|
||||
for d in (dir_a, dir_b):
|
||||
d.mkdir()
|
||||
(d / "SKILL.md").write_text("# Test")
|
||||
(d / "main.py").write_text("print(1)")
|
||||
assert _dir_hash(dir_a) == _dir_hash(dir_b)
|
||||
|
||||
def test_different_content_different_hash(self, tmp_path):
|
||||
dir_a = tmp_path / "a"
|
||||
dir_b = tmp_path / "b"
|
||||
dir_a.mkdir()
|
||||
dir_b.mkdir()
|
||||
(dir_a / "SKILL.md").write_text("# Version 1")
|
||||
(dir_b / "SKILL.md").write_text("# Version 2")
|
||||
assert _dir_hash(dir_a) != _dir_hash(dir_b)
|
||||
|
||||
def test_empty_dir(self, tmp_path):
|
||||
d = tmp_path / "empty"
|
||||
d.mkdir()
|
||||
h = _dir_hash(d)
|
||||
assert isinstance(h, str) and len(h) == 32
|
||||
|
||||
def test_nonexistent_dir(self, tmp_path):
|
||||
h = _dir_hash(tmp_path / "nope")
|
||||
assert isinstance(h, str) # returns hash of empty content
|
||||
|
||||
|
||||
class TestDiscoverBundledSkills:
|
||||
def test_finds_skills_with_skill_md(self, tmp_path):
|
||||
# Create two skills
|
||||
|
|
@ -121,17 +152,20 @@ class TestSyncSkills:
|
|||
|
||||
assert len(result["copied"]) == 2
|
||||
assert result["total_bundled"] == 2
|
||||
assert result["updated"] == []
|
||||
assert result["cleaned"] == []
|
||||
assert (skills_dir / "category" / "new-skill" / "SKILL.md").exists()
|
||||
assert (skills_dir / "old-skill" / "SKILL.md").exists()
|
||||
# DESCRIPTION.md should also be copied
|
||||
assert (skills_dir / "category" / "DESCRIPTION.md").exists()
|
||||
|
||||
def test_update_skips_known_skills(self, tmp_path):
|
||||
def test_user_deleted_skill_not_re_added(self, tmp_path):
|
||||
"""Skill in manifest but not on disk = user deleted it. Don't re-add."""
|
||||
bundled = self._setup_bundled(tmp_path)
|
||||
skills_dir = tmp_path / "user_skills"
|
||||
manifest_file = skills_dir / ".bundled_manifest"
|
||||
skills_dir.mkdir(parents=True)
|
||||
# Pre-populate manifest with old-skill
|
||||
# old-skill is in manifest but NOT on disk (user deleted it)
|
||||
manifest_file.write_text("old-skill\n")
|
||||
|
||||
with patch("tools.skills_sync._get_bundled_dir", return_value=bundled), \
|
||||
|
|
@ -139,17 +173,83 @@ class TestSyncSkills:
|
|||
patch("tools.skills_sync.MANIFEST_FILE", manifest_file):
|
||||
result = sync_skills(quiet=True)
|
||||
|
||||
# Only new-skill should be copied, old-skill skipped
|
||||
# new-skill should be copied, old-skill should be skipped
|
||||
assert "new-skill" in result["copied"]
|
||||
assert "old-skill" not in result["copied"]
|
||||
assert result["skipped"] >= 1
|
||||
assert "old-skill" not in result.get("updated", [])
|
||||
assert not (skills_dir / "old-skill").exists()
|
||||
|
||||
def test_does_not_overwrite_existing_skill_dir(self, tmp_path):
|
||||
def test_existing_skill_gets_updated(self, tmp_path):
|
||||
"""Skill in manifest AND on disk with changed content = updated."""
|
||||
bundled = self._setup_bundled(tmp_path)
|
||||
skills_dir = tmp_path / "user_skills"
|
||||
manifest_file = skills_dir / ".bundled_manifest"
|
||||
|
||||
# Pre-create the skill dir with user content
|
||||
# Pre-create old-skill on disk with DIFFERENT content
|
||||
user_skill = skills_dir / "old-skill"
|
||||
user_skill.mkdir(parents=True)
|
||||
(user_skill / "SKILL.md").write_text("# Old version from last sync")
|
||||
# Mark it in the manifest
|
||||
manifest_file.write_text("old-skill\n")
|
||||
|
||||
with patch("tools.skills_sync._get_bundled_dir", return_value=bundled), \
|
||||
patch("tools.skills_sync.SKILLS_DIR", skills_dir), \
|
||||
patch("tools.skills_sync.MANIFEST_FILE", manifest_file):
|
||||
result = sync_skills(quiet=True)
|
||||
|
||||
# old-skill should be updated
|
||||
assert "old-skill" in result["updated"]
|
||||
assert (user_skill / "SKILL.md").read_text() == "# Old"
|
||||
|
||||
def test_unchanged_skill_not_updated(self, tmp_path):
|
||||
"""Skill in manifest AND on disk with same content = skipped."""
|
||||
bundled = self._setup_bundled(tmp_path)
|
||||
skills_dir = tmp_path / "user_skills"
|
||||
manifest_file = skills_dir / ".bundled_manifest"
|
||||
|
||||
# Pre-create old-skill on disk with SAME content
|
||||
user_skill = skills_dir / "old-skill"
|
||||
user_skill.mkdir(parents=True)
|
||||
(user_skill / "SKILL.md").write_text("# Old")
|
||||
# Mark it in the manifest
|
||||
manifest_file.write_text("old-skill\n")
|
||||
|
||||
with patch("tools.skills_sync._get_bundled_dir", return_value=bundled), \
|
||||
patch("tools.skills_sync.SKILLS_DIR", skills_dir), \
|
||||
patch("tools.skills_sync.MANIFEST_FILE", manifest_file):
|
||||
result = sync_skills(quiet=True)
|
||||
|
||||
# Should be skipped, not updated
|
||||
assert "old-skill" not in result.get("updated", [])
|
||||
assert result["skipped"] >= 1
|
||||
|
||||
def test_stale_manifest_entries_cleaned(self, tmp_path):
|
||||
"""Skills in manifest that no longer exist in bundled dir get cleaned."""
|
||||
bundled = self._setup_bundled(tmp_path)
|
||||
skills_dir = tmp_path / "user_skills"
|
||||
manifest_file = skills_dir / ".bundled_manifest"
|
||||
skills_dir.mkdir(parents=True)
|
||||
# Add a stale entry that doesn't exist in bundled
|
||||
manifest_file.write_text("old-skill\nremoved-skill\n")
|
||||
|
||||
with patch("tools.skills_sync._get_bundled_dir", return_value=bundled), \
|
||||
patch("tools.skills_sync.SKILLS_DIR", skills_dir), \
|
||||
patch("tools.skills_sync.MANIFEST_FILE", manifest_file):
|
||||
result = sync_skills(quiet=True)
|
||||
|
||||
assert "removed-skill" in result["cleaned"]
|
||||
# Verify manifest no longer has removed-skill
|
||||
with patch("tools.skills_sync.MANIFEST_FILE", manifest_file):
|
||||
manifest = _read_manifest()
|
||||
assert "removed-skill" not in manifest
|
||||
|
||||
def test_does_not_overwrite_existing_unmanifested_skill(self, tmp_path):
|
||||
"""New skill whose name collides with user-created skill = skipped."""
|
||||
bundled = self._setup_bundled(tmp_path)
|
||||
skills_dir = tmp_path / "user_skills"
|
||||
manifest_file = skills_dir / ".bundled_manifest"
|
||||
|
||||
# Pre-create the skill dir with user content (not in manifest)
|
||||
user_skill = skills_dir / "category" / "new-skill"
|
||||
user_skill.mkdir(parents=True)
|
||||
(user_skill / "SKILL.md").write_text("# User modified")
|
||||
|
|
@ -165,4 +265,4 @@ class TestSyncSkills:
|
|||
def test_nonexistent_bundled_dir(self, tmp_path):
|
||||
with patch("tools.skills_sync._get_bundled_dir", return_value=tmp_path / "nope"):
|
||||
result = sync_skills(quiet=True)
|
||||
assert result == {"copied": [], "skipped": 0, "total_bundled": 0}
|
||||
assert result == {"copied": [], "updated": [], "skipped": 0, "cleaned": [], "total_bundled": 0}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue