fix: align catalog remove with displayed order

Author: Adr1an04

src/specify_cli/__init__.py

@@ -2744,15 +2744,19 @@ def integration_catalog_add(
     project_root = _require_specify_project()
     catalog = IntegrationCatalog(project_root)
 
+    # Normalize once here so the success message reflects what was actually
+    # stored. ``IntegrationCatalog.add_catalog`` strips again defensively.
+    normalized_url = url.strip()
+
     try:
-        catalog.add_catalog(url, name)
+        catalog.add_catalog(normalized_url, name)
     except IntegrationCatalogError as exc:
         # Covers both URL validation (base class) and config-file validation
         # (IntegrationValidationError subclass).
         console.print(f"[red]Error:[/red] {exc}")
         raise typer.Exit(1)
 
-    console.print(f"[green]✓[/green] Catalog source added: {url}")
+    console.print(f"[green]✓[/green] Catalog source added: {normalized_url}")
 
 
 @integration_catalog_app.command("remove")

src/specify_cli/integrations/catalog.py

@@ -16,7 +16,7 @@
 from dataclasses import dataclass
 from datetime import datetime, timezone
 from pathlib import Path
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, List, Optional, Tuple
 
 import yaml
 from packaging import version as pkg_version
@@ -527,7 +527,18 @@ def add_catalog(self, url: str, name: Optional[str] = None) -> None:
             )
 
     def remove_catalog(self, index: int) -> str:
-        """Remove a catalog source by 0-based index. Returns the removed name."""
+        """Remove a catalog source by 0-based index.
+
+        ``index`` is interpreted in the same display order shown by
+        ``integration catalog list`` (i.e. sorted ascending by priority,
+        with missing priority defaulting to ``yaml_index + 1``, matching
+        ``_load_catalog_config()``). This way, the index a user sees in
+        ``catalog list`` is the index they pass to ``catalog remove``,
+        even if the underlying YAML lists entries in a different order
+        from how they sort by priority.
+
+        Returns the removed catalog's name.
+        """
         config_path = self.project_root / ".specify" / self.CONFIG_FILENAME
         if not config_path.exists():
             raise IntegrationValidationError("No catalog config file found.")
@@ -558,12 +569,33 @@ def remove_catalog(self, index: int) -> str:
                 "Catalog config contains no catalog entries."
             )
 
-        if index < 0 or index >= len(catalogs):
+        # Map displayed index -> raw YAML index using the same priority
+        # defaulting as ``_load_catalog_config``. We deliberately stay
+        # tolerant here (no new validation errors) because the goal is
+        # only to mirror the order shown by ``catalog list``; entries
+        # that ``_load_catalog_config`` would have rejected outright
+        # would have failed ``catalog list`` already.
+        priority_pairs: List[Tuple[int, int]] = []
+        for yaml_idx, item in enumerate(catalogs):
+            if isinstance(item, dict):
+                try:
+                    priority = int(item.get("priority", yaml_idx + 1))
+                except (TypeError, ValueError):
+                    priority = yaml_idx + 1
+            else:
+                priority = yaml_idx + 1
+            priority_pairs.append((priority, yaml_idx))
+        # Stable sort: ties keep their YAML order, matching list-view ordering.
+        priority_pairs.sort(key=lambda p: p[0])
+        display_order: List[int] = [yaml_idx for _, yaml_idx in priority_pairs]
+
+        if index < 0 or index >= len(display_order):
             raise IntegrationValidationError(
-                f"Catalog index {index} out of range (0-{len(catalogs) - 1})."
+                f"Catalog index {index} out of range (0-{len(display_order) - 1})."
             )
 
-        removed = catalogs.pop(index)
+        target_yaml_idx = display_order[index]
+        removed = catalogs.pop(target_yaml_idx)
 
         if catalogs:
             data["catalogs"] = catalogs
@@ -592,8 +624,8 @@ def remove_catalog(self, index: int) -> str:
                 ) from exc
 
         if isinstance(removed, dict):
-            return removed.get("name", f"catalog-{index + 1}")
-        return f"catalog-{index + 1}"
+            return removed.get("name", f"catalog-{target_yaml_idx + 1}")
+        return f"catalog-{target_yaml_idx + 1}"
 
 
 # ---------------------------------------------------------------------------

tests/integrations/test_cli.py

@@ -832,6 +832,41 @@ def test_catalog_add_then_remove_roundtrip(self, tmp_path, monkeypatch):
         assert remove_result.exit_code == 0, remove_result.output
         assert "'mine' removed" in remove_result.output
 
