fix(workflows): require a 'cases' block on switch steps (#4144)
`SwitchStep.validate` requires `expression` and type-checks `cases`, but
never checks that `cases` is PRESENT. It is the only control-flow step whose
branch payload is optional:
if -> requires 'then'
fan-out -> requires 'items' and 'step'
fan-in -> requires a non-empty 'wait_for'
gate -> requires 'message'
switch -> cases optional
So a switch whose branch table is absent or mistyped — `case:` for `cases:`
is the obvious slip — passes validation with zero errors:
if missing then : ["If step 'x' is missing 'then' field."]
fanout missing all: ["Fan-out step 'y' is missing 'items' field.", ...]
switch typo case: : []
switch no cases : []
and then at run time reports COMPLETED with
`matched_case: "__default__"` — a default it does not even declare — having
dispatched nothing, so the whole run "succeeds". That is the "silent empty
result + COMPLETED" wiring bug the fan-in guard exists to prevent.
An explicitly declared but empty `cases: {}` is still a declaration and
stays valid, pinned by a test.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -107,6 +107,19 @@ class SwitchStep(StepBase):
|
||||
f"Switch step {config.get('id', '?')!r} is missing "
|
||||
f"'expression' field."
|
||||
)
|
||||
# Every other control-flow step requires its branch payload: ``if``
|
||||
# requires ``then``, ``fan-out`` requires ``items`` and ``step``,
|
||||
# ``fan-in`` a non-empty ``wait_for``, ``gate`` a ``message``. Without
|
||||
# the same check, a switch whose ``cases:`` block is missing or mistyped
|
||||
# (``case:`` is the obvious slip) validates clean and then reports
|
||||
# COMPLETED with ``matched_case: "__default__"`` -- a default it may not
|
||||
# even declare -- having dispatched nothing. That is the "silent empty
|
||||
# result + COMPLETED" wiring bug the fan-in guard exists to prevent.
|
||||
if "cases" not in config:
|
||||
errors.append(
|
||||
f"Switch step {config.get('id', '?')!r} is missing "
|
||||
f"'cases' field."
|
||||
)
|
||||
cases = config.get("cases", {})
|
||||
if not isinstance(cases, dict):
|
||||
errors.append(
|
||||
|
||||
@@ -3358,6 +3358,38 @@ class TestSwitchStep:
|
||||
errors = step.validate({"id": "test", "cases": {}})
|
||||
assert any("missing 'expression'" in e for e in errors)
|
||||
|
||||
def test_validate_missing_cases(self):
|
||||
"""`cases` is the switch's branch payload and must be required.
|
||||
|
||||
Every other control-flow step requires its own: `if` requires `then`,
|
||||
`fan-out` requires `items` and `step`, `fan-in` a non-empty `wait_for`,
|
||||
`gate` a `message`. Without it, a `case:` typo validated clean and then
|
||||
reported COMPLETED with `matched_case: "__default__"` having dispatched
|
||||
nothing.
|
||||
"""
|
||||
from specify_cli.workflows.steps.switch import SwitchStep
|
||||
|
||||
step = SwitchStep()
|
||||
|
||||
# Absent entirely.
|
||||
errors = step.validate({"id": "route", "expression": "{{ inputs.x }}"})
|
||||
assert any("missing 'cases'" in e for e in errors), errors
|
||||
|
||||
# The realistic slip: `case:` instead of `cases:`.
|
||||
errors = step.validate(
|
||||
{"id": "route", "expression": "{{ inputs.x }}", "case": {"a": []}}
|
||||
)
|
||||
assert any("missing 'cases'" in e for e in errors), errors
|
||||
|
||||
def test_validate_accepts_an_empty_cases_mapping(self):
|
||||
"""An explicitly declared but empty `cases:` is still a declaration."""
|
||||
from specify_cli.workflows.steps.switch import SwitchStep
|
||||
|
||||
errors = SwitchStep().validate(
|
||||
{"id": "route", "expression": "{{ inputs.x }}", "cases": {}}
|
||||
)
|
||||
assert not any("missing 'cases'" in e for e in errors), errors
|
||||
|
||||
def test_validate_invalid_cases_and_default(self):
|
||||
from specify_cli.workflows.steps.switch import SwitchStep
|
||||
|
||||
|
||||
Reference in New Issue
Block a user