fix(workflows): reject a switch expression that is never evaluated

src/specify_cli/workflows/expressions.py

@@ -857,6 +857,30 @@ def condition_is_never_evaluated(condition: Any) -> bool:
     return _first_unclosable_block(stripped) == "verbatim"
 
 
+def switch_expression_is_never_evaluated(expression: Any) -> bool:
+    """True when a switch *expression* is a reference written without its braces.
+
+    ``condition_is_never_evaluated`` cannot be reused here as it stands. It flags
+    every braceless string, because a condition is coerced by ``bool()`` and any
+    non-empty text is therefore always true. A switch instead matches its resolved
+    value against case keys, and a case key is a literal: ``expression: review``
+    resolves to ``"review"`` and dispatches the ``review:`` case, and whitespace
+    strips to the ``""`` key. Both are valid, if constant, switches.
+
+    What is never evaluated is text plainly meant as an expression -- one that opens
+    by walking into a root ``_build_namespace`` supplies, such as ``inputs.mode``.
+    It is matched against the case keys as its own source text, so it falls through
+    to ``default`` on every run. An opening ``{{`` the interpolator emits verbatim
+    is flagged for the same reason, exactly as it is for a condition.
+    """
+    if not isinstance(expression, str):
+        return False
+    stripped = expression.strip()
+    if "{{" in stripped:
+        return _first_unclosable_block(stripped) == "verbatim"
+    return _NAMESPACE_REFERENCE.match(stripped) is not None
+
+
 def condition_is_interpolated_to_text(condition: Any) -> bool:
     """True when *condition* holds ``{{ }}`` blocks but is spliced into text, not evaluated.
 
@@ -1086,6 +1110,11 @@ def _has_incomplete_operand(text: str) -> bool:
 # None, so a correction built on one turns a truthy condition false.
 _NAMESPACE_ROOTS = ("inputs", "steps", "item", "fan_in", "context")
 
+# Text that opens by walking into one of those roots: `inputs.mode`, `item[0]`.
+_NAMESPACE_REFERENCE = re.compile(
+    r"(?:%s)(?:\.[\w-]|\[\d)" % "|".join(_NAMESPACE_ROOTS)
+)
+
 def _is_path_segment(segment: str) -> bool:
     """Whether _resolve_dot_path can walk *segment*: a name, or a name it indexes."""
     return bool(_PLAIN_SEGMENT.match(segment) or _INDEXED_SEGMENT.match(segment))

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,
+    evaluate_expression,
+    switch_expression_is_never_evaluated,
+)
 
 
 class SwitchStep(StepBase):
@@ -107,6 +111,34 @@ 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.
+        #
+        # A switch matches on strings, which moves both boundaries a condition has. A
+        # braceless literal is a valid case key -- `expression: review` dispatches the
+        # `review:` case -- so only text that opens with a namespace reference is
+        # flagged, not every string without braces. And 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 switch_expression_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

tests/test_workflows.py

@@ -3665,6 +3665,117 @@ 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_every_namespace_root_written_without_a_block_is_rejected(self):
+        """Each root `_build_namespace` supplies, walked into without braces."""
+        from specify_cli.workflows.steps.switch import SwitchStep
+
+        cases = {"review": [{"id": "r", "type": "command", "command": "echo"}]}
+        for expression in (
+            "inputs.mode",
+            "steps.check.output.stdout",
+            "item.name",
+            "item[0]",
+            "fan_in.results",
+            "context.run_id",
+            "inputs.mode | default('review')",
+            "inputs.mode == 'review'",
+            "  inputs.mode  ",
+        ):
+            config = {"id": "route", "expression": expression, "cases": cases}
+            errors = [
+                e for e in SwitchStep().validate(config) if "'expression'" in e
+            ]
+            assert len(errors) == 1, expression
+            assert "never evaluated" in errors[0], expression
+
+    def test_a_literal_expression_stays_accepted(self):
+        """A switch matches on strings, and a case key is a literal.
+
+        So a braceless literal is a valid -- if constant -- switch, not a fault:
+        `expression: review` dispatches the `review:` case, and whitespace strips to
+        the `""` key. Only text that walks into a namespace root is flagged.
+        """
+        from specify_cli.workflows.steps.switch import SwitchStep
+        from specify_cli.workflows.base import StepContext, StepStatus
+
+        review = [{"id": "r", "type": "command", "command": "echo"}]
+        blank = [{"id": "b", "type": "command", "command": "echo"}]
+        cases = {"review": review, "": blank, "approve me": review, "inputs": review}
+
+        # Ground truth first: each of these really does dispatch a declared case.
+        for expression, matched in (
+            ("review", "review"),
+            ("   ", ""),
+            ("approve me", "approve me"),
+            ("inputs", "inputs"),
+        ):
+            config = {"id": "route", "expression": expression, "cases": cases}
+            result = SwitchStep().execute(config, StepContext(inputs={}))
+            assert result.status == StepStatus.COMPLETED, expression
+            assert result.output["matched_case"] == matched, expression
+            assert [
+                e for e in SwitchStep().validate(config) if "'expression'" in e
+            ] == [], expression
+
+        # A name that merely starts like a root is not a reference into it.
+        config = {"id": "route", "expression": "inputsX.mode", "cases": cases}
+        assert [e for e in SwitchStep().validate(config) if "'expression'" in e] == []
+
+    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."""