feat(core): IAI - Drop invalid node groups before save (#36812)

packages/@n8n/instance-ai/skills/workflow-builder/SKILL.md

@@ -603,8 +603,9 @@ first.
 
 `.group(name, members, { description })` on the workflow builder; members are the node handles.
 Read `knowledge-base/reference/node-groups.md` for the exact rules (trigger nodes excluded,
-one connected section, AI sub-nodes stay with their Agent) before creating groups — an invalid
-group is rejected on save. When editing an existing workflow, keep existing `.group(...)` calls
+one connected section, AI sub-nodes stay with their Agent) before creating groups. Agent save
+tools drop an invalid group from the saved workflow and report a warning, so fix the source
+instead of re-emitting it. When editing an existing workflow, keep existing `.group(...)` calls
 and their descriptions intact unless the change is about grouping.
 
 ## Workflow Rules

packages/@n8n/instance-ai/src/tools/__tests__/workflows.tool.test.ts

@@ -29,10 +29,14 @@ vi.mock('../workflows/setup-workflow.service', () => ({
 	buildCompletedReport: vi.fn().mockReturnValue([]),
 }));
 
-// Mock the dynamic import of @n8n/workflow-sdk used by get-as-code
-vi.mock('@n8n/workflow-sdk', () => ({
-	generateWorkflowCode: vi.fn().mockReturnValue('// generated code'),
-}));
+// Mock code generation used by get-as-code while keeping shared SDK helpers real.
+vi.mock('@n8n/workflow-sdk', async (importOriginal) => {
+	const actual = await importOriginal<typeof import('@n8n/workflow-sdk')>();
+	return {
+		...actual,
+		generateWorkflowCode: vi.fn().mockReturnValue('// generated code'),
+	};
+});
 
 const emptyList = { workflows: [], total: 0, totalInScope: 0 };
 
@@ -1234,6 +1238,171 @@ describe('workflows tool', () => {
 			});
 		});
 
