From 01ad7f9386d5d2750cdeb8f2a6c400d35ad79f45 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Fri, 10 Jul 2026 17:28:48 +0500 Subject: [PATCH 1/2] fix(workflows): don't crash on membership test against a non-iterable the `in` / `not in` operators in _evaluate_simple_expression only guarded `right is not None`, so `left in right` still raised a raw TypeError when the right operand was any other non-iterable (int, bool, float). a condition like `{{ inputs.tag in inputs.count }}` where count is a number crashed the whole workflow run instead of evaluating. nothing is contained in a non-iterable, so treat membership as False (`not in` as True) via a new _safe_membership helper that swallows TypeError. this generalizes the old None guard and mirrors _safe_compare, which already catches TypeError for the ordering operators. added a regression test; confirmed it fails on the pre-fix code (raw TypeError) and that genuine list/substring membership still works. --- src/specify_cli/workflows/expressions.py | 22 +++++++++++++++++-- tests/test_workflows.py | 27 ++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index d6736bc321..ac43419d29 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -464,9 +464,9 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: if op == "<=": return _safe_compare(left, right, "<=") if op == " in ": - return left in right if right is not None else False + return _safe_membership(left, right, negate=False) if op == " not in ": - return left not in right if right is not None else True + return _safe_membership(left, right, negate=True) # Numeric literal try: @@ -511,6 +511,24 @@ def _coerce_number(value: Any) -> Any: return value +def _safe_membership(left: Any, right: Any, *, negate: bool) -> bool: + """Safely evaluate ``left in right`` (or ``not in``) without crashing. + + A non-iterable right operand (``None``, an int, a bool, ...) makes ``in`` + raise ``TypeError`` in Python. Nothing is contained in such a value, so + treat membership as ``False`` (``not in`` as ``True``) rather than leaking + the error out of the evaluator and crashing the whole workflow. Mirrors the + graceful ``TypeError`` handling in ``_safe_compare`` for the ordering + operators, and generalizes the previous ``right is not None`` guard to + every non-iterable right operand. + """ + try: + contained = left in right + except TypeError: + contained = False + return not contained if negate else contained + + def _safe_compare(left: Any, right: Any, op: str) -> bool: """Compare two values for ordering, coercing numeric strings when possible. diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 00e0bafedb..400dcc33a0 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -460,6 +460,33 @@ def test_pipe_detection_is_quote_aware(self): assert evaluate_expression("{{ inputs.s | contains('ab') }}", ctx2) is True assert evaluate_expression("{{ inputs.missing | default('a|b') }}", ctx2) == "a|b" + def test_membership_against_non_iterable_is_false_not_error(self): + from specify_cli.workflows.expressions import ( + evaluate_condition, + evaluate_expression, + ) + from specify_cli.workflows.base import StepContext + + # A non-iterable right operand (int, bool, None, float) makes a raw + # `x in y` raise TypeError in Python. The evaluator must treat it as + # "not contained" (False, and `not in` as True) instead of leaking the + # TypeError and crashing the whole workflow run. This generalizes the + # previous `right is not None` guard and mirrors _safe_compare, which + # already swallows TypeError for the ordering operators. + ctx = StepContext(inputs={"tag": "x", "count": 5, "flag": True}) + assert evaluate_expression("{{ inputs.tag in inputs.count }}", ctx) is False + assert evaluate_expression("{{ inputs.tag not in inputs.count }}", ctx) is True + assert evaluate_expression("{{ 'a' in inputs.flag }}", ctx) is False + assert evaluate_expression("{{ inputs.tag in inputs.missing }}", ctx) is False + # A condition that would otherwise crash the run now evaluates cleanly. + assert evaluate_condition("{{ inputs.tag in inputs.count }}", ctx) is False + + # Regression: genuine membership over a real iterable still works. + ok = StepContext(inputs={"items": ["x", "y"], "s": "xyz"}) + assert evaluate_expression("{{ 'x' in inputs.items }}", ok) is True + assert evaluate_expression("{{ 'z' not in inputs.items }}", ok) is True + assert evaluate_expression("{{ 'y' in inputs.s }}", ok) is True + def test_filter_default(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext From 2178fb30b262a6e3d87bcc40c945f4cb0fded760 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Sat, 11 Jul 2026 00:01:02 +0500 Subject: [PATCH 2/2] address review: float membership case + broaden _safe_membership docstring - add a float right-operand assertion so the test matches its comment (was claiming float coverage while only exercising int/bool/None). - reword the _safe_membership docstring to describe TypeError generally (non-iterable right is the common case, but also e.g. an unhashable left against a set) rather than implying only the right operand matters. --- src/specify_cli/workflows/expressions.py | 16 +++++++++------- tests/test_workflows.py | 3 ++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index ac43419d29..9953f5ff61 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -514,13 +514,15 @@ def _coerce_number(value: Any) -> Any: def _safe_membership(left: Any, right: Any, *, negate: bool) -> bool: """Safely evaluate ``left in right`` (or ``not in``) without crashing. - A non-iterable right operand (``None``, an int, a bool, ...) makes ``in`` - raise ``TypeError`` in Python. Nothing is contained in such a value, so - treat membership as ``False`` (``not in`` as ``True``) rather than leaking - the error out of the evaluator and crashing the whole workflow. Mirrors the - graceful ``TypeError`` handling in ``_safe_compare`` for the ordering - operators, and generalizes the previous ``right is not None`` guard to - every non-iterable right operand. + ``left in right`` raises ``TypeError`` whenever the operands don't support + membership testing — most commonly a non-iterable right operand (``None``, + an int, a bool), but also cases like an unhashable ``left`` against a set. + In every such case the membership relation is undefined, so treat it as + ``False`` (``not in`` as ``True``) rather than leaking the error out of the + evaluator and crashing the whole workflow. Mirrors the graceful + ``TypeError`` handling in ``_safe_compare`` for the ordering operators, and + generalizes the previous ``right is not None`` guard to any operand pair + that can't be membership-tested. """ try: contained = left in right diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 400dcc33a0..1791f3192f 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -473,9 +473,10 @@ def test_membership_against_non_iterable_is_false_not_error(self): # TypeError and crashing the whole workflow run. This generalizes the # previous `right is not None` guard and mirrors _safe_compare, which # already swallows TypeError for the ordering operators. - ctx = StepContext(inputs={"tag": "x", "count": 5, "flag": True}) + ctx = StepContext(inputs={"tag": "x", "count": 5, "ratio": 1.5, "flag": True}) assert evaluate_expression("{{ inputs.tag in inputs.count }}", ctx) is False assert evaluate_expression("{{ inputs.tag not in inputs.count }}", ctx) is True + assert evaluate_expression("{{ 'a' in inputs.ratio }}", ctx) is False assert evaluate_expression("{{ 'a' in inputs.flag }}", ctx) is False assert evaluate_expression("{{ inputs.tag in inputs.missing }}", ctx) is False # A condition that would otherwise crash the run now evaluates cleanly.