From 7059dea3f8f050f718391753b975287596441636 Mon Sep 17 00:00:00 2001 From: teernisse Date: Thu, 26 Feb 2026 16:58:20 -0500 Subject: [PATCH] test(skills): add comprehensive SkillsMixin tests --- tests/test_skills.py | 549 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 549 insertions(+) create mode 100644 tests/test_skills.py diff --git a/tests/test_skills.py b/tests/test_skills.py new file mode 100644 index 0000000..47c19c7 --- /dev/null +++ b/tests/test_skills.py @@ -0,0 +1,549 @@ +import json +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from amc_server.mixins.skills import SkillsMixin + + +class DummySkillsHandler(SkillsMixin): + def __init__(self): + self.sent = [] + + def _send_json(self, code, payload): + self.sent.append((code, payload)) + + +class TestEnumerateClaudeSkills(unittest.TestCase): + """Tests for _enumerate_claude_skills.""" + + def setUp(self): + self.handler = DummySkillsHandler() + + def test_empty_directory(self): + """Returns empty list when ~/.claude/skills doesn't exist.""" + with tempfile.TemporaryDirectory() as tmpdir: + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + self.assertEqual(result, []) + + def test_reads_skill_md(self): + """Reads description from SKILL.md.""" + with tempfile.TemporaryDirectory() as tmpdir: + skill_dir = Path(tmpdir) / ".claude/skills/my-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("Does something useful") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["name"], "my-skill") + self.assertEqual(result[0]["description"], "Does something useful") + + def test_fallback_to_readme(self): + """Falls back to README.md when SKILL.md is missing.""" + with tempfile.TemporaryDirectory() as tmpdir: + skill_dir = Path(tmpdir) / ".claude/skills/readme-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "README.md").write_text("# Header\n\nReadme description here") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["description"], "Readme description here") + + def test_fallback_priority_order(self): + """SKILL.md takes priority over skill.md, prompt.md, README.md.""" + with tempfile.TemporaryDirectory() as tmpdir: + skill_dir = Path(tmpdir) / ".claude/skills/priority-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("From SKILL.md") + (skill_dir / "README.md").write_text("From README.md") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + + self.assertEqual(result[0]["description"], "From SKILL.md") + + def test_skill_md_lowercase_fallback(self): + """Falls back to skill.md when SKILL.md is missing.""" + with tempfile.TemporaryDirectory() as tmpdir: + skill_dir = Path(tmpdir) / ".claude/skills/lower-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "skill.md").write_text("From lowercase skill.md") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + + self.assertEqual(result[0]["description"], "From lowercase skill.md") + + def test_skips_hidden_dirs(self): + """Ignores directories starting with dot.""" + with tempfile.TemporaryDirectory() as tmpdir: + skills_dir = Path(tmpdir) / ".claude/skills" + skills_dir.mkdir(parents=True) + # Visible skill + visible = skills_dir / "visible" + visible.mkdir() + (visible / "SKILL.md").write_text("Visible skill") + # Hidden skill + hidden = skills_dir / ".hidden" + hidden.mkdir() + (hidden / "SKILL.md").write_text("Hidden skill") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + + names = [s["name"] for s in result] + self.assertIn("visible", names) + self.assertNotIn(".hidden", names) + + def test_skips_files_in_skills_dir(self): + """Only processes directories, not loose files.""" + with tempfile.TemporaryDirectory() as tmpdir: + skills_dir = Path(tmpdir) / ".claude/skills" + skills_dir.mkdir(parents=True) + (skills_dir / "not-a-dir.txt").write_text("stray file") + skill = skills_dir / "real-skill" + skill.mkdir() + (skill / "SKILL.md").write_text("A real skill") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["name"], "real-skill") + + def test_truncates_description(self): + """Description truncated to 100 chars.""" + with tempfile.TemporaryDirectory() as tmpdir: + skill_dir = Path(tmpdir) / ".claude/skills/long-skill" + skill_dir.mkdir(parents=True) + long_desc = "A" * 200 + (skill_dir / "SKILL.md").write_text(long_desc) + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + + self.assertEqual(len(result[0]["description"]), 100) + + def test_skips_headers(self): + """First non-header line used as description.""" + with tempfile.TemporaryDirectory() as tmpdir: + skill_dir = Path(tmpdir) / ".claude/skills/header-skill" + skill_dir.mkdir(parents=True) + content = "# My Skill\n## Subtitle\n\nActual description" + (skill_dir / "SKILL.md").write_text(content) + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + + self.assertEqual(result[0]["description"], "Actual description") + + def test_no_description_uses_fallback(self): + """Empty/header-only skill uses 'Skill: name' fallback.""" + with tempfile.TemporaryDirectory() as tmpdir: + skill_dir = Path(tmpdir) / ".claude/skills/empty-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("# Only Headers\n## Nothing else") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + + self.assertEqual(result[0]["description"], "Skill: empty-skill") + + def test_no_md_files_uses_fallback(self): + """Skill dir with no markdown files uses fallback description.""" + with tempfile.TemporaryDirectory() as tmpdir: + skill_dir = Path(tmpdir) / ".claude/skills/bare-skill" + skill_dir.mkdir(parents=True) + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_claude_skills() + + self.assertEqual(result[0]["description"], "Skill: bare-skill") + + def test_os_error_on_read_continues(self): + """OSError when reading file doesn't crash enumeration.""" + with tempfile.TemporaryDirectory() as tmpdir: + skill_dir = Path(tmpdir) / ".claude/skills/broken-skill" + skill_dir.mkdir(parents=True) + md = skill_dir / "SKILL.md" + md.write_text("content") + + with patch.object(Path, "home", return_value=Path(tmpdir)), \ + patch.object(Path, "read_text", side_effect=OSError("disk error")): + result = self.handler._enumerate_claude_skills() + + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["description"], "Skill: broken-skill") + + +class TestExtractDescription(unittest.TestCase): + """Tests for _extract_description.""" + + def setUp(self): + self.handler = DummySkillsHandler() + + def test_empty_content(self): + self.assertEqual(self.handler._extract_description(""), "") + + def test_plain_text(self): + self.assertEqual( + self.handler._extract_description("Simple description"), + "Simple description", + ) + + def test_yaml_frontmatter_description(self): + content = "---\ndescription: A skill for formatting\n---\nBody text" + self.assertEqual( + self.handler._extract_description(content), + "A skill for formatting", + ) + + def test_yaml_frontmatter_quoted_description(self): + content = '---\ndescription: "Quoted desc"\n---\n' + self.assertEqual( + self.handler._extract_description(content), + "Quoted desc", + ) + + def test_yaml_frontmatter_single_quoted_description(self): + content = "---\ndescription: 'Single quoted'\n---\n" + self.assertEqual( + self.handler._extract_description(content), + "Single quoted", + ) + + def test_yaml_multiline_fold_indicator(self): + """Handles >- style multi-line YAML.""" + content = "---\ndescription: >-\n Multi-line folded text\n---\n" + self.assertEqual( + self.handler._extract_description(content), + "Multi-line folded text", + ) + + def test_yaml_multiline_literal_indicator(self): + """Handles |- style multi-line YAML.""" + content = "---\ndescription: |-\n Literal block text\n---\n" + self.assertEqual( + self.handler._extract_description(content), + "Literal block text", + ) + + def test_yaml_multiline_bare_fold(self): + """Handles > without trailing dash.""" + content = "---\ndescription: >\n Bare fold\n---\n" + self.assertEqual( + self.handler._extract_description(content), + "Bare fold", + ) + + def test_yaml_multiline_bare_literal(self): + """Handles | without trailing dash.""" + content = "---\ndescription: |\n Bare literal\n---\n" + self.assertEqual( + self.handler._extract_description(content), + "Bare literal", + ) + + def test_yaml_empty_description_falls_back_to_body(self): + """Empty description in frontmatter falls back to body text.""" + content = "---\ndescription:\n---\nFallback body line" + self.assertEqual( + self.handler._extract_description(content), + "Fallback body line", + ) + + def test_skips_headers_and_empty_lines(self): + content = "# Title\n\n## Section\n\nActual content" + self.assertEqual( + self.handler._extract_description(content), + "Actual content", + ) + + def test_skips_html_comments(self): + content = "\nReal content" + self.assertEqual( + self.handler._extract_description(content), + "Real content", + ) + + def test_truncates_to_100_chars(self): + long_line = "B" * 150 + self.assertEqual( + len(self.handler._extract_description(long_line)), + 100, + ) + + def test_frontmatter_description_truncated(self): + desc = "C" * 150 + content = f"---\ndescription: {desc}\n---\n" + self.assertEqual( + len(self.handler._extract_description(content)), + 100, + ) + + def test_no_closing_frontmatter_extracts_description(self): + """Unclosed frontmatter still extracts description from the loop.""" + content = "---\ndescription: Orphaned\ntitle: Test" + # The frontmatter loop finds "description:" and returns early, + # even though there's no closing "---" + result = self.handler._extract_description(content) + self.assertEqual(result, "Orphaned") + + def test_body_only_headers_returns_empty(self): + content = "# H1\n## H2\n### H3" + self.assertEqual(self.handler._extract_description(content), "") + + +class TestEnumerateCodexSkills(unittest.TestCase): + """Tests for _enumerate_codex_skills.""" + + def setUp(self): + self.handler = DummySkillsHandler() + + def test_reads_cache(self): + """Reads skills from skills-curated-cache.json.""" + with tempfile.TemporaryDirectory() as tmpdir: + cache_dir = Path(tmpdir) / ".codex/vendor_imports" + cache_dir.mkdir(parents=True) + cache = { + "skills": [ + {"id": "lint", "shortDescription": "Lint code"}, + {"name": "deploy", "description": "Deploy to prod"}, + ] + } + (cache_dir / "skills-curated-cache.json").write_text(json.dumps(cache)) + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + self.assertEqual(len(result), 2) + self.assertEqual(result[0]["name"], "lint") + self.assertEqual(result[0]["description"], "Lint code") + # Falls back to 'name' when 'id' is absent + self.assertEqual(result[1]["name"], "deploy") + self.assertEqual(result[1]["description"], "Deploy to prod") + + def test_id_preferred_over_name(self): + """Uses 'id' field preferentially over 'name'.""" + with tempfile.TemporaryDirectory() as tmpdir: + cache_dir = Path(tmpdir) / ".codex/vendor_imports" + cache_dir.mkdir(parents=True) + cache = {"skills": [{"id": "the-id", "name": "the-name", "shortDescription": "desc"}]} + (cache_dir / "skills-curated-cache.json").write_text(json.dumps(cache)) + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + self.assertEqual(result[0]["name"], "the-id") + + def test_short_description_preferred(self): + """Uses 'shortDescription' preferentially over 'description'.""" + with tempfile.TemporaryDirectory() as tmpdir: + cache_dir = Path(tmpdir) / ".codex/vendor_imports" + cache_dir.mkdir(parents=True) + cache = {"skills": [{"id": "sk", "shortDescription": "short", "description": "long version"}]} + (cache_dir / "skills-curated-cache.json").write_text(json.dumps(cache)) + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + self.assertEqual(result[0]["description"], "short") + + def test_invalid_json(self): + """Continues without cache if JSON is invalid.""" + with tempfile.TemporaryDirectory() as tmpdir: + cache_dir = Path(tmpdir) / ".codex/vendor_imports" + cache_dir.mkdir(parents=True) + (cache_dir / "skills-curated-cache.json").write_text("{not valid json") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + self.assertEqual(result, []) + + def test_combines_cache_and_user(self): + """Returns both curated and user-installed skills.""" + with tempfile.TemporaryDirectory() as tmpdir: + # Curated + cache_dir = Path(tmpdir) / ".codex/vendor_imports" + cache_dir.mkdir(parents=True) + cache = {"skills": [{"id": "curated-skill", "shortDescription": "Curated"}]} + (cache_dir / "skills-curated-cache.json").write_text(json.dumps(cache)) + # User + user_skill = Path(tmpdir) / ".codex/skills/user-skill" + user_skill.mkdir(parents=True) + (user_skill / "SKILL.md").write_text("User installed skill") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + names = [s["name"] for s in result] + self.assertIn("curated-skill", names) + self.assertIn("user-skill", names) + self.assertEqual(len(result), 2) + + def test_no_deduplication(self): + """Duplicate names from cache and user both appear.""" + with tempfile.TemporaryDirectory() as tmpdir: + # Curated with name "dupe" + cache_dir = Path(tmpdir) / ".codex/vendor_imports" + cache_dir.mkdir(parents=True) + cache = {"skills": [{"id": "dupe", "shortDescription": "From cache"}]} + (cache_dir / "skills-curated-cache.json").write_text(json.dumps(cache)) + # User with same name "dupe" + user_skill = Path(tmpdir) / ".codex/skills/dupe" + user_skill.mkdir(parents=True) + (user_skill / "SKILL.md").write_text("From user dir") + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + dupe_entries = [s for s in result if s["name"] == "dupe"] + self.assertEqual(len(dupe_entries), 2) + + def test_empty_no_cache_no_user_dir(self): + """Returns empty list when neither cache nor user dir exists.""" + with tempfile.TemporaryDirectory() as tmpdir: + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + self.assertEqual(result, []) + + def test_skips_entries_without_name_or_id(self): + """Cache entries without name or id are skipped.""" + with tempfile.TemporaryDirectory() as tmpdir: + cache_dir = Path(tmpdir) / ".codex/vendor_imports" + cache_dir.mkdir(parents=True) + cache = {"skills": [{"shortDescription": "No name"}, {"id": "valid", "shortDescription": "OK"}]} + (cache_dir / "skills-curated-cache.json").write_text(json.dumps(cache)) + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["name"], "valid") + + def test_missing_description_uses_fallback(self): + """Cache entry without description uses 'Skill: name' fallback.""" + with tempfile.TemporaryDirectory() as tmpdir: + cache_dir = Path(tmpdir) / ".codex/vendor_imports" + cache_dir.mkdir(parents=True) + cache = {"skills": [{"id": "bare"}]} + (cache_dir / "skills-curated-cache.json").write_text(json.dumps(cache)) + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + self.assertEqual(result[0]["description"], "Skill: bare") + + def test_user_skill_without_skill_md_uses_fallback(self): + """User skill dir without SKILL.md uses fallback description.""" + with tempfile.TemporaryDirectory() as tmpdir: + user_skill = Path(tmpdir) / ".codex/skills/no-md" + user_skill.mkdir(parents=True) + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + self.assertEqual(result[0]["description"], "User skill: no-md") + + def test_user_skills_skip_hidden_dirs(self): + """Hidden directories in user skills dir are skipped.""" + with tempfile.TemporaryDirectory() as tmpdir: + user_dir = Path(tmpdir) / ".codex/skills" + user_dir.mkdir(parents=True) + (user_dir / "visible").mkdir() + (user_dir / ".hidden").mkdir() + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + names = [s["name"] for s in result] + self.assertIn("visible", names) + self.assertNotIn(".hidden", names) + + def test_description_truncated_to_100(self): + """Codex cache description truncated to 100 chars.""" + with tempfile.TemporaryDirectory() as tmpdir: + cache_dir = Path(tmpdir) / ".codex/vendor_imports" + cache_dir.mkdir(parents=True) + cache = {"skills": [{"id": "long", "shortDescription": "D" * 200}]} + (cache_dir / "skills-curated-cache.json").write_text(json.dumps(cache)) + + with patch.object(Path, "home", return_value=Path(tmpdir)): + result = self.handler._enumerate_codex_skills() + + self.assertEqual(len(result[0]["description"]), 100) + + +class TestServeSkills(unittest.TestCase): + """Tests for _serve_skills.""" + + def setUp(self): + self.handler = DummySkillsHandler() + + def test_claude_trigger(self): + """Returns / trigger for claude agent.""" + with patch.object(self.handler, "_enumerate_claude_skills", return_value=[]): + self.handler._serve_skills("claude") + + self.assertEqual(self.handler.sent[0][0], 200) + self.assertEqual(self.handler.sent[0][1]["trigger"], "/") + + def test_codex_trigger(self): + """Returns $ trigger for codex agent.""" + with patch.object(self.handler, "_enumerate_codex_skills", return_value=[]): + self.handler._serve_skills("codex") + + self.assertEqual(self.handler.sent[0][0], 200) + self.assertEqual(self.handler.sent[0][1]["trigger"], "$") + + def test_default_to_claude(self): + """Unknown agent defaults to claude (/ trigger).""" + with patch.object(self.handler, "_enumerate_claude_skills", return_value=[]): + self.handler._serve_skills("unknown-agent") + + self.assertEqual(self.handler.sent[0][1]["trigger"], "/") + + def test_alphabetical_sort(self): + """Skills sorted alphabetically (case-insensitive).""" + skills = [ + {"name": "Zebra", "description": "z"}, + {"name": "alpha", "description": "a"}, + {"name": "Beta", "description": "b"}, + ] + with patch.object(self.handler, "_enumerate_claude_skills", return_value=skills): + self.handler._serve_skills("claude") + + result_names = [s["name"] for s in self.handler.sent[0][1]["skills"]] + self.assertEqual(result_names, ["alpha", "Beta", "Zebra"]) + + def test_response_format(self): + """Response has trigger and skills keys.""" + skills = [{"name": "test", "description": "A test skill"}] + with patch.object(self.handler, "_enumerate_claude_skills", return_value=skills): + self.handler._serve_skills("claude") + + code, payload = self.handler.sent[0] + self.assertEqual(code, 200) + self.assertIn("trigger", payload) + self.assertIn("skills", payload) + self.assertEqual(len(payload["skills"]), 1) + + def test_empty_skills_list(self): + """Empty skill list still returns valid response.""" + with patch.object(self.handler, "_enumerate_claude_skills", return_value=[]): + self.handler._serve_skills("claude") + + payload = self.handler.sent[0][1] + self.assertEqual(payload["skills"], []) + self.assertEqual(payload["trigger"], "/") + + +if __name__ == "__main__": + unittest.main()