diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 79b991f5a5..5219880741 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -28,6 +28,7 @@ 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 @@ def _reconcile_skills(self, command_names: List[str]) -> None: 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 @@ def _skill_title_from_command(cmd_name: str) -> str: 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 @@ def _register_skills( 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 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: 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 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: 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 f1c0e95f4e..29c1a9e5e2 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2346,6 +2346,154 @@ def test_skill_overridden_on_preset_install(self, project_dir, temp_dir): 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_reconcile_override_skill_resolves_command_refs(self, project_dir, temp_dir): + """Reconcile's project-override restore must resolve command tokens (issue #2717). + + When a preset that overrode a command is removed and a project override + becomes the winning layer, ``_reconcile_skills`` rewrites the skill from + the override body — which must also render ``__SPECKIT_COMMAND_*__`` tokens. + """ + self._write_init_options(project_dir, ai="claude") + skills_dir = project_dir / ".claude" / "skills" + self._create_skill(skills_dir, "speckit-specify") + + # Project override wins once the preset is removed; its body carries a + # command cross-reference token. No core template exists for "specify", + # so the skill is restored exclusively via the reconcile override branch. + overrides_dir = project_dir / ".specify" / "templates" / "overrides" + overrides_dir.mkdir(parents=True, exist_ok=True) + (overrides_dir / "speckit.specify.md").write_text( + "---\ndescription: Override specify\n---\n\n" + "Then run `__SPECKIT_COMMAND_PLAN__`.\n" + ) + + preset_dir = self._create_command_preset( + temp_dir, + "cmdref-reconcile", + "speckit.specify", + "Preset specify", + "Preset body\n", + ) + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.5") + manager.remove("cmdref-reconcile") + + content = (skills_dir / "speckit-specify" / "SKILL.md").read_text() + assert "override:speckit.specify" in content, "skill should be restored from the project override" + assert "__SPECKIT_COMMAND_" not in content, "raw command token leaked on reconcile" + assert "/speckit-plan" in content + + def test_extension_restore_resolves_command_refs(self, project_dir, temp_dir): + """Extension-backed skill restore must resolve command tokens (issue #2717). + + When a preset override is removed and the skill is restored from an + extension command body, ``__SPECKIT_COMMAND_*__`` tokens in that body + must render as slash-command invocations like the core-template path. + """ + self._write_init_options(project_dir, ai="claude") + skills_dir = project_dir / ".claude" / "skills" + self._create_skill(skills_dir, "speckit-fakeext-cmd", body="original extension skill") + + extension_dir = project_dir / ".specify" / "extensions" / "fakeext" + (extension_dir / "commands").mkdir(parents=True, exist_ok=True) + (extension_dir / "commands" / "cmd.md").write_text( + "---\ndescription: Extension fakeext cmd\n---\n\n" + "Then run `__SPECKIT_COMMAND_PLAN__`.\n" + ) + extension_manifest = { + "schema_version": "1.0", + "extension": { + "id": "fakeext", + "name": "Fake Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.fakeext.cmd", + "file": "commands/cmd.md", + "description": "Fake extension command", + } + ] + }, + } + with open(extension_dir / "extension.yml", "w") as f: + yaml.dump(extension_manifest, f) + + preset_dir = self._create_command_preset( + temp_dir, + "cmdref-ext-restore", + "speckit.fakeext.cmd", + "Override fakeext cmd", + "Override body\n", + ) + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.5") + manager.remove("cmdref-ext-restore") + + content = (skills_dir / "speckit-fakeext-cmd" / "SKILL.md").read_text() + assert "source: extension:fakeext" in content, "skill should be restored from the extension" + assert "__SPECKIT_COMMAND_" not in content, "raw command token leaked on extension 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")