From bb67923df52782074a8270c21f20d0b03c65c5af Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Sun, 12 Apr 2026 10:22:52 -0400 Subject: [PATCH 1/8] =?UTF-8?q?feat:=20implement=20strategy:=20wrap=20for?= =?UTF-8?q?=20=5Fregister=5Fskills=20=E2=80=94=20substitute=20{CORE=5FTEMP?= =?UTF-8?q?LATE}=20with=20core=20command=20body?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/specify_cli/presets.py | 32 ++++++++++++++++++++++++ tests/test_presets.py | 51 +++++++++++++++++++++++++++++++++++++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 3a0f469a77..d76441308b 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.""" @@ -759,6 +788,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 95af7a900f..2c880110e4 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3043,4 +3043,53 @@ 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 From 864c4403b5ed80a14e5083c4bd706441f87a407e Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Sun, 12 Apr 2026 10:23:34 -0400 Subject: [PATCH 2/8] =?UTF-8?q?feat:=20wire=20strategy:=20wrap=20into=20re?= =?UTF-8?q?gister=5Fcommands=20=E2=80=94=20substitute=20{CORE=5FTEMPLATE}?= =?UTF-8?q?=20for=20all=20agent=20types?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/specify_cli/agents.py | 7 ++++++ tests/test_presets.py | 50 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index ec7af88768..6d8de80f2f 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -414,6 +414,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/tests/test_presets.py b/tests/test_presets.py index 2c880110e4..0578a9a1e6 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3093,3 +3093,53 @@ def test_substitute_core_template_no_op_when_core_missing(self, project_dir): 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 From df00038695be9d1311e9383bada6583cdbd3aed5 Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Sun, 12 Apr 2026 10:25:41 -0400 Subject: [PATCH 3/8] test: end-to-end wrap strategy coverage via self-test preset --- .../self-test/commands/speckit.wrap-test.md | 14 ++++++++ presets/self-test/preset.yml | 5 +++ tests/test_presets.py | 35 ++++++++++++++++++- 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 presets/self-test/commands/speckit.wrap-test.md 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/tests/test_presets.py b/tests/test_presets.py index 0578a9a1e6..84f148b871 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1667,7 +1667,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.""" @@ -3143,3 +3143,36 @@ def test_register_commands_substitutes_core_template_for_wrap_strategy(self, pro 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 6a24268dd47845e981250f6c09553d9bfaa1735d Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Sun, 12 Apr 2026 10:26:23 -0400 Subject: [PATCH 4/8] docs: mark strategy: wrap as implemented for command templates --- presets/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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. From 49c83d2930ecee14ea1ec939cf675ed9f29d9b13 Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Mon, 13 Apr 2026 22:49:52 -0400 Subject: [PATCH 5/8] fix: inherit scripts/agent_scripts from core frontmatter for strategy: wrap _substitute_core_template now returns a (body, core_frontmatter) tuple so callers can merge scripts and agent_scripts from the core template into the preset frontmatter when the preset does not define them. Both _register_skills and register_commands perform this merge before _adjust_script_paths runs so inherited paths are normalised. The TOML render path also calls resolve_skill_placeholders so {SCRIPT}/{AGENT_SCRIPT} placeholders are expanded for Gemini/Tabnine agents. --- src/specify_cli/agents.py | 8 +- src/specify_cli/presets.py | 23 +++-- tests/test_presets.py | 182 ++++++++++++++++++++++++++++++++++++- 3 files changed, 201 insertions(+), 12 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 6d8de80f2f..bdc46cd22b 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -419,7 +419,11 @@ def register_commands( 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, short_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 = self._adjust_script_paths(frontmatter) @@ -444,6 +448,8 @@ def register_commands( elif agent_config["format"] == "markdown": 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']}") diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index d76441308b..315c08594f 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -32,7 +32,7 @@ def _substitute_core_template( short_name: str, project_root: "Path", registrar: "CommandRegistrar", -) -> str: +) -> "tuple[str, dict]": """Substitute {CORE_TEMPLATE} with the body of the installed core command template. Args: @@ -42,18 +42,21 @@ def _substitute_core_template( 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, {} core_file = project_root / ".specify" / "templates" / "commands" / f"{short_name}.md" if not core_file.exists(): - return body + 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 +792,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, short_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 84f148b871..18687f85d5 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3062,12 +3062,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 +3081,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 +3092,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.""" @@ -3176,3 +3179,176 @@ 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: + registrar.AGENT_CONFIGS = 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: + registrar.AGENT_CONFIGS = 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 From 9b32c61d418c2cc8f498d85fc3f713b8145d7127 Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Tue, 14 Apr 2026 16:21:08 -0400 Subject: [PATCH 6/8] fix: consistent short_name resolution and test hygiene for strategy: wrap - Pass raw_short_name (dot-separated) to _substitute_core_template in _register_skills to match register_commands, ensuring consistent core template lookup across both code paths - Restore original two-step core_templates_dir existence check in _substitute_core_template to match codebase style - Restore missing reinstall assertion in bundled preset CLI error test - Restore AGENT_CONFIGS on the class (CommandRegistrar.AGENT_CONFIGS) not the instance to prevent state leaking across tests - Add test covering extension command dot-separated name resolution --- src/specify_cli/presets.py | 9 +++++--- tests/test_presets.py | 45 +++++++++++++++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 315c08594f..45b40a05b0 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -51,8 +51,11 @@ def _substitute_core_template( if "{CORE_TEMPLATE}" not in body: return body, {} - core_file = project_root / ".specify" / "templates" / "commands" / f"{short_name}.md" - if not core_file.exists(): + core_templates_dir = project_root / ".specify" / "templates" / "commands" + core_file = core_templates_dir / f"{short_name}.md" if core_templates_dir.exists() else None + if core_file and not core_file.exists(): + core_file = None + if core_file is None: return body, {} core_frontmatter, core_body = registrar.parse_frontmatter(core_file.read_text(encoding="utf-8")) @@ -792,7 +795,7 @@ def _register_skills( frontmatter, body = registrar.parse_frontmatter(content) if frontmatter.get("strategy") == "wrap": - body, core_frontmatter = _substitute_core_template(body, short_name, self.project_root, registrar) + body, core_frontmatter = _substitute_core_template(body, raw_short_name, self.project_root, registrar) frontmatter = dict(frontmatter) for key in ("scripts", "agent_scripts"): if key not in frontmatter and key in core_frontmatter: diff --git a/tests/test_presets.py b/tests/test_presets.py index 18687f85d5..ce3a4167ab 100644 --- a/tests/test_presets.py +++ b/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: @@ -3139,7 +3140,7 @@ 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 = original written = (agent_dir / "speckit.specify.md").read_text() assert "{CORE_TEMPLATE}" not in written @@ -3295,7 +3296,7 @@ def test_register_commands_inherits_scripts_from_core(self, project_dir): project_dir, ) finally: - registrar.AGENT_CONFIGS = original + CommandRegistrar.AGENT_CONFIGS = original written = (agent_dir / "speckit.specify.md").read_text() assert "{CORE_TEMPLATE}" not in written @@ -3343,7 +3344,7 @@ def test_register_commands_toml_resolves_inherited_scripts(self, project_dir): project_dir, ) finally: - registrar.AGENT_CONFIGS = original + CommandRegistrar.AGENT_CONFIGS = original written = (toml_dir / "speckit.specify.toml").read_text() assert "{CORE_TEMPLATE}" not in written @@ -3352,3 +3353,41 @@ def test_register_commands_toml_resolves_inherited_scripts(self, project_dir): # args token must use TOML format, not the intermediate $ARGUMENTS assert "$ARGUMENTS" not in written assert "{{args}}" in written + + def test_extension_command_resolves_dot_separated_template(self, project_dir): + """Extension command names like speckit.git.feature look up git.feature.md, not git-feature.md. + + Covers both _register_skills (via _substitute_core_template with raw_short_name) + and register_commands (via short_name stripped of speckit. prefix). + """ + 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) + # Template uses dot-separated name — must NOT be named git-feature.md + (core_dir / "git.feature.md").write_text( + "---\ndescription: core git feature\n---\n\n# Git Feature Core\n" + ) + # Ensure the hyphenated variant does NOT exist to prove we pick the right file + assert not (core_dir / "git-feature.md").exists() + + registrar = CommandRegistrar() + body = "## Wrapper\n\n{CORE_TEMPLATE}\n" + + # Simulate register_commands: strips "speckit." only + short_name_commands = "speckit.git.feature" + if short_name_commands.startswith("speckit."): + short_name_commands = short_name_commands[len("speckit."):] + result_commands, _ = _substitute_core_template(body, short_name_commands, project_dir, registrar) + + # Simulate _register_skills: strips "speckit." then hyphenates for skill dirs, + # but passes raw_short_name (pre-hyphenation) to _substitute_core_template + raw_short_name = "speckit.git.feature" + if raw_short_name.startswith("speckit."): + raw_short_name = raw_short_name[len("speckit."):] + result_skills, _ = _substitute_core_template(body, raw_short_name, project_dir, registrar) + + assert "# Git Feature Core" in result_commands, "register_commands path did not resolve extension template" + assert "# Git Feature Core" in result_skills, "_register_skills path did not resolve extension template" + assert result_commands == result_skills, "register_commands and _register_skills resolved differently" From 3336fd4a9b5e7b033c16f932110cc4e7adea85c7 Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Tue, 14 Apr 2026 16:32:28 -0400 Subject: [PATCH 7/8] fix: use PresetResolver for core template lookup in strategy: wrap _substitute_core_template now accepts the full cmd_name and resolves the core template file via PresetResolver, which walks the full priority stack (overrides -> presets -> extensions -> core templates). It tries the full command name first so extension commands (e.g. speckit.git.feature -> extensions/git/commands/speckit.git.feature.md) are found correctly, then falls back to the short name for core commands (e.g. specify -> templates/commands/specify.md). Both call sites (register_commands and _register_skills) now pass cmd_name directly, removing the short-name derivation from the callers. --- src/specify_cli/agents.py | 5 +---- src/specify_cli/presets.py | 21 ++++++++++++------ tests/test_presets.py | 45 +++++++++++++++----------------------- 3 files changed, 33 insertions(+), 38 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index bdc46cd22b..ce47187cca 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -416,10 +416,7 @@ 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, core_frontmatter = _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: diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 45b40a05b0..9a4b13df1c 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -29,7 +29,7 @@ def _substitute_core_template( body: str, - short_name: str, + cmd_name: str, project_root: "Path", registrar: "CommandRegistrar", ) -> "tuple[str, dict]": @@ -37,7 +37,7 @@ def _substitute_core_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. @@ -51,10 +51,17 @@ def _substitute_core_template( if "{CORE_TEMPLATE}" not in body: return body, {} - core_templates_dir = project_root / ".specify" / "templates" / "commands" - core_file = core_templates_dir / f"{short_name}.md" if core_templates_dir.exists() else None - if core_file and not core_file.exists(): - core_file = None + # 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."):] + + 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, {} @@ -795,7 +802,7 @@ def _register_skills( frontmatter, body = registrar.parse_frontmatter(content) if frontmatter.get("strategy") == "wrap": - body, core_frontmatter = _substitute_core_template(body, raw_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: diff --git a/tests/test_presets.py b/tests/test_presets.py index ce3a4167ab..99fc792062 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3354,40 +3354,31 @@ def test_register_commands_toml_resolves_inherited_scripts(self, project_dir): assert "$ARGUMENTS" not in written assert "{{args}}" in written - def test_extension_command_resolves_dot_separated_template(self, project_dir): - """Extension command names like speckit.git.feature look up git.feature.md, not git-feature.md. + def test_extension_command_resolves_via_extension_directory(self, project_dir): + """Extension commands (e.g. speckit.git.feature) resolve from the extension directory. - Covers both _register_skills (via _substitute_core_template with raw_short_name) - and register_commands (via short_name stripped of speckit. prefix). + 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 - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - # Template uses dot-separated name — must NOT be named git-feature.md - (core_dir / "git.feature.md").write_text( - "---\ndescription: core git feature\n---\n\n# Git Feature Core\n" + # 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 the hyphenated variant does NOT exist to prove we pick the right file - assert not (core_dir / "git-feature.md").exists() + # 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" - # Simulate register_commands: strips "speckit." only - short_name_commands = "speckit.git.feature" - if short_name_commands.startswith("speckit."): - short_name_commands = short_name_commands[len("speckit."):] - result_commands, _ = _substitute_core_template(body, short_name_commands, project_dir, registrar) - - # Simulate _register_skills: strips "speckit." then hyphenates for skill dirs, - # but passes raw_short_name (pre-hyphenation) to _substitute_core_template - raw_short_name = "speckit.git.feature" - if raw_short_name.startswith("speckit."): - raw_short_name = raw_short_name[len("speckit."):] - result_skills, _ = _substitute_core_template(body, raw_short_name, project_dir, registrar) - - assert "# Git Feature Core" in result_commands, "register_commands path did not resolve extension template" - assert "# Git Feature Core" in result_skills, "_register_skills path did not resolve extension template" - assert result_commands == result_skills, "register_commands and _register_skills resolved differently" + # 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 1007dcb307dae3781da2bca3f5a6ec9a8371fd4b Mon Sep 17 00:00:00 2001 From: KennedyYau Date: Tue, 14 Apr 2026 16:44:39 -0400 Subject: [PATCH 8/8] test: restore AGENT_CONFIGS in-place to prevent dict reference leaks --- tests/test_presets.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test_presets.py b/tests/test_presets.py index 99fc792062..6dd1df8d46 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3140,7 +3140,8 @@ def test_register_commands_substitutes_core_template_for_wrap_strategy(self, pro project_dir / "preset", project_dir ) finally: - CommandRegistrar.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 @@ -3296,7 +3297,8 @@ def test_register_commands_inherits_scripts_from_core(self, project_dir): project_dir, ) finally: - CommandRegistrar.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 @@ -3344,7 +3346,8 @@ def test_register_commands_toml_resolves_inherited_scripts(self, project_dir): project_dir, ) finally: - CommandRegistrar.AGENT_CONFIGS = original + CommandRegistrar.AGENT_CONFIGS.clear() + CommandRegistrar.AGENT_CONFIGS.update(original) written = (toml_dir / "speckit.specify.toml").read_text() assert "{CORE_TEMPLATE}" not in written