mirror of
https://github.com/github/spec-kit.git
synced 2026-07-04 04:45:43 +08:00
fix: resolve __SPECKIT_COMMAND_*__ refs in preset skill rendering (#2717)
The preset skill layer mirrors command templates into SKILL.md files but only ran resolve_skill_placeholders(), leaving command cross-references as raw __SPECKIT_COMMAND_<NAME>__ placeholders instead of rendering them as /speckit-<cmd> the way CommandRegistrar.register_commands() does. As a result, presets that override core commands under the agent skill layer (e.g. Claude --ai-skills) leaked the raw tokens into SKILL.md. Add a shared PresetManager._resolve_skill_command_refs() helper that maps the agent's invoke separator to IntegrationBase.resolve_command_refs(), and call it right after resolve_skill_placeholders() in every preset skill-rendering path: _register_skills() (install), the _reconcile_skills() override-restoration block, and both _unregister_skills() restore paths. This mirrors register_commands() and addresses the path divergence flagged in #1976. Add regression tests covering the install and restore paths. AI assistance: authored with Claude Code (Anthropic) — analysis, patch, and tests. Verified via the existing pytest suite plus a manual CLI install and remove cycle on a Claude --ai-skills project.
This commit is contained in:
@@ -28,6 +28,7 @@ from packaging import version as pkg_version
|
|||||||
from packaging.specifiers import SpecifierSet, InvalidSpecifier
|
from packaging.specifiers import SpecifierSet, InvalidSpecifier
|
||||||
|
|
||||||
from .extensions import REINSTALL_COMMAND, ExtensionRegistry, normalize_priority
|
from .extensions import REINSTALL_COMMAND, ExtensionRegistry, normalize_priority
|
||||||
|
from .integrations.base import IntegrationBase
|
||||||
|
|
||||||
|
|
||||||
def _substitute_core_template(
|
def _substitute_core_template(
|
||||||
@@ -1058,6 +1059,9 @@ class PresetManager:
|
|||||||
body = registrar.resolve_skill_placeholders(
|
body = registrar.resolve_skill_placeholders(
|
||||||
selected_ai, fm, body, self.project_root
|
selected_ai, fm, body, self.project_root
|
||||||
)
|
)
|
||||||
|
body = self._resolve_skill_command_refs(
|
||||||
|
body, registrar, selected_ai
|
||||||
|
)
|
||||||
fm_data = registrar.build_skill_frontmatter(
|
fm_data = registrar.build_skill_frontmatter(
|
||||||
selected_ai if isinstance(selected_ai, str) else "",
|
selected_ai if isinstance(selected_ai, str) else "",
|
||||||
skill_name, desc,
|
skill_name, desc,
|
||||||
@@ -1134,6 +1138,23 @@ class PresetManager:
|
|||||||
title_name = title_name[len("speckit."):]
|
title_name = title_name[len("speckit."):]
|
||||||
return title_name.replace(".", " ").replace("-", " ").title()
|
return title_name.replace(".", " ").replace("-", " ").title()
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _resolve_skill_command_refs(
|
||||||
|
body: str, registrar: "CommandRegistrar", selected_ai: str
|
||||||
|
) -> str:
|
||||||
|
"""Render ``__SPECKIT_COMMAND_*__`` tokens in a skill body as invocations.
|
||||||
|
|
||||||
|
Looks up the agent's invoke separator and rewrites each
|
||||||
|
``__SPECKIT_COMMAND_<NAME>__`` placeholder into the matching
|
||||||
|
slash-command invocation — ``/speckit-<cmd>`` for a ``-`` separator,
|
||||||
|
``/speckit.<cmd>`` for ``.`` — the same rendering the command layer
|
||||||
|
applies via ``CommandRegistrar.register_commands()``.
|
||||||
|
"""
|
||||||
|
separator = registrar.AGENT_CONFIGS.get(selected_ai, {}).get(
|
||||||
|
"invoke_separator", "."
|
||||||
|
)
|
||||||
|
return IntegrationBase.resolve_command_refs(body, separator)
|
||||||
|
|
||||||
def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]:
|
def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]:
|
||||||
"""Index extension-backed skill restore data by skill directory name."""
|
"""Index extension-backed skill restore data by skill directory name."""
|
||||||
from .extensions import ExtensionManifest, ValidationError
|
from .extensions import ExtensionManifest, ValidationError
|
||||||
@@ -1310,6 +1331,7 @@ class PresetManager:
|
|||||||
body = registrar.resolve_skill_placeholders(
|
body = registrar.resolve_skill_placeholders(
|
||||||
selected_ai, frontmatter, body, self.project_root
|
selected_ai, frontmatter, body, self.project_root
|
||||||
)
|
)
|
||||||
|
body = self._resolve_skill_command_refs(body, registrar, selected_ai)
|
||||||
|
|
||||||
for target_skill_name in target_skill_names:
|
for target_skill_name in target_skill_names:
|
||||||
skill_subdir = skills_dir / target_skill_name
|
skill_subdir = skills_dir / target_skill_name
|
||||||
@@ -1402,6 +1424,9 @@ class PresetManager:
|
|||||||
body = registrar.resolve_skill_placeholders(
|
body = registrar.resolve_skill_placeholders(
|
||||||
selected_ai, frontmatter, body, self.project_root
|
selected_ai, frontmatter, body, self.project_root
|
||||||
)
|
)
|
||||||
|
body = self._resolve_skill_command_refs(
|
||||||
|
body, registrar, selected_ai
|
||||||
|
)
|
||||||
|
|
||||||
original_desc = frontmatter.get("description", "")
|
original_desc = frontmatter.get("description", "")
|
||||||
enhanced_desc = original_desc or SKILL_DESCRIPTIONS.get(
|
enhanced_desc = original_desc or SKILL_DESCRIPTIONS.get(
|
||||||
@@ -1439,6 +1464,9 @@ class PresetManager:
|
|||||||
body = registrar.resolve_skill_placeholders(
|
body = registrar.resolve_skill_placeholders(
|
||||||
selected_ai, frontmatter, body, self.project_root
|
selected_ai, frontmatter, body, self.project_root
|
||||||
)
|
)
|
||||||
|
body = self._resolve_skill_command_refs(
|
||||||
|
body, registrar, selected_ai
|
||||||
|
)
|
||||||
|
|
||||||
command_name = extension_restore["command_name"]
|
command_name = extension_restore["command_name"]
|
||||||
title_name = self._skill_title_from_command(command_name)
|
title_name = self._skill_title_from_command(command_name)
|
||||||
|
|||||||
@@ -2346,6 +2346,62 @@ class TestPresetSkills:
|
|||||||
metadata = manager.registry.get("self-test")
|
metadata = manager.registry.get("self-test")
|
||||||
assert "speckit-specify" in metadata.get("registered_skills", [])
|
assert "speckit-specify" in metadata.get("registered_skills", [])
|
||||||
|
|
||||||
|
def test_register_skills_resolves_command_refs(self, project_dir, temp_dir):
|
||||||
|
"""Preset skill overrides must resolve __SPECKIT_COMMAND_*__ tokens (issue #2717).
|
||||||
|
|
||||||
|
``_register_skills()`` previously ran only ``resolve_skill_placeholders()``,
|
||||||
|
so command cross-references leaked into SKILL.md as raw placeholders
|
||||||
|
instead of rendering as ``/speckit-<cmd>`` like the command layer.
|
||||||
|
"""
|
||||||
|
self._write_init_options(project_dir, ai="claude")
|
||||||
|
skills_dir = project_dir / ".claude" / "skills"
|
||||||
|
self._create_skill(skills_dir, "speckit-specify")
|
||||||
|
|
||||||
|
preset_dir = self._create_command_preset(
|
||||||
|
temp_dir,
|
||||||
|
"cmdref-install",
|
||||||
|
"speckit.specify",
|
||||||
|
"Override specify",
|
||||||
|
"Run `__SPECKIT_COMMAND_SPECIFY__` then `__SPECKIT_COMMAND_PLAN__`.\n",
|
||||||
|
)
|
||||||
|
|
||||||
|
manager = PresetManager(project_dir)
|
||||||
|
manager.install_from_directory(preset_dir, "0.1.5")
|
||||||
|
|
||||||
|
content = (skills_dir / "speckit-specify" / "SKILL.md").read_text()
|
||||||
|
assert "__SPECKIT_COMMAND_" not in content, "raw command token leaked into SKILL.md"
|
||||||
|
# Claude's invoke_separator is "-", so tokens render as /speckit-<cmd>.
|
||||||
|
assert "/speckit-specify" in content
|
||||||
|
assert "/speckit-plan" in content
|
||||||
|
|
||||||
|
def test_restore_skill_resolves_command_refs(self, project_dir, temp_dir):
|
||||||
|
"""Skill restore on preset removal must also resolve command tokens (issue #2717)."""
|
||||||
|
self._write_init_options(project_dir, ai="claude")
|
||||||
|
skills_dir = project_dir / ".claude" / "skills"
|
||||||
|
self._create_skill(skills_dir, "speckit-specify")
|
||||||
|
|
||||||
|
core_cmds = project_dir / ".specify" / "templates" / "commands"
|
||||||
|
core_cmds.mkdir(parents=True, exist_ok=True)
|
||||||
|
(core_cmds / "specify.md").write_text(
|
||||||
|
"---\ndescription: Core specify\n---\n\n"
|
||||||
|
"Then run `__SPECKIT_COMMAND_PLAN__`.\n"
|
||||||
|
)
|
||||||
|
|
||||||
|
preset_dir = self._create_command_preset(
|
||||||
|
temp_dir,
|
||||||
|
"cmdref-restore",
|
||||||
|
"speckit.specify",
|
||||||
|
"Override specify",
|
||||||
|
"Override body\n",
|
||||||
|
)
|
||||||
|
manager = PresetManager(project_dir)
|
||||||
|
manager.install_from_directory(preset_dir, "0.1.5")
|
||||||
|
manager.remove("cmdref-restore")
|
||||||
|
|
||||||
|
content = (skills_dir / "speckit-specify" / "SKILL.md").read_text()
|
||||||
|
assert "__SPECKIT_COMMAND_" not in content, "raw command token leaked on restore"
|
||||||
|
assert "/speckit-plan" in content
|
||||||
|
|
||||||
def test_core_command_override_skill_uses_preset_command_description(self, project_dir, temp_dir):
|
def test_core_command_override_skill_uses_preset_command_description(self, project_dir, temp_dir):
|
||||||
"""Preset skill overrides for core commands should keep preset frontmatter descriptions."""
|
"""Preset skill overrides for core commands should keep preset frontmatter descriptions."""
|
||||||
self._write_init_options(project_dir, ai="claude")
|
self._write_init_options(project_dir, ai="claude")
|
||||||
|
|||||||
Reference in New Issue
Block a user