Skip to content

Commit 3571ba7

Browse files
jawwad-aliclaude
andauthored
fix(workflows): reject bool max_iterations in while/do-while validation (#3237)
* fix(workflows): reject bool max_iterations in while/do-while validation while/do-while validate() checked 'not isinstance(max_iter, int) or max_iter < 1'. Since bool is a subclass of int, isinstance(True, int) is True and True < 1 is False, so 'max_iterations: true' passed validation and then ran as a single iteration (range(True) == range(1)) instead of being reported as a type error. Reject bools explicitly, matching the fail-fast-on-bool handling already used for number inputs and gate options. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: assert empty error list for the valid do-while max_iterations case Address Copilot review: the accepted-config assertion only checked that no error mentioned 'max_iterations', which could let an unrelated validation error pass unnoticed. For a known-good config, assert the entire error list is empty (consistent with the other validate tests in this file). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6fb7e77 commit 3571ba7

3 files changed

Lines changed: 29 additions & 2 deletions

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@ def validate(self, config: dict[str, Any]) -> list[str]:
4848
)
4949
max_iter = config.get("max_iterations")
5050
if max_iter is not None:
51-
if not isinstance(max_iter, int) or max_iter < 1:
51+
# bool is a subclass of int, so isinstance(True, int) is True and
52+
# True < 1 is False; reject bools explicitly so `max_iterations: true`
53+
# is a type error rather than a silent single iteration.
54+
if isinstance(max_iter, bool) or not isinstance(max_iter, int) or max_iter < 1:
5255
errors.append(
5356
f"Do-while step {config.get('id', '?')!r}: "
5457
f"'max_iterations' must be an integer >= 1."

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,10 @@ def validate(self, config: dict[str, Any]) -> list[str]:
5555
)
5656
max_iter = config.get("max_iterations")
5757
if max_iter is not None:
58-
if not isinstance(max_iter, int) or max_iter < 1:
58+
# bool is a subclass of int, so isinstance(True, int) is True and
59+
# True < 1 is False; reject bools explicitly so `max_iterations: true`
60+
# is a type error rather than a silent single iteration.
61+
if isinstance(max_iter, bool) or not isinstance(max_iter, int) or max_iter < 1:
5962
errors.append(
6063
f"While step {config.get('id', '?')!r}: "
6164
f"'max_iterations' must be an integer >= 1."

tests/test_workflows.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1822,6 +1822,12 @@ def test_validate_invalid_max_iterations(self):
18221822
step = WhileStep()
18231823
errors = step.validate({"id": "test", "condition": "{{ true }}", "max_iterations": 0, "steps": []})
18241824
assert any("must be an integer >= 1" in e for e in errors)
1825+
# bool is an int subclass; `max_iterations: true` must be rejected, not
1826+
# silently treated as a single iteration.
1827+
bool_errors = step.validate(
1828+
{"id": "test", "condition": "{{ true }}", "max_iterations": True, "steps": []}
1829+
)
1830+
assert any("must be an integer >= 1" in e for e in bool_errors)
18251831

18261832

18271833
class TestDoWhileStep:
@@ -1861,6 +1867,21 @@ def test_execute_with_true_condition(self):
18611867
assert len(result.next_steps) == 1
18621868
assert result.output["max_iterations"] == 5
18631869

1870+
def test_validate_rejects_bool_max_iterations(self):
1871+
from specify_cli.workflows.steps.do_while import DoWhileStep
1872+
1873+
step = DoWhileStep()
1874+
# bool is an int subclass; `max_iterations: true` must be rejected.
1875+
errors = step.validate(
1876+
{"id": "test", "condition": "{{ true }}", "max_iterations": True, "steps": []}
1877+
)
1878+
assert any("must be an integer >= 1" in e for e in errors)
1879+
# a real positive integer is fully valid (no errors at all).
1880+
ok = step.validate(
1881+
{"id": "test", "condition": "{{ true }}", "max_iterations": 3, "steps": []}
1882+
)
1883+
assert ok == [], ok
1884+
18641885
def test_execute_empty_steps(self):
18651886
from specify_cli.workflows.steps.do_while import DoWhileStep
18661887
from specify_cli.workflows.base import StepContext

0 commit comments

Comments
 (0)