Colliding sanitized workflow identifiers chain Jinja reference rewrites onto the wrong parameter
Found while reading sanitize_workflow_yaml_with_references after Skyvern-AI/skyvern#7279. This is a different defect from #7278 (which covers the direct-string pass) — the Jinja reference rewrites have their own chaining problem.
The rewrites in steps 3/4 run once per renamed identifier, sequentially. So when a collision-driven rename overlaps an earlier one, the later substitution re-hits the earlier one's output. Import this:
workflow_definition:
parameters:
- key: user email # sanitized to user_email
- key: user_email # valid, but now colliding -> suffixed to user_email_2
blocks:
- block_type: task
label: send
navigation_goal: "Send to {{ user email }} and CC {{ user_email }}"The goal comes out as Send to {{ user_email_2 }} and CC {{ user_email_2 }} — both references now point at the second parameter, and the workflow runs with the wrong value. No error at import or run time (default lax templating). Block labels hit the same thing through the {label}_output rewrites: two blocks whose labels collide end up with their output references collapsed onto one block, e.g. {{ foo/bar_output }} vs {{ foo_bar_output }} becomes {{ foo_bar_2_output }} vs {{ foo_bar_2_output }}.
The codebase already treats this chaining as a bug: _rewrite_error_code_mapping_refs_atomic applies all substitutions in one sentinel-guarded pass precisely so foo-bar -> foo_bar and foo_bar -> foo_bar_2 can't combine, and there's a regression test for it. But that guard only covers error_code_mapping — blocks, parameters, and workflow_system_prompt still get the per-entry loop. This runs on POST /workflows and POST /workflows/{id} whenever a yaml_definition is sent.
PR incoming that extends the existing atomic approach to the other three rewrite targets.
Separate but related: replace_jinja_reference only matches a name directly after {{, so references inside {% if %} / {% for %} or mid-expression (like the documented while_loop pattern {{ current_index < max_attempts }}) are never rewritten at all when their identifier is renamed. That's a matching-coverage question rather than a chaining one, so I've left it out of the PR — happy to file it separately.
Source: Skyvern-AI/skyvern