From a7815c6cc5267ba3d0229a3fa10c70c5bb816c87 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 | 11 +++++++++++ tests/test_workflows.py | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index 03afaee74e..13fd633338 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -42,6 +42,17 @@ 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, so construction fails + # before any validation can run. Normalize the local to {} instead: the + # header fields fall back to their defaults and ``validate_workflow`` + # (which reads those parsed attributes) reports the missing + # ``workflow.id``/``workflow.name``. ``self.data`` is deliberately left + # holding the raw value, since it is what gets written back out when a + # definition is serialized. Mirrors 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 54cea4d770..98aa55b719 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -3629,6 +3629,24 @@ def test_resolve_inputs_tolerates_non_mapping_inputs(self, block): resolved = WorkflowEngine()._resolve_inputs(definition, {}) # must not raise assert resolved == {} + @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 to an empty header so + # validate_workflow reports the missing id/name (it reads the parsed + # attributes, not the raw block). + 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 left untouched on .data (used when the definition is + # written back out), so the guard normalizes only the local. + assert "workflow" in definition.data + def test_from_string_invalid(self): from specify_cli.workflows.engine import WorkflowDefinition From 4b26886f8e845b9c327f58fbc6d0da6a8382f395 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 | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 98aa55b719..61575d71df 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -3643,9 +3643,16 @@ 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 left untouched on .data (used when the definition is - # written back out), so the guard normalizes only the local. - assert "workflow" in definition.data + # The RAW malformed value is preserved on .data (the guard only + # normalizes the local var, not self.data) — .data is what gets written + # back out when a definition is serialized. 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