refactor(core): Deduplicate node group validation helpers across agent (#36810)

packages/@n8n/instance-ai/src/tools/workflows/generate-simulation-fixtures.service.ts

@@ -27,6 +27,7 @@ import {
 	findOutputParserTargets,
 	parsePinDataResponse,
 	repairStructuredOutput,
+	toEngineConnections,
 } from '@n8n/workflow-sdk';
 import { getParentNodes, mapConnectionsByDestination, type IConnections } from 'n8n-workflow';
 import { z } from 'zod';
@@ -170,7 +171,7 @@ export async function generateSimulationFixtures(
 	const schemaContextByName = new Map(schemaContexts.map((ctx) => [ctx.nodeName, ctx] as const));
 
 	const connectionsByDestination = mapConnectionsByDestination(
-		(input.workflow.connections ?? {}) as IConnections,
+		toEngineConnections(input.workflow.connections),
 	);
 	const userText = [
 		'Generate realistic mock output (pin-data items) for the following simulated n8n nodes.',

packages/@n8n/instance-ai/src/tools/workflows/validate-workflow.service.ts

@@ -6,7 +6,7 @@
  */
 import { AI_GATEWAY_MANAGED_TAG } from '@n8n/api-types';
 import type { DisplayOptions, NodeJSON, WorkflowJSON } from '@n8n/workflow-sdk';
-import { matchesDisplayOptions } from '@n8n/workflow-sdk';
+import { matchesDisplayOptions, toEngineConnections } from '@n8n/workflow-sdk';
 import type {
 	IConnections,
 	INodeInputConfiguration,
@@ -669,7 +669,7 @@ export async function validateWorkflowConfig(
 	// Invert connections once — every per-node input-issue check needs the
 	// destination-keyed view, and mapConnectionsByDestination is O(n).
 	const connectionsByDestination = mapConnectionsByDestination(
-		(workflowJson.connections ?? {}) as IConnections,
+		toEngineConnections(workflowJson.connections),
 	);
 
 	// Fetch the latest run data once for the workflow. Skip when we have no

packages/@n8n/workflow-sdk/src/codegen/emit-instance-ai.test.ts

@@ -282,6 +282,10 @@ describe('emit-instance-ai', () => {
 			'workflowToMermaid',
 			// Display-options matching
 			'matchesDisplayOptions',
+			// SDK-to-engine adapters for host-side validation and graph helpers
+			'dropInvalidWorkflowJsonGroups',
+			'toEngineConnections',
+			'toGroupValidationNodes',
 			// Plugin registration
 			'registerDefaultPlugins',
 			// Generate-types module (build-time type generation, never appears in workflows)

packages/@n8n/workflow-sdk/src/index.ts

@@ -143,6 +143,11 @@ export {
 
 // Code helpers
 export { runOnceForAllItems, runOnceForEachItem } from './utils/code-helpers';
+export {
+	dropInvalidWorkflowJsonGroups,
+	toEngineConnections,
+	toGroupValidationNodes,
+} from './utils/workflow-json-engine-helpers';
 
 // Utility functions
 export { isPlainObject, getProperty, hasProperty } from './utils/safe-access';

packages/@n8n/workflow-sdk/src/utils/workflow-json-engine-helpers.test.ts

@@ -0,0 +1,291 @@
+import {
+	jsonParse,
+	makeGetNodeTypeForGrouping,
+	mapConnectionsByDestination,
+	NodeConnectionTypes,
+	type INodeTypeDescription,
+	type INodeTypes,
+} from 'n8n-workflow';
+
+import {
+	dropInvalidWorkflowJsonGroups,
+	toEngineConnections,
+	toGroupValidationNodes,
+} from './workflow-json-engine-helpers';
+import type { WorkflowJSON } from '../types/base';
+
+function makeNodeType(overrides: Partial<INodeTypeDescription> = {}): INodeTypeDescription {
+	return {
+		displayName: overrides.displayName ?? 'Set',
+		name: overrides.name ?? 'n8n-nodes-base.set',
+		group: overrides.group ?? ['transform'],
+		version: overrides.version ?? 1,
+		description: overrides.description ?? '',
+		defaults: overrides.defaults ?? { name: 'Set' },
+		inputs: overrides.inputs ?? [NodeConnectionTypes.Main],
+		outputs: overrides.outputs ?? [NodeConnectionTypes.Main],
+		properties: overrides.properties ?? [],
+		...overrides,
+	};
+}
+
+function makeNode(
+	id: string,
+	name: string,
+	type = 'n8n-nodes-base.set',
+): WorkflowJSON['nodes'][number] {
+	return {
+		id,
+		name,
+		type,
+		typeVersion: 1,
+		position: [0, 0],
+		parameters: {},
+	};
+}
+
+function makeWorkflow(
+	nodes: WorkflowJSON['nodes'],
+	connections: WorkflowJSON['connections'] = {},
+	nodeGroups?: WorkflowJSON['nodeGroups'],
+): WorkflowJSON {
+	return {
+		name: 'Test workflow',
+		nodes,
+		connections,
+		...(nodeGroups ? { nodeGroups } : {}),
+	};
+}
+
+function makeNodeTypesProvider(): INodeTypes {
+	const descriptions: Record<string, INodeTypeDescription> = {
+		'n8n-nodes-base.set': makeNodeType(),
+		'n8n-nodes-base.manualTrigger': makeNodeType({
+			name: 'n8n-nodes-base.manualTrigger',
+			group: ['trigger'],
+		}),
+		'@n8n/n8n-nodes-langchain.agent': makeNodeType({
+			name: '@n8n/n8n-nodes-langchain.agent',
+		}),
+		'@n8n/n8n-nodes-langchain.lmChatOpenAi': makeNodeType({
+			name: '@n8n/n8n-nodes-langchain.lmChatOpenAi',
+		}),
+	};
+
+	const getByNameAndVersion = (nodeType: string) => {
+		const description = descriptions[nodeType];
+		if (!description) throw new Error('Unknown node type');
+		return { description };
+	};
+
+	return {
+		getByName: getByNameAndVersion,
+		getByNameAndVersion,
+		getKnownTypes: () => ({}),
+	};
+}
+
+describe('toEngineConnections', () => {
+	it('skips connections whose type is not known to the engine', () => {
+		const connections: WorkflowJSON['connections'] = {
+			Source: {
+				main: [[{ node: 'Target', type: 'unknown_type', index: 0 }]],
+			},
+		};
+
+		expect(toEngineConnections(connections)).toEqual({ Source: { main: [[]] } });
+	});
+
+	it('skips unsafe source-node keys without polluting the prototype', () => {
+		const connections = jsonParse<WorkflowJSON['connections']>(
+			'{"__proto__":{"main":[[{"node":"Target","type":"main","index":0}]]},"Source":{"main":[[{"node":"Target","type":"main","index":0}]]}}',
+		);
+
+		const result = toEngineConnections(connections);
+
+		expect(Object.hasOwn(result, '__proto__')).toBe(false);
+		expect(Object.getPrototypeOf(result)).toBe(Object.prototype);
+		expect(result).toEqual({
+			Source: { main: [[{ node: 'Target', type: NodeConnectionTypes.Main, index: 0 }]] },
+		});
+	});
+
+	it('skips unsafe target-node names before destination mapping', () => {
+		const connections = jsonParse<WorkflowJSON['connections']>(
+			'{"Source":{"main":[[{"node":"__proto__","type":"main","index":0},{"node":"Target","type":"main","index":0}]]}}',
+		);
+
+		const result = toEngineConnections(connections);
+		const connectionsByDestination = mapConnectionsByDestination(result);
+
+		expect(result).toEqual({
+			Source: { main: [[{ node: 'Target', type: NodeConnectionTypes.Main, index: 0 }]] },
+		});
+		expect(Object.hasOwn(connectionsByDestination, '__proto__')).toBe(false);
+		expect(Object.getPrototypeOf(connectionsByDestination)).toBe(Object.prototype);
+		expect(connectionsByDestination).toEqual({
+			Target: { main: [[{ node: 'Source', type: NodeConnectionTypes.Main, index: 0 }]] },
+		});
+	});
+
+	it('preserves null output slots', () => {
+		const connections: WorkflowJSON['connections'] = {
+			Source: {
+				main: [null, [{ node: 'Target', type: 'main', index: 0 }]],
+			},
+		};
+
+		expect(toEngineConnections(connections)).toEqual({
+			Source: { main: [null, [{ node: 'Target', type: NodeConnectionTypes.Main, index: 0 }]] },
+		});
+	});
+});
+
+describe('toGroupValidationNodes', () => {
+	it('defaults a missing node name to an empty string', () => {
+		expect(
+			toGroupValidationNodes([
+				{
+					id: 'node-id',
+					type: 'n8n-nodes-base.set',
+					typeVersion: 1,
+					position: [0, 0],
+				},
+			]),
+		).toEqual([
+			{
+				id: 'node-id',
+				name: '',
+				type: 'n8n-nodes-base.set',
+				typeVersion: 1,
+				position: [0, 0],
+				parameters: {},
+			},
+		]);
+	});
+});
+
+describe('dropInvalidWorkflowJsonGroups', () => {
+	const getNodeType = makeGetNodeTypeForGrouping(makeNodeTypesProvider());
+
+	it('returns no violations and leaves the workflow untouched when nodeGroups is missing', () => {
+		const json = makeWorkflow([makeNode('a', 'A')]);
+		const before = structuredClone(json);
+
+		expect(dropInvalidWorkflowJsonGroups(json, getNodeType)).toEqual([]);
+		expect(json).toEqual(before);
+	});
+
+	it('returns no violations and keeps valid groups untouched', () => {
+		const nodeGroups = [{ id: 'g1', name: 'Valid group', nodeIds: ['a', 'b'] }];
+		const json = makeWorkflow(
+			[makeNode('a', 'A'), makeNode('b', 'B')],
+			{
+				A: { main: [[{ node: 'B', type: NodeConnectionTypes.Main, index: 0 }]] },
+			},
+			nodeGroups,
+		);
+
+		expect(dropInvalidWorkflowJsonGroups(json, getNodeType)).toEqual([]);
+		expect(json.nodeGroups).toBe(nodeGroups);
+		expect(json.nodeGroups).toEqual([{ id: 'g1', name: 'Valid group', nodeIds: ['a', 'b'] }]);
+	});
+
+	it('drops a group containing a trigger node', () => {
+		const json = makeWorkflow(
+			[makeNode('trigger', 'Manual Trigger', 'n8n-nodes-base.manualTrigger')],
+			{},
+			[{ id: 'g1', name: 'Trigger group', nodeIds: ['trigger'] }],
+		);
+
+		const violations = dropInvalidWorkflowJsonGroups(json, getNodeType);
+
+		expect(json.nodeGroups).toEqual([]);
+		expect(violations).toEqual([
+			expect.objectContaining({
+				groupId: 'g1',
+				groupName: 'Trigger group',
+				code: 'trigger-selected',
+			}),
+		]);
+		expect(violations[0]?.message).toContain('cannot contain trigger nodes: Manual Trigger.');
+	});
+
+	it('drops only invalid groups when valid and invalid groups are present', () => {
+		const json = makeWorkflow(
+			[
+				makeNode('a', 'A'),
+				makeNode('b', 'B'),
+				makeNode('trigger', 'Manual Trigger', 'n8n-nodes-base.manualTrigger'),
+			],
+			{
+				A: { main: [[{ node: 'B', type: NodeConnectionTypes.Main, index: 0 }]] },
+			},
+			[
+				{ id: 'valid', name: 'Valid group', nodeIds: ['a', 'b'] },
+				{ id: 'invalid', name: 'Trigger group', nodeIds: ['trigger'] },
+			],
+		);
+
+		const violations = dropInvalidWorkflowJsonGroups(json, getNodeType);
+
+		expect(json.nodeGroups).toEqual([{ id: 'valid', name: 'Valid group', nodeIds: ['a', 'b'] }]);
+		expect(violations).toHaveLength(1);
+		expect(violations[0]?.groupName).toBe('Trigger group');
+	});
+
+	it('returns every violation for dropped groups while dropping each group once', () => {
+		const json = makeWorkflow([makeNode('a', 'A')], {}, [
+			{ id: 'g1', name: 'Missing nodes group', nodeIds: ['missing-1', 'missing-2'] },
+		]);
+
+		const violations = dropInvalidWorkflowJsonGroups(json, getNodeType);
+
+		expect(json.nodeGroups).toEqual([]);
+		expect(violations).toHaveLength(2);
+		expect(violations[0]?.message).toContain('missing-1');
+		expect(violations[1]?.message).toContain('missing-2');
+	});
+
+	it('runs basic checks without getNodeType but skips trigger checks', () => {
+		const basicInvalid = makeWorkflow([makeNode('a', 'A')], {}, [
+			{ id: 'g1', name: 'Unknown node group', nodeIds: ['missing'] },
+		]);
+		const triggerGroup = makeWorkflow(
+			[makeNode('trigger', 'Manual Trigger', 'n8n-nodes-base.manualTrigger')],
+			{},
+			[{ id: 'g1', name: 'Trigger group', nodeIds: ['trigger'] }],
+		);
+
+		expect(dropInvalidWorkflowJsonGroups(basicInvalid, null)).toHaveLength(1);
+		expect(basicInvalid.nodeGroups).toEqual([]);
+		expect(dropInvalidWorkflowJsonGroups(triggerGroup, null)).toEqual([]);
+		expect(triggerGroup.nodeGroups).toEqual([
+			{ id: 'g1', name: 'Trigger group', nodeIds: ['trigger'] },
+		]);
+	});
+
+	it('detects a non-main boundary connection through the shared SDK adapter', () => {
+		const json = makeWorkflow(
+			[
+				makeNode('agent', 'Agent', '@n8n/n8n-nodes-langchain.agent'),
+				makeNode('model', 'Model', '@n8n/n8n-nodes-langchain.lmChatOpenAi'),
+			],
+			{
+				Model: {
+					[NodeConnectionTypes.AiLanguageModel]: [
+						[{ node: 'Agent', type: NodeConnectionTypes.AiLanguageModel, index: 0 }],
+					],
+				},
+			},
+			[{ id: 'g1', name: 'Agent group', nodeIds: ['agent'] }],
+		);
+
+		const violations = dropInvalidWorkflowJsonGroups(json, getNodeType);
+
+		expect(json.nodeGroups).toEqual([]);
+		expect(violations[0]?.message).toContain(
+			'cannot cross the "ai_languageModel" connection between "Model" and "Agent"',
+		);
+	});
+});

packages/@n8n/workflow-sdk/src/utils/workflow-json-engine-helpers.ts

@@ -0,0 +1,85 @@
+import {
+	dropInvalidWorkflowGroups,
+	isNodeConnectionType,
+	isSafeObjectProperty,
+	type GetNodeTypeForGrouping,
+	type IConnections,
+	type INode,
+	type INodeConnections,
+	type WorkflowGroupViolation,
+} from 'n8n-workflow';
+
+import type { WorkflowJSON } from '../types/base';
+
+/**
+ * LOSSY: Bridges the SDK's connections to `n8n-workflow`'s `IConnections`.
+ * Unknown connection types are skipped; this is for graph traversal and group
+ * validation, not for preserving arbitrary serialized data.
+ *
+ * The SDK and engine declare duplicate but formally separate connection types
+ * (the SDK types `IConnection.type` as plain `string`), so values are re-keyed
+ * through the `isNodeConnectionType` guard. Serializer output only ever carries
+ * known connection types; if an unknown one slips through, that connection is
+ * skipped here and the save path still rejects the workflow.
+ *
+ * Node names and connection types come from submitted code, so source keys,
+ * target keys, and connection-type keys that resolve to object internals
+ * (`__proto__`, `constructor`, ...) are skipped — assigning them onto a plain
+ * object would mutate its prototype instead of creating an own property,
+ * silently corrupting the re-keyed connections.
+ */
+export function toEngineConnections(connections: WorkflowJSON['connections']): IConnections {
+	const bySourceNode: IConnections = {};
+	for (const [sourceNode, byType] of Object.entries(connections ?? {})) {
+		if (!isSafeObjectProperty(sourceNode)) continue;
+		const nodeConnections: INodeConnections = {};
+		for (const [connectionType, outputs] of Object.entries(byType)) {
+			if (!isSafeObjectProperty(connectionType)) continue;
+			nodeConnections[connectionType] = outputs.map(
+				(outputConnections) =>
+					outputConnections?.flatMap((connection) =>
+						isNodeConnectionType(connection.type) && isSafeObjectProperty(connection.node)
+							? [{ ...connection, type: connection.type }]
+							: [],
+					) ?? null,
+			);
+		}
+		bySourceNode[sourceNode] = nodeConnections;
+	}
+	return bySourceNode;
+}
+
+/**
+ * Maps SDK nodes to the minimal `INode` shape group validation reads:
+ * id/name/type/typeVersion. Parameters are never read, so an empty object is
+ * passed instead of bridging SDK and engine parameter types.
+ */
+export function toGroupValidationNodes(nodes: WorkflowJSON['nodes']): INode[] {
+	return nodes.map((node) => ({
+		id: node.id,
+		name: node.name ?? '',
+		type: node.type,
+		typeVersion: node.typeVersion,
+		position: node.position,
+		parameters: {},
+	}));
+}
+
+/** Drops invalid groups from SDK `WorkflowJSON`, returning their violations. Mutates `json.nodeGroups`. */
+export function dropInvalidWorkflowJsonGroups(
+	json: WorkflowJSON,
+	getNodeType: GetNodeTypeForGrouping | null,
+	shouldDrop?: (violation: WorkflowGroupViolation) => boolean,
+): WorkflowGroupViolation[] {
+	if (!json.nodeGroups?.length) return [];
+
+	const validationWorkflow = {
+		nodes: toGroupValidationNodes(json.nodes ?? []),
+		connections: toEngineConnections(json.connections),
+		nodeGroups: json.nodeGroups,
+	};
+	const violations = dropInvalidWorkflowGroups(validationWorkflow, getNodeType, shouldDrop);
+	json.nodeGroups = validationWorkflow.nodeGroups;
+
+	return violations;
+}

packages/@n8n/workflow-sdk/src/validation/validate-workflow.ts

@@ -1,11 +1,6 @@
 import { isRecord } from '@n8n/utils/is-record';
 import get from 'lodash/get';
-import type {
-	INodeType,
-	INodeTypes,
-	IConnections as N8nIConnections,
-	IDisplayOptions,
-} from 'n8n-workflow';
+import type { INodeType, INodeTypes, IDisplayOptions } from 'n8n-workflow';
 import { mapConnectionsByDestination, NodeVersionNotFoundError } from 'n8n-workflow';
 
 import { matchesDisplayOptions } from './display-options';
@@ -16,6 +11,7 @@ import { resolveMainOutputCount } from './node-port-resolvers/resolve-main-outpu
 import { isStickyNoteType, isHttpRequestType } from '../constants/node-types';
 import type { WorkflowBuilder, WorkflowJSON } from '../types/base';
 import { isTriggerNodeType } from '../utils/trigger-detection';
+import { toEngineConnections } from '../utils/workflow-json-engine-helpers';
 import { containsPlaceholderMarker } from '../workflow-builder/string-utils';
 
 /**
@@ -684,9 +680,7 @@ export function validateWorkflow(
  * drops the third branch at runtime.
  */
 function checkMergeNodeInputCount(json: WorkflowJSON, warnings: ValidationWarning[]): void {
-	const connectionsByDest = mapConnectionsByDestination(
-		json.connections as unknown as N8nIConnections,
-	);
+	const connectionsByDest = mapConnectionsByDestination(toEngineConnections(json.connections));
 
 	for (const node of json.nodes) {
 		if (!node.name) continue;
@@ -787,10 +781,8 @@ function validateSubnodeParameters(
 	}
 
 	// Invert connections to find incoming connections by destination
-	// Cast to n8n-workflow IConnections since our local type has string for connection type
-	const connectionsByDest = mapConnectionsByDestination(
-		json.connections as unknown as N8nIConnections,
-	);
+	// Convert to n8n-workflow IConnections since our local type has string for connection type
+	const connectionsByDest = mapConnectionsByDestination(toEngineConnections(json.connections));
 
 	// Check each node that might be a parent with AI inputs
 	for (const parentNode of json.nodes) {
@@ -928,9 +920,7 @@ function validateParentSupportsInputs(
 		}
 	}
 
-	const connectionsByDest = mapConnectionsByDestination(
-		json.connections as unknown as N8nIConnections,
-	);
+	const connectionsByDest = mapConnectionsByDestination(toEngineConnections(json.connections));
 
 	for (const parentNode of json.nodes) {
 		if (!parentNode.name) continue;
@@ -1006,9 +996,7 @@ function validateRequiredInputsConnected(
 	nodeTypesProvider: INodeTypes,
 	errors: ValidationError[],
 ): void {
-	const connectionsByDest = mapConnectionsByDestination(
-		json.connections as unknown as N8nIConnections,
-	);
+	const connectionsByDest = mapConnectionsByDestination(toEngineConnections(json.connections));
 
 	for (const parentNode of json.nodes) {
 		if (!parentNode.name) continue;

packages/cli/src/__tests__/workflow-helpers.test.ts

@@ -17,7 +17,6 @@ import { VariablesService } from '@/environments.ee/variables/variables.service.
 import { ExecutionPersistence } from '@/executions/execution-persistence';
 import { OwnershipService } from '@/services/ownership.service';
 import {
-	dropInvalidNodeGroups,
 	getLastExecutedNodeData,
 	getLastExecutedNodeRuns,
 	getVariables,
@@ -694,90 +693,6 @@ describe('validateWorkflowNodeGroups', () => {
 	});
 });
 
-describe('dropInvalidNodeGroups', () => {
-	it('leaves a valid workflow untouched and reports nothing', () => {
-		const workflow = {
-			nodes: [makeNode('n1'), makeNode('n2')],
-			nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: ['n1', 'n2'] }],
-		};
-
-		expect(dropInvalidNodeGroups(workflow, null)).toEqual([]);
-		expect(workflow.nodeGroups).toEqual([{ id: 'g1', name: 'Group', nodeIds: ['n1', 'n2'] }]);
-	});
-
-	it('drops every violating group and keeps the valid ones', () => {
-		const workflow = {
-			nodes: [makeNode('n1')],
-			nodeGroups: [
-				{ id: 'g1', name: 'Valid', nodeIds: ['n1'] },
-				{ id: 'g2', name: 'Unknown member', nodeIds: ['n999'] },
-			],
-		};
-
-		const violations = dropInvalidNodeGroups(workflow, null);
-
-		expect(violations).toHaveLength(1);
-		expect(violations[0]).toMatchObject({ groupId: 'g2', code: 'unknown-node-id' });
-		expect(workflow.nodeGroups).toEqual([{ id: 'g1', name: 'Valid', nodeIds: ['n1'] }]);
-	});
-
-	describe('with a shouldDrop predicate', () => {
-		// Two groups sharing n1: the second is flagged for the overlap, and the
-		// first for holding a node that now belongs elsewhere. A caller that can
-		// only blame one of them must be able to drop just that one.
-		const buildOverlapping = () => ({
-			nodes: connectedNodes,
-			connections: chainConnections,
-			nodeGroups: [
-				{ id: 'g1', name: 'First', nodeIds: ['n1', 'n2'] },
-				{ id: 'g2', name: 'Second', nodeIds: ['n1'] },
-			],
-		});
-
-		const regularType = regularNodeType;
-
-		it('drops only the matching groups and reports only those', () => {
-			const workflow = buildOverlapping();
-
-			const violations = dropInvalidNodeGroups(
-				workflow,
-				() => regularType,
-				(violation) => violation.groupId === 'g2',
-			);
-
-			expect(violations).toHaveLength(1);
-			expect(violations[0].groupId).toBe('g2');
-			expect(workflow.nodeGroups).toEqual([{ id: 'g1', name: 'First', nodeIds: ['n1', 'n2'] }]);
-		});
-
-		it('clears the collateral violation once the culprit is gone', () => {
-			const workflow = buildOverlapping();
-			dropInvalidNodeGroups(
-				workflow,
-				() => regularType,
-				(violation) => violation.groupId === 'g2',
-			);
-
-			// Second pass: "First" only ever failed because "Second" overlapped it.
-			expect(dropInvalidNodeGroups(workflow, () => regularType)).toEqual([]);
-			expect(workflow.nodeGroups).toHaveLength(1);
-		});
-
-		it('keeps the workflow untouched when nothing matches', () => {
-			const workflow = buildOverlapping();
-
-			expect(
-				dropInvalidNodeGroups(
-					workflow,
-					() => regularType,
-					() => false,
-				),
-			).toEqual([]);
-			expect(workflow.nodeGroups).toHaveLength(2);
-		});
-	});
-});
-
 describe('sanitizeNodeGroupDescriptions', () => {
 	it('returns no warnings and leaves descriptions within the cap untouched', () => {
 		const workflow = {

packages/cli/src/modules/mcp/tools/workflow-builder/create-workflow-from-code.tool.ts

@@ -35,7 +35,7 @@ import type { AiGatewayService } from '@/services/ai-gateway.service';
 import type { UrlService } from '@/services/url.service';
 import type { Telemetry } from '@/telemetry';
 import {
-	dropInvalidNodeGroups,
+	dropInvalidWorkflowGroups,
 	makeGetNodeTypeForGrouping,
 	resolveNodeWebhookIds,
 } from '@/workflow-helpers';
@@ -323,7 +323,7 @@ export const createCreateWorkflowFromCodeTool = (
 			// own (fatal) check, so an invalid group is dropped and reported instead
 			// of aborting the whole creation.
 			const skippedGroups = options.canvasGroupsEnabled
-				? dropInvalidNodeGroups(newWorkflow, makeGetNodeTypeForGrouping(nodeTypes)).map(
+				? dropInvalidWorkflowGroups(newWorkflow, makeGetNodeTypeForGrouping(nodeTypes)).map(
 						(violation) => ({ groupName: violation.groupName, reason: violation.message }),
 					)
 				: [];

packages/cli/src/modules/mcp/tools/workflow-builder/update-workflow.tool.ts

@@ -17,7 +17,7 @@ import type { TagService } from '@/services/tag.service';
 import type { UrlService } from '@/services/url.service';
 import type { Telemetry } from '@/telemetry';
 import {
-	dropInvalidNodeGroups,
+	dropInvalidWorkflowGroups,
 	makeGetNodeTypeForGrouping,
 	resolveNodeWebhookIds,
 } from '@/workflow-helpers';
@@ -786,7 +786,7 @@ function assertOperationsSupported(
  * are checked once here rather than per operation. A broken group is dropped
  * and reported; the update still goes through.
  *
- * NOT PURE: `dropInvalidNodeGroups` removes the offending groups from
+ * NOT PURE: `dropInvalidWorkflowGroups` removes the offending groups from
  * `result.workflow.nodeGroups` **in place**, and that mutation is what
  * `buildWorkflowUpdateEntity` later persists. `result` must be passed by
  * reference — cloning it makes the dropped groups silently come back.
@@ -814,12 +814,12 @@ function resolveNodeGroupViolations(
 	const getNodeType = makeGetNodeTypeForGrouping(nodeTypes);
 	const violations = canvasGroupsEnabled
 		? [
-				...dropInvalidNodeGroups(
+				...dropInvalidWorkflowGroups(
 					result.workflow,
 					getNodeType,
 					(violation) => result.groupOperations[violation.groupId] !== undefined,
 				),
-				...dropInvalidNodeGroups(result.workflow, getNodeType),
+				...dropInvalidWorkflowGroups(result.workflow, getNodeType),
 			]
 		: [];
 

packages/cli/src/modules/mcp/tools/workflow-builder/validate-workflow-code.tool.ts

@@ -1,13 +1,6 @@
 import type { User } from '@n8n/db';
-import type { WorkflowJSON } from '@n8n/workflow-sdk';
-import {
-	isNodeConnectionType,
-	isSafeObjectProperty,
-	validateWorkflowGroups,
-	type IConnections,
-	type INode,
-	type INodeConnections,
-} from 'n8n-workflow';
+import { toEngineConnections, toGroupValidationNodes } from '@n8n/workflow-sdk';
+import { validateWorkflowGroups } from 'n8n-workflow';
 import z from 'zod';
 
 import type { NodeTypes } from '@/node-types';
@@ -64,38 +57,6 @@ const outputSchema = {
 		),
 } satisfies z.ZodRawShape;
 
-/**
- * Bridges the SDK's connections to `n8n-workflow`'s `IConnections`. The two
- * declare duplicate but formally separate connection types (the SDK types
- * `IConnection.type` as plain `string`), so the values are re-keyed through the
- * `isNodeConnectionType` guard. Serializer output only ever carries known
- * connection types; if an unknown one ever slips through, that connection is
- * skipped here and the save path still rejects the workflow.
- *
- * Node names and connection types come from submitted code, so keys that
- * resolve to object internals (`__proto__`, `constructor`, ...) are skipped —
- * assigning them onto a plain object would mutate its prototype instead of
- * creating an own property, silently corrupting the re-keyed connections.
- */
-function toWorkflowConnections(connections: WorkflowJSON['connections']): IConnections {
-	const bySourceNode: IConnections = {};
-	for (const [sourceNode, byType] of Object.entries(connections ?? {})) {
-		if (!isSafeObjectProperty(sourceNode)) continue;
-		const nodeConnections: INodeConnections = {};
-		for (const [connectionType, outputs] of Object.entries(byType)) {
-			if (!isSafeObjectProperty(connectionType)) continue;
-			nodeConnections[connectionType] = outputs.map(
-				(outputConnections) =>
-					outputConnections?.flatMap((connection) =>
-						isNodeConnectionType(connection.type) ? [{ ...connection, type: connection.type }] : [],
-					) ?? null,
-			);
-		}
-		bySourceNode[sourceNode] = nodeConnections;
-	}
-	return bySourceNode;
-}
-
 /**
  * MCP tool that validates n8n Workflow SDK code.
  * Parses and validates the code, returning the workflow JSON if valid or errors if not.
@@ -152,21 +113,9 @@ export const createValidateWorkflowCodeTool = (
 			// like the ai_tool-source check above, they hard-block saving. Flag off:
 			// output and telemetry are identical to before groups existed.
 			if (options.canvasGroupsEnabled && (result.workflow.nodeGroups?.length ?? 0) > 0) {
-				// The group validator only reads id/name/type (+ typeVersion via
-				// getNodeType); map the SDK's NodeJSON (optional name/parameters)
-				// to the INode shape it expects. Parameters are never read, so an
-				// empty object is passed instead of bridging the parameter types.
-				const groupValidationNodes: INode[] = result.workflow.nodes.map((node) => ({
-					id: node.id,
-					name: node.name ?? '',
-					type: node.type,
-					typeVersion: node.typeVersion,
-					position: node.position,
-					parameters: {},
-				}));
 				const groupsResult = validateWorkflowGroups({
-					nodes: groupValidationNodes,
-					connectionsBySourceNode: toWorkflowConnections(result.workflow.connections),
+					nodes: toGroupValidationNodes(result.workflow.nodes),
+					connectionsBySourceNode: toEngineConnections(result.workflow.connections),
 					nodeGroups: result.workflow.nodeGroups,
 					getNodeType: makeGetNodeTypeForGrouping(nodeTypes),
 				});

packages/cli/src/modules/n8n-packages/engine/n8n-package-parser.ts

@@ -142,7 +142,7 @@ export class N8nPackageParser {
 
 	/** Drops groups that wouldn't survive the save path, so they can't fail the whole import. */
 	private normalizeNodeGroups(entity: WorkflowEntity, path: string): void {
-		const dropped = WorkflowHelpers.dropInvalidNodeGroups(
+		const dropped = WorkflowHelpers.dropInvalidWorkflowGroups(
 			entity,
 			WorkflowHelpers.makeGetNodeTypeForGrouping(this.nodeTypes),
 		);

packages/cli/src/workflow-helpers.ts

@@ -3,9 +3,11 @@ import { CredentialsRepository } from '@n8n/db';
 import type { WorkflowEntity, WorkflowHistory } from '@n8n/db';
 import { Container } from '@n8n/di';
 import {
+	dropInvalidWorkflowGroups,
 	formatWorkflowStructureIssuePath,
 	GROUP_DESCRIPTION_MAX_LENGTH,
 	isSafeObjectProperty,
+	makeGetNodeTypeForGrouping,
 	normalizeGroupDescription,
 	resolveNodeWebhookId,
 	resolveVariables,
@@ -13,16 +15,14 @@ import {
 	summarizeDynamicCredentialsUsage,
 	validateWorkflowGroups,
 	type IDataObject,
-	type INode,
 	type INodeCredentialsDetails,
-	type INodeTypeDescription,
 	type INodeTypes,
 	type IRun,
 	type ITaskData,
 	type IWorkflowBase,
 	type IWorkflowSettings,
 	type RelatedExecution,
-	type WorkflowGroupViolation,
+	type GetNodeTypeForGrouping,
 	type WorkflowStructureIssue,
 } from 'n8n-workflow';
 import { v4 as uuid } from 'uuid';
@@ -33,6 +33,8 @@ import { ExecutionPersistence } from '@/executions/execution-persistence';
 
 import { OwnershipService } from './services/ownership.service';
 
+export { dropInvalidWorkflowGroups, makeGetNodeTypeForGrouping };
+
 /**
  * Validates that pinned data does not exceed size limits.
  * (Backend counterpart of the frontend's `usePinnedData.isValidSize()`).
@@ -146,27 +148,6 @@ export function resolveNodeWebhookIds(workflow: IWorkflowBase, nodeTypes: INodeT
 	}
 }
 
-/**
- * Resolves a node to its type description, or `null` for unknown node types.
- * Used by the grouping validator to detect trigger nodes.
- */
-type GetNodeTypeForGrouping = (node: INode) => INodeTypeDescription | null;
-
-/**
- * Builds the `getNodeType` callback that the grouping validator needs to resolve
- * a node to its type description (used to detect trigger nodes). Returns `null`
- * for unknown node types so validation degrades gracefully rather than throwing.
- */
-export function makeGetNodeTypeForGrouping(nodeTypes: INodeTypes): GetNodeTypeForGrouping {
-	return (node: INode) => {
-		try {
-			return nodeTypes.getByNameAndVersion(node.type, node.typeVersion).description;
-		} catch {
-			return null;
-		}
-	};
-}
-
 /**
  * Validates nodeGroups on the save path, rejecting with a `BadRequestError`.
  *
@@ -197,48 +178,6 @@ export function validateWorkflowNodeGroups(
 	}
 }
 
-/**
- * Non-fatal counterpart of `validateWorkflowNodeGroups`: drops every offending
- * group instead of throwing, returning the violations of those it dropped.
- * Mutates `nodeGroups`. Groups are cosmetic, so the MCP builder tools drop them
- * to keep the rest of the change; the save path still throws.
- *
- * `shouldDrop` filters which violating groups are removed, letting a caller
- * drop the groups it can blame first and re-check the rest afterwards.
- */
-export function dropInvalidNodeGroups(
-	workflow: Pick<IWorkflowBase, 'nodes' | 'nodeGroups'> & {
-		connections?: IWorkflowBase['connections'];
-	},
-	getNodeType: GetNodeTypeForGrouping | null,
-	shouldDrop: (violation: WorkflowGroupViolation) => boolean = () => true,
-): WorkflowGroupViolation[] {
-	if (!workflow.nodeGroups?.length) {
-		return [];
-	}
-
-	const result = validateWorkflowGroups({
-		nodes: workflow.nodes,
-		connectionsBySourceNode: workflow.connections,
-		nodeGroups: workflow.nodeGroups,
-		getNodeType,
-	});
-
-	if (result.valid) {
-		return [];
-	}
-
-	const dropped = result.violations.filter(shouldDrop);
-	if (dropped.length === 0) {
-		return [];
-	}
-
-	const droppedGroupIds = new Set(dropped.map((violation) => violation.groupId));
-	workflow.nodeGroups = workflow.nodeGroups.filter((group) => !droppedGroupIds.has(group.id));
-
-	return dropped;
-}
-
 /**
  * Normalizes group descriptions on import, mutating in place.
  *

packages/workflow/src/node-grouping-validation.ts

@@ -13,6 +13,7 @@ import {
 	type INodeInputConfiguration,
 	type INodeOutputConfiguration,
 	type INodeTypeDescription,
+	type INodeTypes,
 	type IWorkflowGroup,
 	type NodeConnectionType,
 } from './interfaces';
@@ -191,6 +192,8 @@ export type WorkflowGroupViolation = {
 	message: string;
 };
 
+type WorkflowGroupViolationWithGroup = WorkflowGroupViolation & { group: IWorkflowGroup };
+
 export type WorkflowGroupsValidationInput<TNode extends INode = INode> = {
 	nodes: TNode[];
 	connectionsBySourceNode?: IConnections;
@@ -207,6 +210,23 @@ export type WorkflowGroupsValidationResult =
 	| { valid: true }
 	| { valid: false; violations: [WorkflowGroupViolation, ...WorkflowGroupViolation[]] };
 
+export type GetNodeTypeForGrouping = (node: INode) => INodeTypeDescription | null;
+
+/**
+ * Builds the `getNodeType` callback that the grouping validator needs to resolve
+ * a node to its type description. Returns `null` for unknown node types so
+ * validation degrades gracefully rather than throwing.
+ */
+export function makeGetNodeTypeForGrouping(nodeTypes: INodeTypes): GetNodeTypeForGrouping {
+	return (node: INode) => {
+		try {
+			return nodeTypes.getByNameAndVersion(node.type, node.typeVersion).description;
+		} catch {
+			return null;
+		}
+	};
+}
+
 /**
  * Validates a workflow's `nodeGroups` without throwing, collecting all violations.
  * Single source of truth for group rules: persistence (CLI save path) rejects with
@@ -237,9 +257,39 @@ export function validateWorkflowGroups<TNode extends INode>({
 	nodeGroups,
 	getNodeType,
 }: WorkflowGroupsValidationInput<TNode>): WorkflowGroupsValidationResult {
+	const result = validateWorkflowGroupsWithGroupIdentity({
+		nodes,
+		connectionsBySourceNode,
+		nodeGroups,
+		getNodeType,
+	});
+
+	if (result.valid) return { valid: true };
+
+	const [firstViolation, ...restViolations] = result.violations;
+	return {
+		valid: false,
+		violations: [
+			stripWorkflowGroupIdentity(firstViolation),
+			...restViolations.map(stripWorkflowGroupIdentity),
+		],
+	};
+}
+
+function validateWorkflowGroupsWithGroupIdentity<TNode extends INode>({
+	nodes,
+	connectionsBySourceNode,
+	nodeGroups,
+	getNodeType,
+}: WorkflowGroupsValidationInput<TNode>):
+	| { valid: true }
+	| {
+			valid: false;
+			violations: [WorkflowGroupViolationWithGroup, ...WorkflowGroupViolationWithGroup[]];
+	  } {
 	if (!nodeGroups || nodeGroups.length === 0) return { valid: true };
 
-	const violations: WorkflowGroupViolation[] = [];
+	const violations: WorkflowGroupViolationWithGroup[] = [];
 	// Tracked by object identity: duplicate IDs/names make `group.id` ambiguous.
 	const groupsWithBasicViolations = new Set<IWorkflowGroup>();
 	const addViolation = (
@@ -247,7 +297,7 @@ export function validateWorkflowGroups<TNode extends INode>({
 		code: WorkflowGroupViolationCode,
 		message: string,
 	) => {
-		violations.push({ groupId: group.id, groupName: group.name, code, message });
+		violations.push({ group, groupId: group.id, groupName: group.name, code, message });
 	};
 
 	const nodeById = new Map(nodes.filter((node) => Boolean(node.id)).map((node) => [node.id, node]));
@@ -328,6 +378,54 @@ export function validateWorkflowGroups<TNode extends INode>({
 	return { valid: false, violations: [firstViolation, ...restViolations] };
 }
 
+function stripWorkflowGroupIdentity({
+	groupId,
+	groupName,
+	code,
+	message,
+}: WorkflowGroupViolationWithGroup): WorkflowGroupViolation {
+	return { groupId, groupName, code, message };
+}
+
+/**
+ * Non-fatal twin of `validateWorkflowGroups`: drops every offending group instead
+ * of throwing, returning every violation for the groups it dropped.
+ * Mutates `nodeGroups`.
+ *
+ * `shouldDrop` filters which violating groups are removed, letting a caller
+ * drop the groups it can blame first and re-check the rest afterwards.
+ */
+export function dropInvalidWorkflowGroups<TNode extends INode>(
+	workflow: { nodes: TNode[]; nodeGroups?: IWorkflowGroup[]; connections?: IConnections },
+	getNodeType: GetNodeTypeForGrouping | null,
+	shouldDrop: (violation: WorkflowGroupViolation) => boolean = () => true,
+): WorkflowGroupViolation[] {
+	if (!workflow.nodeGroups?.length) {
+		return [];
+	}
+
+	const result = validateWorkflowGroupsWithGroupIdentity({
+		nodes: workflow.nodes,
+		connectionsBySourceNode: workflow.connections,
+		nodeGroups: workflow.nodeGroups,
+		getNodeType,
+	});
+
+	if (result.valid) {
+		return [];
+	}
+
+	const dropped = result.violations.filter(shouldDrop);
+	if (dropped.length === 0) {
+		return [];
+	}
+
+	const droppedGroups = new Set(dropped.map((violation) => violation.group));
+	workflow.nodeGroups = workflow.nodeGroups.filter((group) => !droppedGroups.has(group));
+
+	return dropped.map(stripWorkflowGroupIdentity);
+}
+
 /**
  * Maps a failed `validateNodeSelectionForGrouping` result to an actionable message
  * that names the offending group and the rule it broke. These strings are the

packages/workflow/test/node-grouping-validation.test.ts

@@ -1,5 +1,7 @@
 import {
+	dropInvalidWorkflowGroups,
 	GROUP_DESCRIPTION_MAX_LENGTH,
+	makeGetNodeTypeForGrouping,
 	normalizeGroupDescription,
 	validateNodeSelectionForExtraction,
 	validateNodeSelectionForGrouping,
@@ -11,6 +13,7 @@ import {
 	type IConnections,
 	type INode,
 	type INodeTypeDescription,
+	type INodeTypes,
 } from '../src';
 
 function makeNode(overrides: Partial<INode> = {}): INode {
@@ -917,3 +920,160 @@ describe('validateWorkflowGroups', () => {
 		]);
 	});
 });
+
+describe('makeGetNodeTypeForGrouping', () => {
+	it('returns the description for a known type and null for an unknown one', () => {
+		const description = makeNodeType({ name: 'known.node' });
+		const nodeTypes = {
+			getByNameAndVersion(nodeType: string) {
+				if (nodeType === 'known.node') return { description };
+				throw new Error('Unknown node type');
+			},
+		} as INodeTypes;
+
+		const getNodeType = makeGetNodeTypeForGrouping(nodeTypes);
+
+		expect(getNodeType(makeNode({ type: 'known.node' }))).toBe(description);
+		expect(getNodeType(makeNode({ type: 'unknown.node' }))).toBeNull();
+	});
+});
+
+describe('dropInvalidWorkflowGroups', () => {
+	it('leaves a valid workflow untouched and reports nothing', () => {
+		const graph = makeLinearGraph();
+		const workflow = {
+			nodes: graph.nodes,
+			connections: graph.connections,
+			nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: ['a', 'b'] }],
+		};
+
+		expect(dropInvalidWorkflowGroups(workflow, null)).toEqual([]);
+		expect(workflow.nodeGroups).toEqual([{ id: 'g1', name: 'Group', nodeIds: ['a', 'b'] }]);
+	});
+
+	it('drops every violating group and keeps the valid ones', () => {
+		const graph = makeLinearGraph();
+		const workflow = {
+			nodes: graph.nodes,
+			connections: graph.connections,
+			nodeGroups: [
+				{ id: 'g1', name: 'Valid', nodeIds: ['a', 'b'] },
+				{ id: 'g2', name: 'Unknown member', nodeIds: ['missing'] },
+			],
+		};
+
+		const violations = dropInvalidWorkflowGroups(workflow, null);
+
+		expect(violations).toHaveLength(1);
+		expect(violations[0]).toMatchObject({ groupId: 'g2', code: 'unknown-node-id' });
+		expect(workflow.nodeGroups).toEqual([{ id: 'g1', name: 'Valid', nodeIds: ['a', 'b'] }]);
+	});
+
+	it('returns every violation for dropped groups while dropping each group once', () => {
+		const graph = makeLinearGraph();
+		const workflow = {
+			nodes: graph.nodes,
+			connections: graph.connections,
+			nodeGroups: [
+				{ id: 'g1', name: 'Duplicate', nodeIds: ['a'] },
+				{ id: 'g2', name: 'Duplicate', nodeIds: [] },
+			],
+		};
+
+		const violations = dropInvalidWorkflowGroups(workflow, null);
+
+		expect(violations).toEqual([
+			expect.objectContaining({
+				groupId: 'g2',
+				groupName: 'Duplicate',
+				code: 'duplicate-group-name',
+			}),
+			expect.objectContaining({
+				groupId: 'g2',
+				groupName: 'Duplicate',
+				code: 'empty-group',
+			}),
+		]);
+		expect(workflow.nodeGroups).toEqual([{ id: 'g1', name: 'Duplicate', nodeIds: ['a'] }]);
+	});
+
+	it('drops only the reported group when duplicate IDs make groupId ambiguous', () => {
+		const graph = makeLinearGraph();
+		const workflow = {
+			nodes: graph.nodes,
+			connections: graph.connections,
+			nodeGroups: [
+				{ id: 'dup', name: 'First', nodeIds: ['a'] },
+				{ id: 'dup', name: 'Second', nodeIds: ['b'] },
+			],
+		};
+
+		const violations = dropInvalidWorkflowGroups(workflow, null);
+
+		expect(violations).toEqual([
+			expect.objectContaining({
+				groupId: 'dup',
+				groupName: 'Second',
+				code: 'duplicate-group-id',
+			}),
+		]);
+		expect(workflow.nodeGroups).toEqual([{ id: 'dup', name: 'First', nodeIds: ['a'] }]);
+	});
+
+	describe('with a shouldDrop predicate', () => {
+		// Two groups sharing A: the second is flagged for the overlap, and the
+		// first for holding a node that now belongs elsewhere. A caller that can
+		// only blame one of them must be able to drop just that one.
+		const buildOverlapping = () => {
+			const graph = makeLinearGraph();
+			return {
+				nodes: graph.nodes,
+				connections: graph.connections,
+				nodeGroups: [
+					{ id: 'g1', name: 'First', nodeIds: ['a', 'b'] },
+					{ id: 'g2', name: 'Second', nodeIds: ['a'] },
+				],
+			};
+		};
+
+		it('drops only the matching groups and reports only those', () => {
+			const workflow = buildOverlapping();
+
+			const violations = dropInvalidWorkflowGroups(
+				workflow,
+				() => makeNodeType(),
+				(violation) => violation.groupId === 'g2',
+			);
+
+			expect(violations).toHaveLength(1);
+			expect(violations[0].groupId).toBe('g2');
+			expect(workflow.nodeGroups).toEqual([{ id: 'g1', name: 'First', nodeIds: ['a', 'b'] }]);
+		});
+
+		it('clears the collateral violation once the culprit is gone', () => {
+			const workflow = buildOverlapping();
+			dropInvalidWorkflowGroups(
+				workflow,
+				() => makeNodeType(),
+				(violation) => violation.groupId === 'g2',
+			);
+
+			// Second pass: "First" only ever failed because "Second" overlapped it.
+			expect(dropInvalidWorkflowGroups(workflow, () => makeNodeType())).toEqual([]);
+			expect(workflow.nodeGroups).toHaveLength(1);
+		});
+
+		it('keeps the workflow untouched when nothing matches', () => {
+			const workflow = buildOverlapping();
+
+			expect(
+				dropInvalidWorkflowGroups(
+					workflow,
+					() => makeNodeType(),
+					() => false,
+				),
+			).toEqual([]);
+			expect(workflow.nodeGroups).toHaveLength(2);
+		});
+	});
+});