Fix skill frontmatter parsing

s07_skill_loading/README.ja.md

@@ -58,11 +58,20 @@ skills/
 class SkillLoader:
     def scan(self):
         self.skills.clear()
+        skills_root = self.skills_dir.resolve()
         for manifest in sorted(self.skills_dir.glob("*/SKILL.md")):
+            if (not manifest.is_file()
+                    or not manifest.resolve().is_relative_to(skills_root)):
+                continue
             content = manifest.read_text()
             metadata, body = self.parse_frontmatter(content)
-            name = str(metadata.get("name") or manifest.parent.name).strip()
-            description = metadata.get("description") or body.splitlines()[0]
+            raw_name = metadata.get("name")
+            name = raw_name.strip() if isinstance(raw_name, str) else ""
+            name = name or manifest.parent.name
+            raw_description = metadata.get("description")
+            description = (raw_description.strip()
+                           if isinstance(raw_description, str) else "")
+            description = description or body.split("\n", 1)[0]
             description = " ".join(str(description).lstrip("# ").split())
             self.skills[name] = {
                 "name": name,

s07_skill_loading/README.md

@@ -58,11 +58,20 @@ skills/
 class SkillLoader:
     def scan(self):
         self.skills.clear()
+        skills_root = self.skills_dir.resolve()
         for manifest in sorted(self.skills_dir.glob("*/SKILL.md")):
+            if (not manifest.is_file()
+                    or not manifest.resolve().is_relative_to(skills_root)):
+                continue
             content = manifest.read_text()
             metadata, body = self.parse_frontmatter(content)
-            name = str(metadata.get("name") or manifest.parent.name).strip()
-            description = metadata.get("description") or body.splitlines()[0]
+            raw_name = metadata.get("name")
+            name = raw_name.strip() if isinstance(raw_name, str) else ""
+            name = name or manifest.parent.name
+            raw_description = metadata.get("description")
+            description = (raw_description.strip()
+                           if isinstance(raw_description, str) else "")
+            description = description or body.split("\n", 1)[0]
             description = " ".join(str(description).lstrip("# ").split())
             self.skills[name] = {
                 "name": name,

s07_skill_loading/README.zh.md

@@ -58,11 +58,20 @@ skills/
 class SkillLoader:
     def scan(self):
         self.skills.clear()
+        skills_root = self.skills_dir.resolve()
         for manifest in sorted(self.skills_dir.glob("*/SKILL.md")):
+            if (not manifest.is_file()
+                    or not manifest.resolve().is_relative_to(skills_root)):
+                continue
             content = manifest.read_text()
             metadata, body = self.parse_frontmatter(content)
-            name = str(metadata.get("name") or manifest.parent.name).strip()
-            description = metadata.get("description") or body.splitlines()[0]
+            raw_name = metadata.get("name")
+            name = raw_name.strip() if isinstance(raw_name, str) else ""
+            name = name or manifest.parent.name
+            raw_description = metadata.get("description")
+            description = (raw_description.strip()
+                           if isinstance(raw_description, str) else "")
+            description = description or body.split("\n", 1)[0]
             description = " ".join(str(description).lstrip("# ").split())
             self.skills[name] = {
                 "name": name,

s07_skill_loading/code.py

@@ -57,29 +57,47 @@ class SkillLoader:
 
     @staticmethod
     def parse_frontmatter(text: str) -> tuple[dict, str]:
-        if not text.startswith("---"):
+        lines = text.splitlines(keepends=True)
+        if not lines or lines[0].rstrip("\r\n") != "---":
             return {}, text
-        parts = text.split("---", 2)
-        if len(parts) < 3:
+
+        closing_index = next(
+            (index for index, line in enumerate(lines[1:], start=1)
+             if line.rstrip("\r\n") == "---"),
+            None,
+        )
+        if closing_index is None:
             return {}, text
+
+        frontmatter = "".join(lines[1:closing_index])
+        body = "".join(lines[closing_index + 1:]).strip()
         try:
-            metadata = yaml.safe_load(parts[1]) or {}
+            metadata = yaml.safe_load(frontmatter) or {}
         except yaml.YAMLError:
             metadata = {}
         if not isinstance(metadata, dict):
             metadata = {}
-        return metadata, parts[2].lstrip()
+        return metadata, body
 
     def scan(self):
         self.skills.clear()
         if not self.skills_dir.exists():
             return
 
+        skills_root = self.skills_dir.resolve()
         for manifest in sorted(self.skills_dir.glob("*/SKILL.md")):
+            if (not manifest.is_file()
+                    or not manifest.resolve().is_relative_to(skills_root)):
+                continue
             content = manifest.read_text()
             metadata, body = self.parse_frontmatter(content)
-            name = str(metadata.get("name") or manifest.parent.name).strip()
-            description = metadata.get("description") or body.splitlines()[0]
+            raw_name = metadata.get("name")
+            name = raw_name.strip() if isinstance(raw_name, str) else ""
+            name = name or manifest.parent.name
+            raw_description = metadata.get("description")
+            description = (raw_description.strip()
+                           if isinstance(raw_description, str) else "")
+            description = description or body.split("\n", 1)[0]
             description = " ".join(str(description).lstrip("# ").split())
             self.skills[name] = {
                 "name": name,

s15_integrated_harness/code.py

@@ -661,32 +661,50 @@ SKILL_REGISTRY: dict[str, dict] = {}
 
 
 def _parse_frontmatter(text: str) -> tuple[dict, str]:
-    if not text.startswith("---"):
+    lines = text.splitlines(keepends=True)
+    if not lines or lines[0].rstrip("\r\n") != "---":
         return {}, text
-    parts = text.split("---", 2)
-    if len(parts) < 3:
+
+    closing_index = next(
+        (index for index, line in enumerate(lines[1:], start=1)
+         if line.rstrip("\r\n") == "---"),
+        None,
+    )
+    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 {}
+        meta = yaml.safe_load(frontmatter) or {}
     except yaml.YAMLError:
         meta = {}
-    return meta, parts[2].strip()
+    if not isinstance(meta, dict):
+        meta = {}
+    return meta, body
 
 
 def scan_skills():
     SKILL_REGISTRY.clear()
     if not SKILLS_DIR.exists():
         return
+    skills_root = SKILLS_DIR.resolve()
     for directory in sorted(SKILLS_DIR.iterdir()):
         if not directory.is_dir():
             continue
         manifest = directory / "SKILL.md"
         if not manifest.exists():
             continue
+        if not manifest.resolve().is_relative_to(skills_root):
+            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)
+        raw_name = meta.get("name")
+        name = raw_name.strip() if isinstance(raw_name, str) else ""
+        name = name or directory.name
+        raw_desc = meta.get("description")
+        desc = raw_desc.strip() if isinstance(raw_desc, str) else ""
+        desc = desc or body.split("\n", 1)[0].lstrip("#").strip()
         SKILL_REGISTRY[name] = {
             "name": name,
             "description": desc,

tests/test_skill_loading.py

@@ -2,15 +2,18 @@ import importlib.util
 import os
 import sys
 import tempfile
+import time
 import types
 from pathlib import Path
 
 
 ROOT = Path(__file__).resolve().parents[1]
 LESSON = ROOT / "s07_skill_loading" / "code.py"
+INTEGRATED_LESSON = ROOT / "s15_integrated_harness" / "code.py"
+SKILL_LESSONS = (LESSON, INTEGRATED_LESSON)
 
 
-def load_lesson(workdir: Path):
+def load_lesson(workdir: Path, lesson_path: Path = LESSON):
     fake_anthropic = types.ModuleType("anthropic")
     fake_dotenv = types.ModuleType("dotenv")
 
@@ -28,12 +31,14 @@ def load_lesson(workdir: Path):
     previous_cwd = Path.cwd()
     previous_model = os.environ.get("MODEL_ID")
 
-    spec = importlib.util.spec_from_file_location("s07_skill_test", LESSON)
+    module_name = f"skill_loading_test_{lesson_path.parent.name}_{time.time_ns()}"
+    spec = importlib.util.spec_from_file_location(module_name, lesson_path)
     assert spec is not None and spec.loader is not None
     module = importlib.util.module_from_spec(spec)
 
     sys.modules["anthropic"] = fake_anthropic
     sys.modules["dotenv"] = fake_dotenv
+    sys.modules[module_name] = module
     try:
         os.chdir(workdir)
         os.environ["MODEL_ID"] = "test-model"
@@ -50,6 +55,13 @@ def load_lesson(workdir: Path):
                 sys.modules.pop(name, None)
             else:
                 sys.modules[name] = previous
+        sys.modules.pop(module_name, None)
+
+
+def parse_frontmatter(lesson, text: str) -> tuple[dict, str]:
+    if hasattr(lesson, "SkillLoader"):
+        return lesson.SkillLoader.parse_frontmatter(text)
+    return lesson._parse_frontmatter(text)
 
 
 def test_catalog_stays_small_and_load_skill_returns_the_full_file() -> None:
@@ -93,3 +105,60 @@ def test_s07_exposes_only_base_tools_and_load_skill() -> None:
             "glob",
             "load_skill",
         ]
+
+
+def test_skill_frontmatter_requires_standalone_delimiters() -> None:
+    invalid_opening = "---not frontmatter\n---\n# Body"
+    block_scalar = """---
+name: demo
+description: |
+  before
+  ---
+  after
+---
+# Body
+"""
+    for lesson_path in SKILL_LESSONS:
+        with tempfile.TemporaryDirectory() as tmp:
+            lesson = load_lesson(Path(tmp), lesson_path)
+            assert parse_frontmatter(lesson, invalid_opening) == ({}, invalid_opening)
+            for text in (block_scalar, block_scalar.replace("\n", "\r\n")):
+                metadata, body = parse_frontmatter(lesson, text)
+                assert metadata["description"] == "before\n---\nafter\n"
+                assert body == "# Body"
+
+
+def test_skill_frontmatter_falls_back_for_invalid_or_empty_metadata() -> None:
+    manifest = "---\nname:\ndescription:\n---\n# Body description\n"
+    for lesson_path in SKILL_LESSONS:
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            skill_dir = root / "skills" / "fallback-skill"
+            skill_dir.mkdir(parents=True)
+            (skill_dir / "SKILL.md").write_text(manifest)
+            empty_dir = root / "skills" / "empty-skill"
+            empty_dir.mkdir()
+            (empty_dir / "SKILL.md").write_text("---\nname: empty-skill\n---\n")
+            typed_dir = root / "skills" / "typed-fallback"
+            typed_dir.mkdir()
+            (typed_dir / "SKILL.md").write_text(
+                "---\nname: [bad]\ndescription: [bad]\n---\n# Typed fallback\n"
+            )
+            outside = root / "outside-skill.md"
+            outside.write_text("# External skill\n\nDO_NOT_LOAD")
+            linked_dir = root / "skills" / "linked-skill"
+            linked_dir.mkdir()
+            (linked_dir / "SKILL.md").symlink_to(outside)
+            lesson = load_lesson(root, lesson_path)
+            registry = (lesson.SKILL_LOADER.skills if hasattr(lesson, "SKILL_LOADER")
+                        else lesson.SKILL_REGISTRY)
+            assert registry["fallback-skill"]["description"] == "Body description"
+            assert registry["empty-skill"]["description"] == ""
+            assert registry["typed-fallback"]["description"] == "Typed fallback"
+            assert "linked-skill" not in registry
+
+            metadata, body = parse_frontmatter(
+                lesson, "---\n- not\n- a mapping\n---\nBody"
+            )
+            assert metadata == {}
+            assert body == "Body"