fix: address catalog idempotency review

Merge current main and escape user-controlled catalog values before Rich rendering. Add regressions for markup-bearing no-op and conflict output across integration, workflow, and step catalogs.

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

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

Author: mnriem

docs/reference/bundles.md

@@ -175,6 +175,10 @@ specify bundle catalog add <url>
 
 Registers a project-scoped catalog source and persists it.
 
+Adding a source is idempotent (identity is the source **id or url**): re-running `catalog add` with the same id/url and identical `--policy`/`--priority` is a successful no-op (exit code 0), so it is safe to include in a re-runnable workflow. Re-adding a matching id/url with *different* settings is rejected as a conflict rather than silently overwriting the existing source — remove it first to change it.
+
+Surrounding whitespace in source ids and URLs is ignored when matching identities and comparing settings. No-ops and conflicts leave the existing configuration unchanged; they do not rewrite stored values to normalize them.
+
 ### Remove a Catalog Source
 
 ```bash

docs/reference/extensions.md

@@ -156,6 +156,12 @@ specify extension catalog add <url>
 
 Adds a catalog to the project's `.specify/extension-catalogs.yml`.
 
+Adding a catalog is idempotent (identity is the catalog **name**): re-running `catalog add` with the same name and identical settings is a successful no-op (exit code 0), so it is safe to include in a re-runnable workflow. Re-adding the same name with *different* settings is rejected as a conflict rather than silently overwriting the existing entry — remove it first to change it.
+
+Surrounding whitespace in catalog names and URLs is ignored when comparing entries and stripped from newly added entries. A no-op leaves the existing configuration unchanged.
+
+Stored priorities may use numeric strings, but YAML booleans (`true`/`false`) are invalid and are never equivalent to integer priorities (`1`/`0`). Re-adding a matching catalog with an invalid stored priority reports a conflict.
+
 ### Remove a Catalog
 
 ```bash

docs/reference/integrations.md

@@ -233,6 +233,8 @@ specify integration catalog add <url>
 
 Adds a custom catalog URL to the project's `.specify/integration-catalogs.yml`. The URL must use HTTPS (except `http://localhost`, `http://127.0.0.1`, or `http://[::1]` for local testing).
 
+Adding a catalog is idempotent (identity is the catalog **URL**): re-running `catalog add` with the same URL and the same (or no) `--name` is a successful no-op (exit code 0), so it is safe to include in a re-runnable workflow. Re-adding the same URL with a *different* `--name` is rejected as a conflict rather than silently overwriting the existing entry — remove it first to change it.
+
 ### Remove a Catalog
 
 ```bash

docs/reference/presets.md

@@ -123,6 +123,12 @@ specify preset catalog add <url>
 
 Adds a catalog to the project's `.specify/preset-catalogs.yml`.
 
+Adding a catalog is idempotent (identity is the catalog **name**): re-running `catalog add` with the same name and identical settings is a successful no-op (exit code 0), so it is safe to include in a re-runnable workflow. Re-adding the same name with *different* settings is rejected as a conflict rather than silently overwriting the existing entry — remove it first to change it.
+
+Surrounding whitespace in catalog URLs is ignored when comparing entries and stripped from newly added entries. A no-op leaves the existing configuration unchanged.
+
+Stored priorities may use numeric strings, but YAML booleans (`true`/`false`) are invalid and are never equivalent to integer priorities (`1`/`0`). Re-adding a matching catalog with an invalid stored priority reports a conflict.
+
 ### Remove a Catalog
 
 ```bash

docs/reference/workflows.md

@@ -405,6 +405,10 @@ specify workflow catalog add <url>
 
 Adds a custom catalog URL to the project's `.specify/workflow-catalogs.yml`.
 
