diff --git a/src/specify_cli/workflows/steps/switch/__init__.py b/src/specify_cli/workflows/steps/switch/__init__.py index 8a2e4b343e..ff597f9069 100644 --- a/src/specify_cli/workflows/steps/switch/__init__.py +++ b/src/specify_cli/workflows/steps/switch/__init__.py @@ -5,7 +5,11 @@ from typing import Any from specify_cli.workflows.base import StepBase, StepContext, StepResult, StepStatus -from specify_cli.workflows.expressions import evaluate_expression +from specify_cli.workflows.expressions import ( + condition_has_malformed_expression_block, + condition_is_never_evaluated, + evaluate_expression, +) class SwitchStep(StepBase): @@ -107,6 +111,31 @@ def validate(self, config: dict[str, Any]) -> list[str]: f"Switch step {config.get('id', '?')!r} is missing " f"'expression' field." ) + # Presence is not enough. `expression` goes through the same + # `evaluate_expression` as a condition, so one written without braces comes + # back as its own source text: `expression: inputs.mode` matches no case key, + # falls through to `default` on every run -- or dispatches nothing at all when + # there is no default -- and still reports COMPLETED. `if`, `while` and + # `do-while` already reject that shape on their `condition`; this is the same + # fault on the same evaluator, one step type over. + # + # Only these two checks apply. A switch matches on strings, so a composite key + # such as `{{ inputs.a }}-{{ inputs.b }}` is legitimate here even though the + # same shape would be a fault in a boolean condition. + elif condition_is_never_evaluated(config["expression"]): + errors.append( + f"Switch step {config.get('id', '?')!r}: 'expression' " + f"{config['expression']!r} has no usable '{{ }}' block, so it is " + "never evaluated: the literal text is matched against the case keys, " + "which falls through to 'default' on every run." + ) + elif condition_has_malformed_expression_block(config["expression"]): + errors.append( + f"Switch step {config.get('id', '?')!r}: 'expression' " + f"{config['expression']!r} opens a '{{' the interpolator cannot " + "close, so it falls back to the first raw '}}' and matches on a " + "truncated expression instead of the one written." + ) # 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 diff --git a/tests/test_workflows.py b/tests/test_workflows.py index d599f3c6a4..c8325b4eed 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -3403,6 +3403,61 @@ def test_validate_invalid_cases_and_default(self): assert any("case 'a' must be a list" in e for e in errors) assert any("'default' must be a list" in e for e in errors) + def test_expression_without_a_block_is_rejected(self): + """`expression: inputs.mode` matches its own source text, not the input. + + `evaluate_expression` only substitutes `{{ ... }}`, so the braceless form comes + back unchanged, matches no case key, and falls through to `default` on every + run while still reporting COMPLETED. + """ + from specify_cli.workflows.steps.switch import SwitchStep + from specify_cli.workflows.base import StepContext, StepStatus + + config = { + "id": "route", + "expression": "inputs.mode", + "cases": {"review": [{"id": "r", "type": "command", "command": "echo"}]}, + "default": [{"id": "d", "type": "command", "command": "echo"}], + } + + # Ground truth first: this is what the step does with it today. + result = SwitchStep().execute(config, StepContext(inputs={"mode": "review"})) + assert result.status == StepStatus.COMPLETED + assert result.output["matched_case"] == "__default__" + assert result.output["expression_value"] == "inputs.mode" + + errors = [e for e in SwitchStep().validate(config) if "'expression'" in e] + assert len(errors) == 1 + assert "never evaluated" in errors[0] + + def test_expression_with_an_unclosable_block_is_rejected(self): + """Different fault, different message: the block is evaluated, but truncated.""" + from specify_cli.workflows.steps.switch import SwitchStep + + cases = {"review": [{"id": "r", "type": "command", "command": "echo"}]} + for expression in ("{{ inputs.x", "{{ inputs.missing | default('oops }}"): + config = {"id": "route", "expression": expression, "cases": cases} + errors = [ + e for e in SwitchStep().validate(config) if "'expression'" in e + ] + assert len(errors) == 1, expression + + def test_a_composite_key_expression_stays_accepted(self): + """A switch matches on strings, so more than one block is legitimate here. + + This is the boundary that keeps the two condition predicates safe to reuse on + a non-boolean field: `{{ a }}-{{ b }}` is a composite case key, not a fault. + A literal `true` and the empty string are likewise ordinary case keys. + """ + from specify_cli.workflows.steps.switch import SwitchStep + + cases = {"a-b": [{"id": "r", "type": "command", "command": "echo"}]} + for expression in ("{{ inputs.a }}-{{ inputs.b }}", "{{ inputs.mode }}", "true", ""): + config = {"id": "route", "expression": expression, "cases": cases} + assert [ + e for e in SwitchStep().validate(config) if "'expression'" in e + ] == [], expression + class TestWhileStep: """Test the while loop step type."""