mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
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.
This commit is contained in:
@@ -1330,19 +1330,20 @@ class ExtensionManager:
|
||||
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 @@ class ExtensionManager:
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user