Merge PR #3141: fix(skills): use yaml.safe_load for frontmatter parsing to handle multiline descriptions

fix(skills): use yaml.safe_load for frontmatter parsing to handle multiline descriptions

nanobot/agent/skills.py

@@ -6,6 +6,8 @@ import re
 import shutil
 from pathlib import Path
 
+import yaml
+
 # Default builtin skills directory (relative to this file)
 BUILTIN_SKILLS_DIR = Path(__file__).parent.parent / "skills"
 
@@ -165,11 +167,19 @@ class SkillsLoader:
             return content[match.end():].strip()
         return content
 
-    def _parse_nanobot_metadata(self, raw: str) -> dict:
-        """Parse skill metadata JSON from frontmatter (supports nanobot and openclaw keys)."""
-        try:
-            data = json.loads(raw)
-        except (json.JSONDecodeError, TypeError):
+    def _parse_nanobot_metadata(self, raw: object) -> dict:
+        """Extract nanobot/openclaw metadata from a frontmatter field.
+
+        ``raw`` may be a dict (already parsed by yaml.safe_load) or a JSON str.
+        """
+        if isinstance(raw, dict):
+            data = raw
+        elif isinstance(raw, str):
+            try:
+                data = json.loads(raw)
+            except (json.JSONDecodeError, TypeError):
+                return {}
+        else:
             return {}
         if not isinstance(data, dict):
             return {}
@@ -187,8 +197,8 @@ class SkillsLoader:
 
     def _get_skill_meta(self, name: str) -> dict:
         """Get nanobot metadata for a skill (cached in frontmatter)."""
-        meta = self.get_skill_metadata(name) or {}
-        return self._parse_nanobot_metadata(meta.get("metadata", ""))
+        raw_meta = self.get_skill_metadata(name) or {}
+        return self._parse_nanobot_metadata(raw_meta.get("metadata"))
 
     def get_always_skills(self) -> list[str]:
         """Get skills marked as always=true that meet requirements."""
@@ -197,7 +207,7 @@ class SkillsLoader:
             for entry in self.list_skills(filter_unavailable=True)
             if (meta := self.get_skill_metadata(entry["name"]) or {})
             and (
-                self._parse_nanobot_metadata(meta.get("metadata", "")).get("always")
+                self._parse_nanobot_metadata(meta.get("metadata")).get("always")
                 or meta.get("always")
             )
         ]
@@ -218,10 +228,15 @@ class SkillsLoader:
         match = _STRIP_SKILL_FRONTMATTER.match(content)
         if not match:
             return None
-        metadata: dict[str, str] = {}
-        for line in match.group(1).splitlines():
-            if ":" not in line:
-                continue
-            key, value = line.split(":", 1)
-            metadata[key.strip()] = value.strip().strip('"\'')
+        try:
+            parsed = yaml.safe_load(match.group(1))
+        except yaml.YAMLError:
+            return None
+        if not isinstance(parsed, dict):
+            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

pyproject.toml

@@ -50,6 +50,7 @@ dependencies = [
     "tiktoken>=0.12.0,<1.0.0",
     "jinja2>=3.1.0,<4.0.0",
     "dulwich>=0.22.0,<1.0.0",
+    "pyyaml>=6.0,<7.0.0",
     "pypdf>=5.0.0,<6.0.0",
     "python-docx>=1.1.0,<2.0.0",
     "openpyxl>=3.1.0,<4.0.0",

tests/agent/test_skills_loader.py

@@ -310,3 +310,90 @@ def test_disabled_skills_excluded_from_get_always_skills(tmp_path: Path) -> None
     always = loader.get_always_skills()
     assert "alpha" not 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)