fix(workflows): refuse a filter mixed with a comparison operator instead of silently mis-binding it - #3894
Open
jawwad-ali wants to merge 1 commit into
Open
Conversation
The pipe is detected before the boolean/comparison operators, so a filter
written on the right-hand operand was applied to the comparison's BOOLEAN
RESULT instead of to the operand:
{{ inputs.count > inputs.limit | default(5) }} -> False
With count=10 and limit missing, `count > limit` is evaluated first and
`default` is then applied to the resulting bool — a no-op, since a bool is
never empty — so the expression silently returns the comparison against
the *unfiltered* operand. The author meant `10 > 5` = True.
This module already refuses the mirror case rather than guessing:
{{ inputs.missing | default('7') > '5' }}
-> ValueError: filter 'default' used in an unsupported form
Same ambiguity, opposite handling. Refuse both the same way so an
ambiguous expression is reported instead of quietly producing the answer
the author did not ask for.
No legitimate expression is affected: applying `default` to a bool is a
no-op, and `join`/`map`/`contains` on a bool is an error, so there is no
working use of a filter on a comparison result.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In
_evaluate_simple_expressionthe pipe is detected before the boolean and comparison operators:So a filter written on the right-hand operand of a comparison is applied to the comparison's boolean result, not to the operand.
Reproduction on current
main(81bf741)With
inputs.count = 10andinputs.limitmissing:count > limitis evaluated first, thendefaultis applied to the resulting bool — which is a no-op, because a bool is never empty. The expression silently returns the comparison against the unfiltered operand. The author meant10 > 5→True.This module already refuses the mirror case
A filter followed by a comparison is explicitly rejected rather than guessed at:
…and the test that pins it (
test_filter_call_with_trailing_tokens_fails_loudly) states the reasoning:Same ambiguity, opposite handling — one raises, the other quietly returns a wrong answer. This PR applies the decision that was already made to the mirror case; it is not proposing a new precedence rule.
Fix
When the segment before the first top-level pipe contains a top-level comparison or boolean operator, report it:
Why nothing legitimate breaks
There is no working use of a filter on a comparison result:
defaulton a bool is a no-op (a bool is neverNoneor empty)join/map/containson a bool is an errorSo every expression this now refuses was already producing either a wrong value or an error. Verified unaffected:
The operator scan uses the same
_find_top_levelhelper and the same token list as the operator parsing below it, so a>inside a quoted operand is not mistaken for one.Verification
test_filter_on_a_comparison_operand_is_refusedfails on unpatchedsrcand passes with the fix.tests/test_workflows.py: 21 failed → 20 failed, 838 → 839 passed.mainbaseline captured on81bf741(all Windows symlink-privilege).uvx ruff@0.15.0 check src tests→ cleanTests sit beside the existing filter-strictness tests, and a second test pins that plain filters, chains, and bare operators are untouched.
Written with assistance from Claude Code. Bug found, reproduced, and verified by me on current
main.