fix(presets): address catalog release review

Reject duplicate catalog keys on network and cache reads, validate historical extension requirements against the manifest, and list versions from one resolved snapshot.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: mnriem

docs/reference/presets.md

@@ -212,6 +212,10 @@ SHA-256 digest; optional `requires` and `provides` apply to that release
 instead of inheriting the current release's fields. Other shared metadata,
 such as the name and description, is inherited. Version keys must be distinct,
 including PEP 440-equivalent spellings, and cannot repeat the current version.
+Duplicate JSON keys are rejected before parsing can discard a release record.
+Historical `requires.extensions` entries follow the preset manifest format:
+extension IDs or mappings with an `id`, optional version constraint, and
+optional boolean `required` flag.
 
 ```json
 {

src/specify_cli/presets/_catalog.py

@@ -23,6 +23,22 @@
 from ._manifest import PresetError, PresetValidationError
 
 
+def _decode_catalog_json(raw: str | bytes, url: str) -> Any:
+    """Reject duplicate keys before JSON parsing discards conflicting records."""
+
+    def unique_object(pairs: list[tuple[str, Any]]) -> dict[str, Any]:
+        result: dict[str, Any] = {}
+        for key, value in pairs:
+            if key in result:
+                raise PresetError(
+                    f"Invalid preset catalog format from {url}: duplicate JSON key '{key}'."
+                )
+            result[key] = value
+        return result
+
+    return json.loads(raw, object_pairs_hook=unique_object)
+
+
 @dataclass
 class PresetCatalogEntry:
     """Represents a single entry in the preset catalog stack."""
@@ -426,7 +442,9 @@ def _fetch_single_catalog(self, entry: PresetCatalogEntry, force_refresh: bool =
         # refreshed.
         if not force_refresh and self._is_url_cache_valid(entry.url):
             try:
-                cached_data = json.loads(cache_file.read_text(encoding="utf-8"))
+                cached_data = _decode_catalog_json(
+                    cache_file.read_text(encoding="utf-8"), entry.url
+                )
                 self._validate_catalog_payload(cached_data, entry.url)
                 return cached_data
             except (json.JSONDecodeError, OSError, UnicodeError, PresetError):
@@ -453,13 +471,14 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
                 final_url = response.geturl()
                 if final_url != entry.url:
                     self._validate_catalog_url(final_url)
-                catalog_data = json.loads(
+                catalog_data = _decode_catalog_json(
                     read_response_limited(
                         response,
                         max_bytes=MAX_JSON_CATALOG_BYTES,
                         error_type=PresetError,
                         label=f"preset catalog {entry.url}",
-                    )
+                    ),
+                    entry.url,
                 )
 
             self._validate_catalog_payload(catalog_data, entry.url)
@@ -601,8 +620,8 @@ def fetch_catalog(self, force_refresh: bool = False) -> Dict[str, Any]:
                     self.cache_metadata_file.read_text(encoding="utf-8")
                 )
                 if metadata.get("catalog_url") == catalog_url:
-                    cached_data = json.loads(
-                        self.cache_file.read_text(encoding="utf-8")
+                    cached_data = _decode_catalog_json(
+                        self.cache_file.read_text(encoding="utf-8"), catalog_url
                     )
                     self._validate_catalog_payload(cached_data, catalog_url)
                     return cached_data
@@ -624,13 +643,14 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
                 final_url = response.geturl()
                 if final_url != catalog_url:
                     self._validate_catalog_url(final_url)
-                catalog_data = json.loads(
+                catalog_data = _decode_catalog_json(
                     read_response_limited(
                         response,
                         max_bytes=MAX_JSON_CATALOG_BYTES,
                         error_type=PresetError,
                         label=f"preset catalog {catalog_url}",
-                    )
+                    ),
+                    catalog_url,
                 )
 
             # Validate catalog structure. Reuses the same helper as

src/specify_cli/presets/_catalog_versions.py

@@ -9,7 +9,7 @@
 from packaging.version import InvalidVersion, Version
 
 from .._download_security import is_https_or_localhost_http
-from ._manifest import PresetError
+from ._manifest import PresetError, PresetManifest, PresetValidationError
 
 _SHA256 = re.compile(r"^[0-9a-fA-F]{64}$")
 _CURRENT_FIELDS = frozenset(
@@ -101,10 +101,13 @@ def _validated_releases(entry: dict[str, Any]) -> dict[str, dict[str, Any]]:
                 raise PresetError(
                     f"Preset '{pack_id}' release '{release_version}' has invalid requires.speckit_version."
                 ) from None
-        if "extensions" in requires and not isinstance(requires["extensions"], list):
-            raise PresetError(
-                f"Preset '{pack_id}' release '{release_version}' has invalid requires.extensions."
-            )
+        if "extensions" in requires:
+            try:
+                PresetManifest._validate_requires_extensions(requires["extensions"])
+            except PresetValidationError as exc:
+                raise PresetError(
+                    f"Preset '{pack_id}' release '{release_version}' has {exc}"
+                ) from exc
         if "bundled" in record and not isinstance(record["bundled"], bool):
             raise PresetError(
                 f"Preset '{pack_id}' release '{release_version}' has invalid bundled."

src/specify_cli/presets/command_info.py

@@ -6,6 +6,7 @@
 from rich.markup import escape as _escape_markup
 
 from .._console import console
+from ._catalog_versions import available_versions
 from ._commands import preset_app
 
 
@@ -25,7 +26,7 @@ def preset_info(
         catalog = PresetCatalog(project_root)
         try:
             pack_info = catalog.get_pack_info(preset_id)
-            available = catalog.get_pack_versions(preset_id) if pack_info else []
+            available = available_versions(pack_info) if pack_info else []
         except PresetError as exc:
             console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}")
             raise typer.Exit(1) from exc

tests/specify_cli/presets/test_catalog_versions.py

@@ -4,7 +4,9 @@
 
 import hashlib
 import io
+import json
 import zipfile
+from datetime import datetime, timezone
 from pathlib import Path
 from unittest.mock import MagicMock, patch
 
@@ -82,6 +84,62 @@ def _response(data: bytes, url: str) -> MagicMock:
     return response
 
 
+def _duplicate_release_json() -> bytes:
+    entry = _entry()
+    payload = json.dumps({"schema_version": "1.0", "presets": {"sample": entry}})
+    record = f'"1.0.0": {json.dumps(entry["releases"]["1.0.0"])}'
+    conflicting = {**entry["releases"]["1.0.0"], "download_url": CURRENT_URL}
+    assert record in payload
+    return payload.replace(
+        record, f'{record}, "1.0.0": {json.dumps(conflicting)}', 1
+    ).encode()
+
+
+@pytest.mark.parametrize("legacy", [False, True], ids=["stack", "single-catalog"])
+def test_duplicate_release_key_rejected_from_network(project_dir, legacy):
+    catalog = PresetCatalog(project_dir)
+    url = catalog.DEFAULT_CATALOG_URL
+    entry = PresetCatalogEntry(url, "default", 1, True)
+    with (
+        patch.object(catalog, "get_catalog_url", return_value=url),
+        patch.object(
+            catalog, "_open_url", return_value=_response(_duplicate_release_json(), url)
+        ),
+        pytest.raises(PresetError, match="duplicate.*1.0.0"),
+    ):
+        if legacy:
+            catalog.fetch_catalog(force_refresh=True)
+        else:
+            catalog._fetch_single_catalog(entry, force_refresh=True)
+    assert not catalog.cache_file.exists()
+
+
+@pytest.mark.parametrize("legacy", [False, True], ids=["stack", "single-catalog"])
+def test_duplicate_release_key_in_cache_refetches(project_dir, legacy):
+    catalog = PresetCatalog(project_dir)
+    url = catalog.DEFAULT_CATALOG_URL
+    entry = PresetCatalogEntry(url, "default", 1, True)
+    catalog.cache_dir.mkdir(parents=True)
+    catalog.cache_file.write_bytes(_duplicate_release_json())
+    catalog.cache_metadata_file.write_text(
+        json.dumps({
+            "cached_at": datetime.now(timezone.utc).isoformat(),
+            "catalog_url": url,
+        })
+    )
+    valid = {"schema_version": "1.0", "presets": {"sample": _entry()}}
+    with (
+        patch.object(catalog, "get_catalog_url", return_value=url),
+        patch.object(
+            catalog, "_open_url", return_value=_response(json.dumps(valid).encode(), url)
+        ) as opened,
+    ):
+        result = catalog.fetch_catalog() if legacy else catalog._fetch_single_catalog(entry)
+    assert result == valid
+    opened.assert_called_once()
+    assert json.loads(catalog.cache_file.read_text()) == valid
+
+
 def test_current_and_exact_selection_keep_current_fields(project_dir):
     catalog = PresetCatalog(project_dir)
     entry = _entry()
@@ -204,6 +262,42 @@ def test_malformed_history_rejected_even_for_current(project_dir, change, error)
         catalog.get_pack_info("sample")
 
 
+@pytest.mark.parametrize(
+    "dependencies",
+    [
+        [123],
+        [{}],
+        [{"id": "dep", "version": 2}],
+        [{"id": "dep", "required": 0}],
+        ["bad id"],
+    ],
+)
+def test_historical_release_rejects_malformed_extension_dependencies(
+    project_dir, dependencies
+):
+    entry = _entry()
+    entry["releases"]["1.0.0"]["requires"]["extensions"] = dependencies
+    catalog = PresetCatalog(project_dir)
+    with (
+        patch.object(catalog, "_get_merged_packs", return_value={"sample": entry}),
+        pytest.raises(PresetError, match="requires.extensions"),
+    ):
+        catalog.get_pack_info("sample")
+
+
+def test_historical_release_accepts_manifest_extension_dependencies(project_dir):
+    dependencies = [
+        "plain-ext",
+        {"id": "other-ext", "version": ">=1.2", "required": False},
+    ]
+    entry = _entry()
+    entry["releases"]["1.0.0"]["requires"]["extensions"] = dependencies
+    catalog = PresetCatalog(project_dir)
+    with patch.object(catalog, "_get_merged_packs", return_value={"sample": entry}):
+        selected = catalog.get_pack_info("sample", "1.0.0")
+    assert selected["requires"]["extensions"] == dependencies
+
+
 def test_winning_source_does_not_fall_back_to_lower_release(project_dir):
     catalog = PresetCatalog(project_dir)
     sources = [
@@ -400,6 +494,21 @@ def open_url(_self, url, **_kwargs):
     assert PresetManager(project_dir).get_pack("sample").version == "1.0.0"
 
 
+def test_cli_versions_use_winning_entry_snapshot(project_dir):
+    first = {**_entry(), "_install_allowed": False}
+    second = {"id": "sample", "version": "3.0.0"}
+    with (
+        patch.object(Path, "cwd", return_value=project_dir),
+        patch.object(PresetCatalog, "get_pack_info", side_effect=[first, second]) as lookup,
+    ):
+        result = CliRunner().invoke(app, ["preset", "info", "sample", "--versions"])
+    assert result.exit_code == 0, result.output
+    assert "2.0.0 (current)" in result.output and "1.0.0" in result.output
+    assert "3.0.0" not in result.output
+    assert "Discovery only" in result.output
+    lookup.assert_called_once_with("sample")
+
+
 def test_cli_rejects_missing_release_and_discovery_without_download(project_dir):
     entry = _entry()
     with (