From e7e2dd1133cd3061e8f89ad2f0f94c07804361fb Mon Sep 17 00:00:00 2001 From: Haoran Date: Sat, 27 Jun 2026 04:36:40 +0800 Subject: [PATCH 1/2] Fix s07 skill frontmatter parsing --- s07_skill_loading/README.en.md | 4 ++-- s07_skill_loading/README.ja.md | 4 ++-- s07_skill_loading/README.md | 4 ++-- s07_skill_loading/code.py | 26 ++++++++++++++++++-------- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/s07_skill_loading/README.en.md b/s07_skill_loading/README.en.md index 35399ddba..115462c74 100644 --- a/s07_skill_loading/README.en.md +++ b/s07_skill_loading/README.en.md @@ -70,8 +70,8 @@ def _scan_skills(): if manifest.exists(): raw = manifest.read_text() meta, body = _parse_frontmatter(raw) - name = meta.get("name", d.name) - desc = meta.get("description", raw.split("\n")[0].lstrip("#").strip()) + name = meta.get("name") or d.name + desc = meta.get("description") or body.split("\n", 1)[0].lstrip("#").strip() SKILL_REGISTRY[name] = {"name": name, "description": desc, "content": raw} _scan_skills() # runs once at startup diff --git a/s07_skill_loading/README.ja.md b/s07_skill_loading/README.ja.md index 7e12baa86..f3c1f19e2 100644 --- a/s07_skill_loading/README.ja.md +++ b/s07_skill_loading/README.ja.md @@ -70,8 +70,8 @@ def _scan_skills(): if manifest.exists(): raw = manifest.read_text() meta, body = _parse_frontmatter(raw) - name = meta.get("name", d.name) - desc = meta.get("description", raw.split("\n")[0].lstrip("#").strip()) + name = meta.get("name") or d.name + desc = meta.get("description") or body.split("\n", 1)[0].lstrip("#").strip() SKILL_REGISTRY[name] = {"name": name, "description": desc, "content": raw} _scan_skills() # runs once at startup diff --git a/s07_skill_loading/README.md b/s07_skill_loading/README.md index 8b53c7644..db6a72de8 100644 --- a/s07_skill_loading/README.md +++ b/s07_skill_loading/README.md @@ -70,8 +70,8 @@ def _scan_skills(): if manifest.exists(): raw = manifest.read_text() meta, body = _parse_frontmatter(raw) - name = meta.get("name", d.name) - desc = meta.get("description", raw.split("\n")[0].lstrip("#").strip()) + name = meta.get("name") or d.name + desc = meta.get("description") or body.split("\n", 1)[0].lstrip("#").strip() SKILL_REGISTRY[name] = {"name": name, "description": desc, "content": raw} _scan_skills() # runs once at startup diff --git a/s07_skill_loading/code.py b/s07_skill_loading/code.py index b4506fc57..28b2af467 100644 --- a/s07_skill_loading/code.py +++ b/s07_skill_loading/code.py @@ -52,16 +52,26 @@ # s07: Skill catalog scan (used by build_system below) def _parse_frontmatter(text: str) -> tuple[dict, str]: """Parse YAML frontmatter from SKILL.md. Returns (meta, body).""" - if not text.startswith("---"): + if not (text.startswith("---\n") or text.startswith("---\r\n")): return {}, text - parts = text.split("---", 2) - if len(parts) < 3: + + lines = text.splitlines(keepends=True) + closing_index = None + for index, line in enumerate(lines[1:], start=1): + if line.strip() == "---": + closing_index = index + break + if closing_index is None: return {}, text + + frontmatter = "".join(lines[1:closing_index]) + body = "".join(lines[closing_index + 1 :]).strip() try: - meta = yaml.safe_load(parts[1]) or {} + loaded = yaml.safe_load(frontmatter) or {} except yaml.YAMLError: - meta = {} - return meta, parts[2].strip() + loaded = {} + meta = loaded if isinstance(loaded, dict) else {} + return meta, body # Build skill registry at startup (used for safe lookup in load_skill) SKILL_REGISTRY: dict[str, dict] = {} @@ -77,8 +87,8 @@ def _scan_skills(): if manifest.exists(): raw = manifest.read_text() meta, body = _parse_frontmatter(raw) - name = meta.get("name", d.name) - desc = meta.get("description", raw.split("\n")[0].lstrip("#").strip()) + name = meta.get("name") or d.name + desc = meta.get("description") or body.split("\n", 1)[0].lstrip("#").strip() SKILL_REGISTRY[name] = {"name": name, "description": desc, "content": raw} _scan_skills() From b1cac56f6e7ddcf5d3ccca0bbb9b4a5185193b40 Mon Sep 17 00:00:00 2001 From: Haoran Date: Sun, 28 Jun 2026 16:38:05 +0800 Subject: [PATCH 2/2] Sync skill frontmatter parsing across later chapters --- s08_context_compact/code.py | 35 +++++--- s20_comprehensive/code.py | 29 +++++-- tests/test_skill_frontmatter_parsing.py | 106 ++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 21 deletions(-) create mode 100644 tests/test_skill_frontmatter_parsing.py diff --git a/s08_context_compact/code.py b/s08_context_compact/code.py index 7186df554..897355ec5 100644 --- a/s08_context_compact/code.py +++ b/s08_context_compact/code.py @@ -29,11 +29,12 @@ Builds on s07 (skill loading). Usage: python s08_context_compact/code.py - Needs: pip install anthropic python-dotenv + ANTHROPIC_API_KEY in .env + Needs: pip install anthropic python-dotenv pyyaml + ANTHROPIC_API_KEY in .env """ import ast, json, os, subprocess, time from pathlib import Path +import yaml try: import readline @@ -57,17 +58,27 @@ # s07: Skill catalog scan (inherited from s07) def _parse_frontmatter(text: str) -> tuple[dict, str]: - if not text.startswith("---"): + """Parse YAML frontmatter from SKILL.md. Returns (meta, body).""" + if not (text.startswith("---\n") or text.startswith("---\r\n")): return {}, text - parts = text.split("---", 2) - if len(parts) < 3: + + lines = text.splitlines(keepends=True) + closing_index = None + for index, line in enumerate(lines[1:], start=1): + if line.strip() == "---": + closing_index = index + break + if closing_index is None: return {}, text - meta = {} - for line in parts[1].strip().splitlines(): - if ":" in line: - k, v = line.split(":", 1) - meta[k.strip()] = v.strip().strip('"').strip("'") - return meta, parts[2].strip() + + frontmatter = "".join(lines[1:closing_index]) + body = "".join(lines[closing_index + 1 :]).strip() + try: + loaded = yaml.safe_load(frontmatter) or {} + except yaml.YAMLError: + loaded = {} + meta = loaded if isinstance(loaded, dict) else {} + return meta, body SKILL_REGISTRY: dict[str, dict] = {} @@ -81,8 +92,8 @@ def _scan_skills(): if manifest.exists(): raw = manifest.read_text() meta, body = _parse_frontmatter(raw) - name = meta.get("name", d.name) - desc = meta.get("description", raw.split("\n")[0].lstrip("#").strip()) + name = meta.get("name") or d.name + desc = meta.get("description") or body.split("\n", 1)[0].lstrip("#").strip() SKILL_REGISTRY[name] = {"name": name, "description": desc, "content": raw} _scan_skills() diff --git a/s20_comprehensive/code.py b/s20_comprehensive/code.py index 722aba153..62810ea79 100644 --- a/s20_comprehensive/code.py +++ b/s20_comprehensive/code.py @@ -288,16 +288,27 @@ def keep_worktree(name: str) -> str: def _parse_frontmatter(text: str) -> tuple[dict, str]: - if not text.startswith("---"): + """Parse YAML frontmatter from SKILL.md. Returns (meta, body).""" + if not (text.startswith("---\n") or text.startswith("---\r\n")): return {}, text - parts = text.split("---", 2) - if len(parts) < 3: + + lines = text.splitlines(keepends=True) + closing_index = None + for index, line in enumerate(lines[1:], start=1): + if line.strip() == "---": + closing_index = index + break + if closing_index is None: return {}, text + + frontmatter = "".join(lines[1:closing_index]) + body = "".join(lines[closing_index + 1 :]).strip() try: - meta = yaml.safe_load(parts[1]) or {} + loaded = yaml.safe_load(frontmatter) or {} except yaml.YAMLError: - meta = {} - return meta, parts[2].strip() + loaded = {} + meta = loaded if isinstance(loaded, dict) else {} + return meta, body def scan_skills(): @@ -311,9 +322,9 @@ def scan_skills(): if not manifest.exists(): continue raw = manifest.read_text() - meta, _ = _parse_frontmatter(raw) - name = meta.get("name", directory.name) - desc = meta.get("description", raw.split("\n")[0].lstrip("#").strip()) + meta, body = _parse_frontmatter(raw) + name = meta.get("name") or directory.name + desc = meta.get("description") or body.split("\n", 1)[0].lstrip("#").strip() SKILL_REGISTRY[name] = { "name": name, "description": desc, diff --git a/tests/test_skill_frontmatter_parsing.py b/tests/test_skill_frontmatter_parsing.py new file mode 100644 index 000000000..691c4b806 --- /dev/null +++ b/tests/test_skill_frontmatter_parsing.py @@ -0,0 +1,106 @@ +import importlib.util +import os +import sys +import tempfile +import types +import unittest +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] +SKILL_MODULES = [ + ("s07", REPO_ROOT / "s07_skill_loading" / "code.py", "_scan_skills"), + ("s08", REPO_ROOT / "s08_context_compact" / "code.py", "_scan_skills"), + ("s20", REPO_ROOT / "s20_comprehensive" / "code.py", "scan_skills"), +] + + +def load_skill_module(module_name: str, module_path: Path, temp_cwd: Path): + fake_anthropic = types.ModuleType("anthropic") + + class FakeAnthropic: + def __init__(self, *args, **kwargs): + self.messages = types.SimpleNamespace(create=None) + + fake_dotenv = types.ModuleType("dotenv") + setattr(fake_anthropic, "Anthropic", FakeAnthropic) + setattr(fake_dotenv, "load_dotenv", lambda override=True: None) + + previous_modules = { + "anthropic": sys.modules.get("anthropic"), + "dotenv": sys.modules.get("dotenv"), + } + previous_cwd = Path.cwd() + previous_model_id = os.environ.get("MODEL_ID") + + spec = importlib.util.spec_from_file_location( + f"{module_name}_frontmatter_test", module_path) + if spec is None or spec.loader is None: + raise RuntimeError(f"Unable to load {module_path}") + module = importlib.util.module_from_spec(spec) + + sys.modules["anthropic"] = fake_anthropic + sys.modules["dotenv"] = fake_dotenv + try: + os.chdir(temp_cwd) + os.environ["MODEL_ID"] = "test-model" + spec.loader.exec_module(module) + return module + finally: + os.chdir(previous_cwd) + if previous_model_id is None: + os.environ.pop("MODEL_ID", None) + else: + os.environ["MODEL_ID"] = previous_model_id + for name, previous in previous_modules.items(): + if previous is None: + sys.modules.pop(name, None) + else: + sys.modules[name] = previous + + +class SkillFrontmatterParsingTests(unittest.TestCase): + def test_scan_skills_falls_back_for_empty_metadata_values(self): + raw = "---\nname:\ndescription:\n---\n# Body description\n\nDetails" + for module_name, module_path, scan_name in SKILL_MODULES: + with self.subTest(module=module_name), tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + skill_dir = tmp_path / "skills" / "fallback-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text(raw) + + module = load_skill_module(module_name, module_path, tmp_path) + module.SKILL_REGISTRY.clear() + getattr(module, scan_name)() + + self.assertIn("fallback-skill", module.SKILL_REGISTRY) + self.assertEqual( + module.SKILL_REGISTRY["fallback-skill"]["description"], + "Body description", + ) + + def test_parse_frontmatter_treats_non_mapping_yaml_as_empty_meta(self): + raw = "---\n- not\n- a\n- mapping\n---\nBody" + for module_name, module_path, _ in SKILL_MODULES: + with self.subTest(module=module_name), tempfile.TemporaryDirectory() as tmp: + module = load_skill_module(module_name, module_path, Path(tmp)) + + meta, body = module._parse_frontmatter(raw) + + self.assertEqual(meta, {}) + self.assertEqual(body, "Body") + + def test_parse_frontmatter_requires_opening_delimiter_on_own_line(self): + raw = "---not frontmatter\n---\n# Body" + for module_name, module_path, _ in SKILL_MODULES: + with self.subTest(module=module_name), tempfile.TemporaryDirectory() as tmp: + module = load_skill_module(module_name, module_path, Path(tmp)) + + meta, body = module._parse_frontmatter(raw) + + self.assertEqual(meta, {}) + self.assertEqual(body, raw) + + +if __name__ == "__main__": + unittest.main()