feat: implement preset wrap strategy#2189
feat: implement preset wrap strategy#2189kennedy-whytech wants to merge 8 commits intogithub:mainfrom
Conversation
…E_TEMPLATE} with core command body
…EMPLATE} for all agent types
604b585 to
444daef
Compare
There was a problem hiding this comment.
Pull request overview
Adds support for strategy: wrap on preset-provided commands, enabling preset authors to inject pre/post content around an existing “core” command body via a {CORE_TEMPLATE} placeholder during command/skill registration.
Changes:
- Introduces
_substitute_core_template()and applies it during preset skill registration (_register_skills) and generic agent command registration (CommandRegistrar.register_commands). - Extends the self-test preset with a new
speckit.wrap-testcommand and updates docs to markwrapas implemented for commands. - Adds unit + E2E-style tests covering placeholder substitution and install behavior.
Show a summary per file
| File | Description |
|---|---|
| tests/test_presets.py | Updates self-test manifest expectations; adds new TestWrapStrategy coverage. |
| src/specify_cli/presets.py | Adds _substitute_core_template() and wires it into preset skill generation. |
| src/specify_cli/agents.py | Wires wrap substitution into agent command registration. |
| presets/self-test/preset.yml | Adds a new wrap-strategy command entry to the self-test preset. |
| presets/self-test/commands/speckit.wrap-test.md | Adds a wrap command template using {CORE_TEMPLATE}. |
| presets/README.md | Updates roadmap/docs to reflect wrap implemented for commands/artifacts. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 1
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback. If not applicable, please explain why
444daef to
53220f8
Compare
|
@mnriem that makes sense to me. updated. |
There was a problem hiding this comment.
Pull request overview
Implements support for strategy: wrap on preset command templates by substituting {CORE_TEMPLATE} with the installed core command template body during installation/registration, enabling presets to extend core commands without copy/paste.
Changes:
- Added
_substitute_core_template()helper and applied it when generating skills from preset commands (Claude skills flow). - Applied the same substitution in
CommandRegistrar.register_commands()for non-skill agent command installation. - Added a self-test preset wrap command + unit/E2E tests, and updated preset strategy documentation.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/presets.py |
Adds {CORE_TEMPLATE} substitution helper and applies it in the skills generation path for wrap-strategy preset commands. |
src/specify_cli/agents.py |
Applies {CORE_TEMPLATE} substitution during normal agent command registration when strategy: wrap is set. |
tests/test_presets.py |
Updates self-test manifest expectations and adds unit + E2E coverage for wrap substitution. |
presets/self-test/preset.yml |
Registers a new wrap-strategy command in the bundled self-test preset. |
presets/self-test/commands/speckit.wrap-test.md |
New wrap command template containing {CORE_TEMPLATE} placeholder. |
presets/README.md |
Updates roadmap/docs to reflect wrap being implemented for commands/artifacts (not scripts). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 2
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
There was a problem hiding this comment.
Pull request overview
Adds a new preset composition option (strategy: wrap) for command/skill overrides, allowing presets to inject content around the existing core command by substituting a {CORE_TEMPLATE} placeholder, rather than fully replacing the command.
Changes:
- Implement
_substitute_core_templatehelper to resolve and substitute{CORE_TEMPLATE}from project core templates, bundled core pack, and installed extensions. - Wire wrap substitution into both preset skill generation (
_register_skills) and agent command registration (register_commands). - Extend self-test preset + add comprehensive tests for wrap behavior, extension resolution, and script placeholder hygiene.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/presets.py |
Introduces {CORE_TEMPLATE} substitution + merges core scripts/agent_scripts into wrapped skill frontmatter. |
src/specify_cli/agents.py |
Enables wrap substitution during agent command registration. |
tests/test_presets.py |
Adds TestWrapStrategy coverage and updates self-test manifest expectations. |
presets/self-test/preset.yml |
Adds a self-test wrap command entry. |
presets/self-test/commands/speckit.wrap-test.md |
New wrap-strategy command template used for end-to-end testing. |
presets/README.md |
Updates documentation to reflect wrap is implemented for commands/artifacts (scripts TBD). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 1
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
…: 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.
4fe7f81 to
49c83d2
Compare
There was a problem hiding this comment.
Pull request overview
Adds support for a new preset composition mode (strategy: wrap) so preset command/skill overrides can inject content around an existing command body via a {CORE_TEMPLATE} placeholder, instead of fully replacing it.
Changes:
- Introduces
_substitute_core_template()and wires it into preset skill generation and agent command registration forstrategy: wrap. - Extends command registration to merge missing
scripts/agent_scriptsfrom the wrapped “core” template and resolves placeholders for TOML command output. - Adds wrap-strategy tests plus a self-test preset command and updates preset documentation.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/presets.py |
Adds core-template substitution helper and applies wrap behavior when generating SKILL.md overrides. |
src/specify_cli/agents.py |
Applies wrap substitution and frontmatter inheritance during command registration; resolves placeholders for TOML output. |
tests/test_presets.py |
Adds wrap-strategy test suite and adjusts self-test preset expectations; modifies bundled preset CLI error assertions. |
presets/self-test/preset.yml |
Registers a new self-test command entry for wrap strategy coverage. |
presets/self-test/commands/speckit.wrap-test.md |
Adds a wrap-strategy self-test command file using {CORE_TEMPLATE}. |
presets/README.md |
Updates documentation to reflect that command/template wrapping is implemented (scripts wrap not yet). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
tests/test_presets.py:3299
- Same AGENT_CONFIGS leakage issue here: the test mutates
registrar.AGENT_CONFIGS[...](class-level) and then restores withregistrar.AGENT_CONFIGS = original(instance-level assignment). This can leavetest-agentin the class-level configs for subsequent tests. Restore onCommandRegistrar.AGENT_CONFIGS(class) or patch the dict in a context manager.
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
tests/test_presets.py:3347
- Same AGENT_CONFIGS leakage issue in the TOML agent test:
registrar.AGENT_CONFIGSis mutated (class-level) but restored via instance assignment (registrar.AGENT_CONFIGS = original). This can leaktest-toml-agentinto other tests. Restore viaCommandRegistrar.AGENT_CONFIGS = originalor patch the dict withpatch.dict/monkeypatch.
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
- Files reviewed: 6/6 changed files
- Comments generated: 4
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
…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
_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.
There was a problem hiding this comment.
Pull request overview
Implements a new preset command override strategy (strategy: wrap) that allows presets to inject content before/after an existing command template via a {CORE_TEMPLATE} placeholder, and wires this behavior into both skills generation and agent command registration paths.
Changes:
- Added
_substitute_core_template()and integrated wrap behavior into preset skill registration (_register_skills) and agent command registration (register_commands). - Updated TOML command generation to resolve script placeholders and convert argument placeholders appropriately.
- Expanded preset test coverage (including self-test preset updates) for wrap strategy behavior and script metadata inheritance.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/presets.py |
Adds {CORE_TEMPLATE} substitution helper and merges core script metadata into wrapped presets for skills generation. |
src/specify_cli/agents.py |
Applies wrap substitution for command registration and resolves placeholders for TOML outputs. |
tests/test_presets.py |
Adds a dedicated wrap strategy test suite and updates self-test preset assertions. |
presets/self-test/preset.yml |
Registers a new self-test wrap command entry. |
presets/self-test/commands/speckit.wrap-test.md |
Adds a wrap-strategy command fixture using {CORE_TEMPLATE}. |
presets/README.md |
Updates documentation to reflect wrap support status (commands/artifacts implemented; scripts not yet). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
tests/test_presets.py:3300
- Same AGENT_CONFIGS restoration issue as above: reassigning
CommandRegistrar.AGENT_CONFIGS = originalwon’t restore the dict object referenced byspecify_cli.extensions.CommandRegistrar.AGENT_CONFIGS, so the temporarytest-agententry can leak into later tests. Prefer an in-place restore (clear/update) orpatch.dict.
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 = original
tests/test_presets.py:3348
- Same AGENT_CONFIGS restoration issue as above for the TOML agent config: reassigning the class attribute can leave
specify_cli.extensions.CommandRegistrar.AGENT_CONFIGSpointing at the mutated dict (withtest-toml-agentstill present). Restore the dict contents in-place or usepatch.dictto avoid cross-test leakage.
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 = original
- Files reviewed: 6/6 changed files
- Comments generated: 3
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
There was a problem hiding this comment.
Pull request overview
Implements a strategy: wrap composition mode for preset command overrides, allowing presets to inject pre/post content around an existing command via a {CORE_TEMPLATE} placeholder while preserving core script metadata needed for placeholder/script resolution.
Changes:
- Added
_substitute_core_template()to replace{CORE_TEMPLATE}with the resolved underlying command body and return the underlying command frontmatter for inheritance. - Wired wrap behavior into both preset skill generation (
_register_skills) and general command registration (register_commands), including inheritingscripts/agent_scriptsfrom the wrapped command. - Expanded self-test preset + added a dedicated wrap strategy test suite covering substitution, script inheritance, TOML placeholder resolution, and extension command lookup.
Show a summary per file
| File | Description |
|---|---|
| tests/test_presets.py | Adds wrap-strategy unit/e2e tests and updates self-test manifest expectations. |
| src/specify_cli/presets.py | Introduces _substitute_core_template() and applies wrap strategy + script inheritance in _register_skills. |
| src/specify_cli/agents.py | Applies wrap strategy + script inheritance in register_commands and resolves {SCRIPT}/{ARGS} placeholders for TOML outputs. |
| presets/self-test/preset.yml | Adds a new self-test wrap-strategy command entry. |
| presets/self-test/commands/speckit.wrap-test.md | Adds a wrap command template demonstrating {CORE_TEMPLATE} usage. |
| presets/README.md | Clarifies wrap is implemented for artifacts/commands and not yet implemented for scripts. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
Description
Fixes in this branch
{SCRIPT} placeholders broken in wrapped commands — when a preset used strategy: wrap, the core template's scripts/agent_scripts frontmatter was silently discarded after body substitution. Presets that wrap an existing
command (e.g. "use Codex for the following: {CORE_TEMPLATE}") now inherit the core's script metadata so prerequisite check scripts still run.
Extension commands never resolved — _substitute_core_template only searched core-pack and project template directories. Wrapping extension commands like speckit.git.feature now also searches
.specify/extensions//commands/, so {CORE_TEMPLATE} resolves correctly for extension overrides.
Two test hygiene fixes (from code review):
Test plan
End-to-end test with my own presets. The wrapping seems doing good. Also, I feel like "wrap" can cover prepend and append already.

Testing
uv run specify --helpuv sync && uv run pytestAI Disclosure
I did not use AI assistance for this contribution
I did use AI assistance (describe below)
AI assistance disclosure
This PR was implemented with AI assistance (Claude Code) for code generation,
test writing, and commit messages. All changes were reviewed and verified
manually.