diff --git a/cypress/e2e/14-mapping.cy.ts b/cypress/e2e/14-mapping.cy.ts index b8d263941f..3af70bc270 100644 --- a/cypress/e2e/14-mapping.cy.ts +++ b/cypress/e2e/14-mapping.cy.ts @@ -1,10 +1,10 @@ +import { WorkflowPage, NDV } from '../pages'; +import { getVisibleSelect } from '../utils'; import { MANUAL_TRIGGER_NODE_NAME, MANUAL_TRIGGER_NODE_DISPLAY_NAME, SCHEDULE_TRIGGER_NODE_NAME, } from './../constants'; -import { WorkflowPage, NDV } from '../pages'; -import { getVisibleSelect } from '../utils'; const workflowPage = new WorkflowPage(); const ndv = new NDV(); diff --git a/cypress/e2e/27-two-factor-authentication.cy.ts b/cypress/e2e/27-two-factor-authentication.cy.ts index 2201e65285..877345ccc1 100644 --- a/cypress/e2e/27-two-factor-authentication.cy.ts +++ b/cypress/e2e/27-two-factor-authentication.cy.ts @@ -1,11 +1,11 @@ import generateOTPToken from 'cypress-otp'; -import { MainSidebar } from './../pages/sidebar/main-sidebar'; import { INSTANCE_OWNER, INSTANCE_ADMIN, BACKEND_BASE_URL } from '../constants'; import { SigninPage } from '../pages'; import { MfaLoginPage } from '../pages/mfa-login'; import { successToast } from '../pages/notifications'; import { PersonalSettingsPage } from '../pages/settings-personal'; +import { MainSidebar } from './../pages/sidebar/main-sidebar'; const MFA_SECRET = 'KVKFKRCPNZQUYMLXOVYDSQKJKZDTSRLD'; diff --git a/cypress/e2e/33-settings-personal.cy.ts b/cypress/e2e/33-settings-personal.cy.ts index 6b5cc94687..183655cbe8 100644 --- a/cypress/e2e/33-settings-personal.cy.ts +++ b/cypress/e2e/33-settings-personal.cy.ts @@ -35,7 +35,7 @@ describe('Personal Settings', () => { successToast().find('.el-notification__closeBtn').click(); }); }); - // eslint-disable-next-line n8n-local-rules/no-skipped-tests + it('not allow malicious values for personal data', () => { cy.visit('/settings/personal'); INVALID_NAMES.forEach((name) => { diff --git a/cypress/pages/ndv.ts b/cypress/pages/ndv.ts index caa2ead737..5b672d83eb 100644 --- a/cypress/pages/ndv.ts +++ b/cypress/pages/ndv.ts @@ -305,7 +305,7 @@ export class NDV extends BasePage { this.actions.typeIntoParameterInput(fieldName, invalidExpression ?? "{{ $('unknown')", { parseSpecialCharSequences: false, }); - this.actions.validateExpressionPreview(fieldName, "node doesn't exist"); + this.actions.validateExpressionPreview(fieldName, 'No path back to node'); }, openSettings: () => { this.getters.nodeSettingsTab().click(); diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index 91e9f540c7..f4237c8adc 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -149,7 +149,7 @@ Cypress.Commands.add('grantBrowserPermissions', (...permissions: string[]) => { }); Cypress.Commands.add('readClipboard', () => - cy.window().then((win) => win.navigator.clipboard.readText()), + cy.window().then(async (win) => await win.navigator.clipboard.readText()), ); Cypress.Commands.add('paste', { prevSubject: true }, (selector, pastePayload) => { diff --git a/packages/workflow/src/errors/expression.error.ts b/packages/workflow/src/errors/expression.error.ts index 8b8bb6ef86..9e97418e92 100644 --- a/packages/workflow/src/errors/expression.error.ts +++ b/packages/workflow/src/errors/expression.error.ts @@ -28,6 +28,25 @@ export interface ExpressionErrorOptions { /** * Class for instantiating an expression error */ +// Expression error constants +export const EXPRESSION_ERROR_MESSAGES = { + NODE_NOT_FOUND: 'Error finding the referenced node', + NODE_REFERENCE_TEMPLATE: + 'Make sure the node you referenced is spelled correctly and is a parent of this node', + NO_EXECUTION_DATA: 'No execution data available', +} as const; + +export const EXPRESSION_ERROR_TYPES = { + PAIRED_ITEM_NO_CONNECTION: 'paired_item_no_connection', +} as const; + +export const EXPRESSION_DESCRIPTION_KEYS = { + NODE_NOT_FOUND: 'nodeNotFound', + NO_NODE_EXECUTION_DATA: 'noNodeExecutionData', + PAIRED_ITEM_NO_CONNECTION: 'pairedItemNoConnection', + PAIRED_ITEM_NO_CONNECTION_CODE_NODE: 'pairedItemNoConnectionCodeNode', +} as const; + export class ExpressionError extends ExecutionBaseError { constructor(message: string, options?: ExpressionErrorOptions) { super(message, { cause: options?.cause, level: 'warning' }); diff --git a/packages/workflow/src/workflow-data-proxy.ts b/packages/workflow/src/workflow-data-proxy.ts index 2787281932..554dace0d6 100644 --- a/packages/workflow/src/workflow-data-proxy.ts +++ b/packages/workflow/src/workflow-data-proxy.ts @@ -8,7 +8,13 @@ import { DateTime, Duration, Interval, Settings } from 'luxon'; import { augmentArray, augmentObject } from './augment-object'; import { AGENT_LANGCHAIN_NODE_TYPE, SCRIPTING_NODE_TYPES } from './constants'; import { ApplicationError } from './errors/application.error'; -import { ExpressionError, type ExpressionErrorOptions } from './errors/expression.error'; +import { + ExpressionError, + type ExpressionErrorOptions, + EXPRESSION_ERROR_MESSAGES, + EXPRESSION_ERROR_TYPES, + EXPRESSION_DESCRIPTION_KEYS, +} from './errors/expression.error'; import { getGlobalState } from './global-state'; import { NodeConnectionTypes } from './interfaces'; import type { @@ -390,11 +396,13 @@ export class WorkflowDataProxy { } if (!that.workflow.getNode(nodeName)) { - throw new ExpressionError("Referenced node doesn't exist", { + throw new ExpressionError(EXPRESSION_ERROR_MESSAGES.NODE_NOT_FOUND, { + messageTemplate: EXPRESSION_ERROR_MESSAGES.NODE_REFERENCE_TEMPLATE, runIndex: that.runIndex, itemIndex: that.itemIndex, nodeCause: nodeName, - descriptionKey: 'nodeNotFound', + descriptionKey: EXPRESSION_DESCRIPTION_KEYS.NODE_NOT_FOUND, + type: EXPRESSION_ERROR_TYPES.PAIRED_ITEM_NO_CONNECTION, }); } @@ -402,11 +410,12 @@ export class WorkflowDataProxy { !that.runExecutionData.resultData.runData.hasOwnProperty(nodeName) && !getPinDataIfManualExecution(that.workflow, nodeName, that.mode) ) { - throw new ExpressionError('Referenced node is unexecuted', { + throw new ExpressionError(EXPRESSION_ERROR_MESSAGES.NODE_NOT_FOUND, { + messageTemplate: EXPRESSION_ERROR_MESSAGES.NODE_REFERENCE_TEMPLATE, runIndex: that.runIndex, itemIndex: that.itemIndex, - type: 'no_node_execution_data', - descriptionKey: 'noNodeExecutionData', + type: EXPRESSION_ERROR_TYPES.PAIRED_ITEM_NO_CONNECTION, + descriptionKey: EXPRESSION_DESCRIPTION_KEYS.NO_NODE_EXECUTION_DATA, nodeCause: nodeName, }); } @@ -496,11 +505,16 @@ export class WorkflowDataProxy { name = name.toString(); if (!node) { - throw new ExpressionError("Referenced node doesn't exist", { + throw new ExpressionError(EXPRESSION_ERROR_MESSAGES.NODE_NOT_FOUND, { + messageTemplate: EXPRESSION_ERROR_MESSAGES.NODE_REFERENCE_TEMPLATE, + functionality: 'pairedItem', + descriptionKey: isScriptingNode(nodeName, that.workflow) + ? EXPRESSION_DESCRIPTION_KEYS.PAIRED_ITEM_NO_CONNECTION_CODE_NODE + : EXPRESSION_DESCRIPTION_KEYS.PAIRED_ITEM_NO_CONNECTION, + type: EXPRESSION_ERROR_TYPES.PAIRED_ITEM_NO_CONNECTION, + nodeCause: nodeName, runIndex: that.runIndex, itemIndex: that.itemIndex, - nodeCause: nodeName, - descriptionKey: 'nodeNotFound', }); } @@ -516,7 +530,7 @@ export class WorkflowDataProxy { if (executionData.length === 0) { if (that.workflow.getParentNodes(nodeName).length === 0) { - throw new ExpressionError('No execution data available', { + throw new ExpressionError(EXPRESSION_ERROR_MESSAGES.NO_EXECUTION_DATA, { messageTemplate: 'No execution data available to expression under ‘%%PARAMETER%%’', descriptionKey: 'noInputConnection', @@ -527,7 +541,7 @@ export class WorkflowDataProxy { }); } - throw new ExpressionError('No execution data available', { + throw new ExpressionError(EXPRESSION_ERROR_MESSAGES.NO_EXECUTION_DATA, { runIndex: that.runIndex, itemIndex: that.itemIndex, type: 'no_execution_data', @@ -693,11 +707,16 @@ export class WorkflowDataProxy { const nodeName = name.toString(); if (that.workflow.getNode(nodeName) === null) { - throw new ExpressionError("Referenced node doesn't exist", { + throw new ExpressionError(EXPRESSION_ERROR_MESSAGES.NODE_NOT_FOUND, { + messageTemplate: EXPRESSION_ERROR_MESSAGES.NODE_REFERENCE_TEMPLATE, + functionality: 'pairedItem', + descriptionKey: isScriptingNode(nodeName, that.workflow) + ? EXPRESSION_DESCRIPTION_KEYS.PAIRED_ITEM_NO_CONNECTION_CODE_NODE + : EXPRESSION_DESCRIPTION_KEYS.PAIRED_ITEM_NO_CONNECTION, + type: EXPRESSION_ERROR_TYPES.PAIRED_ITEM_NO_CONNECTION, + nodeCause: nodeName, runIndex: that.runIndex, itemIndex: that.itemIndex, - nodeCause: nodeName, - descriptionKey: 'nodeNotFound', }); } @@ -814,14 +833,14 @@ export class WorkflowDataProxy { }); }; - const createNoConnectionError = (nodeCause: string) => { - return createExpressionError('Invalid expression', { - messageTemplate: 'No path back to referenced node', + const createNodeReferenceError = (nodeCause: string) => { + return createExpressionError(EXPRESSION_ERROR_MESSAGES.NODE_NOT_FOUND, { + messageTemplate: EXPRESSION_ERROR_MESSAGES.NODE_REFERENCE_TEMPLATE, functionality: 'pairedItem', descriptionKey: isScriptingNode(nodeCause, that.workflow) - ? 'pairedItemNoConnectionCodeNode' - : 'pairedItemNoConnection', - type: 'paired_item_no_connection', + ? EXPRESSION_DESCRIPTION_KEYS.PAIRED_ITEM_NO_CONNECTION_CODE_NODE + : EXPRESSION_DESCRIPTION_KEYS.PAIRED_ITEM_NO_CONNECTION, + type: EXPRESSION_ERROR_TYPES.PAIRED_ITEM_NO_CONNECTION, moreInfoLink: true, nodeCause, }); @@ -990,7 +1009,7 @@ export class WorkflowDataProxy { const matchedItems = results.filter((result) => result.ok).map((result) => result.result); if (matchedItems.length === 0) { - if (sourceArray.length === 0) throw createNoConnectionError(destinationNodeName); + if (sourceArray.length === 0) throw createNodeReferenceError(destinationNodeName); throw createBranchNotFoundError(sourceData.previousNode, pairedItem.item, nodeBeforeLast); } @@ -1031,7 +1050,7 @@ export class WorkflowDataProxy { inputData?.[NodeConnectionTypes.AiTool]?.[0]?.[itemIndex].json; if (!placeholdersDataInputData) { - throw new ExpressionError('No execution data available', { + throw new ExpressionError(EXPRESSION_ERROR_MESSAGES.NO_EXECUTION_DATA, { runIndex, itemIndex, type: 'no_execution_data', @@ -1053,12 +1072,7 @@ export class WorkflowDataProxy { const referencedNode = that.workflow.getNode(nodeName); if (referencedNode === null) { - throw createExpressionError("Referenced node doesn't exist", { - runIndex: that.runIndex, - itemIndex: that.itemIndex, - nodeCause: nodeName, - descriptionKey: 'nodeNotFound', - }); + throw createNodeReferenceError(nodeName); } const ensureNodeExecutionData = () => { @@ -1066,13 +1080,26 @@ export class WorkflowDataProxy { !that?.runExecutionData?.resultData?.runData.hasOwnProperty(nodeName) && !getPinDataIfManualExecution(that.workflow, nodeName, that.mode) ) { - throw createExpressionError('Referenced node is unexecuted', { - runIndex: that.runIndex, - itemIndex: that.itemIndex, - type: 'no_node_execution_data', - descriptionKey: 'noNodeExecutionData', - nodeCause: nodeName, - }); + throw createNodeReferenceError(nodeName); + } + }; + + const ensureValidPath = () => { + // Check path before execution data + const referencedNode = that.workflow.getNode(nodeName); + if (!referencedNode) { + throw createNodeReferenceError(nodeName); + } + + const activeNode = that.workflow.getNode(that.activeNodeName); + let contextNode = that.contextNodeName; + if (activeNode) { + const parentMainInputNode = that.workflow.getParentMainInputNode(activeNode); + contextNode = parentMainInputNode.name ?? contextNode; + } + + if (!that.workflow.hasPath(nodeName, contextNode)) { + throw createNodeReferenceError(nodeName); } }; @@ -1108,7 +1135,13 @@ export class WorkflowDataProxy { property === PAIRED_ITEM_METHOD.ITEM ) { // Before resolving the pairedItem make sure that the requested node comes in the - // graph before the current one + // graph before the current one or exists in the workflow + const referencedNode = that.workflow.getNode(nodeName); + if (!referencedNode) { + // Node doesn't exist in the workflow (could be trimmed manual execution) + throw createNodeReferenceError(nodeName); + } + const activeNode = that.workflow.getNode(that.activeNodeName); let contextNode = that.contextNodeName; @@ -1116,9 +1149,10 @@ export class WorkflowDataProxy { const parentMainInputNode = that.workflow.getParentMainInputNode(activeNode); contextNode = parentMainInputNode.name ?? contextNode; } - const parentNodes = that.workflow.getParentNodes(contextNode); - if (!parentNodes.includes(nodeName)) { - throw createNoConnectionError(nodeName); + + // Use bidirectional path checking to handle AI/tool nodes properly + if (!that.workflow.hasPath(nodeName, contextNode)) { + throw createNodeReferenceError(nodeName); } ensureNodeExecutionData(); @@ -1199,6 +1233,7 @@ export class WorkflowDataProxy { } if (property === 'first') { + ensureValidPath(); ensureNodeExecutionData(); return (branchIndex?: number, runIndex?: number) => { branchIndex = @@ -1217,6 +1252,7 @@ export class WorkflowDataProxy { }; } if (property === 'last') { + ensureValidPath(); ensureNodeExecutionData(); return (branchIndex?: number, runIndex?: number) => { branchIndex = @@ -1238,6 +1274,7 @@ export class WorkflowDataProxy { }; } if (property === 'all') { + ensureValidPath(); ensureNodeExecutionData(); return (branchIndex?: number, runIndex?: number) => { branchIndex = @@ -1276,7 +1313,7 @@ export class WorkflowDataProxy { if (property === 'isProxy') return true; if (that.connectionInputData.length === 0) { - throw createExpressionError('No execution data available', { + throw createExpressionError(EXPRESSION_ERROR_MESSAGES.NO_EXECUTION_DATA, { runIndex: that.runIndex, itemIndex: that.itemIndex, type: 'no_execution_data', diff --git a/packages/workflow/src/workflow.ts b/packages/workflow/src/workflow.ts index 906c0d0997..a7e625e3cf 100644 --- a/packages/workflow/src/workflow.ts +++ b/packages/workflow/src/workflow.ts @@ -1004,4 +1004,62 @@ export class Workflow { return result; } + + /** + * Checks if there's a bidirectional path between two nodes. + * This handles AI/tool nodes that have complex connection patterns + * where simple parent-child traversal doesn't work. + * + * @param fromNodeName The starting node name + * @param toNodeName The target node name + * @param maxDepth Maximum depth to search (default: 50) + * @returns true if there's a path between the nodes + */ + hasPath(fromNodeName: string, toNodeName: string, maxDepth = 50): boolean { + if (fromNodeName === toNodeName) return true; + + const visited = new Set(); + const queue: Array<{ nodeName: string; depth: number }> = [ + { nodeName: fromNodeName, depth: 0 }, + ]; + + while (queue.length > 0) { + const { nodeName, depth } = queue.shift()!; + + if (depth > maxDepth) continue; + if (visited.has(nodeName)) continue; + if (nodeName === toNodeName) return true; + + visited.add(nodeName); + + // Check all connection types for this node + const allConnectionTypes = [ + NodeConnectionTypes.Main, + NodeConnectionTypes.AiTool, + NodeConnectionTypes.AiMemory, + NodeConnectionTypes.AiDocument, + NodeConnectionTypes.AiVectorStore, + ]; + + for (const connectionType of allConnectionTypes) { + // Get children (forward direction) + const children = this.getChildNodes(nodeName, connectionType); + for (const childName of children) { + if (!visited.has(childName)) { + queue.push({ nodeName: childName, depth: depth + 1 }); + } + } + + // Get parents (backward direction) + const parents = this.getParentNodes(nodeName, connectionType); + for (const parentName of parents) { + if (!visited.has(parentName)) { + queue.push({ nodeName: parentName, depth: depth + 1 }); + } + } + } + } + + return false; + } } diff --git a/packages/workflow/test/paired-item-path-detection.test.ts b/packages/workflow/test/paired-item-path-detection.test.ts new file mode 100644 index 0000000000..26ff54da43 --- /dev/null +++ b/packages/workflow/test/paired-item-path-detection.test.ts @@ -0,0 +1,757 @@ +import { NodeTypes } from './helpers'; +import { ExpressionError } from '../src/errors/expression.error'; +import type { IExecuteData, INode, IWorkflowBase, IRun, IConnections } from '../src/interfaces'; +import { NodeConnectionTypes } from '../src/interfaces'; +import { Workflow } from '../src/workflow'; +import { WorkflowDataProxy } from '../src/workflow-data-proxy'; + +describe('Paired Item Path Detection', () => { + /** + * Helper to create a minimal workflow for testing + */ + const createWorkflow = (nodes: INode[], connections: IConnections = {}): IWorkflowBase => ({ + id: '1', + name: 'test-workflow', + nodes, + connections, + active: false, + settings: {}, + isArchived: false, + updatedAt: new Date(), + createdAt: new Date(), + }); + + /** + * Helper to create a WorkflowDataProxy for testing + */ + const createProxy = ( + workflow: IWorkflowBase, + activeNodeName: string, + run?: IRun | null, + executeData?: IExecuteData, + ) => { + const wf = new Workflow({ + id: workflow.id, + name: workflow.name, + nodes: workflow.nodes, + connections: workflow.connections, + active: workflow.active, + nodeTypes: NodeTypes(), + settings: workflow.settings, + }); + + return new WorkflowDataProxy( + wf, + run?.data ?? null, + 0, // runIndex + 0, // itemIndex + activeNodeName, + [], // connectionInputData + {}, // siblingParameters + 'manual', // mode + {}, // additionalKeys + executeData, + ).getDataProxy(); + }; + + describe('AI/Tool Node Scenarios', () => { + test('should detect path in bidirectional AI/tool node setup', () => { + // Scenario: Code1 -> Vector Store <- Default Data Loader + const nodes: INode[] = [ + { + id: '1', + name: 'Code1', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [100, 100], + parameters: {}, + }, + { + id: '2', + name: 'Vector Store', + type: 'n8n-nodes-langchain.vectorStore', + typeVersion: 1, + position: [300, 100], + parameters: {}, + }, + { + id: '3', + name: 'Default Data Loader', + type: 'n8n-nodes-langchain.documentDefaultDataLoader', + typeVersion: 1, + position: [100, 200], + parameters: {}, + }, + { + id: '4', + name: 'Code2', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [500, 100], + parameters: { + jsCode: '// Reference Code1 using $()\nreturn $("Code1").all();', + }, + }, + ]; + + const connections = { + Code1: { + [NodeConnectionTypes.Main]: [ + [{ node: 'Vector Store', type: NodeConnectionTypes.AiVectorStore, index: 0 }], + ], + }, + 'Default Data Loader': { + [NodeConnectionTypes.Main]: [ + [{ node: 'Vector Store', type: NodeConnectionTypes.AiDocument, index: 0 }], + ], + }, + 'Vector Store': { + [NodeConnectionTypes.Main]: [ + [{ node: 'Code2', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + }; + + const workflow = createWorkflow(nodes, connections); + const wf = new Workflow({ + id: workflow.id, + name: workflow.name, + nodes: workflow.nodes, + connections: workflow.connections, + active: workflow.active, + nodeTypes: NodeTypes(), + settings: workflow.settings, + }); + + // Test bidirectional path detection + expect(wf.hasPath('Code1', 'Code2')).toBe(true); + expect(wf.hasPath('Default Data Loader', 'Code2')).toBe(true); + expect(wf.hasPath('Code1', 'Default Data Loader')).toBe(true); // Via Vector Store + + // Test that unconnected nodes return false + const unconnectedNode: INode = { + id: '5', + name: 'Unconnected', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [700, 100], + parameters: {}, + }; + const workflowWithUnconnected = createWorkflow([...nodes, unconnectedNode], connections); + const wfWithUnconnected = new Workflow({ + id: workflowWithUnconnected.id, + name: workflowWithUnconnected.name, + nodes: workflowWithUnconnected.nodes, + connections: workflowWithUnconnected.connections, + active: workflowWithUnconnected.active, + nodeTypes: NodeTypes(), + settings: workflowWithUnconnected.settings, + }); + + expect(wfWithUnconnected.hasPath('Code1', 'Unconnected')).toBe(false); + }); + + test('should handle complex AI tool connection patterns', () => { + // More complex AI scenario with multiple connection types + const nodes: INode[] = [ + { + id: '1', + name: 'Agent', + type: 'n8n-nodes-langchain.agent', + typeVersion: 1, + position: [300, 300], + parameters: {}, + }, + { + id: '2', + name: 'Tool1', + type: 'n8n-nodes-langchain.toolHttpRequest', + typeVersion: 1, + position: [100, 200], + parameters: {}, + }, + { + id: '3', + name: 'Tool2', + type: 'n8n-nodes-langchain.toolCalculator', + typeVersion: 1, + position: [100, 400], + parameters: {}, + }, + { + id: '4', + name: 'Memory', + type: 'n8n-nodes-langchain.memoryBufferMemory', + typeVersion: 1, + position: [200, 100], + parameters: {}, + }, + ]; + + const connections = { + Tool1: { + [NodeConnectionTypes.AiTool]: [ + [{ node: 'Agent', type: NodeConnectionTypes.AiTool, index: 0 }], + ], + }, + Tool2: { + [NodeConnectionTypes.AiTool]: [ + [{ node: 'Agent', type: NodeConnectionTypes.AiTool, index: 1 }], + ], + }, + Memory: { + [NodeConnectionTypes.AiMemory]: [ + [{ node: 'Agent', type: NodeConnectionTypes.AiMemory, index: 0 }], + ], + }, + }; + + const workflow = createWorkflow(nodes, connections); + const wf = new Workflow({ + id: workflow.id, + name: workflow.name, + nodes: workflow.nodes, + connections: workflow.connections, + active: workflow.active, + nodeTypes: NodeTypes(), + settings: workflow.settings, + }); + + // Test all tools can reach the agent + expect(wf.hasPath('Tool1', 'Agent')).toBe(true); + expect(wf.hasPath('Tool2', 'Agent')).toBe(true); + expect(wf.hasPath('Memory', 'Agent')).toBe(true); + + // Test bidirectional paths + expect(wf.hasPath('Agent', 'Tool1')).toBe(true); + expect(wf.hasPath('Agent', 'Tool2')).toBe(true); + expect(wf.hasPath('Agent', 'Memory')).toBe(true); + + // Test indirect connections + expect(wf.hasPath('Tool1', 'Tool2')).toBe(true); // Via Agent + expect(wf.hasPath('Memory', 'Tool1')).toBe(true); // Via Agent + }); + }); + + describe('Manual Execution Node-Not-Found Scenarios', () => { + test('should throw "No path back to referenced node" when node does not exist in trimmed workflow', () => { + // Simulate manual execution scenario where node D is not in the trimmed workflow + const nodes: INode[] = [ + { + id: '1', + name: 'A', + type: 'n8n-nodes-base.start', + typeVersion: 1, + position: [100, 100], + parameters: {}, + }, + { + id: '2', + name: 'B', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [300, 100], + parameters: { + jsCode: 'return $("D").all(); // Reference missing node D', + }, + }, + { + id: '3', + name: 'C', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [500, 100], + parameters: {}, + }, + ]; + + const connections = { + A: { + [NodeConnectionTypes.Main]: [[{ node: 'B', type: NodeConnectionTypes.Main, index: 0 }]], + }, + B: { + [NodeConnectionTypes.Main]: [[{ node: 'C', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }; + + const workflow = createWorkflow(nodes, connections); + const proxy = createProxy(workflow, 'B'); + + // Should throw error when trying to access non-existent node D + expect(() => proxy.$('D')).toThrowError(ExpressionError); + expect(() => proxy.$('D')).toThrow(/Error finding the referenced node/); + }); + + test('should throw "No path back to referenced node" when node exists but has no path', () => { + // Node D exists but is not connected + const nodes: INode[] = [ + { + id: '1', + name: 'A', + type: 'n8n-nodes-base.start', + typeVersion: 1, + position: [100, 100], + parameters: {}, + }, + { + id: '2', + name: 'B', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [300, 100], + parameters: { + jsCode: 'return $("D").all(); // Reference unconnected node D', + }, + }, + { + id: '3', + name: 'C', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [500, 100], + parameters: {}, + }, + { + id: '4', + name: 'D', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [100, 300], + parameters: {}, + }, + ]; + + const connections = { + A: { + [NodeConnectionTypes.Main]: [[{ node: 'B', type: NodeConnectionTypes.Main, index: 0 }]], + }, + B: { + [NodeConnectionTypes.Main]: [[{ node: 'C', type: NodeConnectionTypes.Main, index: 0 }]], + }, + // D is not connected + }; + + const workflow = createWorkflow(nodes, connections); + + // Create executeData to simulate a real execution context + const executeData: IExecuteData = { + data: { + main: [[]], + }, + node: nodes.find((n) => n.name === 'B')!, + source: { + main: [ + { + previousNode: 'A', + previousNodeOutput: 0, + previousNodeRun: 0, + }, + ], + }, + }; + + const proxy = createProxy(workflow, 'B', null, executeData); + + // Should throw error when trying to access paired item from unconnected node D + let error: ExpressionError | undefined; + try { + proxy.$('D').item; + } catch (e) { + error = e as ExpressionError; + } + + expect(error).toBeDefined(); + expect(error).toBeInstanceOf(ExpressionError); + expect(error!.context.type).toBe('paired_item_no_connection'); + expect(error!.context.descriptionKey).toBe('pairedItemNoConnectionCodeNode'); + }); + }); + + describe('Workflow.hasPath method', () => { + test('should handle self-reference', () => { + const nodes: INode[] = [ + { + id: '1', + name: 'A', + type: 'n8n-nodes-base.start', + typeVersion: 1, + position: [100, 100], + parameters: {}, + }, + ]; + + const workflow = createWorkflow(nodes, {}); + const wf = new Workflow({ + id: workflow.id, + name: workflow.name, + nodes: workflow.nodes, + connections: workflow.connections, + active: workflow.active, + nodeTypes: NodeTypes(), + settings: workflow.settings, + }); + + expect(wf.hasPath('A', 'A')).toBe(true); + }); + + test('should respect maximum depth limit', () => { + const nodes: INode[] = [ + { + id: '1', + name: 'A', + type: 'n8n-nodes-base.start', + typeVersion: 1, + position: [100, 100], + parameters: {}, + }, + { + id: '2', + name: 'B', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [300, 100], + parameters: {}, + }, + ]; + + const connections = { + A: { + [NodeConnectionTypes.Main]: [[{ node: 'B', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }; + + const workflow = createWorkflow(nodes, connections); + const wf = new Workflow({ + id: workflow.id, + name: workflow.name, + nodes: workflow.nodes, + connections: workflow.connections, + active: workflow.active, + nodeTypes: NodeTypes(), + settings: workflow.settings, + }); + + // Should find path with sufficient depth + expect(wf.hasPath('A', 'B', 10)).toBe(true); + + // Should not find path with insufficient depth + expect(wf.hasPath('A', 'B', 0)).toBe(false); + }); + + test('should handle cycles without infinite loops', () => { + const nodes: INode[] = [ + { + id: '1', + name: 'A', + type: 'n8n-nodes-base.start', + typeVersion: 1, + position: [100, 100], + parameters: {}, + }, + { + id: '2', + name: 'B', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [300, 100], + parameters: {}, + }, + { + id: '3', + name: 'C', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [500, 100], + parameters: {}, + }, + ]; + + // Create a cycle: A -> B -> C -> A + const connections = { + A: { + [NodeConnectionTypes.Main]: [[{ node: 'B', type: NodeConnectionTypes.Main, index: 0 }]], + }, + B: { + [NodeConnectionTypes.Main]: [[{ node: 'C', type: NodeConnectionTypes.Main, index: 0 }]], + }, + C: { + [NodeConnectionTypes.Main]: [[{ node: 'A', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }; + + const workflow = createWorkflow(nodes, connections); + const wf = new Workflow({ + id: workflow.id, + name: workflow.name, + nodes: workflow.nodes, + connections: workflow.connections, + active: workflow.active, + nodeTypes: NodeTypes(), + settings: workflow.settings, + }); + + // Should handle cycles correctly + expect(wf.hasPath('A', 'C')).toBe(true); + expect(wf.hasPath('B', 'A')).toBe(true); + expect(wf.hasPath('C', 'B')).toBe(true); + }); + }); + + describe('Actual workflow', () => { + test('should show correct error message for disconnected nodes', () => { + // Recreate the exact scenario from the user's workflow + const nodes: INode[] = [ + { + id: 'afc0fc26-d521-4464-9f90-3327559bd4a6', + name: 'On form submission', + type: 'n8n-nodes-base.formTrigger', + typeVersion: 2.2, + position: [0, 0], + parameters: { + formTitle: 'Submit BBS application', + }, + }, + { + id: 'c5861385-d513-4d74-8fe3-e5acbe08a90a', + name: 'Code', + type: 'n8n-nodes-base.code', + typeVersion: 2, + position: [288, 432], + parameters: { + jsCode: "\nreturn $('On form submission').all();", + }, + }, + { + id: '523b019b-e456-4784-a50a-18558c858c3b', + name: "When clicking 'Test workflow'", + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: [0, 288], + parameters: {}, + }, + { + id: '3057aebb-d87a-4142-8354-f298e41ab919', + name: 'Edit Fields', + type: 'n8n-nodes-base.set', + typeVersion: 3.4, + position: [288, 128], + parameters: { + assignments: { + assignments: [ + { + id: '9c260756-a7ce-41ba-ad9b-0eb1ceeaf02b', + name: 'test', + value: "={{ $('On form submission').item.json }}", + type: 'string', + }, + ], + }, + }, + }, + ]; + + const connections = { + 'On form submission': { + [NodeConnectionTypes.Main]: [[]], + }, + "When clicking 'Test workflow'": { + [NodeConnectionTypes.Main]: [ + [ + { node: 'Code', type: NodeConnectionTypes.Main, index: 0 }, + { node: 'Edit Fields', type: NodeConnectionTypes.Main, index: 0 }, + ], + ], + }, + }; + + const workflow = createWorkflow(nodes, connections); + const proxy = createProxy(workflow, 'Code'); + + // Should throw the correct error when trying to access disconnected node + let error: ExpressionError | undefined; + try { + proxy.$('On form submission').all(); + } catch (e) { + error = e as ExpressionError; + } + + expect(error).toBeDefined(); + expect(error).toBeInstanceOf(ExpressionError); + expect(error!.context.type).toBe('paired_item_no_connection'); + expect(error!.context.descriptionKey).toBe('pairedItemNoConnection'); + expect(error!.message).toBe('Error finding the referenced node'); + expect(error!.context.messageTemplate).toBe( + 'Make sure the node you referenced is spelled correctly and is a parent of this node', + ); + }); + + test('should also show correct error for Edit Fields node', () => { + // Test the Edit Fields node as well + const nodes: INode[] = [ + { + id: 'afc0fc26-d521-4464-9f90-3327559bd4a6', + name: 'On form submission', + type: 'n8n-nodes-base.formTrigger', + typeVersion: 2.2, + position: [0, 0], + parameters: { + formTitle: 'Submit BBS application', + }, + }, + { + id: 'c5861385-d513-4d74-8fe3-e5acbe08a90a', + name: 'Code', + type: 'n8n-nodes-base.code', + typeVersion: 2, + position: [288, 432], + parameters: { + jsCode: "\nreturn $('On form submission').all();", + }, + }, + { + id: '523b019b-e456-4784-a50a-18558c858c3b', + name: "When clicking 'Test workflow'", + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: [0, 288], + parameters: {}, + }, + { + id: '3057aebb-d87a-4142-8354-f298e41ab919', + name: 'Edit Fields', + type: 'n8n-nodes-base.set', + typeVersion: 3.4, + position: [288, 128], + parameters: { + assignments: { + assignments: [ + { + id: '9c260756-a7ce-41ba-ad9b-0eb1ceeaf02b', + name: 'test', + value: "={{ $('On form submission').item.json }}", + type: 'string', + }, + ], + }, + }, + }, + ]; + + const connections = { + 'On form submission': { + [NodeConnectionTypes.Main]: [[]], + }, + "When clicking 'Test workflow'": { + [NodeConnectionTypes.Main]: [ + [ + { node: 'Code', type: NodeConnectionTypes.Main, index: 0 }, + { node: 'Edit Fields', type: NodeConnectionTypes.Main, index: 0 }, + ], + ], + }, + }; + + const workflow = createWorkflow(nodes, connections); + const proxy = createProxy(workflow, 'Edit Fields'); + + // Should throw the correct error when trying to access disconnected node + let error: ExpressionError | undefined; + try { + proxy.$('On form submission').item; + } catch (e) { + error = e as ExpressionError; + } + + expect(error).toBeDefined(); + expect(error).toBeInstanceOf(ExpressionError); + expect(error!.context.type).toBe('paired_item_no_connection'); + expect(error!.context.descriptionKey).toBe('pairedItemNoConnection'); + expect(error!.message).toBe('Error finding the referenced node'); + expect(error!.context.messageTemplate).toBe( + 'Make sure the node you referenced is spelled correctly and is a parent of this node', + ); + }); + + test('should show correct error in runtime execution context', () => { + // Test with execution data to simulate real runtime + const nodes: INode[] = [ + { + id: 'afc0fc26-d521-4464-9f90-3327559bd4a6', + name: 'On form submission', + type: 'n8n-nodes-base.formTrigger', + typeVersion: 2.2, + position: [0, 0], + parameters: { + formTitle: 'Submit BBS application', + }, + }, + { + id: 'c5861385-d513-4d74-8fe3-e5acbe08a90a', + name: 'Code', + type: 'n8n-nodes-base.code', + typeVersion: 2, + position: [288, 432], + parameters: { + jsCode: "\nreturn $('On form submission').all();", + }, + }, + { + id: '523b019b-e456-4784-a50a-18558c858c3b', + name: "When clicking 'Test workflow'", + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: [0, 288], + parameters: {}, + }, + ]; + + const connections = { + 'On form submission': { + [NodeConnectionTypes.Main]: [[]], + }, + "When clicking 'Test workflow'": { + [NodeConnectionTypes.Main]: [ + [{ node: 'Code', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + }; + + const workflow = createWorkflow(nodes, connections); + + // Create execution data to simulate real workflow execution + const executeData: IExecuteData = { + data: { + main: [[]], + }, + node: nodes.find((n) => n.name === 'Code')!, + source: { + main: [ + { + previousNode: "When clicking 'Test workflow'", + previousNodeOutput: 0, + previousNodeRun: 0, + }, + ], + }, + }; + + const proxy = createProxy(workflow, 'Code', null, executeData); + + // Should throw the correct error when trying to access disconnected node during execution + let error: ExpressionError | undefined; + try { + proxy.$('On form submission').all(); + } catch (e) { + error = e as ExpressionError; + } + + expect(error).toBeDefined(); + expect(error).toBeInstanceOf(ExpressionError); + expect(error!.context.type).toBe('paired_item_no_connection'); + expect(error!.message).toBe('Error finding the referenced node'); + expect(error!.context.messageTemplate).toBe( + 'Make sure the node you referenced is spelled correctly and is a parent of this node', + ); + }); + }); +}); diff --git a/packages/workflow/test/workflow-data-proxy.test.ts b/packages/workflow/test/workflow-data-proxy.test.ts index c436565309..7cbcf3a8e0 100644 --- a/packages/workflow/test/workflow-data-proxy.test.ts +++ b/packages/workflow/test/workflow-data-proxy.test.ts @@ -235,7 +235,7 @@ describe('WorkflowDataProxy', () => { } catch (error) { expect(error).toBeInstanceOf(ExpressionError); const exprError = error as ExpressionError; - expect(exprError.message).toEqual("Referenced node doesn't exist"); + expect(exprError.message).toEqual('Error finding the referenced node'); } }); @@ -246,7 +246,7 @@ describe('WorkflowDataProxy', () => { } catch (error) { expect(error).toBeInstanceOf(ExpressionError); const exprError = error as ExpressionError; - expect(exprError.message).toEqual('Invalid expression'); + expect(exprError.message).toEqual('Error finding the referenced node'); expect(exprError.context.type).toEqual('paired_item_no_connection'); } }); @@ -262,8 +262,8 @@ describe('WorkflowDataProxy', () => { } catch (error) { expect(error).toBeInstanceOf(ExpressionError); const exprError = error as ExpressionError; - expect(exprError.message).toEqual('Referenced node is unexecuted'); - expect(exprError.context.type).toEqual('no_node_execution_data'); + expect(exprError.message).toEqual('Error finding the referenced node'); + expect(exprError.context.type).toEqual('paired_item_no_connection'); } }); @@ -286,8 +286,8 @@ describe('WorkflowDataProxy', () => { } catch (error) { expect(error).toBeInstanceOf(ExpressionError); const exprError = error as ExpressionError; - expect(exprError.message).toEqual('Referenced node is unexecuted'); - expect(exprError.context.type).toEqual('no_node_execution_data'); + expect(exprError.message).toEqual('Error finding the referenced node'); + expect(exprError.context.type).toEqual('paired_item_no_connection'); } }); diff --git a/packages/workflow/test/workflow.test.ts b/packages/workflow/test/workflow.test.ts index 0c37c3a02f..50de376e37 100644 --- a/packages/workflow/test/workflow.test.ts +++ b/packages/workflow/test/workflow.test.ts @@ -2890,4 +2890,346 @@ describe('Workflow', () => { expect(result).toEqual([]); }); }); + + describe('hasPath method', () => { + test('should return true for self-reference', () => { + const workflow = new Workflow({ + id: 'test', + nodes: [ + { + id: 'Node1', + name: 'Node1', + type: 'test.set', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + active: false, + nodeTypes, + }); + + expect(workflow.hasPath('Node1', 'Node1')).toBe(true); + }); + + test('should return false when nodes are not connected', () => { + const workflow = new Workflow({ + id: 'test', + nodes: [ + { + id: 'Node1', + name: 'Node1', + type: 'test.set', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + { + id: 'Node2', + name: 'Node2', + type: 'test.set', + typeVersion: 1, + position: [100, 0], + parameters: {}, + }, + ], + connections: {}, + active: false, + nodeTypes, + }); + + expect(workflow.hasPath('Node1', 'Node2')).toBe(false); + }); + + test('should return true for directly connected nodes', () => { + const workflow = new Workflow({ + id: 'test', + nodes: [ + { + id: 'Node1', + name: 'Node1', + type: 'test.set', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + { + id: 'Node2', + name: 'Node2', + type: 'test.set', + typeVersion: 1, + position: [100, 0], + parameters: {}, + }, + ], + connections: { + Node1: { + [NodeConnectionTypes.Main]: [ + [{ node: 'Node2', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + }, + active: false, + nodeTypes, + }); + + expect(workflow.hasPath('Node1', 'Node2')).toBe(true); + expect(workflow.hasPath('Node2', 'Node1')).toBe(true); + }); + + test('should respect maximum depth limit', () => { + const workflow = new Workflow({ + id: 'test', + nodes: [ + { + id: 'Node1', + name: 'Node1', + type: 'test.set', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + { + id: 'Node2', + name: 'Node2', + type: 'test.set', + typeVersion: 1, + position: [100, 0], + parameters: {}, + }, + ], + connections: { + Node1: { + [NodeConnectionTypes.Main]: [ + [{ node: 'Node2', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + }, + active: false, + nodeTypes, + }); + + // Should find path with sufficient depth + expect(workflow.hasPath('Node1', 'Node2', 5)).toBe(true); + expect(workflow.hasPath('Node1', 'Node2', 1)).toBe(true); + + // Should not find path with insufficient depth + expect(workflow.hasPath('Node1', 'Node2', 0)).toBe(false); + }); + + test('should handle AI connection types', () => { + const workflow = new Workflow({ + id: 'test', + nodes: [ + { + id: 'Agent', + name: 'Agent', + type: 'test.ai.agent', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + { + id: 'Tool1', + name: 'Tool1', + type: 'test.ai.tool', + typeVersion: 1, + position: [100, 0], + parameters: {}, + }, + { + id: 'Memory', + name: 'Memory', + type: 'test.ai.memory', + typeVersion: 1, + position: [200, 0], + parameters: {}, + }, + ], + connections: { + Tool1: { + [NodeConnectionTypes.AiTool]: [ + [{ node: 'Agent', type: NodeConnectionTypes.AiTool, index: 0 }], + ], + }, + Memory: { + [NodeConnectionTypes.AiMemory]: [ + [{ node: 'Agent', type: NodeConnectionTypes.AiMemory, index: 0 }], + ], + }, + }, + active: false, + nodeTypes, + }); + + expect(workflow.hasPath('Tool1', 'Agent')).toBe(true); + expect(workflow.hasPath('Memory', 'Agent')).toBe(true); + expect(workflow.hasPath('Tool1', 'Memory')).toBe(true); + }); + + test('should handle complex paths with multiple connection types', () => { + const workflow = new Workflow({ + id: 'test', + nodes: [ + { + id: 'Start', + name: 'Start', + type: 'test.start', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + { + id: 'VectorStore', + name: 'VectorStore', + type: 'test.vectorstore', + typeVersion: 1, + position: [100, 0], + parameters: {}, + }, + { + id: 'Document', + name: 'Document', + type: 'test.document', + typeVersion: 1, + position: [200, 0], + parameters: {}, + }, + { + id: 'End', + name: 'End', + type: 'test.end', + typeVersion: 1, + position: [300, 0], + parameters: {}, + }, + ], + connections: { + Start: { + [NodeConnectionTypes.Main]: [ + [{ node: 'VectorStore', type: NodeConnectionTypes.AiVectorStore, index: 0 }], + ], + }, + Document: { + [NodeConnectionTypes.Main]: [ + [{ node: 'VectorStore', type: NodeConnectionTypes.AiDocument, index: 0 }], + ], + }, + VectorStore: { + [NodeConnectionTypes.Main]: [ + [{ node: 'End', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + }, + active: false, + nodeTypes, + }); + + expect(workflow.hasPath('Start', 'End')).toBe(true); + expect(workflow.hasPath('Document', 'End')).toBe(true); + expect(workflow.hasPath('Start', 'Document')).toBe(true); + }); + + test('should handle cyclic graphs without infinite loops', () => { + const workflow = new Workflow({ + id: 'test', + nodes: [ + { + id: 'Node1', + name: 'Node1', + type: 'test.set', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + { + id: 'Node2', + name: 'Node2', + type: 'test.set', + typeVersion: 1, + position: [100, 0], + parameters: {}, + }, + { + id: 'Node3', + name: 'Node3', + type: 'test.set', + typeVersion: 1, + position: [200, 0], + parameters: {}, + }, + ], + connections: { + Node1: { + [NodeConnectionTypes.Main]: [ + [{ node: 'Node2', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + Node2: { + [NodeConnectionTypes.Main]: [ + [{ node: 'Node3', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + Node3: { + [NodeConnectionTypes.Main]: [ + [{ node: 'Node1', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + }, + active: false, + nodeTypes, + }); + + expect(workflow.hasPath('Node1', 'Node3')).toBe(true); + expect(workflow.hasPath('Node2', 'Node1')).toBe(true); + expect(workflow.hasPath('Node3', 'Node2')).toBe(true); + }); + + test('should handle empty workflow', () => { + const workflow = new Workflow({ + id: 'test', + nodes: [], + connections: {}, + active: false, + nodeTypes, + }); + + expect(workflow.hasPath('NonExistent1', 'NonExistent2')).toBe(false); + }); + + test('should handle nodes with no outgoing connections', () => { + const workflow = new Workflow({ + id: 'test', + nodes: [ + { + id: 'Node1', + name: 'Node1', + type: 'test.set', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + { + id: 'Node2', + name: 'Node2', + type: 'test.set', + typeVersion: 1, + position: [100, 0], + parameters: {}, + }, + ], + connections: { + Node1: { + [NodeConnectionTypes.Main]: [[]], + }, + }, + active: false, + nodeTypes, + }); + + expect(workflow.hasPath('Node1', 'Node2')).toBe(false); + expect(workflow.hasPath('Node2', 'Node1')).toBe(false); + }); + }); });