[One Workflow] Fix the variable type detection within liquidjs for loops

[dej611]

Summary

Liquid {% for %} loops in workflow YAML were not validated or autocompleted reliably. Invalid collection paths could slip through, and loop variables inside block-folded (>-) message fields were often treated as unknown because offset mapping used the wrong source text.

This change validates for-loop collection paths against the step context schema (same rules as foreach steps) and reports dedicated diagnostics, separate from variable validation. Loop variables in invalid collections stay permissive in the variable pass so users get one clear error on the collection, not duplicate markers.

Block literal and folded scalars now map cursor offsets using the editor’s YAML source, so template-local context (assign, capture, for-loop scope) lines up with what the user actually typed. Liquid syntax and for-loop collection checks share a single YAML pass when the workflow graph is available, and template parsing skips work when a scalar has no {% tags.

Checklist

• Unit or functional tests were updated or added to match the most common scenarios

[rosomri]

While running the branch locally, I identified a couple of additional false positives in the Liquid {% for %} validation -

{% assign rows = consts.items %}{% for row in rows %} should not report rows as invalid, because rows is a Liquid-local variable. Similarly, {% for i in (1..3) %} is valid Liquid but not a workflow context path.

Can we limit collection-path diagnostics to expressions that are actually workflow context paths, and leave Liquid-local/range/non-path expressions alone unless we can resolve them through template-local type inference?

In the meantime, to unblock users from saving workflows with false-positive Liquid validation errors, I’ll reduce the error severity to a warning.

--- In the meantime, to unblock users from saving workflows with false-positive Liquid validation errors, I’ll reduce the error severity to a warning. ---
^^ nvm, the errors are not Liquid-related but come from variable path validation, so they can’t generally be downgraded to warnings.

Workflows with errors as reference -

name: Local assigned collection
enabled: false
triggers:
  - type: manual
consts:
  items:
    - name: Alice
    - name: Bob
steps:
  - name: summarize
    type: console
    with:
      message: >-
        {% assign rows = consts.items %}
        {% for row in rows %}
        - {{ row.name }}
        {% endfor %}

Why it fails: the validator sees rows as the loop collection and checks it against the workflow context schema. rows is a Liquid-local variable, not a workflow context path, so it reports Collection rows is invalid.

name: Range loop
enabled: false
triggers:
  - type: manual
steps:
  - name: count
    type: console
    with:
      message: >-
        {% for i in (1..3) %}
        Page {{ i }}
        {% endfor %}

Why it fails: (1..3) is valid LiquidJS, but it is not a variable path, so parseVariablePath('(1..3)') rejects it and the validator reports Invalid collection path: (1..3).

name: Nested local collection
enabled: false
triggers:
  - type: manual
consts:
  groups:
    - name: admins
      users:
        - name: Alice
steps:
  - name: summarize
    type: console
    with:
      message: >-
        {% for group in consts.groups %}
          {% assign users = group.users %}
          {% for user in users %}
          {{ group.name }}: {{ user.name }}
          {% endfor %}
        {% endfor %}

Why it fails: users is valid inside Liquid, but the collection validator only validates against the workflow schema and does not account for locals introduced earlier in the template.

[kibanamachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: c9b67d1
• Build duration: 51 mins

Failed CI Steps

• Jest Tests #9
• FTR Configs #6
• FTR Configs #92

Test Failures

• [job] [logs] Jest Tests #9 / StepAboutRuleComponent does not modify the provided risk score until the user changes the severity

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
workflowsManagement	2.6MB	2.6MB	+3.7KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
workflowsManagement	129	130	+1

Total ESLint disabled count

id	before	after	diff
workflowsManagement	152	153	+1

History

• 💛 Build #454054 was flaky d5e88eb
• 💛 Build #453821 was flaky f7801fd
• 💔 Build #453804 failed da3db86
• 💛 Build #453777 was flaky ad293dc
• 💛 Build #448755 was flaky e8964f3
• 💛 Build #447957 was flaky 8381502

cc @dej611

[rosomri]

The changes look good overall. I left two comments on concrete edge cases.

Stepping back a bit, and thinking out loudly (let me know wdyt), but this feels like we’re entering a long-term maintenance race with LiquidJS.

The current direction already needs Kibana-side handling for Liquid scoping semantics: assign, reassignment order, for loop scopes, nested loops, range expressions, filters, forloop, block scalar offset mapping, etc.

My main concern is this becomes a never-ending race where each valid Liquid construct needs another local patch, and a future LiquidJS major version could break assumptions we made about the AST.

Longer term, I think the robust fix is to push this kind of logic into LiquidJS static analysis, rather than expanding custom Liquid semantics inside workflows.

What we need from the Liquid side is something like:

analyzeTemplate(template, {
  resolveGlobal(path),
  getProperty(type, key),
  getIterableItem(type),
  applyFilter(type, filter, args),
})

And it would return typed/resolved references plus locations, for example:

{
  references: [
    {
      sourcePath: ['row', 'name'],
      resolvedPath: ['consts', 'items', '*', 'name'],
      range,
      type,
    }
  ],
  locals,
  scopes,
  diagnostics
}

Liquid should own Liquid semantics: scoping, reassignment order, assign, capture, for, ranges, filters, forloop, increment/decrement, nesting, and future syntax changes.

Kibana should own only the host-specific parts: building the workflow context schema for the current step, resolving workflow paths like steps.foo.output, deciding step visibility, and mapping template-relative ranges back to YAML/Monaco positions.

For this PR, maybe we should keep the validation conservative: validate collection expressions only when we can confidently resolve them as workflow context paths, and avoid hard errors for dynamic/unknown Liquid expressions. Then we can separately explore contributing a schema-aware static analysis API upstream to LiquidJS.

[rosomri]

Dynamic Liquid ranges are still treated as invalid collection paths. LiquidJS accepts {% for i in (1..n) %} and {% for i in (start..finish) %}, but LIQUID_RANGE_LITERAL_REGEX only exempts numeric literals like (1..3). Those valid templates fall through to validateCollectionPath() and get Invalid collection path.

Consider detecting range expressions structurally via LiquidJS token/AST shape, or at least broadening the range exemption beyond numeric literals...

[dej611]

That's expected for now. I've avoided to blow up the scope here. When a range is encountered it will just early exit and fallback to any validation

[rosomri]

resolveAssignChain() uses the first assignment for a local name, but Liquid assignment semantics should use the latest in-scope assignment before the use. This can produce false errors or false passes after reassignment:

{% assign rows = steps.missing.output %}
{% assign rows = consts.items %}
{% for row in rows %}{{ row.name }}{% endfor %}

[dej611]

Yeah, I guess this type of things may be incorrectly detected. It is a very bad pattern tho. Even TS engine would be fooled about it.

[kibanamachine]

Starting backport for target branches: 9.4

https://github.com/elastic/kibana/actions/runs/27546998035

[kibanamachine]

💔 All backports failed

Status	Branch	Result
❌	9.4	Backport failed because of merge conflicts You might need to backport the following PRs to 9.4: - [One Workflow] Support nested data map aliases in validation (#270008)

Manual backport

To create the backport manually run:

node scripts/backport --pr 270596

Questions ?

Please refer to the Backport tool documentation

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 270596 locally
cc: @dej611

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 270596 locally
cc: @dej611

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 270596 locally
cc: @dej611

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 270596 locally
cc: @dej611

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 270596 locally
cc: @dej611

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 270596 locally
cc: @dej611

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 270596 locally
cc: @dej611