refactor(core): Split `WorkflowExecute.runNode` into smaller methods #17864

despairblue · 2025-07-31T13:33:17Z

Summary

Review commit by commit.

This splits WorkflowExecute.runNode. This PR should not change any behaviour.

This allows removing the eslint warning about cyclomatic complexity as well as making it clear which part of that method needs access to what bindings.
It also makes the method more readable and descriptive.

This is done to prepare the work of moving AI tool executions into the engine.

Note:
The additional tests were created by claude code, but I checked that they work via code coverage reports and mutation testing (e.g. willfully breaking the implementation to check if the test works)
The tests mock way to much and are mainly here to prove that moving the code around didn't change any logic.
With further refactors the tests will gradually be removed and replaced with handwritten tests that document that intention of the behaviour as well.

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

PR title and summary are descriptive. (conventions)
~~Docs updated or follow-up ticket created.~~
Tests included.
~~PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)~~

cubic-dev-ai

cubic analysis

1 issue found across 3 files • Review in cubic

_{React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

cubic-dev-ai · 2025-07-31T14:20:07Z

packages/core/src/execution-engine/workflow-execute.ts

-						break;
-					}
+		// We always use the data of main input and the first input for execute
+		let connectionInputData = inputData.main[0] as INodeExecutionData[];


Rule violated: Prefer Typeguards over Type casting

Avoid using as for type narrowing – replace the cast with a proper type annotation.
Rule "Prefer Typeguards over Type casting" forbids as in this context since it is not a DOM/event/const/generic-constraint assertion.

Prompt for AI agents

Address the following comment on packages/core/src/execution-engine/workflow-execute.ts at line 1129: <comment>Avoid using `as` for type narrowing – replace the cast with a proper type annotation. Rule "Prefer Typeguards over Type casting" forbids `as` in this context since it is not a DOM/event/const/generic-constraint assertion.</comment> <file context> @@ -1089,72 +1089,68 @@ export class WorkflowExecute { return customOperation; } - /** Executes the given node */ - // eslint-disable-next-line complexity - async runNode( - workflow: Workflow, - executionData: IExecuteData, - runExecutionData: IRunExecutionData, </file context>

Suggested change

let connectionInputData = inputData.main[0] as INodeExecutionData[];

let connectionInputData: INodeExecutionData[] = inputData.main[0];

schrothbn

Makes sense to me as a first refactor, a great improvement to understand what's happening. Also besides the heavy mocking I think the additional test coverage is quite useful.
I'll let Ivan do the final approval so we have another pair of eyes on it.

ivov

Quick pass, more thorough look with fresh eyes tomorrow.

ivov · 2025-08-04T14:58:58Z

packages/core/src/execution-engine/workflow-execute.ts

-						break;
-					}
+		// We always use the data of main input and the first input for execute
+		let connectionInputData = inputData.main[0] as INodeExecutionData[];


We could do inputData.main.at(0), check it for the empty case, and so allow the typechecker to recognize that connectionInputData is INodeExecutionData[] without overriding TS.

ivov · 2025-08-04T15:01:34Z

packages/core/src/execution-engine/workflow-execute.ts

+			!nodeType.execute &&
+			!customOperation &&
+			(nodeType.poll || nodeType.trigger || nodeType.webhook)


I found this condition a bit easier to read beforehand.

Curious why we need to check that a node type does not have an execute method and does not have a custom node operation in addition to checking that it has a method that identifies it as a trigger, poller or webhook? I'd expect the latter should be enough?

ivov · 2025-08-04T15:03:45Z

packages/core/src/execution-engine/workflow-execute.ts

-
-		let connectionInputData: INodeExecutionData[] = [];
+	/**
+	 * Prepares input data for node execution based on node type and workflow settings


This docline could use some love. What does it mean to "prepare" input data? The args mentioned are already clear from the signature. Let's either improve or remove?

ivov · 2025-08-04T15:07:28Z

packages/core/src/execution-engine/workflow-execute.ts

+		// We always use the data of main input and the first input for execute
+		let connectionInputData = inputData.main[0] as INodeExecutionData[];
+
+		const forceInputNodeExecution = workflow.settings.executionOrder !== 'v1';


Suggested change

const forceInputNodeExecution = workflow.settings.executionOrder !== 'v1';

const legacyExecutionOrder = workflow.settings.executionOrder !== 'v1';

ivov · 2025-08-04T15:08:15Z

packages/core/src/execution-engine/workflow-execute.ts

+		if (connectionInputData.length === 0) {
+			return null;


Missing context so this might be intended, but I notice the return value differs?

ivov · 2025-08-04T15:15:20Z

packages/core/src/execution-engine/workflow-execute.ts

+	/**
+	 * Executes a declarative node using RoutingNode
+	 */


This docline doesn't add much.

ivov · 2025-08-04T15:15:48Z

