fix(bundle): allow 'catalog remove' by the same relative path used to add (#3242)

* fix(bundle): allow 'catalog remove' by the same relative path used to add

add_source canonicalizes a local catalog path to an absolute url before persisting it, but remove_source compared only the raw input against the stored id/url. So 'bundle catalog remove ./cat.json' could not undo 'bundle catalog add ./cat.json' -- the stored url was absolute, the removal target relative, and they never matched ('No project-scoped catalog source found'). Match the canonicalized form too (a no-op for ids and remote urls), so a local source is removable by the same path it was added with.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(bundle): match catalog removal target exactly first, canonical only as fallback

Address Copilot review: canonicalizing the removal target unconditionally could let 'remove <id>' also delete a different source whose url equals that id's canonicalized path (ids are treated as local paths by _canonicalize_url, empty scheme). Try an exact id/url match first; only fall back to a canonicalized-url match when no exact match is found, so relative-path removal still works without collateral deletion.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/specify_cli/bundler/commands_impl/catalog_config.py

@@ -180,9 +180,18 @@ def remove_source(project_root: Path, id_or_url: str) -> str:
         )
 
     catalogs = _read(project_root)
-    remaining = [
-        c for c in catalogs if c.get("id") != target and c.get("url") != target
-    ]
+    # Prefer an exact id/url match.
+    remaining = [c for c in catalogs if c.get("id") != target and c.get("url") != target]
+    if len(remaining) == len(catalogs):
+        # No exact match. add_source canonicalizes a local path to an absolute
+        # url before storing, so fall back to a canonicalized-url match -- this
+        # lets `remove ./cat.json` undo `add ./cat.json` (stored absolute).
+        # Only as a *fallback*: _canonicalize_url treats a bare id as a local
+        # path (empty scheme), so applying it unconditionally could also delete a
+        # different source whose url equals the id's canonicalized path.
+        canonical = _canonicalize_url(target)
+        if canonical != target:
+            remaining = [c for c in catalogs if c.get("url") != canonical]
     if len(remaining) == len(catalogs):
         raise BundlerError(
             f"No project-scoped catalog source matching '{target}' was found."

tests/unit/test_bundler_catalog_config.py

@@ -69,6 +69,49 @@ def test_add_source_persists_absolute_local_path(tmp_path: Path, monkeypatch):
     assert Path(source.url) == catalog.resolve()
 
 
+def test_remove_source_accepts_relative_local_path(tmp_path: Path, monkeypatch):
+    """add_source stores a local path as an absolute url, so remove_source must
+    accept the same relative path the caller added; otherwise `remove ./cat.json`
+    cannot undo `add ./cat.json`."""
+    project = tmp_path / "proj"
+    (project / ".specify").mkdir(parents=True)
+    catalog = project / "sub" / "cat.json"
+    catalog.parent.mkdir()
+    catalog.write_text("{}", encoding="utf-8")
+    monkeypatch.chdir(project)
+
+    cc.add_source(project, "sub/cat.json", policy="install-allowed", priority=50)
+    # Removing with the same relative path must succeed (stored absolute).
+    removed = cc.remove_source(project, "sub/cat.json")
+    assert removed == "sub/cat.json"
+    # And it is actually gone now.
+    with pytest.raises(BundlerError, match="No project-scoped catalog source"):
+        cc.remove_source(project, "sub/cat.json")
+
+
+def test_remove_by_id_does_not_also_delete_canonical_url_match(tmp_path: Path, monkeypatch):
+    """`remove <id>` must remove only the exact-id source, not also a different
+    source whose url happens to equal the id's canonicalized path. (_canonicalize_url
+    treats a bare id as a local path, so the canonical match is only a fallback when
+    there is no exact id/url match.)"""
+    project = tmp_path / "proj"
+    (project / ".specify").mkdir(parents=True)
+    monkeypatch.chdir(project)
+    # Source A: id "local", a remote url.
+    cc.add_source(
+        project, "https://example.com/a.json", source_id="local",
+        policy="install-allowed", priority=10,
+    )
+    # Source B: a local path that canonicalizes to <cwd>/local, with a distinct id.
+    cc.add_source(project, "local", source_id="bsource", policy="install-allowed", priority=20)
+
+    removed = cc.remove_source(project, "local")
+    assert removed == "local"
+    ids = {c["id"] for c in cc._read(project)}
+    assert "local" not in ids   # the exact-id source was removed
+    assert "bsource" in ids     # the canonical-url source survives (not collateral)
+
+
 def test_add_source_refuses_symlinked_specify_escape(tmp_path: Path):
     project = tmp_path / "proj"
     project.mkdir()