fix(core): Adjust starter node priority for manual executions with pinned activators (#8386)

packages/cli/src/workflows/workflowExecution.service.ts

@@ -47,7 +47,7 @@ export class WorkflowExecutionService {
 		user: User,
 		sessionId?: string,
 	) {
-		const pinnedTrigger = this.findPinnedTrigger(workflowData, startNodes, pinData);
+		const pinnedTrigger = this.selectPinnedActivatorStarter(workflowData, startNodes, pinData);
 
 		// If webhooks nodes exist and are active we have to wait for till we receive a call
 		if (
@@ -243,44 +243,64 @@ export class WorkflowExecutionService {
 	}
 
 	/**
-	 * Find the pinned trigger to execute the workflow from, if any.
+	 * Select the pinned activator node to use as starter for a manual execution.
 	 *
-	 * - In a full execution, select the _first_ pinned trigger.
+	 * In a full manual execution, select the pinned activator that was first added
-	 * - In a partial execution,
+	 * to the workflow, prioritizing `n8n-nodes-base.webhook` over other activators.
-	 *   - select the _first_ pinned trigger that leads to the executed node,
+	 *
-	 *   - else select the executed pinned trigger.
+	 * In a partial manual execution, if the executed node has parent nodes among the
+	 * pinned activators, select the pinned activator that was first added to the workflow,
+	 * prioritizing `n8n-nodes-base.webhook` over other activators. If the executed node
+	 * has no upstream nodes and is itself is a pinned activator, select it.
 	 */
-	private findPinnedTrigger(workflow: IWorkflowDb, startNodes?: string[], pinData?: IPinData) {
+	selectPinnedActivatorStarter(workflow: IWorkflowDb, startNodes?: string[], pinData?: IPinData) {
 		if (!pinData || !startNodes) return null;
 
-		const isTrigger = (nodeTypeName: string) =>
+		const allPinnedActivators = this.findAllPinnedActivators(workflow, pinData);
-			['trigger', 'webhook'].some((suffix) => nodeTypeName.toLowerCase().includes(suffix));
 
-		const pinnedTriggers = workflow.nodes.filter(
+		if (allPinnedActivators.length === 0) return null;
-			(node) => !node.disabled && pinData[node.name] && isTrigger(node.type),
-		);
 
-		if (pinnedTriggers.length === 0) return null;
+		const [firstPinnedActivator] = allPinnedActivators;
 
-		if (startNodes?.length === 0) return pinnedTriggers[0]; // full execution
+		// full manual execution
 
-		const [startNodeName] = startNodes;
+		if (startNodes?.length === 0) return firstPinnedActivator ?? null;
 
-		const parentNames = new Workflow({
+		// partial manual execution
+
+		/**
+		 * If the partial manual execution has 2+ start nodes, we search only the zeroth
+		 * start node's parents for a pinned activator. If we had 2+ start nodes without
+		 * a common ancestor and so if we end up finding multiple pinned activators, we
+		 * would still need to return one to comply with existing usage.
+		 */
+		const [firstStartNodeName] = startNodes;
+
+		const parentNodeNames = new Workflow({
 			nodes: workflow.nodes,
 			connections: workflow.connections,
 			active: workflow.active,
 			nodeTypes: this.nodeTypes,
-		}).getParentNodes(startNodeName);
+		}).getParentNodes(firstStartNodeName);
 
-		let checkNodeName = '';
+		if (parentNodeNames.length > 0) {
+			const parentNodeName = parentNodeNames.find((p) => p === firstPinnedActivator.name);
 
-		if (parentNames.length === 0) {
+			return allPinnedActivators.find((pa) => pa.name === parentNodeName) ?? null;
-			checkNodeName = startNodeName;
-		} else {
-			checkNodeName = parentNames.find((pn) => pn === pinnedTriggers[0].name) as string;
 		}
 
-		return pinnedTriggers.find((pt) => pt.name === checkNodeName) ?? null; // partial execution
+		return allPinnedActivators.find((pa) => pa.name === firstStartNodeName) ?? null;
+	}
+
+	private findAllPinnedActivators(workflow: IWorkflowDb, pinData?: IPinData) {
+		return workflow.nodes
+			.filter(
+				(node) =>
+					!node.disabled &&
+					pinData?.[node.name] &&
+					['trigger', 'webhook'].some((suffix) => node.type.toLowerCase().endsWith(suffix)) &&
+					node.type !== 'n8n-nodes-base.respondToWebhook',
+			)
+			.sort((a) => (a.type.endsWith('webhook') ? -1 : 1));
 	}
 }

packages/cli/test/unit/workflow-execution.service.test.ts

@@ -0,0 +1,137 @@
+import type { INode } from 'n8n-workflow';
+import { WorkflowExecutionService } from '@/workflows/workflowExecution.service';
+import type { IWorkflowDb } from '@/Interfaces';
+import { mock } from 'jest-mock-extended';
+
+const webhookNode: INode = {
+	name: 'Webhook',
+	type: 'n8n-nodes-base.webhook',
+	id: '111f1db0-e7be-44c5-9ce9-3e35362490f0',
+	parameters: {},
+	typeVersion: 1,
+	position: [0, 0],
+	webhookId: 'de0f8dcb-7b64-4f22-b66d-d8f74d6aefb7',
+};
+
+const secondWebhookNode = {
+	...webhookNode,
+	name: 'Webhook 2',
+	id: '222f1db0-e7be-44c5-9ce9-3e35362490f1',
+};
+
+const executeWorkflowTriggerNode: INode = {
+	name: 'Execute Workflow Trigger',
+	type: 'n8n-nodes-base.executeWorkflowTrigger',
+	id: '78d63bca-bb6c-4568-948f-8ed9aacb1fe9',
+	parameters: {},
+	typeVersion: 1,
+	position: [0, 0],
+};
+
+const respondToWebhookNode: INode = {
+	name: 'Respond to Webhook',
+	type: 'n8n-nodes-base.respondToWebhook',
+	id: '66d63bca-bb6c-4568-948f-8ed9aacb1fe9',
+	parameters: {},
+	typeVersion: 1,
+	position: [0, 0],
+};
+
+const hackerNewsNode: INode = {
+	name: 'Hacker News',
+	type: 'n8n-nodes-base.hackerNews',
+	id: '55d63bca-bb6c-4568-948f-8ed9aacb1fe9',
+	parameters: {},
+	typeVersion: 1,
+	position: [0, 0],
+};
+
+describe('WorkflowExecutionService', () => {
+	let workflowExecutionService: WorkflowExecutionService;
+
+	beforeAll(() => {
+		workflowExecutionService = new WorkflowExecutionService(
+			mock(),
+			mock(),
+			mock(),
+			mock(),
+			mock(),
+			mock(),
+		);
+	});
+
+	describe('selectPinnedActivatorStarter()', () => {
+		const workflow = mock<IWorkflowDb>({
+			nodes: [],
+		});
+
+		const pinData = {
+			[webhookNode.name]: [{ json: { key: 'value' } }],
+			[executeWorkflowTriggerNode.name]: [{ json: { key: 'value' } }],
+		};
+
+		afterEach(() => {
+			workflow.nodes = [];
+		});
+
+		it('should return `null` if no pindata', () => {
+			const node = workflowExecutionService.selectPinnedActivatorStarter(workflow, []);
+
+			expect(node).toBeNull();
+		});
+
+		it('should return `null` if no starter nodes', () => {
+			const node = workflowExecutionService.selectPinnedActivatorStarter(workflow);
+
+			expect(node).toBeNull();
+		});
+
+		it('should select webhook node if only choice', () => {
+			workflow.nodes.push(webhookNode);
+
+			const node = workflowExecutionService.selectPinnedActivatorStarter(workflow, [], pinData);
+
+			expect(node).toEqual(webhookNode);
+		});
+
+		it('should return `null` if no choice', () => {
+			workflow.nodes.push(hackerNewsNode);
+
+			const node = workflowExecutionService.selectPinnedActivatorStarter(workflow, [], pinData);
+
+			expect(node).toBeNull();
+		});
+
+		it('should return ignore Respond to Webhook', () => {
+			workflow.nodes.push(respondToWebhookNode);
+
+			const node = workflowExecutionService.selectPinnedActivatorStarter(workflow, [], pinData);
+
+			expect(node).toBeNull();
+		});
+
+		it('should select execute workflow trigger if only choice', () => {
+			workflow.nodes.push(executeWorkflowTriggerNode);
+
+			const node = workflowExecutionService.selectPinnedActivatorStarter(workflow, [], pinData);
+
+			expect(node).toEqual(executeWorkflowTriggerNode);
+		});
+
+		it('should favor webhook node over execute workflow trigger', () => {
+			workflow.nodes.push(webhookNode, executeWorkflowTriggerNode);
+
+			const node = workflowExecutionService.selectPinnedActivatorStarter(workflow, [], pinData);
+
+			expect(node).toEqual(webhookNode);
+		});
+
+		it('should favor first webhook node over second webhook node', () => {
+			workflow.nodes.push(webhookNode, secondWebhookNode);
+
+			const node = workflowExecutionService.selectPinnedActivatorStarter(workflow, [], pinData);
+
+			expect(node).toEqual(webhookNode);
+		});
+	});
+});