diff --git a/src/specify_cli/workflows/steps/fan_out/__init__.py b/src/specify_cli/workflows/steps/fan_out/__init__.py index 22b9c37d43..4eedb6f9f1 100644 --- a/src/specify_cli/workflows/steps/fan_out/__init__.py +++ b/src/specify_cli/workflows/steps/fan_out/__init__.py @@ -25,6 +25,33 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult: max_concurrency = config.get("max_concurrency", 1) step_template = config.get("step", {}) + # The engine does not auto-validate step config (see + # ``WorkflowEngine.load_workflow``). On a COMPLETED fan-out it reads the + # ``step_template`` back out and, when it is truthy, calls + # ``template.get("id", ...)`` in ``_run_fan_out``. A truthy non-mapping + # ``step`` (a scalar or list authoring mistake) would crash the whole + # run with AttributeError there — the engine invokes ``execute`` and + # ``_run_fan_out`` with no surrounding try/except. ``validate`` already + # rejects a non-mapping ``step``; fail this step loudly on an + # unvalidated run instead, mirroring the ``items`` guard below. An empty + # or absent ``step`` defaults to ``{}`` (falsy) and the engine's + # ``if template and items`` skips fan-out, so it stays valid here. + if not isinstance(step_template, dict): + return StepResult( + status=StepStatus.FAILED, + error=( + f"Fan-out step {config.get('id', '?')!r}: 'step' must be a " + f"mapping (nested step template), got " + f"{type(step_template).__name__}." + ), + output={ + "items": [], + "max_concurrency": max_concurrency, + "step_template": {}, + "item_count": 0, + }, + ) + if not isinstance(items, list): # A non-list here is a wiring error (the expression did not # resolve to a collection); silently fanning out over zero diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 8fefad740e..340f921e4e 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -2426,6 +2426,36 @@ def test_execute_empty_list_items_is_valid(self): assert result.status == StepStatus.COMPLETED assert result.output["item_count"] == 0 + def test_execute_non_dict_step_fails_loudly(self): + """A truthy non-mapping ``step`` must fail the step, not crash the run. + + ``validate`` rejects a non-dict ``step``, but the engine's ``execute()`` + does not auto-validate (see ``WorkflowEngine.load_workflow``). On a + COMPLETED fan-out the engine reads ``step_template`` back out and, when + it is truthy, calls ``template.get("id", ...)`` in ``_run_fan_out``. A + truthy non-mapping ``step`` (a scalar or list authoring mistake) raised + AttributeError there and took down the whole run. Mirrors the fan-out + non-list ``items`` guard and the switch non-dict ``cases`` guard. + """ + from specify_cli.workflows.steps.fan_out import FanOutStep + from specify_cli.workflows.base import StepContext, StepStatus + + step = FanOutStep() + ctx = StepContext(steps={"tasks": {"output": {"task_list": [1, 2]}}}) + for bad_step in (["impl"], "impl", 5): + result = step.execute( + { + "id": "parallel", + "items": "{{ steps.tasks.output.task_list }}", + "step": bad_step, + }, + ctx, + ) + assert result.status == StepStatus.FAILED + assert "'step' must be a" in (result.error or "") + assert result.output["item_count"] == 0 + assert result.output["step_template"] == {} + def test_validate_missing_fields(self): from specify_cli.workflows.steps.fan_out import FanOutStep