From 844009c5845e5afb15bf4c3bf53ac91e88019365 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Fri, 24 Jul 2026 13:22:35 +0500 Subject: [PATCH 1/3] fix(workflows): reject non-string 'condition' in if/while/do-while steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `if_then`, `while_loop`, and `do_while` validate() confirm `condition` is present but never that it is a string. execute() feeds it to `evaluate_condition()`, which returns a non-string as-is and takes `bool()` of it -- so `condition: [1, 2]` (a list authoring mistake) silently resolves to `True`, branching wrongly / spinning the loop to `max_iterations`, with no error reported. Reject a present-but-non-string `condition` at validation, mirroring the existing prompt/shell/command 'must be a string' guards. `"true"`/`"false"` and expressions like `"{{ ... }}"` are strings, so they stay valid. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../workflows/steps/do_while/__init__.py | 12 +++++++ .../workflows/steps/if_then/__init__.py | 12 +++++++ .../workflows/steps/while_loop/__init__.py | 12 +++++++ tests/test_workflows.py | 35 +++++++++++++++++++ 4 files changed, 71 insertions(+) diff --git a/src/specify_cli/workflows/steps/do_while/__init__.py b/src/specify_cli/workflows/steps/do_while/__init__.py index ca6047a57a..d8ae79b324 100644 --- a/src/specify_cli/workflows/steps/do_while/__init__.py +++ b/src/specify_cli/workflows/steps/do_while/__init__.py @@ -70,6 +70,18 @@ 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): + # The engine re-evaluates 'condition' via evaluate_condition() after + # each iteration; it returns a non-string as-is and takes bool() of + # it -- so a list/dict/number condition silently resolves to a + # 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. + errors.append( + f"Do-while step {config.get('id', '?')!r}: 'condition' must be a " + f"string, got {type(config['condition']).__name__}." + ) max_iter = config.get("max_iterations") if max_iter is not None: # bool is a subclass of int, so isinstance(True, int) is True and diff --git a/src/specify_cli/workflows/steps/if_then/__init__.py b/src/specify_cli/workflows/steps/if_then/__init__.py index b2ed880678..48691285c3 100644 --- a/src/specify_cli/workflows/steps/if_then/__init__.py +++ b/src/specify_cli/workflows/steps/if_then/__init__.py @@ -61,6 +61,18 @@ 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): + # execute() feeds 'condition' to evaluate_condition(), which returns + # a non-string as-is and takes bool() of it -- so a list/dict/number + # condition silently resolves to a truthiness (e.g. condition: [1, 2] + # is always True) with no error, branching wrongly on an authoring + # mistake. Reject non-strings at validation, mirroring the prompt/ + # shell/command 'must be a string' checks. "true"/"false" and an + # expression like "{{ ... }}" are strings, so they stay valid. + errors.append( + f"If step {config.get('id', '?')!r}: 'condition' must be a " + f"string, got {type(config['condition']).__name__}." + ) if "then" not in config: errors.append( f"If step {config.get('id', '?')!r} is missing 'then' field." diff --git a/src/specify_cli/workflows/steps/while_loop/__init__.py b/src/specify_cli/workflows/steps/while_loop/__init__.py index e2dbb19305..0bb592ca1f 100644 --- a/src/specify_cli/workflows/steps/while_loop/__init__.py +++ b/src/specify_cli/workflows/steps/while_loop/__init__.py @@ -79,6 +79,18 @@ 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): + # execute() feeds 'condition' to evaluate_condition(), which returns + # a non-string as-is and takes bool() of it -- so a list/dict/number + # condition silently resolves to a truthiness (e.g. condition: [1, 2] + # is always truthy, spinning the loop 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 "{{ ... }}" are strings, so they stay valid. + errors.append( + f"While step {config.get('id', '?')!r}: 'condition' must be a " + f"string, got {type(config['condition']).__name__}." + ) max_iter = config.get("max_iterations") if max_iter is not None: # bool is a subclass of int, so isinstance(True, int) is True and diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 61c625a396..673e99dd48 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -2490,6 +2490,25 @@ 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]) + def test_validate_rejects_non_string_condition(self, bad): + # A non-string condition is returned as-is by evaluate_condition and + # bool()-coerced, so it silently resolves to a 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 + + @pytest.mark.parametrize("good", ["true", "false", "{{ inputs.flag }}"]) + def test_validate_accepts_string_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 + @pytest.mark.parametrize("bad_branch", [{"id": "x"}, "oops", 5]) def test_execute_non_list_then_fails_loudly(self, bad_branch): """A non-list ``then`` must fail the step, not crash the run. @@ -2880,6 +2899,14 @@ 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]) + 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 + def test_validate_invalid_max_iterations(self): from specify_cli.workflows.steps.while_loop import WhileStep @@ -2994,6 +3021,14 @@ 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]) + 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 + def test_validate_steps_not_list(self): from specify_cli.workflows.steps.do_while import DoWhileStep From 720b6527e2dc67614c0586c34381403f3eee39b2 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Sat, 25 Jul 2026 00:16:18 +0500 Subject: [PATCH 2/3] docs(workflows): describe the evaluate_expression/evaluate_condition split accurately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback: the guard comments attributed the non-string pass-through to evaluate_condition(), which always returns a bool. It is evaluate_expression() (called by evaluate_condition) that returns a non-string unchanged; evaluate_condition then applies bool() to that value. Reword all four sites (if/while/do-while guards + the mirror test comment) to name the two stages correctly. Comments only -- no behaviour change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) --- .../workflows/steps/do_while/__init__.py | 14 ++++++++------ .../workflows/steps/if_then/__init__.py | 16 +++++++++------- .../workflows/steps/while_loop/__init__.py | 16 +++++++++------- tests/test_workflows.py | 7 ++++--- 4 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/specify_cli/workflows/steps/do_while/__init__.py b/src/specify_cli/workflows/steps/do_while/__init__.py index d8ae79b324..290fbaa46a 100644 --- a/src/specify_cli/workflows/steps/do_while/__init__.py +++ b/src/specify_cli/workflows/steps/do_while/__init__.py @@ -72,12 +72,14 @@ def validate(self, config: dict[str, Any]) -> list[str]: ) elif not isinstance(config["condition"], str): # The engine re-evaluates 'condition' via evaluate_condition() after - # each iteration; it returns a non-string as-is and takes bool() of - # it -- so a list/dict/number condition silently resolves to a - # 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. + # 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. errors.append( f"Do-while step {config.get('id', '?')!r}: 'condition' must be a " f"string, got {type(config['condition']).__name__}." diff --git a/src/specify_cli/workflows/steps/if_then/__init__.py b/src/specify_cli/workflows/steps/if_then/__init__.py index 48691285c3..a69c097a39 100644 --- a/src/specify_cli/workflows/steps/if_then/__init__.py +++ b/src/specify_cli/workflows/steps/if_then/__init__.py @@ -62,13 +62,15 @@ def validate(self, config: dict[str, Any]) -> list[str]: f"If step {config.get('id', '?')!r} is missing 'condition' field." ) elif not isinstance(config["condition"], str): - # execute() feeds 'condition' to evaluate_condition(), which returns - # a non-string as-is and takes bool() of it -- so a list/dict/number - # condition silently resolves to a truthiness (e.g. condition: [1, 2] - # is always True) with no error, branching wrongly on an authoring - # mistake. Reject non-strings at validation, mirroring the prompt/ - # shell/command 'must be a string' checks. "true"/"false" and an - # expression like "{{ ... }}" are strings, so they stay valid. + # 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, + # mirroring the prompt/shell/command 'must be a string' checks. + # "true"/"false" and an expression like "{{ ... }}" are strings, so + # they stay valid. errors.append( f"If step {config.get('id', '?')!r}: 'condition' must be a " f"string, got {type(config['condition']).__name__}." diff --git a/src/specify_cli/workflows/steps/while_loop/__init__.py b/src/specify_cli/workflows/steps/while_loop/__init__.py index 0bb592ca1f..b3faf69c99 100644 --- a/src/specify_cli/workflows/steps/while_loop/__init__.py +++ b/src/specify_cli/workflows/steps/while_loop/__init__.py @@ -80,13 +80,15 @@ def validate(self, config: dict[str, Any]) -> list[str]: f"'condition' field." ) elif not isinstance(config["condition"], str): - # execute() feeds 'condition' to evaluate_condition(), which returns - # a non-string as-is and takes bool() of it -- so a list/dict/number - # condition silently resolves to a truthiness (e.g. condition: [1, 2] - # is always truthy, spinning the loop 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 "{{ ... }}" are strings, so they stay valid. + # 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, + # mirroring the prompt/shell/command 'must be a string' checks. + # "true"/"false" and an expression like "{{ ... }}" are strings, so + # they stay valid. errors.append( f"While step {config.get('id', '?')!r}: 'condition' must be a " f"string, got {type(config['condition']).__name__}." diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 673e99dd48..166b37338a 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -2492,9 +2492,10 @@ def test_validate_missing_condition(self): @pytest.mark.parametrize("bad", [["a", "b"], {"k": "v"}, 5, True]) def test_validate_rejects_non_string_condition(self, bad): - # A non-string condition is returned as-is by evaluate_condition and - # bool()-coerced, so it silently resolves to a truthiness (e.g. [1, 2] - # is always True) instead of erroring on the authoring mistake. + # 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. from specify_cli.workflows.steps.if_then import IfThenStep step = IfThenStep() From ce6d3ac318b60afde0ce6bd07a07f808c616cb79 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Sat, 25 Jul 2026 10:15:50 +0500 Subject: [PATCH 3/3] fix(workflows): keep a literal bool 'condition' valid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../workflows/steps/do_while/__init__.py | 14 +++-- .../workflows/steps/if_then/__init__.py | 14 +++-- .../workflows/steps/while_loop/__init__.py | 14 +++-- tests/test_workflows.py | 53 ++++++++++++++----- 4 files changed, 67 insertions(+), 28 deletions(-) diff --git a/src/specify_cli/workflows/steps/do_while/__init__.py b/src/specify_cli/workflows/steps/do_while/__init__.py index 290fbaa46a..024ced55b5 100644 --- a/src/specify_cli/workflows/steps/do_while/__init__.py +++ b/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: diff --git a/src/specify_cli/workflows/steps/if_then/__init__.py b/src/specify_cli/workflows/steps/if_then/__init__.py index a69c097a39..7189ff8150 100644 --- a/src/specify_cli/workflows/steps/if_then/__init__.py +++ b/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( diff --git a/src/specify_cli/workflows/steps/while_loop/__init__.py b/src/specify_cli/workflows/steps/while_loop/__init__.py index b3faf69c99..e80b93d7f2 100644 --- a/src/specify_cli/workflows/steps/while_loop/__init__.py +++ b/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: diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 166b37338a..d94d530ae2 100644 --- a/tests/test_workflows.py +++ b/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