diff --git a/nanobot/agent/skills.py b/nanobot/agent/skills.py index 5d18cce5..b01ca74e 100644 --- a/nanobot/agent/skills.py +++ b/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 diff --git a/pyproject.toml b/pyproject.toml index 54f82be7..57d7826b 100644 --- a/pyproject.toml +++ b/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", diff --git a/tests/agent/test_skills_loader.py b/tests/agent/test_skills_loader.py index 4284fa0c..d9cfa6d1 100644 --- a/tests/agent/test_skills_loader.py +++ b/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)