packages/core/src/execution-engine/workflow-execute.ts

+	/**
+	 * Executes a declarative node using RoutingNode
+	 */
+	private async executeDeclarativeNode(


Suggested change

private async executeDeclarativeNode(

private async executeDeclarativeNodeInTest(

ivov · 2025-08-04T15:17:22Z

packages/core/src/execution-engine/workflow-execute.ts

+	 * @param abortSignal - Signal for canceling execution
+	 * @returns Promise resolving to node execution result
+	 */
+	async runNode(


We have a runNode (which orchestrates) and an executeNode (which actually executes). I find the above docline mostly redundant of the code (does it add much?), but it'd be a good opportunity to document this so folks can clearly differentiate between them. Or maybe finding better names that make this clear outright.

ivov · 2025-08-04T15:21:10Z

packages/core/src/execution-engine/workflow-execute.ts


-				const response = await triggerResponse.manualTriggerResponse!;
+		const nodeType = workflow.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
+		const isDeclarativeNode = nodeType.description.requestDefaults !== undefined;


We can move this after the connectionInputData check.

ivov · 2025-08-04T15:23:05Z

packages/core/src/execution-engine/workflow-execute.ts

+				executionData,
+				abortSignal,
+			);
+		} else if (nodeType.poll) {


For a minimal readability gain in this nightmare file, since all these cases return, can we remove the unnecessary elses and spread out these cases?

despairblue force-pushed the refactor-run-node branch 2 times, most recently from 7b3dbf4 to 4c576f0 Compare July 31, 2025 13:33

despairblue added 25 commits July 31, 2025 15:35

add tests for runNode

009bda3

fix type errors

698c476

rename file to prevent lint issue

553fde7

fix lint issues

6bc865f

add test

64c833c

add test

b1a0630

add test

97cae86

fix test

fb2849b

add test

c5ec0f9

fix type errors

831fa6a

ignore coverage files when linting

205128d

add test

9e5442a

add test

d5f4600

add tests

515d202

extract handling disabled nodes from runNode

d1af487

extract preparing connection input data creation from runNode

3df2b43

extract error handling logic from runNode

4b49e9b

extract handleExecuteOnce from runNode

0bdf068

extract node execution logic from runNode

3f6d09c

extract poll node logic from runNode

e1021d1

extract trigger node logic from runNode

e4793dd

extract declarative node logic from runNode

b9ec6fa

remove eslint disable

6b03a8f

add additional types

0b7b66f

rename test file and add notice about it being generated by AI

230d521

despairblue force-pushed the refactor-run-node branch from 4c576f0 to 230d521 Compare July 31, 2025 13:36

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Jul 31, 2025

despairblue requested review from ivov and schrothbn July 31, 2025 14:13

despairblue marked this pull request as ready for review July 31, 2025 14:13

cubic-dev-ai bot reviewed Jul 31, 2025

View reviewed changes

schrothbn reviewed Aug 4, 2025

View reviewed changes

ivov reviewed Aug 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(core): Split `WorkflowExecute.runNode` into smaller methods #17864

refactor(core): Split `WorkflowExecute.runNode` into smaller methods #17864

despairblue commented Jul 31, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

cubic-dev-ai bot Jul 31, 2025

Uh oh!

schrothbn left a comment

Uh oh!

ivov left a comment

Uh oh!

ivov Aug 4, 2025

Uh oh!

ivov Aug 4, 2025

Uh oh!

ivov Aug 4, 2025

Uh oh!

ivov Aug 4, 2025

Uh oh!

ivov Aug 4, 2025

Uh oh!

ivov Aug 4, 2025

Uh oh!

ivov Aug 4, 2025

Uh oh!

ivov Aug 4, 2025

Uh oh!

ivov Aug 4, 2025

Uh oh!

ivov Aug 4, 2025

Uh oh!

Uh oh!

	let connectionInputData = inputData.main[0] as INodeExecutionData[];
	let connectionInputData: INodeExecutionData[] = inputData.main[0];

	const forceInputNodeExecution = workflow.settings.executionOrder !== 'v1';
	const legacyExecutionOrder = workflow.settings.executionOrder !== 'v1';

	private async executeDeclarativeNode(
	private async executeDeclarativeNodeInTest(

refactor(core): Split WorkflowExecute.runNode into smaller methods #17864

Are you sure you want to change the base?

refactor(core): Split WorkflowExecute.runNode into smaller methods #17864

Conversation

despairblue commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

cubic analysis

Uh oh!

cubic-dev-ai bot Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

schrothbn left a comment

Choose a reason for hiding this comment

Uh oh!

ivov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

refactor(core): Split `WorkflowExecute.runNode` into smaller methods #17864

refactor(core): Split `WorkflowExecute.runNode` into smaller methods #17864

despairblue commented Jul 31, 2025 •

edited

Loading