fix(workflows): reject falsy non-mapping workflow-catalogs.yml top level

[jawwad-ali]

Problem

WorkflowCatalog._load_catalog_config (in src/specify_cli/workflows/catalog.py) parses the project/user workflow-catalogs.yml like this:

data = yaml.safe_load(config_path.read_text(encoding="utf-8")) or {}
...
if not isinstance(data, dict):
    raise WorkflowValidationError(f"Invalid catalog config: expected a mapping, got ...")

The or {} coerces a falsy non-mapping top level to {} before the isinstance guard runs, so those are silently swallowed instead of raising — while a truthy non-mapping raises as intended. Reproduced on main:

top-level value	current behaviour
[], false, 0, '' (falsy non-mappings)	silently returns None → built-in defaults, no error
5, a bare list (truthy non-mappings)	raises expected a mapping ✅
empty document / null	returns None (valid no-op) ✅

So a malformed config like a top-level [] (probably meant catalogs: [...]) is ignored with no diagnostic. This is the same silent-swallow the bundler catalog reader already guards against for its own config.

Fix

Drop the or {} and branch on None explicitly:

data = yaml.safe_load(...)
if data is None:        # empty document / explicit null -> valid no-op
    return None
if not isinstance(data, dict):
    raise WorkflowValidationError(...)   # now catches [], false, 0, '' too

None (empty file / explicit null) stays a valid no-op; every non-mapping — falsy or truthy — now raises the same actionable error. Correct configs are unaffected (no behaviour change).

Verification

• New test_falsy_non_mapping_config_rejected ([]/false/0/''): fails before (silently returns None), passes after (raises).
• New test_empty_or_null_config_is_noop (empty/comment-only/null/~): passes before and after — regression guard that the no-op path is preserved.
• Full TestWorkflowCatalog: 32 passed. ruff clean.

---

AI-assisted: authored with Claude Code. I reproduced the falsy/truthy split on main with a small script before writing the fix, and confirmed the empty/null no-op is preserved.

[Copilot]

Pull request overview

Fixes workflow catalog validation so falsy non-mapping YAML values are rejected while empty or null documents remain valid no-ops.

Changes:

• Handles None explicitly before mapping validation.
• Adds regression tests for falsy non-mappings and empty/null documents.

Show a summary per file

File	Description
src/specify_cli/workflows/catalog.py	Corrects top-level YAML validation.
tests/test_workflows.py	Covers invalid falsy values and valid no-op inputs.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Medium

[Copilot]

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 3
• Review effort level: Medium

[mnriem]

Please address Copilot feedback

[jawwad-ali]

All three addressed in c1dfa8b:

1. Moved the StepCatalog tests into TestStepCatalog — you're right that a targeted pytest ...::TestStepCatalog run skipped them where they were.
2. Added the nested coverage: parametrized falsy catalogs: ({}/''/0/false) plus the absent/null/empty no-op cases. Verified against upstream/main's catalog.py: 8 fail there, 8 pass here, so the duplicated loader can't regress independently now.
3. Removed the parity parenthetical — you're correct that src/specify_cli/catalogs.py is not the behavior this matches: it coerces a null document to {} and raises for missing/empty catalogs rather than treating them as a no-op. The comment now just states what changed (only the misreported shapes) without claiming parity.

[Copilot]

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Medium