Skip to content

Commit 4fe7f81

Browse files
fix: inherit core scripts in wrap strategy and resolve extension commands
When strategy: wrap is used in a preset, the core template's {SCRIPT} and {AGENT_SCRIPT} placeholders were left unresolved because _register_skills discarded the core frontmatter after substitution. Presets that add a single instruction around an existing command (e.g. "use Codex for the following") now correctly inherit scripts/agent_scripts from the core so prerequisite checks still run. _substitute_core_template also gains extension command lookup: wrapping speckit.<ext>.<cmd> now searches .specify/extensions/<ext>/commands/ so {CORE_TEMPLATE} resolves for extension commands, not just built-in ones. Also fixes two test issues flagged in code review: - Restore the missing "reinstall" assertion in the bundled preset CLI test - Restore AGENT_CONFIGS on the class (not the instance) after test mutation
1 parent 53220f8 commit 4fe7f81

File tree

3 files changed

+150
-25
lines changed

3 files changed

+150
-25
lines changed

src/specify_cli/agents.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -416,10 +416,7 @@ def register_commands(
416416

417417
if frontmatter.get("strategy") == "wrap":
418418
from .presets import _substitute_core_template
419-
short_name = cmd_name
420-
if short_name.startswith("speckit."):
421-
short_name = short_name[len("speckit."):]
422-
body = _substitute_core_template(body, short_name, project_root, self)
419+
body, _ = _substitute_core_template(body, cmd_name, project_root, self)
423420

424421
frontmatter = self._adjust_script_paths(frontmatter)
425422

src/specify_cli/presets.py

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,44 +29,60 @@
2929

3030
def _substitute_core_template(
3131
body: str,
32-
short_name: str,
32+
cmd_name: str,
3333
project_root: "Path",
3434
registrar: "CommandRegistrar",
35-
) -> str:
35+
) -> "tuple[str, dict]":
3636
"""Substitute {CORE_TEMPLATE} with the body of the installed core command template.
3737
3838
Args:
3939
body: Preset command body (may contain {CORE_TEMPLATE} placeholder).
40-
short_name: Short command name (e.g. "specify" from "speckit.specify").
40+
cmd_name: Full command name (e.g. "speckit.specify" or "speckit.git.feature").
4141
project_root: Project root path.
4242
registrar: CommandRegistrar instance for parse_frontmatter.
4343
4444
Returns:
45-
Body with {CORE_TEMPLATE} replaced by core template body, or body unchanged
46-
if the placeholder is absent or the core template file does not exist.
45+
Tuple of (substituted_body, core_frontmatter). Returns (body, {}) unchanged
46+
when the placeholder is absent or the core template file cannot be found.
4747
"""
4848
if "{CORE_TEMPLATE}" not in body:
49-
return body
49+
return body, {}
5050

51-
# Prefer the project's installed core template so project-level commands and
52-
# customisations are wrapped correctly. Fall back to the bundled core_pack or
53-
# repo templates for commands that ship with the package.
54-
core_file = project_root / ".specify" / "templates" / "commands" / f"{short_name}.md"
51+
# Derive the filename stem used by core-pack and project template lookups.
52+
# "speckit.specify" → "specify"
53+
# "speckit.git.feature" → "git-feature"
54+
raw = cmd_name[len("speckit."):] if cmd_name.startswith("speckit.") else cmd_name
55+
file_stem = raw.replace(".", "-")
56+
57+
# 1. Project-installed core template (always takes priority).
58+
core_file = project_root / ".specify" / "templates" / "commands" / f"{file_stem}.md"
59+
60+
# 2. Extension command: speckit.<ext>.<cmd> → .specify/extensions/<ext>/commands/<cmd_name>.md
61+
if not core_file.exists():
62+
parts = cmd_name.split(".")
63+
if len(parts) >= 3 and parts[0] == "speckit":
64+
ext_id = parts[1]
65+
ext_file = (
66+
project_root / ".specify" / "extensions" / ext_id / "commands" / f"{cmd_name}.md"
67+
)
68+
if ext_file.exists():
69+
core_file = ext_file
70+
71+
# 3. Bundled core pack, then source/editable-install repo templates.
5572
if not core_file.exists():
5673
from specify_cli import _locate_core_pack
5774
core_pack = _locate_core_pack()
5875
if core_pack is not None:
59-
core_file = core_pack / "commands" / f"{short_name}.md"
76+
core_file = core_pack / "commands" / f"{file_stem}.md"
6077
else:
61-
# Source / editable install: look relative to the package root
6278
repo_root = Path(__file__).parent.parent.parent
63-
core_file = repo_root / "templates" / "commands" / f"{short_name}.md"
79+
core_file = repo_root / "templates" / "commands" / f"{file_stem}.md"
6480

6581
if not core_file.exists():
66-
return body
82+
return body, {}
6783

