fix(extensions): address PR review feedback (#2017)

- _try_correct_command_name: only correct 'X.Y' to 'speckit.ext_id.Y'
  when X matches ext_id, preventing misleading warnings followed by
  install failure due to namespace mismatch
- _validate: add aliases type/string guards matching _collect_manifest
  _command_names defensive checks
- _validate: track command renames and rewrite any hook.*.command
  references that pointed at a renamed command, emitting a warning
- test: fix test_command_name_autocorrect_no_speckit_prefix to use
  ext_id matching the legacy namespace; add namespace-mismatch test
- test: replace redundant preset-catalogs.yml isolation with
  monkeypatch.delenv("SPECKIT_PRESET_CATALOG_URL") so the env var
  cannot bypass catalog restriction in CI environments

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: iamaeroplane

src/specify_cli/extensions.py

@@ -187,7 +187,8 @@ def _validate(self):
         if "commands" not in provides or not provides["commands"]:
             raise ValidationError("Extension must provide at least one command")
 
-        # Validate commands
+        # Validate commands; track renames so hook references can be rewritten.
+        rename_map: Dict[str, str] = {}
         for cmd in provides["commands"]:
             if "name" not in cmd or "file" not in cmd:
                 raise ValidationError("Command missing 'name' or 'file'")
@@ -201,6 +202,7 @@ def _validate(self):
                         f"'speckit.{{extension}}.{{command}}'. Registering as '{corrected}'. "
                         f"The extension author should update the manifest to use this name."
                     )
+                    rename_map[cmd["name"]] = corrected
                     cmd["name"] = corrected
                 else:
                     raise ValidationError(
@@ -209,8 +211,18 @@ def _validate(self):
                     )
 
             # Validate and auto-correct alias name formats
-            aliases = cmd.get("aliases") or []
+            aliases = cmd.get("aliases")
+            if aliases is None:
+                aliases = []
+            if not isinstance(aliases, list):
+                raise ValidationError(
+                    f"Aliases for command '{cmd['name']}' must be a list"
+                )
             for i, alias in enumerate(aliases):
+                if not isinstance(alias, str):
+                    raise ValidationError(
+                        f"Aliases for command '{cmd['name']}' must be strings"
+                    )
                 if not EXTENSION_COMMAND_NAME_PATTERN.match(alias):
                     corrected = self._try_correct_command_name(alias, ext["id"])
                     if corrected:
@@ -219,31 +231,45 @@ def _validate(self):
                             f"'speckit.{{extension}}.{{command}}'. Registering as '{corrected}'. "
                             f"The extension author should update the manifest to use this name."
                         )
+                        rename_map[alias] = corrected
                         aliases[i] = corrected
                     else:
                         raise ValidationError(
                             f"Invalid alias '{alias}': "
                             "must follow pattern 'speckit.{extension}.{command}'"
                         )
 
+        # Rewrite any hook command references that pointed at a renamed command.
+        for hook_name, hook_data in self.data.get("hooks", {}).items():
+            if isinstance(hook_data, dict) and hook_data.get("command") in rename_map:
+                old_ref = hook_data["command"]
+                hook_data["command"] = rename_map[old_ref]
+                self.warnings.append(
+                    f"Hook '{hook_name}' referenced renamed command '{old_ref}'; "
+                    f"updated to '{rename_map[old_ref]}'. "
+                    f"The extension author should update the manifest."
+                )
+
     @staticmethod
     def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]:
         """Try to auto-correct a non-conforming command name to the required pattern.
 
-        Handles the two most common legacy formats used by community extensions:
-          - 'speckit.command'    → 'speckit.{ext_id}.command'
-          - 'extension.command'  → 'speckit.extension.command'
+        Handles the two legacy formats used by community extensions:
+          - 'speckit.command'  → 'speckit.{ext_id}.command'
+          - '{ext_id}.command' → 'speckit.{ext_id}.command'
+
+        The 'X.Y' form is only corrected when X matches ext_id to ensure the
+        result passes the install-time namespace check. Any other prefix is
+        uncorrectable and will produce a ValidationError at the call site.
 
         Returns the corrected name, or None if no safe correction is possible.
         """
         parts = name.split('.')
         if len(parts) == 2:
