From 8dd93c0d71ce10f1803ddf0d015377ffe0287e74 Mon Sep 17 00:00:00 2001 From: Huy Date: Wed, 27 May 2026 11:42:05 +0700 Subject: [PATCH] fix: resolve __SPECKIT_COMMAND_*__ refs in preset skill rendering (#2717) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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___ placeholders instead of rendering them as /speckit- 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. --- src/specify_cli/presets.py | 28 +++++++++++++++++++ tests/test_presets.py | 56 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 79b991f5a..521988074 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -28,6 +28,7 @@ from packaging import version as pkg_version from packaging.specifiers import SpecifierSet, InvalidSpecifier from .extensions import REINSTALL_COMMAND, ExtensionRegistry, normalize_priority +from .integrations.base import IntegrationBase def _substitute_core_template( @@ -1058,6 +1059,9 @@ class PresetManager: body = registrar.resolve_skill_placeholders( selected_ai, fm, body, self.project_root ) + body = self._resolve_skill_command_refs( + body, registrar, selected_ai + ) fm_data = registrar.build_skill_frontmatter( selected_ai if isinstance(selected_ai, str) else "", skill_name, desc, @@ -1134,6 +1138,23 @@ class PresetManager: title_name = title_name[len("speckit."):] 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___`` placeholder into the matching + slash-command invocation — ``/speckit-`` for a ``-`` separator, + ``/speckit.`` 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]]: """Index extension-backed skill restore data by skill directory name.""" from .extensions import ExtensionManifest, ValidationError @@ -1310,6 +1331,7 @@ class PresetManager: body = registrar.resolve_skill_placeholders( 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: skill_subdir = skills_dir / target_skill_name @@ -1402,6 +1424,9 @@ class PresetManager: body = registrar.resolve_skill_placeholders( selected_ai, frontmatter, body, self.project_root ) + body = self._resolve_skill_command_refs( + body, registrar, selected_ai + ) original_desc = frontmatter.get("description", "") enhanced_desc = original_desc or SKILL_DESCRIPTIONS.get( @@ -1439,6 +1464,9 @@ class PresetManager: body = registrar.resolve_skill_placeholders( selected_ai, frontmatter, body, self.project_root ) + body = self._resolve_skill_command_refs( + body, registrar, selected_ai + ) command_name = extension_restore["command_name"] title_name = self._skill_title_from_command(command_name) diff --git a/tests/test_presets.py b/tests/test_presets.py index f1c0e95f4..5f33cf589 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2346,6 +2346,62 @@ class TestPresetSkills: metadata = manager.registry.get("self-test") 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-`` 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-. + 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): """Preset skill overrides for core commands should keep preset frontmatter descriptions.""" self._write_init_options(project_dir, ai="claude")