Skip to content

feat: Finalize Conditional Execution - #2657

Open
camielvs wants to merge 1 commit into
08-20-feat_conditional_execution_-_option_1from
08-20-feat_finalize_conditional_execution
Open

feat: Finalize Conditional Execution#2657
camielvs wants to merge 1 commit into
08-20-feat_conditional_execution_-_option_1from
08-20-feat_finalize_conditional_execution

Conversation

@camielvs

@camielvs camielvs commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Description

Final pass over conditional execution. Nothing changes about what gets sent to the backend — this is UI, vocabulary and test coverage. The visual direction is borrowed from the exploration in #2649.

"Run when", not "isEnabled". isEnabled is spec jargon. The UI now says Run when, and the two literal choices read Always / Never rather than true/false.

Conditional execution gets its own purple box in the Config tab. A single switch turns it on for a task. When it's on, the Run when control appears inside the box — either the Always/Never toggle, or, if something is wired into it, the upstream source it follows (→ Flag.flag), formatted the same way bound inputs are formatted everywhere else in the editor.

On the node, the condition is separate from the inputs. It used to be injected into the task's input list as a fake input, which meant the list had to know about it in every place it did anything (splitting, condensing, counting). It's now its own purple row above the inputs with its own handle, which means:

  • it stays visible when a node's inputs are condensed, instead of being counted into "+3 more"
  • the input-list code went back to exactly what it is on master — the whole special case is gone
  • collapsed nodes get the same treatment: a purple handle plus a branch icon

Dragging off the "Run when" handle no longer creates a graph input called __is_enabled__. The editor names an auto-created input after the port you dragged from, and the reserved port name was leaking into the user's pipeline. It now creates an input called run_condition, typed String — that's the form the condition is actually read in, and it's what lets the input connect to the ports components declare.

Naming and comments. Internal names now say what they are (resetRunCondition, runConditionBinding, setRunCondition, …). Comments that narrated the code were deleted; the ones left explain a decision the code can't.

Tests. New coverage for the shared helpers, the enable/disable actions (including that switching conditional execution off clears both the literal and the connection while leaving other connections alone), the node rendering, the auto-created input, and each of the fixes below.

Fixes from review

  • A hand-written or SDK-generated pipeline can set the condition to an unquoted false. That now reads as Never; before, it showed as Always while the backend skipped the task — the display and the behaviour disagreed.
  • A condition pointing at something that no longer exists now shows what it pointed at, instead of quietly falling back to Always. Loading such a pipeline also keeps the condition rather than leaving the task ungated.
  • Grouping tasks into a subgraph, and ungrouping them again, no longer leaks the internal port name into the pipeline. A promoted condition becomes a readable run_condition input, and a fixed condition survives the round trip.
  • Dragging off the handle of a task set to Never no longer ungates it. The auto-created input now starts out holding the task's own condition, so the task keeps running when it was set to.
  • The Run when control is now properly labelled for screen readers.
  • One violet palette, one icon and one stated reason for both, so the condition row can't drift apart between the full and collapsed node.

Related Issue and Pull requests

Stacked on #2651. UI direction borrowed from #2649. Followed by #2658, which stops conditional execution being set where the backend won't honour it.

Type of Change

  • Improvement

Checklist

  • I have tested this does not break current pipelines / runs functionality
  • I have tested the changes on staging

Screenshots (if applicable)

image.png

image.png

image.png

image.png

Test Instructions

The conditional-execution flag is off by default — turn it on in Settings first.

  1. Select a task → Config tab → toggle Conditional execution. The purple box expands to show Run when, and the node grows a purple row.
  2. Flip Run when between Always and Never; the node's row should follow.
  3. Condense the node's inputs — the condition row stays visible.
  4. Drag from the node's purple handle onto empty canvas. You should get a run_condition String graph input, not one named __is_enabled__.
  5. Set a task to Never, then drag off its handle. The new input should already hold false, and the task should still read as gated off — not silently switch to running.
  6. Wire a task output into the handle. Both the node and the Config panel should read → Task.output.
  7. Delete that edge. The task stays conditional and resets to Always rather than silently staying Never.
  8. Export the pipeline YAML and re-import it — the condition should survive the round trip.
  9. Hand-edit a pipeline's YAML to set a task's condition to a bare false (no quotes), then open it. The task should read Never.
  10. Select a conditional task and a neighbour, group them into a subgraph, then ungroup them. The condition should come back intact, and no input named __is_enabled__ should appear anywhere.

Additional Comments

Turning conditional execution off deliberately clears the condition (both the literal and any connection) rather than remembering it, so re-enabling starts from Always. That's the trade discussed in #2651.

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

