mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-14 16:08:41 +00:00
24001e80b7
* fix(skills): accept portable frontmatter forms * fix(skills): normalize portable tool names * Safely preserve parenthesized portable skill tool patterns Portable Agent Skills declarations such as Bash(tvly *) contain spaces inside a command pattern. Keep those patterns as single literal entries while preserving exact names from the existing YAML-list form, so skill loading no longer fragments valid metadata or rewrites mixed-case MCP tools. Constraint: DeerFlow's current skill policy matches exact tool names and does not inspect Bash arguments Constraint: Agent Skills scalar syntax uses whitespace-separated entries with parenthesized command patterns Rejected: raw.split() | fragments Bash(tvly *) into unrelated tool names Rejected: normalize YAML-list entries | breaks case-sensitive MCP/runtime tool names Rejected: map Bash(...) to bash | broadens command-scoped declarations into unrestricted shell access Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep Bash(...) entries literal and inactive until DeerFlow has an explicit command-pattern authorization model Tested: 175 focused parser, validation, installer, review, loader, and tool-policy tests; Ruff check and format; compileall; git diff --check Not-tested: Full backend suite stopped at pre-existing Windows mode assertion test_runtime_config_store_file_is_owner_only Related: #4912 * Preserve exact custom tool names in portable skill parsing Portable scalar frontmatter needs alias normalization for known DeerFlow-compatible names, but generic case conversion corrupts MCP and custom tool identifiers. The tokenizer also treated quoted or escaped parentheses as structural delimiters, rejecting valid command patterns. Preserve unknown names and parse quoted or escaped patterns without broadening Bash(...) into bash. Constraint: Runtime skill policy uses exact tool-name matching Constraint: Parenthesized patterns remain literal because argument-level authorization is not implemented Rejected: Generic CamelCase-to-snake_case for every scalar | rewrites custom/MCP names Rejected: Map Bash(...) to bash | broadens command-scoped declarations into unrestricted shell access Confidence: high Scope-risk: narrow Reversibility: clean Directive: Add an explicit alias before supporting another portable tool name; keep command-pattern authorization separate Tested: 225 skills tests passed, 1 skipped; Ruff check; Ruff format --check; compileall; git diff --check Not-tested: Full backend suite remains affected by unrelated Windows permissions/path and missing Lark CLI tests Related: #4984; #4912 * Preserve case-sensitive exact tool authorities Case-folding a scalar declaration before alias lookup can turn literal write into write_file, substituting a different runtime authority. Keep exact portable spellings as aliases and preserve lowercase, custom, and MCP names; strengthen activation coverage for spaced Bash patterns and command fragments. Constraint: Runtime skill policy uses exact tool-name matching Constraint: Bash(...) remains literal and inactive because command-pattern authorization is not implemented Rejected: Case-insensitive alias lookup | maps lowercase runtime tools onto built-in authorities Rejected: Broaden the parser into command-pattern authorization | outside this PR's scope Confidence: high Scope-risk: narrow Reversibility: clean Directive: Add aliases only for documented portable spellings; preserve all other scalar names verbatim Tested: 226 skills tests passed, 1 skipped; Ruff check; Ruff format --check; compileall; git diff --check Not-tested: Full backend suite remains affected by unrelated Windows permissions/path and missing Lark CLI tests; GitNexus index refresh remains stale Related: #4984; #5016297602 * Support portable Glob and Grep skill aliases Portable Agent Skills commonly declare Glob and Grep, but DeerFlow exposes the runtime tools as glob and grep. Add explicit exact-spelling aliases and activation coverage so imported skills retain search-tool access without broad normalization. Constraint: Runtime skill policy uses exact tool-name matching Constraint: Alias conversion is limited to documented portable spellings Rejected: Case-fold all scalar names | can substitute custom or MCP authorities Rejected: Map arbitrary names by convention | breaks exact runtime compatibility Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep the alias table explicit and preserve unknown scalar names verbatim Tested: 228 skills tests passed, 1 skipped; Ruff check; Ruff format --check; compileall; git diff --check Not-tested: Full backend suite has unrelated environment failures on Windows; GitNexus index reports stale line mappings Related: #4984; #5026257899 --------- Co-authored-by: kriptoburak <kriptoburak@users.noreply.github.com>
234 lines
8.6 KiB
Python
234 lines
8.6 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_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"
|