From 159b774944e5abb37981da051b369cc06481cc5d Mon Sep 17 00:00:00 2001 From: Daoyuan Li <94409450+DaoyuanLi2816@users.noreply.github.com> Date: Fri, 24 Jul 2026 06:25:53 -0700 Subject: [PATCH] fix(skills): handle non-string frontmatter keys (#4167) Normalize YAML frontmatter keys in the shared parser so validation and review report malformed fields instead of failing while sorting mixed key types. --- .../harness/deerflow/skills/frontmatter.py | 4 +++ backend/tests/test_skill_review_core.py | 30 +++++++++++++++++++ backend/tests/test_skills_validation.py | 11 +++++++ 3 files changed, 45 insertions(+) diff --git a/backend/packages/harness/deerflow/skills/frontmatter.py b/backend/packages/harness/deerflow/skills/frontmatter.py index 779bd84b7..f327d8fab 100644 --- a/backend/packages/harness/deerflow/skills/frontmatter.py +++ b/backend/packages/harness/deerflow/skills/frontmatter.py @@ -57,6 +57,10 @@ def split_skill_markdown(content: str) -> tuple[SkillMarkdownParts | None, str | if not isinstance(metadata, dict): return None, "Frontmatter must be a YAML dictionary" + # YAML permits non-string keys, but downstream validation expects field + # names to be strings. + metadata = {str(key): value for key, value in metadata.items()} + return ( SkillMarkdownParts( metadata=metadata, diff --git a/backend/tests/test_skill_review_core.py b/backend/tests/test_skill_review_core.py index 849f386a8..179794d56 100644 --- a/backend/tests/test_skill_review_core.py +++ b/backend/tests/test_skill_review_core.py @@ -56,6 +56,22 @@ def test_review_core_reports_missing_description_blocker(tmp_path): assert any(f["rule_id"] == "structure.missing-description" for f in facts["findings"]) +def test_review_core_reports_non_string_frontmatter_key_as_unknown_field(tmp_path): + _write( + tmp_path / "SKILL.md", + "---\nname: demo-skill\ndescription: Demo skill. Invoke when testing review.\n42: stray-value\nunexpected-field: another-value\n---\n\n# Demo\n\nFollow the steps and stop.\n", + ) + + facts = analyze_skill_package(LocalDirectoryReader(tmp_path).read()) + + assert facts["summary"]["blockers"] == 0 + finding = next(f for f in facts["findings"] if f["rule_id"] == "structure.unknown-frontmatter-field") + assert finding["severity"] == "warning" + assert finding["evidence"] == ["42", "unexpected-field"] + assert "42" in finding["message"] + assert "unexpected-field" in finding["message"] + + def test_resource_graph_reports_unreferenced_resource(tmp_path): _write(tmp_path / "SKILL.md", _valid_skill()) _write(tmp_path / "references" / "unused.md", "# Unused\n") @@ -277,6 +293,20 @@ def test_cli_fail_on_error(tmp_path, capsys): assert "structure.missing-description" in output +def test_cli_reports_non_string_frontmatter_key_without_crashing(tmp_path, capsys): + _write( + tmp_path / "SKILL.md", + "---\nname: demo-skill\ndescription: Demo skill. Invoke when testing review.\n42: stray-value\nunexpected-field: another-value\n---\n\n# Demo\n\nFollow the steps and stop.\n", + ) + + exit_code = review_cli_main([str(tmp_path), "--format", "text", "--fail-on", "error", "--fail-on-incomplete"]) + output = capsys.readouterr().out + + assert exit_code == 0 + assert "structure.unknown-frontmatter-field" in output + assert "Unknown frontmatter field(s): 42, unexpected-field" in output + + def test_cli_fail_on_incomplete_package(tmp_path, capsys): _write(tmp_path / "SKILL.md", _valid_skill()) _write(tmp_path / "references" / "large.md", "x" * 32) diff --git a/backend/tests/test_skills_validation.py b/backend/tests/test_skills_validation.py index 705b824d4..50620d38d 100644 --- a/backend/tests/test_skills_validation.py +++ b/backend/tests/test_skills_validation.py @@ -116,6 +116,17 @@ class TestValidateSkillFrontmatter: 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,