fix(core): Run full manual execution when a trigger is executed even if run data exists (#13194)

packages/cli/src/workflows/__tests__/workflow-execution.service.test.ts

@@ -1,7 +1,8 @@
 import { mock } from 'jest-mock-extended';
-import type { INode, IWorkflowBase, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
+import type { INode, IWorkflowBase, INodeType, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
 
 import type { User } from '@/databases/entities/user';
+import type { NodeTypes } from '@/node-types';
 import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
 import type { WorkflowRunner } from '@/workflow-runner';
 import { WorkflowExecutionService } from '@/workflows/workflow-execution.service';
@@ -52,13 +53,14 @@ const hackerNewsNode: INode = {
 };
 
 describe('WorkflowExecutionService', () => {
+	const nodeTypes = mock<NodeTypes>();
 	const workflowRunner = mock<WorkflowRunner>();
 	const workflowExecutionService = new WorkflowExecutionService(
 		mock(),
 		mock(),
 		mock(),
 		mock(),
-		mock(),
+		nodeTypes,
 		mock(),
 		workflowRunner,
 		mock(),
@@ -86,7 +88,10 @@ describe('WorkflowExecutionService', () => {
 			const executionId = 'fake-execution-id';
 			const userId = 'user-id';
 			const user = mock<User>({ id: userId });
-			const runPayload = mock<WorkflowRequest.ManualRunPayload>({ startNodes: [] });
+			const runPayload = mock<WorkflowRequest.ManualRunPayload>({
+				startNodes: [],
+				destinationNode: undefined,
+			});
 
 			workflowRunner.run.mockResolvedValue(executionId);
 
@@ -108,6 +113,41 @@ describe('WorkflowExecutionService', () => {
 			expect(result).toEqual({ executionId });
 		});
 
+		test('removes runData if the destination node is a trigger', async () => {
+			const executionId = 'fake-execution-id';
+			const userId = 'user-id';
+			const user = mock<User>({ id: userId });
+			const node = mock<INode>();
+			const runPayload = mock<WorkflowRequest.ManualRunPayload>({
+				workflowData: { nodes: [node] },
+				startNodes: [],
+				destinationNode: node.name,
+			});
+
+			jest
+				.spyOn(nodeTypes, 'getByNameAndVersion')
+				.mockReturnValueOnce(mock<INodeType>({ description: { group: ['trigger'] } }));
+
+			workflowRunner.run.mockResolvedValue(executionId);
+
+			const result = await workflowExecutionService.executeManually(runPayload, user);
+
+			expect(workflowRunner.run).toHaveBeenCalledWith({
+				destinationNode: runPayload.destinationNode,
+				executionMode: 'manual',
+				runData: undefined,
+				pinData: runPayload.workflowData.pinData,
+				pushRef: undefined,
+				workflowData: runPayload.workflowData,
+				userId,
+				partialExecutionVersion: 1,
+				startNodes: runPayload.startNodes,
+				dirtyNodeNames: runPayload.dirtyNodeNames,
+				triggerToStartFrom: runPayload.triggerToStartFrom,
+			});
+			expect(result).toEqual({ executionId });
+		});
+
 		[
 			{
 				name: 'trigger',

packages/cli/src/workflows/workflow-execution.service.ts

@@ -87,6 +87,18 @@ export class WorkflowExecutionService {
 		return await this.workflowRunner.run(runData, true, undefined, undefined, responsePromise);
 	}
 
+	private isDestinationNodeATrigger(destinationNode: string, workflow: IWorkflowBase) {
+		const node = workflow.nodes.find((n) => n.name === destinationNode);
+
+		if (node === undefined) {
+			return false;
+		}
+
+		const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
+
+		return nodeType.description.group.includes('trigger');
+	}
+
 	async executeManually(
 		{
 			workflowData,
@@ -107,6 +119,23 @@ export class WorkflowExecutionService {
 			pinData,
 		);
 
+		// TODO: Reverse the order of events, first find out if the execution is
+		// partial or full, if it's partial create the execution and run, if it's
+		// full get the data first and only then create the execution.
+		//
+		// If the destination node is a trigger, then per definition this
+		// is not a partial execution and thus we can ignore the run data.
+		// If we don't do this we'll end up creating an execution, calling the
+		// partial execution flow, finding out that we don't have run data to
+		// create the execution stack and have to cancel the execution, come back
+		// here and either create the runData (e.g. scheduler trigger) or wait for
+		// a webhook or event.
+		if (destinationNode) {
+			if (this.isDestinationNodeATrigger(destinationNode, workflowData)) {
+				runData = undefined;
+			}
+		}
+
 		// if we have a trigger to start from and it's not the pinned trigger
 		// ignore the pinned trigger
 		if (pinnedTrigger && triggerToStartFrom && pinnedTrigger.name !== triggerToStartFrom.name) {
@@ -178,7 +207,7 @@ export class WorkflowExecutionService {
 				},
 				resultData: {
 					pinData,
-					runData,
+					runData: runData ?? {},
 				},
 				manualData: {
 					userId: data.userId,

packages/cli/src/workflows/workflow.request.ts

@@ -26,7 +26,7 @@ export declare namespace WorkflowRequest {
 
 	type ManualRunPayload = {
 		workflowData: IWorkflowBase;
-		runData: IRunData;
+		runData?: IRunData;
 		startNodes?: StartNodeData[];
 		destinationNode?: string;
 		dirtyNodeNames?: string[];