fix: strategy wrap correctness

Author: kennedy-whytech

src/specify_cli/__init__.py

@@ -1273,7 +1273,6 @@ def init(
                 "integration": resolved_integration.key,
                 "branch_numbering": branch_numbering or "sequential",
                 "here": here,
-                "preset": preset,
                 "script": selected_script,
                 "speckit_version": get_speckit_version(),
             }

src/specify_cli/agents.py

@@ -416,10 +416,12 @@ def register_commands(
 
             if frontmatter.get("strategy") == "wrap":
                 from .presets import _substitute_core_template
-                short_name = cmd_name
-                if short_name.startswith("speckit."):
-                    short_name = short_name[len("speckit."):]
-                body = _substitute_core_template(body, short_name, project_root, self)
+                body, core_frontmatter = _substitute_core_template(body, cmd_name, project_root, self)
+                frontmatter = dict(frontmatter)
+                for key in ("scripts", "agent_scripts"):
+                    if key not in frontmatter and key in core_frontmatter:
+                        frontmatter[key] = core_frontmatter[key]
+                frontmatter.pop("strategy", None)
 
             frontmatter = self._adjust_script_paths(frontmatter)
 
@@ -442,8 +444,12 @@ def register_commands(
                     agent_name, output_name, frontmatter, body, source_id, cmd_file, project_root
                 )
             elif agent_config["format"] == "markdown":
+                body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root)
+                body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"])
                 output = self.render_markdown_command(frontmatter, body, source_id, context_note)
             elif agent_config["format"] == "toml":
+                body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root)
+                body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"])
                 output = self.render_toml_command(frontmatter, body, source_id)
             else:
                 raise ValueError(f"Unsupported format: {agent_config['format']}")

src/specify_cli/presets.py

@@ -16,7 +16,10 @@
 import shutil
 from dataclasses import dataclass
 from pathlib import Path
-from typing import Optional, Dict, List, Any
+from typing import TYPE_CHECKING, Optional, Dict, List, Any
+
+if TYPE_CHECKING:
+    from .agents import CommandRegistrar
 from datetime import datetime, timezone
 import re
 
@@ -29,31 +32,44 @@
 
 def _substitute_core_template(
     body: str,
-    short_name: str,
+    cmd_name: str,
     project_root: "Path",
     registrar: "CommandRegistrar",
-) -> str:
+) -> "tuple[str, dict]":
     """Substitute {CORE_TEMPLATE} with the body of the installed core command template.
 
     Args:
         body: Preset command body (may contain {CORE_TEMPLATE} placeholder).
-        short_name: Short command name (e.g. "specify" from "speckit.specify").
+        cmd_name: Full command name (e.g. "speckit.git.feature" or "speckit.specify").
         project_root: Project root path.
         registrar: CommandRegistrar instance for parse_frontmatter.
 
     Returns:
-        Body with {CORE_TEMPLATE} replaced by core template body, or body unchanged
-        if the placeholder is absent or the core template file does not exist.
+        A tuple of (body, core_frontmatter) where body has {CORE_TEMPLATE} replaced
+        by the core template body and core_frontmatter holds the core template's parsed
+        frontmatter (so callers can inherit scripts/agent_scripts from it).  Both are
+        unchanged / empty when the placeholder is absent or the core template file does
+        not exist.
     """
     if "{CORE_TEMPLATE}" not in body:
-        return body
+        return body, {}
+
+    # Derive the short name (strip "speckit." prefix) used by core command templates.
+    short_name = cmd_name
+    if short_name.startswith("speckit."):
+        short_name = short_name[len("speckit."):]
 
-    core_file = project_root / ".specify" / "templates" / "commands" / f"{short_name}.md"
-    if not core_file.exists():
-        return body
+    resolver = PresetResolver(project_root)
+    # Try the full command name first so extension commands
+    # (e.g. speckit.git.feature -> extensions/git/commands/speckit.git.feature.md)
+    # are found before falling back to the short name used by core commands
+    # (e.g. specify -> templates/commands/specify.md).
+    core_file = resolver.resolve(cmd_name, "command") or resolver.resolve(short_name, "command")
+    if core_file is None:
+        return body, {}
 
-    _, core_body = registrar.parse_frontmatter(core_file.read_text(encoding="utf-8"))
-    return body.replace("{CORE_TEMPLATE}", core_body)
+    core_frontmatter, core_body = registrar.parse_frontmatter(core_file.read_text(encoding="utf-8"))
+    return body.replace("{CORE_TEMPLATE}", core_body), core_frontmatter
 
 
 @dataclass
