test_validation_rules.py

"""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(self):
        """Check that ALLOWED_FRONTMATTER_KEYS has expected 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_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_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):
        """meta with non-string 'name' → fatal (e.g., name: yes → True)."""
        issues = validate_skill_record(
            {"name": "", "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):
        """meta with non-string 'description' → fatal."""
        issues = validate_skill_record(
            {"name": "test", "description": "", "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


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()