fix(web): allow duplicate Output node variables on mutually exclusive branches - #39785
fix(web): allow duplicate Output node variables on mutually exclusive branches#39785lyfuci wants to merge 2 commits into
Conversation
… branches Publishing was rejected whenever two Output (End) nodes declared the same output variable name, regardless of graph topology. When those nodes sit on mutually exclusive branches only one of them runs per execution, so reusing a variable name there is legitimate and should not block publishing. Move the graph reasoning into getDuplicateEndOutputVariables, which walks the graph from its entry nodes and records the branch decisions that lead to each node. Two Output nodes are exclusive when every way of reaching one contradicts every way of reaching the other, so a duplicate is reported only when both can actually run in the same execution. This keeps the existing protection for Output nodes on parallel branches and for a variable declared twice by a single Output node. Temporary dependency-highlight edges are skipped, since their source_tmp handles would otherwise invent paths that bypass real branches. Fixes langgenius#38440
iamjoel
left a comment
There was a problem hiding this comment.
When mutually exclusive branches define the same Output variable with different types—for example, result is a string on the true branch and an object on the false branch—the current implementation tracks only the variable name and allows the duplicate because the branches are mutually exclusive.
However, the workflow tool output schema merges same-named definitions, with the later End node overriding the earlier type. As a result, one branch may return a value that does not match the published tool schema, potentially breaking consumers.
Please only allow same-named outputs when their value_types are compatible. Otherwise, retain a validation error or introduce a dedicated output-schema conflict error, with a regression test covering mismatched types such as string and object.
… types Allowing a reused output variable name on mutually exclusive branches is only safe when the declared types agree. The published output keeps a single definition per name — get_workflow_graph_output lets the later Output node win — so a branch declaring `result` as an object while another declares it as a string could return a value that does not match the schema consumers were given. Track value_type alongside the variable name and report a dedicated conflictingOutputVariableTypes error for that case, keeping the existing duplicate-name error for Output nodes that can run in the same execution. Type compatibility mirrors filterVar / filterVarByType: `any` matches everything, and an absent type cannot prove a clash.
|
Good catch, thank you — you're right, and it's the same merge behaviour I cited as an argument for the
Reusing a name across exclusive branches is still allowed when the types agree. I added a dedicated error rather than reusing the duplicate-name one, since the fix a user needs is Type compatibility mirrors the existing
Regression tests added, including the string/object case you asked for:
25 unit tests plus 2 checklist-level tests; One thing to flag: I added the new key to |
Summary
Output (End) nodes on mutually exclusive branches may reuse the same output variable name.
Since v1.15.0 (#35511) the editor rejects any two Output nodes that share an output variable name.
The check exists for a good reason — per #35510 item 3, two Output nodes that both emit the same name
silently overwrite each other — but it ignores graph topology. A workflow whose
trueandfalsebranches each end in their own Output node therefore cannot be published, even though only one of those
Output nodes can ever run.
This makes the check branch-aware instead of removing it: a duplicate is reported only when the two
Output nodes can actually run in the same execution.
Worth noting that the backend already treats same-named outputs as one variable —
WorkflowToolConfigurationUtils.get_workflow_graph_outputcollapses them, with "Later end nodesoverride duplicated variable definitions." For mutually exclusive branches that is exactly the schema a
user wants: a single
resultregardless of which branch ran. Nothing server-side rejects the duplicate,so the editor was the only thing blocking it.
Fixes #38440
Credit to @EvanYao826, who diagnosed this in #38488 and correctly identified
getDuplicateEndOutputMessagesas the cause. That PR removes the check outright, which also drops theprotection for Output nodes on parallel branches (#35510 item 3) and for a variable declared twice
inside one Output node; this takes the narrower route instead.
How it works
Exclusive branching is already visible in the graph: edges leaving one node through distinct
sourceHandles are alternatives, whereas several edges leaving the same handle are a parallelfan-out where every target runs. That covers every branching node in the editor without a node-type
allow-list — If/Else (
true/ case ids /false), Question Classifier (class ids), error handling(
sourcevsfail-branch) and Human Input (action ids vs__timeout).getDuplicateEndOutputVariableswalks the graph from its entry nodes and records, for each node, thesets of branch decisions that lead to it (
branch node -> handle taken). Two Output nodes are mutuallyexclusive when every way of reaching one contradicts every way of reaching the other. Tracking whole
decision sets rather than one branch at a time matters, because failure branches often merge into a
single shared error Output node — checking branches independently reports a false positive there.
Details:
the tracked combination limit falls back to "may run together", keeping the warning rather than
dropping a real conflict.
triggers do not collide.
data._isTemp, injected byhandleNodeSelectwhile highlighting a node's variabledependencies) are skipped. They use a
source_tmphandle and would otherwise invent paths that bypassreal branches, making the warning appear and disappear as nodes are selected.
inside the existing
useMemo.Preserved behaviour: Output nodes on parallel branches sharing a name are still reported, a single Output
node declaring the same name twice is still reported, and blank names are still ignored.
Screenshots
No visual change — this is validation logic, so the only observable difference is that a workflow which
previously refused to publish now publishes. The failing state is captured in #38440. Repro and
verification steps below instead.
How to verify
Start → If/Else, with thetruebranch going to one Output node and thefalsebranch to another.
result.test-run either, since
useWorkflowRunValidationgates on the same checklist.Startat two Output nodes directly (a parallel fan-out, whereboth run) with the same variable name, and a single Output node declaring the same name twice.
Changes
web/app/components/workflow/utils/end-output-conflicts.ts(new): puregetDuplicateEndOutputVariables(nodes, edges)returning the conflicting variable names per Outputnode id.
web/app/components/workflow/hooks/use-checklist.ts:getDuplicateEndOutputMessagesdelegates tothat helper and only maps results to i18n messages; both call sites pass
edges.web/app/components/workflow/utils/__tests__/end-output-conflicts.spec.ts(new): 20 unit tests —if/else, success vs
fail-branch, Human Input timeout, several classifier classes merging into oneOutput node, a shared fallback Output node behind nested branches, if/else chains, branch bypass,
multiple entry nodes,
parentIdscopes, cyclic graphs, temporary edges, disconnected Output nodes,blank names, partial name overlap.
web/app/components/workflow/hooks/__tests__/use-checklist.spec.ts: added a checklist-level testfor the How can different end nodes output the same key variable? #38440 topology. The existing
should detect duplicate output variables across end nodestestis unchanged and still passes, since its graph is a parallel fan-out.
One thing left for maintainers to decide:
errorMsg.duplicateOutputVariablestill reads "Output nodevariable names must be unique." That is accurate whenever the warning now fires, but it slightly
overstates the rule. I left the copy alone rather than machine-translate a reworded string into 22
locales — happy to update it if you tell me the wording you want.
Checklist
single atomic change.
pnpm exec vp stagedis clean; note it has to run from the repo root, sincethe
stagedconfig lives in the rootvite.config.tsrather thanweb/. Also ranvp test app/components/workflow(600 files / 3145 tests, all passing).From Claude Code