Skip to content

Commit 296a7b3

Browse files
fix: resolve merge conflict for strategy wrap correctness
1 parent 4ba8216 commit 296a7b3

File tree

4 files changed

+257
-25
lines changed

4 files changed

+257
-25
lines changed

src/specify_cli/__init__.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1371,7 +1371,6 @@ def init(
13711371
"branch_numbering": branch_numbering or "sequential",
13721372
"context_file": resolved_integration.context_file,
13731373
"here": here,
1374-
"preset": preset,
13751374
"script": selected_script,
13761375
"speckit_version": get_speckit_version(),
13771376
}

src/specify_cli/agents.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -447,10 +447,12 @@ def register_commands(
447447

448448
if frontmatter.get("strategy") == "wrap":
449449
from .presets import _substitute_core_template
450-
short_name = cmd_name
451-
if short_name.startswith("speckit."):
452-
short_name = short_name[len("speckit."):]
453-
body = _substitute_core_template(body, short_name, project_root, self)
450+
body, core_frontmatter = _substitute_core_template(body, cmd_name, project_root, self)
451+
frontmatter = dict(frontmatter)
452+
for key in ("scripts", "agent_scripts"):
453+
if key not in frontmatter and key in core_frontmatter:
454+
frontmatter[key] = core_frontmatter[key]
455+
frontmatter.pop("strategy", None)
454456

455457
frontmatter = self._adjust_script_paths(frontmatter)
456458

@@ -479,10 +481,12 @@ def register_commands(
479481
project_root,
480482
)
481483
elif agent_config["format"] == "markdown":
482-
output = self.render_markdown_command(
483-
frontmatter, body, source_id, context_note
484-
)
484+
body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root)
485+
body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"])
486+
output = self.render_markdown_command(frontmatter, body, source_id, context_note)
485487
elif agent_config["format"] == "toml":
488+
body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root)
489+
body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"])
486490
output = self.render_toml_command(frontmatter, body, source_id)
487491
elif agent_config["format"] == "yaml":
488492
output = self.render_yaml_command(

src/specify_cli/presets.py

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@
1616
import shutil
1717
from dataclasses import dataclass
1818
from pathlib import Path
19-
from typing import Optional, Dict, List, Any
19+
from typing import TYPE_CHECKING, Optional, Dict, List, Any
20+
21+
if TYPE_CHECKING:
22+
from .agents import CommandRegistrar
2023
from datetime import datetime, timezone
2124
import re
2225

@@ -29,31 +32,44 @@
2932

3033
def _substitute_core_template(
3134
body: str,
32-
short_name: str,
35+
cmd_name: str,
3336
project_root: "Path",
3437
registrar: "CommandRegistrar",
35-
) -> str:
38+
) -> "tuple[str, dict]":
3639
"""Substitute {CORE_TEMPLATE} with the body of the installed core command template.
3740
3841
Args:
3942
body: Preset command body (may contain {CORE_TEMPLATE} placeholder).
40-
short_name: Short command name (e.g. "specify" from "speckit.specify").
43+
cmd_name: Full command name (e.g. "speckit.git.feature" or "speckit.specify").
4144
project_root: Project root path.
4245
registrar: CommandRegistrar instance for parse_frontmatter.
4346
4447
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.
48+
A tuple of (body, core_frontmatter) where body has {CORE_TEMPLATE} replaced
49+
by the core template body and core_frontmatter holds the core template's parsed
50+
frontmatter (so callers can inherit scripts/agent_scripts from it). Both are
51+
unchanged / empty when the placeholder is absent or the core template file does
52+
not exist.
4753
"""
4854
if "{CORE_TEMPLATE}" not in body:
49-
return body
55+
return body, {}
56+
57+
# Derive the short name (strip "speckit." prefix) used by core command templates.
58+
short_name = cmd_name
59+
if short_name.startswith("speckit."):
60+
short_name = short_name[len("speckit."):]
5061