68-
_, core_body = registrar.parse_frontmatter(core_file.read_text(encoding="utf-8"))
69-
return body.replace("{CORE_TEMPLATE}", core_body)
84+
core_frontmatter, core_body = registrar.parse_frontmatter(core_file.read_text(encoding="utf-8"))
85+
return body.replace("{CORE_TEMPLATE}", core_body), core_frontmatter
7086

7187

7288
@dataclass
@@ -802,7 +818,15 @@ def _register_skills(
802818
frontmatter, body = registrar.parse_frontmatter(content)
803819

804820
if frontmatter.get("strategy") == "wrap":
805-
body = _substitute_core_template(body, short_name, self.project_root, registrar)
821+
body, core_fm = _substitute_core_template(body, cmd_name, self.project_root, registrar)
822+
# Inherit script keys from the core template when the preset doesn't
823+
# define its own — required so {SCRIPT}/{AGENT_SCRIPT} placeholders
824+
# embedded in the substituted core body still resolve correctly.
825+
if core_fm:
826+
frontmatter = dict(frontmatter)
827+
for key in ("scripts", "agent_scripts"):
828+
if key not in frontmatter and key in core_fm:
829+
frontmatter[key] = core_fm[key]
806830

807831
original_desc = frontmatter.get("description", "")
808832
enhanced_desc = SKILL_DESCRIPTIONS.get(

tests/test_presets.py

Lines changed: 108 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3043,6 +3043,7 @@ def test_bundled_preset_missing_locally_cli_error(self, project_dir):
30433043
assert result.exit_code == 1
30443044
output = strip_ansi(result.output).lower()
30453045
assert "bundled" in output, result.output
3046+
assert "reinstall" in output, result.output
30463047

30473048

30483049
class TestWrapStrategy:
@@ -3062,7 +3063,7 @@ def test_substitute_core_template_replaces_placeholder(self, project_dir):
30623063

30633064
registrar = CommandRegistrar()
30643065
body = "## Pre-Logic\n\nBefore stuff.\n\n{CORE_TEMPLATE}\n\n## Post-Logic\n\nAfter stuff.\n"
3065-
result = _substitute_core_template(body, "specify", project_dir, registrar)
3066+
result, _ = _substitute_core_template(body, "speckit.specify", project_dir, registrar)
30663067

30673068
assert "{CORE_TEMPLATE}" not in result
30683069
assert "# Core Specify" in result
@@ -3080,7 +3081,7 @@ def test_substitute_core_template_no_op_when_placeholder_absent(self, project_di
30803081

30813082
registrar = CommandRegistrar()
30823083
body = "## No placeholder here.\n"
3083-
result = _substitute_core_template(body, "specify", project_dir, registrar)
3084+
result, _ = _substitute_core_template(body, "speckit.specify", project_dir, registrar)
30843085
assert result == body
30853086

30863087
def test_substitute_core_template_no_op_when_core_missing(self, project_dir):
@@ -3090,9 +3091,10 @@ def test_substitute_core_template_no_op_when_core_missing(self, project_dir):
30903091

30913092
registrar = CommandRegistrar()
30923093
body = "Pre.\n\n{CORE_TEMPLATE}\n\nPost.\n"
3093-
result = _substitute_core_template(body, "nonexistent", project_dir, registrar)
3094+
result, core_fm = _substitute_core_template(body, "speckit.nonexistent", project_dir, registrar)
30943095
assert result == body
30953096
assert "{CORE_TEMPLATE}" in result
3097+
assert core_fm == {}
30963098

30973099
def test_register_commands_substitutes_core_template_for_wrap_strategy(self, project_dir):
30983100
"""register_commands substitutes {CORE_TEMPLATE} when strategy: wrap."""
@@ -3136,7 +3138,7 @@ def test_register_commands_substitutes_core_template_for_wrap_strategy(self, pro
31363138
project_dir / "preset", project_dir
31373139
)
31383140
finally:
3139-
registrar.AGENT_CONFIGS = original
3141+
CommandRegistrar.AGENT_CONFIGS = original
31403142

31413143
written = (agent_dir / "speckit.specify.md").read_text()
31423144
assert "{CORE_TEMPLATE}" not in written
@@ -3176,3 +3178,105 @@ def test_end_to_end_wrap_via_self_test_preset(self, project_dir):
31763178
assert "# Core Wrap-Test Body" in written
31773179
assert "preset:self-test wrap-pre" in written
31783180
assert "preset:self-test wrap-post" in written
3181+
3182+
def test_substitute_core_template_returns_core_frontmatter(self, project_dir):
3183+
"""_substitute_core_template returns core frontmatter as second tuple element."""
3184+
from specify_cli.presets import _substitute_core_template
3185+
from specify_cli.agents import CommandRegistrar
3186+
3187+
core_dir = project_dir / ".specify" / "templates" / "commands"
3188+
core_dir.mkdir(parents=True, exist_ok=True)
3189+
(core_dir / "specify.md").write_text(
3190+
"---\ndescription: core\nscripts:\n sh: scripts/bash/setup.sh\n---\n\n# Core Specify\n"
3191+
)
3192+
3193+
registrar = CommandRegistrar()
3194+
body = "{CORE_TEMPLATE}\n\n## Post\n"
3195+
result_body, core_fm = _substitute_core_template(body, "speckit.specify", project_dir, registrar)
3196+
3197+
assert "{CORE_TEMPLATE}" not in result_body
3198+
assert "# Core Specify" in result_body
3199+
assert core_fm.get("scripts") == {"sh": "scripts/bash/setup.sh"}
3200+
3201+
def test_substitute_core_template_no_core_returns_empty_frontmatter(self, project_dir):
3202+
"""Returns empty dict as frontmatter when core template does not exist."""
3203+
from specify_cli.presets import _substitute_core_template
3204+
from specify_cli.agents import CommandRegistrar
3205+
3206+
registrar = CommandRegistrar()
3207+
body = "{CORE_TEMPLATE}\n\n## Post\n"
3208+
result_body, core_fm = _substitute_core_template(body, "speckit.nonexistent", project_dir, registrar)
3209+
3210+
assert result_body == body
3211+
assert core_fm == {}
3212+
3213+
def test_substitute_core_template_resolves_extension_command(self, project_dir):
3214+
"""_substitute_core_template resolves {CORE_TEMPLATE} from an extension command directory."""
3215+
from specify_cli.presets import _substitute_core_template
3216+
from specify_cli.agents import CommandRegistrar
3217+
3218+
# Simulate an installed extension
3219+
ext_cmd_dir = project_dir / ".specify" / "extensions" / "git" / "commands"
3220+
ext_cmd_dir.mkdir(parents=True, exist_ok=True)
3221+
(ext_cmd_dir / "speckit.git.feature.md").write_text(
3222+
"---\ndescription: git feature core\nhandoffs:\n - label: Next\n agent: speckit.plan\n---\n\n# Core Git Feature\n"
3223+
)
3224+
3225+
registrar = CommandRegistrar()
3226+
body = "## Pre\n\n{CORE_TEMPLATE}\n\n## Post\n"
3227+
result_body, core_fm = _substitute_core_template(
3228+
body, "speckit.git.feature", project_dir, registrar
3229+
)
3230+
3231+
assert "{CORE_TEMPLATE}" not in result_body
3232+
assert "# Core Git Feature" in result_body
3233+
assert core_fm.get("handoffs") is not None
3234+
3235+
def test_wrap_strategy_inherits_core_scripts_in_skills(self, project_dir):
3236+
"""_register_skills merges core scripts into preset frontmatter for {SCRIPT} resolution."""
3237+
import json
3238+
from specify_cli.presets import PresetManager
3239+
from pathlib import Path
3240+
3241+
# Set up core command template with scripts frontmatter
3242+
core_dir = project_dir / ".specify" / "templates" / "commands"
3243+
core_dir.mkdir(parents=True, exist_ok=True)
3244+
(core_dir / "specify.md").write_text(
3245+
"---\ndescription: core specify\nscripts:\n sh: scripts/bash/check.sh\n---\n\n"
3246+
"Run: {SCRIPT}\n\n# Core Specify\n"
3247+
)
3248+
3249+
# Write init-options
3250+
(project_dir / ".specify" / "init-options.json").write_text(
3251+
json.dumps({"ai": "claude", "ai_skills": True})
3252+
)
3253+
3254+
# Create a preset with a wrap command that has no scripts of its own
3255+
preset_dir = project_dir / "test-preset"
3256+
(preset_dir / "commands").mkdir(parents=True)
3257+
(preset_dir / "commands" / "speckit.specify.md").write_text(
3258+
"---\ndescription: preset wrap\nstrategy: wrap\n---\n\n{CORE_TEMPLATE}\n\n## Post\n"
3259+
)
3260+
(preset_dir / "preset.yml").write_text(
3261+
"schema_version: '1.0'\n"
3262+
"preset:\n id: test-scripts-preset\n name: Test\n version: 1.0.0\n"
3263+
" description: Test\n author: test\n"
3264+
"requires:\n speckit_version: '>=0.5.0'\n"
3265+
"provides:\n templates:\n"
3266+
" - type: command\n name: speckit.specify\n"
3267+
" file: commands/speckit.specify.md\n description: wrap\n"
3268+
)
3269+
3270+
# Create the skill dir so _register_skills will overwrite it
3271+
skills_dir = project_dir / ".claude" / "skills" / "speckit-specify"
3272+
skills_dir.mkdir(parents=True)
3273+
(skills_dir / "SKILL.md").write_text("---\nname: speckit-specify\n---\n\nold\n")
3274+
3275+
manager = PresetManager(project_dir)
3276+
manager.install_from_directory(preset_dir, "1.0.0")
3277+
3278+
written = (skills_dir / "SKILL.md").read_text()
3279+
# {SCRIPT} should have been resolved using the core template's scripts
3280+
assert "{SCRIPT}" not in written
3281+
assert "scripts/bash/check.sh" in written
3282+

0 commit comments

Comments
 (0)