From 1884c59240a38082ca72ff503613dfa1423b4b53 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Mon, 29 Jun 2026 21:22:37 +0500 Subject: [PATCH 1/2] 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) --- .../workflows/steps/do_while/__init__.py | 5 ++++- .../workflows/steps/while_loop/__init__.py | 5 ++++- tests/test_workflows.py | 21 +++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/workflows/steps/do_while/__init__.py b/src/specify_cli/workflows/steps/do_while/__init__.py index 47a4d34437..f69a682140 100644 --- a/src/specify_cli/workflows/steps/do_while/__init__.py +++ b/src/specify_cli/workflows/steps/do_while/__init__.py @@ -48,7 +48,10 @@ def validate(self, config: dict[str, Any]) -> list[str]: ) max_iter = config.get("max_iterations") if max_iter is not None: - if not isinstance(max_iter, int) or max_iter < 1: + # bool is a subclass of int, so isinstance(True, int) is True and + # True < 1 is False; reject bools explicitly so `max_iterations: true` + # is a type error rather than a silent single iteration. + if isinstance(max_iter, bool) or not isinstance(max_iter, int) or max_iter < 1: errors.append( f"Do-while step {config.get('id', '?')!r}: " f"'max_iterations' must be an integer >= 1." diff --git a/src/specify_cli/workflows/steps/while_loop/__init__.py b/src/specify_cli/workflows/steps/while_loop/__init__.py index 18c2f46050..ea272543b6 100644 --- a/src/specify_cli/workflows/steps/while_loop/__init__.py +++ b/src/specify_cli/workflows/steps/while_loop/__init__.py @@ -55,7 +55,10 @@ def validate(self, config: dict[str, Any]) -> list[str]: ) max_iter = config.get("max_iterations") if max_iter is not None: - if not isinstance(max_iter, int) or max_iter < 1: + # bool is a subclass of int, so isinstance(True, int) is True and + # True < 1 is False; reject bools explicitly so `max_iterations: true` + # is a type error rather than a silent single iteration. + if isinstance(max_iter, bool) or not isinstance(max_iter, int) or max_iter < 1: errors.append( f"While step {config.get('id', '?')!r}: " f"'max_iterations' must be an integer >= 1." diff --git a/tests/test_workflows.py b/tests/test_workflows.py index e1b4a5ec23..53ba0d82d1 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1822,6 +1822,12 @@ def test_validate_invalid_max_iterations(self): step = WhileStep() errors = step.validate({"id": "test", "condition": "{{ true }}", "max_iterations": 0, "steps": []}) assert any("must be an integer >= 1" in e for e in errors) + # bool is an int subclass; `max_iterations: true` must be rejected, not + # silently treated as a single iteration. + bool_errors = step.validate( + {"id": "test", "condition": "{{ true }}", "max_iterations": True, "steps": []} + ) + assert any("must be an integer >= 1" in e for e in bool_errors) class TestDoWhileStep: @@ -1861,6 +1867,21 @@ def test_execute_with_true_condition(self): assert len(result.next_steps) == 1 assert result.output["max_iterations"] == 5 + def test_validate_rejects_bool_max_iterations(self): + from specify_cli.workflows.steps.do_while import DoWhileStep + + step = DoWhileStep() + # bool is an int subclass; `max_iterations: true` must be rejected. + errors = step.validate( + {"id": "test", "condition": "{{ true }}", "max_iterations": True, "steps": []} + ) + assert any("must be an integer >= 1" in e for e in errors) + # a real positive integer is still accepted. + ok = step.validate( + {"id": "test", "condition": "{{ true }}", "max_iterations": 3, "steps": []} + ) + assert not any("max_iterations" in e for e in ok) + def test_execute_empty_steps(self): from specify_cli.workflows.steps.do_while import DoWhileStep from specify_cli.workflows.base import StepContext From 93609313e67df3d48bdd8f5ed4aa47f797b5477d Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Tue, 30 Jun 2026 19:53:25 +0500 Subject: [PATCH 2/2] 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) --- tests/test_workflows.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 53ba0d82d1..c2ec4acb4e 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1876,11 +1876,11 @@ def test_validate_rejects_bool_max_iterations(self): {"id": "test", "condition": "{{ true }}", "max_iterations": True, "steps": []} ) assert any("must be an integer >= 1" in e for e in errors) - # a real positive integer is still accepted. + # a real positive integer is fully valid (no errors at all). ok = step.validate( {"id": "test", "condition": "{{ true }}", "max_iterations": 3, "steps": []} ) - assert not any("max_iterations" in e for e in ok) + assert ok == [], ok def test_execute_empty_steps(self): from specify_cli.workflows.steps.do_while import DoWhileStep