Skip to content

fix(workflow): correct env variable picker validation#34666

Merged
lyzno1 merged 1 commit intomainfrom
codex/fix-workflow-env-var-picker-icon
Apr 7, 2026
Merged

fix(workflow): correct env variable picker validation#34666
lyzno1 merged 1 commit intomainfrom
codex/fix-workflow-env-var-picker-icon

Conversation

@lyzno1
Copy link
Copy Markdown
Member

@lyzno1 lyzno1 commented Apr 7, 2026

Summary

  • fix the workflow env variable picker so existing env references are validated against the available variable pool instead of source node metadata
  • keep invalid-state rendering aligned with product semantics by only showing the error icon when a selector is actually invalid
  • add regression coverage for valid env references and invalid selectors in the picker helper and branch tests

Root Cause

The picker refactor in #34013 split validation and rendering logic. env selectors were still treated as valid in helper logic, but the trigger rendered the red error icon whenever outputVarNode had no title. Because environment variables are workflow-level data rather than graph nodes, they do not have source node metadata and were incorrectly flagged as errors.

Impact

  • existing workflow environment variables no longer show a false invalid indicator in the variable picker
  • missing env selectors still surface the invalid state once workflow data is loaded
  • picker behavior now matches the prompt editor's stricter env validation semantics

Validation

  • pnpm --dir web test app/components/workflow/nodes/_base/components/variable/__tests__/var-reference-picker.helpers.spec.ts app/components/workflow/nodes/_base/components/variable/__tests__/var-reference-picker.branches.spec.tsx
  • pre-commit hooks: eslint, pnpm --dir web type-check:tsgo, pnpm --dir web knip

@github-actions github-actions Bot added the web This relates to changes on the web. label Apr 7, 2026
@lyzno1 lyzno1 changed the title [codex] fix(workflow): correct env variable picker validation fix(workflow): correct env variable picker validation Apr 7, 2026
@lyzno1 lyzno1 marked this pull request as ready for review April 7, 2026 09:21
@lyzno1 lyzno1 requested review from iamjoel and zxhlyh as code owners April 7, 2026 09:21
@lyzno1 lyzno1 enabled auto-merge April 7, 2026 09:21
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Apr 7, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.75%. Comparing base (75ed38f) to head (5b072b1).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #34666   +/-   ##
=======================================
  Coverage   83.75%   83.75%           
=======================================
  Files        4170     4170           
  Lines      176792   176801    +9     
  Branches    35358    35368   +10     
=======================================
+ Hits       148074   148082    +8     
- Misses      25838    25839    +1     
  Partials     2880     2880           
Flag Coverage Δ
web 85.45% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lyzno1 lyzno1 added this pull request to the merge queue Apr 7, 2026
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Apr 7, 2026
Merged via the queue into main with commit 3891c0a Apr 7, 2026
34 of 35 checks passed
@lyzno1 lyzno1 deleted the codex/fix-workflow-env-var-picker-icon branch April 7, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files. web This relates to changes on the web.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants