From b019c5912a0ba18d373241ef7d1b3e54f03dc92f Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Fri, 11 Sep 2026 21:10:04 +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. Co-Authored-By: Claude Opus 5 (1M context) --- .../bundler/commands_impl/catalog_config.py | 10 ++++-- tests/unit/test_bundler_catalog_config.py | 36 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/bundler/commands_impl/catalog_config.py b/src/specify_cli/bundler/commands_impl/catalog_config.py index f763a21c65..c4d303506c 100644 --- a/src/specify_cli/bundler/commands_impl/catalog_config.py +++ b/src/specify_cli/bundler/commands_impl/catalog_config.py @@ -205,13 +205,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/unit/test_bundler_catalog_config.py b/tests/unit/test_bundler_catalog_config.py index 46c333700a..6cd619875e 100644 --- a/tests/unit/test_bundler_catalog_config.py +++ b/tests/unit/test_bundler_catalog_config.py @@ -89,6 +89,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