fix: address thirteenth round of Copilot PR review feedback

- Add frontmatter composition test: verify core+preset command layers
  with frontmatter produce only one frontmatter block in composed output
- Defer resolve_with_source: only call when collect_all_layers returns
  empty, avoiding redundant filesystem walk
- Reuse CommandRegistrar.parse_frontmatter: replace local
  _strip_frontmatter helper with _split_frontmatter that delegates to
  the shared parse_frontmatter method
- Fix removed_cmd_names collection: gather ALL command names before
  filtering out skill agents so reconciliation covers all affected
  commands including those only registered for skill-based agents
- Preserve trailing newlines in bash: use sentinel character technique
  (cat + printf x + strip) to avoid command substitution stripping

Author: mnriem

scripts/bash/common.sh

@@ -536,7 +536,9 @@ except Exception:
         local path="${layer_paths[$i]}"
         local strat="${layer_strategies[$i]}"
         local layer_content
-        layer_content=$(cat "$path")
+        # Preserve trailing newlines: append sentinel, then strip it
+        layer_content=$(cat "$path"; printf x)
+        layer_content="${layer_content%x}"
 
         if [ "$started" = false ]; then
             if [ "$strat" = "replace" ]; then

src/specify_cli/__init__.py

@@ -2597,7 +2597,6 @@ def preset_resolve(
 
     resolver = PresetResolver(project_root)
     layers = resolver.collect_all_layers(template_name)
-    result = resolver.resolve_with_source(template_name)
 
     if layers:
         # Use the highest-priority layer for display because the final output
@@ -2618,12 +2617,15 @@ def preset_resolve(
                 if strategy_label == "replace":
                     strategy_label = "base"
                 console.print(f"    {i + 1}. [{strategy_label}] {layer['source']} → {layer['path']}")
-    elif result:
-        console.print(f"  [bold]{template_name}[/bold]: {result['path']}")
-        console.print(f"    [dim](from: {result['source']})[/dim]")
     else:
-        console.print(f"  [yellow]{template_name}[/yellow]: not found")
-        console.print("    [dim]No template with this name exists in the resolution stack[/dim]")
+        # No layers found — fall back to resolve_with_source for non-composition cases
+        result = resolver.resolve_with_source(template_name)
+        if result:
+            console.print(f"  [bold]{template_name}[/bold]: {result['path']}")
+            console.print(f"    [dim](from: {result['source']})[/dim]")
+        else:
+            console.print(f"  [yellow]{template_name}[/yellow]: not found")
+            console.print("    [dim]No template with this name exists in the resolution stack[/dim]")
 
 
 @preset_app.command("info")

src/specify_cli/presets.py

@@ -1361,6 +1361,13 @@ def remove(self, pack_id: str) -> bool:
         registered_skills = metadata.get("registered_skills", []) if metadata else []
         registered_commands = metadata.get("registered_commands", {}) if metadata else {}
         pack_dir = self.presets_dir / pack_id
+
+        # Collect ALL command names before filtering for reconciliation,
+        # so commands registered only for skill-based agents are also reconciled.
+        removed_cmd_names = set()
+        for cmd_names in registered_commands.values():
+            removed_cmd_names.update(cmd_names)
+
         if registered_skills:
             self._unregister_skills(registered_skills, pack_dir)
             try:
@@ -1375,10 +1382,6 @@ def remove(self, pack_id: str) -> bool:
                 }
 
         # Unregister non-skill command files from AI agents.
-        # Collect all command names for post-removal reconciliation.
-        removed_cmd_names = set()
-        for cmd_names in registered_commands.values():
-            removed_cmd_names.update(cmd_names)
         if registered_commands:
             self._unregister_commands(registered_commands)
 
@@ -2415,19 +2418,19 @@ def resolve_content(
         is_command = template_type == "command"
         top_frontmatter_text = None
 
-        def _strip_frontmatter(text: str) -> tuple:
-            """Return (frontmatter_text_with_fences, body) or (None, text)."""
-            if not text.startswith("---"):
-                return None, text
-            end = text.find("---", 3)
-            if end == -1:
-                return None, text
-            fm_block = text[:end + 3]
-            body = text[end + 3:].strip()
-            return fm_block, body
+        def _split_frontmatter(text: str) -> tuple:
+            """Return (frontmatter_block_with_fences, body) or (None, text)."""
+            from .agents import CommandRegistrar
+            fm_dict, body = CommandRegistrar.parse_frontmatter(text)
+            if fm_dict:
+                # Re-extract the raw frontmatter block (text before body)
+                end = text.find("---", 3)
+                fm_block = text[:end + 3]
+                return fm_block, body
+            return None, text
 
         if is_command:
-            fm, body = _strip_frontmatter(content)
+            fm, body = _split_frontmatter(content)
             if fm:
                 top_frontmatter_text = fm
                 content = body
@@ -2438,7 +2441,7 @@ def _strip_frontmatter(text: str) -> tuple:
             strategy = layer["strategy"]
 
             if is_command:
-                fm, layer_body = _strip_frontmatter(layer_content)
+                fm, layer_body = _split_frontmatter(layer_content)
                 layer_content = layer_body
                 # Track the highest-priority frontmatter seen
                 if fm:

tests/test_presets.py

@@ -3460,6 +3460,49 @@ def test_resolve_content_command_type(self, project_dir, temp_dir, valid_pack_da
         assert "Core Plan Command" in content
         assert "Additional Instructions" in content
 
+    def test_resolve_content_command_frontmatter_stripping(self, project_dir, temp_dir, valid_pack_data):
+        """Test that command composition strips frontmatter from lower layers
+        and reattaches only the highest-priority frontmatter."""
+        # Create core command with frontmatter
+        commands_dir = project_dir / ".specify" / "templates" / "commands"
+        commands_dir.mkdir(parents=True, exist_ok=True)
+        (commands_dir / "check.md").write_text(
+            "---\ndescription: Core check command\n---\nCore body content\n"
+        )
+
+        pack_data = {**valid_pack_data}
+        pack_data["preset"] = {**valid_pack_data["preset"], "id": "fm-test", "name": "FmTest"}
+        pack_data["provides"] = {
+            "templates": [{
+                "type": "command",
+                "name": "speckit.check",
+                "file": "commands/speckit.check.md",
+                "strategy": "append",
+            }]
+        }
+        pack_dir = temp_dir / "fm-test"
+        pack_dir.mkdir()
+        with open(pack_dir / "preset.yml", 'w') as f:
+            yaml.dump(pack_data, f)
+        (pack_dir / "commands").mkdir()
+        (pack_dir / "commands" / "speckit.check.md").write_text(
+            "---\ndescription: Preset check override\n---\nPreset body content\n"
+        )
+
+        manager = PresetManager(project_dir)
+        manager.install_from_directory(pack_dir, "0.1.5")
+
+        resolver = PresetResolver(project_dir)
+        content = resolver.resolve_content("speckit.check", "command")
+        assert content is not None
+        # Should have the preset (highest-priority) frontmatter
+        assert "Preset check override" in content
+        # Should have both bodies
+        assert "Core body content" in content
+        assert "Preset body content" in content
+        # Core frontmatter should NOT appear in the body
+        assert content.count("---") == 2  # only one frontmatter block (opening + closing)
+
     def test_resolve_content_blank_line_separator(self, project_dir, temp_dir, valid_pack_data):
         """Test that prepend/append use blank line separator."""
         pack_data = {**valid_pack_data}