fix(workflows): evaluate 'in'/'not in' safely on a non-iterable right operand (#3447) by Noor-ul-ain001 · Pull Request #3468 · github/spec-kit

Noor-ul-ain001 · 2026-07-11T05:56:05Z

Summary

Closes #3447. The in / not in operators in _evaluate_simple_expression (src/specify_cli/workflows/expressions.py) only guarded right is not None:

if op == " in ":
    return left in right if right is not None else False

But left in right also raises TypeError for any other non-iterable right operand (int, bool, float). So a workflow condition like {{ inputs.tag in inputs.count }} where count is a number leaked a raw TypeError: argument of type 'int' is not iterable and crashed the whole run, instead of evaluating like the None case right beside it.

This is asymmetric with _safe_compare, which already swallows TypeError and returns False for the ordering operators (<, >, etc.).

Before:

evaluate_condition("{{ inputs.tag in inputs.count }}", StepContext(inputs={"tag":"x","count":5}))
# TypeError: argument of type 'int' is not iterable  (crashes the run)

After: evaluates to False (nothing is contained in a non-container), same as the right is None branch.

Changes

Add a _safe_contains(left, right) helper mirroring _safe_compare: a None or non-container right means "nothing is contained", so in → False and not in → True rather than a raw TypeError.
Route both membership branches through it (not in negates the result), so the two operators can't drift.

Testing

Added test_in_operator_non_iterable_right_operand: asserts in/not in against int/bool/float/None right operands don't crash and return the None-branch result, and that genuine containment against iterables still works.
Test-the-test: confirmed the new test fails without the source change (raw TypeError propagates).
pytest tests/test_workflows.py → 395 passed, 1 skipped. (The 11 TestWorkflow*SymlinkGuard failures are pre-existing on Windows without elevation and unrelated to this change.)

🤖 Generated with Claude Code

… operand (github#3447) The `in` / `not in` operators in `_evaluate_simple_expression` only guarded `right is not None`, but `left in right` also raises `TypeError` for any other non-iterable right operand (int, bool, float). So a workflow condition like `{{ inputs.tag in inputs.count }}` where `count` is a number leaked a raw `TypeError: argument of type 'int' is not iterable` and crashed the whole run, instead of evaluating like the None case beside it. This was asymmetric with `_safe_compare`, which already swallows `TypeError` and returns False for the ordering operators. Add a `_safe_contains` helper (mirroring `_safe_compare`) that treats both a None and a non-container right operand as "nothing is contained": `in` -> False, `not in` -> True. Add a regression test covering int/bool/float/None right operands and asserting genuine containment against iterables still works. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Fixes workflow expression evaluation so in / not in conditions don’t crash the run when the right-hand operand isn’t iterable (e.g., int, bool, float), aligning membership semantics with the existing “safe” behavior used for ordering comparisons.

Changes:

Added _safe_contains(left, right) to handle in safely (returning False on None or TypeError instead of raising).
Routed both " in " and " not in " operator handling through _safe_contains to keep behavior consistent and avoid drift.
Added regression tests covering in/not in against non-iterable right operands, plus a quick iterable containment sanity check.

Show a summary per file

File	Description
`src/specify_cli/workflows/expressions.py`	Introduces `_safe_contains` and uses it for `"in"` / `"not in"` to prevent TypeError crashes on non-iterable RHS.
`tests/test_workflows.py`	Adds a focused regression test ensuring non-iterable RHS doesn’t crash and returns the expected boolean results.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Low

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Review details

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Medium

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…r-non-iterable # Conflicts: # src/specify_cli/workflows/expressions.py

Copilot

Review details

Files reviewed: 2/2 changed files
Comments generated: 2
Review effort level: Medium

+                return _safe_contains(left, right)
            if op == " not in ":
-                return _safe_membership(left, right, negate=True)
+                return not _safe_contains(left, right)


+    def test_in_operator_non_iterable_right_operand(self):
+        """`in`/`not in` against a non-iterable right operand must not crash.


mnriem

Please address Copilot feedback

…r-non-iterable

github#3447 was fixed independently by github#3448 (merged first), which added the same _safe_membership helper this branch introduced. Per Copilot review: - Revert the redundant _safe_contains rename in expressions.py so the file matches main; the working membership guard already lives there. - Drop the duplicate test_in_operator_non_iterable_right_operand test and fold its only new coverage (not in against float/bool/None right operands, which the base test only checked for the int case) into the existing test_membership_against_non_iterable_is_false_not_error. Also merges latest upstream/main into the branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Noor-ul-ain001 · 2026-07-14T04:02:52Z

Thanks for the review. Addressed the Copilot feedback:

On the redundant refactor (main finding): You are right — #3447 was fixed independently by #3448, which merged first and added the same _safe_membership helper this branch introduced. I have reverted the _safe_contains rename so expressions.py now matches main exactly (empty diff). The membership guard that fixes #3447 already lives on main.

On the duplicate test: Removed test_in_operator_non_iterable_right_operand and folded its only genuinely-new coverage into the existing test_membership_against_non_iterable_is_false_not_error — the base test only exercised not in for the int case, so I extended it to loop not in over the float/bool/None right operands too. No second regression test.

On rebase: Merged the latest upstream/main into the branch.

The remaining diff is now just that small test-hardening addition on top of the already-merged fix. The earlier docstring/indentation comments were resolved in prior commits. pytest tests/test_workflows.py::TestExpressions → 42 passed.

Noor-ul-ain001 requested a review from mnriem as a code owner July 11, 2026 05:56

mnriem requested a review from Copilot July 13, 2026 13:16

Copilot started reviewing on behalf of mnriem July 13, 2026 13:20 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

Comment thread src/specify_cli/workflows/expressions.py Outdated

Potential fix for pull request finding

5b59a9f

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

mnriem requested a review from Copilot July 13, 2026 15:55

Copilot started reviewing on behalf of mnriem July 13, 2026 15:56 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

Comment thread src/specify_cli/workflows/expressions.py Outdated

Noor-ul-ain001 and others added 2 commits July 13, 2026 21:01

Potential fix for pull request finding

f7927de

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Merge remote-tracking branch 'upstream/main' into fix/3447-in-operato…

447d815

…r-non-iterable # Conflicts: # src/specify_cli/workflows/expressions.py

mnriem requested a review from Copilot July 13, 2026 19:49

Copilot started reviewing on behalf of mnriem July 13, 2026 19:49 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

mnriem requested changes Jul 13, 2026

View reviewed changes

Noor-ul-ain001 and others added 2 commits July 14, 2026 09:00

Merge remote-tracking branch 'upstream/main' into fix/3447-in-operato…

bd17df9

…r-non-iterable

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(workflows): evaluate 'in'/'not in' safely on a non-iterable right operand (#3447)#3468

fix(workflows): evaluate 'in'/'not in' safely on a non-iterable right operand (#3447)#3468
Noor-ul-ain001 wants to merge 6 commits into
github:mainfrom
Noor-ul-ain001:fix/3447-in-operator-non-iterable

Noor-ul-ain001 commented Jul 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

mnriem left a comment

Uh oh!

Noor-ul-ain001 commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		def test_in_operator_non_iterable_right_operand(self):
		"""`in`/`not in` against a non-iterable right operand must not crash.

Uh oh!

Conversation

Noor-ul-ain001 commented Jul 11, 2026

Summary

Changes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Review details

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Review details

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

Noor-ul-ain001 commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants