mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-14 16:08:41 +00:00
df68b59149
* fix(skills): reject a blank SKILL.md description at the write gate
_validate_skill_frontmatter only applied its description rules when the
value was truthy, so a blank or whitespace-only description passed the
gate. The loader rejects it, so the file was written to disk and then
disappeared from every consumer.
On the PUT edit endpoint that meant the write committed first and the
response was a 404 -- the previously working skill was gone with no
rollback. Same shape via the .skill install path and the agent-facing
skill_manage tool, which reported success for a skill that never loads.
Empty names were already rejected; description was the only field where
the write gate and the loader disagreed.
* test(skills): pin rollback rejection of a blank-description history entry
Add a regression test for the rollback path: restoring a history entry
whose stored content has an empty description must return 400
("Description cannot be empty") and leave the on-disk SKILL.md untouched,
rather than the previous destructive path that wrote the unloadable
content and then 404'd. On main this returns 404, so the test also pins
the intended status-code change on this path.
265 lines
9.7 KiB
Python
265 lines
9.7 KiB
Python
"""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\nallowed-tools: [bash, read_file]\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_allows_empty_allowed_tools(self, tmp_path):
|
|
skill_dir = _write_skill(
|
|
tmp_path,
|
|
"---\nname: my-skill\ndescription: A skill\nallowed-tools: []\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_allows_argument_hint(self, tmp_path):
|
|
skill_dir = _write_skill(
|
|
tmp_path,
|
|
"---\nname: my-skill\ndescription: A skill\nargument-hint: '[issue-number]'\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_allows_allowed_tools_string(self, tmp_path):
|
|
skill_dir = _write_skill(
|
|
tmp_path,
|
|
"---\nname: my-skill\ndescription: A skill\nallowed-tools: Bash(tvly *) Bash(playwright-cli:*)\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_rejects_allowed_tools_non_string_entry(self, tmp_path):
|
|
skill_dir = _write_skill(
|
|
tmp_path,
|
|
"---\nname: my-skill\ndescription: A skill\nallowed-tools: [bash, 1]\n---\n\nBody\n",
|
|
)
|
|
valid, msg, name = _validate_skill_frontmatter(skill_dir)
|
|
assert valid is False
|
|
assert "allowed-tools" in msg
|
|
assert str(tmp_path) not in msg
|
|
assert "SKILL.md" in msg
|
|
assert name is None
|
|
|
|
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_non_string_frontmatter_key_reports_cleanly_instead_of_crashing(self, tmp_path):
|
|
skill_dir = _write_skill(
|
|
tmp_path,
|
|
"---\nname: my-skill\ndescription: test\n42: bad\ncustom-field: bad\n---\n\nBody\n",
|
|
)
|
|
valid, msg, name = _validate_skill_frontmatter(skill_dir)
|
|
assert valid is False
|
|
assert "custom-field" in msg
|
|
assert "42" in msg
|
|
assert name is None
|
|
|
|
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_description_at_max_length_accepted(self, tmp_path):
|
|
max_desc = "a" * 1024
|
|
skill_dir = _write_skill(
|
|
tmp_path,
|
|
f"---\nname: my-skill\ndescription: {max_desc}\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_empty_description_rejected(self, tmp_path):
|
|
skill_dir = _write_skill(
|
|
tmp_path,
|
|
"---\nname: my-skill\ndescription: ''\n---\n\nBody\n",
|
|
)
|
|
valid, msg, name = _validate_skill_frontmatter(skill_dir)
|
|
assert valid is False
|
|
assert "empty" in msg.lower()
|
|
assert name is None
|
|
|
|
def test_whitespace_only_description_rejected(self, tmp_path):
|
|
skill_dir = _write_skill(
|
|
tmp_path,
|
|
"---\nname: my-skill\ndescription: ' '\n---\n\nBody\n",
|
|
)
|
|
valid, msg, name = _validate_skill_frontmatter(skill_dir)
|
|
assert valid is False
|
|
assert "empty" in msg.lower()
|
|
assert name is None
|
|
|
|
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"
|