From c37134b642d9a0bc0903b4053fe2990927d8c56d Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Sat, 15 Aug 2026 20:46:48 +0500 Subject: [PATCH] fix(workflows): require a 'cases' block on switch steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- .../workflows/steps/switch/__init__.py | 13 ++++++++ tests/test_workflows.py | 32 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/specify_cli/workflows/steps/switch/__init__.py b/src/specify_cli/workflows/steps/switch/__init__.py index 690df0f19a..977c8cbf3e 100644 --- a/src/specify_cli/workflows/steps/switch/__init__.py +++ b/src/specify_cli/workflows/steps/switch/__init__.py @@ -96,6 +96,19 @@ def validate(self, config: dict[str, Any]) -> list[str]: 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( diff --git a/tests/test_workflows.py b/tests/test_workflows.py index afd70adecf..08c61d1440 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -3309,6 +3309,38 @@ def test_validate_missing_expression(self): 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