@@ -789,7 +805,11 @@ def _register_skills(
             frontmatter, body = registrar.parse_frontmatter(content)
 
             if frontmatter.get("strategy") == "wrap":
-                body = _substitute_core_template(body, short_name, self.project_root, registrar)
+                body, core_frontmatter = _substitute_core_template(body, cmd_name, self.project_root, registrar)
+                frontmatter = dict(frontmatter)
+                for key in ("scripts", "agent_scripts"):
+                    if key not in frontmatter and key in core_frontmatter:
+                        frontmatter[key] = core_frontmatter[key]
 
             original_desc = frontmatter.get("description", "")
             enhanced_desc = SKILL_DESCRIPTIONS.get(

tests/test_presets.py

@@ -3043,6 +3043,7 @@ def test_bundled_preset_missing_locally_cli_error(self, project_dir):
         assert result.exit_code == 1
         output = strip_ansi(result.output).lower()
         assert "bundled" in output, result.output
+        assert "reinstall" in output, result.output
 
 
 class TestWrapStrategy:
@@ -3062,12 +3063,13 @@ def test_substitute_core_template_replaces_placeholder(self, project_dir):
 
         registrar = CommandRegistrar()
         body = "## Pre-Logic\n\nBefore stuff.\n\n{CORE_TEMPLATE}\n\n## Post-Logic\n\nAfter stuff.\n"
-        result = _substitute_core_template(body, "specify", project_dir, registrar)
+        result, core_fm = _substitute_core_template(body, "specify", project_dir, registrar)
 
         assert "{CORE_TEMPLATE}" not in result
         assert "# Core Specify" in result
         assert "## Pre-Logic" in result
         assert "## Post-Logic" in result
+        assert core_fm.get("description") == "core"
 
     def test_substitute_core_template_no_op_when_placeholder_absent(self, project_dir):
         """Returns body unchanged when {CORE_TEMPLATE} is not present."""
@@ -3080,8 +3082,9 @@ def test_substitute_core_template_no_op_when_placeholder_absent(self, project_di
 
         registrar = CommandRegistrar()
         body = "## No placeholder here.\n"
-        result = _substitute_core_template(body, "specify", project_dir, registrar)
+        result, core_fm = _substitute_core_template(body, "specify", project_dir, registrar)
         assert result == body
+        assert core_fm == {}
 
     def test_substitute_core_template_no_op_when_core_missing(self, project_dir):
         """Returns body unchanged when core template file does not exist."""
@@ -3090,9 +3093,10 @@ def test_substitute_core_template_no_op_when_core_missing(self, project_dir):
 
         registrar = CommandRegistrar()
         body = "Pre.\n\n{CORE_TEMPLATE}\n\nPost.\n"
-        result = _substitute_core_template(body, "nonexistent", project_dir, registrar)
+        result, core_fm = _substitute_core_template(body, "nonexistent", project_dir, registrar)
         assert result == body
         assert "{CORE_TEMPLATE}" in result
+        assert core_fm == {}
 
     def test_register_commands_substitutes_core_template_for_wrap_strategy(self, project_dir):
         """register_commands substitutes {CORE_TEMPLATE} when strategy: wrap."""
@@ -3136,7 +3140,8 @@ def test_register_commands_substitutes_core_template_for_wrap_strategy(self, pro
                 project_dir / "preset", project_dir
             )
         finally:
-            registrar.AGENT_CONFIGS = original
+            CommandRegistrar.AGENT_CONFIGS.clear()
+            CommandRegistrar.AGENT_CONFIGS.update(original)
 
         written = (agent_dir / "speckit.specify.md").read_text()
         assert "{CORE_TEMPLATE}" not in written
@@ -3176,3 +3181,207 @@ def test_end_to_end_wrap_via_self_test_preset(self, project_dir):
         assert "# Core Wrap-Test Body" in written
         assert "preset:self-test wrap-pre" in written
         assert "preset:self-test wrap-post" in written
+
+    def test_substitute_core_template_returns_core_scripts(self, project_dir):
+        """core_frontmatter in the returned tuple includes scripts/agent_scripts."""
+        from specify_cli.presets import _substitute_core_template
+        from specify_cli.agents import CommandRegistrar
+
+        core_dir = project_dir / ".specify" / "templates" / "commands"
+        core_dir.mkdir(parents=True, exist_ok=True)
+        (core_dir / "specify.md").write_text(
+            "---\ndescription: core\nscripts:\n  sh: run.sh\nagent_scripts:\n  sh: agent-run.sh\n---\n\n# Body\n"
+        )
+
+        registrar = CommandRegistrar()
+        body = "## Wrapper\n\n{CORE_TEMPLATE}\n"
+        result, core_fm = _substitute_core_template(body, "specify", project_dir, registrar)
+
+        assert "# Body" in result
+        assert core_fm.get("scripts") == {"sh": "run.sh"}
+        assert core_fm.get("agent_scripts") == {"sh": "agent-run.sh"}
+
+    def test_register_skills_inherits_scripts_from_core_when_preset_omits_them(self, project_dir):
+        """_register_skills merges scripts/agent_scripts from core when preset lacks them."""
+        from specify_cli.presets import PresetManager
+        import json
+
+        # Core template with scripts
+        core_dir = project_dir / ".specify" / "templates" / "commands"
+        core_dir.mkdir(parents=True, exist_ok=True)
+        (core_dir / "wrap-test.md").write_text(
+            "---\ndescription: core\nscripts:\n  sh: .specify/scripts/run.sh\n---\n\n"
+            "Run: {SCRIPT}\n"
+        )
+
+        # Skills dir for claude
+        skills_dir = project_dir / ".claude" / "skills"
+        skills_dir.mkdir(parents=True, exist_ok=True)
+        skill_subdir = skills_dir / "speckit-wrap-test"
+        skill_subdir.mkdir()
+        (skill_subdir / "SKILL.md").write_text("---\nname: speckit-wrap-test\n---\n\nold\n")
+
+        (project_dir / ".specify" / "init-options.json").write_text(
+            json.dumps({"ai": "claude", "ai_skills": True})
+        )
+
+        manager = PresetManager(project_dir)
+        manager.install_from_directory(SELF_TEST_PRESET_DIR, "0.1.5")
+
+        written = (skill_subdir / "SKILL.md").read_text()
+        # {SCRIPT} should have been resolved (not left as a literal placeholder)
+        assert "{SCRIPT}" not in written
+
+    def test_register_skills_preset_scripts_take_precedence_over_core(self, project_dir):
+        """preset-defined scripts/agent_scripts are not overwritten by core frontmatter."""
+        from specify_cli.presets import _substitute_core_template
+        from specify_cli.agents import CommandRegistrar
+
+        core_dir = project_dir / ".specify" / "templates" / "commands"
+        core_dir.mkdir(parents=True, exist_ok=True)
+        (core_dir / "specify.md").write_text(
+            "---\ndescription: core\nscripts:\n  sh: core-run.sh\n---\n\nCore body.\n"
+        )
+
+        registrar = CommandRegistrar()
+        body = "{CORE_TEMPLATE}"
+        _, core_fm = _substitute_core_template(body, "specify", project_dir, registrar)
+
+        # Simulate preset frontmatter that already defines scripts
+        preset_fm = {"description": "preset", "strategy": "wrap", "scripts": {"sh": "preset-run.sh"}}
+        for key in ("scripts", "agent_scripts"):
+            if key not in preset_fm and key in core_fm:
+                preset_fm[key] = core_fm[key]
+
+        # Preset's scripts must not be overwritten by core
+        assert preset_fm["scripts"] == {"sh": "preset-run.sh"}
+
+    def test_register_commands_inherits_scripts_from_core(self, project_dir):
+        """register_commands merges scripts/agent_scripts from core and normalizes paths."""
+        from specify_cli.agents import CommandRegistrar
+        import copy
+
+        core_dir = project_dir / ".specify" / "templates" / "commands"
+        core_dir.mkdir(parents=True, exist_ok=True)
+        (core_dir / "specify.md").write_text(
+            "---\ndescription: core\nscripts:\n  sh: .specify/scripts/run.sh {ARGS}\n---\n\n"
+            "Run: {SCRIPT}\n"
+        )
+
+        cmd_dir = project_dir / "preset" / "commands"
+        cmd_dir.mkdir(parents=True, exist_ok=True)
+        # Preset has strategy: wrap but no scripts of its own
+        (cmd_dir / "speckit.specify.md").write_text(
+            "---\ndescription: wrap no scripts\nstrategy: wrap\n---\n\n"
+            "## Pre\n\n{CORE_TEMPLATE}\n\n## Post\n"
+        )
+
+        agent_dir = project_dir / ".claude" / "commands"
+        agent_dir.mkdir(parents=True, exist_ok=True)
+
+        registrar = CommandRegistrar()
+        original = copy.deepcopy(registrar.AGENT_CONFIGS)
+        registrar.AGENT_CONFIGS["test-agent"] = {
+            "dir": str(agent_dir.relative_to(project_dir)),
+            "format": "markdown",
+            "args": "$ARGUMENTS",
+            "extension": ".md",
+            "strip_frontmatter_keys": [],
+        }
+        try:
+            registrar.register_commands(
+                "test-agent",
+                [{"name": "speckit.specify", "file": "commands/speckit.specify.md"}],
+                "test-preset",
+                project_dir / "preset",
+                project_dir,
+            )
+        finally:
+            CommandRegistrar.AGENT_CONFIGS.clear()
+            CommandRegistrar.AGENT_CONFIGS.update(original)
+
+        written = (agent_dir / "speckit.specify.md").read_text()
+        assert "{CORE_TEMPLATE}" not in written
+        assert "Run:" in written
+        assert "scripts:" in written
+        assert "run.sh" in written
+
+    def test_register_commands_toml_resolves_inherited_scripts(self, project_dir):
+        """TOML agents resolve {SCRIPT} from inherited core scripts when preset omits them."""
+        from specify_cli.agents import CommandRegistrar
+        import copy
+
+        core_dir = project_dir / ".specify" / "templates" / "commands"
+        core_dir.mkdir(parents=True, exist_ok=True)
+        (core_dir / "specify.md").write_text(
+            "---\ndescription: core\nscripts:\n  sh: .specify/scripts/run.sh {ARGS}\n---\n\n"
+            "Run: {SCRIPT}\n"
+        )
+
+        cmd_dir = project_dir / "preset" / "commands"
+        cmd_dir.mkdir(parents=True, exist_ok=True)
+        (cmd_dir / "speckit.specify.md").write_text(
+            "---\ndescription: toml wrap\nstrategy: wrap\n---\n\n"
+            "## Pre\n\n{CORE_TEMPLATE}\n\n## Post\n"
+        )
+
+        toml_dir = project_dir / ".gemini" / "commands"
+        toml_dir.mkdir(parents=True, exist_ok=True)
+
+        registrar = CommandRegistrar()
+        original = copy.deepcopy(registrar.AGENT_CONFIGS)
+        registrar.AGENT_CONFIGS["test-toml-agent"] = {
+            "dir": str(toml_dir.relative_to(project_dir)),
+            "format": "toml",
+            "args": "{{args}}",
+            "extension": ".toml",
+            "strip_frontmatter_keys": [],
+        }
+        try:
+            registrar.register_commands(
+                "test-toml-agent",
+                [{"name": "speckit.specify", "file": "commands/speckit.specify.md"}],
+                "test-preset",
+                project_dir / "preset",
+                project_dir,
+            )
+        finally:
+            CommandRegistrar.AGENT_CONFIGS.clear()
+            CommandRegistrar.AGENT_CONFIGS.update(original)
+
+        written = (toml_dir / "speckit.specify.toml").read_text()
+        assert "{CORE_TEMPLATE}" not in written
+        assert "{SCRIPT}" not in written
+        assert "run.sh" in written
+        # args token must use TOML format, not the intermediate $ARGUMENTS
+        assert "$ARGUMENTS" not in written
+        assert "{{args}}" in written
+
+    def test_extension_command_resolves_via_extension_directory(self, project_dir):
+        """Extension commands (e.g. speckit.git.feature) resolve from the extension directory.
+
+        Both _register_skills and register_commands pass the full cmd_name to
+        _substitute_core_template, which tries the full name first via PresetResolver
+        and finds speckit.git.feature.md in the extension commands directory.
+        """
+        from specify_cli.presets import _substitute_core_template
+        from specify_cli.agents import CommandRegistrar
+
+        # Place the template where a real extension would install it
+        ext_cmd_dir = project_dir / ".specify" / "extensions" / "git" / "commands"
+        ext_cmd_dir.mkdir(parents=True, exist_ok=True)
+        (ext_cmd_dir / "speckit.git.feature.md").write_text(
+            "---\ndescription: git feature core\n---\n\n# Git Feature Core\n"
+        )
+        # Ensure a hyphenated or dot-separated fallback does NOT exist
+        assert not (project_dir / ".specify" / "templates" / "commands" / "git.feature.md").exists()
+        assert not (project_dir / ".specify" / "templates" / "commands" / "git-feature.md").exists()
+
+        registrar = CommandRegistrar()
+        body = "## Wrapper\n\n{CORE_TEMPLATE}\n"
+
+        # Both call sites now pass the full cmd_name
+        result, _ = _substitute_core_template(body, "speckit.git.feature", project_dir, registrar)
+
+        assert "# Git Feature Core" in result
+        assert "{CORE_TEMPLATE}" not in result