Skip to content

Commit 3bd4421

Browse files
jawwad-aliclaude
andcommitted
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) <noreply@anthropic.com>
1 parent 5364d7c commit 3bd4421

2 files changed

Lines changed: 44 additions & 2 deletions

File tree

‎src/specify_cli/bundles/catalog_config.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,19 @@ def add_source(
225225

226226
def remove_source(project_root: Path, id_or_url: str) -> str:
227227
target = id_or_url.strip()
228-
if target in _BUILTIN_IDS:
228+
catalogs = _read(project_root)
229+
# Refuse a built-in id only when there is nothing project-scoped to remove.
230+
# This message tells the user to "add a same-id source to override it" --
231+
# and once they did, the same guard refused to delete that override, so the
232+
# documented workflow had no way back short of hand-editing the config.
233+
# A project-scoped entry is the user's own file and is theirs to remove;
234+
# deleting it simply restores the built-in default.
235+
if target in _BUILTIN_IDS and not any(c.get("id") == target for c in catalogs):
229236
raise BundlerError(
230237
f"'{target}' is a built-in default source and cannot be deleted "
231238
"(add a same-id source to override it instead)."
232239
)
233240

234-
catalogs = _read(project_root)
235241
# Prefer an exact id/url match.
236242
remaining = [c for c in catalogs if c.get("id") != target and c.get("url") != target]
237243
if len(remaining) == len(catalogs):

‎tests/specify_cli/bundles/test_catalog_config.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,42 @@ def test_remove_source_accepts_relative_local_path(tmp_path: Path, monkeypatch):
207207
cc.remove_source(project, "sub/cat.json")
208208

209209

210+
def test_remove_deletes_a_project_source_overriding_a_builtin_id(tmp_path: Path):
211+
"""The documented override must be undoable.
212+
213+
`remove_source` refused any built-in id before looking at what the project
214+
config actually held — yet its own error tells the user to "add a same-id
215+
source to override it instead". Once they did, the same guard refused to
216+
delete that override, leaving no CLI path back short of hand-editing the
217+
config.
218+
"""
219+
builtin_id = sorted(cc._BUILTIN_IDS)[0]
220+
project = tmp_path / "proj"
221+
(project / ".specify").mkdir(parents=True)
222+
223+
cc.add_source(
224+
project,
225+
"https://example.test/override.json",
226+
source_id=builtin_id,
227+
policy="install-allowed",
228+
priority=1,
229+
)
230+
assert [c["id"] for c in cc._read(project)] == [builtin_id]
231+
232+
assert cc.remove_source(project, builtin_id) == builtin_id
233+
assert cc._read(project) == []
234+
235+
236+
def test_remove_builtin_without_an_override_is_still_refused(tmp_path: Path):
237+
"""Deleting the built-in default itself stays refused."""
238+
builtin_id = sorted(cc._BUILTIN_IDS)[0]
239+
project = tmp_path / "proj"
240+
(project / ".specify").mkdir(parents=True)
241+
242+
with pytest.raises(BundlerError, match="built-in default source"):
243+
cc.remove_source(project, builtin_id)
244+
245+
210246
def test_remove_by_id_does_not_also_delete_canonical_url_match(tmp_path: Path, monkeypatch):
211247
"""`remove <id>` must remove only the exact-id source, not also a different
212248
source whose url happens to equal the id's canonicalized path. (_canonicalize_url

0 commit comments

Comments
 (0)