Skip to content

Commit 094f1bf

Browse files
jawwad-aliclaude
andcommitted
fix(bundler): reject falsy non-list 'catalogs' too (is None, not falsy)
Address review: the isinstance guard sat after `if not catalogs: return`, so falsy non-list values (`catalogs: false`, `0`, `''`, `{}`) hit the early return and were silently accepted instead of raising the promised BundlerError. Only an absent/None value means "nothing to merge". Change the early return to `if catalogs is None`, mirroring the sibling reader (commands_impl/catalog_config._read). An empty list stays valid (the merge loop is a no-op). Add parametrized tests for the falsy non-list cases and for the absent/empty-list no-op. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 923e446 commit 094f1bf

2 files changed

Lines changed: 34 additions & 6 deletions

File tree

src/specify_cli/bundler/models/catalog.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -251,14 +251,18 @@ def _merge_config(by_id: dict[str, CatalogSource], config_path: Path, scope: Sco
251251
return
252252
data = load_yaml(config_path)
253253
catalogs = data.get("catalogs") if isinstance(data, dict) else None
254-
if not catalogs:
254+
if catalogs is None:
255255
return
256256
if not isinstance(catalogs, list):
257-
# A non-empty scalar (``catalogs: 5``) passes the falsy check above and
258-
# would raise a raw ``TypeError: 'int' object is not iterable`` from the
259-
# loop below. Report the same actionable BundlerError the sibling reader
260-
# of this file raises (commands_impl/catalog_config.py) so both readers
261-
# of bundle-catalogs.yml agree.
257+
# Treat only an absent/``None`` ``catalogs`` as "nothing to merge"; any
258+
# other non-list value (``catalogs: 5``, ``false``, ``0``, ``''``,
259+
# ``{}``) is a malformed config and must raise, not be silently skipped
260+
# by a falsy check. Otherwise a truthy scalar would raise a raw
261+
# ``TypeError: 'int' object is not iterable`` from the loop below, while
262+
# falsy non-lists would be swallowed. Report the same actionable
263+
# BundlerError the sibling reader of this file raises
264+
# (commands_impl/catalog_config.py) so both readers of
265+
# bundle-catalogs.yml agree. An empty list stays valid (loop is a no-op).
262266
raise BundlerError(
263267
f"Malformed catalog config at {config_path}: 'catalogs' must be a "
264268
f"list, got {type(catalogs).__name__}."

tests/contract/test_catalog_schema.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,30 @@ def test_non_list_catalogs_raises_actionable_error(tmp_path: Path):
5555
load_source_stack(tmp_path)
5656

5757

58+
@pytest.mark.parametrize("value", ["false", "0", "''", "{}"])
59+
def test_falsy_non_list_catalogs_still_raises(tmp_path: Path, value: str):
60+
"""A *falsy* non-list ``catalogs:`` value (false/0/''/{}) must also raise —
61+
only an absent/``None`` value means "nothing to merge". A plain falsy check
62+
would silently swallow these, diverging from the sibling reader."""
63+
make_project(tmp_path)
64+
(tmp_path / ".specify" / "bundle-catalogs.yml").write_text(
65+
f"catalogs: {value}\n", encoding="utf-8"
66+
)
67+
with pytest.raises(BundlerError, match="must be a list"):
68+
load_source_stack(tmp_path)
69+
70+
71+
@pytest.mark.parametrize("body", ["catalogs:\n", "catalogs: []\n"])
72+
def test_absent_or_empty_catalogs_is_noop(tmp_path: Path, body: str):
73+
"""An absent (``None``) or empty-list ``catalogs:`` is valid: it contributes
74+
no project sources and falls back to the built-in default stack."""
75+
make_project(tmp_path)
76+
(tmp_path / ".specify" / "bundle-catalogs.yml").write_text(body, encoding="utf-8")
77+
# Does not raise; still yields the built-in defaults.
78+
sources = load_source_stack(tmp_path)
79+
assert len(sources) > 0
80+
81+
5882
def test_project_config_overrides_same_id(tmp_path: Path):
5983
make_project(tmp_path)
6084
config = {

0 commit comments

Comments
 (0)