refactor(presets): extract shared _manifest_declared_template for resolve()/collect_all_layers()

Both methods reimplemented the manifest-entry lookup + authoritative-fallback
rules independently — the exact duplication that let them diverge and caused the
bug this PR fixes. Extract a single _manifest_declared_template(pack_dir, name,
type) -> (entry, candidate) helper (candidate is the declared file only when it
is_file(); a declared-but-unusable file returns (entry, None) so callers skip the
convention fallback). resolve() and collect_all_layers() now both call it, so
their manifest-first resolution cannot silently diverge again.

Pure refactor, behavior-preserving: full test_presets.py (331) still passes,
including the directory-at-file:, missing-file, and manifest-file-wins cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: jawwad-ali

src/specify_cli/presets/__init__.py

@@ -2574,6 +2574,39 @@ def _get_manifest(self, pack_dir: Path) -> Optional["PresetManifest"]:
                 self._manifest_cache[key] = None
         return self._manifest_cache[key]
 
+    def _manifest_declared_template(
+        self, pack_dir: Path, template_name: str, template_type: str
+    ) -> tuple[dict | None, Path | None]:
+        """Resolve a preset's manifest-declared template entry and usable file.
+
+        Returns ``(entry, candidate)``:
+        - ``entry`` is the matching ``provides.templates`` mapping, or ``None`` if
+          the manifest is absent or does not list this ``(name, type)``.
+        - ``candidate`` is the declared ``file:`` resolved under ``pack_dir`` IFF
+          it is a regular file (``is_file()``); ``None`` otherwise — a missing,
+          empty, or non-file (e.g. directory) declaration yields ``(entry, None)``.
+
+        The manifest is authoritative: when it declares a template (``entry`` is
+        not ``None``) but the file is unusable (``candidate`` is ``None``),
+        callers must NOT fall back to the convention lookup — that would mask a
+        typo or pick up an undeclared file. Shared by ``resolve()`` and
+        ``collect_all_layers()`` so their manifest-first resolution cannot
+        silently diverge again (the divergence this fix addressed).
+        """
+        manifest = self._get_manifest(pack_dir)
+        if not manifest:
+            return None, None
+        for tmpl in manifest.templates:
+            if tmpl.get("name") == template_name and tmpl.get("type") == template_type:
+                file_path = tmpl.get("file")
+                if file_path:
+                    manifest_candidate = pack_dir / file_path
+                    return tmpl, (
+                        manifest_candidate if manifest_candidate.is_file() else None
+                    )
+                return tmpl, None
+        return None, None
+
     def _get_all_extensions_by_priority(self) -> list[tuple[int, str, dict | None]]:
         """Build unified list of registered and unregistered extensions sorted by priority.
 
@@ -2685,32 +2718,17 @@ def resolve(
                 # collect_all_layers()/resolve_content() so resolve() and
                 # resolve_with_source() agree with them instead of returning
                 # the core template (or a stray convention file).
-                manifest_file_path = None
-                manifest_found_entry = False
-                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):
-                            manifest_file_path = tmpl.get("file")
-                            manifest_found_entry = True
-                            break
-                if manifest_file_path:
-                    manifest_candidate = pack_dir / manifest_file_path
-                    # is_file() (not exists()) so a manifest ``file:`` that points
-                    # at a directory is treated as missing rather than returned to
-                    # callers that will read_text() it and crash.
-                    if manifest_candidate.is_file():
-                        return manifest_candidate
-                    # Declared file missing/non-file: skip this pack's convention
-                    # fallback.
-                    continue
-                if manifest_found_entry:
-                    # Manifest lists this template but with an empty/falsey
-                    # ``file`` value (``file`` is a required key per
-                    # PresetManifest._validate(), so this is a non-usable path,
-                    # not a truly absent one). Don't fall through to convention
-                    # — mirrors collect_all_layers().
+                entry, manifest_candidate = self._manifest_declared_template(
+                    pack_dir, template_name, template_type
+                )
+                if manifest_candidate is not None:
+                    return manifest_candidate
+                if entry is not None:
+                    # Manifest declares this template but the file is missing,
+                    # non-file (e.g. a directory), or an empty/falsey ``file``
+                    # value. The manifest is authoritative, so skip this pack's
+                    # convention fallback rather than mask a typo — mirrors
+                    # collect_all_layers().
                     continue
                 for subdir in subdirs:
                     if subdir:
@@ -2979,34 +2997,22 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]:
                 pack_dir = self.presets_dir / pack_id
                 # Read strategy and manifest file path from preset manifest
                 strategy = "replace"
-                manifest_file_path = None
                 manifest_has_strategy = False
-                manifest_found_entry = False
-                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_has_strategy = "strategy" in tmpl
-                            manifest_file_path = tmpl.get("file")
-                            manifest_found_entry = True
-                            break
-                # Use manifest file path if specified, otherwise convention-based
-                # lookup — but only when the manifest doesn't exist or doesn't
-                # list this template, so preset.yml stays authoritative.
+                entry, manifest_candidate = self._manifest_declared_template(
+                    pack_dir, template_name, template_type
+                )
+                if entry is not None:
+                    strategy = entry.get("strategy", "replace")
+                    manifest_has_strategy = "strategy" in entry
+                # Use the manifest's declared file when it's a usable regular file;
+                # only fall back to convention-based lookup when the manifest
+                # doesn't list this template at all, so preset.yml stays
+                # authoritative (a declared-but-unusable file skips convention —
+                # parity with resolve()).
                 candidate = None
-                if manifest_file_path:
-                    manifest_candidate = pack_dir / manifest_file_path
-                    # is_file() (not exists()): a directory at the declared path is
-                    # not a usable layer, so treat it as missing (parity with
-                    # resolve()).
-                    if manifest_candidate.is_file():
-                        candidate = manifest_candidate
-                    # Explicit file path that isn't a regular file: skip convention
-                    # fallback to avoid masking typos or picking up unintended files.
-                elif not manifest_found_entry:
-                    # Manifest doesn't list this template — check convention paths
+                if manifest_candidate is not None:
+                    candidate = manifest_candidate
+                elif entry is None:
                     candidate = _find_in_subdirs(pack_dir)
                 if candidate:
                     # Legacy fallback: if manifest doesn't explicitly declare a