Skip to content

Commit 66884db

Browse files
authored
fix: resolve __SPECKIT_COMMAND_*__ refs in preset skill rendering (#2717) (#2718)
* 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. * test: cover reconcile-override and extension restore command-ref paths (#2718 review) Copilot review flagged that the install and core-template restore paths gained regression tests, but the reconcile project-override branch and the extension-backed restore branch were uncovered. Add focused tests for both: - test_reconcile_override_skill_resolves_command_refs: a project override wins after preset removal; _reconcile_skills must render command refs. - test_extension_restore_resolves_command_refs: a skill restored from an extension command body must also render command refs. Both fail on main and pass with the fix in 8dd93c0.
1 parent 9af5411 commit 66884db

2 files changed

Lines changed: 176 additions & 0 deletions

File tree

src/specify_cli/presets.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from packaging.specifiers import SpecifierSet, InvalidSpecifier
2929

3030
from .extensions import REINSTALL_COMMAND, ExtensionRegistry, normalize_priority
31+
from .integrations.base import IntegrationBase
3132

3233

3334
def _substitute_core_template(
@@ -1058,6 +1059,9 @@ def _reconcile_skills(self, command_names: List[str]) -> None:
10581059
body = registrar.resolve_skill_placeholders(
10591060
selected_ai, fm, body, self.project_root
10601061
)
1062+
body = self._resolve_skill_command_refs(
1063+
body, registrar, selected_ai
1064+
)
10611065
fm_data = registrar.build_skill_frontmatter(
10621066
selected_ai if isinstance(selected_ai, str) else "",
10631067
skill_name, desc,
@@ -1134,6 +1138,23 @@ def _skill_title_from_command(cmd_name: str) -> str:
11341138
title_name = title_name[len("speckit."):]
11351139
return title_name.replace(".", " ").replace("-", " ").title()
11361140

1141+
@staticmethod
1142+
def _resolve_skill_command_refs(
1143+
body: str, registrar: "CommandRegistrar", selected_ai: str
1144+
) -> str:
1145+
"""Render ``__SPECKIT_COMMAND_*__`` tokens in a skill body as invocations.
1146+
1147+
Looks up the agent's invoke separator and rewrites each
1148+
``__SPECKIT_COMMAND_<NAME>__`` placeholder into the matching
1149+
slash-command invocation — ``/speckit-<cmd>`` for a ``-`` separator,
1150+
``/speckit.<cmd>`` for ``.`` — the same rendering the command layer
1151+
applies via ``CommandRegistrar.register_commands()``.
1152+
"""
1153+
separator = registrar.AGENT_CONFIGS.get(selected_ai, {}).get(
1154+
"invoke_separator", "."
1155+
)
1156+
return IntegrationBase.resolve_command_refs(body, separator)
1157+
11371158
def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]:
11381159
"""Index extension-backed skill restore data by skill directory name."""
11391160
from .extensions import ExtensionManifest, ValidationError
@@ -1310,6 +1331,7 @@ def _register_skills(
13101331
body = registrar.resolve_skill_placeholders(
13111332
selected_ai, frontmatter, body, self.project_root
13121333
)
1334+
body = self._resolve_skill_command_refs(body, registrar, selected_ai)
13131335

13141336
for target_skill_name in target_skill_names:
13151337
skill_subdir = skills_dir / target_skill_name
@@ -1402,6 +1424,9 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None:
14021424
body = registrar.resolve_skill_placeholders(
14031425
selected_ai, frontmatter, body, self.project_root
14041426
)
1427+
body = self._resolve_skill_command_refs(
1428+
body, registrar, selected_ai
1429+
)
14051430

14061431
original_desc = frontmatter.get("description", "")
14071432
enhanced_desc = original_desc or SKILL_DESCRIPTIONS.get(
@@ -1439,6 +1464,9 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None:
14391464
body = registrar.resolve_skill_placeholders(
14401465
selected_ai, frontmatter, body, self.project_root
14411466
)
1467+
body = self._resolve_skill_command_refs(
1468+
body, registrar, selected_ai
1469+
)
14421470

14431471
command_name = extension_restore["command_name"]
14441472
title_name = self._skill_title_from_command(command_name)

tests/test_presets.py

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2346,6 +2346,154 @@ def test_skill_overridden_on_preset_install(self, project_dir, temp_dir):
23462346
metadata = manager.registry.get("self-test")
23472347
assert "speckit-specify" in metadata.get("registered_skills", [])
23482348

2349+
def test_register_skills_resolves_command_refs(self, project_dir, temp_dir):
2350+
"""Preset skill overrides must resolve __SPECKIT_COMMAND_*__ tokens (issue #2717).
2351+
2352+
``_register_skills()`` previously ran only ``resolve_skill_placeholders()``,
2353+
so command cross-references leaked into SKILL.md as raw placeholders
2354+
instead of rendering as ``/speckit-<cmd>`` like the command layer.
2355+
"""
2356+
self._write_init_options(project_dir, ai="claude")
2357+
skills_dir = project_dir / ".claude" / "skills"
2358+
self._create_skill(skills_dir, "speckit-specify")
2359+
2360+
preset_dir = self._create_command_preset(
2361+
temp_dir,
2362+
"cmdref-install",
2363+
"speckit.specify",
2364+
"Override specify",
2365+
"Run `__SPECKIT_COMMAND_SPECIFY__` then `__SPECKIT_COMMAND_PLAN__`.\n",
2366+
)
2367+
2368+
manager = PresetManager(project_dir)
2369+
manager.install_from_directory(preset_dir, "0.1.5")
2370+
2371+
content = (skills_dir / "speckit-specify" / "SKILL.md").read_text()
2372+
assert "__SPECKIT_COMMAND_" not in content, "raw command token leaked into SKILL.md"
2373+
# Claude's invoke_separator is "-", so tokens render as /speckit-<cmd>.
2374+
assert "/speckit-specify" in content
2375+
assert "/speckit-plan" in content
2376+
2377+
def test_restore_skill_resolves_command_refs(self, project_dir, temp_dir):
2378+
"""Skill restore on preset removal must also resolve command tokens (issue #2717)."""
2379+
self._write_init_options(project_dir, ai="claude")
2380+
skills_dir = project_dir / ".claude" / "skills"
2381+
self._create_skill(skills_dir, "speckit-specify")
2382+
2383+
core_cmds = project_dir / ".specify" / "templates" / "commands"
2384+
core_cmds.mkdir(parents=True, exist_ok=True)
2385+
(core_cmds / "specify.md").write_text(
2386+
"---\ndescription: Core specify\n---\n\n"
2387+
"Then run `__SPECKIT_COMMAND_PLAN__`.\n"
2388+
)
2389+
2390+
preset_dir = self._create_command_preset(
2391+
temp_dir,
2392+
"cmdref-restore",
2393+
"speckit.specify",
2394+
"Override specify",
2395+
"Override body\n",
2396+
)
2397+
manager = PresetManager(project_dir)
2398+
manager.install_from_directory(preset_dir, "0.1.5")
2399+
manager.remove("cmdref-restore")
2400+
2401+
content = (skills_dir / "speckit-specify" / "SKILL.md").read_text()
2402+
assert "__SPECKIT_COMMAND_" not in content, "raw command token leaked on restore"
2403+
assert "/speckit-plan" in content
2404+
2405+
def test_reconcile_override_skill_resolves_command_refs(self, project_dir, temp_dir):
2406+
"""Reconcile's project-override restore must resolve command tokens (issue #2717).
2407+
2408+
When a preset that overrode a command is removed and a project override
2409+
becomes the winning layer, ``_reconcile_skills`` rewrites the skill from
2410+
the override body — which must also render ``__SPECKIT_COMMAND_*__`` tokens.
2411+
"""
2412+
self._write_init_options(project_dir, ai="claude")
2413+
skills_dir = project_dir / ".claude" / "skills"
2414+
self._create_skill(skills_dir, "speckit-specify")
2415+
2416+
# Project override wins once the preset is removed; its body carries a
2417+
# command cross-reference token. No core template exists for "specify",
2418+
# so the skill is restored exclusively via the reconcile override branch.
2419+
overrides_dir = project_dir / ".specify" / "templates" / "overrides"
2420+
overrides_dir.mkdir(parents=True, exist_ok=True)
2421+
(overrides_dir / "speckit.specify.md").write_text(
2422+
"---\ndescription: Override specify\n---\n\n"
2423+
"Then run `__SPECKIT_COMMAND_PLAN__`.\n"
2424+
)
2425+
2426+
preset_dir = self._create_command_preset(
2427+
temp_dir,
2428+
"cmdref-reconcile",
2429+
"speckit.specify",
2430+
"Preset specify",
2431+
"Preset body\n",
2432+
)
2433+
manager = PresetManager(project_dir)
2434+
manager.install_from_directory(preset_dir, "0.1.5")
2435+
manager.remove("cmdref-reconcile")
2436+
2437+
content = (skills_dir / "speckit-specify" / "SKILL.md").read_text()
2438+
assert "override:speckit.specify" in content, "skill should be restored from the project override"
2439+
assert "__SPECKIT_COMMAND_" not in content, "raw command token leaked on reconcile"
2440+
assert "/speckit-plan" in content
2441+
2442+
def test_extension_restore_resolves_command_refs(self, project_dir, temp_dir):
2443+
"""Extension-backed skill restore must resolve command tokens (issue #2717).
2444+
2445+
When a preset override is removed and the skill is restored from an
2446+
extension command body, ``__SPECKIT_COMMAND_*__`` tokens in that body
2447+
must render as slash-command invocations like the core-template path.
2448+
"""
2449+
self._write_init_options(project_dir, ai="claude")
2450+
skills_dir = project_dir / ".claude" / "skills"
2451+
self._create_skill(skills_dir, "speckit-fakeext-cmd", body="original extension skill")
2452+
2453+
extension_dir = project_dir / ".specify" / "extensions" / "fakeext"
2454+
(extension_dir / "commands").mkdir(parents=True, exist_ok=True)
2455+
(extension_dir / "commands" / "cmd.md").write_text(
2456+
"---\ndescription: Extension fakeext cmd\n---\n\n"
2457+
"Then run `__SPECKIT_COMMAND_PLAN__`.\n"
2458+
)
2459+
extension_manifest = {
2460+
"schema_version": "1.0",
2461+
"extension": {
2462+
"id": "fakeext",
2463+
"name": "Fake Extension",
2464+
"version": "1.0.0",
2465+
"description": "Test",
2466+
},
2467+
"requires": {"speckit_version": ">=0.1.0"},
2468+
"provides": {
2469+
"commands": [
2470+
{
2471+
"name": "speckit.fakeext.cmd",
2472+
"file": "commands/cmd.md",
2473+
"description": "Fake extension command",
2474+
}
2475+
]
2476+
},
2477+
}
2478+
with open(extension_dir / "extension.yml", "w") as f:
2479+
yaml.dump(extension_manifest, f)
2480+
2481+
preset_dir = self._create_command_preset(
2482+
temp_dir,
2483+
"cmdref-ext-restore",
2484+
"speckit.fakeext.cmd",
2485+
"Override fakeext cmd",
2486+
"Override body\n",
2487+
)
2488+
manager = PresetManager(project_dir)
2489+
manager.install_from_directory(preset_dir, "0.1.5")
2490+
manager.remove("cmdref-ext-restore")
2491+
2492+
content = (skills_dir / "speckit-fakeext-cmd" / "SKILL.md").read_text()
2493+
assert "source: extension:fakeext" in content, "skill should be restored from the extension"
2494+
assert "__SPECKIT_COMMAND_" not in content, "raw command token leaked on extension restore"
2495+
assert "/speckit-plan" in content
2496+
23492497
def test_core_command_override_skill_uses_preset_command_description(self, project_dir, temp_dir):
23502498
"""Preset skill overrides for core commands should keep preset frontmatter descriptions."""
23512499
self._write_init_options(project_dir, ai="claude")

0 commit comments

Comments
 (0)