fix: address code review feedback

- list_by_priority(): add secondary sort by ID for deterministic ordering,
  return deep copies to prevent mutation
- install_from_directory/zip: validate priority >= 1 early
- extension add CLI: validate --priority >= 1 before install
- PresetRegistry.update(): preserve installed_at timestamp
- Test assertions: use exact source string instead of substring match

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: iamaeroplane

src/specify_cli/__init__.py

@@ -2831,6 +2831,11 @@ def extension_add(
         console.print("Run this command from a spec-kit project root")
         raise typer.Exit(1)
 
+    # Validate priority
+    if priority < 1:
+        console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)")
+        raise typer.Exit(1)
+
     manager = ExtensionManager(project_root)
     speckit_version = get_speckit_version()
 

src/specify_cli/extensions.py

@@ -328,14 +328,18 @@ def list_by_priority(self) -> List[tuple]:
         """Get all installed extensions sorted by priority.
 
         Lower priority number = higher precedence (checked first).
+        Extensions with equal priority are sorted alphabetically by ID
+        for deterministic ordering.
 
         Returns:
-            List of (extension_id, metadata) tuples sorted by priority
+            List of (extension_id, metadata_copy) tuples sorted by priority.
+            Metadata is deep-copied to prevent accidental mutation.
         """
+        import copy
         extensions = self.data["extensions"]
         return sorted(
-            extensions.items(),
-            key=lambda item: item[1].get("priority", 10),
+            [(ext_id, copy.deepcopy(meta)) for ext_id, meta in extensions.items()],
+            key=lambda item: (item[1].get("priority", 10), item[0]),
         )
 
 
@@ -469,9 +473,13 @@ def install_from_directory(
             Installed extension manifest
 
         Raises:
-            ValidationError: If manifest is invalid
+            ValidationError: If manifest is invalid or priority is invalid
             CompatibilityError: If extension is incompatible
         """
+        # Validate priority
+        if priority < 1:
+            raise ValidationError("Priority must be a positive integer (1 or higher)")
+
         # Load and validate manifest
         manifest_path = source_dir / "extension.yml"
         manifest = ExtensionManifest(manifest_path)
@@ -536,9 +544,13 @@ def install_from_zip(
             Installed extension manifest
 
         Raises:
-            ValidationError: If manifest is invalid
+            ValidationError: If manifest is invalid or priority is invalid
             CompatibilityError: If extension is incompatible
         """
+        # Validate priority early
+        if priority < 1:
+            raise ValidationError("Priority must be a positive integer (1 or higher)")
+
         with tempfile.TemporaryDirectory() as tmpdir:
             temp_path = Path(tmpdir)
 

src/specify_cli/presets.py

@@ -276,6 +276,10 @@ def remove(self, pack_id: str):
     def update(self, pack_id: str, updates: dict):
         """Update preset metadata in registry.
 
+        Merges the provided updates with the existing entry, preserving any
+        fields not specified. The installed_at timestamp is always preserved
+        from the original entry.
+
         Args:
             pack_id: Preset ID
             updates: Partial metadata to merge into existing metadata
@@ -285,7 +289,13 @@ def update(self, pack_id: str, updates: dict):
         """
         if pack_id not in self.data["presets"]:
             raise KeyError(f"Preset '{pack_id}' not found in registry")
-        self.data["presets"][pack_id].update(updates)
+        existing = self.data["presets"][pack_id]
+        # Merge: existing fields preserved, new fields override
+        merged = {**existing, **updates}
+        # Always preserve original installed_at
+        if "installed_at" in existing:
+            merged["installed_at"] = existing["installed_at"]
+        self.data["presets"][pack_id] = merged
         self._save()
 
     def get(self, pack_id: str) -> Optional[dict]:
@@ -311,14 +321,18 @@ def list_by_priority(self) -> List[tuple]:
         """Get all installed presets sorted by priority.
 
         Lower priority number = higher precedence (checked first).
+        Presets with equal priority are sorted alphabetically by ID
+        for deterministic ordering.
 
         Returns:
-            List of (pack_id, metadata) tuples sorted by priority
+            List of (pack_id, metadata_copy) tuples sorted by priority.
+            Metadata is deep-copied to prevent accidental mutation.
         """
+        import copy
         packs = self.data["presets"]
         return sorted(
-            packs.items(),
-            key=lambda item: item[1].get("priority", 10),
+            [(pack_id, copy.deepcopy(meta)) for pack_id, meta in packs.items()],
+            key=lambda item: (item[1].get("priority", 10), item[0]),
         )
 
     def is_installed(self, pack_id: str) -> bool:

tests/test_presets.py

@@ -755,7 +755,7 @@ def test_resolve_with_source_extension(self, project_dir):
         resolver = PresetResolver(project_dir)
         result = resolver.resolve_with_source("unique-template")
         assert result is not None
-        assert "extension:my-ext" in result["source"]
+        assert result["source"] == "extension:my-ext v1.0.0"
 
     def test_resolve_with_source_not_found(self, project_dir):
         """Test resolve_with_source for nonexistent template."""
@@ -996,7 +996,7 @@ def test_override_beats_pack_beats_extension_beats_core(self, project_dir, pack_
         ext_registry.add("my-ext", {"version": "1.0.0", "priority": 10})
 
         result = resolver.resolve_with_source("spec-template")
-        assert "extension:my-ext" in result["source"]
+        assert result["source"] == "extension:my-ext v1.0.0"
 
         # Install pack — should win over extension
         manager = PresetManager(project_dir)