fix(skills): require complete frontmatter delimiter lines (#6817)

This commit is contained in:
Fish 2026-10-14 07:51:50 +08:00 committed by GitHub
parent 12397d2142
commit bf40985fa8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 59 additions and 1 deletions

View File

@ -1357,6 +1357,8 @@ Discovery follows operator-managed directory symlinks, but skips links back to a
Skill Markdown and bundled text resources use UTF-8. Skill-creator CLI and review utilities read and write text explicitly as UTF-8 so localized skills behave consistently across operating systems.
`SKILL.md` YAML frontmatter must open and close with `---` on its own line (trailing spaces or tabs are allowed). A line such as `---suffix` is not a closing delimiter.
Custom skill history preserves Unicode line separators inside saved content and metadata, keeping those revisions readable for history inspection and rollback. Malformed JSON history records are still rejected.
The bundled `github-deep-research` API client also works without `requests`, using

View File

@ -1,5 +1,7 @@
### Skills System (`packages/harness/deerflow/skills/`)
- **Frontmatter boundaries**: `frontmatter.py` owns standalone `---` delimiter lines for runtime parsing, installation validation, and review. Accept trailing spaces/tabs, LF/CRLF, a leading UTF-8 BOM, and a closing delimiter at EOF. Preserve body whitespace; prefix-only lines such as `---suffix` cannot close frontmatter. Regression coverage lives in `tests/test_skill_frontmatter_boundaries.py`.
- **Management YAML diagnostics**: `parse_skill_file(..., diagnostics=...)` optionally collects `SkillLoadDiagnostic` values separately from valid `Skill` entries. Only `UserScopedSkillStorage.load_custom_skill_diagnostics()` supplies that sink, after validating root ancestors and each resolved file against the caller's custom root. Exclude legacy/shared locations and external symlinks; do not weaken runtime discovery's separate compatibility policy. `GET /api/skills/diagnostics/custom` is admin-only and returns stable codes plus package-relative identifiers and 1-based file positions. Never return `_format_yaml_error`, exceptions or source excerpts. The management UI posts the existing admin-only reload before refetching diagnostics and normal skills; active Agent snapshots are unchanged.
- **Diagnostic scope**: only `yaml.YAMLError` produces a management diagnostic. Missing frontmatter, non-mapping or invalid metadata, non-UTF-8 files and read errors (including permission denied) remain excluded; the UI and README state those limits. An empty diagnostic list is not a package-validity check. `tests/test_skill_load_diagnostics.py` pins these exclusions and permission-error redaction without changing runtime parsing.
- **History JSONL**: split records on LF, not Unicode text separators; preserve U+0085/U+2028/U+2029 in saved fields. Keep malformed JSON rejected. Tests: `tests/test_skill_history_jsonl.py`.

View File

@ -29,7 +29,9 @@ ALLOWED_FRONTMATTER_PROPERTIES = {
# `\ufeff` is the UTF-8 byte-order mark that Windows Notepad and PowerShell's
# `Set-Content -Encoding UTF8` prepend to a saved file. It is consumed by the match, so
# neither the front matter nor the body carries it downstream.
_FRONTMATTER_RE = re.compile(r"^\ufeff?---\s*\n(.*?)\n---\s*\n?", re.DOTALL)
# Delimiter whitespace must stay on its own line; do not consume body
# indentation or accept a closing line that merely starts with three hyphens.
_FRONTMATTER_RE = re.compile(r"^\ufeff?---[ \t]*\r?\n(.*?)\r?\n---[ \t]*(?:\r?\n|$)", re.DOTALL)
@dataclass(frozen=True)

View File

@ -0,0 +1,52 @@
"""Regression coverage for complete SKILL.md delimiter lines."""
from pathlib import Path
import pytest
from deerflow.skills.frontmatter import split_skill_markdown
from deerflow.skills.parser import parse_skill_file
from deerflow.skills.validation import validate_skill_frontmatter_text
@pytest.mark.parametrize("closing", ["---suffix", "----", "--- body text"])
def test_non_delimiter_line_cannot_close_frontmatter(tmp_path: Path, closing: str) -> None:
content = f"---\nname: my-skill\ndescription: A valid skill\n{closing}\nInstructions\n"
skill_file = tmp_path / "SKILL.md"
skill_file.write_text(content, encoding="utf-8")
parts, error = split_skill_markdown(content)
assert parts is None
assert error == "No YAML frontmatter found"
assert parse_skill_file(skill_file, category="custom") is None
valid, _, name = validate_skill_frontmatter_text(content)
assert not valid
assert name is None
@pytest.mark.parametrize("newline", ["\n", "\r\n"])
@pytest.mark.parametrize("bom", ["", "\ufeff"])
def test_delimiter_preserves_body_whitespace(newline: str, bom: str) -> None:
body = f"{newline} print('keep indentation'){newline}{newline}Instructions{newline}"
content = f"{bom}--- \t{newline}name: my-skill{newline}description: A valid skill{newline}--- \t{newline}{body}"
parts, error = split_skill_markdown(content)
assert error is None
assert parts is not None
assert parts.metadata == {"name": "my-skill", "description": "A valid skill"}
assert parts.body == body
assert validate_skill_frontmatter_text(content) == (True, "Skill is valid!", "my-skill")
@pytest.mark.parametrize("newline", ["\n", "\r\n"])
def test_closing_delimiter_at_end_of_file(newline: str) -> None:
content = f"---{newline}name: my-skill{newline}description: A valid skill{newline}--- \t"
parts, error = split_skill_markdown(content)
assert error is None
assert parts is not None
assert parts.metadata["name"] == "my-skill"
assert parts.body == ""