-            if parts[0] == 'speckit':
+            if parts[0] == 'speckit' or parts[0] == ext_id:
                 candidate = f"speckit.{ext_id}.{parts[1]}"
-            else:
-                candidate = f"speckit.{parts[0]}.{parts[1]}"
-            if EXTENSION_COMMAND_NAME_PATTERN.match(candidate):
-                return candidate
+                if EXTENSION_COMMAND_NAME_PATTERN.match(candidate):
+                    return candidate
         return None
 
     @property

tests/test_extensions.py

@@ -270,10 +270,12 @@ def test_command_name_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_
         assert "speckit.hello" in manifest.warnings[0]
         assert "speckit.test-ext.hello" in manifest.warnings[0]
 
-    def test_command_name_autocorrect_no_speckit_prefix(self, temp_dir, valid_manifest_data):
-        """Test that 'extension.command' is auto-corrected to 'speckit.extension.command'."""
+    def test_command_name_autocorrect_matching_ext_id_prefix(self, temp_dir, valid_manifest_data):
+        """Test that '{ext_id}.command' is auto-corrected to 'speckit.{ext_id}.command'."""
         import yaml
 
+        # Set ext_id to match the legacy namespace so correction is valid
+        valid_manifest_data["extension"]["id"] = "docguard"
         valid_manifest_data["provides"]["commands"][0]["name"] = "docguard.guard"
 
         manifest_path = temp_dir / "extension.yml"
@@ -287,6 +289,20 @@ def test_command_name_autocorrect_no_speckit_prefix(self, temp_dir, valid_manife
         assert "docguard.guard" in manifest.warnings[0]
         assert "speckit.docguard.guard" in manifest.warnings[0]
 
+    def test_command_name_mismatched_namespace_not_corrected(self, temp_dir, valid_manifest_data):
+        """Test that 'X.command' is NOT corrected when X doesn't match ext_id."""
+        import yaml
+
+        # ext_id is "test-ext" but command uses a different namespace
+        valid_manifest_data["provides"]["commands"][0]["name"] = "docguard.guard"
+
+        manifest_path = temp_dir / "extension.yml"
+        with open(manifest_path, 'w') as f:
+            yaml.dump(valid_manifest_data, f)
+
+        with pytest.raises(ValidationError, match="Invalid command name"):
+            ExtensionManifest(manifest_path)
+
     def test_alias_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_data):
         """Test that a legacy 'speckit.command' alias is auto-corrected."""
         import yaml

tests/test_presets.py

@@ -1174,21 +1174,10 @@ def test_search_with_cached_data(self, project_dir, monkeypatch):
         """Test search with cached catalog data."""
         from unittest.mock import patch
 
-        # Only use the default catalog to prevent fetching the community catalog from the network
-        monkeypatch.setenv("SPECKIT_PRESET_CATALOG_URL", PresetCatalog.DEFAULT_CATALOG_URL)
+        monkeypatch.delenv("SPECKIT_PRESET_CATALOG_URL", raising=False)
         catalog = PresetCatalog(project_dir)
         catalog.cache_dir.mkdir(parents=True, exist_ok=True)
 
-        # Restrict to default catalog only — prevents community catalog network calls
-        # which would add extra results and make the count assertions flaky.
-        (project_dir / ".specify" / "preset-catalogs.yml").write_text(
-            f"catalogs:\n"
-            f"  - url: \"{PresetCatalog.DEFAULT_CATALOG_URL}\"\n"
-            f"    name: default\n"
-            f"    priority: 1\n"
-            f"    install_allowed: true\n"
-        )
-
         catalog_data = {
             "schema_version": "1.0",
             "presets": {