fix(skills): use yaml.safe_load for frontmatter parsing to handle multiline descriptions
The hand-rolled line-by-line YAML parser treated each line independently, so YAML multiline scalars (folded `>` and literal `|`) were captured as the literal characters ">" or "|" instead of the actual text content.
This commit is contained in:
+29
-14
@@ -6,6 +6,8 @@ import re
|
|||||||
import shutil
|
import shutil
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
import yaml
|
||||||
|
|
||||||
# Default builtin skills directory (relative to this file)
|
# Default builtin skills directory (relative to this file)
|
||||||
BUILTIN_SKILLS_DIR = Path(__file__).parent.parent / "skills"
|
BUILTIN_SKILLS_DIR = Path(__file__).parent.parent / "skills"
|
||||||
|
|
||||||
@@ -171,11 +173,19 @@ class SkillsLoader:
|
|||||||
return content[match.end():].strip()
|
return content[match.end():].strip()
|
||||||
return content
|
return content
|
||||||
|
|
||||||
def _parse_nanobot_metadata(self, raw: str) -> dict:
|
def _parse_nanobot_metadata(self, raw: object) -> dict:
|
||||||
"""Parse skill metadata JSON from frontmatter (supports nanobot and openclaw keys)."""
|
"""Extract nanobot/openclaw metadata from a frontmatter field.
|
||||||
try:
|
|
||||||
data = json.loads(raw)
|
``raw`` may be a dict (already parsed by yaml.safe_load) or a JSON str.
|
||||||
except (json.JSONDecodeError, TypeError):
|
"""
|
||||||
|
if isinstance(raw, dict):
|
||||||
|
data = raw
|
||||||
|
elif isinstance(raw, str):
|
||||||
|
try:
|
||||||
|
data = json.loads(raw)
|
||||||
|
except (json.JSONDecodeError, TypeError):
|
||||||
|
return {}
|
||||||
|
else:
|
||||||
return {}
|
return {}
|
||||||
if not isinstance(data, dict):
|
if not isinstance(data, dict):
|
||||||
return {}
|
return {}
|
||||||
@@ -193,8 +203,8 @@ class SkillsLoader:
|
|||||||
|
|
||||||
def _get_skill_meta(self, name: str) -> dict:
|
def _get_skill_meta(self, name: str) -> dict:
|
||||||
"""Get nanobot metadata for a skill (cached in frontmatter)."""
|
"""Get nanobot metadata for a skill (cached in frontmatter)."""
|
||||||
meta = self.get_skill_metadata(name) or {}
|
raw_meta = self.get_skill_metadata(name) or {}
|
||||||
return self._parse_nanobot_metadata(meta.get("metadata", ""))
|
return self._parse_nanobot_metadata(raw_meta.get("metadata"))
|
||||||
|
|
||||||
def get_always_skills(self) -> list[str]:
|
def get_always_skills(self) -> list[str]:
|
||||||
"""Get skills marked as always=true that meet requirements."""
|
"""Get skills marked as always=true that meet requirements."""
|
||||||
@@ -203,7 +213,7 @@ class SkillsLoader:
|
|||||||
for entry in self.list_skills(filter_unavailable=True)
|
for entry in self.list_skills(filter_unavailable=True)
|
||||||
if (meta := self.get_skill_metadata(entry["name"]) or {})
|
if (meta := self.get_skill_metadata(entry["name"]) or {})
|
||||||
and (
|
and (
|
||||||
self._parse_nanobot_metadata(meta.get("metadata", "")).get("always")
|
self._parse_nanobot_metadata(meta.get("metadata")).get("always")
|
||||||
or meta.get("always")
|
or meta.get("always")
|
||||||
)
|
)
|
||||||
]
|
]
|
||||||
@@ -224,10 +234,15 @@ class SkillsLoader:
|
|||||||
match = _STRIP_SKILL_FRONTMATTER.match(content)
|
match = _STRIP_SKILL_FRONTMATTER.match(content)
|
||||||
if not match:
|
if not match:
|
||||||
return None
|
return None
|
||||||
metadata: dict[str, str] = {}
|
try:
|
||||||
for line in match.group(1).splitlines():
|
parsed = yaml.safe_load(match.group(1))
|
||||||
if ":" not in line:
|
except yaml.YAMLError:
|
||||||
continue
|
return None
|
||||||
key, value = line.split(":", 1)
|
if not isinstance(parsed, dict):
|
||||||
metadata[key.strip()] = value.strip().strip('"\'')
|
return None
|
||||||
|
# yaml.safe_load returns native types (int, bool, list, etc.);
|
||||||
|
# keep values as-is so downstream consumers get correct types.
|
||||||
|
metadata: dict[str, object] = {}
|
||||||
|
for key, value in parsed.items():
|
||||||
|
metadata[str(key)] = value
|
||||||
return metadata
|
return metadata
|
||||||
|
|||||||
@@ -50,6 +50,7 @@ dependencies = [
|
|||||||
"tiktoken>=0.12.0,<1.0.0",
|
"tiktoken>=0.12.0,<1.0.0",
|
||||||
"jinja2>=3.1.0,<4.0.0",
|
"jinja2>=3.1.0,<4.0.0",
|
||||||
"dulwich>=0.22.0,<1.0.0",
|
"dulwich>=0.22.0,<1.0.0",
|
||||||
|
"pyyaml>=6.0,<7.0.0",
|
||||||
"filelock>=3.25.2",
|
"filelock>=3.25.2",
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|||||||
@@ -310,3 +310,90 @@ def test_disabled_skills_excluded_from_get_always_skills(tmp_path: Path) -> None
|
|||||||
always = loader.get_always_skills()
|
always = loader.get_always_skills()
|
||||||
assert "alpha" not in always
|
assert "alpha" not in always
|
||||||
assert "beta" in always
|
assert "beta" in always
|
||||||
|
|
||||||
|
|
||||||
|
# -- multiline description tests (YAML folded > and literal |) -----------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_skills_summary_folded_description(tmp_path: Path) -> None:
|
||||||
|
"""description: > (YAML folded scalar) should be parsed correctly."""
|
||||||
|
workspace = tmp_path / "ws"
|
||||||
|
ws_skills = workspace / "skills"
|
||||||
|
ws_skills.mkdir(parents=True)
|
||||||
|
skill_dir = ws_skills / "pdf"
|
||||||
|
skill_dir.mkdir(parents=True)
|
||||||
|
skill_path = skill_dir / "SKILL.md"
|
||||||
|
skill_path.write_text(
|
||||||
|
"---\n"
|
||||||
|
"name: pdf\n"
|
||||||
|
"description: >\n"
|
||||||
|
" Use this skill when visual quality and design identity matter for a PDF.\n"
|
||||||
|
" CREATE (generate from scratch): \"make a PDF\".\n"
|
||||||
|
"---\n\n# PDF Skill\n",
|
||||||
|
encoding="utf-8",
|
||||||
|
)
|
||||||
|
builtin = tmp_path / "builtin"
|
||||||
|
builtin.mkdir()
|
||||||
|
|
||||||
|
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||||
|
summary = loader.build_skills_summary()
|
||||||
|
assert "pdf" in summary
|
||||||
|
assert "visual quality" in summary
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_skills_summary_literal_description(tmp_path: Path) -> None:
|
||||||
|
"""description: | (YAML literal scalar) should be parsed correctly."""
|
||||||
|
workspace = tmp_path / "ws"
|
||||||
|
ws_skills = workspace / "skills"
|
||||||
|
ws_skills.mkdir(parents=True)
|
||||||
|
skill_dir = ws_skills / "multi"
|
||||||
|
skill_dir.mkdir(parents=True)
|
||||||
|
skill_path = skill_dir / "SKILL.md"
|
||||||
|
skill_path.write_text(
|
||||||
|
"---\n"
|
||||||
|
"name: multi\n"
|
||||||
|
"description: |\n"
|
||||||
|
" Line one of description.\n"
|
||||||
|
" Line two of description.\n"
|
||||||
|
"---\n\n# Multi\n",
|
||||||
|
encoding="utf-8",
|
||||||
|
)
|
||||||
|
builtin = tmp_path / "builtin"
|
||||||
|
builtin.mkdir()
|
||||||
|
|
||||||
|
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||||
|
meta = loader.get_skill_metadata("multi")
|
||||||
|
assert meta is not None
|
||||||
|
desc = meta.get("description")
|
||||||
|
assert isinstance(desc, str)
|
||||||
|
assert "Line one" in desc
|
||||||
|
assert "Line two" in desc
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_skill_metadata_handles_yaml_types(tmp_path: Path) -> None:
|
||||||
|
"""yaml.safe_load returns native types; always should be True, not 'true'."""
|
||||||
|
workspace = tmp_path / "ws"
|
||||||
|
ws_skills = workspace / "skills"
|
||||||
|
ws_skills.mkdir(parents=True)
|
||||||
|
skill_dir = ws_skills / "typed"
|
||||||
|
skill_dir.mkdir(parents=True)
|
||||||
|
payload = json.dumps({"nanobot": {"requires": {"bins": ["gh"]}, "always": True}}, separators=(",", ":"))
|
||||||
|
skill_path = skill_dir / "SKILL.md"
|
||||||
|
skill_path.write_text(
|
||||||
|
"---\n"
|
||||||
|
"name: typed\n"
|
||||||
|
f"metadata: {payload}\n"
|
||||||
|
"always: true\n"
|
||||||
|
"---\n\n# Typed\n",
|
||||||
|
encoding="utf-8",
|
||||||
|
)
|
||||||
|
builtin = tmp_path / "builtin"
|
||||||
|
builtin.mkdir()
|
||||||
|
|
||||||
|
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||||
|
meta = loader.get_skill_metadata("typed")
|
||||||
|
assert meta is not None
|
||||||
|
# YAML parsed 'true' to Python True
|
||||||
|
assert meta.get("always") is True
|
||||||
|
# metadata is a parsed dict, not a JSON string
|
||||||
|
assert isinstance(meta.get("metadata"), dict)
|
||||||
|
|||||||
Reference in New Issue
Block a user