fix(workflows): keep a literal bool 'condition' valid

Self-review catch: the guard rejected EVERY non-string, which broke an input
that previously worked. An unquoted ``condition: false`` is idiomatic YAML and
resolves exactly today -- evaluate_expression passes the bool through and
evaluate_condition's bool() is a no-op (verified: evaluate_condition(False) is
False, (True) is True). The if/while steps even default ``condition`` to the
bool ``False`` themselves, so bool is the field's natural type, not an
authoring mistake.

Accept (str, bool) and reject only the genuinely silent-coercion types
(list/dict/int/float, e.g. condition: [1, 2] is always True). Message updated
to "must be a string or boolean"; the bad-value tests drop True and gain 1.5,
and each step gains a positive bool case.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: jawwad-ali

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

@@ -70,19 +70,23 @@ def validate(self, config: dict[str, Any]) -> list[str]:
                 f"Do-while step {config.get('id', '?')!r} is missing "
                 f"'condition' field."
             )
-        elif not isinstance(config["condition"], str):
+        elif not isinstance(config["condition"], (str, bool)):
             # The engine re-evaluates 'condition' via evaluate_condition() after
             # each iteration. That call first delegates to
             # evaluate_expression() -- which returns a non-string unchanged --
             # and then coerces the result with bool(). So a list/dict/number
             # condition silently resolves to its truthiness (e.g.
             # condition: [1, 2] is always truthy, looping to max_iterations)
-            # with no error. Reject non-strings at validation, mirroring the
-            # prompt/shell/command 'must be a string' checks. "true"/"false"
-            # and an expression like "{{ ... }}" stay valid.
+            # with no error. Reject those at validation, mirroring the
+            # prompt/shell/command 'must be a string' checks.
+            #
+            # A literal ``bool`` stays valid: an unquoted ``condition: false``
+            # is idiomatic YAML and evaluate_condition() already resolves it
+            # exactly (bool passthrough, then a no-op bool()). "true"/"false"
+            # and an expression like "{{ ... }}" stay valid too.
             errors.append(
                 f"Do-while step {config.get('id', '?')!r}: 'condition' must be a "
-                f"string, got {type(config['condition']).__name__}."
+                f"string or boolean, got {type(config['condition']).__name__}."
             )
         max_iter = config.get("max_iterations")
         if max_iter is not None:

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

@@ -61,19 +61,23 @@ def validate(self, config: dict[str, Any]) -> list[str]:
             errors.append(
                 f"If step {config.get('id', '?')!r} is missing 'condition' field."
             )
-        elif not isinstance(config["condition"], str):
+        elif not isinstance(config["condition"], (str, bool)):
             # execute() feeds 'condition' to evaluate_condition(), which first
             # delegates to evaluate_expression() -- that returns a non-string
             # unchanged -- and then coerces the result with bool(). So a
             # list/dict/number condition silently resolves to its truthiness
             # (e.g. condition: [1, 2] is always True) with no error, branching
-            # wrongly on an authoring mistake. Reject non-strings at validation,
+            # wrongly on an authoring mistake. Reject those at validation,
             # mirroring the prompt/shell/command 'must be a string' checks.
-            # "true"/"false" and an expression like "{{ ... }}" are strings, so
-            # they stay valid.
+            #
+            # A literal ``bool`` stays valid: an unquoted ``condition: false``
+            # is idiomatic YAML, evaluate_condition() already resolves it
+            # exactly (bool passthrough, then a no-op bool()), and this step
+            # itself defaults ``condition`` to ``False``. "true"/"false" and an
+            # expression like "{{ ... }}" are strings, so they stay valid too.
             errors.append(
                 f"If step {config.get('id', '?')!r}: 'condition' must be a "
-                f"string, got {type(config['condition']).__name__}."
+                f"string or boolean, got {type(config['condition']).__name__}."
             )
         if "then" not in config:
             errors.append(

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

@@ -79,19 +79,23 @@ def validate(self, config: dict[str, Any]) -> list[str]:
                 f"While step {config.get('id', '?')!r} is missing "
                 f"'condition' field."
             )
-        elif not isinstance(config["condition"], str):
+        elif not isinstance(config["condition"], (str, bool)):
             # execute() feeds 'condition' to evaluate_condition(), which first
             # delegates to evaluate_expression() -- that returns a non-string
             # unchanged -- and then coerces the result with bool(). So a
             # list/dict/number condition silently resolves to its truthiness
             # (e.g. condition: [1, 2] is always truthy, spinning the loop to
-            # max_iterations) with no error. Reject non-strings at validation,
+            # max_iterations) with no error. Reject those at validation,
             # mirroring the prompt/shell/command 'must be a string' checks.
-            # "true"/"false" and an expression like "{{ ... }}" are strings, so
-            # they stay valid.
+            #
+            # A literal ``bool`` stays valid: an unquoted ``condition: false``
+            # is idiomatic YAML, evaluate_condition() already resolves it
+            # exactly (bool passthrough, then a no-op bool()), and this step
+            # itself defaults ``condition`` to ``False``. "true"/"false" and an
+            # expression like "{{ ... }}" are strings, so they stay valid too.
             errors.append(
                 f"While step {config.get('id', '?')!r}: 'condition' must be a "
-                f"string, got {type(config['condition']).__name__}."
+                f"string or boolean, got {type(config['condition']).__name__}."
             )
         max_iter = config.get("max_iterations")
         if max_iter is not None:

