From a113e64ba071396bd386f11bf1644f1d9db60ae1 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 13 Jul 2026 12:36:16 -0700 Subject: [PATCH] fix(designer): populate Set Variable name dropdown after pasting Condition scope When a Condition (or other scope) containing an InitializeVariable-referencing Set Variable is copy-pasted into another location, the pasted Set Variable's Name dropdown was empty. Root cause: pasteScopeOperation passes the paste-site upstream slice via pasteParams.existingOutputTokens, but initializeOperationMetadata was not forwarding those upstream node ids into initializeOutputTokensForOperations (defaulted to []), so each pasted node's upstreamNodeIds only contained fragment-internal predecessors -- missing the outer InitializeVariable. Fixes: * Forward Object.keys(pasteParams.existingOutputTokens) as the 5th arg to initializeOutputTokensForOperations so pasted nodes inherit the paste-site upstream slice. * Add a safety fallback in getEditorAndOptions: when the scoped variable list is empty but variables are declared elsewhere, show all declared variables filtered by supportedTypes. The runtime does not scope variable assignments by graph position, so any declared variable is legally assignable. * Applied identically to designer-v2 (which is what the customer's app renders). Tests: * New operationdeserializer.spec.ts covers the pasteParams plumbing. * getEditorAndOptions.spec.ts extended with parameterized fallback tests across all 5 variablename operations. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: af9ea0d5-71e4-45d9-9790-0edc869beadd --- .../__test__/operationdeserializer.spec.ts | 53 +++++++++++++++- .../bjsworkflow/operationdeserializer.ts | 10 +++- .../__test__/getEditorAndOptions.spec.ts | 60 +++++++++++++++++++ .../tabs/parametersTab/index.tsx | 19 +++--- .../__test__/operationdeserializer.spec.ts | 53 ++++++++++++++++ .../bjsworkflow/operationdeserializer.ts | 10 +++- .../__test__/getEditorAndOptions.spec.ts | 60 +++++++++++++++++++ .../tabs/parametersTab/index.tsx | 19 +++--- 8 files changed, 265 insertions(+), 19 deletions(-) create mode 100644 libs/designer/src/lib/core/actions/bjsworkflow/__test__/operationdeserializer.spec.ts diff --git a/libs/designer-v2/src/lib/core/actions/bjsworkflow/__test__/operationdeserializer.spec.ts b/libs/designer-v2/src/lib/core/actions/bjsworkflow/__test__/operationdeserializer.spec.ts index f905a865269..73de3ed5dad 100644 --- a/libs/designer-v2/src/lib/core/actions/bjsworkflow/__test__/operationdeserializer.spec.ts +++ b/libs/designer-v2/src/lib/core/actions/bjsworkflow/__test__/operationdeserializer.spec.ts @@ -1,7 +1,9 @@ -import { initializeOperationDetailsForManagedMcpServer } from '../operationdeserializer'; +import { initializeOperationDetailsForManagedMcpServer, initializeOutputTokensForOperations } from '../operationdeserializer'; import { describe, vi, beforeEach, it, expect } from 'vitest'; import { getConnectorWithSwagger } from '../../../queries/connections'; import { getOperationManifest } from '../../../queries/operation'; +import type { WorkflowNode } from '../../../parsers/models/workflowNode'; +import type { NodesMetadata, Operations } from '../../../state/workflow/workflowInterfaces'; vi.mock('../../../queries/connections', () => ({ getConnectorWithSwagger: vi.fn(), @@ -267,3 +269,52 @@ describe('operationdeserializer', () => { }); }); }); + +// Regression coverage for a scope-paste bug where a pasted node's upstreamNodeIds +// omitted every id that lived outside the pasted fragment. In particular, the +// paste-site's InitializeVariable node was dropped, which left the SetVariable +// "Name" dropdown empty because getAvailableVariables filters variables by the +// current node's upstream slice. The fix threads the paste-site upstream ids +// through initializeOperationMetadata into initializeOutputTokensForOperations +// so every pasted op picks them up. +// +// The function is verified indirectly: with empty allNodesData it silently catches +// per-op errors and still returns a NodeTokens entry for each op containing the +// passed-in existingOutputTokens appended to whatever getTokenNodeIds computed. + +describe('initializeOutputTokensForOperations - existingOutputTokens plumbing', () => { + const graph: WorkflowNode = { + id: 'root', + type: 'GRAPH_NODE', + children: [], + edges: [], + } as unknown as WorkflowNode; + + const nodesMetadata: NodesMetadata = { + op1: { graphId: 'root' }, + op2: { graphId: 'root' }, + } as NodesMetadata; + + const operations: Operations = { + op1: { type: 'Http' }, + op2: { type: 'Compose' }, + } as unknown as Operations; + + it('appends every id in existingOutputTokens to each op upstreamNodeIds', () => { + const result = initializeOutputTokensForOperations(/* allNodesData */ [], operations, graph, nodesMetadata, ['initVarId', 'triggerId']); + + expect(Object.keys(result).sort()).toEqual(['op1', 'op2']); + for (const opId of ['op1', 'op2']) { + expect(result[opId].upstreamNodeIds).toEqual(expect.arrayContaining(['initVarId', 'triggerId'])); + } + }); + + it('defaults existingOutputTokens to [] when not provided', () => { + const result = initializeOutputTokensForOperations(/* allNodesData */ [], operations, graph, nodesMetadata); + + for (const opId of ['op1', 'op2']) { + // With no fragment predecessors and no existingOutputTokens, upstream is empty. + expect(result[opId].upstreamNodeIds).toEqual([]); + } + }); +}); diff --git a/libs/designer-v2/src/lib/core/actions/bjsworkflow/operationdeserializer.ts b/libs/designer-v2/src/lib/core/actions/bjsworkflow/operationdeserializer.ts index 0d3a9dd5832..b71c34b9953 100644 --- a/libs/designer-v2/src/lib/core/actions/bjsworkflow/operationdeserializer.ts +++ b/libs/designer-v2/src/lib/core/actions/bjsworkflow/operationdeserializer.ts @@ -224,7 +224,13 @@ export const initializeOperationMetadata = async ( const variables = initializeVariables(operations, allNodeData); const agentParameters = initializeAgentParameters(nodesMetadata, allNodeData); - const outputTokens = initializeOutputTokensForOperations(allNodeData, operations, graph, nodesMetadata); + const outputTokens = initializeOutputTokensForOperations( + allNodeData, + operations, + graph, + nodesMetadata, + pasteParams ? Object.keys(pasteParams.existingOutputTokens) : [] + ); dispatch( initializeTokensAndVariables({ outputTokens, @@ -610,7 +616,7 @@ const updateTokenMetadataInParameters = ( } }; -const initializeOutputTokensForOperations = ( +export const initializeOutputTokensForOperations = ( allNodesData: NodeDataWithOperationMetadata[], operations: Operations, graph: WorkflowNode, diff --git a/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/__test__/getEditorAndOptions.spec.ts b/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/__test__/getEditorAndOptions.spec.ts index 215cced1168..24f39d135cf 100644 --- a/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/__test__/getEditorAndOptions.spec.ts +++ b/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/__test__/getEditorAndOptions.spec.ts @@ -108,6 +108,66 @@ describe('getEditorAndOptions', () => { expect(getEditorAndOptions(operationInfo, parameter, upstreamNodeIds, variables)).toEqual(expectedEditorAndOptions); }); + describe('"variablename" editor fallback when scoped list is empty', () => { + const variables: Record = { + initInt: [{ name: 'intVar', type: 'integer' }], + initFloat: [{ name: 'floatVar', type: 'float' }], + initStr: [{ name: 'strVar', type: 'string' }], + initArr: [{ name: 'arrVar', type: 'array' }], + initBool: [{ name: 'boolVar', type: 'boolean' }], + }; + + // These supportedTypes values mirror the 5 built-in operations that use the + // 'variablename' editor (see libs/logic-apps-shared/.../manifests/variables.ts): + // setVariable (any), incrementVariable/decrementVariable (float,integer), + // appendToStringVariable (string), appendToArrayVariable (array). + it.each` + operation | supportedTypes | expectedNames + ${'setVariable'} | ${[]} | ${['intVar', 'floatVar', 'strVar', 'arrVar', 'boolVar']} + ${'incrementVariable'} | ${['float', 'integer']} | ${['intVar', 'floatVar']} + ${'decrementVariable'} | ${['float', 'integer']} | ${['intVar', 'floatVar']} + ${'appendToStringVariable'} | ${['string']} | ${['strVar']} + ${'appendToArrayVariable'} | ${['array']} | ${['arrVar']} + `( + 'falls back to all declared variables (respecting supportedTypes) for $operation when upstreamNodeIds is empty', + ({ supportedTypes, expectedNames }) => { + const parameter = getParameterInfo(); + parameter.editor = 'variablename'; + parameter.editorOptions = { supportedTypes }; + + const result = getEditorAndOptions(operationInfo, parameter, /* upstreamNodeIds */ [], variables); + + expect(result.editor).toBe('dropdown'); + expect((result.editorOptions?.options ?? []).map((o: any) => o.value)).toEqual(expectedNames); + } + ); + + it('returns an empty options list when no variables are declared at all', () => { + const parameter = getParameterInfo(); + parameter.editor = 'variablename'; + parameter.editorOptions = { supportedTypes: ['string'] }; + + const result = getEditorAndOptions(operationInfo, parameter, /* upstreamNodeIds */ [], /* variables */ {}); + + expect(result).toEqual({ editor: 'dropdown', editorOptions: { options: [] } }); + }); + + it('does not fall back when the scoped list already has matches (no regression)', () => { + const parameter = getParameterInfo(); + parameter.editor = 'variablename'; + parameter.editorOptions = { supportedTypes: ['string'] }; + + // upstream includes initStr (matches) but excludes initArr — the fallback must NOT + // add arrVar because scopedOptions is non-empty. + const result = getEditorAndOptions(operationInfo, parameter, ['initStr'], variables); + + expect(result).toEqual({ + editor: 'dropdown', + editorOptions: { options: [{ value: 'strVar', displayName: 'strVar' }] }, + }); + }); + }); + it('should support EditorService override', () => { const customEditorOptions = { EditorComponent: vi.fn() }; const editorService = { diff --git a/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx b/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx index 37a04a9ef96..de3f4bde504 100644 --- a/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx +++ b/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx @@ -46,7 +46,7 @@ import { } from '../../../../../core/utils/parameters/helper'; import type { TokenGroup } from '../../../../../core/utils/tokens'; import { createValueSegmentFromToken, getExpressionTokenSections, getOutputTokenSections } from '../../../../../core/utils/tokens'; -import { getAvailableVariables } from '../../../../../core/utils/variables'; +import { getAllVariables, getAvailableVariables } from '../../../../../core/utils/variables'; import { SettingsSection } from '../../../../settings/settingsection'; import type { Settings } from '../../../../settings/settingsection'; import { ConnectionDisplay } from './connectionDisplay'; @@ -1834,12 +1834,17 @@ export const getEditorAndOptions = ( // Handle variable dropdown editor if (equals(editor, constants.EDITOR.VARIABLE_NAME)) { - const options = getAvailableVariables(variables, upstreamNodeIds) - .filter((variable) => supportedTypes.length === 0 || supportedTypes.includes(variable.type)) - .map((variable) => ({ - value: variable.name, - displayName: variable.name, - })); + const toOption = (variable: VariableDeclaration) => ({ value: variable.name, displayName: variable.name }); + const typeMatches = (variable: VariableDeclaration) => supportedTypes.length === 0 || supportedTypes.includes(variable.type); + + const scopedOptions = getAvailableVariables(variables, upstreamNodeIds).filter(typeMatches).map(toOption); + + // Fallback: when the scoped list is empty but variables are declared elsewhere in the workflow, + // show all declared variables. The runtime does not scope variables by graph position, so any + // declared variable is legally assignable (see: pasteScopeOperation bug where a pasted node's + // upstreamNodeIds does not include the root InitializeVariable). Preserves the ideal UX when + // the scoped list is non-empty; only kicks in to prevent an empty-dropdown dead-end. + const options = scopedOptions.length === 0 ? getAllVariables(variables).filter(typeMatches).map(toOption) : scopedOptions; return { editor: 'dropdown', diff --git a/libs/designer/src/lib/core/actions/bjsworkflow/__test__/operationdeserializer.spec.ts b/libs/designer/src/lib/core/actions/bjsworkflow/__test__/operationdeserializer.spec.ts new file mode 100644 index 00000000000..5fde4580be0 --- /dev/null +++ b/libs/designer/src/lib/core/actions/bjsworkflow/__test__/operationdeserializer.spec.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest'; +import { initializeOutputTokensForOperations } from '../operationdeserializer'; +import type { WorkflowNode } from '../../../parsers/models/workflowNode'; +import type { NodesMetadata, Operations } from '../../../state/workflow/workflowInterfaces'; + +// Regression coverage for a scope-paste bug where a pasted node's upstreamNodeIds +// omitted every id that lived outside the pasted fragment. In particular, the +// paste-site's InitializeVariable node was dropped, which left the SetVariable +// "Name" dropdown empty because getAvailableVariables filters variables by the +// current node's upstream slice. The fix threads the paste-site upstream ids +// through initializeOperationMetadata into initializeOutputTokensForOperations +// so every pasted op picks them up. +// +// The function is verified indirectly: with empty allNodesData it silently catches +// per-op errors and still returns a NodeTokens entry for each op containing the +// passed-in existingOutputTokens appended to whatever getTokenNodeIds computed. + +describe('initializeOutputTokensForOperations - existingOutputTokens plumbing', () => { + const graph: WorkflowNode = { + id: 'root', + type: 'GRAPH_NODE', + children: [], + edges: [], + } as unknown as WorkflowNode; + + const nodesMetadata: NodesMetadata = { + op1: { graphId: 'root' }, + op2: { graphId: 'root' }, + } as NodesMetadata; + + const operations: Operations = { + op1: { type: 'Http' }, + op2: { type: 'Compose' }, + } as unknown as Operations; + + it('appends every id in existingOutputTokens to each op upstreamNodeIds', () => { + const result = initializeOutputTokensForOperations(/* allNodesData */ [], operations, graph, nodesMetadata, ['initVarId', 'triggerId']); + + expect(Object.keys(result).sort()).toEqual(['op1', 'op2']); + for (const opId of ['op1', 'op2']) { + expect(result[opId].upstreamNodeIds).toEqual(expect.arrayContaining(['initVarId', 'triggerId'])); + } + }); + + it('defaults existingOutputTokens to [] when not provided', () => { + const result = initializeOutputTokensForOperations(/* allNodesData */ [], operations, graph, nodesMetadata); + + for (const opId of ['op1', 'op2']) { + // With no fragment predecessors and no existingOutputTokens, upstream is empty. + expect(result[opId].upstreamNodeIds).toEqual([]); + } + }); +}); diff --git a/libs/designer/src/lib/core/actions/bjsworkflow/operationdeserializer.ts b/libs/designer/src/lib/core/actions/bjsworkflow/operationdeserializer.ts index 4f0d2700716..72611ef7140 100644 --- a/libs/designer/src/lib/core/actions/bjsworkflow/operationdeserializer.ts +++ b/libs/designer/src/lib/core/actions/bjsworkflow/operationdeserializer.ts @@ -195,7 +195,13 @@ export const initializeOperationMetadata = async ( const variables = initializeVariables(operations, allNodeData); const agentParameters = initializeAgentParameters(nodesMetadata, allNodeData); - const outputTokens = initializeOutputTokensForOperations(allNodeData, operations, graph, nodesMetadata); + const outputTokens = initializeOutputTokensForOperations( + allNodeData, + operations, + graph, + nodesMetadata, + pasteParams ? Object.keys(pasteParams.existingOutputTokens) : [] + ); dispatch( initializeTokensAndVariables({ outputTokens, @@ -583,7 +589,7 @@ const updateTokenMetadataInParameters = ( } }; -const initializeOutputTokensForOperations = ( +export const initializeOutputTokensForOperations = ( allNodesData: NodeDataWithOperationMetadata[], operations: Operations, graph: WorkflowNode, diff --git a/libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/__test__/getEditorAndOptions.spec.ts b/libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/__test__/getEditorAndOptions.spec.ts index 215cced1168..24f39d135cf 100644 --- a/libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/__test__/getEditorAndOptions.spec.ts +++ b/libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/__test__/getEditorAndOptions.spec.ts @@ -108,6 +108,66 @@ describe('getEditorAndOptions', () => { expect(getEditorAndOptions(operationInfo, parameter, upstreamNodeIds, variables)).toEqual(expectedEditorAndOptions); }); + describe('"variablename" editor fallback when scoped list is empty', () => { + const variables: Record = { + initInt: [{ name: 'intVar', type: 'integer' }], + initFloat: [{ name: 'floatVar', type: 'float' }], + initStr: [{ name: 'strVar', type: 'string' }], + initArr: [{ name: 'arrVar', type: 'array' }], + initBool: [{ name: 'boolVar', type: 'boolean' }], + }; + + // These supportedTypes values mirror the 5 built-in operations that use the + // 'variablename' editor (see libs/logic-apps-shared/.../manifests/variables.ts): + // setVariable (any), incrementVariable/decrementVariable (float,integer), + // appendToStringVariable (string), appendToArrayVariable (array). + it.each` + operation | supportedTypes | expectedNames + ${'setVariable'} | ${[]} | ${['intVar', 'floatVar', 'strVar', 'arrVar', 'boolVar']} + ${'incrementVariable'} | ${['float', 'integer']} | ${['intVar', 'floatVar']} + ${'decrementVariable'} | ${['float', 'integer']} | ${['intVar', 'floatVar']} + ${'appendToStringVariable'} | ${['string']} | ${['strVar']} + ${'appendToArrayVariable'} | ${['array']} | ${['arrVar']} + `( + 'falls back to all declared variables (respecting supportedTypes) for $operation when upstreamNodeIds is empty', + ({ supportedTypes, expectedNames }) => { + const parameter = getParameterInfo(); + parameter.editor = 'variablename'; + parameter.editorOptions = { supportedTypes }; + + const result = getEditorAndOptions(operationInfo, parameter, /* upstreamNodeIds */ [], variables); + + expect(result.editor).toBe('dropdown'); + expect((result.editorOptions?.options ?? []).map((o: any) => o.value)).toEqual(expectedNames); + } + ); + + it('returns an empty options list when no variables are declared at all', () => { + const parameter = getParameterInfo(); + parameter.editor = 'variablename'; + parameter.editorOptions = { supportedTypes: ['string'] }; + + const result = getEditorAndOptions(operationInfo, parameter, /* upstreamNodeIds */ [], /* variables */ {}); + + expect(result).toEqual({ editor: 'dropdown', editorOptions: { options: [] } }); + }); + + it('does not fall back when the scoped list already has matches (no regression)', () => { + const parameter = getParameterInfo(); + parameter.editor = 'variablename'; + parameter.editorOptions = { supportedTypes: ['string'] }; + + // upstream includes initStr (matches) but excludes initArr — the fallback must NOT + // add arrVar because scopedOptions is non-empty. + const result = getEditorAndOptions(operationInfo, parameter, ['initStr'], variables); + + expect(result).toEqual({ + editor: 'dropdown', + editorOptions: { options: [{ value: 'strVar', displayName: 'strVar' }] }, + }); + }); + }); + it('should support EditorService override', () => { const customEditorOptions = { EditorComponent: vi.fn() }; const editorService = { diff --git a/libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx b/libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx index b224731f895..194355fb307 100644 --- a/libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx +++ b/libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx @@ -46,7 +46,7 @@ import { } from '../../../../../core/utils/parameters/helper'; import type { TokenGroup } from '../../../../../core/utils/tokens'; import { createValueSegmentFromToken, getExpressionTokenSections, getOutputTokenSections } from '../../../../../core/utils/tokens'; -import { getAvailableVariables } from '../../../../../core/utils/variables'; +import { getAllVariables, getAvailableVariables } from '../../../../../core/utils/variables'; import { SettingsSection } from '../../../../settings/settingsection'; import type { Settings } from '../../../../settings/settingsection'; import { ConnectionDisplay } from './connectionDisplay'; @@ -1859,12 +1859,17 @@ export const getEditorAndOptions = ( // Handle variable dropdown editor if (equals(editor, constants.EDITOR.VARIABLE_NAME)) { - const options = getAvailableVariables(variables, upstreamNodeIds) - .filter((variable) => supportedTypes.length === 0 || supportedTypes.includes(variable.type)) - .map((variable) => ({ - value: variable.name, - displayName: variable.name, - })); + const toOption = (variable: VariableDeclaration) => ({ value: variable.name, displayName: variable.name }); + const typeMatches = (variable: VariableDeclaration) => supportedTypes.length === 0 || supportedTypes.includes(variable.type); + + const scopedOptions = getAvailableVariables(variables, upstreamNodeIds).filter(typeMatches).map(toOption); + + // Fallback: when the scoped list is empty but variables are declared elsewhere in the workflow, + // show all declared variables. The runtime does not scope variables by graph position, so any + // declared variable is legally assignable (see: pasteScopeOperation bug where a pasted node's + // upstreamNodeIds does not include the root InitializeVariable). Preserves the ideal UX when + // the scoped list is non-empty; only kicks in to prevent an empty-dropdown dead-end. + const options = scopedOptions.length === 0 ? getAllVariables(variables).filter(typeMatches).map(toOption) : scopedOptions; return { editor: 'dropdown',