+    def test_catalog_add_strips_whitespace_in_success_output_and_storage(
+        self, tmp_path, monkeypatch
+    ):
+        """Surrounding whitespace in the URL must not appear in the success
+        message or be persisted to the YAML config."""
+        project = self._make_project(tmp_path)
+        monkeypatch.setenv("HOME", str(tmp_path))
+        monkeypatch.setenv("USERPROFILE", str(tmp_path))
+        monkeypatch.delenv("SPECKIT_INTEGRATION_CATALOG_URL", raising=False)
+
+        padded_url = "  https://padded.example.com/catalog.json  "
+        clean_url = "https://padded.example.com/catalog.json"
+
+        add_result = self._invoke(
+            [
+                "integration",
+                "catalog",
+                "add",
+                padded_url,
+                "--name",
+                "padded",
+            ],
+            project,
+        )
+        assert add_result.exit_code == 0, add_result.output
+        assert clean_url in add_result.output
+        assert padded_url not in add_result.output
+
+        cfg_path = project / ".specify" / "integration-catalogs.yml"
+        import yaml as _yaml
+        data = _yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
+        urls = [c["url"] for c in data["catalogs"]]
+        assert clean_url in urls
+        assert padded_url not in urls
+
     def test_catalog_add_rejects_invalid_url(self, tmp_path, monkeypatch):
         project = self._make_project(tmp_path)
         result = self._invoke(

tests/integrations/test_integration_catalog.py

@@ -955,3 +955,95 @@ def test_remove_last_catalog_deletes_file_and_restores_defaults(
         # Follow-up loads fall back to built-in defaults, not an error.
         active = cat.get_active_catalogs()
         assert [e.name for e in active] == ["default", "community"]
+
+    def test_remove_catalog_uses_display_order_with_explicit_priorities(
+        self, tmp_path, monkeypatch
+    ):
+        """`remove_catalog(index)` must remove the entry shown at that index by
+        `catalog list`, not the entry at that raw YAML position."""
+        self._isolate(tmp_path, monkeypatch)
+        # YAML order: alpha (priority=20), beta (priority=10), gamma (priority=15).
+        # Display (sorted by priority asc): beta (10), gamma (15), alpha (20).
+        cfg_path = tmp_path / ".specify" / "integration-catalogs.yml"
+        cfg_path.parent.mkdir(parents=True, exist_ok=True)
+        cfg_path.write_text(
+            yaml.dump(
+                {
+                    "catalogs": [
+                        {"url": "https://alpha.example.com/c.json", "name": "alpha", "priority": 20},
+                        {"url": "https://beta.example.com/c.json", "name": "beta", "priority": 10},
+                        {"url": "https://gamma.example.com/c.json", "name": "gamma", "priority": 15},
+                    ]
+                }
+            ),
+            encoding="utf-8",
+        )
+        cat = IntegrationCatalog(tmp_path)
+
+        # Display index 0 = beta (lowest priority), not alpha (raw YAML idx 0).
+        removed = cat.remove_catalog(0)
+        assert removed == "beta"
+
+        data = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
+        remaining_names = [c["name"] for c in data["catalogs"]]
+        # YAML order is preserved for the survivors; only beta is gone.
+        assert remaining_names == ["alpha", "gamma"]
+
+    def test_remove_catalog_display_order_with_missing_priorities(
+        self, tmp_path, monkeypatch
+    ):
+        """Entries without `priority` default to `idx + 1` (matching
+        `_load_catalog_config`), so display order tracks YAML order and the
+        first display entry is the first YAML entry."""
+        self._isolate(tmp_path, monkeypatch)
+        cfg_path = tmp_path / ".specify" / "integration-catalogs.yml"
+        cfg_path.parent.mkdir(parents=True, exist_ok=True)
+        cfg_path.write_text(
+            yaml.dump(
+                {
+                    "catalogs": [
+                        {"url": "https://one.example.com/c.json", "name": "one"},
+                        {"url": "https://two.example.com/c.json", "name": "two"},
+                        {"url": "https://three.example.com/c.json", "name": "three"},
+                    ]
+                }
+            ),
+            encoding="utf-8",
+        )
+        cat = IntegrationCatalog(tmp_path)
+
+        # Implicit priorities: one=1, two=2, three=3 → display order matches YAML.
+        removed = cat.remove_catalog(0)
+        assert removed == "one"
+
+        data = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
+        assert [c["name"] for c in data["catalogs"]] == ["two", "three"]
+
+    def test_remove_catalog_display_order_mixes_explicit_and_default(
+        self, tmp_path, monkeypatch
+    ):
+        """An explicit low priority should sort ahead of default-priority
+        siblings, even if it appears later in the YAML."""
+        self._isolate(tmp_path, monkeypatch)
+        cfg_path = tmp_path / ".specify" / "integration-catalogs.yml"
+        cfg_path.parent.mkdir(parents=True, exist_ok=True)
+        # Defaults: a=1, b=2 (implicit). Explicit c=0 → display: c, a, b.
+        cfg_path.write_text(
+            yaml.dump(
+                {
+                    "catalogs": [
+                        {"url": "https://a.example.com/c.json", "name": "a"},
+                        {"url": "https://b.example.com/c.json", "name": "b"},
+                        {"url": "https://c.example.com/c.json", "name": "c", "priority": 0},
+                    ]
+                }
+            ),
+            encoding="utf-8",
+        )
+        cat = IntegrationCatalog(tmp_path)
+
+        removed = cat.remove_catalog(0)
+        assert removed == "c"
+
+        data = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
+        assert [c["name"] for c in data["catalogs"]] == ["a", "b"]