fix: address eighteenth round of Copilot PR review feedback

- Add public register_commands_for_non_skill_agents() on CommandRegistrar:
  reconciliation no longer accesses private _ensure_configs() or iterates
  AGENT_CONFIGS directly
- Cache parsed preset manifests in PresetResolver: _get_manifest() caches
  by pack_dir to avoid re-parsing YAML on each collect_all_layers() call
- Fix frontmatter detection for empty dicts: detect based on --- fence
  presence rather than parsed dict truthiness, so empty frontmatter
  blocks are properly stripped during command composition

Author: mnriem

src/specify_cli/agents.py

@@ -620,6 +620,46 @@ def register_commands_for_all_agents(
 
         return results
 
+    def register_commands_for_non_skill_agents(
+        self,
+        commands: List[Dict[str, Any]],
+        source_id: str,
+        source_dir: Path,
+        project_root: Path,
+    ) -> Dict[str, List[str]]:
+        """Register commands for all non-skill agents in the project.
+
+        Like register_commands_for_all_agents but skips skill-based agents
+        (those with extension '/SKILL.md'). Used by reconciliation to avoid
+        overwriting properly formatted SKILL.md files.
+
+        Args:
+            commands: List of command info dicts
+            source_id: Identifier of the source
+            source_dir: Directory containing command source files
+            project_root: Path to project root
+
+        Returns:
+            Dictionary mapping agent names to list of registered commands
+        """
+        results = {}
+        self._ensure_configs()
+        for agent_name, agent_config in self.AGENT_CONFIGS.items():
+            if agent_config.get("extension") == "/SKILL.md":
+                continue
+            agent_dir = project_root / agent_config["dir"]
+            if agent_dir.exists():
+                try:
+                    registered = self.register_commands(
+                        agent_name, commands, source_id,
+                        source_dir, project_root,
+                    )
+                    if registered:
+                        results[agent_name] = registered
+                except ValueError:
+                    continue
+        return results
+
     def unregister_commands(
         self, registered_commands: Dict[str, List[str]], project_root: Path
     ) -> None:

src/specify_cli/presets.py

@@ -777,19 +777,9 @@ def _register_for_non_skill_agents(
         Used during reconciliation to avoid overwriting properly formatted
         SKILL.md files that were written by _register_skills().
         """
-        registrar._ensure_configs()
-        for agent_name, agent_config in registrar.AGENT_CONFIGS.items():
-            if agent_config.get("extension") == "/SKILL.md":
-                continue
-            agent_dir = self.project_root / agent_config["dir"]
-            if agent_dir.exists():
-                try:
-                    registrar.register_commands(
-                        agent_name, commands, source_id,
-                        source_dir, self.project_root,
-                    )
-                except ValueError:
-                    continue
+        registrar.register_commands_for_non_skill_agents(
+            commands, source_id, source_dir, self.project_root
+        )
 
     def _reconcile_skills(self, command_names: List[str]) -> None:
         """Re-register skills for commands whose winning layer changed.
@@ -2018,6 +2008,21 @@ def __init__(self, project_root: Path):
         self.presets_dir = project_root / ".specify" / "presets"
         self.overrides_dir = self.templates_dir / "overrides"
         self.extensions_dir = project_root / ".specify" / "extensions"
+        self._manifest_cache: Dict[str, Optional["PresetManifest"]] = {}
+
+    def _get_manifest(self, pack_dir: Path) -> Optional["PresetManifest"]:
+        """Get a cached preset manifest, parsing it on first access."""
+        key = str(pack_dir)
+        if key not in self._manifest_cache:
+            manifest_path = pack_dir / "preset.yml"
+            if manifest_path.exists():
+                try:
+                    self._manifest_cache[key] = PresetManifest(manifest_path)
+                except PresetValidationError:
+                    self._manifest_cache[key] = None
+            else:
+                self._manifest_cache[key] = None
+        return self._manifest_cache[key]
 
     def _get_all_extensions_by_priority(self) -> list[tuple[int, str, dict | None]]:
         """Build unified list of registered and unregistered extensions sorted by priority.
@@ -2285,20 +2290,14 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]:
                 # Read strategy and manifest file path from preset manifest
                 strategy = "replace"
                 manifest_file_path = None
-                manifest_path = pack_dir / "preset.yml"
-                if manifest_path.exists():
-                    try:
-                        manifest = PresetManifest(manifest_path)
-                        for tmpl in manifest.templates:
-                            if (tmpl.get("name") == template_name
-                                    and tmpl.get("type") == template_type):
-                                strategy = tmpl.get("strategy", "replace")
-                                manifest_file_path = tmpl.get("file")
-                                break
-                    except PresetValidationError:
-                        # Invalid manifest — fall back to default "replace"
-                        # strategy so layer resolution still works.
-                        pass
+                manifest = self._get_manifest(pack_dir)
+                if manifest:
+                    for tmpl in manifest.templates:
+                        if (tmpl.get("name") == template_name
+                                and tmpl.get("type") == template_type):
+                            strategy = tmpl.get("strategy", "replace")
+                            manifest_file_path = tmpl.get("file")
+                            break
                 # Use manifest file path if specified, otherwise convention-based lookup
                 candidate = None
                 if manifest_file_path:
@@ -2444,15 +2443,20 @@ def resolve_content(
         top_frontmatter_text = None
 
         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
+            """Return (frontmatter_block_with_fences, body) or (None, text).
+
+            Detects frontmatter by the presence of ``---`` fences rather than
+            whether the parsed YAML has keys, so empty frontmatter blocks
+            (``---\\n---``) are also stripped.
+            """
+            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
 
         if is_command:
             fm, body = _split_frontmatter(content)