fix(core): Fix new partial execution flow not working with workflows with multiple disabled nodes in a row (no-changelog) (#11024)

packages/core/src/PartialExecutionUtils/DirectedGraph.ts

@@ -67,6 +67,63 @@ export class DirectedGraph {
 		return this;
 	}
 
+	/**
+	 * Removes a node from the graph.
+	 *
+	 * By default it will also remove all connections that use that node and
+	 * return nothing.
+	 *
+	 * If you pass `{ reconnectConnections: true }` it will rewire all
+	 * connections making sure all parent nodes are connected to all child nodes
+	 * and return the new connections.
+	 */
+	removeNode(node: INode, options?: { reconnectConnections: true }): GraphConnection[];
+	removeNode(node: INode, options?: { reconnectConnections: false }): undefined;
+	removeNode(node: INode, { reconnectConnections = false } = {}): undefined | GraphConnection[] {
+		if (reconnectConnections) {
+			const incomingConnections = this.getDirectParents(node);
+			const outgoingConnections = this.getDirectChildren(node);
+
+			const newConnections: GraphConnection[] = [];
+
+			for (const incomingConnection of incomingConnections) {
+				for (const outgoingConnection of outgoingConnections) {
+					const newConnection = {
+						...incomingConnection,
+						to: outgoingConnection.to,
+						inputIndex: outgoingConnection.inputIndex,
+					};
+
+					newConnections.push(newConnection);
+				}
+			}
+
+			for (const [key, connection] of this.connections.entries()) {
+				if (connection.to === node || connection.from === node) {
+					this.connections.delete(key);
+				}
+			}
+
+			for (const newConnection of newConnections) {
+				this.connections.set(this.makeKey(newConnection), newConnection);
+			}
+
+			this.nodes.delete(node.name);
+
+			return newConnections;
+		} else {
+			for (const [key, connection] of this.connections.entries()) {
+				if (connection.to === node || connection.from === node) {
+					this.connections.delete(key);
+				}
+			}
+
+			this.nodes.delete(node.name);
+
+			return;
+		}
+	}
+
 	addConnection(connectionInput: {
 		from: INode;
 		to: INode;

packages/core/src/PartialExecutionUtils/__tests__/DirectedGraph.test.ts

@@ -9,6 +9,8 @@
 // XX denotes that the node is disabled
 // PD denotes that the node has pinned data
 
+import { NodeConnectionType } from 'n8n-workflow';
+
 import { createNodeData, defaultWorkflowParameter } from './helpers';
 import { DirectedGraph } from '../DirectedGraph';
 
@@ -86,4 +88,202 @@ describe('DirectedGraph', () => {
 			expect(children).toEqual(new Set([node1, node2, node3]));
 		});
 	});
+
+	describe('removeNode', () => {
+		//              XX
+		//  ┌─────┐    ┌─────┐   ┌─────┐
+		//  │node0├───►│node1├──►│node2│
+		//  └─────┘    └─────┘   └─────┘
+		// turns into
+		//  ┌─────┐              ┌─────┐
+		//  │node0│              │node2│
+		//  └─────┘              └─────┘
+		test('remove node and all connections', () => {
+			// ARRANGE
+			const node0 = createNodeData({ name: 'node0' });
+			const node1 = createNodeData({ name: 'node1' });
+			const node2 = createNodeData({ name: 'node2' });
+			const graph = new DirectedGraph()
+				.addNodes(node0, node1, node2)
+				.addConnections({ from: node0, to: node1 }, { from: node0, to: node2 });
+
+			// ACT
+			graph.removeNode(node1);
+
+			// ASSERT
+			expect(graph).toEqual(
+				new DirectedGraph().addNodes(node0, node2).addConnections({ from: node0, to: node2 }),
+			);
+		});
+
+		//              XX
+		//  ┌─────┐    ┌─────┐   ┌─────┐
+		//  │node0├───►│node1├──►│node2│
+		//  └─────┘    └─────┘   └─────┘
+		// turns into
+		//  ┌─────┐   ┌─────┐
+		//  │node0├──►│node2│
+		//  └─────┘   └─────┘
+		test('remove node, but reconnect connections', () => {
+			// ARRANGE
+			const node0 = createNodeData({ name: 'node0' });
+			const node1 = createNodeData({ name: 'node1' });
+			const node2 = createNodeData({ name: 'node2' });
+			const graph = new DirectedGraph()
+				.addNodes(node0, node1, node2)
+				.addConnections({ from: node0, to: node1 }, { from: node1, to: node2 });
+
+			// ACT
+			const newConnections = graph.removeNode(node1, { reconnectConnections: true });
+
+			// ASSERT
+			expect(newConnections).toHaveLength(1);
+			expect(newConnections[0]).toEqual({
+				from: node0,
+				outputIndex: 0,
+				type: NodeConnectionType.Main,
+				inputIndex: 0,
+				to: node2,
+			});
+			expect(graph).toEqual(
+				new DirectedGraph().addNodes(node0, node2).addConnections({ from: node0, to: node2 }),
+			);
+		});
+
+		//               XX
+		//  ┌─────┐     ┌─────┐     ┌─────┐
+		//  │     │o   o│     │o   o│     │
+		//  │     │o─┐ o│     │o   o│     │
+		//  │node0│o └►o│node1│o   o│node2│
+		//  │     │o   o│     │o─┐ o│     │
+		//  │     │o   o│     │o └►o│     │
+		//  └─────┘     └─────┘     └─────┘
+		// turns into
+		//  ┌─────┐                 ┌─────┐
+		//  │     │o               o│     │
+		//  │     │o───────┐       o│     │
+		//  │node0│o       │       o│node2│
+		//  │     │o       │       o│     │
+		//  │     │o       └──────►o│     │
+		//  └─────┘                 └─────┘
+		test('remove node, reconnect connections and retaining the input indexes', () => {
+			// ARRANGE
+			const node0 = createNodeData({ name: 'node0' });
+			const node1 = createNodeData({ name: 'node1' });
+			const node2 = createNodeData({ name: 'node2' });
+			const graph = new DirectedGraph()
+				.addNodes(node0, node1, node2)
+				.addConnections(
+					{ from: node0, outputIndex: 1, inputIndex: 2, to: node1 },
+					{ from: node1, outputIndex: 3, inputIndex: 4, to: node2 },
+				);
+
+			// ACT
+			const newConnections = graph.removeNode(node1, { reconnectConnections: true });
+
+			// ASSERT
+			expect(newConnections).toHaveLength(1);
+			expect(newConnections[0]).toEqual({
+				from: node0,
+				outputIndex: 1,
+				type: NodeConnectionType.Main,
+				inputIndex: 4,
+				to: node2,
+			});
+			expect(graph).toEqual(
+				new DirectedGraph()
+					.addNodes(node0, node2)
+					.addConnections({ from: node0, outputIndex: 1, inputIndex: 4, to: node2 }),
+			);
+		});
+
+		//               XX
+		//  ┌─────┐     ┌─────┐     ┌─────┐
+		//  │     │o   o│     │o    │     │
+		//  │     │o─┐ o│     │o    │     │
+		//  │node0│  └►o│node1│o ┌►o│node2│
+		//  │     │     │     │o─┘  │     │
+		//  │     │     │     │     │     │
+		//  └─────┘     └─────┘     └─────┘
+		// turns into
+		//  ┌─────┐                 ┌─────┐
+		//  │     │o                │     │
+		//  │     │o───────┐        │     │
+		//  │node0│        └──────►o│node2│
+		//  │     │                 │     │
+		//  │     │                 │     │
+		//  └─────┘                 └─────┘
+		test('remove node, reconnect connections and retaining the input indexes, even if the child has less inputs than the than the removed node had', () => {
+			// ARRANGE
+			const node0 = createNodeData({ name: 'node0' });
+			const node1 = createNodeData({ name: 'node1' });
+			const node2 = createNodeData({ name: 'node2' });
+			const graph = new DirectedGraph()
+				.addNodes(node0, node1, node2)
+				.addConnections(
+					{ from: node0, outputIndex: 1, inputIndex: 2, to: node1 },
+					{ from: node1, outputIndex: 3, inputIndex: 0, to: node2 },
+				);
+
+			// ACT
+			const newConnections = graph.removeNode(node1, { reconnectConnections: true });
+
+			// ASSERT
+			const expectedGraph = new DirectedGraph()
+				.addNodes(node0, node2)
+				.addConnections({ from: node0, outputIndex: 1, inputIndex: 0, to: node2 });
+			expect(newConnections).toHaveLength(1);
+			expect(newConnections).toEqual(expectedGraph.getConnections());
+			expect(graph).toEqual(expectedGraph);
+		});
+
+		//  ┌─────┐                ┌──────┐
+		//  │left0├─┐   XX       ┌►│right0│
+		//  └─────┘ │  ┌──────┐  │ └──────┘
+		//          ├─►│center├──┤
+		//  ┌─────┐ │  └──────┘  │ ┌──────┐
+		//  │left1├─┘            └►│right1│
+		//  └─────┘                └──────┘
+		// turns into
+		//
+		//  ┌─────┐                ┌──────┐
+		//  │left0├─┐           ┌─►│right0│
+		//  └─────┘ │           │  └──────┘
+		//          ├───────────┤
+		//  ┌─────┐ │           │  ┌──────┐
+		//  │left1├─┘           └─►│right1│
+		//  └─────┘                └──────┘
+		test('remove node, reconnect connections and multiplexes them', () => {
+			// ARRANGE
+			const left0 = createNodeData({ name: 'left0' });
+			const left1 = createNodeData({ name: 'left1' });
+			const center = createNodeData({ name: 'center' });
+			const right0 = createNodeData({ name: 'right0' });
+			const right1 = createNodeData({ name: 'right1' });
+			const graph = new DirectedGraph()
+				.addNodes(left0, left1, center, right0, right1)
+				.addConnections(
+					{ from: left0, to: center },
+					{ from: left1, to: center },
+					{ from: center, to: right0 },
+					{ from: center, to: right1 },
+				);
+
+			// ACT
+			const newConnections = graph.removeNode(center, { reconnectConnections: true });
+
+			// ASSERT
+			const expectedGraph = new DirectedGraph()
+				.addNodes(left0, left1, right0, right1)
+				.addConnections(
+					{ from: left0, to: right0 },
+					{ from: left0, to: right1 },
+					{ from: left1, to: right0 },
+					{ from: left1, to: right1 },
+				);
+			expect(newConnections).toHaveLength(4);
+			expect(newConnections).toEqual(expectedGraph.getConnections());
+			expect(graph).toEqual(expectedGraph);
+		});
+	});
 });

