439 lines
16 KiB
Python
439 lines
16 KiB
Python
"""Tests for provider-attached skills system."""
|
|
|
|
import textwrap
|
|
from types import SimpleNamespace
|
|
|
|
import pytest
|
|
|
|
from cmdforge.skills import (
|
|
Skill,
|
|
SkillSet,
|
|
_parse_frontmatter,
|
|
_validate_provider_name,
|
|
_validate_skill_name,
|
|
load_provider_skills,
|
|
inject_skills,
|
|
)
|
|
from cmdforge.providers import ProviderResult
|
|
from cmdforge.runner import execute_prompt_step
|
|
from cmdforge.tool import PromptStep
|
|
|
|
|
|
def write_skill(root, provider, name, content="Skill instructions.", description="A useful skill"):
|
|
directory = root / provider / "skills" / name
|
|
directory.mkdir(parents=True)
|
|
(directory / "SKILL.md").write_text(
|
|
f"---\nname: {name}\ndescription: {description}\n---\n{content}",
|
|
encoding="utf-8",
|
|
)
|
|
return directory
|
|
|
|
|
|
class TestValidation:
|
|
def test_valid_provider_name(self):
|
|
_validate_provider_name("claude")
|
|
_validate_provider_name("openrouter")
|
|
_validate_provider_name("my-provider_1")
|
|
|
|
def test_invalid_provider_name_slash(self):
|
|
with pytest.raises(ValueError):
|
|
_validate_provider_name("claude/evil")
|
|
|
|
def test_invalid_provider_name_dots(self):
|
|
with pytest.raises(ValueError):
|
|
_validate_provider_name("../../etc")
|
|
|
|
def test_invalid_provider_name_empty(self):
|
|
with pytest.raises(ValueError):
|
|
_validate_provider_name("")
|
|
|
|
def test_valid_skill_name(self):
|
|
_validate_skill_name("python-utils", "claude")
|
|
|
|
@pytest.mark.parametrize("name", ["Python", "python_utils", "-python", "python-", "python--utils"])
|
|
def test_invalid_standard_skill_names(self, name):
|
|
with pytest.raises(ValueError, match="Invalid skill name"):
|
|
_validate_skill_name(name, "claude")
|
|
|
|
def test_skill_name_max_length(self):
|
|
_validate_skill_name("a" * 64, "claude")
|
|
with pytest.raises(ValueError, match="1-64"):
|
|
_validate_skill_name("a" * 65, "claude")
|
|
|
|
def test_invalid_skill_name_slash(self):
|
|
with pytest.raises(ValueError):
|
|
_validate_skill_name("a/b", "claude")
|
|
|
|
def test_invalid_skill_name_dotdot(self):
|
|
with pytest.raises(ValueError):
|
|
_validate_skill_name("..", "claude")
|
|
|
|
|
|
class TestFrontmatter:
|
|
def test_basic(self):
|
|
text = textwrap.dedent("""\
|
|
---
|
|
name: python-utils
|
|
description: Python helpers
|
|
---
|
|
Use list comprehensions.
|
|
""")
|
|
meta, content = _parse_frontmatter(text)
|
|
assert meta["name"] == "python-utils"
|
|
assert meta["description"] == "Python helpers"
|
|
assert content == "Use list comprehensions."
|
|
|
|
def test_no_frontmatter(self):
|
|
with pytest.raises(ValueError, match="must begin"):
|
|
_parse_frontmatter("Just raw content")
|
|
|
|
def test_unterminated_frontmatter(self):
|
|
with pytest.raises(ValueError, match="not terminated"):
|
|
_parse_frontmatter("---\nname: test\ndescription: Test")
|
|
|
|
def test_invalid_yaml_frontmatter(self):
|
|
text = "---\n: invalid yaml\n---\ncontent"
|
|
with pytest.raises(ValueError, match="Invalid YAML"):
|
|
_parse_frontmatter(text)
|
|
|
|
def test_non_dict_frontmatter(self):
|
|
text = "---\n- list item\n---\ncontent"
|
|
with pytest.raises(ValueError, match="mapping"):
|
|
_parse_frontmatter(text)
|
|
|
|
|
|
class TestLoadProviderSkills:
|
|
@pytest.fixture
|
|
def temp_providers_dir(self, tmp_path, monkeypatch):
|
|
monkeypatch.setattr("cmdforge.skills.PROVIDERS_DIR", tmp_path)
|
|
return tmp_path
|
|
|
|
def test_no_directory(self, temp_providers_dir):
|
|
skills = load_provider_skills("nonexistent")
|
|
assert isinstance(skills, SkillSet)
|
|
assert skills.skills == []
|
|
|
|
def test_empty_skills_dir(self, temp_providers_dir):
|
|
d = temp_providers_dir / "claude" / "skills"
|
|
d.mkdir(parents=True)
|
|
skills = load_provider_skills("claude")
|
|
assert skills.skills == []
|
|
|
|
def test_loads_skill(self, temp_providers_dir):
|
|
d = temp_providers_dir / "claude" / "skills" / "python"
|
|
d.mkdir(parents=True)
|
|
(d / "SKILL.md").write_text(textwrap.dedent("""\
|
|
---
|
|
name: python
|
|
description: Python coding patterns
|
|
---
|
|
Use type hints always.
|
|
"""))
|
|
|
|
skillset = load_provider_skills("claude")
|
|
assert len(skillset.skills) == 1
|
|
skill = skillset.skills[0]
|
|
assert skill.name == "python"
|
|
assert skill.description == "Python coding patterns"
|
|
assert "type hints" in skill.content
|
|
|
|
def test_skips_non_md_files(self, temp_providers_dir):
|
|
d = write_skill(temp_providers_dir, "claude", "python")
|
|
(d / "readme.txt").write_text("ignore me")
|
|
|
|
skillset = load_provider_skills("claude")
|
|
assert len(skillset.skills) == 1
|
|
|
|
def test_sorted_by_name(self, temp_providers_dir):
|
|
for name in ("z-skill", "a-skill", "m-skill"):
|
|
write_skill(temp_providers_dir, "claude", name)
|
|
|
|
skillset = load_provider_skills("claude")
|
|
names = [s.name for s in skillset.skills]
|
|
assert names == ["a-skill", "m-skill", "z-skill"]
|
|
|
|
def test_frontmatter_name_must_match_directory(self, temp_providers_dir):
|
|
d = temp_providers_dir / "claude" / "skills" / "python"
|
|
d.mkdir(parents=True)
|
|
(d / "SKILL.md").write_text(
|
|
"---\nname: wrong-name\ndescription: Wrong\n---\ncontent"
|
|
)
|
|
|
|
with pytest.raises(ValueError, match="does not match"):
|
|
load_provider_skills("claude")
|
|
|
|
def test_invalid_provider_name_traversal(self, temp_providers_dir):
|
|
with pytest.raises(ValueError):
|
|
load_provider_skills("../secrets")
|
|
|
|
def test_skips_non_directories(self, temp_providers_dir):
|
|
d = temp_providers_dir / "claude" / "skills"
|
|
d.mkdir(parents=True)
|
|
(d / "file.txt").write_text("not a skill dir")
|
|
skillset = load_provider_skills("claude")
|
|
assert skillset.skills == []
|
|
|
|
def test_no_skill_md(self, temp_providers_dir):
|
|
d = temp_providers_dir / "claude" / "skills" / "empty-dir"
|
|
d.mkdir(parents=True)
|
|
skillset = load_provider_skills("claude")
|
|
assert skillset.skills == []
|
|
|
|
@pytest.mark.parametrize(
|
|
("frontmatter", "message"),
|
|
[
|
|
("description: Useful", "required 'name'"),
|
|
("name: python", "required 'description'"),
|
|
("name: python\ndescription: ''", "non-empty"),
|
|
],
|
|
)
|
|
def test_required_metadata(self, temp_providers_dir, frontmatter, message):
|
|
directory = temp_providers_dir / "claude" / "skills" / "python"
|
|
directory.mkdir(parents=True)
|
|
(directory / "SKILL.md").write_text(
|
|
f"---\n{frontmatter}\n---\ncontent"
|
|
)
|
|
with pytest.raises(ValueError, match=message):
|
|
load_provider_skills("claude")
|
|
|
|
def test_description_max_length(self, temp_providers_dir):
|
|
write_skill(
|
|
temp_providers_dir,
|
|
"claude",
|
|
"python",
|
|
description="x" * 1025,
|
|
)
|
|
with pytest.raises(ValueError, match="1024"):
|
|
load_provider_skills("claude")
|
|
|
|
def test_rejects_symlinked_provider_directory(self, temp_providers_dir, tmp_path):
|
|
outside = tmp_path / "outside-provider"
|
|
outside.mkdir()
|
|
(temp_providers_dir / "claude").symlink_to(outside, target_is_directory=True)
|
|
with pytest.raises(ValueError, match="symlinked provider"):
|
|
load_provider_skills("claude")
|
|
|
|
def test_rejects_symlinked_skill_directory(self, temp_providers_dir, tmp_path):
|
|
skills_dir = temp_providers_dir / "claude" / "skills"
|
|
skills_dir.mkdir(parents=True)
|
|
outside = tmp_path / "outside-skill"
|
|
outside.mkdir()
|
|
(skills_dir / "python").symlink_to(outside, target_is_directory=True)
|
|
with pytest.raises(ValueError, match="symlinked skill directory"):
|
|
load_provider_skills("claude")
|
|
|
|
def test_rejects_symlinked_skill_file(self, temp_providers_dir, tmp_path):
|
|
directory = temp_providers_dir / "claude" / "skills" / "python"
|
|
directory.mkdir(parents=True)
|
|
outside = tmp_path / "outside-skill.md"
|
|
outside.write_text("---\nname: python\ndescription: Test\n---\ncontent")
|
|
(directory / "SKILL.md").symlink_to(outside)
|
|
with pytest.raises(ValueError, match="symlinked SKILL.md"):
|
|
load_provider_skills("claude")
|
|
|
|
|
|
class TestSkillSetSelection:
|
|
def test_get_all_when_none(self):
|
|
skills = SkillSet("test", [
|
|
Skill("a", "a-dir", "desc A", "content A"),
|
|
Skill("b", "b-dir", "desc B", "content B"),
|
|
])
|
|
result = skills.get(None)
|
|
assert len(result) == 2
|
|
|
|
def test_get_specific(self):
|
|
skills = SkillSet("test", [
|
|
Skill("a", "a-dir", "desc A", "content A"),
|
|
Skill("b", "b-dir", "desc B", "content B"),
|
|
])
|
|
result = skills.get(["a"])
|
|
assert len(result) == 1
|
|
assert result[0].name == "a"
|
|
|
|
def test_get_empty_list(self):
|
|
skills = SkillSet("test", [Skill("a", "a-dir", "desc", "c")])
|
|
assert skills.get([]) == []
|
|
|
|
def test_warn_on_missing(self, capsys):
|
|
skills = SkillSet("test", [Skill("a", "a-dir", "desc", "c")])
|
|
result = skills.get(["missing"], on_missing="warn")
|
|
assert result == []
|
|
captured = capsys.readouterr()
|
|
assert "not found" in captured.err
|
|
|
|
def test_error_on_missing(self):
|
|
skills = SkillSet("test", [Skill("a", "a-dir", "desc", "c")])
|
|
with pytest.raises(KeyError, match="not found"):
|
|
skills.get(["missing"], on_missing="error")
|
|
|
|
def test_ignore_on_missing(self):
|
|
skills = SkillSet("test", [Skill("a", "a-dir", "desc", "c")])
|
|
result = skills.get(["missing"], on_missing="ignore")
|
|
assert result == []
|
|
|
|
def test_invalid_missing_policy(self):
|
|
skills = SkillSet("test")
|
|
with pytest.raises(ValueError, match="missing-skill policy"):
|
|
skills.get([], on_missing="silent")
|
|
|
|
|
|
class TestInjectSkills:
|
|
@pytest.fixture
|
|
def temp_providers_dir(self, tmp_path, monkeypatch):
|
|
monkeypatch.setattr("cmdforge.skills.PROVIDERS_DIR", tmp_path)
|
|
return tmp_path
|
|
|
|
def test_no_skills_no_injection(self, temp_providers_dir):
|
|
d = temp_providers_dir / "empty" / "skills"
|
|
d.mkdir(parents=True)
|
|
result = inject_skills("Hello", "empty")
|
|
assert result == "Hello"
|
|
|
|
def test_injects_skills_before_prompt(self, temp_providers_dir):
|
|
write_skill(temp_providers_dir, "claude", "python", "Use type hints.")
|
|
|
|
result = inject_skills("Do X", "claude")
|
|
assert result.startswith("## Skill: python")
|
|
assert "Do X" in result
|
|
assert result.index("## Skill") < result.index("Do X")
|
|
|
|
def test_specific_skill_names(self, temp_providers_dir):
|
|
for name in ("python", "git", "testing"):
|
|
write_skill(temp_providers_dir, "claude", name, f"{name} content")
|
|
|
|
result = inject_skills("Prompt", "claude", ["python"])
|
|
assert "python content" in result
|
|
assert "git content" not in result
|
|
assert "testing content" not in result
|
|
|
|
def test_invalid_provider_name(self, temp_providers_dir):
|
|
with pytest.raises(ValueError, match="Invalid provider"):
|
|
inject_skills("Prompt", "../../etc")
|
|
|
|
|
|
class TestPromptStepSkills:
|
|
def test_to_dict_excludes_none(self):
|
|
step = PromptStep(
|
|
prompt="Test",
|
|
provider="claude",
|
|
output_var="out",
|
|
)
|
|
d = step.to_dict()
|
|
assert "skills" not in d
|
|
|
|
def test_to_dict_includes_skills(self):
|
|
step = PromptStep(
|
|
prompt="Test",
|
|
provider="claude",
|
|
output_var="out",
|
|
skills=["python", "git"],
|
|
)
|
|
d = step.to_dict()
|
|
assert d["skills"] == ["python", "git"]
|
|
|
|
def test_roundtrip_with_skills(self):
|
|
step = PromptStep(
|
|
prompt="Test",
|
|
provider="claude",
|
|
output_var="out",
|
|
skills=["python"],
|
|
)
|
|
restored = PromptStep.from_dict(step.to_dict())
|
|
assert restored.skills == ["python"]
|
|
|
|
def test_roundtrip_without_skills(self):
|
|
step = PromptStep(
|
|
prompt="Test",
|
|
provider="claude",
|
|
output_var="out",
|
|
)
|
|
restored = PromptStep.from_dict(step.to_dict())
|
|
assert restored.skills is None
|
|
|
|
def test_roundtrip_with_explicit_empty_skills(self):
|
|
step = PromptStep("Test", "claude", "out", skills=[])
|
|
serialized = step.to_dict()
|
|
assert serialized["skills"] == []
|
|
assert PromptStep.from_dict(serialized).skills == []
|
|
|
|
@pytest.mark.parametrize("skills", ["python", [1], ["*", "python"], ["python", "python"]])
|
|
def test_rejects_invalid_skill_lists(self, skills):
|
|
with pytest.raises(ValueError):
|
|
PromptStep("Test", "claude", "out", skills=skills)
|
|
|
|
def test_rejects_invalid_skill_identifier(self):
|
|
with pytest.raises(ValueError, match="Invalid skill name"):
|
|
PromptStep("Test", "claude", "out", skills=["Python_Utils"])
|
|
|
|
|
|
class TestRunnerSkillIntegration:
|
|
@pytest.fixture
|
|
def configured_skill(self, tmp_path, monkeypatch):
|
|
monkeypatch.setattr("cmdforge.skills.PROVIDERS_DIR", tmp_path)
|
|
write_skill(tmp_path, "override", "python", "SKILL INSTRUCTIONS")
|
|
return tmp_path
|
|
|
|
def test_plain_text_uses_effective_provider_and_context_order(
|
|
self, configured_skill, monkeypatch
|
|
):
|
|
captured = {}
|
|
|
|
def fake_call(provider, prompt, max_tokens=None):
|
|
captured.update(provider=provider, prompt=prompt)
|
|
return ProviderResult(text="done", success=True)
|
|
|
|
monkeypatch.setattr("cmdforge.runner.call_provider", fake_call)
|
|
monkeypatch.setattr(
|
|
"cmdforge.runner.load_profile",
|
|
lambda name: SimpleNamespace(system_prompt="PROFILE INSTRUCTIONS"),
|
|
)
|
|
step = PromptStep(
|
|
prompt="USER PROMPT",
|
|
provider="original",
|
|
output_var="out",
|
|
profile="reviewer",
|
|
plain_text=True,
|
|
skills=["python"],
|
|
)
|
|
|
|
output, success = execute_prompt_step(
|
|
step, {}, provider_override="override"
|
|
)
|
|
|
|
assert success is True
|
|
assert output == "done"
|
|
assert captured["provider"] == "override"
|
|
prompt = captured["prompt"]
|
|
assert prompt.index("PROFILE INSTRUCTIONS") < prompt.index("## Skill: python")
|
|
assert prompt.index("## Skill: python") < prompt.index("USER PROMPT")
|
|
|
|
def test_structured_path_preserves_context_order(
|
|
self, configured_skill, monkeypatch
|
|
):
|
|
captured = {}
|
|
|
|
def fake_call(provider, prompt, max_tokens=None):
|
|
captured.update(provider=provider, prompt=prompt)
|
|
return ProviderResult(text='{"output": "done"}', success=True)
|
|
|
|
monkeypatch.setattr("cmdforge.runner.call_provider", fake_call)
|
|
monkeypatch.setattr(
|
|
"cmdforge.runner.load_profile",
|
|
lambda name: SimpleNamespace(system_prompt="PROFILE INSTRUCTIONS"),
|
|
)
|
|
step = PromptStep(
|
|
prompt="USER PROMPT",
|
|
provider="override",
|
|
output_var="out",
|
|
profile="reviewer",
|
|
skills=["python"],
|
|
)
|
|
|
|
output, success = execute_prompt_step(step, {})
|
|
|
|
assert success is True
|
|
assert output == {"output": "done"}
|
|
prompt = captured["prompt"]
|
|
assert prompt.index("PROFILE INSTRUCTIONS") < prompt.index("## Skill: python")
|
|
assert prompt.index("## Skill: python") < prompt.index("USER PROMPT")
|