Skip to content

Commit 0d0382f

Browse files
committed
Harden extension native skill registration
1 parent bd7f6a1 commit 0d0382f

2 files changed

Lines changed: 132 additions & 21 deletions

File tree

src/specify_cli/extensions.py

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -511,24 +511,32 @@ def _ignore(directory: str, entries: List[str]) -> Set[str]:
511511
return _ignore
512512

513513
def _get_skills_dir(self) -> Optional[Path]:
514-
"""Return the skills directory if ``--ai-skills`` was used during init.
514+
"""Return the active skills directory for extension skill registration.
515515
516516
Reads ``.specify/init-options.json`` to determine whether skills
517517
are enabled and which agent was selected, then delegates to
518518
the module-level ``_get_skills_dir()`` helper for the concrete path.
519519
520+
Kimi is treated as a native-skills agent: if ``ai == "kimi"`` and
521+
``.kimi/skills`` exists, extension installs should still propagate
522+
command skills even when ``ai_skills`` is false.
523+
520524
Returns:
521525
The skills directory ``Path``, or ``None`` if skills were not
522-
enabled or the init-options file is missing.
526+
enabled and no native-skills fallback applies.
523527
"""
524528
from . import load_init_options, _get_skills_dir as resolve_skills_dir
525529

526530
opts = load_init_options(self.project_root)
527-
if not opts.get("ai_skills"):
528-
return None
531+
if not isinstance(opts, dict):
532+
opts = {}
529533

530534
agent = opts.get("ai")
531-
if not agent:
535+
if not isinstance(agent, str) or not agent:
536+
return None
537+
538+
ai_skills_enabled = bool(opts.get("ai_skills"))
539+
if not ai_skills_enabled and agent != "kimi":
532540
return None
533541

534542
skills_dir = resolve_skills_dir(self.project_root, agent)
@@ -560,9 +568,18 @@ def _register_extension_skills(
560568
if not skills_dir:
561569
return []
562570

571+
from . import load_init_options
572+
from .agents import CommandRegistrar
563573
import yaml
564574

565575
written: List[str] = []
576+
opts = load_init_options(self.project_root)
577+
if not isinstance(opts, dict):
578+
opts = {}
579+
selected_ai = opts.get("ai")
580+
if not isinstance(selected_ai, str) or not selected_ai:
581+
return []
582+
registrar = CommandRegistrar()
566583

567584
for cmd_info in manifest.commands:
568585
cmd_name = cmd_info["name"]
@@ -612,22 +629,11 @@ def _register_extension_skills(
612629
except OSError:
613630
pass # best-effort cleanup
614631
continue
615-
if content.startswith("---"):
616-
parts = content.split("---", 2)
617-
if len(parts) >= 3:
618-
try:
619-
frontmatter = yaml.safe_load(parts[1])
620-
except yaml.YAMLError:
621-
frontmatter = {}
622-
if not isinstance(frontmatter, dict):
623-
frontmatter = {}
624-
body = parts[2].strip()
625-
else:
626-
frontmatter = {}
627-
body = content
628-
else:
629-
frontmatter = {}
630-
body = content
632+
frontmatter, body = registrar.parse_frontmatter(content)
633+
frontmatter = registrar._adjust_script_paths(frontmatter)
634+
body = registrar.resolve_skill_placeholders(
635+
selected_ai, frontmatter, body, self.project_root
636+
)
631637

632638
original_desc = frontmatter.get("description", "")
633639
description = original_desc or f"Extension command: {cmd_name}"

tests/test_extension_skills.py

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,24 @@ def test_returns_none_when_skills_dir_missing(self, project_dir):
192192
result = manager._get_skills_dir()
193193
assert result is None
194194

195+
def test_returns_kimi_skills_dir_when_ai_skills_disabled(self, project_dir):
196+
"""Kimi should still use its native skills dir when ai_skills is false."""
197+
_create_init_options(project_dir, ai="kimi", ai_skills=False)
198+
skills_dir = _create_skills_dir(project_dir, ai="kimi")
199+
manager = ExtensionManager(project_dir)
200+
result = manager._get_skills_dir()
201+
assert result == skills_dir
202+
203+
def test_returns_none_for_non_dict_init_options(self, project_dir):
204+
"""Corrupted-but-parseable init-options should not crash skill-dir lookup."""
205+
opts_file = project_dir / ".specify" / "init-options.json"
206+
opts_file.parent.mkdir(parents=True, exist_ok=True)
207+
opts_file.write_text("[]")
208+
_create_skills_dir(project_dir, ai="claude")
209+
manager = ExtensionManager(project_dir)
210+
result = manager._get_skills_dir()
211+
assert result is None
212+
195213

196214
# ===== Extension Skill Registration Tests =====
197215

@@ -326,6 +344,78 @@ def test_kimi_uses_hyphenated_skill_names(self, project_dir, temp_dir):
326344
assert "speckit-test-ext-hello" in metadata["registered_skills"]
327345
assert "speckit-test-ext-world" in metadata["registered_skills"]
328346

347+
def test_kimi_creates_skills_when_ai_skills_disabled(self, project_dir, temp_dir):
348+
"""Kimi should still auto-register extension skills in native-skills mode."""
349+
_create_init_options(project_dir, ai="kimi", ai_skills=False)
350+
skills_dir = _create_skills_dir(project_dir, ai="kimi")
351+
ext_dir = _create_extension_dir(temp_dir, ext_id="test-ext")
352+
353+
manager = ExtensionManager(project_dir)
354+
manifest = manager.install_from_directory(
355+
ext_dir, "0.1.0", register_commands=False
356+
)
357+
358+
metadata = manager.registry.get(manifest.id)
359+
assert "speckit-test-ext-hello" in metadata["registered_skills"]
360+
assert "speckit-test-ext-world" in metadata["registered_skills"]
361+
assert (skills_dir / "speckit-test-ext-hello" / "SKILL.md").exists()
362+
363+
def test_skill_registration_resolves_script_placeholders(self, project_dir, temp_dir):
364+
"""Auto-registered extension skills should resolve script placeholders."""
365+
_create_init_options(project_dir, ai="claude", ai_skills=True)
366+
skills_dir = _create_skills_dir(project_dir, ai="claude")
367+
368+
ext_dir = temp_dir / "scripted-ext"
369+
ext_dir.mkdir()
370+
manifest_data = {
371+
"schema_version": "1.0",
372+
"extension": {
373+
"id": "scripted-ext",
374+
"name": "Scripted Extension",
375+
"version": "1.0.0",
376+
"description": "Test",
377+
},
378+
"requires": {"speckit_version": ">=0.1.0"},
379+
"provides": {
380+
"commands": [
381+
{
382+
"name": "speckit.scripted-ext.plan",
383+
"file": "commands/plan.md",
384+
"description": "Scripted plan command",
385+
}
386+
]
387+
},
388+
}
389+
with open(ext_dir / "extension.yml", "w") as f:
390+
yaml.dump(manifest_data, f)
391+
392+
(ext_dir / "commands").mkdir()
393+
(ext_dir / "commands" / "plan.md").write_text(
394+
"---\n"
395+
"description: Scripted plan command\n"
396+
"scripts:\n"
397+
" sh: ../../scripts/bash/setup-plan.sh --json \"{ARGS}\"\n"
398+
"agent_scripts:\n"
399+
" sh: ../../scripts/bash/update-agent-context.sh __AGENT__\n"
400+
"---\n\n"
401+
"Run {SCRIPT}\n"
402+
"Then {AGENT_SCRIPT}\n"
403+
"Review templates/checklist.md and memory/constitution.md for __AGENT__.\n"
404+
)
405+
406+
manager = ExtensionManager(project_dir)
407+
manager.install_from_directory(ext_dir, "0.1.0", register_commands=False)
408+
409+
content = (skills_dir / "speckit-scripted-ext-plan" / "SKILL.md").read_text()
410+
assert "{SCRIPT}" not in content
411+
assert "{AGENT_SCRIPT}" not in content
412+
assert "{ARGS}" not in content
413+
assert "__AGENT__" not in content
414+
assert '.specify/scripts/bash/setup-plan.sh --json "$ARGUMENTS"' in content
415+
assert ".specify/scripts/bash/update-agent-context.sh claude" in content
416+
assert ".specify/templates/checklist.md" in content
417+
assert ".specify/memory/constitution.md" in content
418+
329419
def test_missing_command_file_skipped(self, skills_project, temp_dir):
330420
"""Commands with missing source files should be skipped gracefully."""
331421
project_dir, skills_dir = skills_project
@@ -453,6 +543,21 @@ def test_remove_no_skills_when_not_active(self, no_skills_project, extension_dir
453543
class TestExtensionSkillEdgeCases:
454544
"""Test edge cases in extension skill registration."""
455545

546+
def test_install_with_non_dict_init_options_does_not_crash(self, project_dir, extension_dir):
547+
"""Corrupted init-options payloads should disable skill registration, not crash install."""
548+
opts_file = project_dir / ".specify" / "init-options.json"
549+
opts_file.parent.mkdir(parents=True, exist_ok=True)
550+
opts_file.write_text("[]")
551+
_create_skills_dir(project_dir, ai="claude")
552+
553+
manager = ExtensionManager(project_dir)
554+
manifest = manager.install_from_directory(
555+
extension_dir, "0.1.0", register_commands=False
556+
)
557+
558+
metadata = manager.registry.get(manifest.id)
559+
assert metadata["registered_skills"] == []
560+
456561
def test_command_without_frontmatter(self, skills_project, temp_dir):
457562
"""Commands without YAML frontmatter should still produce valid skills."""
458563
project_dir, skills_dir = skills_project

0 commit comments

Comments
 (0)