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',