🎩 Preview

A preview build has been created at: 08-20-feat_finalize_conditional_execution/20b5ec3

camielvs commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Comment thread src/utils/conditionalExecution.ts Outdated
Comment thread src/routes/v2/pages/Editor/store/actions/io.actions.ts Outdated
Comment thread src/routes/v2/shared/nodes/TaskNode/TaskNodeCard.test.tsx
Comment thread src/routes/v2/shared/nodes/TaskNode/TaskNodeCard.tsx Outdated
Comment thread src/routes/v2/shared/nodes/TaskNode/TaskNodeCard.tsx Outdated
Comment thread src/routes/v2/shared/nodes/TaskNode/TaskNodeSimplified.tsx Outdated
Comment thread src/utils/componentSpec.ts
@camielvs

Copy link
Copy Markdown
Collaborator Author

🤖 This is an AI-generated review summary.

Carry-over audit: does this stack close out #2574's review comments?

#2574's comments aren't meant to be actioned there — the expectation is that #2651 and #2657 deal with them. This is the audit of that, posted at the top of the stack so it's in one place. 23 comments on the root PR: 10 resolved, 8 still open, 1 changed shape, 1 partial.

Resolved by #2651 / #2657

# Comment Where it was addressed
1 as ArgumentType cast on the literal #2651 — literals now flow through typed helpers, no cast
2 EnableTaskMode cast #2651 — the mode enum is gone with the dropdown
3 Two sources of truth / annotation namespace #2651 — the annotation is removed entirely; conditional-ness is derived
4 Re-export through annotations.ts #2651 — no longer needed
5 is_enabled isn't a real input, don't render it in the input list #2657 — own row above the inputs, input-list code back to master
6 Ark-kun: "a separate middle section" for it #2657 — that's what the Run-when row is
7 No canvas signal that a task is disabled #2657 — purple handle + branch icon on collapsed nodes
8 "Conditional with nothing wired is a silent no-op" #2657 — Always / Never literal means there's always a defined state
9 Beta flag #2657 — behind a flag, off by default
10 Mode-transition tests #2651 / #2657 — 20 new tests across the two

Still open

# Comment Status
1 isEnabled may arrive as a boolean / "False" — strict === "false" is wrong Not fixed. Now centralized in toConditionLiteral, so it's a one-line fix — see the inline comment on conditionalExecution.ts:23.
2 Conditional execution on subgraph nodes (r3686778763 never answered) Not fixed — see the inline comment on ConfigurationSection.tsx:198.
3 Fail-open when the reference can't be resolved (yamlDeserializer.ts:161, :200) Unchanged by either PR.
4 Sentinel port name leaking through createSubgraph.ts:131 Unchanged. #2657 fixes the leak into the auto-created graph input name, not this path.
5 A component legitimately declaring an input called __is_enabled__ Unchanged.
6 Duplicated reference regexes (yamlDeserializer.ts:28-29 vs conditionalExecution.ts:29-30) Unchanged.
7 No warning when the connected output isn't String/Boolean Unchanged.
8 Test gaps: createSubgraph/unpackSubgraph with a conditional binding; boolean and uppercase literals Unchanged.

Changed shape

Boolean port type / quick-connect compatibility. The original comment was about the __is_enabled__ port; #2657 removes that surface but re-introduces the same mismatch on the auto-created graph input, which is typed Boolean in io.actions.ts:241. r3686547226 recommended String or no type at all, and that was accepted on the root PR — inline comment on io.actions.ts:241.

Partially addressed

Accessible labelling. The Config panel switch now has a real <Label htmlFor>, which is the bigger half. The Run-when Tabs (Always / Never) still has no accessible name.

Cross-stack note

resolveConditionalReference duplicates JsonSerializer.bindingToArgument. It's introduced in #2651, so the inline comment lives there — flagging it here so it doesn't get lost when the stack merges.

@camielvs
camielvs force-pushed the 08-20-feat_finalize_conditional_execution branch from 325c67e to 2d0ce95 Compare August 21, 2026 16:52
@camielvs
camielvs marked this pull request as ready for review August 21, 2026 17:26
@camielvs
camielvs requested a review from a team as a code owner August 21, 2026 17:26
Comment thread src/routes/v2/pages/Editor/store/actions/io.actions.ts
@camielvs
camielvs requested a review from Mbeaulne August 21, 2026 22:38
@camielvs
camielvs force-pushed the 08-20-feat_finalize_conditional_execution branch from 2d0ce95 to 20b5ec3 Compare August 21, 2026 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants