"""Unit tests for skill validation rules (Agent Skills spec)."""
import pytest
from skillport.modules.skills.internal.validation import (
ALLOWED_FRONTMATTER_KEYS,
COMPATIBILITY_MAX_LENGTH,
DESCRIPTION_MAX_LENGTH,
NAME_MAX_LENGTH,
RESERVED_WORDS,
SKILL_LINE_THRESHOLD,
validate_skill_record,
)
class TestValidationFatal:
"""Fatal validation rules (exit code 1)."""
def test_name_required(self):
"""Missing name → fatal."""
issues = validate_skill_record({"name": "", "description": "desc", "path": "/a/b"})
fatal = [i for i in issues if i.severity == "fatal" and i.field == "name"]
assert len(fatal) == 1
assert "missing" in fatal[0].message.lower()
def test_description_required(self):
"""Missing description → fatal."""
issues = validate_skill_record({"name": "test", "description": "", "path": "/a/test"})
fatal = [i for i in issues if i.severity == "fatal" and i.field == "description"]
assert len(fatal) == 1
assert "missing" in fatal[0].message.lower()
def test_name_must_match_directory(self):
"""name != directory name → fatal."""
issues = validate_skill_record(
{
"name": "wrong-name",
"description": "desc",
"path": "/skills/correct-name",
}
)
fatal = [i for i in issues if i.severity == "fatal" and "match" in i.message.lower()]
assert len(fatal) == 1
assert "wrong-name" in fatal[0].message
assert "correct-name" in fatal[0].message
def test_name_max_length(self):
"""name > 64 chars → fatal."""
long_name = "a" * (NAME_MAX_LENGTH + 1)
issues = validate_skill_record(
{"name": long_name, "description": "desc", "path": f"/skills/{long_name}"}
)
fatal = [i for i in issues if i.severity == "fatal" and "chars" in i.message.lower()]
assert len(fatal) == 1
assert str(NAME_MAX_LENGTH) in fatal[0].message
def test_name_exactly_64_chars_ok(self):
"""name = 64 chars → ok."""
name = "a" * NAME_MAX_LENGTH
issues = validate_skill_record(
{"name": name, "description": "desc", "path": f"/skills/{name}"}
)
length_issues = [i for i in issues if "chars" in i.message.lower() and i.field == "name"]
assert len(length_issues) == 0
def test_name_invalid_chars_uppercase(self):
"""name with uppercase → fatal."""
issues = validate_skill_record(
{"name": "MySkill", "description": "desc", "path": "/skills/MySkill"}
)
fatal = [i for i in issues if i.severity == "fatal" and "invalid" in i.message.lower()]
assert len(fatal) == 1
assert "lowercase" in fatal[0].message.lower()
def test_name_invalid_chars_underscore(self):
"""name with underscore → fatal."""
issues = validate_skill_record(
{"name": "my_skill", "description": "desc", "path": "/skills/my_skill"}
)
fatal = [i for i in issues if i.severity == "fatal" and "invalid" in i.message.lower()]
assert len(fatal) == 1
def test_name_invalid_chars_space(self):
"""name with space → fatal."""
issues = validate_skill_record(
{"name": "my skill", "description": "desc", "path": "/skills/my skill"}
)
fatal = [i for i in issues if i.severity == "fatal" and "invalid" in i.message.lower()]
assert len(fatal) == 1
def test_name_valid_chars(self):
"""name with a-z, 0-9, - → ok."""
issues = validate_skill_record(
{
"name": "my-skill-123",
"description": "desc",
"path": "/skills/my-skill-123",
}
)
pattern_issues = [i for i in issues if "invalid" in i.message.lower()]
assert len(pattern_issues) == 0
def test_name_leading_hyphen(self):
"""name starting with hyphen → fatal."""
issues = validate_skill_record(
{"name": "-my-skill", "description": "desc", "path": "/skills/-my-skill"}
)
fatal = [i for i in issues if i.severity == "fatal" and "start or end" in i.message.lower()]
assert len(fatal) == 1
def test_name_trailing_hyphen(self):
"""name ending with hyphen → fatal."""
issues = validate_skill_record(
{"name": "my-skill-", "description": "desc", "path": "/skills/my-skill-"}
)
fatal = [i for i in issues if i.severity == "fatal" and "start or end" in i.message.lower()]
assert len(fatal) == 1
def test_name_consecutive_hyphens(self):
"""name with consecutive hyphens → fatal."""
issues = validate_skill_record(
{"name": "my--skill", "description": "desc", "path": "/skills/my--skill"}
)
fatal = [i for i in issues if i.severity == "fatal" and "consecutive" in i.message.lower()]
assert len(fatal) == 1
def test_name_valid_hyphens(self):
"""name with valid hyphen usage → ok."""
issues = validate_skill_record(
{
"name": "my-skill-name",
"description": "desc",
"path": "/skills/my-skill-name",
}
)
hyphen_issues = [i for i in issues if "hyphen" in i.message.lower()]
assert len(hyphen_issues) == 0
@pytest.mark.parametrize("reserved", list(RESERVED_WORDS))
def test_name_reserved_word(self, reserved: str):
"""name containing reserved word → fatal."""
name = f"my-{reserved}-skill"
issues = validate_skill_record(
{"name": name, "description": "desc", "path": f"/skills/{name}"}
)
fatal = [i for i in issues if i.severity == "fatal" and "reserved" in i.message.lower()]
assert len(fatal) == 1
assert reserved in fatal[0].message
def test_name_reserved_word_case_insensitive(self):
"""Reserved word check should be case-insensitive."""
# Note: name validation already fails on uppercase, but reserved check is independent
issues = validate_skill_record(
{"name": "my-skill", "description": "desc", "path": "/skills/my-skill"}
)
reserved_issues = [i for i in issues if "reserved" in i.message.lower()]
assert len(reserved_issues) == 0
def test_name_xml_tags(self):
"""name containing XML tags → fatal."""
issues = validate_skill_record(
{"name": "<script>", "description": "desc", "path": "/skills/<script>"}
)
fatal = [i for i in issues if i.severity == "fatal" and "xml" in i.message.lower()]
assert len(fatal) == 1
def test_description_xml_tags(self):
"""description containing XML tags → fatal."""
issues = validate_skill_record(
{
"name": "my-skill",
"description": "A skill with <script>alert('xss')</script> injection",
"path": "/skills/my-skill",
}
)
fatal = [i for i in issues if i.severity == "fatal" and "xml" in i.message.lower()]
assert len(fatal) == 1
assert "description" in fatal[0].field
def test_description_no_xml_tags_ok(self):
"""description without XML tags → ok."""
issues = validate_skill_record(
{
"name": "my-skill",
"description": "A valid description with math like 3 < 5 or x > y",
"path": "/skills/my-skill",
}
)
xml_issues = [i for i in issues if "xml" in i.message.lower()]
# "<" and ">" used separately (not forming tags) should pass
assert len(xml_issues) == 0
class TestValidationWarning:
"""Warning validation rules (exit code 0)."""
def test_skill_md_over_500_lines(self):
"""SKILL.md > 500 lines → warning."""
issues = validate_skill_record(
{
"name": "test",
"description": "desc",
"path": "/skills/test",
"lines": SKILL_LINE_THRESHOLD + 1,
}
)
warning = [i for i in issues if i.severity == "warning" and "lines" in i.message.lower()]
assert len(warning) == 1
assert str(SKILL_LINE_THRESHOLD) in warning[0].message
def test_skill_md_exactly_500_lines_ok(self):
"""SKILL.md = 500 lines → ok."""
issues = validate_skill_record(
{
"name": "test",
"description": "desc",
"path": "/skills/test",
"lines": SKILL_LINE_THRESHOLD,
}
)
line_issues = [i for i in issues if "lines" in i.message.lower()]
assert len(line_issues) == 0
def test_description_over_1024_chars(self):
"""description > 1024 chars → fatal."""
long_desc = "a" * (DESCRIPTION_MAX_LENGTH + 1)
issues = validate_skill_record(
{"name": "test", "description": long_desc, "path": "/skills/test"}
)
fatal = [i for i in issues if i.severity == "fatal" and "description" in i.message.lower()]
assert len(fatal) == 1
assert str(DESCRIPTION_MAX_LENGTH) in fatal[0].message
def test_description_exactly_1024_chars_ok(self):
"""description = 1024 chars → ok."""
desc = "a" * DESCRIPTION_MAX_LENGTH
issues = validate_skill_record(
{"name": "test", "description": desc, "path": "/skills/test"}
)
desc_length_issues = [
i
for i in issues
if i.severity == "warning"
and "description" in i.message.lower()
and "chars" in i.message.lower()
]
assert len(desc_length_issues) == 0
class TestValidationExitCode:
"""Exit code determination."""
def test_only_warnings_is_valid(self):
"""Only warnings → valid (exit 0)."""
issues = validate_skill_record(
{
"name": "test",
"description": "valid description",
"path": "/skills/test",
"lines": SKILL_LINE_THRESHOLD + 1, # warning (lines > 500)
}
)
# All issues should be warnings
assert all(i.severity == "warning" for i in issues)
assert len(issues) >= 1
def test_any_fatal_is_invalid(self):
"""Any fatal → invalid (exit 1)."""
issues = validate_skill_record(
{
"name": "", # fatal
"description": "desc",
"path": "/skills/test",
}
)
has_fatal = any(i.severity == "fatal" for i in issues)
assert has_fatal
def test_no_issues_is_valid(self):
"""No issues → valid (exit 0)."""
issues = validate_skill_record(
{
"name": "test",
"description": "A valid description",
"path": "/skills/test",
"lines": 100,
}
)
assert len(issues) == 0
class TestNameValidation:
"""Name character validation tests (Unicode lowercase support)."""
@pytest.mark.parametrize(
"valid_name",
[
"a",
"test",
"my-skill",
"skill-123",
"123-test",
"a-b-c-d",
"pdf",
"hello-world",
],
)
def test_valid_names(self, valid_name: str):
"""Valid name patterns should pass validation."""
issues = validate_skill_record(
{"name": valid_name, "description": "desc", "path": f"/skills/{valid_name}"}
)
invalid_issues = [i for i in issues if "invalid" in i.message.lower()]
assert len(invalid_issues) == 0
@pytest.mark.parametrize(
"invalid_name",
[
"A",
"Test",
"my_skill",
"my skill",
"my.skill",
"my/skill",
],
)
def test_invalid_names(self, invalid_name: str):
"""Invalid name patterns should fail validation."""
issues = validate_skill_record(
{"name": invalid_name, "description": "desc", "path": f"/skills/{invalid_name}"}
)
invalid_issues = [i for i in issues if "invalid" in i.message.lower()]
assert len(invalid_issues) == 1
def test_unicode_lowercase_allowed(self):
"""Unicode lowercase letters should be allowed (per Agent Skills spec)."""
# Japanese hiragana are lowercase letters (Ll category)
issues = validate_skill_record(
{"name": "skill-日本語", "description": "desc", "path": "/skills/skill-日本語"}
)
# Note: CJK ideographs are category Lo (letter, other), not Ll
# Only true lowercase letters like hiragana should pass
# This test documents the expected behavior
invalid_issues = [i for i in issues if "invalid" in i.message.lower()]
# CJK ideographs fail because they're Lo, not Ll
assert len(invalid_issues) == 1
class TestFrontmatterKeys:
"""Allowed frontmatter keys validation."""
def test_allowed_keys_set_agentskills_io(self):
"""Check that ALLOWED_FRONTMATTER_KEYS has agentskills.io spec values."""
assert "name" in ALLOWED_FRONTMATTER_KEYS
assert "description" in ALLOWED_FRONTMATTER_KEYS
assert "license" in ALLOWED_FRONTMATTER_KEYS
assert "allowed-tools" in ALLOWED_FRONTMATTER_KEYS
assert "metadata" in ALLOWED_FRONTMATTER_KEYS
assert "compatibility" in ALLOWED_FRONTMATTER_KEYS
def test_allowed_keys_set_claude_code_2_1(self):
"""Check that ALLOWED_FRONTMATTER_KEYS has Claude Code 2.1.0+ runtime fields."""
assert "model" in ALLOWED_FRONTMATTER_KEYS
assert "context" in ALLOWED_FRONTMATTER_KEYS
assert "agent" in ALLOWED_FRONTMATTER_KEYS
assert "hooks" in ALLOWED_FRONTMATTER_KEYS
assert "user-invocable" in ALLOWED_FRONTMATTER_KEYS
def test_unexpected_key_detected(self, tmp_path):
"""Unexpected frontmatter key → fatal (per Agent Skills spec)."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
skill_md = skill_dir / "SKILL.md"
skill_md.write_text("""---
name: my-skill
description: A test skill
author: someone
version: 1.0.0
---
# My Skill
""")
issues = validate_skill_record(
{"name": "my-skill", "description": "A test skill", "path": str(skill_dir)}
)
fatal = [
i for i in issues if i.severity == "fatal" and "unexpected" in i.message.lower()
]
assert len(fatal) == 1
assert "author" in fatal[0].message
assert "version" in fatal[0].message
def test_allowed_keys_no_warning(self, tmp_path):
"""All allowed frontmatter keys → no issues."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
skill_md = skill_dir / "SKILL.md"
skill_md.write_text("""---
name: my-skill
description: A test skill
license: MIT
compatibility: Requires Python 3.10+
allowed-tools:
- Read
- Write
metadata:
skillport:
category: test
---
# My Skill
""")
issues = validate_skill_record(
{"name": "my-skill", "description": "A test skill", "path": str(skill_dir)}
)
frontmatter_issues = [i for i in issues if "unexpected" in i.message.lower()]
assert len(frontmatter_issues) == 0
def test_claude_code_2_1_fields_allowed(self, tmp_path):
"""Claude Code 2.1.0+ runtime fields should pass validation."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
skill_md = skill_dir / "SKILL.md"
skill_md.write_text("""---
name: my-skill
description: A test skill with Claude Code 2.1.0 fields
model: claude-sonnet-4-20250514
context: fork
agent: Explore
user-invocable: false
hooks:
PreToolUse:
- matcher: Bash
hooks:
- type: command
command: echo "pre-hook"
---
# My Skill
""")
issues = validate_skill_record(
{"name": "my-skill", "description": "A test skill with Claude Code 2.1.0 fields", "path": str(skill_dir)}
)
frontmatter_issues = [i for i in issues if "unexpected" in i.message.lower()]
assert len(frontmatter_issues) == 0
def test_no_path_skips_frontmatter_check(self):
"""No path → skip frontmatter key check."""
issues = validate_skill_record({"name": "test", "description": "A test skill", "path": ""})
frontmatter_issues = [i for i in issues if "unexpected" in i.message.lower()]
assert len(frontmatter_issues) == 0
def test_nonexistent_path_skips_frontmatter_check(self):
"""Non-existent path → skip frontmatter key check."""
issues = validate_skill_record(
{
"name": "test",
"description": "A test skill",
"path": "/nonexistent/path/test",
}
)
frontmatter_issues = [i for i in issues if "unexpected" in i.message.lower()]
assert len(frontmatter_issues) == 0
class TestMetaKeyExistence:
"""Key existence checks when meta is provided."""
def test_name_key_missing_in_frontmatter(self):
"""meta without 'name' key → fatal."""
issues = validate_skill_record(
{"name": "test", "description": "desc", "path": "/skills/test"},
meta={"description": "desc"},
)
fatal = [
i for i in issues if i.severity == "fatal" and "'name' key is missing" in i.message
]
assert len(fatal) == 1
def test_description_key_missing_in_frontmatter(self):
"""meta without 'description' key → fatal."""
issues = validate_skill_record(
{"name": "test", "description": "desc", "path": "/skills/test"},
meta={"name": "test"},
)
fatal = [
i
for i in issues
if i.severity == "fatal" and "'description' key is missing" in i.message
]
assert len(fatal) == 1
def test_meta_none_skips_key_check(self):
"""meta=None → skip key existence check."""
issues = validate_skill_record(
{"name": "test", "description": "desc", "path": "/skills/test"},
meta=None,
)
key_missing = [i for i in issues if "key is missing" in i.message]
assert len(key_missing) == 0
def test_name_not_string_in_frontmatter(self):
"""Non-string 'name' → fatal (e.g., name: yes → True)."""
issues = validate_skill_record(
{"name": True, "description": "desc", "path": "/skills/test"},
meta={"name": True, "description": "desc"}, # YAML: name: yes
)
fatal = [
i for i in issues if i.severity == "fatal" and "must be a string" in i.message
]
assert len(fatal) == 1
assert "name" in fatal[0].field
assert "bool" in fatal[0].message
def test_description_not_string_in_frontmatter(self):
"""Non-string 'description' → fatal."""
issues = validate_skill_record(
{"name": "test", "description": ["item1", "item2"], "path": "/skills/test"},
meta={"name": "test", "description": ["item1", "item2"]}, # YAML list
)
fatal = [
i for i in issues if i.severity == "fatal" and "must be a string" in i.message
]
assert len(fatal) == 1
assert "description" in fatal[0].field
assert "list" in fatal[0].message
def test_non_string_description_no_crash(self):
"""Non-string description in skill dict should not crash on XML check."""
# This tests the case where skill dict has non-string values
# (e.g., from index or malformed input)
issues = validate_skill_record(
{"name": "test", "description": ["item1", "item2"], "path": "/skills/test"},
meta={"name": "test", "description": ["item1", "item2"]},
)
# Should not raise TypeError, should return type error issue
fatal = [i for i in issues if i.severity == "fatal"]
assert len(fatal) >= 1
assert any("must be a string" in i.message for i in fatal)
def test_non_string_name_no_crash(self):
"""Non-string name in skill dict should not crash on validation."""
issues = validate_skill_record(
{"name": True, "description": "desc", "path": "/skills/test"},
meta={"name": True, "description": "desc"},
)
# Should not raise TypeError
fatal = [i for i in issues if i.severity == "fatal"]
assert len(fatal) >= 1
assert any("must be a string" in i.message for i in fatal)
def test_non_string_without_meta(self):
"""Type check works even without meta (e.g., index-based validation)."""
# This is the critical case: validate via index without meta
issues = validate_skill_record(
{"name": True, "description": ["a", "b"], "path": "/skills/test"},
meta=None, # No meta provided
)
fatal = [i for i in issues if i.severity == "fatal"]
# Should detect both type errors
type_errors = [i for i in fatal if "must be a string" in i.message]
assert len(type_errors) == 2
assert any("name" in i.field for i in type_errors)
assert any("description" in i.field for i in type_errors)
def test_falsy_non_string_name_detected(self):
"""Falsy non-string name (null, [], False) should be detected as type error."""
for falsy_value in [None, [], False, 0]:
issues = validate_skill_record(
{"name": falsy_value, "description": "desc", "path": "/skills/test"},
)
fatal = [i for i in issues if i.severity == "fatal" and i.field == "name"]
assert len(fatal) >= 1, f"Failed for name={falsy_value!r}"
assert any("must be a string" in i.message for i in fatal), f"Failed for name={falsy_value!r}"
def test_falsy_non_string_description_detected(self):
"""Falsy non-string description (null, [], False) should be detected as type error."""
for falsy_value in [None, [], False, 0]:
issues = validate_skill_record(
{"name": "test", "description": falsy_value, "path": "/skills/test"},
)
fatal = [i for i in issues if i.severity == "fatal" and i.field == "description"]
assert len(fatal) >= 1, f"Failed for description={falsy_value!r}"
assert any("must be a string" in i.message for i in fatal), f"Failed for description={falsy_value!r}"
class TestStrictMode:
"""strict mode behavior tests."""
def test_strict_mode_filters_warnings(self):
"""strict=True → only fatal issues returned."""
issues = validate_skill_record(
{
"name": "test",
"description": "desc",
"path": "/skills/test",
"lines": SKILL_LINE_THRESHOLD + 1, # would be warning
},
strict=True,
)
# lines > 500 is a warning, should be filtered out
assert len(issues) == 0
def test_strict_false_includes_warnings(self):
"""strict=False → all issues returned."""
issues = validate_skill_record(
{
"name": "test",
"description": "desc",
"path": "/skills/test",
"lines": SKILL_LINE_THRESHOLD + 1, # warning
},
strict=False,
)
warnings = [i for i in issues if i.severity == "warning"]
assert len(warnings) == 1
class TestCompatibilityValidation:
"""Compatibility field validation tests."""
def test_compatibility_valid(self, tmp_path):
"""Valid compatibility field → no issues."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
skill_md = skill_dir / "SKILL.md"
skill_md.write_text("""---
name: my-skill
description: A test skill
compatibility: Requires Python 3.10+
---
# My Skill
""")
issues = validate_skill_record(
{"name": "my-skill", "description": "A test skill", "path": str(skill_dir)}
)
compat_issues = [i for i in issues if "compatibility" in i.message.lower()]
assert len(compat_issues) == 0
def test_compatibility_over_max_length(self, tmp_path):
"""compatibility > 500 chars → fatal."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
long_compat = "a" * (COMPATIBILITY_MAX_LENGTH + 1)
skill_md = skill_dir / "SKILL.md"
skill_md.write_text(f"""---
name: my-skill
description: A test skill
compatibility: {long_compat}
---
# My Skill
""")
issues = validate_skill_record(
{"name": "my-skill", "description": "A test skill", "path": str(skill_dir)}
)
fatal = [
i for i in issues if i.severity == "fatal" and "compatibility" in i.message.lower()
]
assert len(fatal) == 1
assert str(COMPATIBILITY_MAX_LENGTH) in fatal[0].message
def test_compatibility_exactly_max_length(self, tmp_path):
"""compatibility = 500 chars → ok."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
compat = "a" * COMPATIBILITY_MAX_LENGTH
skill_md = skill_dir / "SKILL.md"
skill_md.write_text(f"""---
name: my-skill
description: A test skill
compatibility: {compat}
---
# My Skill
""")
issues = validate_skill_record(
{"name": "my-skill", "description": "A test skill", "path": str(skill_dir)}
)
compat_issues = [i for i in issues if "compatibility" in i.message.lower()]
assert len(compat_issues) == 0
def test_compatibility_not_string(self, tmp_path):
"""compatibility not a string → fatal."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
skill_md = skill_dir / "SKILL.md"
skill_md.write_text("""---
name: my-skill
description: A test skill
compatibility:
- item1
- item2
---
# My Skill
""")
issues = validate_skill_record(
{"name": "my-skill", "description": "A test skill", "path": str(skill_dir)}
)
fatal = [
i for i in issues if i.severity == "fatal" and "compatibility" in i.message.lower()
]
assert len(fatal) == 1
assert "string" in fatal[0].message.lower()