+Adding a catalog is idempotent (identity is the catalog **URL**): re-running `catalog add` with the same URL and the same (or no) `--name` is a successful no-op (exit code 0), so it is safe to include in a re-runnable workflow. Re-adding the same URL with a *different* `--name` is rejected as a conflict rather than silently overwriting the existing entry — remove it first to change it.
+
+Surrounding whitespace in catalog URLs is ignored when comparing entries and stripped from newly added entries. A no-op leaves the existing configuration unchanged.
+
 ### Remove a Catalog
 
 ```bash
@@ -422,6 +426,20 @@ Catalogs are resolved in this order (first match wins):
 3. **User config** — `~/.specify/workflow-catalogs.yml`
 4. **Built-in defaults** — official catalog + community catalog
 
+### Step Catalogs
+
+Custom step types have a separate catalog stack:
+
+```bash
+specify workflow step catalog list
+specify workflow step catalog add <url> [--name <name>]
+specify workflow step catalog remove <index>
+```
+
+`step catalog add` writes to `.specify/step-catalogs.yml`. Like workflow catalogs, step catalogs use the **URL** as their identity, ignoring surrounding whitespace. Adding the same URL with the same (or no) `--name` is a successful no-op (exit code 0) that leaves the configuration unchanged. A different `--name` for that URL is a conflict (exit code 1); remove the existing entry first to change it. New entries store the URL without surrounding whitespace.
+
+`step catalog list` shows the active sources, and `step catalog remove` removes a project entry by its index. Step catalog resolution uses `SPECKIT_STEP_CATALOG_URL`, then the project config, then `~/.specify/step-catalogs.yml`, then built-in defaults.
+
 ## Workflow Definition
 
 Workflows are defined in YAML files. Here is the built-in **Full SDD Cycle** workflow that ships with Spec Kit:

src/specify_cli/bundler/commands_impl/catalog_config.py

@@ -139,7 +139,7 @@ def add_source(
     policy: str,
     priority: int,
     source_id: str | None = None,
-) -> CatalogSource:
+) -> tuple[CatalogSource, str]:
     url = url.strip()
     if not url:
         raise BundlerError("A catalog url is required.")
@@ -186,21 +186,44 @@ def add_source(
     resolved_id = (source_id or _derive_id(url)).strip()
 
     catalogs = _read(project_root)
-    for existing in catalogs:
-        if existing.get("id") == resolved_id or existing.get("url") == url:
-            raise BundlerError(
-                f"Catalog source '{resolved_id}' (or url) already exists in this project."
-            )
-
-    entry = {
+    desired = {
         "id": resolved_id,
         "url": url,
         "priority": int(priority),
         "install_policy": install_policy.value,
     }
-    catalogs.append(entry)
+    for existing in catalogs:
+        if (
+            str(existing.get("id", "")).strip() == resolved_id
+            or str(existing.get("url", "")).strip() == url
+        ):
+            # Idempotent add (#4505): identity is the source id or url. A rerun
+            # requesting the same settings is a successful no-op; differing
+            # settings are a conflict rather than a silent overwrite.
+            #
+            # Parse the matching entry through CatalogSource.from_dict first:
+            # _read() only checks that entries are mappings, so a hand-edited
+            # entry may carry a non-integer priority. Normalizing here surfaces
+            # that as a clean BundlerError (matching catalog parsing) instead of
+            # leaking int()'s ValueError/OverflowError past the CLI's
+            # `except BundlerError`, and lets supported representations (e.g. a
+            # string priority) compare equal to the requested defaults.
+            existing_source = CatalogSource.from_dict(dict(existing), Scope.PROJECT)
+            if (
+                existing_source.id == resolved_id
+                and existing_source.url == url
+                and existing_source.priority == desired["priority"]
+                and existing_source.install_policy.value == desired["install_policy"]
+            ):
+                return existing_source, "unchanged"
+            raise BundlerError(
+                f"Catalog source '{resolved_id}' (or url) already exists in this "
+                "project with different settings. Remove it first to change it."
+            )
+
+    catalogs.append(desired)
     _write(project_root, catalogs)
-    return CatalogSource.from_dict(entry, Scope.PROJECT)
+    return CatalogSource.from_dict(desired, Scope.PROJECT), "added"
 
 
 def remove_source(project_root: Path, id_or_url: str) -> str:

src/specify_cli/commands/bundle/__init__.py

@@ -672,15 +672,21 @@ def catalog_add(
         project_root = require_project_root()
         from ...bundler.commands_impl.catalog_config import add_source
 
-        source = add_source(project_root, url, policy=policy, priority=priority, source_id=source_id)
+        source, status = add_source(project_root, url, policy=policy, priority=priority, source_id=source_id)
     except BundlerError as exc:
         _fail(str(exc))
         return
 
-    console.print(
-        f"[green]✓[/green] Added catalog '{_escape_markup(str(source.id))}' "
-        f"(priority {source.priority}, {source.install_policy.value})."
-    )
+    if status == "unchanged":
+        console.print(
+            f"[green]✓[/green] Catalog '{_escape_markup(str(source.id))}' is already "
+            f"configured (priority {source.priority}, {source.install_policy.value})."
+        )
+    else:
+        console.print(
+            f"[green]✓[/green] Added catalog '{_escape_markup(str(source.id))}' "
+            f"(priority {source.priority}, {source.install_policy.value})."
+        )
 
 
 @bundle_catalog_app.command("remove")

src/specify_cli/extensions/_commands.py

@@ -327,6 +327,36 @@ def install_extension_from_url(
                 pass
 
 
+def _normalize_catalog_priority(value: object) -> int | None:
+    """Normalize a stored catalog priority the way the catalog reader does.
+
+    The reader (``specify_cli/catalogs.py``) accepts integer-string priorities
+    like ``"10"`` but rejects bools. Mirror that here so an equivalent rerun
+    whose persisted priority is a supported string representation is still a
+    no-op rather than a false conflict (#4505). A value that cannot be
+    normalized returns ``None`` so it cannot compare equal to an integer.
+    """
+    if isinstance(value, bool):
+        return None
+    try:
+        return int(value)
+    except (TypeError, ValueError, OverflowError):
+        return None
+
+
+def _normalize_catalog_install_allowed(value: object) -> bool:
+    """Normalize a stored ``install_allowed`` the way the catalog reader does.
+
+    The reader treats the strings ``"true"``/``"yes"``/``"1"`` (case- and
+    whitespace-insensitive) as truthy; everything else falls back to ``bool``.
+    Comparing raw values instead would report ``install_allowed: "false"`` as a
+    conflict because ``bool("false")`` is ``True``.
+    """
+    if isinstance(value, str):
+        return value.strip().lower() in ("true", "yes", "1")
+    return bool(value)
+
+
 def _load_catalog_command_config(project_root: Path, config_path: Path) -> dict:
     """Load extension catalog CLI config with user-facing shape errors."""
     try:
@@ -615,6 +645,8 @@ def catalog_add(
 
     project_root = _require_specify_project()
     specify_dir = project_root / ".specify"
+    url = url.strip()
+    name = name.strip()
 
     # Validate URL
     tmp_catalog = ExtensionCatalog(project_root)
@@ -640,10 +672,30 @@ def catalog_add(
     safe_name = _escape_markup(name)
     safe_url = _escape_markup(url)
 
-    # Check for duplicate name
+    # Idempotent add (#4505): a rerun that requests an identical entry is a
+    # successful no-op so the same `catalog add` can live in a re-runnable
+    # workflow without failing. A same-name entry whose settings differ is
+    # still a conflict — we refuse to silently change priority/install
+    # permissions and ask the user to remove it first.
     for existing in catalogs:
-        if isinstance(existing, dict) and existing.get("name") == name:
-            console.print(f"[yellow]Warning:[/yellow] A catalog named '{safe_name}' already exists.")
+        if isinstance(existing, dict) and str(existing.get("name", "")).strip() == name:
+            if (
+                str(existing.get("url", "")).strip() == url
+                and _normalize_catalog_priority(existing.get("priority")) == priority
+                and _normalize_catalog_install_allowed(
+                    existing.get("install_allowed", False)
+                ) == install_allowed
+                and str(existing.get("description", "")) == description
+            ):
+                console.print(
+                    f"[green]✓[/green] Catalog '[bold]{safe_name}[/bold]' is already "
+                    "configured with these settings; nothing to do."
+                )
+                return
+            console.print(
+                f"[red]Error:[/red] A catalog named '{safe_name}' already exists with "
+                "different settings."
+            )
             console.print("Use 'specify extension catalog remove' first, or choose a different name.")
             raise typer.Exit(1)
 

src/specify_cli/integrations/_query_commands.py

@@ -543,14 +543,20 @@ def integration_catalog_add(
     normalized_url = url.strip()
 
     try:
-        catalog.add_catalog(normalized_url, name)
+        status = 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}")
+        console.print(f"[red]Error:[/red] {_rich_escape(str(exc))}")
         raise typer.Exit(1)
 
-    console.print(f"[green]✓[/green] Catalog source added: {normalized_url}")
+    safe_url = _rich_escape(normalized_url)
+    if status == "unchanged":
+        console.print(
+            f"[green]✓[/green] Catalog source already configured: {safe_url}"
+        )
+    else:
+        console.print(f"[green]✓[/green] Catalog source added: {safe_url}")
 
 
 @integration_catalog_app.command("remove")

src/specify_cli/integrations/catalog.py

@@ -390,14 +390,20 @@ def get_project_catalog_configs(self) -> Optional[List[Dict[str, Any]]]:
             for e in entries
         ]
 
-    def add_catalog(self, url: str, name: Optional[str] = None) -> None:
+    def add_catalog(self, url: str, name: Optional[str] = None) -> str:
         """Add a catalog source to the project-level config file.
 
         The URL is normalized (whitespace stripped) and validated before being
-        written. Duplicate URLs are rejected, including near-duplicates that
-        differ only by surrounding whitespace. Priority is derived as
-        ``max(existing) + 1`` so the new entry sorts last in the resolution
-        order unless the user edits the file manually.
+        written. Identity for an integration catalog is the (normalized) URL.
+        Adding a URL that is already configured is idempotent (#4505): a rerun
+        that requests the same name (or no explicit name) is a successful
+        no-op, while a rerun that requests a *different* name is rejected as a
+        conflict rather than silently overwriting the stored entry. Priority is
+        derived as ``max(existing) + 1`` so a newly added entry sorts last in
+        the resolution order unless the user edits the file manually.
+
+        Returns ``"added"`` when a new entry is written, or ``"unchanged"``
+        when an equivalent entry already existed.
         """
         url = url.strip()
         if not url:
@@ -432,6 +438,7 @@ def add_catalog(self, url: str, name: Optional[str] = None) -> None:
         # Validate each existing entry before mutating anything. Fail fast so
         # we don't silently preserve a corrupt sibling entry or derive a new
         # priority from a bogus value.
+        requested_name = str(name).strip() if name is not None else ""
         existing_priorities: List[int] = []
         valid_catalog_count = 0
         for idx, cat in enumerate(catalogs):
@@ -452,8 +459,14 @@ def add_catalog(self, url: str, name: Optional[str] = None) -> None:
                     f"Invalid catalog entry at index {idx} in {config_path}: {exc}"
                 ) from exc
             if existing_url == url:
+                # Idempotent add (#4505): same URL already configured.
+                existing_name = str(cat.get("name", "")).strip()
+                if not requested_name or requested_name == existing_name:
+                    return "unchanged"
                 raise IntegrationValidationError(
-                    f"Catalog URL already configured: {url}"
+                    f"Catalog URL already configured with a different name "
+                    f"('{existing_name}'): {url}. Remove it first or pass "
+                    f"--name '{existing_name}'."
                 )
             valid_catalog_count += 1
             if "priority" in cat:
@@ -502,6 +515,7 @@ def add_catalog(self, url: str, name: Optional[str] = None) -> None:
                 sort_keys=False,
                 allow_unicode=True,
             )
+        return "added"
 
     def remove_catalog(self, index: int) -> str:
         """Remove a catalog source by 0-based index.