tests/test_workflows.py

@@ -2490,25 +2490,32 @@ def test_validate_missing_condition(self):
         errors = step.validate({"id": "test", "then": []})
         assert any("missing 'condition'" in e for e in errors)
 
-    @pytest.mark.parametrize("bad", [["a", "b"], {"k": "v"}, 5, True])
+    @pytest.mark.parametrize("bad", [["a", "b"], {"k": "v"}, 5, 1.5])
     def test_validate_rejects_non_string_condition(self, bad):
-        # A non-string condition is returned unchanged by evaluate_expression,
-        # and evaluate_condition then bool()-coerces it, so it silently resolves
-        # to its truthiness (e.g. [1, 2] is always True) instead of erroring on
-        # the authoring mistake.
+        # A list/dict/number condition is returned unchanged by
+        # evaluate_expression, and evaluate_condition then bool()-coerces it, so
+        # it silently resolves to its truthiness (e.g. [1, 2] is always True)
+        # instead of erroring on the authoring mistake.
         from specify_cli.workflows.steps.if_then import IfThenStep
 
         step = IfThenStep()
         errors = step.validate({"id": "test", "condition": bad, "then": []})
-        assert any("'condition' must be a string" in e for e in errors), bad
+        assert any("'condition' must be a" in e for e in errors), bad
 
-    @pytest.mark.parametrize("good", ["true", "false", "{{ inputs.flag }}"])
-    def test_validate_accepts_string_condition(self, good):
+    @pytest.mark.parametrize(
+        "good",
+        [
+            "true", "false", "{{ inputs.flag }}",
+            True, False,  # unquoted YAML bool: resolved exactly, and it is the
+                          # default this step itself uses -- must stay valid
+        ],
+    )
+    def test_validate_accepts_string_or_bool_condition(self, good):
         from specify_cli.workflows.steps.if_then import IfThenStep
 
         step = IfThenStep()
         errors = step.validate({"id": "test", "condition": good, "then": []})
-        assert not any("'condition' must be a string" in e for e in errors), good
+        assert not any("'condition' must be a" in e for e in errors), good
 
     @pytest.mark.parametrize("bad_branch", [{"id": "x"}, "oops", 5])
     def test_execute_non_list_then_fails_loudly(self, bad_branch):
@@ -2900,13 +2907,23 @@ def test_validate_missing_fields(self):
         assert any("missing 'condition'" in e for e in errors)
         # max_iterations is optional (defaults to 10)
 
-    @pytest.mark.parametrize("bad", [["a", "b"], {"k": "v"}, 5, True])
+    @pytest.mark.parametrize("bad", [["a", "b"], {"k": "v"}, 5, 1.5])
     def test_validate_rejects_non_string_condition(self, bad):
         from specify_cli.workflows.steps.while_loop import WhileStep
 
         step = WhileStep()
         errors = step.validate({"id": "test", "condition": bad, "steps": []})
-        assert any("'condition' must be a string" in e for e in errors), bad
+        assert any("'condition' must be a" in e for e in errors), bad
+
+    @pytest.mark.parametrize("good", [True, False, "true", "{{ inputs.go }}"])
+    def test_validate_accepts_string_or_bool_condition(self, good):
+        # ``condition: false`` unquoted is idiomatic YAML and is this step's own
+        # default, so a literal bool must not be rejected.
+        from specify_cli.workflows.steps.while_loop import WhileStep
+
+        step = WhileStep()
+        errors = step.validate({"id": "test", "condition": good, "steps": []})
+        assert not any("'condition' must be a" in e for e in errors), good
 
     def test_validate_invalid_max_iterations(self):
         from specify_cli.workflows.steps.while_loop import WhileStep
@@ -3022,13 +3039,23 @@ def test_validate_missing_fields(self):
         assert any("missing 'condition'" in e for e in errors)
         # max_iterations is optional (defaults to 10)
 
-    @pytest.mark.parametrize("bad", [["a", "b"], {"k": "v"}, 5, True])
+    @pytest.mark.parametrize("bad", [["a", "b"], {"k": "v"}, 5, 1.5])
     def test_validate_rejects_non_string_condition(self, bad):
         from specify_cli.workflows.steps.do_while import DoWhileStep
 
         step = DoWhileStep()
         errors = step.validate({"id": "test", "condition": bad, "steps": []})
-        assert any("'condition' must be a string" in e for e in errors), bad
+        assert any("'condition' must be a" in e for e in errors), bad
+
+    @pytest.mark.parametrize("good", [True, False, "true", "{{ inputs.go }}"])
+    def test_validate_accepts_string_or_bool_condition(self, good):
+        # ``condition: false`` unquoted is idiomatic YAML; evaluate_condition
+        # resolves a literal bool exactly, so it must not be rejected.
+        from specify_cli.workflows.steps.do_while import DoWhileStep
+
+        step = DoWhileStep()
+        errors = step.validate({"id": "test", "condition": good, "steps": []})
+        assert not any("'condition' must be a" in e for e in errors), good
 
     def test_validate_steps_not_list(self):
         from specify_cli.workflows.steps.do_while import DoWhileStep