diff --git a/README.md b/README.md index 2ea27c2f3f..7aa07d1b51 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/backend/packages/harness/deerflow/skills/AGENTS.md b/backend/packages/harness/deerflow/skills/AGENTS.md index a8a3b78a45..8048b45b9b 100644 --- a/backend/packages/harness/deerflow/skills/AGENTS.md +++ b/backend/packages/harness/deerflow/skills/AGENTS.md @@ -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`. diff --git a/backend/packages/harness/deerflow/skills/frontmatter.py b/backend/packages/harness/deerflow/skills/frontmatter.py index b1187a2616..b4ff2b86e4 100644 --- a/backend/packages/harness/deerflow/skills/frontmatter.py +++ b/backend/packages/harness/deerflow/skills/frontmatter.py @@ -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) diff --git a/backend/tests/test_skill_frontmatter_boundaries.py b/backend/tests/test_skill_frontmatter_boundaries.py new file mode 100644 index 0000000000..828e592fdf --- /dev/null +++ b/backend/tests/test_skill_frontmatter_boundaries.py @@ -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 == ""