From 3bd442187c6073563ead51bbfadfe006b4d85c19 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Mon, 5 Oct 2026 21:25:20 +0500 Subject: [PATCH] fix(bundler): let 'catalog remove' delete a project source overriding a built-in `remove_source` refused any built-in id before looking at what the project config actually held: if target in _BUILTIN_IDS: raise BundlerError( f"'{target}' is a built-in default source and cannot be deleted " "(add a same-id source to override it instead)." ) That message documents the override workflow -- and the same guard then made the override permanent. Reproduced on main: add_source -> OK (documented override). stored: ['community'] remove_source-> REFUSED: 'community' is a built-in default source and cannot be deleted (add a same-id source to override it instead). STILL STORED : ['community'] The user could not undo their own project-scoped entry through the CLI at all; the only way back was hand-editing the config the command exists to manage. Now the built-in id is refused only when there is no project-scoped entry to remove. Deleting the user's own entry simply restores the built-in default. The existing CLI guard (tests/contract/test_bundle_cli.py test_catalog_remove_builtin_is_refused) runs against a project with no such entry and still passes unchanged. Rebased onto current main (files moved in the workflow/bundler restructure). Co-Authored-By: Claude Opus 5.5 (1M context) --- src/specify_cli/bundles/catalog_config.py | 10 ++++-- .../bundles/test_catalog_config.py | 36 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/bundles/catalog_config.py b/src/specify_cli/bundles/catalog_config.py index efa4d1789e..db1833a4ca 100644 --- a/src/specify_cli/bundles/catalog_config.py +++ b/src/specify_cli/bundles/catalog_config.py @@ -225,13 +225,19 @@ def add_source( def remove_source(project_root: Path, id_or_url: str) -> str: target = id_or_url.strip() - if target in _BUILTIN_IDS: + catalogs = _read(project_root) + # Refuse a built-in id only when there is nothing project-scoped to remove. + # This message tells the user to "add a same-id source to override it" -- + # and once they did, the same guard refused to delete that override, so the + # documented workflow had no way back short of hand-editing the config. + # A project-scoped entry is the user's own file and is theirs to remove; + # deleting it simply restores the built-in default. + if target in _BUILTIN_IDS and not any(c.get("id") == target for c in catalogs): raise BundlerError( f"'{target}' is a built-in default source and cannot be deleted " "(add a same-id source to override it instead)." ) - catalogs = _read(project_root) # 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): diff --git a/tests/specify_cli/bundles/test_catalog_config.py b/tests/specify_cli/bundles/test_catalog_config.py index 347f9098e3..19fc79b37a 100644 --- a/tests/specify_cli/bundles/test_catalog_config.py +++ b/tests/specify_cli/bundles/test_catalog_config.py @@ -207,6 +207,42 @@ def test_remove_source_accepts_relative_local_path(tmp_path: Path, monkeypatch): cc.remove_source(project, "sub/cat.json") +def test_remove_deletes_a_project_source_overriding_a_builtin_id(tmp_path: Path): + """The documented override must be undoable. + + `remove_source` refused any built-in id before looking at what the project + config actually held — yet its own error tells the user to "add a same-id + source to override it instead". Once they did, the same guard refused to + delete that override, leaving no CLI path back short of hand-editing the + config. + """ + builtin_id = sorted(cc._BUILTIN_IDS)[0] + project = tmp_path / "proj" + (project / ".specify").mkdir(parents=True) + + cc.add_source( + project, + "https://example.test/override.json", + source_id=builtin_id, + policy="install-allowed", + priority=1, + ) + assert [c["id"] for c in cc._read(project)] == [builtin_id] + + assert cc.remove_source(project, builtin_id) == builtin_id + assert cc._read(project) == [] + + +def test_remove_builtin_without_an_override_is_still_refused(tmp_path: Path): + """Deleting the built-in default itself stays refused.""" + builtin_id = sorted(cc._BUILTIN_IDS)[0] + project = tmp_path / "proj" + (project / ".specify").mkdir(parents=True) + + with pytest.raises(BundlerError, match="built-in default source"): + cc.remove_source(project, builtin_id) + + def test_remove_by_id_does_not_also_delete_canonical_url_match(tmp_path: Path, monkeypatch): """`remove ` must remove only the exact-id source, not also a different source whose url happens to equal the id's canonicalized path. (_canonicalize_url