Skip to content

Commit 22e7699

Browse files
feat: implement preset wrap strategy (#2189)
* feat: implement strategy: wrap * fix: resolve merge conflict for strategy wrap correctness * 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 * fix: resolve extension commands via manifest file mapping 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 <cmd_name>.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. * fix: add bundled core_pack as Priority 5 in PresetResolver.resolve() resolve_core() was returning None for built-in commands (implement, specify, etc.) because PresetResolver only checked .specify/templates/ commands/ (Priority 4), which is never populated for commands in a normal project. strategy:wrap presets rely on resolve_core() to fetch the {CORE_TEMPLATE} body, so the wrap was silently skipped and SKILL.md was never updated. Priority 5 now checks core_pack/commands/ (wheel install) or repo_root/templates/commands/ (source checkout), mirroring the pattern used by _locate_core_pack() elsewhere. Updated two tests whose assertions assumed resolve_core() always returned None when .specify/templates/commands/ was absent. * fix: harden preset wrap replay removal * fix: stabilize existing directory error output * fix: track outermost_pack_id from contributing preset; use Path.parts in tests - outermost_pack_id now updates alongside outermost_frontmatter inside the wrap loop, so it reflects the actual last contributing preset rather than always taking wrap_presets[0] (which may have been skipped) - Replace str(path) substring checks in TestResolveCore with Path.parts tuple comparisons for correct behaviour on Windows (CI runs windows-latest) * fix: guard against non-mapping YAML manifests; apply integration post-processing in replay - ExtensionManifest._load raises ValidationError for non-dict YAML roots instead of TypeError - PresetManager._replay_wraps_for_command calls integration.post_process_skill_content, matching _register_skills behaviour - PresetResolver skips extensions that raise OSError/TypeError/AttributeError on manifest load - Tests: non-mapping YAML, OSError manifest skip, and replay integration post-processing
1 parent 569d18a commit 22e7699

File tree

10 files changed

+1771
-16
lines changed

10 files changed

+1771
-16
lines changed

presets/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,5 +116,5 @@ The following enhancements are under consideration for future releases:
116116
| **command** | ✓ (default) ||||
117117
| **script** | ✓ (default) ||||
118118

119-
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.
119+
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).
120120
- **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.
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
description: "Self-test wrap command — pre/post around core"
3+
strategy: wrap
4+
---
5+
6+
## Preset Pre-Logic
7+
8+
preset:self-test wrap-pre
9+
10+
{CORE_TEMPLATE}
11+
12+
## Preset Post-Logic
13+
14+
preset:self-test wrap-post

presets/self-test/preset.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ provides:
5656
description: "Self-test override of the specify command"
5757
replaces: "speckit.specify"
5858

59+
- type: "command"
60+
name: "speckit.wrap-test"
61+
file: "commands/speckit.wrap-test.md"
62+
description: "Self-test wrap strategy command"
63+
5964
tags:
6065
- "testing"
6166
- "self-test"

src/specify_cli/__init__.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,7 +1115,7 @@ def init(
11151115
console.print(f"[cyan]--force supplied: merging into existing directory '[cyan]{project_name}[/cyan]'[/cyan]")
11161116
else:
11171117
error_panel = Panel(
1118-
f"Directory '[cyan]{project_name}[/cyan]' already exists\n"
1118+
f"Directory already exists: '[cyan]{project_name}[/cyan]'\n"
11191119
"Please choose a different project name or remove the existing directory.\n"
11201120
"Use [bold]--force[/bold] to merge into the existing directory.",
11211121
title="[red]Directory Conflict[/red]",
@@ -1371,7 +1371,6 @@ def init(
13711371
"branch_numbering": branch_numbering or "sequential",
13721372
"context_file": resolved_integration.context_file,
13731373
"here": here,
1374-
"preset": preset,
13751374
"script": selected_script,
13761375
"speckit_version": get_speckit_version(),
13771376
}

src/specify_cli/agents.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,15 @@ def register_commands(
468468
content = source_file.read_text(encoding="utf-8")
469469
frontmatter, body = self.parse_frontmatter(content)
470470

471+
if frontmatter.get("strategy") == "wrap":
472+
from .presets import _substitute_core_template
473+
body, core_frontmatter = _substitute_core_template(body, cmd_name, project_root, self)
474+
frontmatter = dict(frontmatter)
475+
for key in ("scripts", "agent_scripts"):
476+
if key not in frontmatter and key in core_frontmatter:
477+
frontmatter[key] = core_frontmatter[key]
478+
frontmatter.pop("strategy", None)
479+
471480
frontmatter = self._adjust_script_paths(frontmatter)
472481

473482
for key in agent_config.get("strip_frontmatter_keys", []):
@@ -495,10 +504,12 @@ def register_commands(
495504
project_root,
496505
)
497506
elif agent_config["format"] == "markdown":
498-
output = self.render_markdown_command(
499-
frontmatter, body, source_id, context_note
500-
)
507+
body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root)
508+
body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"])
509+
output = self.render_markdown_command(frontmatter, body, source_id, context_note)
501510
elif agent_config["format"] == "toml":
511+
body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root)
512+
body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"])
502513
output = self.render_toml_command(frontmatter, body, source_id)
503514
elif agent_config["format"] == "yaml":
504515
output = self.render_yaml_command(

src/specify_cli/extensions.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,16 @@ def _load_yaml(self, path: Path) -> dict:
140140
"""Load YAML file safely."""
141141
try:
142142
with open(path, 'r') as f:
143-
return yaml.safe_load(f) or {}
143+
data = yaml.safe_load(f)
144144
except yaml.YAMLError as e:
145145
raise ValidationError(f"Invalid YAML in {path}: {e}")
146146
except FileNotFoundError:
147147
raise ValidationError(f"Manifest not found: {path}")
148+
if not isinstance(data, dict):
149+
raise ValidationError(
150+
f"Manifest must be a YAML mapping, got {type(data).__name__}: {path}"
151+
)
152+
return data
148153

149154
def _validate(self):
150155
"""Validate manifest structure and required fields."""

0 commit comments

Comments
 (0)