fix: reject invalid catalog equivalence

Reject boolean priorities during extension and preset equivalence checks and normalize stored bundle identities before duplicate matching. Add regression coverage and document these rules. Scope CLI test working-directory changes so Windows can clean up temporary projects before fixture teardown.

Assisted-by: GitHub Copilot (model: gpt-6-astra, autonomous)

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

Author: mnriem

docs/reference/bundles.md

@@ -154,6 +154,8 @@ 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

@@ -144,6 +144,8 @@ Adding a catalog is idempotent (identity is the catalog **name**): re-running `c
 
 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/presets.md

@@ -113,6 +113,8 @@ Adding a catalog is idempotent (identity is the catalog **name**): re-running `c
 
 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

src/specify_cli/bundler/commands_impl/catalog_config.py

@@ -193,7 +193,10 @@ def add_source(
         "install_policy": install_policy.value,
     }
     for existing in catalogs:
-        if existing.get("id") == resolved_id or existing.get("url") == url:
+        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.

src/specify_cli/extensions/_commands.py

@@ -320,21 +320,21 @@ def install_extension_from_url(
                 pass
 
 
-def _normalize_catalog_priority(value: object) -> object:
+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 is returned unchanged so it simply fails to compare equal.
+    normalized returns ``None`` so it cannot compare equal to an integer.
     """
     if isinstance(value, bool):
-        return value
+        return None
     try:
         return int(value)
     except (TypeError, ValueError, OverflowError):
-        return value
+        return None
 
 
 def _normalize_catalog_install_allowed(value: object) -> bool:

src/specify_cli/presets/_commands.py

@@ -40,22 +40,22 @@
 preset_app.add_typer(preset_catalog_app, name="catalog")
 
 
-def _normalize_catalog_priority(value: object) -> object:
+def _normalize_catalog_priority(value: object) -> int | None:
     """Normalize a stored catalog priority the way the preset reader does.
 
     The preset reader (``specify_cli/presets/__init__.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 is returned unchanged so it simply fails to
-    compare equal.
+    value that cannot be normalized returns ``None`` so it cannot compare equal
+    to an integer.
     """
     if isinstance(value, bool):
-        return value
+        return None
     try:
         return int(value)
     except (TypeError, ValueError, OverflowError):
-        return value
+        return None
 
 
 def _normalize_catalog_install_allowed(value: object) -> bool:

tests/contract/test_bundle_cli.py

@@ -228,6 +228,32 @@ def test_catalog_add_duplicate_is_idempotent(project: Path):
     assert "already" in second.output
 
 
+@pytest.mark.parametrize("priority,exit_code", [(10, 0), (20, 1)])
+def test_catalog_add_normalizes_stored_identity(project: Path, priority, exit_code):
+    config_path = project / ".specify" / "bundle-catalogs.yml"
+    config_path.write_text(yaml.safe_dump({
+        "schema_version": "1.0",
+        "catalogs": [{
+            "id": " \tlocal\t ",
+            "url": " \thttps://example.com/catalog.json\t ",
+            "priority": 10,
+            "install_policy": "install-allowed",
+        }],
+    }), encoding="utf-8")
+    original = config_path.read_bytes()
+    modified_at = config_path.stat().st_mtime_ns
+
+    result = runner.invoke(app, [
+        "bundle", "catalog", "add", "https://example.com/catalog.json",
+        "--id", "local", "--policy", "install-allowed", "--priority", str(priority),
+    ], catch_exceptions=False)
+
+    assert result.exit_code == exit_code, result.output
+    assert ("already" if exit_code == 0 else "different settings") in result.output
+    assert config_path.read_bytes() == original
+    assert config_path.stat().st_mtime_ns == modified_at
+
+
 def test_catalog_add_duplicate_different_settings_conflicts(project: Path):
     catalog = project / "local-catalog.json"
     write_catalog_file(catalog, {"demo": catalog_entry_dict("demo")})

tests/test_extensions.py

@@ -7468,7 +7468,6 @@ def test_catalog_add_normalizes_existing_identity(
         from typer.testing import CliRunner
         from specify_cli import app
 
-        monkeypatch.chdir(project_dir)
         config_path = project_dir / ".specify" / "extension-catalogs.yml"
         config_path.write_text(yaml.safe_dump({"catalogs": [{
             "name": stored_name,
@@ -7479,11 +7478,13 @@ def test_catalog_add_normalizes_existing_identity(
         original = config_path.read_bytes()
         modified_at = config_path.stat().st_mtime_ns
 
-        result = CliRunner().invoke(app, [
-            "extension", "catalog", "add",
-            f"{padding}https://example.com/catalog.json{padding}",
-            "--name", name, "--priority", str(priority),
-        ], catch_exceptions=False)
+        with monkeypatch.context() as scoped:
+            scoped.chdir(project_dir)
+            result = CliRunner().invoke(app, [
+                "extension", "catalog", "add",
+                f"{padding}https://example.com/catalog.json{padding}",
+                "--name", name, "--priority", str(priority),
+            ], catch_exceptions=False)
 
         assert result.exit_code == (0 if priority == 10 else 1), result.output
         assert ("nothing to do" if priority == 10 else "different settings") in result.output
@@ -7551,6 +7552,36 @@ def test_catalog_add_string_representations_are_idempotent(self, tmp_path):
         assert result.exit_code == 0, result.output
         assert "nothing to do" in result.output
 
+    @pytest.mark.parametrize("stored_priority,priority", [
+        (True, 1), (False, 0), (1, 1), (0, 0), ("1", 1), ("0", 0),
+    ])
+    def test_catalog_add_priority_equivalence(self, project_dir, monkeypatch, stored_priority, priority):
+        from typer.testing import CliRunner
+        from specify_cli import app
+
+        config_path = project_dir / ".specify" / "extension-catalogs.yml"
+        config_path.write_text(yaml.safe_dump({"catalogs": [{
+            "name": "mine",
+            "url": "https://example.com/catalog.json",
+            "priority": stored_priority,
+            "install_allowed": False,
+        }]}), encoding="utf-8")
+        original = config_path.read_bytes()
+        modified_at = config_path.stat().st_mtime_ns
+
+        with monkeypatch.context() as scoped:
+            scoped.chdir(project_dir)
+            result = CliRunner().invoke(app, [
+                "extension", "catalog", "add", "https://example.com/catalog.json",
+                "--name", "mine", "--priority", str(priority),
+            ], catch_exceptions=False)
+
+        invalid = isinstance(stored_priority, bool)
+        assert result.exit_code == (1 if invalid else 0), result.output
+        assert ("different settings" if invalid else "nothing to do") in result.output
+        assert config_path.read_bytes() == original
+        assert config_path.stat().st_mtime_ns == modified_at
+
     def test_catalog_add_escapes_config_saved_path_markup(self, tmp_path):
         """Catalog add's saved-path label should render literally under Rich."""
         from typer.testing import CliRunner

tests/test_presets.py

@@ -3939,7 +3939,6 @@ def test_catalog_add_normalizes_existing_url(
         from typer.testing import CliRunner
         from specify_cli import app
 
-        monkeypatch.chdir(project_dir)
         config_path = project_dir / ".specify" / "preset-catalogs.yml"
         config_path.write_text(yaml.safe_dump({"catalogs": [{
             "name": "mine",
@@ -3950,10 +3949,12 @@ def test_catalog_add_normalizes_existing_url(
         original = config_path.read_bytes()
         modified_at = config_path.stat().st_mtime_ns
 
-        result = CliRunner().invoke(app, [
-            "preset", "catalog", "add", f"{padding}https://example.com/c.json{padding}",
-            "--name", "mine", "--priority", str(priority),
-        ], catch_exceptions=False)
+        with monkeypatch.context() as scoped:
+            scoped.chdir(project_dir)
+            result = CliRunner().invoke(app, [
+                "preset", "catalog", "add", f"{padding}https://example.com/c.json{padding}",
+                "--name", "mine", "--priority", str(priority),
+            ], catch_exceptions=False)
 
         assert result.exit_code == (0 if priority == 10 else 1), result.output
         assert ("nothing to do" if priority == 10 else "different settings") in result.output
@@ -4011,6 +4012,36 @@ def test_catalog_add_string_representations_are_idempotent(self, project_dir):
         assert result.exit_code == 0, result.output
         assert "nothing to do" in result.output
 
+    @pytest.mark.parametrize("stored_priority,priority", [
+        (True, 1), (False, 0), (1, 1), (0, 0), ("1", 1), ("0", 0),
+    ])
+    def test_catalog_add_priority_equivalence(self, project_dir, monkeypatch, stored_priority, priority):
+        from typer.testing import CliRunner
+        from specify_cli import app
+
+        config_path = project_dir / ".specify" / "preset-catalogs.yml"
+        config_path.write_text(yaml.safe_dump({"catalogs": [{
+            "name": "mine",
+            "url": "https://example.com/catalog.json",
+            "priority": stored_priority,
+            "install_allowed": False,
+        }]}), encoding="utf-8")
+        original = config_path.read_bytes()
+        modified_at = config_path.stat().st_mtime_ns
+
+        with monkeypatch.context() as scoped:
+            scoped.chdir(project_dir)
+            result = CliRunner().invoke(app, [
+                "preset", "catalog", "add", "https://example.com/catalog.json",
+                "--name", "mine", "--priority", str(priority),
+            ], catch_exceptions=False)
+
+        invalid = isinstance(stored_priority, bool)
+        assert result.exit_code == (1 if invalid else 0), result.output
+        assert ("different settings" if invalid else "nothing to do") in result.output
+        assert config_path.read_bytes() == original
+        assert config_path.stat().st_mtime_ns == modified_at
+
     def test_catalog_remove_escapes_rich_markup(self, project_dir):
         """`preset catalog remove` must not parse the name as Rich markup."""
         from typer.testing import CliRunner

tests/test_workflows.py

@@ -9764,20 +9764,23 @@ def test_add_catalog_duplicate_outcomes(
         from typer.testing import CliRunner
         from specify_cli import app
 
-        monkeypatch.chdir(project_dir)
         url = "https://example.com/catalog.json"
         name_args = ["--name", name] if name is not None else []
         runner = CliRunner()
-        first = runner.invoke(
-            app, [*command, f"{padding}{url}{padding}", *name_args], catch_exceptions=False
-        )
+        with monkeypatch.context() as scoped:
+            scoped.chdir(project_dir)
+            first = runner.invoke(
+                app, [*command, f"{padding}{url}{padding}", *name_args], catch_exceptions=False
+            )
         assert first.exit_code == 0, first.output
         assert "source added" in first.output
         config_path = project_dir / ".specify" / config_filename
         original = config_path.read_bytes()
         modified_at = config_path.stat().st_mtime_ns
 
-        second = runner.invoke(app, [*command, url, *name_args], catch_exceptions=False)
+        with monkeypatch.context() as scoped:
+            scoped.chdir(project_dir)
+            second = runner.invoke(app, [*command, url, *name_args], catch_exceptions=False)
 
         assert second.exit_code == 0, second.output
         assert "already configured" in second.output
@@ -9789,9 +9792,11 @@ def test_add_catalog_duplicate_outcomes(
         assert entries[0]["url"] == url
         assert entries[0]["name"] == (name or "catalog-1")
 
-        conflict = runner.invoke(
-            app, [*command, url, "--name", "different"], catch_exceptions=False
-        )
+        with monkeypatch.context() as scoped:
+            scoped.chdir(project_dir)
+            conflict = runner.invoke(
+                app, [*command, url, "--name", "different"], catch_exceptions=False
+            )
         assert conflict.exit_code == 1, conflict.output
         assert "different name" in conflict.output
         assert "source added" not in conflict.output
@@ -9808,7 +9813,6 @@ def test_add_catalog_existing_url_outcomes(
         from typer.testing import CliRunner
         from specify_cli import app
 
-        monkeypatch.chdir(project_dir)
         url = "https://example.com/catalog.json"
         config_path = project_dir / ".specify" / config_filename
         config_path.write_text(yaml.safe_dump({"catalogs": [{
@@ -9822,9 +9826,11 @@ def test_add_catalog_existing_url_outcomes(
         modified_at = config_path.stat().st_mtime_ns
         name_args = ["--name", name] if name is not None else []
 
-        result = CliRunner().invoke(
-            app, [*command, f"{padding}{url}{padding}", *name_args], catch_exceptions=False
-        )
+        with monkeypatch.context() as scoped:
+            scoped.chdir(project_dir)
+            result = CliRunner().invoke(
+                app, [*command, f"{padding}{url}{padding}", *name_args], catch_exceptions=False
+            )
 
         if name == "different":
             assert result.exit_code == 1, result.output