packages/core/src/PartialExecutionUtils/__tests__/findSubgraph.test.ts

@@ -13,7 +13,7 @@ import { createNodeData } from './helpers';
 import { DirectedGraph } from '../DirectedGraph';
 import { findSubgraph } from '../findSubgraph';
 
-describe('findSubgraph2', () => {
+describe('findSubgraph', () => {
 	//                 ►►
 	//  ┌───────┐     ┌───────────┐
 	//  │trigger├────►│destination│
@@ -83,6 +83,12 @@ describe('findSubgraph2', () => {
 	//  │trigger│         │disabled├─────►│destination│
 	//  │       ├────────►│        │      └───────────┘
 	//  └───────┘         └────────┘
+	// turns into
+	//  ┌───────┐       ►►
+	//  │       │      ┌───────────┐
+	//  │trigger├─────►│destination│
+	//  │       │      └───────────┘
+	//  └───────┘
 	test('skip disabled nodes', () => {
 		const trigger = createNodeData({ name: 'trigger' });
 		const disabled = createNodeData({ name: 'disabled', disabled: true });
@@ -101,6 +107,40 @@ describe('findSubgraph2', () => {
 		);
 	});
 
+	//                 XX          XX
+	//  ┌───────┐     ┌─────┐     ┌─────┐     ┌───────────┐
+	//  │trigger├────►│node1├────►│node2├────►│destination│
+	//  └───────┘     └─────┘     └─────┘     └───────────┘
+	// turns into
+	//  ┌───────┐     ┌───────────┐
+	//  │trigger├────►│destination│
+	//  └───────┘     └───────────┘
+	test('skip multiple disabled nodes', () => {
+		// ARRANGE
+		const trigger = createNodeData({ name: 'trigger' });
+		const disabledNode1 = createNodeData({ name: 'disabledNode1', disabled: true });
+		const disabledNode2 = createNodeData({ name: 'disabledNode2', disabled: true });
+		const destination = createNodeData({ name: 'destination' });
+
+		const graph = new DirectedGraph()
+			.addNodes(trigger, disabledNode1, disabledNode2, destination)
+			.addConnections(
+				{ from: trigger, to: disabledNode1 },
+				{ from: disabledNode1, to: disabledNode2 },
+				{ from: disabledNode2, to: destination },
+			);
+
+		// ACT
+		const subgraph = findSubgraph(graph, destination, trigger);
+
+		// ASSERT
+		expect(subgraph).toEqual(
+			new DirectedGraph()
+				.addNodes(trigger, destination)
+				.addConnections({ from: trigger, to: destination }),
+		);
+	});
+
 	//                                ►►
 	//  ┌───────┐       ┌─────┐      ┌─────┐
 	//  │Trigger├───┬──►│Node1├───┬─►│Node2│

packages/core/src/PartialExecutionUtils/findSubgraph.ts

@@ -51,27 +51,14 @@ function findSubgraphRecursive(
 	// Take every incoming connection and connect it to every node that is
 	// connected to the current node’s first output
 	if (current.disabled) {
-		const incomingConnections = graph.getDirectParents(current);
-		const outgoingConnections = graph
-			.getDirectChildren(current)
-			// NOTE: When a node is disabled only the first output gets data
-			.filter((connection) => connection.outputIndex === 0);
+		// The last segment on the current branch is still pointing to the removed
+		// node, so let's remove it.
+		currentBranch.pop();
 
-		parentConnections = [];
-
-		for (const incomingConnection of incomingConnections) {
-			for (const outgoingConnection of outgoingConnections) {
-				const newConnection = {
-					...incomingConnection,
-					to: outgoingConnection.to,
-					inputIndex: outgoingConnection.inputIndex,
-				};
-
-				parentConnections.push(newConnection);
-				currentBranch.pop();
-				currentBranch.push(newConnection);
-			}
-		}
+		// The node is replaced by a set of new connections, connecting the parents
+		// and children of it directly. In the recursive call below we'll follow
+		// them further.
+		parentConnections = graph.removeNode(current, { reconnectConnections: true });
 	}
 
 	// Recurse on each parent.

packages/core/src/PartialExecutionUtils/recreateNodeExecutionStack.ts

@@ -44,12 +44,12 @@ export function recreateNodeExecutionStack(
 	// Validate invariants.
 
 	// The graph needs to be free of disabled nodes. If it's not it hasn't been
-	// passed through findSubgraph2.
+	// passed through findSubgraph.
 	for (const node of graph.getNodes().values()) {
 		a.notEqual(
 			node.disabled,
 			true,
-			`Graph contains disabled nodes. This is not supported. Make sure to pass the graph through "findSubgraph2" before calling "recreateNodeExecutionStack". The node in question is "${node.name}"`,
+			`Graph contains disabled nodes. This is not supported. Make sure to pass the graph through "findSubgraph" before calling "recreateNodeExecutionStack". The node in question is "${node.name}"`,
 		);
 	}