From 4ba8216ed080f2333957267f88d56481597f8e46 Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Sun, 12 Apr 2026 10:22:52 -0400 Subject: [PATCH 1/4] feat: implement strategy: wrap --- presets/README.md | 2 +- .../self-test/commands/speckit.wrap-test.md | 14 ++ presets/self-test/preset.yml | 5 + src/specify_cli/agents.py | 7 + src/specify_cli/presets.py | 32 +++++ tests/test_presets.py | 136 +++++++++++++++++- 6 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 presets/self-test/commands/speckit.wrap-test.md diff --git a/presets/README.md b/presets/README.md index dd3997b239..72751b4bfb 100644 --- a/presets/README.md +++ b/presets/README.md @@ -116,5 +116,5 @@ The following enhancements are under consideration for future releases: | **command** | ✓ (default) | ✓ | ✓ | ✓ | | **script** | ✓ (default) | — | — | ✓ | - For artifacts and commands (which are LLM directives), `wrap` would inject preset content before and after the core template using a `{CORE_TEMPLATE}` placeholder. For scripts, `wrap` would run custom logic before/after the core script via a `$CORE_SCRIPT` variable. + For artifacts and commands (which are LLM directives), `wrap` injects preset content before and after the core template using a `{CORE_TEMPLATE}` placeholder (implemented). For scripts, `wrap` would run custom logic before/after the core script via a `$CORE_SCRIPT` variable (not yet implemented). - **Script overrides** — Enable presets to provide alternative versions of core scripts (e.g. `create-new-feature.sh`) for workflow customization. A `strategy: "wrap"` option could allow presets to run custom logic before/after the core script without fully replacing it. diff --git a/presets/self-test/commands/speckit.wrap-test.md b/presets/self-test/commands/speckit.wrap-test.md new file mode 100644 index 0000000000..78ace30ea8 --- /dev/null +++ b/presets/self-test/commands/speckit.wrap-test.md @@ -0,0 +1,14 @@ +--- +description: "Self-test wrap command — pre/post around core" +strategy: wrap +--- + +## Preset Pre-Logic + +preset:self-test wrap-pre + +{CORE_TEMPLATE} + +## Preset Post-Logic + +preset:self-test wrap-post diff --git a/presets/self-test/preset.yml b/presets/self-test/preset.yml index 82c7b068ad..8e718430aa 100644 --- a/presets/self-test/preset.yml +++ b/presets/self-test/preset.yml @@ -56,6 +56,11 @@ provides: description: "Self-test override of the specify command" replaces: "speckit.specify" + - type: "command" + name: "speckit.wrap-test" + file: "commands/speckit.wrap-test.md" + description: "Self-test wrap strategy command" + tags: - "testing" - "self-test" diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 1a0e5a8317..61526ddf90 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -445,6 +445,13 @@ def register_commands( content = source_file.read_text(encoding="utf-8") frontmatter, body = self.parse_frontmatter(content) + 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) + frontmatter = self._adjust_script_paths(frontmatter) for key in agent_config.get("strip_frontmatter_keys", []): diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index d5513c8323..8b051ca22e 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -27,6 +27,35 @@ from .extensions import ExtensionRegistry, normalize_priority +def _substitute_core_template( + body: str, + short_name: str, + project_root: "Path", + registrar: "CommandRegistrar", +) -> str: + """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"). + 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. + """ + if "{CORE_TEMPLATE}" not in body: + return body + + core_file = project_root / ".specify" / "templates" / "commands" / f"{short_name}.md" + if not core_file.exists(): + return body + + _, core_body = registrar.parse_frontmatter(core_file.read_text(encoding="utf-8")) + return body.replace("{CORE_TEMPLATE}", core_body) + + @dataclass class PresetCatalogEntry: """Represents a single entry in the preset catalog stack.""" @@ -761,6 +790,9 @@ def _register_skills( content = source_file.read_text(encoding="utf-8") frontmatter, body = registrar.parse_frontmatter(content) + if frontmatter.get("strategy") == "wrap": + body = _substitute_core_template(body, short_name, self.project_root, registrar) + original_desc = frontmatter.get("description", "") enhanced_desc = SKILL_DESCRIPTIONS.get( short_name, diff --git a/tests/test_presets.py b/tests/test_presets.py index 35c19bdd7f..6d034424bc 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1665,7 +1665,7 @@ def test_self_test_manifest_valid(self): assert manifest.id == "self-test" assert manifest.name == "Self-Test Preset" assert manifest.version == "1.0.0" - assert len(manifest.templates) == 7 # 6 templates + 1 command + assert len(manifest.templates) == 8 # 6 templates + 2 commands def test_self_test_provides_all_core_templates(self): """Verify the self-test preset provides an override for every core template.""" @@ -3040,4 +3040,136 @@ 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: + """Tests for strategy: wrap preset command substitution.""" + + def test_substitute_core_template_replaces_placeholder(self, project_dir): + """Core template body replaces {CORE_TEMPLATE} in preset command body.""" + from specify_cli.presets import _substitute_core_template + from specify_cli.agents import CommandRegistrar + + # Set up a core command template + core_dir = project_dir / ".specify" / "templates" / "commands" + core_dir.mkdir(parents=True, exist_ok=True) + (core_dir / "specify.md").write_text( + "---\ndescription: core\n---\n\n# Core Specify\n\nDo the thing.\n" + ) + + 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) + + assert "{CORE_TEMPLATE}" not in result + assert "# Core Specify" in result + assert "## Pre-Logic" in result + assert "## Post-Logic" in result + + def test_substitute_core_template_no_op_when_placeholder_absent(self, project_dir): + """Returns body unchanged when {CORE_TEMPLATE} is not present.""" + 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\n---\n\nCore body.\n") + + registrar = CommandRegistrar() + body = "## No placeholder here.\n" + result = _substitute_core_template(body, "specify", project_dir, registrar) + assert result == body + + def test_substitute_core_template_no_op_when_core_missing(self, project_dir): + """Returns body unchanged when core template file does not exist.""" + from specify_cli.presets import _substitute_core_template + from specify_cli.agents import CommandRegistrar + + registrar = CommandRegistrar() + body = "Pre.\n\n{CORE_TEMPLATE}\n\nPost.\n" + result = _substitute_core_template(body, "nonexistent", project_dir, registrar) + assert result == body + assert "{CORE_TEMPLATE}" in result + + def test_register_commands_substitutes_core_template_for_wrap_strategy(self, project_dir): + """register_commands substitutes {CORE_TEMPLATE} when strategy: wrap.""" + from specify_cli.agents import CommandRegistrar + + # Set up core command template + core_dir = project_dir / ".specify" / "templates" / "commands" + core_dir.mkdir(parents=True, exist_ok=True) + (core_dir / "specify.md").write_text( + "---\ndescription: core\n---\n\n# Core Specify\n\nCore body here.\n" + ) + + # Create a preset command dir with a wrap-strategy command + cmd_dir = project_dir / "preset" / "commands" + cmd_dir.mkdir(parents=True, exist_ok=True) + (cmd_dir / "speckit.specify.md").write_text( + "---\ndescription: wrap test\nstrategy: wrap\n---\n\n" + "## Pre\n\n{CORE_TEMPLATE}\n\n## Post\n" + ) + + commands = [{"name": "speckit.specify", "file": "commands/speckit.specify.md"}] + registrar = CommandRegistrar() + + # Use a generic agent that writes markdown to commands/ + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=True, exist_ok=True) + + # Patch AGENT_CONFIGS to use a simple markdown agent pointing at our dir + import copy + 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", commands, "test-preset", + project_dir / "preset", project_dir + ) + finally: + registrar.AGENT_CONFIGS = original + + written = (agent_dir / "speckit.specify.md").read_text() + assert "{CORE_TEMPLATE}" not in written + assert "# Core Specify" in written + assert "## Pre" in written + assert "## Post" in written + + def test_end_to_end_wrap_via_self_test_preset(self, project_dir): + """Installing self-test preset with a wrap command substitutes {CORE_TEMPLATE}.""" + from specify_cli.presets import PresetManager + + # Install a core template that wrap-test will wrap around + core_dir = project_dir / ".specify" / "templates" / "commands" + core_dir.mkdir(parents=True, exist_ok=True) + (core_dir / "wrap-test.md").write_text( + "---\ndescription: core wrap-test\n---\n\n# Core Wrap-Test Body\n" + ) + + # Set up skills dir (simulating --ai 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 content\n") + + # Write init-options so _register_skills finds the claude skills dir + import json + (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() + assert "{CORE_TEMPLATE}" not in written + assert "# Core Wrap-Test Body" in written + assert "preset:self-test wrap-pre" in written + assert "preset:self-test wrap-post" in written From 296a7b345c37d77bf1a6acc4030d48333ef8c6b2 Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Sat, 18 Apr 2026 12:41:17 -0400 Subject: [PATCH 2/4] fix: resolve merge conflict for strategy wrap correctness --- src/specify_cli/__init__.py | 1 - src/specify_cli/agents.py | 18 +-- src/specify_cli/presets.py | 46 +++++--- tests/test_presets.py | 217 +++++++++++++++++++++++++++++++++++- 4 files changed, 257 insertions(+), 25 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 8c6fd02b9f..5a849e7c23 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1371,7 +1371,6 @@ def init( "branch_numbering": branch_numbering or "sequential", "context_file": resolved_integration.context_file, "here": here, - "preset": preset, "script": selected_script, "speckit_version": get_speckit_version(), } diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 61526ddf90..e4bcd0fe08 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -447,10 +447,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) @@ -479,10 +481,12 @@ def register_commands( project_root, ) elif agent_config["format"] == "markdown": - output = self.render_markdown_command( - frontmatter, body, source_id, context_note - ) + 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) elif agent_config["format"] == "yaml": output = self.render_yaml_command( diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 8b051ca22e..06c96aac49 100644 --- a/src/specify_cli/presets.py +++ b/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 @@ -791,7 +807,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( diff --git a/tests/test_presets.py b/tests/test_presets.py index 6d034424bc..6397c5bb63 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3040,6 +3040,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: @@ -3059,12 +3060,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.""" @@ -3077,8 +3079,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.""" @@ -3087,9 +3090,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.""" @@ -3133,7 +3137,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 @@ -3173,3 +3178,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 From 7c643f93d9f5b5fc9505bb9ee6efb2fb558f6cf5 Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Sat, 18 Apr 2026 12:23:53 -0400 Subject: [PATCH 3/4] feat: multi-preset composable wrapping with priority ordering Implements comment #4 from PR review: multiple installed wrap presets now compose in priority order rather than overwriting each other. Key changes: - PresetResolver.resolve() gains skip_presets flag; resolve_core() wraps it to skip tier 2, preventing accidental nesting during replay - _replay_wraps_for_command() recomposed all enabled wrap presets for a command in ascending priority order (innermost-first) after any install or remove - _replay_skill_override() keeps SKILL.md in sync with the recomposed command body for ai-skills-enabled projects - install_from_directory() detects strategy: wrap commands, stores wrap_commands in the registry entry, and calls replay after install - remove() reads wrap_commands before deletion, removes registry entry before rmtree so replay sees post-removal state, then replays remaining wraps or unregisters when none remain Tests: TestResolveCore (5), TestReplayWrapsForCommand (5), TestInstallRemoveWrapLifecycle (5), plus 2 skill/alias regression tests --- src/specify_cli/presets.py | 282 ++++++++++++++- tests/test_presets.py | 690 +++++++++++++++++++++++++++++++++++++ 2 files changed, 966 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 06c96aac49..b0d6ce0587 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -64,7 +64,9 @@ def _substitute_core_template( # (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") + # Use resolve_core() to skip installed presets (tier 2), preventing accidental + # nesting where another preset's wrap output is mistaken for the real core. + core_file = resolver.resolve_core(cmd_name, "command") or resolver.resolve_core(short_name, "command") if core_file is None: return body, {} @@ -600,6 +602,202 @@ def _unregister_commands(self, registered_commands: Dict[str, List[str]]) -> Non registrar = CommandRegistrar() registrar.unregister_commands(registered_commands, self.project_root) + def _replay_wraps_for_command(self, cmd_name: str) -> None: + """Recompose and rewrite agent files for a wrap-strategy command. + + Collects all installed presets that declare cmd_name in their + wrap_commands registry field, sorts them so the highest-precedence + preset (lowest priority number) wraps outermost, then writes the + fully composed output to every agent directory. + + Called after every install and remove to keep agent files correct + regardless of installation order. + + Args: + cmd_name: Full command name (e.g. "speckit.specify") + """ + try: + from .agents import CommandRegistrar + except ImportError: + return + + # Collect enabled presets that wrap this command, sorted ascending + # (lowest priority number = highest precedence = outermost). + wrap_presets = [] + for pack_id, metadata in self.registry.list_by_priority(include_disabled=False): + if cmd_name not in metadata.get("wrap_commands", []): + continue + pack_dir = self.presets_dir / pack_id + if not pack_dir.is_dir(): + continue # corrupted state — skip + wrap_presets.append((pack_id, pack_dir)) + + if not wrap_presets: + return + + # Derive short name for core resolution fallback. + short_name = cmd_name + if short_name.startswith("speckit."): + short_name = short_name[len("speckit."):] + + resolver = PresetResolver(self.project_root) + core_file = ( + resolver.resolve_core(cmd_name, "command") + or resolver.resolve_core(short_name, "command") + ) + if core_file is None: + return + + registrar = CommandRegistrar() + core_frontmatter, core_body = registrar.parse_frontmatter( + core_file.read_text(encoding="utf-8") + ) + replay_aliases: List[str] = [] + seen_aliases: set[str] = set() + + # Apply wraps innermost-first (reverse of ascending list). + accumulated_body = core_body + outermost_frontmatter = {} + for pack_id, pack_dir in reversed(wrap_presets): + cmd_file = pack_dir / "commands" / f"{cmd_name}.md" + if not cmd_file.exists(): + continue + manifest_path = pack_dir / "preset.yml" + if manifest_path.exists(): + manifest = PresetManifest(manifest_path) + for template in manifest.templates: + if template.get("type") != "command" or template.get("name") != cmd_name: + continue + aliases = template.get("aliases", []) + if not isinstance(aliases, list): + aliases = [] + for alias in aliases: + if isinstance(alias, str) and alias not in seen_aliases: + replay_aliases.append(alias) + seen_aliases.add(alias) + wrap_fm, wrap_body = registrar.parse_frontmatter( + cmd_file.read_text(encoding="utf-8") + ) + accumulated_body = wrap_body.replace("{CORE_TEMPLATE}", accumulated_body) + outermost_frontmatter = wrap_fm # last iteration = outermost preset + + # Build final frontmatter: outermost preset wins; fall back to core for + # scripts/agent_scripts if the outermost preset does not define them. + final_frontmatter = dict(outermost_frontmatter) + final_frontmatter.pop("strategy", None) + for key in ("scripts", "agent_scripts"): + if key not in final_frontmatter and key in core_frontmatter: + final_frontmatter[key] = core_frontmatter[key] + + outermost_pack_id = wrap_presets[-1][0] + composed_content = ( + registrar.render_frontmatter(final_frontmatter) + "\n" + accumulated_body + ) + + self._replay_skill_override(cmd_name, composed_content, outermost_pack_id) + + with tempfile.TemporaryDirectory() as tmpdir: + tmp_path = Path(tmpdir) + cmd_dir = tmp_path / "commands" + cmd_dir.mkdir() + (cmd_dir / f"{cmd_name}.md").write_text(composed_content, encoding="utf-8") + registrar._ensure_configs() + for agent_name, agent_config in registrar.AGENT_CONFIGS.items(): + if agent_config.get("extension") == "/SKILL.md": + continue + agent_dir = self.project_root / agent_config["dir"] + if not agent_dir.exists(): + continue + try: + registrar.register_commands( + agent_name, + [{ + "name": cmd_name, + "file": f"commands/{cmd_name}.md", + "aliases": replay_aliases, + }], + f"preset:{outermost_pack_id}", + tmp_path, + self.project_root, + ) + except ValueError: + continue + + def _replay_skill_override( + self, + cmd_name: str, + composed_content: str, + outermost_pack_id: str, + ) -> None: + """Rewrite any active SKILL.md override for a replayed wrap command.""" + skills_dir = self._get_skills_dir() + if not skills_dir: + return + + from . import SKILL_DESCRIPTIONS, load_init_options + from .agents import CommandRegistrar + + init_opts = load_init_options(self.project_root) + if not isinstance(init_opts, dict): + init_opts = {} + selected_ai = init_opts.get("ai") + if not isinstance(selected_ai, str): + return + + registrar = CommandRegistrar() + agent_config = registrar.AGENT_CONFIGS.get(selected_ai, {}) + create_missing_skills = bool(init_opts.get("ai_skills")) and agent_config.get("extension") != "/SKILL.md" + + skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name) + target_skill_names: List[str] = [] + if (skills_dir / skill_name).is_dir(): + target_skill_names.append(skill_name) + if legacy_skill_name != skill_name and (skills_dir / legacy_skill_name).is_dir(): + target_skill_names.append(legacy_skill_name) + if not target_skill_names and create_missing_skills: + missing_skill_dir = skills_dir / skill_name + if not missing_skill_dir.exists(): + target_skill_names.append(skill_name) + if not target_skill_names: + return + + raw_short_name = cmd_name + if raw_short_name.startswith("speckit."): + raw_short_name = raw_short_name[len("speckit."):] + short_name = raw_short_name.replace(".", "-") + skill_title = self._skill_title_from_command(cmd_name) + + frontmatter, body = registrar.parse_frontmatter(composed_content) + original_desc = frontmatter.get("description", "") + enhanced_desc = SKILL_DESCRIPTIONS.get( + short_name, + original_desc or f"Spec-kit workflow command: {short_name}", + ) + body = registrar.resolve_skill_placeholders( + selected_ai, dict(frontmatter), body, self.project_root + ) + + for target_skill_name in target_skill_names: + skill_subdir = skills_dir / target_skill_name + if skill_subdir.exists() and not skill_subdir.is_dir(): + continue + skill_subdir.mkdir(parents=True, exist_ok=True) + frontmatter_data = registrar.build_skill_frontmatter( + selected_ai, + target_skill_name, + enhanced_desc, + f"preset:{outermost_pack_id}", + ) + frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() + skill_content = ( + f"---\n" + f"{frontmatter_text}\n" + f"---\n\n" + f"# Speckit {skill_title} Skill\n\n" + f"{body}\n" + ) + (skill_subdir / "SKILL.md").write_text(skill_content, encoding="utf-8") + def _get_skills_dir(self) -> Optional[Path]: """Return the active skills directory for preset skill overrides. @@ -1026,6 +1224,24 @@ def install_from_directory( # Update corresponding skills when --ai-skills was previously used registered_skills = self._register_skills(manifest, dest_dir) + # Detect wrap commands before registry.add() so a read failure doesn't + # leave a partially-committed registry entry. + wrap_commands = [] + try: + from .agents import CommandRegistrar as _CR + _registrar = _CR() + for cmd_tmpl in manifest.templates: + if cmd_tmpl.get("type") != "command": + continue + cmd_file = dest_dir / cmd_tmpl["file"] + if not cmd_file.exists(): + continue + cmd_fm, _ = _registrar.parse_frontmatter(cmd_file.read_text(encoding="utf-8")) + if cmd_fm.get("strategy") == "wrap": + wrap_commands.append(cmd_tmpl["name"]) + except ImportError: + pass + self.registry.add(manifest.id, { "version": manifest.version, "source": "local", @@ -1034,8 +1250,12 @@ def install_from_directory( "priority": priority, "registered_commands": registered_commands, "registered_skills": registered_skills, + "wrap_commands": wrap_commands, }) + for cmd_name in wrap_commands: + self._replay_wraps_for_command(cmd_name) + return manifest def install_from_zip( @@ -1110,9 +1330,16 @@ def remove(self, pack_id: str) -> bool: # Restore original skills when preset is removed registered_skills = metadata.get("registered_skills", []) if metadata else [] registered_commands = metadata.get("registered_commands", {}) if metadata else {} + wrap_commands = metadata.get("wrap_commands", []) if metadata else [] pack_dir = self.presets_dir / pack_id + + # _unregister_skills must run before directory deletion (reads preset files) if registered_skills: self._unregister_skills(registered_skills, pack_dir) + # When _unregister_skills has already handled skill-agent files, strip + # those entries from registered_commands to avoid double-deletion. + # (When registered_skills is empty, skill-agent entries in + # registered_commands are the only deletion path for those files.) try: from .agents import CommandRegistrar except ImportError: @@ -1124,14 +1351,42 @@ def remove(self, pack_id: str) -> bool: if CommandRegistrar.AGENT_CONFIGS.get(agent_name, {}).get("extension") != "/SKILL.md" } - # Unregister non-skill command files from AI agents. - if registered_commands: - self._unregister_commands(registered_commands) + # Remove from registry BEFORE deleting the directory so that + # _replay_wraps_for_command sees the post-removal registry state. + self.registry.remove(pack_id) if pack_dir.exists(): shutil.rmtree(pack_dir) - self.registry.remove(pack_id) + # Separate wrap commands from non-wrap commands in registered_commands. + non_wrap_commands = { + agent_name: [c for c in cmd_names if c not in wrap_commands] + for agent_name, cmd_names in registered_commands.items() + } + non_wrap_commands = {k: v for k, v in non_wrap_commands.items() if v} + + # Unregister non-wrap command files from AI agents. + if non_wrap_commands: + self._unregister_commands(non_wrap_commands) + + # For each wrapped command, either re-compose remaining wraps or delete. + for cmd_name in wrap_commands: + remaining = [ + pid for pid, meta in self.registry.list().items() + if cmd_name in meta.get("wrap_commands", []) + ] + if remaining: + self._replay_wraps_for_command(cmd_name) + else: + # No wrap presets remain — delete the agent file entirely. + wrap_agent_commands = { + agent_name: [c for c in cmd_names if c == cmd_name] + for agent_name, cmd_names in registered_commands.items() + } + wrap_agent_commands = {k: v for k, v in wrap_agent_commands.items() if v} + if wrap_agent_commands: + self._unregister_commands(wrap_agent_commands) + return True def list_installed(self) -> List[Dict[str, Any]]: @@ -1787,6 +2042,7 @@ def resolve( self, template_name: str, template_type: str = "template", + skip_presets: bool = False, ) -> Optional[Path]: """Resolve a template name to its file path. @@ -1795,6 +2051,8 @@ def resolve( Args: template_name: Template name (e.g., "spec-template") template_type: Template type ("template", "command", or "script") + skip_presets: When True, skip tier 2 (installed presets). Use + resolve_core() as the preferred caller-facing API for this. Returns: Path to the resolved template file, or None if not found @@ -1823,7 +2081,7 @@ def resolve( return override # Priority 2: Installed presets (sorted by priority — lower number wins) - if self.presets_dir.exists(): + if not skip_presets and self.presets_dir.exists(): registry = PresetRegistry(self.presets_dir) for pack_id, _metadata in registry.list_by_priority(): pack_dir = self.presets_dir / pack_id @@ -1864,6 +2122,18 @@ def resolve( return None + def resolve_core( + self, + template_name: str, + template_type: str = "template", + ) -> Optional[Path]: + """Resolve against tiers 1, 3, and 4 only — skipping installed presets. + + Use when resolving {CORE_TEMPLATE} to guarantee the result is actual + base content, never another preset's wrap output. + """ + return self.resolve(template_name, template_type, skip_presets=True) + def resolve_with_source( self, template_name: str, diff --git a/tests/test_presets.py b/tests/test_presets.py index 6397c5bb63..3679d04ecf 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -999,6 +999,65 @@ def test_resolve_skips_hidden_extension_dirs(self, project_dir): assert result is None +class TestResolveCore: + """Test PresetResolver.resolve_core() skips the installed-presets tier.""" + + def test_resolve_core_does_not_return_preset_files(self, project_dir): + """resolve_core must not return files from .specify/presets/.""" + preset_cmd_dir = project_dir / ".specify" / "presets" / "my-preset" / "commands" + preset_cmd_dir.mkdir(parents=True) + (preset_cmd_dir / "specify.md").write_text("---\ndescription: preset wrap\n---\n\nwrap body\n") + + resolver = PresetResolver(project_dir) + result = resolver.resolve_core("specify", "command") + assert result is None + + def test_resolve_core_returns_core_template(self, project_dir): + """resolve_core falls through to core templates (tier 4).""" + core_cmd_dir = project_dir / ".specify" / "templates" / "commands" + core_cmd_dir.mkdir(parents=True, exist_ok=True) + (core_cmd_dir / "specify.md").write_text("---\ndescription: core\n---\n\ncore body\n") + + # Also place a preset file — resolve_core must still return the core + preset_cmd_dir = project_dir / ".specify" / "presets" / "my-preset" / "commands" + preset_cmd_dir.mkdir(parents=True) + (preset_cmd_dir / "specify.md").write_text("---\ndescription: preset wrap\n---\n\nwrap body\n") + + resolver = PresetResolver(project_dir) + result = resolver.resolve_core("specify", "command") + assert result is not None + assert "presets" not in str(result) + assert "templates/commands/specify.md" in str(result) + + def test_resolve_core_returns_override(self, project_dir): + """resolve_core returns tier-1 override if present.""" + override_dir = project_dir / ".specify" / "templates" / "overrides" + override_dir.mkdir(parents=True) + (override_dir / "specify.md").write_text("---\ndescription: override\n---\n\noverride body\n") + + resolver = PresetResolver(project_dir) + result = resolver.resolve_core("specify", "command") + assert result is not None + assert "overrides/specify.md" in str(result) + + def test_resolve_core_returns_extension_template(self, project_dir): + """resolve_core returns extension templates (tier 3).""" + ext_cmd_dir = project_dir / ".specify" / "extensions" / "myext" / "commands" + ext_cmd_dir.mkdir(parents=True) + (ext_cmd_dir / "myext-cmd.md").write_text("---\ndescription: ext\n---\n\next body\n") + + resolver = PresetResolver(project_dir) + result = resolver.resolve_core("myext-cmd", "command") + assert result is not None + assert "extensions/myext/commands" in str(result) + + def test_resolve_core_returns_none_when_nothing_found(self, project_dir): + """resolve_core returns None when no file found in tiers 1/3/4.""" + resolver = PresetResolver(project_dir) + result = resolver.resolve_core("nonexistent", "command") + assert result is None + + class TestExtensionPriorityResolution: """Test extension priority resolution with registered and unregistered extensions.""" @@ -3354,6 +3413,109 @@ def test_register_commands_toml_resolves_inherited_scripts(self, project_dir): assert "$ARGUMENTS" not in written assert "{{args}}" in written + def test_register_commands_markdown_resolves_inherited_scripts(self, project_dir): + """Markdown 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: markdown wrap\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-md-agent"] = { + "dir": str(agent_dir.relative_to(project_dir)), + "format": "markdown", + "args": "$ARGUMENTS", + "extension": ".md", + "strip_frontmatter_keys": [], + } + try: + registrar.register_commands( + "test-md-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 "{SCRIPT}" not in written + assert "run.sh" in written + assert "strategy" not in written + + def test_register_commands_markdown_converts_args_after_script_resolution(self, project_dir): + """Markdown agents re-run arg placeholder conversion after resolve_skill_placeholders. + + resolve_skill_placeholders injects $ARGUMENTS (via {ARGS} expansion). A second + _convert_argument_placeholder call must convert those to the agent's native format. + """ + 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: forge wrap\nstrategy: wrap\n---\n\n" + "## Pre\n\n{CORE_TEMPLATE}\n\n## Post\n" + ) + + agent_dir = project_dir / ".forge" / "commands" + agent_dir.mkdir(parents=True, exist_ok=True) + + registrar = CommandRegistrar() + original = copy.deepcopy(registrar.AGENT_CONFIGS) + registrar.AGENT_CONFIGS["test-forge-agent"] = { + "dir": str(agent_dir.relative_to(project_dir)), + "format": "markdown", + "args": "{{parameters}}", + "extension": ".md", + "strip_frontmatter_keys": [], + } + try: + registrar.register_commands( + "test-forge-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 "{SCRIPT}" not in written + assert "run.sh" in written + # $ARGUMENTS injected by resolve_skill_placeholders must be re-converted + assert "$ARGUMENTS" not in written + assert "{{parameters}}" 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. @@ -3382,3 +3544,531 @@ def test_extension_command_resolves_via_extension_directory(self, project_dir): assert "# Git Feature Core" in result assert "{CORE_TEMPLATE}" not in result + + +# ===== _replay_wraps_for_command Tests ===== + +def _make_wrap_preset_dir( + base: Path, + preset_id: str, + cmd_name: str, + pre: str, + post: str, + aliases: list[str] | None = None, +) -> Path: + """Create a minimal wrap-strategy preset directory for testing.""" + preset_dir = base / preset_id + cmd_dir = preset_dir / "commands" + cmd_dir.mkdir(parents=True) + short = cmd_name.split(".")[-1] + template = { + "type": "command", + "name": cmd_name, + "file": f"commands/{cmd_name}.md", + "description": f"{preset_id} wrap", + } + if aliases is not None: + template["aliases"] = aliases + manifest = { + "schema_version": "1.0", + "preset": { + "id": preset_id, + "name": preset_id, + "version": "1.0.0", + "description": f"Preset {preset_id}", + "author": "test", + "repository": "https://example.com", + "license": "MIT", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [template] + }, + "tags": [], + } + import yaml as _yaml + (preset_dir / "preset.yml").write_text(_yaml.dump(manifest)) + (cmd_dir / f"{cmd_name}.md").write_text( + f"---\ndescription: {preset_id} wrap\nstrategy: wrap\n---\n\n" + f"[{pre}]\n\n{{CORE_TEMPLATE}}\n\n[{post}]\n" + ) + return preset_dir + + +class TestReplayWrapsForCommand: + """Tests for PresetManager._replay_wraps_for_command().""" + + def test_replay_no_op_when_no_wrap_presets(self, project_dir): + """replay does nothing when no presets declare wrap_commands for the command.""" + manager = PresetManager(project_dir) + # Should not raise + manager._replay_wraps_for_command("speckit.specify") + + def test_replay_no_op_when_core_missing(self, project_dir, temp_dir): + """replay exits gracefully when resolve_core returns None.""" + from specify_cli.agents import CommandRegistrar + import copy + + preset_dir = _make_wrap_preset_dir(temp_dir, "preset-a", "speckit.specify", "pre-a", "post-a") + installed = project_dir / ".specify" / "presets" / "preset-a" + import shutil as _shutil + _shutil.copytree(preset_dir, installed) + + manager = PresetManager(project_dir) + manager.registry.add("preset-a", { + "version": "1.0.0", "source": "local", "enabled": True, + "priority": 10, "manifest_hash": "x", + "registered_commands": {}, "registered_skills": [], + "wrap_commands": ["speckit.specify"], + }) + + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=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: + # No core file exists — replay should return without writing + manager._replay_wraps_for_command("speckit.specify") + finally: + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) + + assert not (agent_dir / "speckit.specify.md").exists() + + def test_replay_single_preset_writes_composed_output(self, project_dir, temp_dir): + """Single wrap preset: replay writes pre + core + post to agent dirs.""" + from specify_cli.agents import CommandRegistrar + import copy, shutil as _shutil + + # Core template + core_dir = project_dir / ".specify" / "templates" / "commands" + core_dir.mkdir(parents=True, exist_ok=True) + (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\ncore body\n") + + # Install preset-a + preset_dir = _make_wrap_preset_dir(temp_dir, "preset-a", "speckit.specify", "pre-a", "post-a") + installed = project_dir / ".specify" / "presets" / "preset-a" + _shutil.copytree(preset_dir, installed) + + manager = PresetManager(project_dir) + manager.registry.add("preset-a", { + "version": "1.0.0", "source": "local", "enabled": True, + "priority": 10, "manifest_hash": "x", + "registered_commands": {}, "registered_skills": [], + "wrap_commands": ["speckit.specify"], + }) + + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=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: + manager._replay_wraps_for_command("speckit.specify") + finally: + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) + + written = (agent_dir / "speckit.specify.md").read_text() + assert "[pre-a]" in written + assert "core body" in written + assert "[post-a]" in written + assert "{CORE_TEMPLATE}" not in written + assert "strategy" not in written + + def test_replay_priority_order_lower_number_outermost(self, project_dir, temp_dir): + """Two wrap presets: lower priority number = outermost wrapper.""" + from specify_cli.agents import CommandRegistrar + import copy, shutil as _shutil + + core_dir = project_dir / ".specify" / "templates" / "commands" + core_dir.mkdir(parents=True, exist_ok=True) + (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") + + for pid in ("preset-outer", "preset-inner"): + src = _make_wrap_preset_dir(temp_dir, pid, "speckit.specify", f"pre-{pid}", f"post-{pid}") + _shutil.copytree(src, project_dir / ".specify" / "presets" / pid) + + manager = PresetManager(project_dir) + # preset-outer has priority 1 (highest precedence = outermost) + # preset-inner has priority 10 (lowest precedence = innermost) + for pid, pri in (("preset-outer", 1), ("preset-inner", 10)): + manager.registry.add(pid, { + "version": "1.0.0", "source": "local", "enabled": True, + "priority": pri, "manifest_hash": "x", + "registered_commands": {}, "registered_skills": [], + "wrap_commands": ["speckit.specify"], + }) + + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=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: + manager._replay_wraps_for_command("speckit.specify") + finally: + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) + + written = (agent_dir / "speckit.specify.md").read_text() + # Outermost (preset-outer, p=1) wraps everything; innermost (preset-inner, p=10) is next + outer_pre = written.index("[pre-preset-outer]") + inner_pre = written.index("[pre-preset-inner]") + core_pos = written.index("CORE") + inner_post = written.index("[post-preset-inner]") + outer_post = written.index("[post-preset-outer]") + assert outer_pre < inner_pre < core_pos < inner_post < outer_post + + def test_replay_install_order_independent(self, project_dir, temp_dir): + """Nesting order is determined by priority, not install order.""" + from specify_cli.agents import CommandRegistrar + import copy, shutil as _shutil + + core_dir = project_dir / ".specify" / "templates" / "commands" + core_dir.mkdir(parents=True, exist_ok=True) + (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") + + for pid in ("preset-a", "preset-b"): + src = _make_wrap_preset_dir(temp_dir, pid, "speckit.specify", f"pre-{pid}", f"post-{pid}") + _shutil.copytree(src, project_dir / ".specify" / "presets" / pid) + + manager = PresetManager(project_dir) + # preset-a priority=5 (outermost), preset-b priority=10 (innermost) + # Install in reverse order to verify install order doesn't affect nesting + for pid, pri in (("preset-b", 10), ("preset-a", 5)): + manager.registry.add(pid, { + "version": "1.0.0", "source": "local", "enabled": True, + "priority": pri, "manifest_hash": "x", + "registered_commands": {}, "registered_skills": [], + "wrap_commands": ["speckit.specify"], + }) + + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=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: + manager._replay_wraps_for_command("speckit.specify") + finally: + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) + + written = (agent_dir / "speckit.specify.md").read_text() + a_pre = written.index("[pre-preset-a]") + b_pre = written.index("[pre-preset-b]") + core_pos = written.index("CORE") + b_post = written.index("[post-preset-b]") + a_post = written.index("[post-preset-a]") + # preset-a (p=5) is outermost regardless of install order + assert a_pre < b_pre < core_pos < b_post < a_post + + def test_replay_updates_skill_outputs(self, project_dir, temp_dir): + """Replay also rewrites SKILL.md-backed agent outputs.""" + import json + import shutil as _shutil + + core_dir = project_dir / ".specify" / "templates" / "commands" + core_dir.mkdir(parents=True, exist_ok=True) + (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") + + preset_dir = _make_wrap_preset_dir(temp_dir, "preset-a", "speckit.specify", "pre-a", "post-a") + _shutil.copytree(preset_dir, project_dir / ".specify" / "presets" / "preset-a") + + manager = PresetManager(project_dir) + manager.registry.add("preset-a", { + "version": "1.0.0", "source": "local", "enabled": True, + "priority": 10, "manifest_hash": "x", + "registered_commands": {}, "registered_skills": [], + "wrap_commands": ["speckit.specify"], + }) + + skills_dir = project_dir / ".claude" / "skills" + skill_subdir = skills_dir / "speckit-specify" + skill_subdir.mkdir(parents=True) + (skill_subdir / "SKILL.md").write_text("---\nname: speckit-specify\n---\n\nold\n") + (project_dir / ".specify" / "init-options.json").write_text( + json.dumps({"ai": "claude", "ai_skills": True}) + ) + + manager._replay_wraps_for_command("speckit.specify") + + written = (skill_subdir / "SKILL.md").read_text() + assert "[pre-a]" in written + assert "CORE" in written + assert "[post-a]" in written + + +class TestInstallRemoveWrapLifecycle: + """Tests for wrap_commands stored on install and replayed on remove.""" + + def _setup_agent(self, project_dir, registrar, agent_configs_dict): + """Register a test markdown agent and return its commands dir.""" + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=True, exist_ok=True) + agent_configs_dict["test-agent"] = { + "dir": str(agent_dir.relative_to(project_dir)), + "format": "markdown", "args": "$ARGUMENTS", + "extension": ".md", "strip_frontmatter_keys": [], + } + return agent_dir + + def test_install_stores_wrap_commands_in_registry(self, project_dir, temp_dir): + """install_from_directory stores wrap_commands in the registry entry.""" + 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\n---\n\ncore\n") + + preset_src = _make_wrap_preset_dir(temp_dir, "preset-a", "speckit.specify", "pre", "post") + + registrar = CommandRegistrar() + original = copy.deepcopy(registrar.AGENT_CONFIGS) + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=True, exist_ok=True) + registrar.AGENT_CONFIGS["test-agent"] = { + "dir": str(agent_dir.relative_to(project_dir)), + "format": "markdown", "args": "$ARGUMENTS", + "extension": ".md", "strip_frontmatter_keys": [], + } + try: + manager = PresetManager(project_dir) + manager.install_from_directory(preset_src, "0.1.0", priority=10) + finally: + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) + + meta = manager.registry.get("preset-a") + assert "wrap_commands" in meta + assert "speckit.specify" in meta["wrap_commands"] + + def test_install_replay_produces_correct_nested_output(self, project_dir, temp_dir): + """After installing two wrap presets, agent file contains correctly nested output.""" + from specify_cli.agents import CommandRegistrar + import copy, shutil as _shutil + + core_dir = project_dir / ".specify" / "templates" / "commands" + core_dir.mkdir(parents=True, exist_ok=True) + (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") + + registrar = CommandRegistrar() + original = copy.deepcopy(registrar.AGENT_CONFIGS) + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=True, exist_ok=True) + registrar.AGENT_CONFIGS["test-agent"] = { + "dir": str(agent_dir.relative_to(project_dir)), + "format": "markdown", "args": "$ARGUMENTS", + "extension": ".md", "strip_frontmatter_keys": [], + } + try: + manager = PresetManager(project_dir) + # Install outermost first (priority=5), then innermost (priority=10) + outer_src = _make_wrap_preset_dir(temp_dir, "preset-outer", "speckit.specify", "OUTER-PRE", "OUTER-POST") + # Rename to avoid id conflict with fixture + inner_src = _make_wrap_preset_dir(temp_dir, "preset-inner", "speckit.specify", "INNER-PRE", "INNER-POST") + manager.install_from_directory(outer_src, "0.1.0", priority=5) + manager.install_from_directory(inner_src, "0.1.0", priority=10) + finally: + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) + + written = (agent_dir / "speckit.specify.md").read_text() + outer_pre = written.index("OUTER-PRE") + inner_pre = written.index("INNER-PRE") + core_pos = written.index("CORE") + inner_post = written.index("INNER-POST") + outer_post = written.index("OUTER-POST") + assert outer_pre < inner_pre < core_pos < inner_post < outer_post + + def test_remove_replays_remaining_wraps(self, project_dir, temp_dir): + """Removing one wrap preset re-composes the remaining wraps correctly.""" + 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\n---\n\nCORE\n") + + registrar = CommandRegistrar() + original = copy.deepcopy(registrar.AGENT_CONFIGS) + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=True, exist_ok=True) + registrar.AGENT_CONFIGS["test-agent"] = { + "dir": str(agent_dir.relative_to(project_dir)), + "format": "markdown", "args": "$ARGUMENTS", + "extension": ".md", "strip_frontmatter_keys": [], + } + try: + manager = PresetManager(project_dir) + outer_src = _make_wrap_preset_dir(temp_dir, "preset-outer", "speckit.specify", "OUTER-PRE", "OUTER-POST") + inner_src = _make_wrap_preset_dir(temp_dir, "preset-inner", "speckit.specify", "INNER-PRE", "INNER-POST") + manager.install_from_directory(outer_src, "0.1.0", priority=5) + manager.install_from_directory(inner_src, "0.1.0", priority=10) + manager.remove("preset-outer") + finally: + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) + + written = (agent_dir / "speckit.specify.md").read_text() + # Only inner wrap remains — should be: INNER-PRE + CORE + INNER-POST, no OUTER + assert "INNER-PRE" in written + assert "CORE" in written + assert "INNER-POST" in written + assert "OUTER-PRE" not in written + assert "OUTER-POST" not in written + + def test_wrap_aliases_are_replayed_and_removed(self, project_dir, temp_dir): + """Replay preserves wrap aliases across install/remove lifecycle changes.""" + 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\n---\n\nCORE\n") + + registrar = CommandRegistrar() + original = copy.deepcopy(registrar.AGENT_CONFIGS) + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=True, exist_ok=True) + registrar.AGENT_CONFIGS["test-agent"] = { + "dir": str(agent_dir.relative_to(project_dir)), + "format": "markdown", "args": "$ARGUMENTS", + "extension": ".md", "strip_frontmatter_keys": [], + } + try: + manager = PresetManager(project_dir) + outer_src = _make_wrap_preset_dir( + temp_dir, + "preset-outer", + "speckit.specify", + "OUTER-PRE", + "OUTER-POST", + aliases=["speckit.alias"], + ) + inner_src = _make_wrap_preset_dir( + temp_dir, "preset-inner", "speckit.specify", "INNER-PRE", "INNER-POST" + ) + manager.install_from_directory(outer_src, "0.1.0", priority=5) + manager.install_from_directory(inner_src, "0.1.0", priority=10) + + alias_file = agent_dir / "speckit.alias.md" + written = alias_file.read_text() + assert "OUTER-PRE" in written + assert "INNER-PRE" in written + assert "INNER-POST" in written + assert "OUTER-POST" in written + + manager.remove("preset-inner") + written = alias_file.read_text() + assert "OUTER-PRE" in written + assert "OUTER-POST" in written + assert "INNER-PRE" not in written + assert "INNER-POST" not in written + + manager.remove("preset-outer") + finally: + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) + + assert not (agent_dir / "speckit.alias.md").exists() + + def test_remove_last_wrap_preset_deletes_agent_file(self, project_dir, temp_dir): + """Removing the only wrap preset deletes the agent command file.""" + 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\n---\n\nCORE\n") + + registrar = CommandRegistrar() + original = copy.deepcopy(registrar.AGENT_CONFIGS) + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=True, exist_ok=True) + registrar.AGENT_CONFIGS["test-agent"] = { + "dir": str(agent_dir.relative_to(project_dir)), + "format": "markdown", "args": "$ARGUMENTS", + "extension": ".md", "strip_frontmatter_keys": [], + } + try: + manager = PresetManager(project_dir) + src = _make_wrap_preset_dir(temp_dir, "preset-only", "speckit.specify", "PRE", "POST") + manager.install_from_directory(src, "0.1.0", priority=10) + assert (agent_dir / "speckit.specify.md").exists() + manager.remove("preset-only") + finally: + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) + + assert not (agent_dir / "speckit.specify.md").exists() + + def test_non_wrap_commands_unaffected_by_wrap_lifecycle(self, project_dir, temp_dir): + """wrap_commands is empty for a preset with no strategy:wrap commands.""" + from specify_cli.agents import CommandRegistrar + import copy + import yaml as _yaml + + # Create a preset with a non-wrap command + preset_dir = temp_dir / "non-wrap-preset" + cmd_dir = preset_dir / "commands" + cmd_dir.mkdir(parents=True) + manifest = { + "schema_version": "1.0", + "preset": { + "id": "non-wrap-preset", "name": "Non-wrap", "version": "1.0.0", + "description": "no wrap", "author": "test", + "repository": "https://example.com", "license": "MIT", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"templates": [ + {"type": "command", "name": "speckit.specify", + "file": "commands/speckit.specify.md", "description": "override"}, + ]}, + "tags": [], + } + (preset_dir / "preset.yml").write_text(_yaml.dump(manifest)) + (cmd_dir / "speckit.specify.md").write_text( + "---\ndescription: plain override\n---\n\nplain body\n" + ) + + registrar = CommandRegistrar() + original = copy.deepcopy(registrar.AGENT_CONFIGS) + agent_dir = project_dir / ".claude" / "commands" + agent_dir.mkdir(parents=True, exist_ok=True) + registrar.AGENT_CONFIGS["test-agent"] = { + "dir": str(agent_dir.relative_to(project_dir)), + "format": "markdown", "args": "$ARGUMENTS", + "extension": ".md", "strip_frontmatter_keys": [], + } + try: + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.0", priority=10) + finally: + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) + + meta = manager.registry.get("non-wrap-preset") + assert meta.get("wrap_commands", []) == [] + written = (agent_dir / "speckit.specify.md").read_text() + assert "plain body" in written From 2b600869fe6fe84b36b3c5615526bcf35b650397 Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Sat, 18 Apr 2026 12:56:47 -0400 Subject: [PATCH 4/4] fix: resolve extension commands via manifest file mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PresetResolver.resolve_extension_command_via_manifest() consults each installed extension.yml to find the actual file declared for a command name, rather than assuming the file is named .md. This fixes _substitute_core_template for extensions like selftest where the manifest maps speckit.selftest.extension → commands/selftest.md. Resolution order in _substitute_core_template is now: 1. resolve_core(cmd_name) — project overrides win, then name-based lookup 2. resolve_extension_command_via_manifest(cmd_name) — manifest fallback 3. resolve_core(short_name) — core template short-name fallback Path traversal guard mirrors the containment check already present in ExtensionManager to reject absolute paths or paths escaping the extension root. --- src/specify_cli/presets.py | 71 ++++++++++++++++++++++++++++++++++---- tests/test_presets.py | 39 +++++++++++++++++++++ 2 files changed, 103 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index b0d6ce0587..a7b89b8820 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -60,13 +60,22 @@ def _substitute_core_template( short_name = short_name[len("speckit."):] 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). - # Use resolve_core() to skip installed presets (tier 2), preventing accidental - # nesting where another preset's wrap output is mistaken for the real core. - core_file = resolver.resolve_core(cmd_name, "command") or resolver.resolve_core(short_name, "command") + # Resolution order for the core template: + # 1. resolve_core(cmd_name) — covers tier-1 project overrides and tier-3/4 + # name-based lookup (file named .md). Checked first so that a + # local override always wins, even for extension commands. + # 2. resolve_extension_command_via_manifest(cmd_name) — manifest-based tier-3 + # fallback for extension commands whose file is named differently from the + # command name (e.g. speckit.selftest.extension → commands/selftest.md). + # 3. resolve_core(short_name) — core template fallback using the unprefixed + # name (e.g. specify → templates/commands/specify.md). + # resolve_core() skips installed presets (tier 2) to prevent accidental nesting + # where another preset's wrap output is mistaken for the real core. + core_file = ( + resolver.resolve_core(cmd_name, "command") + or resolver.resolve_extension_command_via_manifest(cmd_name) + or resolver.resolve_core(short_name, "command") + ) if core_file is None: return body, {} @@ -2134,6 +2143,54 @@ def resolve_core( """ return self.resolve(template_name, template_type, skip_presets=True) + def resolve_extension_command_via_manifest(self, cmd_name: str) -> Optional[Path]: + """Resolve an extension command by consulting installed extension manifests. + + Walks installed extension directories in priority order, loads each + extension.yml via ExtensionManifest, and looks up the command by its + declared name to find the actual file path. This is necessary because + the manifest's ``provides.commands[].file`` field is authoritative and + may differ from the command name + (e.g. ``speckit.selftest.extension`` → ``commands/selftest.md``). + + Returns None if no manifest maps the given command name, so the caller + can fall back to the name-based lookup. + """ + if not self.extensions_dir.exists(): + return None + + from .extensions import ExtensionManifest, ValidationError + + for _priority, ext_id, _metadata in self._get_all_extensions_by_priority(): + ext_dir = self.extensions_dir / ext_id + manifest_path = ext_dir / "extension.yml" + if not manifest_path.is_file(): + continue + try: + manifest = ExtensionManifest(manifest_path) + except (ValidationError, Exception): + continue + for cmd_info in manifest.commands: + if cmd_info.get("name") != cmd_name: + continue + file_rel = cmd_info.get("file") + if not file_rel: + continue + # Mirror the containment check in ExtensionManager to guard against + # path traversal via a malformed manifest (e.g. file: ../../AGENTS.md). + cmd_path = Path(file_rel) + if cmd_path.is_absolute(): + continue + try: + ext_root = ext_dir.resolve() + candidate = (ext_root / cmd_path).resolve() + candidate.relative_to(ext_root) # raises ValueError if outside + except (OSError, ValueError): + continue + if candidate.is_file(): + return candidate + return None + def resolve_with_source( self, template_name: str, diff --git a/tests/test_presets.py b/tests/test_presets.py index 3679d04ecf..c11d44c7d0 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3545,6 +3545,45 @@ def test_extension_command_resolves_via_extension_directory(self, project_dir): assert "# Git Feature Core" in result assert "{CORE_TEMPLATE}" not in result + def test_extension_command_resolves_via_manifest_when_filename_differs(self, project_dir): + """Extension commands whose filename differs from the command name resolve via extension.yml. + + The selftest extension maps speckit.selftest.extension → commands/selftest.md. + Name-based lookup would look for commands/speckit.selftest.extension.md and fail; + manifest-based lookup must find the actual file declared in the manifest. + """ + from specify_cli.presets import _substitute_core_template + from specify_cli.agents import CommandRegistrar + + ext_dir = project_dir / ".specify" / "extensions" / "selftest" + cmd_dir = ext_dir / "commands" + cmd_dir.mkdir(parents=True, exist_ok=True) + + # File is named selftest.md, NOT speckit.selftest.extension.md + (cmd_dir / "selftest.md").write_text( + "---\ndescription: selftest core\n---\n\n# Selftest Core\n" + ) + # Manifest maps the command name to the actual file + (ext_dir / "extension.yml").write_text( + "schema_version: '1.0'\n" + "extension:\n id: selftest\n name: Self-Test\n version: 1.0.0\n" + " description: test\n author: test\n repository: https://example.com\n" + " license: MIT\n" + "requires:\n speckit_version: '>=0.2.0'\n" + "provides:\n" + " commands:\n" + " - name: speckit.selftest.extension\n" + " file: commands/selftest.md\n" + " description: Selftest command\n" + ) + + registrar = CommandRegistrar() + body = "## Wrapper\n\n{CORE_TEMPLATE}\n" + result, _ = _substitute_core_template(body, "speckit.selftest.extension", project_dir, registrar) + + assert "# Selftest Core" in result + assert "{CORE_TEMPLATE}" not in result + # ===== _replay_wraps_for_command Tests =====