test: add unit tests for skill frontmatter validation (#1309)

* test: add unit tests for skill frontmatter validation

Cover _validate_skill_frontmatter logic:
- Valid minimal and full-field skills
- Missing SKILL.md, missing frontmatter, invalid YAML
- Required field validation (name, description)
- Unexpected key rejection
- Name format: hyphen-case, no leading/trailing/consecutive hyphens
- Name and description length limits
- Angle bracket rejection in description

* test: fix unused variables flagged by ruff F841

Replace unused tuple elements with _ and add assertions on
msg/name return values in success-path tests.

* test: address review feedback on unused variables

* test: consolidate validation tests into single module

Move the UTF-8/windows-locale test from test_skills_router.py into
test_skills_validation.py and remove test_skills_router.py to eliminate
duplicated assertions and future maintenance drift.

* fix: match assertion strings to actual validation messages

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>

backend/tests/test_skills_router.py

@@ -1,88 +0,0 @@
-from collections.abc import Callable
-from pathlib import Path
-from typing import cast
-
-from deerflow.skills.validation import _validate_skill_frontmatter
-
-VALIDATE_SKILL_FRONTMATTER = cast(
-    Callable[[Path], tuple[bool, str, str | None]],
-    _validate_skill_frontmatter,
-)
-
-
-def _write_skill(skill_dir: Path, frontmatter: str) -> None:
-    skill_dir.mkdir(parents=True, exist_ok=True)
-    (skill_dir / "SKILL.md").write_text(frontmatter, encoding="utf-8")
-
-
-def test_validate_skill_frontmatter_allows_standard_optional_metadata(tmp_path: Path) -> None:
-    skill_dir = tmp_path / "demo-skill"
-    _write_skill(
-        skill_dir,
-        """---
-name: demo-skill
-description: Demo skill
-version: 1.0.0
-author: example.com/demo
-compatibility: OpenClaw >= 1.0
-license: MIT
----
-
-# Demo Skill
-""",
-    )
-
-    valid, message, skill_name = VALIDATE_SKILL_FRONTMATTER(skill_dir)
-
-    assert valid is True
-    assert message == "Skill is valid!"
-    assert skill_name == "demo-skill"
-
-
-def test_validate_skill_frontmatter_still_rejects_unknown_keys(tmp_path: Path) -> None:
-    skill_dir = tmp_path / "demo-skill"
-    _write_skill(
-        skill_dir,
-        """---
-name: demo-skill
-description: Demo skill
-unsupported: true
----
-
-# Demo Skill
-""",
-    )
-
-    valid, message, skill_name = VALIDATE_SKILL_FRONTMATTER(skill_dir)
-
-    assert valid is False
-    assert "unsupported" in message
-    assert skill_name is None
-
-
-def test_validate_skill_frontmatter_reads_utf8_on_windows_locale(tmp_path, monkeypatch) -> None:
-    skill_dir = tmp_path / "demo-skill"
-    _write_skill(
-        skill_dir,
-        """---
-name: demo-skill
-description: "Curly quotes: \u201cutf8\u201d"
----
-
-# Demo Skill
-""",
-    )
-
-    original_read_text = Path.read_text
-
-    def read_text_with_gbk_default(self, *args, **kwargs):
-        kwargs.setdefault("encoding", "gbk")
-        return original_read_text(self, *args, **kwargs)
-
-    monkeypatch.setattr(Path, "read_text", read_text_with_gbk_default)
-
-    valid, message, skill_name = VALIDATE_SKILL_FRONTMATTER(skill_dir)
-
-    assert valid is True
-    assert message == "Skill is valid!"
-    assert skill_name == "demo-skill"

backend/tests/test_skills_validation.py

@@ -0,0 +1,180 @@
+"""Tests for skill frontmatter validation.
+
+Consolidates all _validate_skill_frontmatter tests (previously split across
+test_skills_router.py and this module) into a single dedicated module.
+"""
+
+from pathlib import Path
+
+from deerflow.skills.validation import ALLOWED_FRONTMATTER_PROPERTIES, _validate_skill_frontmatter
+
+
+def _write_skill(tmp_path: Path, content: str) -> Path:
+    """Write a SKILL.md file and return its parent directory."""
+    skill_file = tmp_path / "SKILL.md"
+    skill_file.write_text(content, encoding="utf-8")
+    return tmp_path
+
+
+class TestValidateSkillFrontmatter:
+    def test_valid_minimal_skill(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\nname: my-skill\ndescription: A valid skill\n---\n\nBody\n",
+        )
+        valid, msg, name = _validate_skill_frontmatter(skill_dir)
+        assert valid is True
+        assert msg == "Skill is valid!"
+        assert name == "my-skill"
+
+    def test_valid_with_all_allowed_fields(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\nname: my-skill\ndescription: A skill\nlicense: MIT\nversion: '1.0'\nauthor: test\n---\n\nBody\n",
+        )
+        valid, msg, name = _validate_skill_frontmatter(skill_dir)
+        assert valid is True
+        assert msg == "Skill is valid!"
+        assert name == "my-skill"
+
+    def test_missing_skill_md(self, tmp_path):
+        valid, msg, name = _validate_skill_frontmatter(tmp_path)
+        assert valid is False
+        assert "not found" in msg
+        assert name is None
+
+    def test_no_frontmatter(self, tmp_path):
+        skill_dir = _write_skill(tmp_path, "# Just markdown\n\nNo front matter.\n")
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "frontmatter" in msg.lower()
+
+    def test_invalid_yaml(self, tmp_path):
+        skill_dir = _write_skill(tmp_path, "---\n[invalid yaml: {{\n---\n\nBody\n")
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "YAML" in msg
+
+    def test_missing_name(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\ndescription: A skill without a name\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "name" in msg.lower()
+
+    def test_missing_description(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\nname: my-skill\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "description" in msg.lower()
+
+    def test_unexpected_keys_rejected(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\nname: my-skill\ndescription: test\ncustom-field: bad\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "custom-field" in msg
+
+    def test_name_must_be_hyphen_case(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\nname: MySkill\ndescription: test\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "hyphen-case" in msg
+
+    def test_name_no_leading_hyphen(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\nname: -my-skill\ndescription: test\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "hyphen" in msg
+
+    def test_name_no_trailing_hyphen(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\nname: my-skill-\ndescription: test\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "hyphen" in msg
+
+    def test_name_no_consecutive_hyphens(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\nname: my--skill\ndescription: test\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "hyphen" in msg
+
+    def test_name_too_long(self, tmp_path):
+        long_name = "a" * 65
+        skill_dir = _write_skill(
+            tmp_path,
+            f"---\nname: {long_name}\ndescription: test\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "too long" in msg.lower()
+
+    def test_description_no_angle_brackets(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\nname: my-skill\ndescription: Has <html> tags\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "angle brackets" in msg.lower()
+
+    def test_description_too_long(self, tmp_path):
+        long_desc = "a" * 1025
+        skill_dir = _write_skill(
+            tmp_path,
+            f"---\nname: my-skill\ndescription: {long_desc}\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "too long" in msg.lower()
+
+    def test_empty_name_rejected(self, tmp_path):
+        skill_dir = _write_skill(
+            tmp_path,
+            "---\nname: ''\ndescription: test\n---\n\nBody\n",
+        )
+        valid, msg, _ = _validate_skill_frontmatter(skill_dir)
+        assert valid is False
+        assert "empty" in msg.lower()
+
+    def test_allowed_properties_constant(self):
+        assert "name" in ALLOWED_FRONTMATTER_PROPERTIES
+        assert "description" in ALLOWED_FRONTMATTER_PROPERTIES
+        assert "license" in ALLOWED_FRONTMATTER_PROPERTIES
+
+    def test_reads_utf8_on_windows_locale(self, tmp_path, monkeypatch):
+        skill_dir = _write_skill(
+            tmp_path,
+            '---\nname: demo-skill\ndescription: "Curly quotes: \u201cutf8\u201d"\n---\n\n# Demo Skill\n',
+        )
+        original_read_text = Path.read_text
+
+        def read_text_with_gbk_default(self, *args, **kwargs):
+            kwargs.setdefault("encoding", "gbk")
+            return original_read_text(self, *args, **kwargs)
+
+        monkeypatch.setattr(Path, "read_text", read_text_with_gbk_default)
+
+        valid, msg, name = _validate_skill_frontmatter(skill_dir)
+        assert valid is True
+        assert msg == "Skill is valid!"
+        assert name == "demo-skill"