+		it('drops invalid node groups before saving and reports coded warnings', async () => {
+			const context = createMockContext({ permissions: { updateWorkflow: 'always_allow' } });
+			(context.workflowService.updateFromWorkflowJSON as Mock).mockResolvedValue({
+				id: 'wf1',
+				versionId: 'v2',
+				checksum: 'checksum-saved',
+			});
+			const workflow = {
+				name: 'Updated WF',
+				nodes: [
+					{
+						id: 'node-1',
+						name: 'Set',
+						type: 'n8n-nodes-base.set',
+						typeVersion: 1,
+						position: [0, 0],
+						parameters: {},
+					},
+				],
+				connections: {},
+				nodeGroups: [
+					{
+						id: 'group-1',
+						name: 'Broken group',
+						nodeIds: ['missing-node', 'another-missing-node'],
+					},
+				],
+			};
+
+			const result = await executeTool<{
+				success: boolean;
+				workflowId?: string;
+				warnings?: string[];
+			}>(
+				createWorkflowsTool(context, 'full'),
+				{ action: 'update', workflowId: 'wf1', workflow },
+				{} as never,
+			);
+
+			expect(result).toMatchObject({ success: true, workflowId: 'wf1' });
+			expect(result.warnings).toHaveLength(1);
+			expect(result.warnings?.join('\n')).toContain('[NODE_GROUP_DROPPED]');
+			expect(result.warnings?.join('\n')).toContain('Broken group');
+			expect(result.warnings?.join('\n')).toContain('missing-node');
+			expect(result.warnings?.join('\n')).toContain('another-missing-node');
+			expect(context.workflowService.updateFromWorkflowJSON).toHaveBeenCalledWith(
+				'wf1',
+				expect.objectContaining({ nodeGroups: [] }),
+			);
+		});
+
+		it('saves valid node groups unchanged and returns no node-group warnings', async () => {
+			const context = createMockContext({ permissions: { updateWorkflow: 'always_allow' } });
+			(context.workflowService.updateFromWorkflowJSON as Mock).mockResolvedValue({
+				id: 'wf1',
+				versionId: 'v2',
+				checksum: 'checksum-saved',
+			});
+			const workflow = {
+				name: 'Updated WF',
+				nodes: [
+					{
+						id: 'a',
+						name: 'A',
+						type: 'n8n-nodes-base.set',
+						typeVersion: 1,
+						position: [0, 0],
+						parameters: {},
+					},
+					{
+						id: 'b',
+						name: 'B',
+						type: 'n8n-nodes-base.set',
+						typeVersion: 1,
+						position: [100, 0],
+						parameters: {},
+					},
+				],
+				connections: { A: { main: [[{ node: 'B', type: 'main', index: 0 }]] } },
+				nodeGroups: [{ id: 'group-1', name: 'Valid group', nodeIds: ['a', 'b'] }],
+			};
+
+			const result = await executeTool<{
+				success: boolean;
+				workflowId?: string;
+				warnings?: string[];
+			}>(
+				createWorkflowsTool(context, 'full'),
+				{ action: 'update', workflowId: 'wf1', workflow },
+				{} as never,
+			);
+
+			expect(result).toMatchObject({ success: true, workflowId: 'wf1' });
+			expect(result.warnings).toBeUndefined();
+			expect(context.workflowService.updateFromWorkflowJSON).toHaveBeenCalledWith('wf1', workflow);
+		});
+
+		it('normalizes duplicate and blank node IDs before node-group validation', async () => {
+			const context = createMockContext({ permissions: { updateWorkflow: 'always_allow' } });
+			(context.workflowService.updateFromWorkflowJSON as Mock).mockResolvedValue({
+				id: 'wf1',
+				versionId: 'v2',
+				checksum: 'checksum-saved',
+			});
+			const workflow = {
+				name: 'Updated WF',
+				nodes: [
+					{
+						id: '',
+						name: 'A',
+						type: 'n8n-nodes-base.set',
+						typeVersion: 1,
+						position: [0, 0],
+						parameters: {},
+					},
+					{
+						id: '',
+						name: 'B',
+						type: 'n8n-nodes-base.set',
+						typeVersion: 1,
+						position: [100, 0],
+						parameters: {},
+					},
+				],
+				connections: { A: { main: [[{ node: 'B', type: 'main', index: 0 }]] } },
+				nodeGroups: [{ id: 'group-1', name: 'Healed group', nodeIds: ['', ''] }],
+			};
+
+			const result = await executeTool<{
+				success: boolean;
+				workflowId?: string;
+				warnings?: string[];
+			}>(
+				createWorkflowsTool(context, 'full'),
+				{ action: 'update', workflowId: 'wf1', workflow },
+				{} as never,
+			);
+
+			const savedWorkflow = (context.workflowService.updateFromWorkflowJSON as Mock).mock
+				.calls[0][1] as typeof workflow;
+			const nodeIds = savedWorkflow.nodes.map((node) => node.id);
+			expect(result).toMatchObject({ success: true, workflowId: 'wf1' });
+			expect(result.warnings).toBeUndefined();
+			expect(nodeIds.every(Boolean)).toBe(true);
+			expect(new Set(nodeIds)).toHaveProperty('size', 2);
+			expect(savedWorkflow.nodeGroups).toEqual([{ id: 'group-1', name: 'Healed group', nodeIds }]);
+		});
+
+		it('returns a tool error when raw update ID normalization cannot read nodes', async () => {
+			const context = createMockContext({ permissions: { updateWorkflow: 'always_allow' } });
+
+			const result = await executeTool(
+				createWorkflowsTool(context, 'full'),
+				{
+					action: 'update',
+					workflowId: 'wf1',
+					workflow: { name: 'Updated WF', nodes: [null], connections: {} },
+				},
+				{} as never,
+			);
+
+			expect(result).toMatchObject({ success: false });
+			expect(context.workflowService.updateFromWorkflowJSON).not.toHaveBeenCalled();
+		});
+
 		it('tells the agent what to do when a user is editing the workflow in the editor', async () => {
 			const context = createMockContext({ permissions: { updateWorkflow: 'always_allow' } });
 			(context.workflowService.updateFromWorkflowJSON as Mock).mockRejectedValue(

packages/@n8n/instance-ai/src/tools/workflows.tool.ts

@@ -5,7 +5,8 @@
  */
 import { Tool } from '@n8n/agents';
 import { isRecord } from '@n8n/utils/is-record';
-import type { WorkflowJSON } from '@n8n/workflow-sdk';
+import { dropInvalidWorkflowJsonGroups, type WorkflowJSON } from '@n8n/workflow-sdk';
+import { makeGetNodeTypeForGrouping } from 'n8n-workflow';
 import { nanoid } from 'nanoid';
 import { z } from 'zod';
 
@@ -52,9 +53,11 @@ import { validateWorkflowConfig } from './workflows/validate-workflow.service';
 import {
 	grantSessionWorkflowUpdate,
 	canSkipWorkflowUpdateHitl,
+	formatWarning,
 } from './workflows/workflow-build-context';
 import { refreshWorkflowSourceFileBindingFromWorkflow } from './workflows/workflow-file-bindings';
-import { getReferencedWorkflowIds } from './workflows/workflow-json-utils';
+import { ensureUniqueNodeIds, getReferencedWorkflowIds } from './workflows/workflow-json-utils';
+import { nodeGroupDroppedWarnings } from './workflows/workflow-validation-warnings';
 
 // ── Action schemas ──────────────────────────────────────────────────────────
 
@@ -1320,6 +1323,13 @@ async function handleUpdate(
 	const expectedChecksum = await getObservedWorkflowChecksum(context, input.workflowId);
 
 	try {
+		ensureUniqueNodeIds(input.workflow);
+		const droppedGroupWarnings = nodeGroupDroppedWarnings(
+			dropInvalidWorkflowJsonGroups(
+				input.workflow,
+				context.nodeTypesProvider ? makeGetNodeTypeForGrouping(context.nodeTypesProvider) : null,
+			),
+		);
 		const saved = expectedChecksum
 			? await context.workflowService.updateFromWorkflowJSON(input.workflowId, input.workflow, {
 					expectedChecksum,
@@ -1331,7 +1341,17 @@ async function handleUpdate(
 		if (saved.checksum) {
 			await rememberObservedWorkflowChecksum(context, input.workflowId, saved.checksum);
 		}
-		return { success: true, workflowId: input.workflowId };
+		return {
+			success: true,
+			workflowId: input.workflowId,
+			...(droppedGroupWarnings.length > 0
+				? {
+						warnings: droppedGroupWarnings.map((warning) =>
+							formatWarning(warning.code, warning.message),
+						),
+					}
+				: {}),
+		};
 	} catch (error) {
 		if (error instanceof WorkflowSaveConflictError) {
 			return {

packages/@n8n/instance-ai/src/tools/workflows/__tests__/build-workflow.tool.test.ts

@@ -28,9 +28,15 @@ vi.mock('../../../tracing/langsmith-tracing', async () => {
 	};
 });
 
-vi.mock('../workflow-validation-warnings', () => ({
-	partitionWarnings: vi.fn((warnings: unknown[]) => ({ blocking: [], informational: warnings })),
-}));
+vi.mock('../workflow-validation-warnings', async () => {
+	const actual = await vi.importActual<typeof import('../workflow-validation-warnings')>(
+		'../workflow-validation-warnings',
+	);
+	return {
+		...actual,
+		partitionWarnings: vi.fn((warnings: unknown[]) => ({ blocking: [], informational: warnings })),
+	};
+});
 
 const generatedWorkflow = {
 	name: 'Generated workflow',
@@ -320,6 +326,59 @@ describe('createBuildWorkflowTool', () => {
 		expect(result.postBuildFlow?.instructions).not.toContain('# Post-Build Flow');
 	});
 
+	it('drops invalid node groups before saving and reports the drop', async () => {
+		const source = 'workflow source from workspace';
+		const { context, filePath, trackTelemetry } = makeContext({ source });
+		vi.mocked(compileWorkflowSource).mockResolvedValueOnce({
+			success: true,
+			workflow: {
+				name: 'Grouped workflow',
+				nodes: [
+					{
+						id: 'node-1',
+						name: 'Set',
+						type: 'n8n-nodes-base.set',
+						typeVersion: 1,
+						position: [0, 0],
+						parameters: {},
+					},
+				],
+				connections: {},
+				nodeGroups: [
+					{
+						id: 'group-1',
+						name: 'Broken group',
+						nodeIds: ['missing-node', 'another-missing-node'],
+					},
+				],
+			},
+			warnings: [],
+			compiler: 'sandbox-tsx',
+		});
+
+		const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
+			filePath,
+		});
+
+		expect(result.success).toBe(true);
+		expect(result.warnings).toHaveLength(1);
+		expect(result.warnings?.join('\n')).toContain('[NODE_GROUP_DROPPED]');
+		expect(result.warnings?.join('\n')).toContain('Broken group');
+		expect(result.warnings?.join('\n')).toContain('missing-node');
+		expect(result.warnings?.join('\n')).toContain('another-missing-node');
+
+		const savedWorkflow = vi.mocked(context.workflowService.createFromWorkflowJSON).mock
+			.calls[0]?.[0];
+		expect(savedWorkflow?.nodeGroups).toEqual([]);
+		expect(trackTelemetry).toHaveBeenCalledWith(
+			'instance_ai_workflow_source_build',
+			expect.objectContaining({
+				dropped_group_count: 1,
+				warning_count: 1,
+			}),
+		);
+	});
+
 	it('falls back to the post-build-flow handoff for a triggerless one-off build', async () => {
 		const source = 'workflow source from workspace';
 		const { context, filePath } = makeContext({ source });

packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts

@@ -4,7 +4,12 @@ import {
 	instanceAiConfirmationSeveritySchema,
 } from '@n8n/api-types';
 import { hasPlaceholderDeep } from '@n8n/utils/placeholder';
-import { SDK_IMPORTABLE_FUNCTIONS, type WorkflowJSON } from '@n8n/workflow-sdk';
+import {
+	dropInvalidWorkflowJsonGroups,
+	SDK_IMPORTABLE_FUNCTIONS,
+	type WorkflowJSON,
+} from '@n8n/workflow-sdk';
+import { makeGetNodeTypeForGrouping } from 'n8n-workflow';
 import { nanoid } from 'nanoid';
 import { readFileSync } from 'node:fs';
 import { join } from 'node:path';
@@ -71,7 +76,11 @@ import {
 } from './workflow-json-utils';
 import { computeChangedNodeNames, downgradeUnchangedNodeBlockers } from './workflow-node-diff';
 import { compileWorkflowSource } from './workflow-source-compiler';
-import { partitionWarnings, type ValidationWarning } from './workflow-validation-warnings';
+import {
+	nodeGroupDroppedWarnings,
+	partitionWarnings,
+	type ValidationWarning,
+} from './workflow-validation-warnings';
 import { WorkflowSaveConflictError } from '../../errors/workflow-save-conflict.error';
 import { INSTANCE_AI_SKILLS_DIR } from '../../skills/runtime-skills';
 import { emitTraceOnlyChildRun } from '../../tracing/langsmith-tracing';
@@ -1012,6 +1021,7 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
 			await stripStaleCredentialsFromWorkflow(context, json);
 
 			try {
+				let droppedGroupCount = 0;
 				// Runs first: the passes below key off node ids, so they must be unique.
 				ensureUniqueNodeIds(json);
 				// Recovers the saved id of a surviving node whose source declared none — layered
@@ -1021,6 +1031,17 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
 				await ensureWebhookIds(json, targetWorkflowId, context);
 				await preserveExistingNodeGroupIds(json, targetWorkflowId, context);
 				await preserveExistingNodePositions(json, targetWorkflowId, context);
+				const groupCountBeforeDrop = json.nodeGroups?.length ?? 0;
+				const droppedGroupWarnings = nodeGroupDroppedWarnings(
+					dropInvalidWorkflowJsonGroups(
+						json,
+						context.nodeTypesProvider
+							? makeGetNodeTypeForGrouping(context.nodeTypesProvider)
+							: null,
+					),
+				);
+				droppedGroupCount = groupCountBeforeDrop - (json.nodeGroups?.length ?? 0);
+				informational.push(...droppedGroupWarnings);
 
 				if (await hasLostAllSavedNodeIds(json, targetWorkflowId, context)) {
 					context.logger.debug('Build kept none of the saved node ids', {
@@ -1202,6 +1223,7 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
 						isSupportingWorkflow,
 						isAuxiliarySupportingWorkflow,
 						warningCount: informational.length,
+						droppedGroupCount,
 					});
 
 					return {

packages/@n8n/instance-ai/src/tools/workflows/workflow-build-telemetry.ts

@@ -27,6 +27,7 @@ export function trackWorkflowSourceBuild(
 		remediation?: RemediationMetadata;
 		errorCount?: number;
 		warningCount?: number;
+		droppedGroupCount?: number;
 	},
 ): void {
 	const buildContext = context.workflowBuildContext;
@@ -44,6 +45,7 @@ export function trackWorkflowSourceBuild(
 		is_auxiliary_supporting_workflow: input.isAuxiliarySupportingWorkflow === true,
 		error_count: input.errorCount ?? 0,
 		warning_count: input.warningCount ?? 0,
+		dropped_group_count: input.droppedGroupCount ?? 0,
 		...(input.targetWorkflowId ? { target_workflow_id: input.targetWorkflowId } : {}),
 		...(input.savedWorkflowId ? { workflow_id: input.savedWorkflowId } : {}),
 		...(input.binding.sourceHash ? { source_hash: input.binding.sourceHash } : {}),

packages/@n8n/instance-ai/src/tools/workflows/workflow-validation-warnings.ts

@@ -1,4 +1,7 @@
 import { partitionValidationIssues, type IssueSeverity } from '@n8n/workflow-sdk';
+import type { WorkflowGroupViolation } from 'n8n-workflow';
+
+export const NODE_GROUP_DROPPED_CODE = 'NODE_GROUP_DROPPED';
 
 export interface ValidationWarning {
 	code: string;
@@ -36,3 +39,35 @@ export function partitionWarnings(warnings: ValidationWarning[]): {
 	// {@link partitionValidationIssues}.
 	return partitionValidationIssues(warnings);
 }
+
+export function nodeGroupDroppedWarnings(
+	violations: WorkflowGroupViolation[],
+): ValidationWarning[] {
+	const violationsByGroup = new Map<string, WorkflowGroupViolation[]>();
+	for (const violation of violations) {
+		const key = JSON.stringify([violation.groupId, violation.groupName]);
+		const groupViolations = violationsByGroup.get(key);
+		if (groupViolations) {
+			groupViolations.push(violation);
+		} else {
+			violationsByGroup.set(key, [violation]);
+		}
+	}
+
+	const warnings: ValidationWarning[] = [];
+	for (const groupViolations of violationsByGroup.values()) {
+		const firstViolation = groupViolations[0];
+		if (!firstViolation) continue;
+		const messages = groupViolations.map(({ message }) => message);
+		warnings.push(formatNodeGroupDroppedWarning(firstViolation.groupName, messages));
+	}
+	return warnings;
+}
+
+function formatNodeGroupDroppedWarning(groupName: string, messages: string[]): ValidationWarning {
+	return {
+		code: NODE_GROUP_DROPPED_CODE,
+		severity: 'informational',
+		message: `Node group "${groupName}" was removed from the saved workflow: ${messages.join(' ')}`,
+	};
+}

packages/@n8n/workflow-sdk/src/prompts/sdk-reference/sdk-language.test.ts

@@ -114,6 +114,12 @@ describe('NODE_GROUPS_REFERENCE', () => {
 		expect(NODE_GROUPS_REFERENCE).toMatch(/keep the .+ and their descriptions\s+intact/is);
 	});
 
+	it('tells agents invalid groups are dropped with warnings and source should be fixed', () => {
+		expect(NODE_GROUPS_REFERENCE).toMatch(/drop an invalid group.+report a warning/is);
+		expect(NODE_GROUPS_REFERENCE).toMatch(/fix the source.+not re-emitted/is);
+		expect(NODE_GROUPS_REFERENCE).not.toContain('rejected on save');
+	});
+
 	it('states the single entry/exit boundary rule that grouping enforces', () => {
 		// reason: 'invalid-subgraph' — grouping rejects a group with more than one
 		// incoming or outgoing main connection (single entry/exit *boundary*).

packages/@n8n/workflow-sdk/src/prompts/sdk-reference/sdk-language.ts

@@ -106,8 +106,9 @@ sentence — anything past ${GROUP_DESCRIPTION_MAX_LENGTH} characters is cut off
 When editing an existing workflow, **keep the \`.group(...)\` calls and their descriptions
 intact** unless the change is specifically about grouping.
 
-An invalid group is rejected on save, so these following rules MUST be followed when
-creating or editing groups.
+Agent save tools drop an invalid group from the saved workflow and report a warning.
+Fix the source so the invalid group is not re-emitted. These rules MUST be followed
+when creating or editing groups.
 
 Rules:
 ${renderRulesLines()}