Skip to content

fix(workflows): stop list-literal-then-index expressions silently corrupting - #4572

Open
Noor-ul-ain001 wants to merge 1 commit into
github:mainfrom
Noor-ul-ain001:fix/expression-list-literal-index-misparse
Open

Noor-ul-ain001 wants to merge 1 commit into
github:mainfrom
Noor-ul-ain001:fix/expression-list-literal-index-misparse

Conversation

@Noor-ul-ain001

Copy link
Copy Markdown
Contributor

Summary

_evaluate_simple_expression's list-literal detection (workflows/expressions.py) is expr.startswith("[") and expr.endswith("]"). This is true not only for a genuine literal like [1, 2, 3] but also for a list literal immediately followed by an index suffix, e.g. [1,2,3][1] — which reads as "index 1 of [1,2,3]" (i.e. 2).

Naively stripping the outer brackets from "[1,2,3][1]" produces "1,2,3][1", which the comma-splitter then breaks into ["1", "2", "3][1"]. The malformed last segment resolves to None via the existing dot-path fallback, so the whole expression silently evaluates to [1, 2, None] — no error, no warning, just wrong data.

>>> evaluate_expression("{{ [1,2,3][1] }}", ctx)
[1, 2, None]          # should read as index 1 of [1,2,3], i.e. 2

This is the exact "grabs the wrong span" failure mode the adjacent string-literal check already guards against a few lines above (verifying the matching quote is the last character, not just present anywhere) — the list-literal branch never got the same rigor.

Fix

Added _is_single_list_literal: a quote/bracket-depth scan (mirroring _split_top_level_commas's existing tracking style in the same file) that confirms the opening [ closes exactly at the final character before treating the expression as one literal. A misclassified expression now falls through to the existing dot-path resolution and evaluates to None — consistent with how every other unresolvable expression in this module already behaves, not a new failure mode. This does not add support for indexing directly into a literal list (an author would still write steps.x.output.list[0], which the dot-path resolver already supports); it only stops the misclassification from silently corrupting the result.

Test plan

  • ruff check . clean
  • New test fails against the pre-fix code (assert [1, 2, None] is None → confirmed via test-the-test) and passes with the fix
  • Full test_workflows.py suite (985 tests) passes

…rupting

_evaluate_simple_expression's list-literal detection was
`expr.startswith("[") and expr.endswith("]")` -- true not only for a
genuine literal like `[1, 2, 3]` but also for a list literal immediately
followed by an index suffix, e.g. `[1,2,3][1]` (read as "index 1 of
[1,2,3]", i.e. 2). Naively stripping the outer brackets from that string
produces the garbage `1,2,3][1`, which the comma-splitter then breaks
into `["1", "2", "3][1"]`; the last segment resolves to None via the
dot-path fallback, so `{{ [1,2,3][1] }}` silently evaluated to
`[1, 2, None]` instead of raising or resolving the index -- no error, no
warning, just wrong data.

This is the same "grabs the wrong span" failure mode the adjacent
string-literal check already guards against (verifying the matching
quote is the last character, not just present), just never given the
same treatment for brackets.

Added _is_single_list_literal: a quote/bracket-depth scan (mirroring
_split_top_level_commas's existing tracking in this same file) that
confirms the opening `[` closes exactly at the final character before
treating the expression as one literal. A misclassified expression now
falls through to the existing dot-path resolution and evaluates to None
-- consistent with how every other unresolvable expression in this
module already behaves, not a new failure mode.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U74yBbvVQCPwB7Ed8Dzeu6
@mnriem mnriem added author-over-cap Over the 3-open-PR cap or repetitive batch submissions — please consolidate triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review labels Sep 14, 2026
@mnriem

mnriem commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

The list-literal misclassification is reproducible, but the change replaces a malformed list result with an unresolved value rather than supporting the expression or reporting an error.

Please provide a concrete affected workflow and its before/after outcome, and narrow the claim that returning None is safe. Depending on the consumer, that value can become empty text or a false condition. We’re keeping this as triage-can-wait pending that context; this is not a request to add literal-list indexing or redesign the parser.

Please also complete the AI disclosure with the tool, mode/settings, and extent of assistance; the Claude Sonnet 5 attribution is already present.

Drafted for @mnriem with assistance from GitHub Copilot (model: GPT-6 Astra; interactive comment drafting).

@mnriem mnriem added triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate author-needs-disclosure AI use, or the agent/model/settings behind it, not disclosed per CONTRIBUTING author-awaiting Waiting on author response author-needs-info Missing detail needed to assess — supply requested info and removed author-over-cap Over the 3-open-PR cap or repetitive batch submissions — please consolidate triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review labels Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author-awaiting Waiting on author response author-needs-disclosure AI use, or the agent/model/settings behind it, not disclosed per CONTRIBUTING author-needs-info Missing detail needed to assess — supply requested info triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants