Skip to content

Commit 3d35a88

Browse files
committed
fix: address second round of Copilot PR review feedback
- Fix 'preset resolve' CLI to derive display from _collect_all_layers() instead of resolve_with_source(), showing top layer path and noting when final output is composed from multiple layers - Add _reconcile_composed_commands() for stack-level command reconciliation: after install or remove, recompute effective command content from the full priority stack so command files always reflect the current state - Call reconciliation after preset remove so the next layer takes effect instead of leaving stale/missing command files - Persist registry incrementally: save registered_commands immediately after command registration, then registered_skills after skill registration, so interrupted installs can be cleaned up properly
1 parent 18346df commit 3d35a88

File tree

2 files changed

+114
-12
lines changed

2 files changed

+114
-12
lines changed

src/specify_cli/__init__.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2596,22 +2596,28 @@ def preset_resolve(
25962596
raise typer.Exit(1)
25972597

25982598
resolver = PresetResolver(project_root)
2599+
layers = resolver._collect_all_layers(template_name)
25992600
result = resolver.resolve_with_source(template_name)
26002601

2601-
if result:
2602-
console.print(f" [bold]{template_name}[/bold]: {result['path']}")
2603-
console.print(f" [dim](from: {result['source']})[/dim]")
2602+
if layers:
2603+
# Use the highest-priority layer for display because the final output
2604+
# may be composed and may not map to resolve_with_source()'s single path.
2605+
display_layer = layers[0]
2606+
console.print(f" [bold]{template_name}[/bold]: {display_layer['path']}")
2607+
console.print(f" [dim](top layer from: {display_layer['source']})[/dim]")
26042608

2605-
# Show composition chain if any layers use non-replace strategies
2606-
layers = resolver._collect_all_layers(template_name)
2607-
has_composition = any(layer["strategy"] != "replace" for layer in layers)
2609+
has_composition = len(layers) > 1 or any(layer["strategy"] != "replace" for layer in layers)
26082610
if has_composition:
2611+
console.print(" [dim]Final output is composed from multiple preset layers; the path above is the highest-priority contributing layer.[/dim]")
26092612
console.print("\n [bold]Composition chain:[/bold]")
26102613
for i, layer in enumerate(reversed(layers)):
26112614
strategy_label = layer["strategy"]
26122615
if strategy_label == "replace":
26132616
strategy_label = "base"
26142617
console.print(f" {i + 1}. [{strategy_label}] {layer['source']}{layer['path']}")
2618+
elif result:
2619+
console.print(f" [bold]{template_name}[/bold]: {result['path']}")
2620+
console.print(f" [dim](from: {result['source']})[/dim]")
26152621
else:
26162622
console.print(f" [yellow]{template_name}[/yellow]: not found")
26172623
console.print(" [dim]No template with this name exists in the resolution stack[/dim]")

src/specify_cli/presets.py

Lines changed: 102 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,88 @@ def _unregister_commands(self, registered_commands: Dict[str, List[str]]) -> Non
600600
registrar = CommandRegistrar()
601601
registrar.unregister_commands(registered_commands, self.project_root)
602602

603+
def _reconcile_composed_commands(self, command_names: List[str]) -> None:
604+
"""Re-resolve and re-register composed commands from the full stack.
605+
606+
After install or remove, recompute the effective content for each
607+
command name that participates in composition, and write the winning
608+
content to the agent directories. This ensures command files always
609+
reflect the current priority stack rather than depending on
610+
install/remove order.
611+
612+
Args:
613+
command_names: List of command names to reconcile
614+
"""
615+
if not command_names:
616+
return
617+
618+
try:
619+
from .agents import CommandRegistrar
620+
except ImportError:
621+
return
622+
623+
resolver = PresetResolver(self.project_root)
624+
registrar = CommandRegistrar()
625+
626+
for cmd_name in command_names:
627+
layers = resolver._collect_all_layers(cmd_name, "command")
628+
if not layers:
629+
continue
630+
631+
has_composition = any(l["strategy"] != "replace" for l in layers)
632+
if not has_composition:
633+
# Pure replace — the top layer wins. Find which preset owns it
634+
# and re-register from that preset's file.
635+
top_layer = layers[0]
636+
top_path = top_layer["path"]
637+
# Find the preset that owns this layer
638+
for pack_id, _meta in PresetRegistry(self.presets_dir).list_by_priority():
639+
pack_dir = self.presets_dir / pack_id
640+
if str(top_path).startswith(str(pack_dir)):
641+
manifest_path = pack_dir / "preset.yml"
642+
if manifest_path.exists():
643+
try:
644+
manifest = PresetManifest(manifest_path)
645+
except PresetValidationError:
646+
continue
647+
for tmpl in manifest.templates:
648+
if tmpl.get("name") == cmd_name and tmpl.get("type") == "command":
649+
registrar.register_commands_for_all_agents(
650+
[tmpl], manifest.id, pack_dir, self.project_root
651+
)
652+
break
653+
break
654+
else:
655+
# Composed command — resolve from full stack
656+
composed = resolver.resolve_content(cmd_name, "command")
657+
if not composed:
658+
continue
659+
660+
# Write to the highest-priority preset's .composed dir
661+
for pack_id, _meta in PresetRegistry(self.presets_dir).list_by_priority():
662+
pack_dir = self.presets_dir / pack_id
663+
manifest_path = pack_dir / "preset.yml"
664+
if not manifest_path.exists():
665+
continue
666+
try:
667+
manifest = PresetManifest(manifest_path)
668+
except PresetValidationError:
669+
continue
670+
for tmpl in manifest.templates:
671+
if tmpl.get("name") == cmd_name and tmpl.get("type") == "command":
672+
composed_dir = pack_dir / ".composed"
673+
composed_dir.mkdir(parents=True, exist_ok=True)
674+
composed_file = composed_dir / f"{cmd_name}.md"
675+
composed_file.write_text(composed, encoding="utf-8")
676+
registrar.register_commands_for_all_agents(
677+
[{**tmpl, "file": f".composed/{cmd_name}.md"}],
678+
manifest.id, pack_dir, self.project_root,
679+
)
680+
break
681+
else:
682+
continue
683+
break
684+
603685
def _get_skills_dir(self) -> Optional[Path]:
604686
"""Return the active skills directory for preset skill overrides.
605687
@@ -1032,21 +1114,25 @@ def install_from_directory(
10321114
})
10331115

10341116
try:
1035-
# Register command overrides with AI agents
1117+
# Register command overrides with AI agents and persist the result
1118+
# immediately so cleanup can recover even if installation stops
1119+
# before later phases complete.
10361120
registered_commands = self._register_commands(manifest, dest_dir)
1121+
self.registry.update(manifest.id, {
1122+
"registered_commands": registered_commands,
1123+
})
10371124

10381125
# Update corresponding skills when --ai-skills was previously used
1126+
# and persist that result as well.
10391127
registered_skills = self._register_skills(manifest, dest_dir)
1128+
self.registry.update(manifest.id, {
1129+
"registered_skills": registered_skills,
1130+
})
10401131
except Exception:
10411132
# Roll back registry entry on failure
10421133
self.registry.remove(manifest.id)
10431134
raise
10441135

1045-
self.registry.update(manifest.id, {
1046-
"registered_commands": registered_commands,
1047-
"registered_skills": registered_skills,
1048-
})
1049-
10501136
return manifest
10511137

10521138
def install_from_zip(
@@ -1136,13 +1222,23 @@ def remove(self, pack_id: str) -> bool:
11361222
}
11371223

11381224
# Unregister non-skill command files from AI agents.
1225+
# Collect all command names for post-removal reconciliation.
1226+
removed_cmd_names = set()
1227+
for cmd_names in registered_commands.values():
1228+
removed_cmd_names.update(cmd_names)
11391229
if registered_commands:
11401230
self._unregister_commands(registered_commands)
11411231

11421232
if pack_dir.exists():
11431233
shutil.rmtree(pack_dir)
11441234

11451235
self.registry.remove(pack_id)
1236+
1237+
# Reconcile: if other presets still provide these commands,
1238+
# re-resolve from the remaining stack so the next layer takes effect.
1239+
if removed_cmd_names:
1240+
self._reconcile_composed_commands(list(removed_cmd_names))
1241+
11461242
return True
11471243

11481244
def list_installed(self) -> List[Dict[str, Any]]:

0 commit comments

Comments
 (0)