fix(extensions): parse SKILL.md on the --- delimiter line during removal (#3634)

* fix(extensions): parse SKILL.md on the --- delimiter line during removal

ExtensionManager._unregister_extension_skills verified an installed skill
before deleting it by reading metadata.source back from its SKILL.md with a
raw split("---", 2). That substring split stops at the first "---" anywhere
after the opening delimiter, including one embedded in a command description
(e.g. "Separate sections with --- markers"). The frontmatter was then
truncated mid-value, metadata.source parsed empty, the skill looked
unrelated, and its directory was left orphaned on uninstall.

Parse on the "---" delimiter *line* instead, reusing CommandRegistrar.
parse_frontmatter (the line-anchored parser from #3590) in both the fast
(registry-driven) and fallback (directory-scan) removal paths.

Add a regression test that installs an extension whose command description
contains "---", removes it, and asserts the skill directory is gone. Fails
before the fix (dir orphaned), passes after.

* test: cover the fallback scan branch for the --- SKILL.md parse

Copilot noted the new regression test only exercised the fast removal
path (skills_project keeps ai_skills enabled, so remove() resolves the
skills dir directly). Add test_skills_removed_with_dashes_via_fallback_scan,
which deletes init-options.json after install so _get_skills_dir() returns
None and removal takes the fallback directory-scan branch. That branch
re-reads metadata.source with an independently duplicated parser; reverting
it to the old substring split now fails this test (dir orphaned) while the
fast-path test still passes.

Author: Quratulain-bilal

src/specify_cli/extensions/__init__.py

@@ -1330,19 +1330,20 @@ def _unregister_extension_skills(
                 if not skill_md.is_file():
                     continue
                 try:
-                    import yaml as _yaml
+                    from ..agents import CommandRegistrar as _Registrar
 
                     raw = skill_md.read_text(encoding="utf-8")
-                    source = ""
-                    if raw.startswith("---"):
-                        parts = raw.split("---", 2)
-                        if len(parts) >= 3:
-                            fm = _yaml.safe_load(parts[1]) or {}
-                            source = (
-                                fm.get("metadata", {}).get("source", "")
-                                if isinstance(fm, dict)
-                                else ""
-                            )
+                    # Parse on the ``---`` delimiter *line*, not any ``---``
+                    # substring: a description containing ``---`` would trip a
+                    # raw ``split("---", 2)`` and hide metadata.source, so this
+                    # extension's own skill would look unrelated and be left
+                    # orphaned. Mirrors the #3590 parse_frontmatter fix.
+                    fm, _ = _Registrar.parse_frontmatter(raw)
+                    source = (
+                        fm.get("metadata", {}).get("source", "")
+                        if isinstance(fm, dict)
+                        else ""
+                    )
                     if source != f"extension:{extension_id}":
                         continue
                 except (OSError, UnicodeDecodeError, Exception):
@@ -1386,19 +1387,20 @@ def _unregister_extension_skills(
                     if not skill_md.is_file():
                         continue
                     try:
-                        import yaml as _yaml
+                        from ..agents import CommandRegistrar as _Registrar
 
                         raw = skill_md.read_text(encoding="utf-8")
-                        source = ""
-                        if raw.startswith("---"):
-                            parts = raw.split("---", 2)
-                            if len(parts) >= 3:
-                                fm = _yaml.safe_load(parts[1]) or {}
-                                source = (
-                                    fm.get("metadata", {}).get("source", "")
-                                    if isinstance(fm, dict)
-                                    else ""
-                                )
+                        # Parse on the ``---`` delimiter *line*, not any ``---``
+                        # substring: a description containing ``---`` would trip
+                        # a raw ``split("---", 2)`` and hide metadata.source, so
+                        # this extension's own skill would look unrelated and be
+                        # left orphaned. Mirrors the #3590 parse_frontmatter fix.
+                        fm, _ = _Registrar.parse_frontmatter(raw)
+                        source = (
+                            fm.get("metadata", {}).get("source", "")
+                            if isinstance(fm, dict)
+                            else ""
+                        )
                         # Only remove skills explicitly created by this extension
                         if source != f"extension:{extension_id}":
                             continue

tests/test_extension_skills.py

@@ -163,6 +163,58 @@ def _create_unicode_extension_dir(temp_dir: Path, ext_id: str = "uni-ext") -> Pa
     return ext_dir
 
 
+def _create_dashed_description_extension_dir(
+    temp_dir: Path, ext_id: str = "dash-ext"
+) -> Path:
+    """Create an extension whose command description contains a ``---`` run.
+
+    A ``---`` inside the description survives into the generated SKILL.md
+    frontmatter and exercises the delimiter-line parsing used when reading
+    metadata.source back during removal (regression guard for the
+    split("---", 2) substring bug, mirroring #3590).
+    """
+    ext_dir = temp_dir / ext_id
+    ext_dir.mkdir()
+    description = "Separate sections with --- markers"
+
+    manifest_data = {
+        "schema_version": "1.0",
+        "extension": {
+            "id": ext_id,
+            "name": "Dashed Extension",
+            "version": "1.0.0",
+            "description": description,
+        },
+        "requires": {"speckit_version": ">=0.1.0"},
+        "provides": {
+            "commands": [
+                {
+                    "name": f"speckit.{ext_id}.hello",
+                    "file": "commands/hello.md",
+                    "description": description,
+                },
+            ]
+        },
+    }
+
+    with open(ext_dir / "extension.yml", "w", encoding="utf-8") as f:
+        yaml.safe_dump(manifest_data, f, allow_unicode=True)
+
+    commands_dir = ext_dir / "commands"
+    commands_dir.mkdir()
+    (commands_dir / "hello.md").write_text(
+        "---\n"
+        f'description: "{description}"\n'
+        "---\n"
+        "\n"
+        "# Hello\n"
+        "\n"
+        "Body.\n",
+        encoding="utf-8",
+    )
+    return ext_dir
+
+
 def _can_create_symlink(temp_dir: Path) -> bool:
     """Return True when the current platform/user can create file symlinks."""
     target = temp_dir / "symlink-target.txt"
@@ -1658,6 +1710,65 @@ def test_skills_removed_on_extension_remove(self, skills_project, extension_dir)
         assert not (skills_dir / "speckit-test-ext-hello").exists()
         assert not (skills_dir / "speckit-test-ext-world").exists()
 
+    def test_skills_removed_when_description_contains_dashes(
+        self, skills_project, temp_dir
+    ):
+        """A ``---`` in the command description must not orphan the skill dir.
+
+        The removal safety check reads metadata.source back from the generated
+        SKILL.md. A raw ``split("---", 2)`` stopped at the ``---`` embedded in
+        the description, so metadata.source parsed empty, the skill looked
+        unrelated, and its directory was left behind. Regression guard for the
+        delimiter-line fix (mirrors #3590).
+        """
+        project_dir, skills_dir = skills_project
+        ext_dir = _create_dashed_description_extension_dir(temp_dir)
+        manager = ExtensionManager(project_dir)
+        manifest = manager.install_from_directory(
+            ext_dir, "0.1.0", register_commands=False
+        )
+
+        skill_dir = skills_dir / "speckit-dash-ext-hello"
+        skill_md = skill_dir / "SKILL.md"
+        assert skill_md.exists()
+        # The dashed description must have survived into the frontmatter.
+        assert "--- markers" in skill_md.read_text(encoding="utf-8")
+
+        result = manager.remove(manifest.id, keep_config=False)
+        assert result is True
+
+        # The extension's own skill must be recognised and removed, not orphaned.
+        assert not skill_dir.exists()
+
+    def test_skills_removed_with_dashes_via_fallback_scan(
+        self, skills_project, temp_dir
+    ):
+        """Same ``---`` guard, but exercised through the fallback scan branch.
+
+        The fast path resolves the skills dir from init-options; the fallback
+        branch scans every candidate agent dir when that resolution returns
+        None, and it re-reads metadata.source with an independently duplicated
+        parser. Deleting init-options.json after install forces removal down
+        the fallback path so a substring-split regression there is caught too.
+        """
+        project_dir, skills_dir = skills_project
+        ext_dir = _create_dashed_description_extension_dir(temp_dir)
+        manager = ExtensionManager(project_dir)
+        manifest = manager.install_from_directory(
+            ext_dir, "0.1.0", register_commands=False
+        )
+
+        skill_dir = skills_dir / "speckit-dash-ext-hello"
+        assert (skill_dir / "SKILL.md").exists()
+
+        # Drop init-options so _get_skills_dir() returns None and removal takes
+        # the fallback directory-scan branch instead of the fast path.
+        (project_dir / ".specify" / "init-options.json").unlink()
+
+        result = manager.remove(manifest.id, keep_config=False)
+        assert result is True
+        assert not skill_dir.exists()
+
     def test_other_skills_preserved_on_remove(self, skills_project, extension_dir):
         """Non-extension skills should not be affected by extension removal."""
         project_dir, skills_dir = skills_project