From 426dbb8115e4d2957cddcd8289457ba175616f4c Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Thu, 23 Jul 2026 23:48:10 +0500 Subject: [PATCH 1/2] fix(workflows): guard non-mapping 'workflow:' block in WorkflowDefinition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A present-but-non-mapping top-level `workflow:` block (bare `workflow:` -> YAML null, or `workflow: ` / `workflow: [..]`) crashed WorkflowDefinition.__init__ with AttributeError: the `{}` default of `data.get("workflow", {})` only applies when the key is ABSENT, so a non-dict value reached `workflow.get("id", ...)`. This fires inside from_yaml/ from_string — before validate_workflow can report the malformed shape — and in the CLI escapes as a raw traceback (load_workflow is wrapped to catch only FileNotFoundError/ValueError). Normalize the local `workflow` to {} when it is not a mapping (self.data keeps the raw value so validate_workflow still reports it), mirroring the adjacent default_options guard. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/workflows/engine.py | 8 ++++++++ tests/test_workflows.py | 17 +++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index 98005a790b..eeb8cba715 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -42,6 +42,14 @@ def __init__(self, data: dict[str, Any], source_path: Path | None = None) -> Non self.source_path = source_path workflow = data.get("workflow", {}) + # A present-but-non-mapping ``workflow:`` block (bare ``workflow:`` -> + # None, or ``workflow: ``) would crash the following + # ``workflow.get(...)`` calls with AttributeError before validate can + # report the malformed shape. Normalize the local to {} (self.data + # keeps the raw value for validate_workflow), mirroring the + # default_options guard below. + if not isinstance(workflow, dict): + workflow = {} self.id: str = workflow.get("id", "") self.name: str = workflow.get("name", "") self.version: str = workflow.get("version", "0.0.0") diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 1c29ab56e6..8642bc19ef 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -3612,6 +3612,23 @@ def test_from_string(self, sample_workflow_yaml): assert definition.id == "test-workflow" assert len(definition.inputs) == 2 + @pytest.mark.parametrize( + "block", ["workflow:\nsteps: []\n", "workflow: hi\nsteps: []\n", "workflow: [a]\nsteps: []\n"] + ) + def test_non_mapping_workflow_block_parses_then_validates(self, block): + # A present-but-non-mapping `workflow:` block must not crash construction + # with AttributeError; it should parse (empty header) and let + # validate_workflow report the missing id/name, mirroring how the other + # raw fields are validated later. + from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow + + definition = WorkflowDefinition.from_string(block) # must not raise + assert definition.id == "" + errors = validate_workflow(definition) + assert any("workflow.id" in e for e in errors) + # The raw value is preserved on .data for validation/inspection. + assert "workflow" in definition.data + def test_from_string_invalid(self): from specify_cli.workflows.engine import WorkflowDefinition From 81abc3c85f2ee8bec4bfe088c941c94010654250 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Fri, 24 Jul 2026 12:55:30 +0500 Subject: [PATCH 2/2] test(workflows): assert self.data preserves the raw non-mapping workflow value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review: the previous assertion only proved the key stayed present; it would pass even if construction replaced the malformed value with {}. Assert definition.data["workflow"] equals the original parsed value and is still a non-mapping, proving the guard normalizes only the local variable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_workflows.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 8642bc19ef..bf5af5a3f1 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -3626,8 +3626,15 @@ def test_non_mapping_workflow_block_parses_then_validates(self, block): assert definition.id == "" errors = validate_workflow(definition) assert any("workflow.id" in e for e in errors) - # The raw value is preserved on .data for validation/inspection. - assert "workflow" in definition.data + # The RAW malformed value is preserved on .data (the guard only + # normalizes the local var, not self.data). Assert it was NOT replaced + # with {} by comparing against the original parse and confirming it is + # still a non-mapping. + import yaml + + raw_workflow = yaml.safe_load(block).get("workflow") + assert definition.data["workflow"] == raw_workflow + assert not isinstance(definition.data["workflow"], dict) def test_from_string_invalid(self): from specify_cli.workflows.engine import WorkflowDefinition