Skip to content

Commit fb92c65

Browse files
fix(workflows): fail switch step on non-mapping cases instead of crashing
`SwitchStep.validate()` already rejects a non-mapping `cases`, but the engine's `execute()` path does not auto-validate (see `WorkflowEngine.load_workflow`, whose docstring notes the definition is "not yet validated"). On an unvalidated run, `execute` called `cases.items()` on the raw value, so a list or scalar `cases` authoring mistake raised `AttributeError` and took down the whole run — the engine invokes `step_impl.execute()` with no surrounding try/except. Guard `execute` to return a FAILED StepResult naming the type error instead, mirroring the fan-out step's non-list `items` handling. The expression is still evaluated first, so its value is surfaced in the step output for downstream context. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1be4299 commit fb92c65

2 files changed

Lines changed: 43 additions & 0 deletions

File tree

src/specify_cli/workflows/steps/switch/__init__.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,20 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
2626
str_value = str(value) if value is not None else ""
2727

2828
cases = config.get("cases", {})
29+
if not isinstance(cases, dict):
30+
# The engine does not auto-validate step config, so an unvalidated
31+
# run with a non-mapping ``cases`` (a list/scalar authoring mistake)
32+
# would otherwise raise AttributeError from ``.items()`` below and
33+
# crash the whole run. Fail this step loudly instead, mirroring the
34+
# fan-out step's non-list ``items`` handling.
35+
return StepResult(
36+
status=StepStatus.FAILED,
37+
error=(
38+
f"Switch step {config.get('id', '?')!r}: 'cases' must be a "
39+
f"mapping, got {type(cases).__name__}."
40+
),
41+
output={"matched_case": None, "expression_value": value},
42+
)
2943
for case_key, case_steps in cases.items():
3044
if str(case_key) == str_value:
3145
return StepResult(

tests/test_workflows.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2052,6 +2052,35 @@ def test_execute_no_default_no_match(self):
20522052
assert result.output["matched_case"] == "__default__"
20532053
assert result.next_steps == []
20542054

2055+
def test_execute_non_dict_cases_fails_loudly(self):
2056+
"""A non-mapping ``cases`` must fail the step, not crash the run.
2057+
2058+
``validate`` rejects a non-dict ``cases``, but the engine's
2059+
``execute()`` does not auto-validate (see ``WorkflowEngine.load_workflow``
2060+
docstring). Before the guard, ``execute`` called ``cases.items()`` on the
2061+
raw value, so an unvalidated run with a list/scalar ``cases`` raised
2062+
AttributeError and took down the whole run instead of failing this step.
2063+
Mirrors the fan-out step's non-list ``items`` handling.
2064+
"""
2065+
from specify_cli.workflows.steps.switch import SwitchStep
2066+
from specify_cli.workflows.base import StepContext, StepStatus
2067+
2068+
step = SwitchStep()
2069+
ctx = StepContext(steps={"review": {"output": {"choice": "approve"}}})
2070+
for bad_cases in (["approve"], "approve", 5):
2071+
result = step.execute(
2072+
{
2073+
"id": "route",
2074+
"expression": "{{ steps.review.output.choice }}",
2075+
"cases": bad_cases,
2076+
},
2077+
ctx,
2078+
)
2079+
assert result.status == StepStatus.FAILED
2080+
assert "'cases' must be a mapping" in (result.error or "")
2081+
# expression is still evaluated, so its value is surfaced for context.
2082+
assert result.output["expression_value"] == "approve"
2083+
20552084
def test_validate_missing_expression(self):
20562085
from specify_cli.workflows.steps.switch import SwitchStep
20572086

0 commit comments

Comments
 (0)