51-
core_file = project_root / ".specify" / "templates" / "commands" / f"{short_name}.md"
52-
if not core_file.exists():
53-
return body
62+
resolver = PresetResolver(project_root)
63+
# Try the full command name first so extension commands
64+
# (e.g. speckit.git.feature -> extensions/git/commands/speckit.git.feature.md)
65+
# are found before falling back to the short name used by core commands
66+
# (e.g. specify -> templates/commands/specify.md).
67+
core_file = resolver.resolve(cmd_name, "command") or resolver.resolve(short_name, "command")
68+
if core_file is None:
69+
return body, {}
5470

55-
_, core_body = registrar.parse_frontmatter(core_file.read_text(encoding="utf-8"))
56-
return body.replace("{CORE_TEMPLATE}", core_body)
71+
core_frontmatter, core_body = registrar.parse_frontmatter(core_file.read_text(encoding="utf-8"))
72+
return body.replace("{CORE_TEMPLATE}", core_body), core_frontmatter
5773

5874

5975
@dataclass
@@ -791,7 +807,11 @@ def _register_skills(
791807
frontmatter, body = registrar.parse_frontmatter(content)
792808

793809
if frontmatter.get("strategy") == "wrap":
794-
body = _substitute_core_template(body, short_name, self.project_root, registrar)
810+
body, core_frontmatter = _substitute_core_template(body, cmd_name, self.project_root, registrar)
811+
frontmatter = dict(frontmatter)
812+
for key in ("scripts", "agent_scripts"):
813+
if key not in frontmatter and key in core_frontmatter:
814+
frontmatter[key] = core_frontmatter[key]
795815

796816
original_desc = frontmatter.get("description", "")
797817
enhanced_desc = SKILL_DESCRIPTIONS.get(

tests/test_presets.py

Lines changed: 213 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3040,6 +3040,7 @@ def test_bundled_preset_missing_locally_cli_error(self, project_dir):
30403040
assert result.exit_code == 1
30413041
output = strip_ansi(result.output).lower()
30423042
assert "bundled" in output, result.output
3043+
assert "reinstall" in output, result.output
30433044

30443045

30453046
class TestWrapStrategy:
@@ -3059,12 +3060,13 @@ def test_substitute_core_template_replaces_placeholder(self, project_dir):
30593060

30603061
registrar = CommandRegistrar()
30613062
body = "## Pre-Logic\n\nBefore stuff.\n\n{CORE_TEMPLATE}\n\n## Post-Logic\n\nAfter stuff.\n"
3062-
result = _substitute_core_template(body, "specify", project_dir, registrar)
3063+
result, core_fm = _substitute_core_template(body, "specify", project_dir, registrar)
30633064

30643065
assert "{CORE_TEMPLATE}" not in result
30653066
assert "# Core Specify" in result
30663067
assert "## Pre-Logic" in result
30673068
assert "## Post-Logic" in result
3069+
assert core_fm.get("description") == "core"
30683070

30693071
def test_substitute_core_template_no_op_when_placeholder_absent(self, project_dir):
30703072
"""Returns body unchanged when {CORE_TEMPLATE} is not present."""
@@ -3077,8 +3079,9 @@ def test_substitute_core_template_no_op_when_placeholder_absent(self, project_di
30773079

30783080
registrar = CommandRegistrar()
30793081
body = "## No placeholder here.\n"
3080-
result = _substitute_core_template(body, "specify", project_dir, registrar)
3082+
result, core_fm = _substitute_core_template(body, "specify", project_dir, registrar)
30813083
assert result == body
3084+
assert core_fm == {}
30823085

30833086
def test_substitute_core_template_no_op_when_core_missing(self, project_dir):
30843087
"""Returns body unchanged when core template file does not exist."""
@@ -3087,9 +3090,10 @@ def test_substitute_core_template_no_op_when_core_missing(self, project_dir):
30873090

30883091
registrar = CommandRegistrar()
30893092
body = "Pre.\n\n{CORE_TEMPLATE}\n\nPost.\n"
3090-
result = _substitute_core_template(body, "nonexistent", project_dir, registrar)
3093+
result, core_fm = _substitute_core_template(body, "nonexistent", project_dir, registrar)
30913094
assert result == body
30923095
assert "{CORE_TEMPLATE}" in result
3096+
assert core_fm == {}
30933097

30943098
def test_register_commands_substitutes_core_template_for_wrap_strategy(self, project_dir):
30953099
"""register_commands substitutes {CORE_TEMPLATE} when strategy: wrap."""
@@ -3133,7 +3137,8 @@ def test_register_commands_substitutes_core_template_for_wrap_strategy(self, pro
31333137
project_dir / "preset", project_dir
31343138
)
31353139
finally:
3136-
registrar.AGENT_CONFIGS = original
3140+
CommandRegistrar.AGENT_CONFIGS.clear()
3141+
CommandRegistrar.AGENT_CONFIGS.update(original)
31373142

31383143
written = (agent_dir / "speckit.specify.md").read_text()
31393144
assert "{CORE_TEMPLATE}" not in written
@@ -3173,3 +3178,207 @@ def test_end_to_end_wrap_via_self_test_preset(self, project_dir):
31733178
assert "# Core Wrap-Test Body" in written
31743179
assert "preset:self-test wrap-pre" in written
31753180
assert "preset:self-test wrap-post" in written
3181+
3182+
def test_substitute_core_template_returns_core_scripts(self, project_dir):
3183+
"""core_frontmatter in the returned tuple includes scripts/agent_scripts."""
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: run.sh\nagent_scripts:\n sh: agent-run.sh\n---\n\n# Body\n"
3191+
)
3192+
3193+
registrar = CommandRegistrar()
3194+
body = "## Wrapper\n\n{CORE_TEMPLATE}\n"
3195+
result, core_fm = _substitute_core_template(body, "specify", project_dir, registrar)
3196+
3197+
assert "# Body" in result
3198+
assert core_fm.get("scripts") == {"sh": "run.sh"}
3199+
assert core_fm.get("agent_scripts") == {"sh": "agent-run.sh"}
3200+
3201+
def test_register_skills_inherits_scripts_from_core_when_preset_omits_them(self, project_dir):
3202+
"""_register_skills merges scripts/agent_scripts from core when preset lacks them."""
3203+
from specify_cli.presets import PresetManager
3204+
import json
3205+
3206+
# Core template with scripts
3207+
core_dir = project_dir / ".specify" / "templates" / "commands"
3208+
core_dir.mkdir(parents=True, exist_ok=True)
3209+
(core_dir / "wrap-test.md").write_text(
3210+
"---\ndescription: core\nscripts:\n sh: .specify/scripts/run.sh\n---\n\n"
3211+
"Run: {SCRIPT}\n"
3212+
)
3213+
3214+
# Skills dir for claude
3215+
skills_dir = project_dir / ".claude" / "skills"
3216+
skills_dir.mkdir(parents=True, exist_ok=True)
3217+
skill_subdir = skills_dir / "speckit-wrap-test"
3218+
skill_subdir.mkdir()
3219+
(skill_subdir / "SKILL.md").write_text("---\nname: speckit-wrap-test\n---\n\nold\n")
3220+
3221+
(project_dir / ".specify" / "init-options.json").write_text(
3222+
json.dumps({"ai": "claude", "ai_skills": True})
3223+
)
3224+
3225+
manager = PresetManager(project_dir)
3226+
manager.install_from_directory(SELF_TEST_PRESET_DIR, "0.1.5")
3227+
3228+
written = (skill_subdir / "SKILL.md").read_text()
3229+
# {SCRIPT} should have been resolved (not left as a literal placeholder)
3230+
assert "{SCRIPT}" not in written
3231+
3232+
def test_register_skills_preset_scripts_take_precedence_over_core(self, project_dir):
3233+
"""preset-defined scripts/agent_scripts are not overwritten by core frontmatter."""
3234+
from specify_cli.presets import _substitute_core_template
3235+
from specify_cli.agents import CommandRegistrar
3236+
3237+
core_dir = project_dir / ".specify" / "templates" / "commands"
3238+
core_dir.mkdir(parents=True, exist_ok=True)
3239+
(core_dir / "specify.md").write_text(
3240+
"---\ndescription: core\nscripts:\n sh: core-run.sh\n---\n\nCore body.\n"
3241+
)
3242+
3243+
registrar = CommandRegistrar()
3244+
body = "{CORE_TEMPLATE}"
3245+
_, core_fm = _substitute_core_template(body, "specify", project_dir, registrar)
3246+
3247+
# Simulate preset frontmatter that already defines scripts
3248+
preset_fm = {"description": "preset", "strategy": "wrap", "scripts": {"sh": "preset-run.sh"}}
3249+
for key in ("scripts", "agent_scripts"):
3250+
if key not in preset_fm and key in core_fm:
3251+
preset_fm[key] = core_fm[key]
3252+
3253+
# Preset's scripts must not be overwritten by core
3254+
assert preset_fm["scripts"] == {"sh": "preset-run.sh"}
3255+
3256+
def test_register_commands_inherits_scripts_from_core(self, project_dir):
3257+
"""register_commands merges scripts/agent_scripts from core and normalizes paths."""
3258+
from specify_cli.agents import CommandRegistrar
3259+
import copy
3260+
3261+
core_dir = project_dir / ".specify" / "templates" / "commands"
3262+
core_dir.mkdir(parents=True, exist_ok=True)
3263+
(core_dir / "specify.md").write_text(
3264+
"---\ndescription: core\nscripts:\n sh: .specify/scripts/run.sh {ARGS}\n---\n\n"
3265+
"Run: {SCRIPT}\n"
3266+
)
3267+
3268+
cmd_dir = project_dir / "preset" / "commands"
3269+
cmd_dir.mkdir(parents=True, exist_ok=True)
3270+
# Preset has strategy: wrap but no scripts of its own
3271+
(cmd_dir / "speckit.specify.md").write_text(
3272+
"---\ndescription: wrap no scripts\nstrategy: wrap\n---\n\n"
3273+
"## Pre\n\n{CORE_TEMPLATE}\n\n## Post\n"
3274+
)
3275+
3276+
agent_dir = project_dir / ".claude" / "commands"
3277+
agent_dir.mkdir(parents=True, exist_ok=True)
3278+
3279+
registrar = CommandRegistrar()
3280+
original = copy.deepcopy(registrar.AGENT_CONFIGS)
3281+
registrar.AGENT_CONFIGS["test-agent"] = {
3282+
"dir": str(agent_dir.relative_to(project_dir)),
3283+
"format": "markdown",
3284+
"args": "$ARGUMENTS",
3285+
"extension": ".md",
3286+
"strip_frontmatter_keys": [],
3287+
}
3288+
try:
3289+
registrar.register_commands(
3290+
"test-agent",
3291+
[{"name": "speckit.specify", "file": "commands/speckit.specify.md"}],
3292+
"test-preset",
3293+
project_dir / "preset",
3294+
project_dir,
3295+
)
3296+
finally:
3297+
CommandRegistrar.AGENT_CONFIGS.clear()
3298+
CommandRegistrar.AGENT_CONFIGS.update(original)
3299+
3300+
written = (agent_dir / "speckit.specify.md").read_text()
3301+
assert "{CORE_TEMPLATE}" not in written
3302+
assert "Run:" in written
3303+
assert "scripts:" in written
3304+
assert "run.sh" in written
3305+
3306+
def test_register_commands_toml_resolves_inherited_scripts(self, project_dir):
3307+
"""TOML agents resolve {SCRIPT} from inherited core scripts when preset omits them."""
3308+
from specify_cli.agents import CommandRegistrar
3309+
import copy
3310+
3311+
core_dir = project_dir / ".specify" / "templates" / "commands"
3312+
core_dir.mkdir(parents=True, exist_ok=True)
3313+
(core_dir / "specify.md").write_text(
3314+
"---\ndescription: core\nscripts:\n sh: .specify/scripts/run.sh {ARGS}\n---\n\n"
3315+
"Run: {SCRIPT}\n"
3316+
)
3317+
3318+
cmd_dir = project_dir / "preset" / "commands"
3319+
cmd_dir.mkdir(parents=True, exist_ok=True)
3320+
(cmd_dir / "speckit.specify.md").write_text(
3321+
"---\ndescription: toml wrap\nstrategy: wrap\n---\n\n"
3322+
"## Pre\n\n{CORE_TEMPLATE}\n\n## Post\n"
3323+
)
3324+
3325+
toml_dir = project_dir / ".gemini" / "commands"
3326+
toml_dir.mkdir(parents=True, exist_ok=True)
3327+
3328+
registrar = CommandRegistrar()
3329+
original = copy.deepcopy(registrar.AGENT_CONFIGS)
3330+
registrar.AGENT_CONFIGS["test-toml-agent"] = {
3331+
"dir": str(toml_dir.relative_to(project_dir)),
3332+
"format": "toml",
3333+
"args": "{{args}}",
3334+
"extension": ".toml",
3335+
"strip_frontmatter_keys": [],
3336+
}
3337+
try:
3338+
registrar.register_commands(
3339+
"test-toml-agent",
3340+
[{"name": "speckit.specify", "file": "commands/speckit.specify.md"}],
3341+
"test-preset",
3342+
project_dir / "preset",
3343+
project_dir,
3344+
)
3345+
finally:
3346+
CommandRegistrar.AGENT_CONFIGS.clear()
3347+
CommandRegistrar.AGENT_CONFIGS.update(original)
3348+
3349+
written = (toml_dir / "speckit.specify.toml").read_text()
3350+
assert "{CORE_TEMPLATE}" not in written
3351+
assert "{SCRIPT}" not in written
3352+
assert "run.sh" in written
3353+
# args token must use TOML format, not the intermediate $ARGUMENTS
3354+
assert "$ARGUMENTS" not in written
3355+
assert "{{args}}" in written
3356+
3357+
def test_extension_command_resolves_via_extension_directory(self, project_dir):
3358+
"""Extension commands (e.g. speckit.git.feature) resolve from the extension directory.
3359+
3360+
Both _register_skills and register_commands pass the full cmd_name to
3361+
_substitute_core_template, which tries the full name first via PresetResolver
3362+
and finds speckit.git.feature.md in the extension commands directory.
3363+
"""
3364+
from specify_cli.presets import _substitute_core_template
3365+
from specify_cli.agents import CommandRegistrar
3366+
3367+
# Place the template where a real extension would install it
3368+
ext_cmd_dir = project_dir / ".specify" / "extensions" / "git" / "commands"
3369+
ext_cmd_dir.mkdir(parents=True, exist_ok=True)
3370+
(ext_cmd_dir / "speckit.git.feature.md").write_text(
3371+
"---\ndescription: git feature core\n---\n\n# Git Feature Core\n"
3372+
)
3373+
# Ensure a hyphenated or dot-separated fallback does NOT exist
3374+
assert not (project_dir / ".specify" / "templates" / "commands" / "git.feature.md").exists()
3375+
assert not (project_dir / ".specify" / "templates" / "commands" / "git-feature.md").exists()
3376+
3377+
registrar = CommandRegistrar()
3378+
body = "## Wrapper\n\n{CORE_TEMPLATE}\n"
3379+
3380+
# Both call sites now pass the full cmd_name
3381+
result, _ = _substitute_core_template(body, "speckit.git.feature", project_dir, registrar)
3382+
3383+
assert "# Git Feature Core" in result
3384+
assert "{CORE_TEMPLATE}" not in result

0 commit comments

Comments
 (0)