Skip to content

Commit 7c643f9

Browse files
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
1 parent 296a7b3 commit 7c643f9

File tree

2 files changed

+966
-6
lines changed

2 files changed

+966
-6
lines changed

src/specify_cli/presets.py

Lines changed: 276 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,9 @@ def _substitute_core_template(
6464
# (e.g. speckit.git.feature -> extensions/git/commands/speckit.git.feature.md)
6565
# are found before falling back to the short name used by core commands
6666
# (e.g. specify -> templates/commands/specify.md).
67-
core_file = resolver.resolve(cmd_name, "command") or resolver.resolve(short_name, "command")
67+
# Use resolve_core() to skip installed presets (tier 2), preventing accidental
68+
# nesting where another preset's wrap output is mistaken for the real core.
69+
core_file = resolver.resolve_core(cmd_name, "command") or resolver.resolve_core(short_name, "command")
6870
if core_file is None:
6971
return body, {}
7072

@@ -600,6 +602,202 @@ def _unregister_commands(self, registered_commands: Dict[str, List[str]]) -> Non
600602
registrar = CommandRegistrar()
601603
registrar.unregister_commands(registered_commands, self.project_root)
602604

605+
def _replay_wraps_for_command(self, cmd_name: str) -> None:
606+
"""Recompose and rewrite agent files for a wrap-strategy command.
607+
608+
Collects all installed presets that declare cmd_name in their
609+
wrap_commands registry field, sorts them so the highest-precedence
610+
preset (lowest priority number) wraps outermost, then writes the
611+
fully composed output to every agent directory.
612+
613+
Called after every install and remove to keep agent files correct
614+
regardless of installation order.
615+
616+
Args:
617+
cmd_name: Full command name (e.g. "speckit.specify")
618+
"""
619+
try:
620+
from .agents import CommandRegistrar
621+
except ImportError:
622+
return
623+
624+
# Collect enabled presets that wrap this command, sorted ascending
625+
# (lowest priority number = highest precedence = outermost).
626+
wrap_presets = []
627+
for pack_id, metadata in self.registry.list_by_priority(include_disabled=False):
628+
if cmd_name not in metadata.get("wrap_commands", []):
629+
continue
630+
pack_dir = self.presets_dir / pack_id
631+
if not pack_dir.is_dir():
632+
continue # corrupted state — skip
633+
wrap_presets.append((pack_id, pack_dir))
634+
635+
if not wrap_presets:
636+
return
637+
638+
# Derive short name for core resolution fallback.
639+
short_name = cmd_name
640+
if short_name.startswith("speckit."):
641+
short_name = short_name[len("speckit."):]
642+
643+
resolver = PresetResolver(self.project_root)
644+
core_file = (
645+
resolver.resolve_core(cmd_name, "command")
646+
or resolver.resolve_core(short_name, "command")
647+
)
648+
if core_file is None:
649+
return
650+
651+
registrar = CommandRegistrar()
652+
core_frontmatter, core_body = registrar.parse_frontmatter(
653+
core_file.read_text(encoding="utf-8")
654+
)
655+
replay_aliases: List[str] = []
656+
seen_aliases: set[str] = set()
657+
658+
# Apply wraps innermost-first (reverse of ascending list).
659+
accumulated_body = core_body
660+
outermost_frontmatter = {}
661+
for pack_id, pack_dir in reversed(wrap_presets):
662+
cmd_file = pack_dir / "commands" / f"{cmd_name}.md"
663+
if not cmd_file.exists():
664+
continue
665+
manifest_path = pack_dir / "preset.yml"
666+
if manifest_path.exists():
667+
manifest = PresetManifest(manifest_path)
668+
for template in manifest.templates:
669+
if template.get("type") != "command" or template.get("name") != cmd_name:
670+
continue
671+
aliases = template.get("aliases", [])
672+
if not isinstance(aliases, list):
673+
aliases = []
674+
for alias in aliases:
675+
if isinstance(alias, str) and alias not in seen_aliases:
676+
replay_aliases.append(alias)
677+
seen_aliases.add(alias)
678+
wrap_fm, wrap_body = registrar.parse_frontmatter(
679+
cmd_file.read_text(encoding="utf-8")
680+
)
681+
accumulated_body = wrap_body.replace("{CORE_TEMPLATE}", accumulated_body)
682+
outermost_frontmatter = wrap_fm # last iteration = outermost preset
683+
684+
# Build final frontmatter: outermost preset wins; fall back to core for
685+
# scripts/agent_scripts if the outermost preset does not define them.
686+
final_frontmatter = dict(outermost_frontmatter)
687+
final_frontmatter.pop("strategy", None)
688+
for key in ("scripts", "agent_scripts"):
689+
if key not in final_frontmatter and key in core_frontmatter:
690+
final_frontmatter[key] = core_frontmatter[key]
691+
692+
outermost_pack_id = wrap_presets[-1][0]
693+
composed_content = (
694+
registrar.render_frontmatter(final_frontmatter) + "\n" + accumulated_body
695+
)
696+
697+
self._replay_skill_override(cmd_name, composed_content, outermost_pack_id)
698+
699+
with tempfile.TemporaryDirectory() as tmpdir:
700+
tmp_path = Path(tmpdir)
701+
cmd_dir = tmp_path / "commands"
702+
cmd_dir.mkdir()
703+
(cmd_dir / f"{cmd_name}.md").write_text(composed_content, encoding="utf-8")
704+
registrar._ensure_configs()
705+
for agent_name, agent_config in registrar.AGENT_CONFIGS.items():
706+
if agent_config.get("extension") == "/SKILL.md":
707+
continue
708+
agent_dir = self.project_root / agent_config["dir"]
709+
if not agent_dir.exists():
710+
continue
711+
try:
712+
registrar.register_commands(
713+
agent_name,
714+
[{
715+
"name": cmd_name,
716+
"file": f"commands/{cmd_name}.md",
717+
"aliases": replay_aliases,
718+
}],
719+
f"preset:{outermost_pack_id}",
720+
tmp_path,
721+
self.project_root,
722+
)
723+
except ValueError:
724+
continue
725+
726+
def _replay_skill_override(
727+
self,
728+
cmd_name: str,
729+
composed_content: str,
730+
outermost_pack_id: str,
731+
) -> None:
732+
"""Rewrite any active SKILL.md override for a replayed wrap command."""
733+
skills_dir = self._get_skills_dir()
734+
if not skills_dir:
735+
return
736+
737+
from . import SKILL_DESCRIPTIONS, load_init_options
738+
from .agents import CommandRegistrar
739+
740+
init_opts = load_init_options(self.project_root)
741+
if not isinstance(init_opts, dict):
742+
init_opts = {}
743+
selected_ai = init_opts.get("ai")
744+
if not isinstance(selected_ai, str):
745+
return
746+
747+
registrar = CommandRegistrar()
748+
agent_config = registrar.AGENT_CONFIGS.get(selected_ai, {})
749+
create_missing_skills = bool(init_opts.get("ai_skills")) and agent_config.get("extension") != "/SKILL.md"
750+
751+
skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name)
752+
target_skill_names: List[str] = []
753+
if (skills_dir / skill_name).is_dir():
754+
target_skill_names.append(skill_name)
755+
if legacy_skill_name != skill_name and (skills_dir / legacy_skill_name).is_dir():
756+
target_skill_names.append(legacy_skill_name)
757+
if not target_skill_names and create_missing_skills:
758+
missing_skill_dir = skills_dir / skill_name
759+
if not missing_skill_dir.exists():
760+
target_skill_names.append(skill_name)
761+
if not target_skill_names:
762+
return
763+
764+
raw_short_name = cmd_name
765+
if raw_short_name.startswith("speckit."):
766+
raw_short_name = raw_short_name[len("speckit."):]
767+
short_name = raw_short_name.replace(".", "-")
768+
skill_title = self._skill_title_from_command(cmd_name)
769+
770+
frontmatter, body = registrar.parse_frontmatter(composed_content)
771+
original_desc = frontmatter.get("description", "")
772+
enhanced_desc = SKILL_DESCRIPTIONS.get(
773+
short_name,
774+
original_desc or f"Spec-kit workflow command: {short_name}",
775+
)
776+
body = registrar.resolve_skill_placeholders(
777+
selected_ai, dict(frontmatter), body, self.project_root
778+
)
779+
780+
for target_skill_name in target_skill_names:
781+
skill_subdir = skills_dir / target_skill_name
782+
if skill_subdir.exists() and not skill_subdir.is_dir():
783+
continue
784+
skill_subdir.mkdir(parents=True, exist_ok=True)
785+
frontmatter_data = registrar.build_skill_frontmatter(
786+
selected_ai,
787+
target_skill_name,
788+
enhanced_desc,
789+
f"preset:{outermost_pack_id}",
790+
)
791+
frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip()
792+
skill_content = (
793+
f"---\n"
794+
f"{frontmatter_text}\n"
795+
f"---\n\n"
796+
f"# Speckit {skill_title} Skill\n\n"
797+
f"{body}\n"
798+
)
799+
(skill_subdir / "SKILL.md").write_text(skill_content, encoding="utf-8")
800+
603801
def _get_skills_dir(self) -> Optional[Path]:
604802
"""Return the active skills directory for preset skill overrides.
605803
@@ -1026,6 +1224,24 @@ def install_from_directory(
10261224
# Update corresponding skills when --ai-skills was previously used
10271225
registered_skills = self._register_skills(manifest, dest_dir)
10281226

1227+
# Detect wrap commands before registry.add() so a read failure doesn't
1228+
# leave a partially-committed registry entry.
1229+
wrap_commands = []
1230+
try:
1231+
from .agents import CommandRegistrar as _CR
1232+
_registrar = _CR()
1233+
for cmd_tmpl in manifest.templates:
1234+
if cmd_tmpl.get("type") != "command":
1235+
continue
1236+
cmd_file = dest_dir / cmd_tmpl["file"]
1237+
if not cmd_file.exists():
1238+
continue
1239+
cmd_fm, _ = _registrar.parse_frontmatter(cmd_file.read_text(encoding="utf-8"))
1240+
if cmd_fm.get("strategy") == "wrap":
1241+
wrap_commands.append(cmd_tmpl["name"])
1242+
except ImportError:
1243+
pass
1244+
10291245
self.registry.add(manifest.id, {
10301246
"version": manifest.version,
10311247
"source": "local",
@@ -1034,8 +1250,12 @@ def install_from_directory(
10341250
"priority": priority,
10351251
"registered_commands": registered_commands,
10361252
"registered_skills": registered_skills,
1253+
"wrap_commands": wrap_commands,
10371254
})
10381255

1256+
for cmd_name in wrap_commands:
1257+
self._replay_wraps_for_command(cmd_name)
1258+
10391259
return manifest
10401260

10411261
def install_from_zip(
@@ -1110,9 +1330,16 @@ def remove(self, pack_id: str) -> bool:
11101330
# Restore original skills when preset is removed
11111331
registered_skills = metadata.get("registered_skills", []) if metadata else []
11121332
registered_commands = metadata.get("registered_commands", {}) if metadata else {}
1333+
wrap_commands = metadata.get("wrap_commands", []) if metadata else []
11131334
pack_dir = self.presets_dir / pack_id
1335+
1336+
# _unregister_skills must run before directory deletion (reads preset files)
11141337
if registered_skills:
11151338
self._unregister_skills(registered_skills, pack_dir)
1339+
# When _unregister_skills has already handled skill-agent files, strip
1340+
# those entries from registered_commands to avoid double-deletion.
1341+
# (When registered_skills is empty, skill-agent entries in
1342+
# registered_commands are the only deletion path for those files.)
11161343
try:
11171344
from .agents import CommandRegistrar
11181345
except ImportError:
@@ -1124,14 +1351,42 @@ def remove(self, pack_id: str) -> bool:
11241351
if CommandRegistrar.AGENT_CONFIGS.get(agent_name, {}).get("extension") != "/SKILL.md"
11251352
}
11261353

1127-
# Unregister non-skill command files from AI agents.
1128-
if registered_commands:
1129-
self._unregister_commands(registered_commands)
1354+
# Remove from registry BEFORE deleting the directory so that
1355+
# _replay_wraps_for_command sees the post-removal registry state.
1356+
self.registry.remove(pack_id)
11301357

11311358
if pack_dir.exists():
11321359
shutil.rmtree(pack_dir)
11331360

1134-
self.registry.remove(pack_id)
1361+
# Separate wrap commands from non-wrap commands in registered_commands.
1362+
non_wrap_commands = {
1363+
agent_name: [c for c in cmd_names if c not in wrap_commands]
1364+
for agent_name, cmd_names in registered_commands.items()
1365+
}
1366+
non_wrap_commands = {k: v for k, v in non_wrap_commands.items() if v}
1367+
1368+
# Unregister non-wrap command files from AI agents.
1369+
if non_wrap_commands:
1370+
self._unregister_commands(non_wrap_commands)
1371+
1372+
# For each wrapped command, either re-compose remaining wraps or delete.
1373+
for cmd_name in wrap_commands:
1374+
remaining = [
1375+
pid for pid, meta in self.registry.list().items()
1376+
if cmd_name in meta.get("wrap_commands", [])
1377+
]
1378+
if remaining:
1379+
self._replay_wraps_for_command(cmd_name)
1380+
else:
1381+
# No wrap presets remain — delete the agent file entirely.
1382+
wrap_agent_commands = {
1383+
agent_name: [c for c in cmd_names if c == cmd_name]
1384+
for agent_name, cmd_names in registered_commands.items()
1385+
}
1386+
wrap_agent_commands = {k: v for k, v in wrap_agent_commands.items() if v}
1387+
if wrap_agent_commands:
1388+
self._unregister_commands(wrap_agent_commands)
1389+
11351390
return True
11361391

11371392
def list_installed(self) -> List[Dict[str, Any]]:
@@ -1787,6 +2042,7 @@ def resolve(
17872042
self,
17882043
template_name: str,
17892044
template_type: str = "template",
2045+
skip_presets: bool = False,
17902046
) -> Optional[Path]:
17912047
"""Resolve a template name to its file path.
17922048
@@ -1795,6 +2051,8 @@ def resolve(
17952051
Args:
17962052
template_name: Template name (e.g., "spec-template")
17972053
template_type: Template type ("template", "command", or "script")
2054+
skip_presets: When True, skip tier 2 (installed presets). Use
2055+
resolve_core() as the preferred caller-facing API for this.
17982056
17992057
Returns:
18002058
Path to the resolved template file, or None if not found
@@ -1823,7 +2081,7 @@ def resolve(
18232081
return override
18242082

18252083
# Priority 2: Installed presets (sorted by priority — lower number wins)
1826-
if self.presets_dir.exists():
2084+
if not skip_presets and self.presets_dir.exists():
18272085
registry = PresetRegistry(self.presets_dir)
18282086
for pack_id, _metadata in registry.list_by_priority():
18292087
pack_dir = self.presets_dir / pack_id
@@ -1864,6 +2122,18 @@ def resolve(
18642122

18652123
return None
18662124

2125+
def resolve_core(
2126+
self,
2127+
template_name: str,
2128+
template_type: str = "template",
2129+
) -> Optional[Path]:
2130+
"""Resolve against tiers 1, 3, and 4 only — skipping installed presets.
2131+
2132+
Use when resolving {CORE_TEMPLATE} to guarantee the result is actual
2133+
base content, never another preset's wrap output.
2134+
"""
2135+
return self.resolve(template_name, template_type, skip_presets=True)
2136+
18672137
def resolve_with_source(
18682138
self,
18692139
template_name: str,

0 commit comments

Comments
 (0)