diff --git a/packages/@n8n/ai-workflow-builder.ee/src/code-builder/handlers/parse-validate-handler.ts b/packages/@n8n/ai-workflow-builder.ee/src/code-builder/handlers/parse-validate-handler.ts index a274c87150f..5df36b4b9cc 100644 --- a/packages/@n8n/ai-workflow-builder.ee/src/code-builder/handlers/parse-validate-handler.ts +++ b/packages/@n8n/ai-workflow-builder.ee/src/code-builder/handlers/parse-validate-handler.ts @@ -247,8 +247,14 @@ export class ParseValidateHandler { builder.generatePinData({ beforeWorkflow: currentWorkflow }); } + // Preserve IDs of groups that already exist (by name) so editing a workflow doesn't + // skew the diff; new groups fall back to a deterministic ID. + const existingGroupIdsByName = new Map( + (currentWorkflow?.nodeGroups ?? []).map((group) => [group.name, group.id]), + ); + // Convert to JSON with Dagre layout matching the FE's tidy-up - const workflowJson: WorkflowJSON = builder.toJSON({ tidyUp: true }); + const workflowJson: WorkflowJSON = builder.toJSON({ tidyUp: true, existingGroupIdsByName }); this.logger?.debug('Parsed workflow', { id: workflowJson.id, diff --git a/packages/@n8n/ai-workflow-builder.ee/src/code-builder/handlers/test/parse-validate-handler.test.ts b/packages/@n8n/ai-workflow-builder.ee/src/code-builder/handlers/test/parse-validate-handler.test.ts index fdbe1f74c1b..0ef7fcc78d5 100644 --- a/packages/@n8n/ai-workflow-builder.ee/src/code-builder/handlers/test/parse-validate-handler.test.ts +++ b/packages/@n8n/ai-workflow-builder.ee/src/code-builder/handlers/test/parse-validate-handler.test.ts @@ -276,6 +276,39 @@ describe('ParseValidateHandler', () => { expect(mockBuilder.regenerateNodeIds).toHaveBeenCalledWith(new Map()); }); + it('should preserve existing group IDs (matched by name) when serializing', async () => { + const currentWorkflow = { + id: 'current', + name: 'Current', + nodes: [], + connections: {}, + nodeGroups: [ + { id: 'group-random-1', name: 'Ingestion', nodeIds: [] }, + { id: 'group-random-2', name: 'Processing', nodeIds: [] }, + ], + } as unknown as WorkflowJSON; + + const mockBuilder = { + regenerateNodeIds: vi.fn(), + validate: vi.fn().mockReturnValue({ valid: true, errors: [], warnings: [] }), + generatePinData: vi.fn(), + toJSON: vi.fn().mockReturnValue({ id: 'test', name: 'Test', nodes: [], connections: {} }), + }; + + mockParseWorkflowCodeToBuilder.mockReturnValue(mockBuilder); + mockValidateWorkflow.mockReturnValue({ valid: true, errors: [], warnings: [] }); + + await handler.parseAndValidate('code', currentWorkflow); + + expect(mockBuilder.toJSON).toHaveBeenCalledWith({ + tidyUp: true, + existingGroupIdsByName: new Map([ + ['Ingestion', 'group-random-1'], + ['Processing', 'group-random-2'], + ]), + }); + }); + it('should throw on parse error', async () => { mockParseWorkflowCodeToBuilder.mockImplementation(() => { throw new Error('Syntax error at line 5'); diff --git a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-json-utils.test.ts b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-json-utils.test.ts index 668b03d1649..0c2e712f30a 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-json-utils.test.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-json-utils.test.ts @@ -1,7 +1,7 @@ import type { WorkflowJSON } from '@n8n/workflow-sdk'; import type { InstanceAiContext } from '../../../types'; -import { ensureWebhookIds } from '../workflow-json-utils'; +import { ensureWebhookIds, preserveExistingNodeGroupIds } from '../workflow-json-utils'; describe('ensureWebhookIds', () => { it('fails updates when existing webhook IDs cannot be loaded', async () => { @@ -31,3 +31,72 @@ describe('ensureWebhookIds', () => { expect(workflow.nodes[0]?.webhookId).toBeUndefined(); }); }); + +describe('preserveExistingNodeGroupIds', () => { + const buildWorkflow = (groupId: string): WorkflowJSON => ({ + name: 'Grouped workflow', + nodes: [], + connections: {}, + nodeGroups: [{ id: groupId, name: 'Group 1', nodeIds: ['node-a'] }], + }); + + const contextWithExisting = (nodeGroups: Array<{ id: string; name: string }>) => + ({ + workflowService: { + getAsWorkflowJSON: vi.fn().mockResolvedValue({ + name: 'Grouped workflow', + nodes: [], + connections: {}, + nodeGroups: nodeGroups.map((g) => ({ ...g, nodeIds: [] })), + }), + }, + }) as unknown as InstanceAiContext; + + it('reuses the existing group id when the group name matches', async () => { + const workflow = buildWorkflow('deterministic-id'); + + await preserveExistingNodeGroupIds( + workflow, + 'wf-1', + contextWithExisting([{ id: 'editor-id', name: 'Group 1' }]), + ); + + expect(workflow.nodeGroups?.[0]?.id).toBe('editor-id'); + }); + + it('keeps the generated id for a group not present in the existing workflow', async () => { + const workflow = buildWorkflow('deterministic-id'); + + await preserveExistingNodeGroupIds( + workflow, + 'wf-1', + contextWithExisting([{ id: 'other-id', name: 'Some Other Group' }]), + ); + + expect(workflow.nodeGroups?.[0]?.id).toBe('deterministic-id'); + }); + + it('does not fetch or modify groups for new workflows (no workflowId)', async () => { + const workflow = buildWorkflow('deterministic-id'); + const getAsWorkflowJSON = vi.fn(); + const context = { workflowService: { getAsWorkflowJSON } } as unknown as InstanceAiContext; + + await preserveExistingNodeGroupIds(workflow, undefined, context); + + expect(getAsWorkflowJSON).not.toHaveBeenCalled(); + expect(workflow.nodeGroups?.[0]?.id).toBe('deterministic-id'); + }); + + it('fails updates when the existing workflow cannot be loaded', async () => { + const workflow = buildWorkflow('deterministic-id'); + const context = { + workflowService: { + getAsWorkflowJSON: vi.fn().mockRejectedValue(new Error('Workflow not found')), + }, + } as unknown as InstanceAiContext; + + await expect(preserveExistingNodeGroupIds(workflow, 'wf-1', context)).rejects.toThrow( + 'Failed to load existing workflow wf-1 to preserve node-group IDs: Workflow not found', + ); + }); +}); diff --git a/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts b/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts index e6b0fd9fd0e..6cf0583cac8 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts @@ -39,6 +39,7 @@ import { ensureWebhookIds, getReferencedWorkflowIds, isTriggerNodeType, + preserveExistingNodeGroupIds, } from './workflow-json-utils'; import { compileWorkflowSource } from './workflow-source-compiler'; import { partitionWarnings, type ValidationWarning } from './workflow-validation-warnings'; @@ -521,6 +522,7 @@ export function createBuildWorkflowTool(context: InstanceAiContext) { try { await ensureWebhookIds(json, targetWorkflowId, context); + await preserveExistingNodeGroupIds(json, targetWorkflowId, context); const hasMockedCredentialNodes = mockResult.mockedNodeNames.length > 0; const referencedWorkflowIds = getReferencedWorkflowIds(json); diff --git a/packages/@n8n/instance-ai/src/tools/workflows/workflow-json-utils.ts b/packages/@n8n/instance-ai/src/tools/workflows/workflow-json-utils.ts index ba7df68ad45..a79df2179d8 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/workflow-json-utils.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/workflow-json-utils.ts @@ -95,3 +95,38 @@ export async function ensureWebhookIds( } } } + +/** + * For updates, preserve existing node-group IDs by group name. The sandbox SDK + * build has no view of the saved workflow, so toJSON() mints a fresh deterministic + * ID for every group — overwriting the stable ID of a group the user created in + * the editor. Reconciling by name here keeps it stable, mirroring ensureWebhookIds. + */ +export async function preserveExistingNodeGroupIds( + json: WorkflowJSON, + workflowId: string | undefined, + ctx: InstanceAiContext, +): Promise { + if (!workflowId || !json.nodeGroups?.length) return; + + let existingGroupIdsByName: Map; + try { + const existing = await ctx.workflowService.getAsWorkflowJSON(workflowId); + existingGroupIdsByName = new Map( + (existing.nodeGroups ?? []).map((group): [string, string] => [group.name, group.id]), + ); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error( + `Failed to load existing workflow ${workflowId} to preserve node-group IDs: ${message}`, + { cause: error }, + ); + } + + for (const group of json.nodeGroups) { + const existingId = existingGroupIdsByName.get(group.name); + if (existingId) { + group.id = existingId; + } + } +} diff --git a/packages/@n8n/workflow-sdk/src/ast-interpreter/validators.ts b/packages/@n8n/workflow-sdk/src/ast-interpreter/validators.ts index de4f8452283..17a4eee36d5 100644 --- a/packages/@n8n/workflow-sdk/src/ast-interpreter/validators.ts +++ b/packages/@n8n/workflow-sdk/src/ast-interpreter/validators.ts @@ -82,6 +82,7 @@ export const SDK_METHODS: readonly SdkMethodSpec[] = [ // Workflow builder methods { name: 'add', group: 'workflow', public: true }, { name: 'to', group: 'workflow', public: true }, + { name: 'group', group: 'workflow', public: true }, // Node builder methods { name: 'input', group: 'node', public: true }, diff --git a/packages/@n8n/workflow-sdk/src/codegen/code-generator.ts b/packages/@n8n/workflow-sdk/src/codegen/code-generator.ts index eeb928218ab..f7113ff61c3 100644 --- a/packages/@n8n/workflow-sdk/src/codegen/code-generator.ts +++ b/packages/@n8n/workflow-sdk/src/codegen/code-generator.ts @@ -1446,6 +1446,22 @@ export function generateCode( } } + // Emit node group definitions. Members are referenced by node variable (the declared + // `const`), so a workflow → code → build round-trip preserves groups. + if (json.nodeGroups && json.nodeGroups.length > 0) { + const idToNodeName = new Map(); + for (const node of json.nodes) { + if (node.name !== undefined) idToNodeName.set(node.id, node.name); + } + for (const group of json.nodeGroups) { + const memberVars = group.nodeIds.flatMap((id) => { + const nodeName = idToNodeName.get(id); + return nodeName !== undefined ? [getVarName(nodeName, ctx)] : []; + }); + workflowCalls.push(` .group('${escapeString(group.name)}', [${memberVars.join(', ')}])`); + } + } + // Add workflow calls and return statement lines.push(''); diff --git a/packages/@n8n/workflow-sdk/src/codegen/codegen-roundtrip.test.ts b/packages/@n8n/workflow-sdk/src/codegen/codegen-roundtrip.test.ts index f425161640b..17e26cba9ba 100644 --- a/packages/@n8n/workflow-sdk/src/codegen/codegen-roundtrip.test.ts +++ b/packages/@n8n/workflow-sdk/src/codegen/codegen-roundtrip.test.ts @@ -2748,6 +2748,22 @@ describe('Codegen Roundtrip with Real Workflows', () => { expect(parsedJson.settings).toEqual(json.settings); } + // Verify node groups survive the roundtrip. Node and group ids are + // regenerated deterministically on parse, so compare each group by name + // and the names of its member nodes rather than by raw id. + if (json.nodeGroups && json.nodeGroups.length > 0) { + const groupsByMemberName = (wf: WorkflowJSON) => { + const nameById = new Map(wf.nodes.map((n) => [n.id, n.name])); + return (wf.nodeGroups ?? []) + .map((g) => ({ + name: g.name, + members: g.nodeIds.flatMap((id) => nameById.get(id) ?? []).sort(), + })) + .sort((a, b) => a.name.localeCompare(b.name)); + }; + expect(groupsByMemberName(parsedJson)).toEqual(groupsByMemberName(json)); + } + // Filter connections from non-existent nodes (orphaned connections in original workflow) const validNodeNames = new Set( json.nodes.map((n) => n.name).filter((name): name is string => !!name), diff --git a/packages/@n8n/workflow-sdk/src/node-groups.test.ts b/packages/@n8n/workflow-sdk/src/node-groups.test.ts new file mode 100644 index 00000000000..0d893e2c361 --- /dev/null +++ b/packages/@n8n/workflow-sdk/src/node-groups.test.ts @@ -0,0 +1,287 @@ +import { generateWorkflowCode } from './codegen'; +import { parseWorkflowCodeToBuilder } from './codegen/parse-workflow-code'; +import type { WorkflowJSON } from './types/base'; +import { workflow } from './workflow-builder'; +import { node, trigger } from './workflow-builder/node-builders/node-builder'; +import { generateDeterministicGroupId } from './workflow-builder/string-utils'; + +const WF_ID = 'wf-groups-1'; + +function buildGroupedWorkflow() { + const start = trigger({ + type: 'n8n-nodes-base.manualTrigger', + version: 1, + config: { name: 'Start' }, + }); + const fetchNode = node({ + type: 'n8n-nodes-base.httpRequest', + version: 4.2, + config: { name: 'Fetch data' }, + }); + const transform = node({ + type: 'n8n-nodes-base.set', + version: 3, + config: { name: 'Transform' }, + }); + + return workflow(WF_ID, 'Grouped workflow') + .add(start) + .to(fetchNode) + .to(transform) + .group('Ingestion', [fetchNode, transform]); +} + +describe('SDK node groups', () => { + describe('builder.group() → toJSON()', () => { + it('emits nodeGroups with a deterministic id and member ids resolved from names', () => { + const json = buildGroupedWorkflow().toJSON(); + + expect(json.nodeGroups).toHaveLength(1); + const group = json.nodeGroups![0]; + expect(group.name).toBe('Ingestion'); + expect(group.id).toBe(generateDeterministicGroupId(WF_ID, 'Ingestion')); + + const idByName = new Map(json.nodes.map((n) => [n.name, n.id])); + expect(group.nodeIds).toEqual([idByName.get('Fetch data'), idByName.get('Transform')]); + }); + + it('drops members that do not resolve to a node in the workflow', () => { + const start = trigger({ + type: 'n8n-nodes-base.manualTrigger', + version: 1, + config: { name: 'Start' }, + }); + const a = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'A' } }); + // `orphan` is never added to the workflow, so it resolves to nothing and is dropped. + const orphan = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'Orphan' } }); + + const json = workflow(WF_ID, 'wf').add(start).to(a).group('G', [a, orphan]).toJSON(); + + const idByName = new Map(json.nodes.map((n) => [n.name, n.id])); + expect(json.nodeGroups![0].nodeIds).toEqual([idByName.get('A')]); + }); + + it('omits nodeGroups entirely when no group was declared', () => { + const start = trigger({ + type: 'n8n-nodes-base.manualTrigger', + version: 1, + config: { name: 'Start' }, + }); + const json = workflow(WF_ID, 'wf').add(start).toJSON(); + expect(json.nodeGroups).toBeUndefined(); + }); + }); + + describe('regenerateNodeIds()', () => { + it('keeps group member ids consistent with the regenerated node ids (never dangling)', () => { + const builder = buildGroupedWorkflow(); + builder.regenerateNodeIds(); + const json = builder.toJSON(); + + const emittedIds = new Set(json.nodes.map((n) => n.id)); + expect(json.nodeGroups![0].nodeIds.length).toBe(2); + for (const id of json.nodeGroups![0].nodeIds) { + expect(emittedIds.has(id)).toBe(true); + } + }); + + it('resolves a grouped auto-renamed duplicate handle to the right node after regeneration', () => { + const start = trigger({ + type: 'n8n-nodes-base.manualTrigger', + version: 1, + config: { name: 'Start' }, + }); + // Two nodes named "A": the builder auto-renames the second to "A 1". + const first = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'A' } }); + const second = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'A' } }); + + const builder = workflow(WF_ID, 'wf').add(start).to(first).to(second).group('G', [second]); // group the SECOND duplicate by handle + builder.regenerateNodeIds(); + const json = builder.toJSON(); + + const renamed = json.nodes.find((n) => n.name === 'A 1'); + expect(renamed).toBeDefined(); + // The group must reference the renamed duplicate, not the first "A". + expect(json.nodeGroups![0].nodeIds).toEqual([renamed!.id]); + }); + }); + + describe('fromJSON() → toJSON() round-trip', () => { + it('preserves the group id and members across a round-trip', () => { + const source: WorkflowJSON = { + id: WF_ID, + name: 'wf', + nodes: [ + { + id: 'id-a', + name: 'A', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [0, 0], + parameters: {}, + }, + { + id: 'id-b', + name: 'B', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [10, 0], + parameters: {}, + }, + ], + connections: { A: { main: [[{ node: 'B', type: 'main', index: 0 }]] } }, + // A UI-assigned (non-deterministic) id, as the canvas creates. + nodeGroups: [{ id: 'random-uuid', name: 'G', nodeIds: ['id-a', 'id-b'] }], + }; + + const json = workflow.fromJSON(source).toJSON(); + + // Compare the whole group so a dropped/changed field (e.g. id) is caught. + // The source id is carried through, not re-derived — round-trip is lossless. + expect(json.nodeGroups).toEqual([ + { id: 'random-uuid', name: 'G', nodeIds: ['id-a', 'id-b'] }, + ]); + }); + + it('drops a group member whose id has no matching node', () => { + const source: WorkflowJSON = { + id: WF_ID, + name: 'wf', + nodes: [ + { + id: 'id-a', + name: 'A', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + // `ghost-id` references no node, so it is dropped on import. + nodeGroups: [{ id: 'random-uuid', name: 'G', nodeIds: ['id-a', 'ghost-id'] }], + }; + + const json = workflow.fromJSON(source).toJSON(); + + expect(json.nodeGroups![0].nodeIds).toEqual(['id-a']); + }); + + it('leaves nodeGroups undefined when the source declares none', () => { + const source: WorkflowJSON = { + id: WF_ID, + name: 'wf', + nodes: [ + { + id: 'id-a', + name: 'A', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + }; + + const json = workflow.fromJSON(source).toJSON(); + + expect(json.nodeGroups).toBeUndefined(); + }); + }); + + describe('deterministic group id', () => { + it('produces the same id for the same name and a different id for a different workflow', () => { + expect(generateDeterministicGroupId(WF_ID, 'G')).toBe( + generateDeterministicGroupId(WF_ID, 'G'), + ); + expect(generateDeterministicGroupId(WF_ID, 'G')).not.toBe( + generateDeterministicGroupId('other-wf', 'G'), + ); + }); + + it('never collides with a node id derived from the same inputs', () => { + const json = buildGroupedWorkflow().toJSON(); + const nodeIds = new Set(json.nodes.map((n) => n.id)); + expect(nodeIds.has(json.nodeGroups![0].id)).toBe(false); + }); + }); + + describe('existingGroupIdsByName (toJSON option)', () => { + it('reuses an existing group id matched by name and derives one for a new group', () => { + const start = trigger({ + type: 'n8n-nodes-base.manualTrigger', + version: 1, + config: { name: 'Start' }, + }); + const a = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'A' } }); + const b = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'B' } }); + + const json = workflow(WF_ID, 'wf') + .add(start) + .to(a) + .to(b) + .group('Existing', [a]) + .group('New', [b]) + // 'Existing' matches a (random) UI-assigned id; 'New' has no match. + .toJSON({ existingGroupIdsByName: new Map([['Existing', 'ui-random-id']]) }); + + const idByName = new Map(json.nodes.map((n) => [n.name, n.id])); + expect(json.nodeGroups).toEqual([ + { id: 'ui-random-id', name: 'Existing', nodeIds: [idByName.get('A')] }, + { + id: generateDeterministicGroupId(WF_ID, 'New'), + name: 'New', + nodeIds: [idByName.get('B')], + }, + ]); + }); + + it('lets a group carried from fromJSON keep its id over a name match', () => { + const source: WorkflowJSON = { + id: WF_ID, + name: 'wf', + nodes: [ + { + id: 'id-a', + name: 'A', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + nodeGroups: [{ id: 'source-id', name: 'G', nodeIds: ['id-a'] }], + }; + + // The carried source id is authoritative and wins over the name-keyed map. + const json = workflow + .fromJSON(source) + .toJSON({ existingGroupIdsByName: new Map([['G', 'other-id']]) }); + + expect(json.nodeGroups![0].id).toBe('source-id'); + }); + }); + + describe('code round-trip (generate → parse → build)', () => { + it('emits a .group(...) call and preserves the group through a re-parse', () => { + const builder = buildGroupedWorkflow(); + builder.regenerateNodeIds(); + const json1 = builder.toJSON(); + + const code = generateWorkflowCode(json1); + expect(code).toContain(".group('Ingestion', [fetch_data, transform])"); + + const rebuilt = parseWorkflowCodeToBuilder(code); + rebuilt.regenerateNodeIds(); + const json2 = rebuilt.toJSON(); + + expect(json2.nodeGroups).toHaveLength(1); + expect(json2.nodeGroups![0].name).toBe('Ingestion'); + // Deterministic ids mean the group survives identically across the round-trip. + expect(json2.nodeGroups![0].id).toBe(json1.nodeGroups![0].id); + expect(json2.nodeGroups![0].nodeIds).toEqual(json1.nodeGroups![0].nodeIds); + }); + }); +}); diff --git a/packages/@n8n/workflow-sdk/src/prompts/sdk-reference/sdk-language.ts b/packages/@n8n/workflow-sdk/src/prompts/sdk-reference/sdk-language.ts index 2bc0e73dce2..96af86cf045 100644 --- a/packages/@n8n/workflow-sdk/src/prompts/sdk-reference/sdk-language.ts +++ b/packages/@n8n/workflow-sdk/src/prompts/sdk-reference/sdk-language.ts @@ -66,6 +66,26 @@ ${renderMethodLines()} ${SAFE_METHODS_SENTENCE} +## Node groups + +A node group is a named, visual grouping of nodes (a frame on the canvas). +Declare one with \`.group(name, members)\` on the workflow. Members are the node +handles (the \`const\` from \`node(...)\`) — the same way connections reference nodes: + +\`\`\`typescript +const fetch = node({ /* ... name: 'Fetch data' */ }); +const transform = node({ /* ... name: 'Transform' */ }); +export default workflow('id', 'My workflow') + .add(fetch) + .to(transform) + .group('Ingestion', [fetch, transform]); +\`\`\` + +When editing an existing workflow, **keep the \`.group(...)\` calls intact** unless +the change is specifically about grouping. Group members must form a single +connected section of the graph and cannot be trigger nodes; an invalid group is +rejected on save. + ## Forbidden constructs ${renderForbiddenLines()} diff --git a/packages/@n8n/workflow-sdk/src/types/base.ts b/packages/@n8n/workflow-sdk/src/types/base.ts index 467d299b487..c83b0585ee6 100644 --- a/packages/@n8n/workflow-sdk/src/types/base.ts +++ b/packages/@n8n/workflow-sdk/src/types/base.ts @@ -4,9 +4,13 @@ * Core types for building n8n workflows programmatically. */ +import type { IWorkflowGroup } from 'n8n-workflow'; + import type { ValidationOptions, ValidationResult } from '../validation/index'; import type { PluginRegistry } from '../workflow-builder/plugins/registry'; +export type { IWorkflowGroup }; + // ============================================================================= // Data Types // ============================================================================= @@ -330,6 +334,13 @@ export interface WorkflowJSON { templateId?: string; instanceId?: string; }; + /** + * Node groups, referencing their members by node ID. Internally the SDK carries + * group members by node *handle* (the value from `node(...)`) and resolves them to + * the emitted node IDs here at the JSON boundary, so groups survive + * `regenerateNodeIds()` like connections do. + */ + nodeGroups?: IWorkflowGroup[]; } // ============================================================================= @@ -996,11 +1007,23 @@ export interface GeneratePinDataOptions { export interface ToJSONOptions { /** Use Dagre-based layout matching the FE's tidy-up algorithm. Defaults to false (BFS layout). */ tidyUp?: boolean; + /** + * Reuse existing group IDs (keyed by group name) instead of deriving deterministic ones. + * Lets an edit of an existing workflow keep its (UI-assigned, random) group IDs so the diff + * isn't skewed; groups without a match fall back to a deterministic ID. + */ + existingGroupIdsByName?: Map; } /** * Workflow builder for constructing workflows with a fluent API */ +/** + * A reference to a node when defining a group: a node handle — the value + * returned by `node(...)`. Trigger nodes cannot be grouped. + */ +export type GroupMember = NodeInstance; + export interface WorkflowBuilder { readonly id: string; readonly name: string; @@ -1073,6 +1096,25 @@ export interface WorkflowBuilder { getNode(name: string): NodeInstance | undefined; + /** + * Define a node group — a named, semantic grouping of nodes shown as a frame on + * the canvas. Members are referenced the same way connections reference nodes: by + * node handle (the `const` from `node(...)`). Trigger nodes cannot be grouped. + * Members resolve to the emitted node IDs in `toJSON()`, so groups survive + * `regenerateNodeIds()` like connections do. Chainable. + * + * @example + * ```typescript + * const fetch = node({ ... }); + * const transform = node({ ... }); + * workflow('id', 'Name') + * .add(fetch) + * .to(transform) + * .group('Data ingestion', [fetch, transform]); + * ``` + */ + group(name: string, members: GroupMember[]): WorkflowBuilder; + /** * Validate the workflow graph structure. * Returns errors and warnings without converting to JSON. diff --git a/packages/@n8n/workflow-sdk/src/workflow-builder.test.ts b/packages/@n8n/workflow-sdk/src/workflow-builder.test.ts index c13027c72b1..44de2ed9663 100644 --- a/packages/@n8n/workflow-sdk/src/workflow-builder.test.ts +++ b/packages/@n8n/workflow-sdk/src/workflow-builder.test.ts @@ -8,6 +8,7 @@ import { tool, outputParser, } from './workflow-builder/node-builders/subnode-builders'; +import { generateDeterministicGroupId } from './workflow-builder/string-utils'; describe('Workflow Builder', () => { describe('workflow()', () => { @@ -951,6 +952,109 @@ describe('Workflow Builder', () => { }); }); + describe('node groups', () => { + it('emits a group authored via .group() with members resolved to node ids', () => { + const t = trigger({ + type: 'n8n-nodes-base.manualTrigger', + version: 1, + config: { name: 'Start' }, + }); + const fetch = node({ + type: 'n8n-nodes-base.httpRequest', + version: 4.2, + config: { name: 'Fetch' }, + }); + const transform = node({ + type: 'n8n-nodes-base.set', + version: 3, + config: { name: 'Transform' }, + }); + + const json = workflow('wf-1', 'Test') + .add(t) + .to(fetch) + .to(transform) + .group('Ingestion', [fetch, transform]) + .toJSON(); + + const idByName = new Map(json.nodes.map((n) => [n.name, n.id])); + expect(json.nodeGroups).toEqual([ + { + id: generateDeterministicGroupId('wf-1', 'Ingestion'), + name: 'Ingestion', + nodeIds: [idByName.get('Fetch'), idByName.get('Transform')], + }, + ]); + }); + + it('omits nodeGroups when no group was declared', () => { + const t = trigger({ + type: 'n8n-nodes-base.manualTrigger', + version: 1, + config: { name: 'Start' }, + }); + + expect(workflow('wf-1', 'Test').add(t).toJSON().nodeGroups).toBeUndefined(); + }); + + it('preserves a group id imported via fromJSON across a round-trip', () => { + const source: WorkflowJSON = { + id: 'wf-1', + name: 'Test', + nodes: [ + { + id: 'id-a', + name: 'A', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [0, 0], + parameters: {}, + }, + { + id: 'id-b', + name: 'B', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [10, 0], + parameters: {}, + }, + ], + connections: { A: { main: [[{ node: 'B', type: 'main', index: 0 }]] } }, + nodeGroups: [{ id: 'ui-random-id', name: 'G', nodeIds: ['id-a', 'id-b'] }], + }; + + const json = workflow.fromJSON(source).toJSON(); + + expect(json.nodeGroups).toEqual([ + { id: 'ui-random-id', name: 'G', nodeIds: ['id-a', 'id-b'] }, + ]); + }); + + it('keeps an imported group after the builder is modified', () => { + const source: WorkflowJSON = { + id: 'wf-1', + name: 'Test', + nodes: [ + { + id: 'id-a', + name: 'A', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + nodeGroups: [{ id: 'ui-random-id', name: 'G', nodeIds: ['id-a'] }], + }; + + const added = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'B' } }); + const json = workflow.fromJSON(source).to(added).toJSON(); + + expect(json.nodeGroups).toEqual([{ id: 'ui-random-id', name: 'G', nodeIds: ['id-a'] }]); + }); + }); + describe('.toString()', () => { it('should serialize to JSON string', () => { const wf = workflow('test-id', 'Test Workflow'); diff --git a/packages/@n8n/workflow-sdk/src/workflow-builder.ts b/packages/@n8n/workflow-sdk/src/workflow-builder.ts index 63ce5039517..b1c239b2d7a 100644 --- a/packages/@n8n/workflow-sdk/src/workflow-builder.ts +++ b/packages/@n8n/workflow-sdk/src/workflow-builder.ts @@ -7,6 +7,7 @@ import type { NodeInstance, ConnectionTarget, GraphNode, + GroupMember, IDataObject, NodeChain, GeneratePinDataOptions, @@ -56,6 +57,8 @@ class WorkflowBuilderImpl implements WorkflowBuilder { private _branchDepth = 0; private _dispatchedComposites = new WeakSet(); private static readonly MAX_BRANCH_DEPTH = 500; + /** Node groups, carried by member node handle and resolved to IDs in toJSON(). */ + private _nodeGroups: Array<{ id?: string; name: string; members: GroupMember[] }>; constructor( id: string, @@ -66,6 +69,7 @@ class WorkflowBuilderImpl implements WorkflowBuilder { pinData?: Record, meta?: { templateId?: string; instanceId?: string; [key: string]: unknown }, registry?: PluginRegistry, + nodeGroups?: Array<{ id?: string; name: string; members: GroupMember[] }>, ) { this.id = id; this.name = name; @@ -76,6 +80,9 @@ class WorkflowBuilderImpl implements WorkflowBuilder { this._pinData = pinData; this._meta = meta; this._registry = registry; + this._nodeGroups = nodeGroups + ? nodeGroups.map((g) => ({ id: g.id, name: g.name, members: [...g.members] })) + : []; } /** @@ -491,6 +498,31 @@ class WorkflowBuilderImpl implements WorkflowBuilder { return this; } + group(name: string, members: GroupMember[]): WorkflowBuilder { + this._nodeGroups.push({ name, members: [...members] }); + return this; + } + + /** + * Resolve each group's members to the IDs the emitted nodes will carry. Each member + * handle resolves to its current map key (stable across regenerateNodeIds() via + * _staleIdToKeyMap, exactly as connection targets do); the live instance under that + * key holds the ID the serializer emits. Unresolvable members are dropped. + */ + private resolveNodeGroups(): Array<{ id?: string; name: string; memberIds: string[] }> { + return this._nodeGroups.map((group) => { + const memberIds: string[] = []; + for (const member of group.members) { + const key = this.resolveTargetNodeName(member, this._staleIdToKeyMap); + const id = key ? this._nodes.get(key)?.instance.id : undefined; + if (id && !memberIds.includes(id)) { + memberIds.push(id); + } + } + return { id: group.id, name: group.name, memberIds }; + }); + } + getNode(name: string): NodeInstance | undefined { // First try direct lookup (for backward compatibility and nodes added via add/then) const directLookup = this._nodes.get(name); @@ -525,6 +557,8 @@ class WorkflowBuilderImpl implements WorkflowBuilder { meta: this._meta, tidyUp: options?.tidyUp ?? false, resolveTargetNodeName: (target: unknown) => this.resolveTargetNodeName(target), + nodeGroups: this._nodeGroups.length > 0 ? this.resolveNodeGroups() : undefined, + existingGroupIdsByName: options?.existingGroupIdsByName, }; return jsonSerializer.serialize(ctx); @@ -1325,6 +1359,8 @@ function fromJSON(json: WorkflowJSON): WorkflowBuilder { parsed.lastNode, parsed.pinData, parsed.meta, + undefined, + parsed.nodeGroups, ); } diff --git a/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/serializers/json-serializer.test.ts b/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/serializers/json-serializer.test.ts index bee56837d47..ab07e070a65 100644 --- a/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/serializers/json-serializer.test.ts +++ b/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/serializers/json-serializer.test.ts @@ -1,6 +1,7 @@ import { jsonSerializer } from './json-serializer'; import type { GraphNode } from '../../../types/base'; -import { trigger } from '../../node-builders/node-builder'; +import { node, trigger } from '../../node-builders/node-builder'; +import { generateDeterministicGroupId } from '../../string-utils'; import type { SerializerContext } from '../types'; // Helper to create a mock serializer context @@ -111,5 +112,66 @@ describe('jsonSerializer', () => { expect(result.nodes[0]?.parameters).toEqual({}); }); + + describe('node groups', () => { + it('omits nodeGroups when the context has none', () => { + const result = jsonSerializer.serialize(createMockSerializerContext()); + + expect(result.nodeGroups).toBeUndefined(); + }); + + it('derives a deterministic id when a group carries none and no name matches', () => { + const ctx = createMockSerializerContext({ + workflowId: 'wf-1', + nodeGroups: [{ name: 'G', memberIds: [] }], + }); + + const result = jsonSerializer.serialize(ctx); + + expect(result.nodeGroups).toEqual([ + { id: generateDeterministicGroupId('wf-1', 'G'), name: 'G', nodeIds: [] }, + ]); + }); + + it('reuses an existing id matched by name when the group carries none', () => { + const ctx = createMockSerializerContext({ + nodeGroups: [{ name: 'G', memberIds: [] }], + existingGroupIdsByName: new Map([['G', 'ui-id']]), + }); + + const result = jsonSerializer.serialize(ctx); + + expect(result.nodeGroups![0].id).toBe('ui-id'); + }); + + it("prefers the group's own id over a name match", () => { + const ctx = createMockSerializerContext({ + nodeGroups: [{ id: 'carried-id', name: 'G', memberIds: [] }], + existingGroupIdsByName: new Map([['G', 'ui-id']]), + }); + + const result = jsonSerializer.serialize(ctx); + + expect(result.nodeGroups![0].id).toBe('carried-id'); + }); + + it('drops member ids that are not present in the emitted nodes', () => { + const fetch = node({ + type: 'n8n-nodes-base.httpRequest', + version: 4.2, + config: { name: 'Fetch' }, + }); + const ctx = createMockSerializerContext({ + nodes: new Map([ + ['Fetch', { instance: fetch, connections: new Map() }], + ]), + nodeGroups: [{ name: 'G', memberIds: [fetch.id, 'ghost-id'] }], + }); + + const result = jsonSerializer.serialize(ctx); + + expect(result.nodeGroups![0].nodeIds).toEqual([fetch.id]); + }); + }); }); }); diff --git a/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/serializers/json-serializer.ts b/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/serializers/json-serializer.ts index e4c1358c966..f81f58c8c7e 100644 --- a/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/serializers/json-serializer.ts +++ b/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/serializers/json-serializer.ts @@ -21,6 +21,7 @@ import { normalizeResourceLocators, escapeNewlinesInExpressionStrings, parseVersion, + generateDeterministicGroupId, } from '../../string-utils'; import type { SerializerPlugin, SerializerContext } from '../types'; @@ -272,6 +273,23 @@ export const jsonSerializer: SerializerPlugin = { json.meta = ctx.meta; } + // Members already carry the emitted nodes' IDs; filter out any that aren't present + // in the output (defensive — should never happen). Group ID precedence: own ID + // (carried through fromJSON for a lossless round-trip), then a name match (preserves + // UI-assigned IDs across edits), else a deterministic ID from the name. + if (ctx.nodeGroups && ctx.nodeGroups.length > 0) { + const emittedIds = new Set(nodes.map((node) => node.id)); + + json.nodeGroups = ctx.nodeGroups.map((group) => ({ + id: + group.id ?? + ctx.existingGroupIdsByName?.get(group.name) ?? + generateDeterministicGroupId(ctx.workflowId, group.name), + name: group.name, + nodeIds: group.memberIds.filter((id) => emittedIds.has(id)), + })); + } + return json; }, }; diff --git a/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/types.ts b/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/types.ts index de580f0be42..e0ce27a4507 100644 --- a/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/types.ts +++ b/packages/@n8n/workflow-sdk/src/workflow-builder/plugins/types.ts @@ -351,6 +351,20 @@ export interface SerializerContext extends PluginContext { /** Whether to use Dagre-based layout for node positioning */ readonly tidyUp?: boolean; + + /** + * Node groups carried by member node *ID* — already resolved to the IDs the emitted + * nodes carry. `id`, when present, is the source group ID (from fromJSON); the serializer + * reuses it, otherwise assigns one. + */ + readonly nodeGroups?: ReadonlyArray<{ id?: string; name: string; memberIds: string[] }>; + + /** + * Existing group IDs keyed by group name. When a group name matches, the serializer + * reuses that ID instead of deriving a deterministic one (preserves UI-assigned IDs + * across edits). Groups without a match fall back to the deterministic ID. + */ + readonly existingGroupIdsByName?: ReadonlyMap; } /** diff --git a/packages/@n8n/workflow-sdk/src/workflow-builder/string-utils.test.ts b/packages/@n8n/workflow-sdk/src/workflow-builder/string-utils.test.ts index 5fe1b3ba969..e19ab369dae 100644 --- a/packages/@n8n/workflow-sdk/src/workflow-builder/string-utils.test.ts +++ b/packages/@n8n/workflow-sdk/src/workflow-builder/string-utils.test.ts @@ -9,6 +9,7 @@ import { escapeNewlinesInStringLiterals, escapeNewlinesInExpressionStrings, generateDeterministicNodeId, + generateDeterministicGroupId, } from './string-utils'; describe('workflow-builder/string-utils', () => { @@ -276,4 +277,33 @@ describe('workflow-builder/string-utils', () => { expect(id1).not.toBe(id2); }); }); + + describe('generateDeterministicGroupId', () => { + it('generates UUID format', () => { + const id = generateDeterministicGroupId('wf1', 'My Group'); + expect(id).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/); + }); + + it('is deterministic for same inputs', () => { + expect(generateDeterministicGroupId('wf1', 'G')).toBe( + generateDeterministicGroupId('wf1', 'G'), + ); + }); + + it('produces different IDs for a different workflow or group name', () => { + expect(generateDeterministicGroupId('wf1', 'G')).not.toBe( + generateDeterministicGroupId('wf2', 'G'), + ); + expect(generateDeterministicGroupId('wf1', 'A')).not.toBe( + generateDeterministicGroupId('wf1', 'B'), + ); + }); + + it('produces a different id than a node id for the same workflow and name', () => { + // The ':group:' seed segment keeps group ids disjoint from node ids. + expect(generateDeterministicGroupId('wf1', 'Ingestion')).not.toBe( + generateDeterministicNodeId('wf1', 'n8n-nodes-base.set', 'Ingestion'), + ); + }); + }); }); diff --git a/packages/@n8n/workflow-sdk/src/workflow-builder/string-utils.ts b/packages/@n8n/workflow-sdk/src/workflow-builder/string-utils.ts index 7f3e4cac05a..183b3e455c3 100644 --- a/packages/@n8n/workflow-sdk/src/workflow-builder/string-utils.ts +++ b/packages/@n8n/workflow-sdk/src/workflow-builder/string-utils.ts @@ -319,19 +319,11 @@ export function escapeNewlinesInExpressionStrings(value: unknown): unknown { } /** - * Generate a deterministic UUID based on workflow ID, node type, and node name. - * This ensures that the same workflow structure always produces the same node IDs, - * which is critical for the AI workflow builder where code may be re-parsed multiple times. + * Hash a seed string and format the digest as a deterministic UUID v4 structure. + * Shared by the node-id and group-id generators so they format identically. */ -export function generateDeterministicNodeId( - workflowId: string, - nodeType: string, - nodeName: string, -): string { - const hash = createHash('sha256') - .update(`${workflowId}:${nodeType}:${nodeName}`) - .digest('hex') - .slice(0, 32); +function deterministicUuidFromSeed(seed: string): string { + const hash = createHash('sha256').update(seed).digest('hex').slice(0, 32); // Format as valid UUID v4 structure return [ @@ -342,3 +334,26 @@ export function generateDeterministicNodeId( hash.slice(20, 32), ].join('-'); } + +/** + * Generate a deterministic UUID based on workflow ID, node type, and node name. + * This ensures that the same workflow structure always produces the same node IDs, + * which is critical for the AI workflow builder where code may be re-parsed multiple times. + */ +export function generateDeterministicNodeId( + workflowId: string, + nodeType: string, + nodeName: string, +): string { + return deterministicUuidFromSeed(`${workflowId}:${nodeType}:${nodeName}`); +} + +/** + * Generate a deterministic UUID for a node group based on workflow ID and group name. + * Group names are unique within a workflow, so the same name always yields the same ID + * on every round-trip — no persisted-workflow lookup needed. The `:group:` segment keeps + * group IDs from ever colliding with node IDs from {@link generateDeterministicNodeId}. + */ +export function generateDeterministicGroupId(workflowId: string, groupName: string): string { + return deterministicUuidFromSeed(`${workflowId}:group:${groupName}`); +} diff --git a/packages/@n8n/workflow-sdk/src/workflow-builder/workflow-import.test.ts b/packages/@n8n/workflow-sdk/src/workflow-builder/workflow-import.test.ts index e7fedc4184b..84096dedf78 100644 --- a/packages/@n8n/workflow-sdk/src/workflow-builder/workflow-import.test.ts +++ b/packages/@n8n/workflow-sdk/src/workflow-builder/workflow-import.test.ts @@ -388,4 +388,84 @@ describe('parseWorkflowJSON', () => { expect(json.nodes[0].typeVersion).toBe(originalTypeVersion); expect(JSON.stringify(json.connections)).toBe(originalConnections); }); + + it('rebuilds node groups, mapping member ids to their node instances', () => { + const json: WorkflowJSON = { + id: 'wf', + name: 'Grouped', + nodes: [ + { + id: 'id-a', + name: 'A', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [0, 0], + parameters: {}, + }, + { + id: 'id-b', + name: 'B', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [10, 0], + parameters: {}, + }, + ], + connections: {}, + // The incoming group id is irrelevant here — parse carries members as instances. + nodeGroups: [{ id: 'group-1', name: 'G', nodeIds: ['id-a', 'id-b'] }], + }; + + const result = parseWorkflowJSON(json); + + expect(result.nodeGroups).toHaveLength(1); + expect(result.nodeGroups![0].name).toBe('G'); + expect(result.nodeGroups![0].members.map((m) => m.id)).toEqual(['id-a', 'id-b']); + expect(result.nodeGroups![0].members.map((m) => m.name)).toEqual(['A', 'B']); + }); + + it('drops group members whose ids do not match a node', () => { + const json: WorkflowJSON = { + id: 'wf', + name: 'Grouped', + nodes: [ + { + id: 'id-a', + name: 'A', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + nodeGroups: [{ id: 'group-1', name: 'G', nodeIds: ['id-a', 'ghost'] }], + }; + + const result = parseWorkflowJSON(json); + + expect(result.nodeGroups![0].members.map((m) => m.id)).toEqual(['id-a']); + }); + + it('returns undefined nodeGroups when none are declared', () => { + const json: WorkflowJSON = { + id: 'wf', + name: 'No groups', + nodes: [ + { + id: 'id-a', + name: 'A', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + }; + + const result = parseWorkflowJSON(json); + + expect(result.nodeGroups).toBeUndefined(); + }); }); diff --git a/packages/@n8n/workflow-sdk/src/workflow-builder/workflow-import.ts b/packages/@n8n/workflow-sdk/src/workflow-builder/workflow-import.ts index 73e7d92cda9..06ad4a56846 100644 --- a/packages/@n8n/workflow-sdk/src/workflow-builder/workflow-import.ts +++ b/packages/@n8n/workflow-sdk/src/workflow-builder/workflow-import.ts @@ -30,6 +30,13 @@ export interface ParsedWorkflow { readonly lastNode: string | null; readonly pinData?: Record; readonly meta?: { templateId?: string; instanceId?: string; [key: string]: unknown }; + /** Node groups reconstructed by mapping the JSON's member IDs back to node handles. */ + readonly nodeGroups?: Array<{ + /** Source group ID from the JSON. */ + id?: string; + name: string; + members: Array>; + }>; } /** @@ -40,6 +47,9 @@ export function parseWorkflowJSON(json: WorkflowJSON): ParsedWorkflow { const nodes = new Map(); // Map from connection name (how nodes reference each other) to map key const nameToKey = new Map(); + // Map from n8n node ID to the created node handle, used to rebuild groups (which + // reference members by ID) as node refs — the same shape `.group()` authoring uses. + const idToInstance = new Map>(); // Create node instances from JSON (shallow-clone each node to avoid mutating the input) let unnamedCounter = 0; @@ -116,6 +126,10 @@ export function parseWorkflowJSON(json: WorkflowJSON): ParsedWorkflow { instance, connections: connectionsMap, }); + + // Groups reference members by ID; record ID → handle so we can carry groups as + // node refs (resolved back to IDs in toJSON, exactly like authored groups). + if (n8nNode.id) idToInstance.set(n8nNode.id, instance); } // Rebuild connections (deep-clone to avoid mutating the input) @@ -161,6 +175,19 @@ export function parseWorkflowJSON(json: WorkflowJSON): ParsedWorkflow { lastNode = name; } + // Rebuild groups by mapping each member ID back to its node handle (unresolvable IDs + // are dropped). The group's `id` is carried through so a round-trip preserves it. + const nodeGroups = json.nodeGroups?.length + ? json.nodeGroups.map((group) => ({ + id: group.id, + name: group.name, + members: group.nodeIds.flatMap((id) => { + const instance = idToInstance.get(id); + return instance !== undefined ? [instance] : []; + }), + })) + : undefined; + return { id: json.id ?? '', name: json.name, @@ -169,5 +196,6 @@ export function parseWorkflowJSON(json: WorkflowJSON): ParsedWorkflow { lastNode, pinData: json.pinData, meta: json.meta, + nodeGroups, }; } diff --git a/packages/@n8n/workflow-sdk/test-fixtures/committed-workflows/10.json b/packages/@n8n/workflow-sdk/test-fixtures/committed-workflows/10.json new file mode 100644 index 00000000000..6bf25813d5c --- /dev/null +++ b/packages/@n8n/workflow-sdk/test-fixtures/committed-workflows/10.json @@ -0,0 +1,69 @@ +{ + "id": "wf-committed-10", + "name": "Node groups - linear chain with a grouped section", + "nodes": [ + { + "parameters": {}, + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [-200, 0], + "id": "trigger-10-0001", + "name": "When clicking 'Execute workflow'" + }, + { + "parameters": { + "assignments": { + "assignments": [ + { + "id": "assign-10-0001", + "name": "label", + "value": "raw", + "type": "string" + } + ] + }, + "options": {} + }, + "type": "n8n-nodes-base.set", + "typeVersion": 3.4, + "position": [0, 0], + "id": "set-10-0001", + "name": "Normalize" + }, + { + "parameters": { + "assignments": { + "assignments": [ + { + "id": "assign-10-0002", + "name": "enriched", + "value": "={{ $json.label }}-enriched", + "type": "string" + } + ] + }, + "options": {} + }, + "type": "n8n-nodes-base.set", + "typeVersion": 3.4, + "position": [200, 0], + "id": "set-10-0002", + "name": "Enrich" + } + ], + "connections": { + "When clicking 'Execute workflow'": { + "main": [[{ "node": "Normalize", "type": "main", "index": 0 }]] + }, + "Normalize": { + "main": [[{ "node": "Enrich", "type": "main", "index": 0 }]] + } + }, + "nodeGroups": [ + { + "id": "grp-10-0001", + "name": "Transform", + "nodeIds": ["set-10-0001", "set-10-0002"] + } + ] +} diff --git a/packages/@n8n/workflow-sdk/test-fixtures/committed-workflows/manifest.json b/packages/@n8n/workflow-sdk/test-fixtures/committed-workflows/manifest.json index a92894f88ad..11072ccdab2 100644 --- a/packages/@n8n/workflow-sdk/test-fixtures/committed-workflows/manifest.json +++ b/packages/@n8n/workflow-sdk/test-fixtures/committed-workflows/manifest.json @@ -104,6 +104,11 @@ "id": 9, "name": "If/Switch branch wired directly to multi-input merge", "success": true + }, + { + "id": 10, + "name": "Node groups - linear chain with a grouped section", + "success": true } ] } diff --git a/packages/cli/src/__tests__/workflow-helpers.test.ts b/packages/cli/src/__tests__/workflow-helpers.test.ts index 84007234a91..b9669eea99f 100644 --- a/packages/cli/src/__tests__/workflow-helpers.test.ts +++ b/packages/cli/src/__tests__/workflow-helpers.test.ts @@ -453,66 +453,146 @@ describe('validateWorkflowNodeGroups', () => { it('should pass when nodeGroups is undefined', () => { expect(() => - validateWorkflowNodeGroups({ nodes: [makeNode('n1')], nodeGroups: undefined }), + validateWorkflowNodeGroups({ nodes: [makeNode('n1')], nodeGroups: undefined }, null), ).not.toThrow(); }); it('should pass when nodeGroups is empty', () => { expect(() => - validateWorkflowNodeGroups({ nodes: [makeNode('n1')], nodeGroups: [] }), + validateWorkflowNodeGroups({ nodes: [makeNode('n1')], nodeGroups: [] }, null), ).not.toThrow(); }); it('should pass when all nodeIds reference existing nodes', () => { expect(() => - validateWorkflowNodeGroups({ - nodes: [makeNode('n1'), makeNode('n2')], - nodeGroups: [{ id: 'g1', name: 'Group 1', nodeIds: ['n1', 'n2'] }], - }), + validateWorkflowNodeGroups( + { + nodes: [makeNode('n1'), makeNode('n2')], + nodeGroups: [{ id: 'g1', name: 'Group 1', nodeIds: ['n1', 'n2'] }], + }, + null, + ), ).not.toThrow(); }); it('should throw when a nodeId does not reference an existing node', () => { expect(() => - validateWorkflowNodeGroups({ - nodes: [makeNode('n1')], - nodeGroups: [{ id: 'g1', name: 'My Group', nodeIds: ['n1', 'n999'] }], - }), + validateWorkflowNodeGroups( + { + nodes: [makeNode('n1')], + nodeGroups: [{ id: 'g1', name: 'My Group', nodeIds: ['n1', 'n999'] }], + }, + null, + ), ).toThrow('Group "My Group" references node ID "n999" that does not exist in the workflow.'); }); it('should throw for the first invalid nodeId found', () => { expect(() => - validateWorkflowNodeGroups({ - nodes: [], - nodeGroups: [{ id: 'g1', name: 'Empty Group', nodeIds: ['bad1', 'bad2'] }], - }), + validateWorkflowNodeGroups( + { + nodes: [], + nodeGroups: [{ id: 'g1', name: 'Empty Group', nodeIds: ['bad1', 'bad2'] }], + }, + null, + ), ).toThrow('Group "Empty Group" references node ID "bad1"'); }); it('should throw when a node belongs to multiple groups', () => { expect(() => - validateWorkflowNodeGroups({ - nodes: [makeNode('n1'), makeNode('n2')], - nodeGroups: [ - { id: 'g1', name: 'Group A', nodeIds: ['n1'] }, - { id: 'g2', name: 'Group B', nodeIds: ['n1', 'n2'] }, - ], - }), + validateWorkflowNodeGroups( + { + nodes: [makeNode('n1'), makeNode('n2')], + nodeGroups: [ + { id: 'g1', name: 'Group A', nodeIds: ['n1'] }, + { id: 'g2', name: 'Group B', nodeIds: ['n1', 'n2'] }, + ], + }, + null, + ), ).toThrow('Node "n1" belongs to multiple groups: "Group A" and "Group B".'); }); it('should throw when group names are not unique', () => { expect(() => - validateWorkflowNodeGroups({ - nodes: [makeNode('n1')], - nodeGroups: [ - { id: 'g1', name: 'Duplicate', nodeIds: ['n1'] }, - { id: 'g2', name: 'Duplicate', nodeIds: [] }, - ], - }), + validateWorkflowNodeGroups( + { + nodes: [makeNode('n1')], + nodeGroups: [ + { id: 'g1', name: 'Duplicate', nodeIds: ['n1'] }, + { id: 'g2', name: 'Duplicate', nodeIds: [] }, + ], + }, + null, + ), ).toThrow('Duplicate node group name "Duplicate".'); }); + + it('should throw when group ids are not unique', () => { + expect(() => + validateWorkflowNodeGroups( + { + nodes: [makeNode('n1'), makeNode('n2')], + nodeGroups: [ + { id: 'dup', name: 'Group A', nodeIds: ['n1'] }, + { id: 'dup', name: 'Group B', nodeIds: ['n2'] }, + ], + }, + null, + ), + ).toThrow('Duplicate node group ID "dup".'); + }); + + describe('full validation', () => { + const triggerType = { group: ['trigger'] } as never; + const regularType = { group: ['transform'] } as never; + // Two nodes connected n1 → n2 form a groupable chain. + const connectedNodes = [makeNode('n1'), makeNode('n2')]; + const connections = { + 'Node n1': { main: [[{ node: 'Node n2', type: 'main', index: 0 }]] }, + } as never; + + it('passes for a valid connected group', () => { + expect(() => + validateWorkflowNodeGroups( + { + nodes: connectedNodes, + connections, + nodeGroups: [{ id: 'g1', name: 'Chain', nodeIds: ['n1', 'n2'] }], + }, + () => regularType, + ), + ).not.toThrow(); + }); + + it('rejects a group that contains a trigger node', () => { + expect(() => + validateWorkflowNodeGroups( + { + nodes: [makeNode('n1')], + connections: {}, + nodeGroups: [{ id: 'g1', name: 'Has trigger', nodeIds: ['n1'] }], + }, + () => triggerType, + ), + ).toThrow('Node group "Has trigger" (g1) cannot contain trigger nodes'); + }); + + it('does not run full checks when getNodeType is null (basic-only)', () => { + // Same trigger-in-group that fails under full validation passes under basic-only. + expect(() => + validateWorkflowNodeGroups( + { + nodes: [makeNode('n1')], + connections: {}, + nodeGroups: [{ id: 'g1', name: 'Has trigger', nodeIds: ['n1'] }], + }, + null, + ), + ).not.toThrow(); + }); + }); }); describe('validatePinDataSize', () => { diff --git a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts index ded71091fa9..a9ca8ee4f01 100644 --- a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts +++ b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts @@ -1934,6 +1934,47 @@ describe('createWorkflowAdapter', () => { ); }); + it('clears existing node groups when the SDK workflow declares none (update is authoritative)', async () => { + // Regression: the SDK omits `nodeGroups` when no `.group(...)` is declared. The + // update path must treat that as "no groups" and send [] so a removed group is + // dropped — not silently preserved and then rejected by group validation. + const { adapter, mockWorkflowService } = createWorkflowAdapterForTests(); + + await adapter.updateFromWorkflowJSON('wf-new', minimalWorkflowJSON); + + expect(mockWorkflowService.update).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ nodeGroups: [] }), + expect.anything(), + expect.anything(), + ); + }); + + it('writes the node groups the SDK workflow declares', async () => { + const { adapter, mockWorkflowService } = createWorkflowAdapterForTests(); + const nodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: ['node-1'] }]; + const workflow = { + name: 'Test', + nodes: [ + { + id: 'node-1', + name: 'Set', + type: 'n8n-nodes-base.set', + typeVersion: 3, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + nodeGroups, + } as unknown as WorkflowJSON; + + await adapter.updateFromWorkflowJSON('wf-new', workflow); + + const updateData = mockWorkflowService.update.mock.calls[0]?.[1] as { nodeGroups: unknown }; + expect(updateData.nodeGroups).toEqual(nodeGroups); + }); + it('strips id-less credential references before creating a workflow', async () => { const { adapter, mockWorkflowService } = createWorkflowAdapterForTests(); const workflow = { diff --git a/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts b/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts index 4af96937ba2..f6df5a29778 100644 --- a/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts +++ b/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts @@ -594,6 +594,7 @@ export class InstanceAiAdapterService { connections: json.connections as unknown as IConnections, settings, pinData: sdkPinDataToRuntime(json.pinData), + nodeGroups: sdkNodeGroupsToRuntime(json.nodeGroups), } as Partial); let updated: WorkflowEntity; @@ -682,6 +683,7 @@ export class InstanceAiAdapterService { connections: json.connections as unknown as IConnections, settings, pinData: sdkPinDataToRuntime(json.pinData), + nodeGroups: sdkNodeGroupsToRuntime(json.nodeGroups), } as Partial); let updated: WorkflowEntity; @@ -781,6 +783,9 @@ export class InstanceAiAdapterService { const updateData = workflowRepository.create({ nodes: version.nodes, connections: version.connections, + // Restore the group state from the same snapshot so groups stay consistent + // with the restored graph (history rows always carry nodeGroups). + nodeGroups: version.nodeGroups, } as Partial); await workflowService.update(user, updateData, workflowId, { @@ -3164,6 +3169,16 @@ function sdkPinDataToRuntime(pinData: Record | undefined): IP return result; } +/** + * Groups are authoritative on save: persist the emitted groups, or [] to clear when the + * agent removed every `.group(...)`. `undefined` would leave the NOT-NULL column stale. + */ +function sdkNodeGroupsToRuntime( + nodeGroups: WorkflowJSON['nodeGroups'], +): NonNullable { + return nodeGroups ?? []; +} + function hasCredentialId(value: unknown): boolean { if (typeof value !== 'object' || value === null) return false; const id = Reflect.get(value, 'id'); @@ -3222,6 +3237,7 @@ function toWorkflowJSON( })), connections: workflow.connections as WorkflowJSON['connections'], settings: workflow.settings as WorkflowJSON['settings'], + ...(workflow.nodeGroups ? { nodeGroups: workflow.nodeGroups } : {}), }; } diff --git a/packages/cli/src/modules/mcp/__tests__/workflow-operations.test.ts b/packages/cli/src/modules/mcp/__tests__/workflow-operations.test.ts index 17d401db01d..fa891569bff 100644 --- a/packages/cli/src/modules/mcp/__tests__/workflow-operations.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/workflow-operations.test.ts @@ -3,6 +3,7 @@ import type { IConnections, INode } from 'n8n-workflow'; import { applyOperations, partialUpdateOperationSchema, + toWorkflowSlice, type PartialUpdateOperation, } from '../tools/workflow-builder/workflow-operations'; @@ -870,4 +871,64 @@ describe('applyOperations', () => { } }); }); + + describe('setNodeGroups', () => { + test('sets node groups on the workflow', () => { + const result = applyOperations(baseWorkflow(), [ + { type: 'setNodeGroups', nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: ['a', 'b'] }] }, + ]); + expect(result.success).toBe(true); + if (!result.success) return; + expect(result.workflow.nodeGroups).toEqual([ + { id: 'g1', name: 'Group', nodeIds: ['a', 'b'] }, + ]); + }); + + test('generates an id when omitted', () => { + const result = applyOperations(baseWorkflow(), [ + { type: 'setNodeGroups', nodeGroups: [{ name: 'Group', nodeIds: ['a'] }] }, + ]); + expect(result.success).toBe(true); + if (!result.success) return; + expect(result.workflow.nodeGroups).toHaveLength(1); + expect(result.workflow.nodeGroups![0].id).toEqual(expect.any(String)); + expect(result.workflow.nodeGroups![0].id.length).toBeGreaterThan(0); + }); + + test('clears all groups with an empty array', () => { + const wf = { ...baseWorkflow(), nodeGroups: [{ id: 'g1', name: 'Old', nodeIds: ['a'] }] }; + const result = applyOperations(wf, [{ type: 'setNodeGroups', nodeGroups: [] }]); + expect(result.success).toBe(true); + if (!result.success) return; + expect(result.workflow.nodeGroups).toEqual([]); + }); + + test('does not mutate the input workflow', () => { + const wf = baseWorkflow(); + applyOperations(wf, [ + { type: 'setNodeGroups', nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: ['a'] }] }, + ]); + expect((wf as { nodeGroups?: unknown }).nodeGroups).toBeUndefined(); + }); + + test('schema parses a valid setNodeGroups op', () => { + const parsed = partialUpdateOperationSchema.safeParse({ + type: 'setNodeGroups', + nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: ['a', 'b'] }], + }); + expect(parsed.success).toBe(true); + }); + }); + + describe('toWorkflowSlice', () => { + test('carries nodeGroups through from the workflow entity', () => { + const slice = toWorkflowSlice({ + name: 'wf', + nodes: [], + connections: {}, + nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: ['a'] }], + } as never); + expect(slice.nodeGroups).toEqual([{ id: 'g1', name: 'Group', nodeIds: ['a'] }]); + }); + }); }); diff --git a/packages/cli/src/modules/mcp/tools/get-workflow-details.tool.ts b/packages/cli/src/modules/mcp/tools/get-workflow-details.tool.ts index 275933a2e42..667404a3e7a 100644 --- a/packages/cli/src/modules/mcp/tools/get-workflow-details.tool.ts +++ b/packages/cli/src/modules/mcp/tools/get-workflow-details.tool.ts @@ -134,6 +134,7 @@ export async function getWorkflowDetails( ({ credentials: _credentials, ...node }) => node, ), connections: workflow.activeVersion.connections ?? {}, + nodeGroups: workflow.activeVersion.nodeGroups ?? [], } : null; @@ -163,6 +164,7 @@ export async function getWorkflowDetails( settings: workflow.settings ?? null, connections, nodes: nodes.map(({ credentials: _credentials, ...node }) => node), + nodeGroups: workflow.nodeGroups ?? [], activeVersion, tags: toTagSummary(workflow.tags), meta: workflow.meta ?? null, diff --git a/packages/cli/src/modules/mcp/tools/schemas.ts b/packages/cli/src/modules/mcp/tools/schemas.ts index 75ef8680b1b..2ac7bd83700 100644 --- a/packages/cli/src/modules/mcp/tools/schemas.ts +++ b/packages/cli/src/modules/mcp/tools/schemas.ts @@ -70,6 +70,14 @@ export const successMessageOutputSchema = { message: z.string().describe('Description of the result'), } satisfies z.ZodRawShape; +const nodeGroupSchema = z + .object({ + id: z.string(), + name: z.string(), + nodeIds: z.array(z.string()), + }) + .describe('A named visual grouping of nodes'); + export const workflowDetailsOutputSchema = z.object({ workflow: z .object({ @@ -88,10 +96,12 @@ export const workflowDetailsOutputSchema = z.object({ settings: workflowSettingsSchema, connections: z.record(z.unknown()), nodes: z.array(nodeSchema), + nodeGroups: z.array(nodeGroupSchema).describe('Node groups in the workflow'), activeVersion: z .object({ nodes: z.array(nodeSchema), connections: z.record(z.unknown()), + nodeGroups: z.array(nodeGroupSchema).describe('Node groups in the active version'), }) .nullable() .describe('Active workflow graph, if available'), diff --git a/packages/cli/src/modules/mcp/tools/workflow-builder/update-workflow.tool.ts b/packages/cli/src/modules/mcp/tools/workflow-builder/update-workflow.tool.ts index b87f94eddc1..1ea770fd13d 100644 --- a/packages/cli/src/modules/mcp/tools/workflow-builder/update-workflow.tool.ts +++ b/packages/cli/src/modules/mcp/tools/workflow-builder/update-workflow.tool.ts @@ -49,6 +49,7 @@ const operationTypeSchema = z.enum([ 'setWorkflowMetadata', 'addTags', 'removeTags', + 'setNodeGroups', ]); const positionInputSchema = z.array(z.number()).length(2).describe('Canvas [x, y].'); @@ -117,6 +118,16 @@ const operationInputSchema = z name: z.string().max(128).optional().describe('Only used for setWorkflowMetadata.'), description: z.string().max(255).optional().describe('Only used for setWorkflowMetadata.'), names: z.array(z.string()).optional().describe('For addTags / removeTags.'), + nodeGroups: z + .array( + z.object({ + id: z.string().optional(), + name: z.string(), + nodeIds: z.array(z.string()), + }), + ) + .optional() + .describe('For setNodeGroups. Replaces all node groups; pass [] to clear.'), }) .describe('Workflow update operation. Provide fields matching type.'); @@ -356,6 +367,10 @@ export const createUpdateWorkflowTool = ( (op) => op.type !== 'addTags' && op.type !== 'removeTags', ); + // Only persist nodeGroups when a setNodeGroups op ran; otherwise omit the key so + // WorkflowService preserves the existing groups (preserve-on-omit). + const hasNodeGroupOperation = strictOperations.some((op) => op.type === 'setNodeGroups'); + const workflowUpdateData = new WorkflowEntity(); Object.assign(workflowUpdateData, { name: result.workflow.name, @@ -364,6 +379,7 @@ export const createUpdateWorkflowTool = ( : {}), nodes: result.workflow.nodes, connections: result.workflow.connections, + ...(hasNodeGroupOperation ? { nodeGroups: result.workflow.nodeGroups } : {}), meta: hasNonTagOperations ? { ...(existingWorkflow.meta ?? {}), diff --git a/packages/cli/src/modules/mcp/tools/workflow-builder/workflow-operations.ts b/packages/cli/src/modules/mcp/tools/workflow-builder/workflow-operations.ts index c0264000367..cd1882d7dbc 100644 --- a/packages/cli/src/modules/mcp/tools/workflow-builder/workflow-operations.ts +++ b/packages/cli/src/modules/mcp/tools/workflow-builder/workflow-operations.ts @@ -4,6 +4,7 @@ import type { INode, INodeParameters, IWorkflowBase, + IWorkflowGroup, NodeConnectionType, } from 'n8n-workflow'; import { isSafeObjectProperty, NodeConnectionTypes } from 'n8n-workflow'; @@ -184,6 +185,22 @@ export const partialUpdateOperationSchema = z.discriminatedUnion('type', [ .max(50) .describe('Tag names to detach from the workflow. Unknown names are ignored.'), }), + z.object({ + type: z.literal('setNodeGroups'), + nodeGroups: z + .array( + z.object({ + id: z.string().trim().min(1).optional().describe('Group id. Generated if omitted.'), + name: z.string().trim().min(1).describe('Unique group name.'), + nodeIds: z + .array(z.string().trim().min(1)) + .describe('IDs of the nodes that belong to this group.'), + }), + ) + .describe( + 'Replaces the workflow node groups entirely. Pass [] to remove all groups. Each nodeId must reference an existing node, and every group must form a valid, connected, trigger-free section of the graph (validated on save).', + ), + }), ]); export type PartialUpdateOperation = z.infer; @@ -193,6 +210,8 @@ interface WorkflowSlice { description?: string; nodes: INode[]; connections: IConnections; + /** Node groups on the workflow. Undefined when never touched; set by setNodeGroups. */ + nodeGroups?: IWorkflowGroup[]; /** Existing tag names on the workflow. Undefined when not loaded; tag ops require this. */ tagNames?: string[]; } @@ -218,6 +237,7 @@ const cloneWorkflow = (workflow: WorkflowSlice): WorkflowSlice => ({ description: workflow.description, nodes: workflow.nodes.map((node) => structuredClone(node)), connections: structuredClone(workflow.connections), + nodeGroups: workflow.nodeGroups ? structuredClone(workflow.nodeGroups) : undefined, tagNames: workflow.tagNames ? [...workflow.tagNames] : undefined, }); @@ -590,6 +610,15 @@ export function applyOperations( break; } + case 'setNodeGroups': { + workflow.nodeGroups = op.nodeGroups.map((group) => ({ + id: group.id ?? uuid(), + name: group.name, + nodeIds: [...group.nodeIds], + })); + break; + } + case 'addTags': case 'removeTags': { if (workflow.tagNames === undefined) { @@ -644,6 +673,7 @@ export function toWorkflowSlice( description: (workflow as { description?: string }).description, nodes: workflow.nodes, connections: workflow.connections, + nodeGroups: workflow.nodeGroups, tagNames, }; } diff --git a/packages/cli/src/modules/source-control.ee/source-control-import.service.ee.ts b/packages/cli/src/modules/source-control.ee/source-control-import.service.ee.ts index d31d5d45995..65bd84e923f 100644 --- a/packages/cli/src/modules/source-control.ee/source-control-import.service.ee.ts +++ b/packages/cli/src/modules/source-control.ee/source-control-import.service.ee.ts @@ -754,7 +754,9 @@ export class SourceControlImportService { importedWorkflow.nodeGroups ??= []; try { - validateWorkflowNodeGroups(importedWorkflow); + // Basic checks only: a git import should not reset groups for graph-shape + // rules, only for structural integrity (missing refs, duplicate ids/names). + validateWorkflowNodeGroups(importedWorkflow, null); } catch { this.logger.warn( `Workflow file ${candidate.file} has invalid nodeGroups, resetting to empty`, diff --git a/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/activeVersion.yml b/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/activeVersion.yml index 2afb5e1da71..e35ea77627b 100644 --- a/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/activeVersion.yml +++ b/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/activeVersion.yml @@ -22,6 +22,12 @@ properties: type: object readOnly: true example: { Jira: { main: [[{ node: 'Jira', type: 'main', index: 0 }]] } } + nodeGroups: + type: array + readOnly: true + description: Visual groupings of nodes shown as frames on the canvas + items: + $ref: './workflowNodeGroup.yml' authors: type: string readOnly: true diff --git a/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflow.yml b/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflow.yml index bda31f004ea..a3aab21b4e7 100644 --- a/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflow.yml +++ b/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflow.yml @@ -46,6 +46,11 @@ properties: connections: type: object example: { Jira: { main: [[{ node: 'Jira', type: 'main', index: 0 }]] } } + nodeGroups: + type: array + description: Visual groupings of nodes shown as frames on the canvas + items: + $ref: './workflowNodeGroup.yml' settings: $ref: './workflowSettings.yml' staticData: diff --git a/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowCreate.yml b/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowCreate.yml index 223ae63e418..db90c7327b7 100644 --- a/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowCreate.yml +++ b/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowCreate.yml @@ -42,6 +42,11 @@ properties: connections: type: object example: { Jira: { main: [[{ node: 'Jira', type: 'main', index: 0 }]] } } + nodeGroups: + type: array + description: Visual groupings of nodes shown as frames on the canvas + items: + $ref: './workflowNodeGroup.yml' settings: $ref: './workflowSettings.yml' staticData: diff --git a/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowNodeGroup.yml b/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowNodeGroup.yml new file mode 100644 index 00000000000..5ee804c4e88 --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowNodeGroup.yml @@ -0,0 +1,20 @@ +type: object +additionalProperties: false +required: + - id + - name + - nodeIds +properties: + id: + type: string + description: Unique identifier for the node group + example: 9b1c8e2a-4d3f-4a6b-8c7d-1e2f3a4b5c6d + name: + type: string + description: Display name of the node group + example: Data processing + nodeIds: + type: array + description: IDs of the nodes that belong to this group + items: + type: string diff --git a/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowVersion.yml b/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowVersion.yml index b882dd2f067..43d6ba93007 100644 --- a/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowVersion.yml +++ b/packages/cli/src/public-api/v1/handlers/workflows/spec/schemas/workflowVersion.yml @@ -28,6 +28,12 @@ properties: example: { Jira: { main: [[{ node: 'Jira', type: 'main', index: 0 }]] } } description: Connections as they were in this version readOnly: true + nodeGroups: + type: array + readOnly: true + description: Node groups as they were in this version + items: + $ref: './workflowNodeGroup.yml' authors: type: string readOnly: true diff --git a/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts b/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts index a2f685abeb2..1b63158ad1f 100644 --- a/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts +++ b/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts @@ -249,6 +249,7 @@ const workflowHandlers: WorkflowHandlers = { 'isArchived', 'nodes', 'connections', + 'nodeGroups', 'settings', 'staticData', 'meta', diff --git a/packages/cli/src/public-api/v1/shared/spec/schemas/_index.yml b/packages/cli/src/public-api/v1/shared/spec/schemas/_index.yml index f37446c2b8d..396d005616d 100644 --- a/packages/cli/src/public-api/v1/shared/spec/schemas/_index.yml +++ b/packages/cli/src/public-api/v1/shared/spec/schemas/_index.yml @@ -6,6 +6,8 @@ Execution: $ref: './../../../handlers/executions/spec/schemas/execution.yml' Node: $ref: './../../../handlers/workflows/spec/schemas/node.yml' +WorkflowNodeGroup: + $ref: './../../../handlers/workflows/spec/schemas/workflowNodeGroup.yml' Tag: $ref: './../../../handlers/tags/spec/schemas/tag.yml' Workflow: diff --git a/packages/cli/src/workflow-helpers.ts b/packages/cli/src/workflow-helpers.ts index ac174b2865a..5cd1212b359 100644 --- a/packages/cli/src/workflow-helpers.ts +++ b/packages/cli/src/workflow-helpers.ts @@ -6,13 +6,18 @@ import { formatWorkflowStructureIssuePath, resolveNodeWebhookId, safeParseWorkflowStructure, + validateNodeSelectionForGrouping, type IDataObject, + type INode, type INodeCredentialsDetails, + type INodeTypeDescription, type INodeTypes, type IRun, type ITaskData, type IWorkflowBase, + type IWorkflowGroup, type IWorkflowSettings, + type NodeGroupValidationResult, type RelatedExecution, type WorkflowStructureIssue, } from 'n8n-workflow'; @@ -138,20 +143,97 @@ export function resolveNodeWebhookIds(workflow: IWorkflowBase, nodeTypes: INodeT } /** - * Validates nodeGroups: unique group names, all referenced node IDs exist, - * and each node belongs to at most one group. + * 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; + } + }; +} + +/** + * Maps a failed `validateNodeSelectionForGrouping` result to an actionable + * `BadRequestError` that names the offending group and the rule it broke. + */ +function nodeGroupValidationError( + group: IWorkflowGroup, + result: Extract, +): BadRequestError { + const label = `Node group "${group.name}" (${group.id})`; + switch (result.reason) { + case 'trigger-selected': + return new BadRequestError( + `${label} cannot contain trigger nodes: ${result.triggers.join(', ')}.`, + ); + case 'invalid-subgraph': + return new BadRequestError( + `${label} must form a single connected subgraph with a single entry and exit.`, + ); + case 'multiple-input-branches': + return new BadRequestError(`${label} has multiple input branches at node "${result.node}".`); + case 'multiple-output-branches': + return new BadRequestError(`${label} has multiple output branches at node "${result.node}".`); + case 'node-already-grouped': + return new BadRequestError( + `${label} contains nodes that already belong to another group: ${result.nodeIds.join(', ')}.`, + ); + case 'non-main-boundary': + return new BadRequestError( + `${label} cannot cross the "${result.connection.type}" connection between "${result.connection.source}" and "${result.connection.target}".`, + ); + } +} + +/** + * Validates nodeGroups. + * + * Basic checks (always run): unique group IDs, unique group names, all referenced + * node IDs exist, and each node belongs to at most one group. + * + * Full checks (run only when `getNodeType` is non-null): each group must satisfy + * the same grouping rules the canvas enforces — no triggers, a single connected + * subgraph, and no non-main connection crossing the group boundary — validated + * against the other groups as existing groups. Pass the `getNodeType` callback to + * run the full checks (on create, and on an update that changed the graph or the + * groups); pass `null` to run basic checks only (e.g. a git import, so + * legacy-invalid groups don't block the import). + * * Note for frontend: Must be called after `addNodeIds` since nodes created via the API * may not have IDs until that step assigns them. */ -export function validateWorkflowNodeGroups(workflow: Pick) { +export function validateWorkflowNodeGroups( + workflow: Pick & { + connections?: IWorkflowBase['connections']; + }, + getNodeType: GetNodeTypeForGrouping | null, +) { const { nodeGroups, nodes } = workflow; if (!nodeGroups || nodeGroups.length === 0) return; const nodeIds = new Set(nodes.map((n) => n.id).filter(Boolean)); + const seenGroupIds = new Set(); const seenGroupNames = new Set(); const nodeToGroup = new Map(); for (const group of nodeGroups) { + // Unique group IDs + if (seenGroupIds.has(group.id)) { + throw new BadRequestError(`Duplicate node group ID "${group.id}".`); + } + seenGroupIds.add(group.id); + // Unique group names if (seenGroupNames.has(group.name)) { throw new BadRequestError(`Duplicate node group name "${group.name}".`); @@ -175,6 +257,24 @@ export function validateWorkflowNodeGroups(workflow: Pick [node.id, node])); + const connectionsBySourceNode = workflow.connections ?? {}; + + for (const group of nodeGroups) { + const groupNodes = group.nodeIds.flatMap((id) => nodeById.get(id) ?? []); + const result = validateNodeSelectionForGrouping({ + nodes: groupNodes, + connectionsBySourceNode, + getNodeType, + existingNodeGroups: nodeGroups.filter((other) => other.id !== group.id), + }); + if (!result.valid) { + throw nodeGroupValidationError(group, result); + } + } } /** diff --git a/packages/cli/src/workflows/__tests__/workflow.service.test.ts b/packages/cli/src/workflows/__tests__/workflow.service.test.ts index 330a5fd91c1..a14d1abd85f 100644 --- a/packages/cli/src/workflows/__tests__/workflow.service.test.ts +++ b/packages/cli/src/workflows/__tests__/workflow.service.test.ts @@ -302,6 +302,7 @@ describe('WorkflowService', () => { const userHasScopesMock = jest.mocked(userHasScopes); let workflowService: WorkflowService; let workflowFinderServiceMock: MockProxy; + let workflowHistoryServiceMock: MockProxy; let licenseStateMock: MockProxy; let redactionEnforcementServiceMock: MockProxy; let workflowRepositoryMock: MockProxy<{ @@ -311,6 +312,7 @@ describe('WorkflowService', () => { beforeEach(() => { workflowFinderServiceMock = mock(); + workflowHistoryServiceMock = mock(); workflowRepositoryMock = mock(); licenseStateMock = mock(); licenseStateMock.isDataRedactionLicensed.mockReturnValue(true); @@ -329,7 +331,7 @@ describe('WorkflowService', () => { mock(), // binaryDataService ownershipServiceMock, // ownershipService mock(), // tagService - mock(), // workflowHistoryService + workflowHistoryServiceMock, // workflowHistoryService mock(), // externalHooks mock(), // activeWorkflowManager mock(), // roleService @@ -436,20 +438,76 @@ describe('WorkflowService', () => { ); }); - test('should validate nodeGroups against existing workflow when not in payload', async () => { + test('validates the existing nodeGroups (full) when the graph changes but groups are omitted', async () => { + const existingNodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: ['n1'] }]; + const existingWorkflow = setupExistingWorkflow(); + existingWorkflow.nodeGroups = existingNodeGroups; + + // The getNodeType callback being passed through is the signal that full checks ran. + const getNodeTypeStub = jest.fn(); + jest.mocked(WorkflowHelpers.makeGetNodeTypeForGrouping).mockReturnValue(getNodeTypeStub); + + // Change the nodes so validation runs; omit nodeGroups so they are backfilled. + const changedNodes = [ + { id: 'n1', name: 'N1', type: 't', typeVersion: 1, position: [0, 0], parameters: {} }, + ]; + const user = mock(); + await workflowService.update( + user, + { nodes: changedNodes } as unknown as WorkflowEntity, + 'workflow-1', + { forceSave: true }, + ); + + expect(WorkflowHelpers.validateWorkflowNodeGroups).toHaveBeenCalledWith( + expect.objectContaining({ + nodes: changedNodes, + nodeGroups: existingNodeGroups, + }), + getNodeTypeStub, + ); + }); + + test('skips nodeGroup validation on a metadata-only edit (nodes/connections/groups unchanged)', async () => { + const existingWorkflow = setupExistingWorkflow(); + existingWorkflow.nodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: ['n1'] }]; + + const user = mock(); + await workflowService.update( + user, + { name: 'Renamed workflow' } as unknown as WorkflowEntity, + 'workflow-1', + { forceSave: true }, + ); + + expect(WorkflowHelpers.validateWorkflowNodeGroups).not.toHaveBeenCalled(); + }); + + test('backfills existing nodeGroups into the saved history version when omitted', async () => { const existingNodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: ['n1'] }]; const existingWorkflow = setupExistingWorkflow(); existingWorkflow.nodeGroups = existingNodeGroups; const user = mock(); - await workflowService.update(user, { nodes: [] } as unknown as WorkflowEntity, 'workflow-1', { - forceSave: true, - }); + // Change nodes (forces a new version) while omitting nodeGroups. + await workflowService.update( + user, + { + nodes: [ + { id: 'n1', name: 'N1', type: 't', typeVersion: 1, position: [0, 0], parameters: {} }, + ], + } as unknown as WorkflowEntity, + 'workflow-1', + { forceSave: true }, + ); - expect(WorkflowHelpers.validateWorkflowNodeGroups).toHaveBeenCalledWith({ - nodes: [], - nodeGroups: existingNodeGroups, - }); + // The history version must record the live (effective) groups, not empty. + expect(workflowHistoryServiceMock.saveVersion).toHaveBeenCalledWith( + user, + expect.objectContaining({ nodeGroups: existingNodeGroups }), + 'workflow-1', + false, + ); }); test('should throw BadRequestError for invalid workflow structure', async () => { diff --git a/packages/cli/src/workflows/workflow-creation.service.ts b/packages/cli/src/workflows/workflow-creation.service.ts index ffaa15ea8a4..988e398306a 100644 --- a/packages/cli/src/workflows/workflow-creation.service.ts +++ b/packages/cli/src/workflows/workflow-creation.service.ts @@ -124,7 +124,10 @@ export class WorkflowCreationService { WorkflowHelpers.addNodeIds(newWorkflow); WorkflowHelpers.resolveNodeWebhookIds(newWorkflow, this.nodeTypes); WorkflowHelpers.validateWorkflowStructure(newWorkflow); - WorkflowHelpers.validateWorkflowNodeGroups(newWorkflow); + WorkflowHelpers.validateWorkflowNodeGroups( + newWorkflow, + WorkflowHelpers.makeGetNodeTypeForGrouping(this.nodeTypes), + ); if (parentFolderId && parentFolderId !== PROJECT_ROOT) { await this.findParentFolderInProjectOrFail(parentFolderId, effectiveProjectId); diff --git a/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.test.ts b/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.test.ts index 847a6c363d1..869d4c1f11a 100644 --- a/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.test.ts +++ b/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.test.ts @@ -503,6 +503,7 @@ describe('WorkflowHistoryService', () => { workflow.id = 'wf-1'; workflow.versionId = 'wfv-fresh'; workflow.connections = {}; + workflow.nodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: ['uuid-1234'] }]; workflowRepository.findOneBy.mockResolvedValueOnce(workflow); // First findOne: no existing history row → insert path. // Second findOne: post-save verification that the row now exists. @@ -521,6 +522,7 @@ describe('WorkflowHistoryService', () => { workflowId: 'wf-1', nodes: workflow.nodes, connections: workflow.connections, + nodeGroups: workflow.nodeGroups, }), ); }); diff --git a/packages/cli/src/workflows/workflow-history/workflow-history.service.ts b/packages/cli/src/workflows/workflow-history/workflow-history.service.ts index 2311f4a70a2..5d3ab270a18 100644 --- a/packages/cli/src/workflows/workflow-history/workflow-history.service.ts +++ b/packages/cli/src/workflows/workflow-history/workflow-history.service.ts @@ -140,6 +140,7 @@ export class WorkflowHistoryService { versionId: workflow.versionId, nodes: workflow.nodes, connections: workflow.connections, + nodeGroups: workflow.nodeGroups, }, workflowId, ); diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index 95799d36290..1d183a2bd86 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -419,9 +419,11 @@ export class WorkflowService { }, ); - // To save a version, we need both nodes and connections + // A saved version needs nodes, connections, and node groups; backfill any the update + // omitted from the persisted workflow so the history row records the effective state. workflowUpdateData.nodes = workflowUpdateData.nodes ?? workflow.nodes; workflowUpdateData.connections = workflowUpdateData.connections ?? workflow.connections; + workflowUpdateData.nodeGroups = workflowUpdateData.nodeGroups ?? workflow.nodeGroups; } else { // Do not let users change versionId directly workflowUpdateData.versionId = workflow.versionId; @@ -433,10 +435,18 @@ export class WorkflowService { nodes: workflowUpdateData.nodes ?? workflow.nodes, connections: workflowUpdateData.connections ?? workflow.connections, }); - WorkflowHelpers.validateWorkflowNodeGroups({ - nodes: workflowUpdateData.nodes ?? workflow.nodes, - nodeGroups: workflowUpdateData.nodeGroups ?? workflow.nodeGroups, - }); + // Validate node groups only for structural changes; a metadata-only edit re-persists + // already-validated groups, so re-checking is redundant and could block on legacy data. + if (saveNewVersion) { + WorkflowHelpers.validateWorkflowNodeGroups( + { + nodes: workflowUpdateData.nodes, + nodeGroups: workflowUpdateData.nodeGroups, + connections: workflowUpdateData.connections, + }, + WorkflowHelpers.makeGetNodeTypeForGrouping(this.nodeTypes), + ); + } // Strip redactionPolicy if instance lacks data-redaction license if ( diff --git a/packages/cli/test/integration/public-api/workflows.test.ts b/packages/cli/test/integration/public-api/workflows.test.ts index 47280f7112d..22f3871e6ce 100644 --- a/packages/cli/test/integration/public-api/workflows.test.ts +++ b/packages/cli/test/integration/public-api/workflows.test.ts @@ -187,6 +187,7 @@ describe('GET /workflows', () => { activeVersionId, staticData, nodes, + nodeGroups, settings, name, createdAt, @@ -205,6 +206,7 @@ describe('GET /workflows', () => { expect(activeVersionId).toBeNull(); expect(staticData).toBeDefined(); expect(nodes).toBeDefined(); + expect(nodeGroups).toBeDefined(); expect(tags).toBeDefined(); expect(settings).toBeDefined(); expect(createdAt).toBeDefined(); @@ -216,6 +218,34 @@ describe('GET /workflows', () => { } }); + test('should include node groups when returning owned workflows', async () => { + const workflow = await createWorkflowWithHistory( + { + nodes: [ + { + id: 'uuid-1234', + name: 'Schedule Trigger', + parameters: {}, + position: [-20, 260], + type: 'n8n-nodes-base.scheduleTrigger', + typeVersion: 1, + }, + ], + nodeGroups: [{ id: 'group-1', name: 'Processing', nodeIds: ['uuid-1234'] }], + }, + member, + ); + + const response = await authMemberAgent.get('/workflows'); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toHaveLength(1); + expect(response.body.data[0].id).toBe(workflow.id); + expect(response.body.data[0].nodeGroups).toEqual([ + { id: 'group-1', name: 'Processing', nodeIds: ['uuid-1234'] }, + ]); + }); + test('should return all owned workflows with pagination', async () => { await Promise.all([ createWorkflowWithHistory({}, member), @@ -1581,6 +1611,54 @@ describe('POST /workflows', () => { expect(sharedWorkflow?.role).toEqual('workflow:owner'); }); + test('should create workflow with node groups', async () => { + const payload = { + name: 'grouped', + nodes: [ + triggerNode, + { + id: 'uuid-5678', + parameters: {}, + name: 'Step A', + type: 'n8n-nodes-base.noOp', + typeVersion: 1, + position: [460, 300], + }, + { + id: 'uuid-9012', + parameters: {}, + name: 'Step B', + type: 'n8n-nodes-base.noOp', + typeVersion: 1, + position: [680, 300], + }, + ], + connections: { + Start: { main: [[{ node: 'Step A', type: 'main', index: 0 }]] }, + 'Step A': { main: [[{ node: 'Step B', type: 'main', index: 0 }]] }, + }, + settings: { executionOrder: 'v1' }, + nodeGroups: [{ id: 'group-1', name: 'Processing', nodeIds: ['uuid-5678', 'uuid-9012'] }], + }; + + const response = await authOwnerAgent.post('/workflows').send(payload); + + expect(response.statusCode).toBe(200); + expect(response.body.nodeGroups).toEqual(payload.nodeGroups); + }); + + test('should reject workflow with a node group referencing a missing node', async () => { + const payload = { + ...mockPostWorkflowPayload('bad-group'), + nodeGroups: [{ id: 'group-1', name: 'Ghost', nodeIds: ['does-not-exist'] }], + }; + + const response = await authOwnerAgent.post('/workflows').send(payload); + + expect(response.statusCode).toBe(400); + expect(response.body.message).toContain('does not exist in the workflow'); + }); + test('should assign webhookId to webhook nodes created via public API', async () => { const payload = { name: 'webhook-test',