From d3173d3893ca05aa33689fee4a8854f6971e4207 Mon Sep 17 00:00:00 2001 From: mfsiega <93014743+mfsiega@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:17:18 +0000 Subject: [PATCH] fix(core): Remove the Execute Sub-workflow each-mode breaking-change rule and migration (no-changelog) (#40753) Co-authored-by: Claude Fable 5.1 --- ...breaking-changes.migration.service.test.ts | 118 ++++---- ...te-workflow-each-to-loop.migration.test.ts | 272 ------------------ ...execute-workflow-each-to-loop.migration.ts | 213 -------------- .../breaking-changes/migrations/index.ts | 3 +- .../modules/breaking-changes/rules/index.ts | 1 - .../execute-workflow-each-mode.rule.test.ts | 82 ------ .../v3/execute-workflow-each-mode.rule.ts | 60 ---- .../ExecuteWorkflow/ExecuteWorkflow.node.ts | 12 - 8 files changed, 57 insertions(+), 704 deletions(-) delete mode 100644 packages/cli/src/modules/breaking-changes/migrations/__tests__/execute-workflow-each-to-loop.migration.test.ts delete mode 100644 packages/cli/src/modules/breaking-changes/migrations/execute-workflow-each-to-loop.migration.ts delete mode 100644 packages/cli/src/modules/breaking-changes/rules/v3/__tests__/execute-workflow-each-mode.rule.test.ts delete mode 100644 packages/cli/src/modules/breaking-changes/rules/v3/execute-workflow-each-mode.rule.ts diff --git a/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.migration.service.test.ts b/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.migration.service.test.ts index 8a87760528d..9eccf1c4a59 100644 --- a/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.migration.service.test.ts +++ b/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.migration.service.test.ts @@ -20,7 +20,7 @@ import { AiTransformDeprecatedRule, AI_TRANSFORM_NODE_TYPE, } from '../rules/v3/ai-transform-deprecated.rule'; -import { ExecuteWorkflowEachModeRule } from '../rules/v3/execute-workflow-each-mode.rule'; +import type { WorkflowMigration } from '../migrations/node-migration'; import { createNode } from './test-helpers'; describe('BreakingChangeMigrationService', () => { @@ -256,77 +256,71 @@ describe('BreakingChangeMigrationService', () => { }); describe('workflow-level migrations', () => { - const EACH_RULE_ID = 'execute-workflow-each-mode-v3'; - const EXECUTE_WORKFLOW = 'n8n-nodes-base.executeWorkflow'; + // A migration that adds a node in front of each flagged node and rewires the edge. + const insertBefore: WorkflowMigration = { + ruleId: RULE_ID, + migrateWorkflow: ({ nodes, connections, affectedNodeIds }) => { + const [target] = nodes.filter((node) => affectedNodeIds.has(node.id)); + return { + nodes: [...nodes, createNode('Added', 'n8n-nodes-base.noOp')], + connections: { + ...connections, + Added: { main: [[{ node: target.name, type: 'main', index: 0 }]] }, + }, + migratedNodeIds: [target.id], + notes: ['Review the migrated workflow'], + }; + }, + }; - beforeEach(() => { - ruleRegistry.registerAll([new ExecuteWorkflowEachModeRule()]); + it('saves the graph the migration returns and requires review', async () => { + const aiNode = createNode('Transform', AI_TRANSFORM_NODE_TYPE, { jsCode: 'return items;' }); + // A plain object rather than mock: a deep mock proxy turns + // the spread of `connections` into garbage. + const workflow = { + id: 'wf-1', + name: 'My WF', + nodes: [aiNode], + versionId: 'v1', + activeVersionId: 'v1', + connections: {}, + } as unknown as WorkflowEntity; + workflowFinderService.findWorkflowForUser.mockResolvedValue(workflow); + workflowService.update.mockResolvedValue(mock({ versionId: 'new-version' })); + workflowValidationService.validateForActivation.mockResolvedValue({ isValid: true }); + vi.spyOn(migrationRegistry, 'get').mockReturnValue(insertBefore); + + const result = await service.migrateWorkflow(RULE_ID, 'wf-1', user); + + expect(result).toEqual({ + workflowId: 'wf-1', + newVersionId: 'new-version', + migratedNodeIds: [aiNode.id], + unmapped: [], + notes: ['Review the migrated workflow'], + republishable: false, + }); + + const [, updateData] = workflowService.update.mock.calls[0]; + expect(updateData.nodes.map((n) => n.name)).toEqual(['Transform', 'Added']); + expect(updateData.connections).toEqual({ + Added: { main: [[{ node: 'Transform', type: 'main', index: 0 }]] }, + }); + // Review is required even when the graph would pass activation validation. + expect(workflowValidationService.validateForActivation).not.toHaveBeenCalled(); }); - it.each([true, false])( - 'saves the graph and requires review when wait is %s', - async (waitForSubWorkflow) => { - const trigger = createNode('Trigger', 'n8n-nodes-base.scheduleTrigger'); - const sub = createNode('Sub', EXECUTE_WORKFLOW, { - mode: 'each', - options: { waitForSubWorkflow }, - }); - // A plain object rather than mock: the migration deep-copies - // `connections`, and the deep mock proxy turns that copy into garbage. - const workflow = { - id: 'wf-1', - name: 'My WF', - nodes: [trigger, sub], - versionId: 'v1', - activeVersionId: 'v1', - connections: { Trigger: { main: [[{ node: 'Sub', type: 'main', index: 0 }]] } }, - } as unknown as WorkflowEntity; - workflowFinderService.findWorkflowForUser.mockResolvedValue(workflow); - workflowService.update.mockResolvedValue( - mock({ versionId: 'new-version' }), - ); - workflowValidationService.validateForActivation.mockResolvedValue({ isValid: true }); - - const result = await service.migrateWorkflow(EACH_RULE_ID, 'wf-1', user); - - expect(result).toEqual({ - workflowId: 'wf-1', - newVersionId: 'new-version', - migratedNodeIds: [sub.id], - unmapped: [], - notes: expect.arrayContaining([ - expect.stringContaining('Review and test the migrated workflow before publishing'), - ]), - republishable: false, - }); - - const [, updateData] = workflowService.update.mock.calls[0]; - expect(updateData.nodes.map((n) => n.name)).toEqual(['Trigger', 'Loop Over Items', 'Sub']); - expect(updateData.connections).toEqual({ - Trigger: { main: [[{ node: 'Loop Over Items', type: 'main', index: 0 }]] }, - 'Loop Over Items': { - main: [[], [{ node: 'Sub', type: 'main', index: 0 }]], - }, - Sub: { main: [[{ node: 'Loop Over Items', type: 'main', index: 0 }]] }, - }); - // Review is required even when the graph would pass activation validation. - expect(workflowValidationService.validateForActivation).not.toHaveBeenCalled(); - }, - ); - it('surfaces a migration failure as a bad request without saving', async () => { - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each' }); - workflowFinderService.findWorkflowForUser.mockResolvedValue(buildWorkflow([sub])); + const aiNode = createNode('Transform', AI_TRANSFORM_NODE_TYPE, { jsCode: 'return items;' }); + workflowFinderService.findWorkflowForUser.mockResolvedValue(buildWorkflow([aiNode])); vi.spyOn(migrationRegistry, 'get').mockReturnValue({ - ruleId: EACH_RULE_ID, + ruleId: RULE_ID, migrateWorkflow: () => { throw new Error('cannot rewire'); }, }); - await expect(service.migrateWorkflow(EACH_RULE_ID, 'wf-1', user)).rejects.toThrow( - 'cannot rewire', - ); + await expect(service.migrateWorkflow(RULE_ID, 'wf-1', user)).rejects.toThrow('cannot rewire'); expect(workflowService.update).not.toHaveBeenCalled(); }); }); diff --git a/packages/cli/src/modules/breaking-changes/migrations/__tests__/execute-workflow-each-to-loop.migration.test.ts b/packages/cli/src/modules/breaking-changes/migrations/__tests__/execute-workflow-each-to-loop.migration.test.ts deleted file mode 100644 index 4970718318f..00000000000 --- a/packages/cli/src/modules/breaking-changes/migrations/__tests__/execute-workflow-each-to-loop.migration.test.ts +++ /dev/null @@ -1,272 +0,0 @@ -import type { IConnections, INode } from 'n8n-workflow'; - -import { createNode } from '../../__tests__/test-helpers'; -import { executeWorkflowEachToLoop } from '../execute-workflow-each-to-loop.migration'; - -const EXECUTE_WORKFLOW = 'n8n-nodes-base.executeWorkflow'; -const LOOP = 'n8n-nodes-base.splitInBatches'; -const NO_WAIT = { options: { waitForSubWorkflow: false } }; - -const edge = (node: string, index = 0) => ({ node, type: 'main' as const, index }); - -const migrate = (nodes: INode[], connections: IConnections, affected: INode[]) => - executeWorkflowEachToLoop.migrateWorkflow({ - nodes, - connections, - affectedNodeIds: new Set(affected.map((n) => n.id)), - }); - -describe('executeWorkflowEachToLoop migration', () => { - it('is keyed by the each-mode rule id', () => { - expect(executeWorkflowEachToLoop.ruleId).toBe('execute-workflow-each-mode-v3'); - }); - - it('wraps a waiting node in a loop and warns about extra empty items', () => { - const trigger = createNode('Trigger', 'n8n-nodes-base.manualTrigger'); - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each', workflowId: 'abc' }); - sub.position = [400, 200]; - const set = createNode('Set', 'n8n-nodes-base.set'); - const connections: IConnections = { - Trigger: { main: [[edge('Sub')]] }, - Sub: { main: [[edge('Set')]] }, - }; - - const result = migrate([trigger, sub, set], connections, [sub]); - - // The loop is inserted before the node it wraps. - expect(result.nodes.map((n) => n.name)).toEqual(['Trigger', 'Loop Over Items', 'Sub', 'Set']); - const [, loop, migratedSub] = result.nodes; - expect(loop).toMatchObject({ - type: LOOP, - typeVersion: 3, - parameters: { - batchSize: 1, - options: { reset: '={{ $node["Loop Over Items"].context["done"] ?? false }}' }, - }, - position: [400, 200], - }); - expect(loop.id).toEqual(expect.any(String)); - // The sub-workflow node switches mode, keeps its identity, moves into the loop - // body, and always emits an item so the loop cannot stall. - expect(migratedSub).toMatchObject({ - id: sub.id, - name: 'Sub', - type: EXECUTE_WORKFLOW, - parameters: { mode: 'once', workflowId: 'abc' }, - position: [640, 380], - alwaysOutputData: true, - }); - - expect(result.connections).toEqual({ - Trigger: { main: [[edge('Loop Over Items')]] }, - 'Loop Over Items': { main: [[edge('Set')], [edge('Sub')]] }, - Sub: { main: [[edge('Loop Over Items')]] }, - }); - expect(result.migratedNodeIds).toEqual([sub.id]); - expect(result.notes).toEqual([ - expect.stringContaining('Review and test the migrated workflow before publishing'), - expect.stringContaining( - 'Each sub-workflow call that returns no items produces an empty item', - ), - ]); - expect(result.unmapped).toBeUndefined(); - }); - - it('wraps a fire-and-forget node in a plain loop, without filter or alwaysOutputData', () => { - const trigger = createNode('Trigger', 'n8n-nodes-base.manualTrigger'); - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each', ...NO_WAIT }); - const set = createNode('Set', 'n8n-nodes-base.set'); - const connections: IConnections = { - Trigger: { main: [[edge('Sub')]] }, - Sub: { main: [[edge('Set')]] }, - }; - - const result = migrate([trigger, sub, set], connections, [sub]); - - expect(result.nodes.map((n) => n.name)).toEqual(['Trigger', 'Loop Over Items', 'Sub', 'Set']); - expect(result.nodes[2].alwaysOutputData).toBeUndefined(); - expect(result.notes).toEqual([ - expect.stringContaining('Review and test the migrated workflow before publishing'), - ]); - expect(result.connections).toEqual({ - Trigger: { main: [[edge('Loop Over Items')]] }, - 'Loop Over Items': { main: [[edge('Set')], [edge('Sub')]] }, - Sub: { main: [[edge('Loop Over Items')]] }, - }); - }); - - it('does not mutate its input', () => { - const trigger = createNode('Trigger', 'n8n-nodes-base.manualTrigger'); - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each' }); - const nodes = [trigger, sub]; - const connections: IConnections = { Trigger: { main: [[edge('Sub')]] } }; - const nodesSnapshot = structuredClone(nodes); - const connectionsSnapshot = structuredClone(connections); - - migrate(nodes, connections, [sub]); - - expect(nodes).toEqual(nodesSnapshot); - expect(connections).toEqual(connectionsSnapshot); - }); - - it('handles several predecessors and several successors', () => { - const a = createNode('A', 'n8n-nodes-base.set'); - const b = createNode('B', 'n8n-nodes-base.set'); - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each' }); - const c = createNode('C', 'n8n-nodes-base.set'); - const d = createNode('D', 'n8n-nodes-base.set'); - const connections: IConnections = { - A: { main: [[edge('Sub')]] }, - B: { main: [[edge('Sub'), edge('C')]] }, - Sub: { main: [[edge('C'), edge('D', 1)]] }, - }; - - const result = migrate([a, b, sub, c, d], connections, [sub]); - - expect(result.connections).toEqual({ - A: { main: [[edge('Loop Over Items')]] }, - // B's edge to C is unrelated and stays. - B: { main: [[edge('Loop Over Items'), edge('C')]] }, - 'Loop Over Items': { main: [[edge('C'), edge('D', 1)], [edge('Sub')]] }, - Sub: { main: [[edge('Loop Over Items')]] }, - }); - }); - - it('handles several successors in fire-and-forget mode without a filter', () => { - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each', ...NO_WAIT }); - const c = createNode('C', 'n8n-nodes-base.set'); - const d = createNode('D', 'n8n-nodes-base.set'); - const connections: IConnections = { Sub: { main: [[edge('C'), edge('D', 1)]] } }; - - const result = migrate([sub, c, d], connections, [sub]); - - expect(result.connections).toEqual({ - 'Loop Over Items': { main: [[edge('C'), edge('D', 1)], [edge('Sub')]] }, - Sub: { main: [[edge('Loop Over Items')]] }, - }); - }); - - it('wraps a flagged node that has no successors', () => { - const trigger = createNode('Trigger', 'n8n-nodes-base.manualTrigger'); - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each' }); - const connections: IConnections = { Trigger: { main: [[edge('Sub')]] } }; - - const result = migrate([trigger, sub], connections, [sub]); - - expect(result.connections).toEqual({ - Trigger: { main: [[edge('Loop Over Items')]] }, - 'Loop Over Items': { main: [[], [edge('Sub')]] }, - Sub: { main: [[edge('Loop Over Items')]] }, - }); - }); - - it('gives each wrapped node uniquely named helper nodes and skips unaffected nodes', () => { - const existingLoop = createNode('Loop Over Items', LOOP); - const first = createNode('First', EXECUTE_WORKFLOW, { mode: 'each' }); - const second = createNode('Second', EXECUTE_WORKFLOW, { mode: 'each' }); - const untouched = createNode('Untouched', EXECUTE_WORKFLOW, { mode: 'once' }); - // First (each) → Second (each) → Untouched (once) - const connections: IConnections = { - First: { main: [[edge('Second')]] }, - Second: { main: [[edge('Untouched')]] }, - }; - - const result = migrate([existingLoop, first, second, untouched], connections, [first, second]); - - expect(result.nodes.map((n) => n.name)).toEqual([ - 'Loop Over Items', - 'Loop Over Items1', - 'First', - 'Loop Over Items2', - 'Second', - 'Untouched', - ]); - expect(result.nodes.find((n) => n.name === 'Untouched')).toBe(untouched); - expect(result.connections).toEqual({ - 'Loop Over Items1': { main: [[edge('Loop Over Items2')], [edge('First')]] }, - First: { main: [[edge('Loop Over Items1')]] }, - 'Loop Over Items2': { main: [[edge('Untouched')], [edge('Second')]] }, - Second: { main: [[edge('Loop Over Items2')]] }, - }); - for (const name of ['Loop Over Items1', 'Loop Over Items2']) { - expect(result.nodes.find((node) => node.name === name)?.parameters.options).toEqual({ - reset: `={{ $node["${name}"].context["done"] ?? false }}`, - }); - } - expect(result.migratedNodeIds).toEqual([first.id, second.id]); - }); - - it('keeps the error output wired and warns that failed items leave the loop', () => { - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each', ...NO_WAIT }); - sub.onError = 'continueErrorOutput'; - const ok = createNode('Ok', 'n8n-nodes-base.set'); - const failed = createNode('Failed', 'n8n-nodes-base.set'); - const connections: IConnections = { - Sub: { main: [[edge('Ok')], [edge('Failed')]] }, - }; - - const result = migrate([sub, ok, failed], connections, [sub]); - - expect(result.connections.Sub).toEqual({ - main: [[edge('Loop Over Items')], [edge('Failed')]], - }); - expect(result.connections['Loop Over Items']).toEqual({ main: [[edge('Ok')], [edge('Sub')]] }); - expect(result.notes).toContainEqual(expect.stringContaining('error output')); - }); - - it('warns when "Execute Once" is enabled on the wrapped node', () => { - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each' }); - sub.executeOnce = true; - - const result = migrate([sub], {}, [sub]); - - expect(result.notes).toContainEqual(expect.stringContaining('Execute Once')); - }); - - it('warns when "Retry On Fail" is enabled on the wrapped node', () => { - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each' }); - sub.retryOnFail = true; - - const result = migrate([sub], {}, [sub]); - - expect(result.notes).toContainEqual(expect.stringContaining('Retry On Fail')); - }); - - it('warns when other nodes read the wrapped node by name in expressions', () => { - const sub = createNode('Sub WF', EXECUTE_WORKFLOW, { mode: 'each' }); - const reader = createNode('Reader', 'n8n-nodes-base.set', { - assignments: { assignments: [{ value: "={{ $('Sub WF').all().length }}" }] }, - }); - const legacyReader = createNode('Legacy', 'n8n-nodes-base.set', { - value: '={{ $node["Sub WF"].json.x }}', - }); - const unrelated = createNode('Unrelated', 'n8n-nodes-base.set', { - value: "={{ $('Other').item.json.x }}", - }); - - const result = migrate([sub, reader, legacyReader, unrelated], {}, [sub]); - - expect(result.notes).toContainEqual( - expect.stringContaining('"Reader", "Legacy" reference "Sub WF"'), - ); - // Points at the node that now carries the aggregated output. - expect(result.notes).toContainEqual(expect.stringContaining('read from "Loop Over Items"')); - }); - - it('detects dot-notation references for identifier-like node names only', () => { - const sub = createNode('Sub', EXECUTE_WORKFLOW, { mode: 'each' }); - const dotted = createNode('Dotted', 'n8n-nodes-base.set', { - value: '={{ $node.Sub.all().length }}', - }); - const longerName = createNode('Longer', 'n8n-nodes-base.set', { - value: '={{ $node.Sub2.json.x }}', - }); - - expect(migrate([sub, dotted], {}, [sub]).notes).toContainEqual( - expect.stringContaining('"Dotted" reference "Sub"'), - ); - expect(migrate([sub, longerName], {}, [sub]).notes).not.toContainEqual( - expect.stringContaining('reference "Sub"'), - ); - }); -}); diff --git a/packages/cli/src/modules/breaking-changes/migrations/execute-workflow-each-to-loop.migration.ts b/packages/cli/src/modules/breaking-changes/migrations/execute-workflow-each-to-loop.migration.ts deleted file mode 100644 index 9beee6cb633..00000000000 --- a/packages/cli/src/modules/breaking-changes/migrations/execute-workflow-each-to-loop.migration.ts +++ /dev/null @@ -1,213 +0,0 @@ -import type { IConnection, IConnections, INode, NodeInputConnections } from 'n8n-workflow'; -import { deepCopy, NodeConnectionTypes } from 'n8n-workflow'; -import { randomUUID } from 'node:crypto'; - -import type { WorkflowMigration } from './node-migration'; - -const LOOP_NODE_TYPE = 'n8n-nodes-base.splitInBatches'; -const LOOP_NODE_VERSION = 3; -const LOOP_NODE_DEFAULT_NAME = 'Loop Over Items'; -// Loop Over Items v3 outputs: index 0 fires once with every item after the last -// iteration, index 1 fires per batch with the items of that batch. -const LOOP_DONE_OUTPUT = 0; -const LOOP_BATCH_OUTPUT = 1; - -// One canvas column to the right; one row down puts a node inside the loop body. -const COLUMN_OFFSET = 240; -const ROW_OFFSET = 180; - -const uniqueNodeName = (base: string, taken: Set): string => { - if (!taken.has(base)) return base; - for (let i = 1; ; i++) { - const candidate = `${base}${i}`; - if (!taken.has(candidate)) return candidate; - } -}; - -const JS_IDENTIFIER = /^[A-Za-z_$][\w$]*$/; -const JS_IDENTIFIER_CHAR = /[\w$]/; - -/** - * Whether any string inside `value` references `nodeName` the way expressions - * do: `$('…')`, `$node['…']`, `$items('…')`, or `$node.Name` for names that are - * valid identifiers. Mirrors the access patterns the node-rename logic handles. - */ -const referencesNodeByName = (value: unknown, nodeName: string): boolean => { - const quoted = ['$(', '$node[', '$items('].flatMap((prefix) => [ - `${prefix}'${nodeName}'`, - `${prefix}"${nodeName}"`, - ]); - const dotted = JS_IDENTIFIER.test(nodeName) ? `$node.${nodeName}` : undefined; - - const matches = (text: string): boolean => { - if (quoted.some((needle) => text.includes(needle))) return true; - if (!dotted) return false; - // `$node.Sub` must not match `$node.Sub2`. - for (let from = 0; ; ) { - const at = text.indexOf(dotted, from); - if (at === -1) return false; - const next = text[at + dotted.length]; - if (next === undefined || !JS_IDENTIFIER_CHAR.test(next)) return true; - from = at + dotted.length; - } - }; - const visit = (candidate: unknown): boolean => { - if (typeof candidate === 'string') return matches(candidate); - if (Array.isArray(candidate)) return candidate.some(visit); - if (candidate && typeof candidate === 'object') return Object.values(candidate).some(visit); - return false; - }; - return visit(value); -}; - -const mainConnection = (node: string): IConnection => ({ - node, - type: NodeConnectionTypes.Main, - index: 0, -}); - -/** "Wait for sub-workflow completion" is on unless the option is explicitly off. */ -const waitsForSubWorkflow = (node: INode): boolean => { - const options = node.parameters.options; - if (!options || typeof options !== 'object') return true; - return (options as { waitForSubWorkflow?: unknown }).waitForSubWorkflow !== false; -}; - -const makeLoopNode = (name: string, position: INode['position']): INode => ({ - id: randomUUID(), - name, - type: LOOP_NODE_TYPE, - typeVersion: LOOP_NODE_VERSION, - position, - parameters: { - batchSize: 1, - // Start a new batch when an outer loop reaches this node again. - options: { reset: `={{ $node[${JSON.stringify(name)}].context["done"] ?? false }}` }, - }, -}); - -/** - * Every rewrite needs review. Specific notes help users find known changes. - * `tailName` is the node that now carries the aggregated output. - */ -const driftNotes = (node: INode, allNodes: INode[], tailName: string): string[] => { - const notes: string[] = [ - `Review and test the migrated workflow before publishing. The loop around "${node.name}" preserves the common execution pattern, but it can change behavior. Check expressions, item and run indexes, error handling, retry settings, and pinned data.`, - ]; - if (waitsForSubWorkflow(node)) { - notes.push( - `"${node.name}" now uses "Always Output Data" to keep the loop running. Each sub-workflow call that returns no items produces an empty item instead. These extra items can trigger downstream actions. Review how the workflow handles empty items.`, - ); - } - if (node.onError === 'continueErrorOutput') { - notes.push( - `"${node.name}" routes failed items to its error output. These items do not return to the loop. An error can stop the loop before it processes all inputs. Review the error path.`, - ); - } - if (node.executeOnce) { - notes.push( - `"${node.name}" has "Execute Once" enabled, which used to limit it to the first item. Inside the loop it now runs for every item; disable the loop or the setting if that is not intended.`, - ); - } - if (node.retryOnFail) { - notes.push( - `"${node.name}" has "Retry On Fail" enabled. A failure used to retry the sub-workflow for every item; inside the loop only the failing item is retried.`, - ); - } - const referencing = allNodes - .filter((other) => other.id !== node.id && referencesNodeByName(other.parameters, node.name)) - .map((other) => other.name); - if (referencing.length > 0) { - notes.push( - `${referencing.map((name) => `"${name}"`).join(', ')} reference "${node.name}" in expressions. It now runs once per loop iteration, so \`$('${node.name}').all()\` returns only the last iteration's output; read from "${tailName}" instead.`, - ); - } - return notes; -}; - -/** - * Execute Sub-workflow "Run once for each item" → Loop Over Items (batch size 1) - * wrapped around the same node in "Run once with all items" mode. - * - * This is a suggested workflow, not an equivalent replacement. Every migration - * reports a note so the user must review and test it before publishing. - * - * The rewiring for one flagged node E with predecessors P and successors S: - * P → E → S becomes P → Loop ─done→ S - * └─loop→ E → Loop - * Other outputs of E (for example an error output) keep their edges. - * - * The engine only continues past a node that produced at least one item. When E - * waits for the sub-workflow and that run returns nothing for an item, the loop - * would stall and never reach done. So in that mode E gets `alwaysOutputData`, - * which emits one empty item instead. The loop returns this item along with - * actual results, including real empty items. The migration reports this change. - * In fire-and-forget mode E echoes its input item, so the setting is not needed. - */ -export const executeWorkflowEachToLoop: WorkflowMigration = { - ruleId: 'execute-workflow-each-mode-v3', - migrateWorkflow: ({ nodes, connections, affectedNodeIds }) => { - const nextConnections: IConnections = deepCopy(connections); - const takenNames = new Set(nodes.map((node) => node.name)); - const claimName = (base: string) => { - const name = uniqueNodeName(base, takenNames); - takenNames.add(name); - return name; - }; - const nextNodes: INode[] = []; - const migratedNodeIds: string[] = []; - const notes: string[] = []; - - for (const original of nodes) { - if (!affectedNodeIds.has(original.id)) { - nextNodes.push(original); - continue; - } - - const [x, y] = original.position; - const waits = waitsForSubWorkflow(original); - - const loopNode = makeLoopNode(claimName(LOOP_NODE_DEFAULT_NAME), [x, y]); - const node: INode = { - ...original, - position: [x + COLUMN_OFFSET, y + ROW_OFFSET], - parameters: { ...original.parameters, mode: 'once' }, - ...(waits ? { alwaysOutputData: true } : {}), - }; - - // 1. Every main edge that fed the node now feeds the loop. - for (const outputs of Object.values(nextConnections)) { - for (const targets of outputs[NodeConnectionTypes.Main] ?? []) { - for (const target of targets ?? []) { - if (target.node === node.name) target.node = loopNode.name; - } - } - } - - // 2. The node's main output goes back into the loop; its old successors hang - // off "done". - const nodeOutputs = nextConnections[node.name] ?? {}; - const mainOutputs: NodeInputConnections = [...(nodeOutputs[NodeConnectionTypes.Main] ?? [])]; - const successors = mainOutputs[0] ?? []; - mainOutputs[0] = [mainConnection(loopNode.name)]; - nextConnections[node.name] = { ...nodeOutputs, [NodeConnectionTypes.Main]: mainOutputs }; - - const loopOutputs: NodeInputConnections = []; - loopOutputs[LOOP_DONE_OUTPUT] = successors; - loopOutputs[LOOP_BATCH_OUTPUT] = [mainConnection(node.name)]; - nextConnections[loopNode.name] = { [NodeConnectionTypes.Main]: loopOutputs }; - notes.push(...driftNotes(node, nodes, loopNode.name)); - - nextNodes.push(loopNode); - nextNodes.push(node); - migratedNodeIds.push(node.id); - } - - return { - nodes: nextNodes, - connections: nextConnections, - migratedNodeIds, - notes: notes.length > 0 ? notes : undefined, - }; - }, -}; diff --git a/packages/cli/src/modules/breaking-changes/migrations/index.ts b/packages/cli/src/modules/breaking-changes/migrations/index.ts index 04226929e70..db3a469c235 100644 --- a/packages/cli/src/modules/breaking-changes/migrations/index.ts +++ b/packages/cli/src/modules/breaking-changes/migrations/index.ts @@ -1,6 +1,5 @@ import { aiTransformToCode } from './ai-transform-to-code.migration'; -import { executeWorkflowEachToLoop } from './execute-workflow-each-to-loop.migration'; import type { Migration } from './node-migration'; // All registered migrations. A rule is auto-migratable only if it appears here. -export const nodeMigrations: Migration[] = [aiTransformToCode, executeWorkflowEachToLoop]; +export const nodeMigrations: Migration[] = [aiTransformToCode]; diff --git a/packages/cli/src/modules/breaking-changes/rules/index.ts b/packages/cli/src/modules/breaking-changes/rules/index.ts index 3fa097d32c4..a4f602c8e5a 100644 --- a/packages/cli/src/modules/breaking-changes/rules/index.ts +++ b/packages/cli/src/modules/breaking-changes/rules/index.ts @@ -28,7 +28,6 @@ import './v3/chat-hub-deprecated.rule'; import './v3/chat-trigger-embedded-json.rule'; import './v3/compression-node-limits.rule'; import './v3/docker-only-deployment.rule'; -import './v3/execute-workflow-each-mode.rule'; import './v3/execute-workflow-source-modes.rule'; import './v3/external-secrets-project-roles-default.rule'; import './v3/get-paired-item.rule'; diff --git a/packages/cli/src/modules/breaking-changes/rules/v3/__tests__/execute-workflow-each-mode.rule.test.ts b/packages/cli/src/modules/breaking-changes/rules/v3/__tests__/execute-workflow-each-mode.rule.test.ts deleted file mode 100644 index d809586c2bd..00000000000 --- a/packages/cli/src/modules/breaking-changes/rules/v3/__tests__/execute-workflow-each-mode.rule.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { createNode, createWorkflow } from '../../../__tests__/test-helpers'; -import { ExecuteWorkflowEachModeRule } from '../execute-workflow-each-mode.rule'; - -describe('ExecuteWorkflowEachModeRule', () => { - let rule: ExecuteWorkflowEachModeRule; - - beforeEach(() => { - rule = new ExecuteWorkflowEachModeRule(); - }); - - describe('detectWorkflow()', () => { - it('should not be affected when there is no Execute Workflow node', async () => { - const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [ - createNode('HTTP', 'n8n-nodes-base.httpRequest'), - ]); - - const result = await rule.detectWorkflow(workflow, nodesGroupedByType); - - expect(result.isAffected).toBe(false); - expect(result.issues).toHaveLength(0); - }); - - it('should not be affected when Execute Workflow node uses "once" mode', async () => { - const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [ - createNode('SubWF', 'n8n-nodes-base.executeWorkflow', { mode: 'once' }), - ]); - - const result = await rule.detectWorkflow(workflow, nodesGroupedByType); - - expect(result.isAffected).toBe(false); - expect(result.issues).toHaveLength(0); - }); - - it('should not be affected when mode is unset (defaults to "once")', async () => { - const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [ - createNode('SubWF', 'n8n-nodes-base.executeWorkflow'), - ]); - - const result = await rule.detectWorkflow(workflow, nodesGroupedByType); - - expect(result.isAffected).toBe(false); - expect(result.issues).toHaveLength(0); - }); - - it('should detect Execute Workflow node using "each" mode', async () => { - const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [ - createNode('SubWF', 'n8n-nodes-base.executeWorkflow', { mode: 'each' }), - ]); - - const result = await rule.detectWorkflow(workflow, nodesGroupedByType); - - expect(result.isAffected).toBe(true); - expect(result.issues).toHaveLength(1); - expect(result.issues[0].title).toContain('SubWF'); - expect(result.issues[0].title).toContain('Run once for each item'); - expect(result.issues[0].level).toBe('error'); - expect(result.issues[0].nodeName).toBe('SubWF'); - }); - - it('should flag only the "each" mode nodes when multiple Execute Workflow nodes exist', async () => { - const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [ - createNode('EachOne', 'n8n-nodes-base.executeWorkflow', { mode: 'each' }), - createNode('OnceOne', 'n8n-nodes-base.executeWorkflow', { mode: 'once' }), - ]); - - const result = await rule.detectWorkflow(workflow, nodesGroupedByType); - - expect(result.isAffected).toBe(true); - expect(result.issues).toHaveLength(1); - expect(result.issues[0].nodeName).toBe('EachOne'); - }); - }); - - describe('getRecommendations()', () => { - it('should recommend replacing with a Loop Over Items node', async () => { - const recommendations = await rule.getRecommendations([]); - - expect(recommendations).toHaveLength(1); - expect(recommendations[0].action).toContain('Loop Over Items'); - }); - }); -}); diff --git a/packages/cli/src/modules/breaking-changes/rules/v3/execute-workflow-each-mode.rule.ts b/packages/cli/src/modules/breaking-changes/rules/v3/execute-workflow-each-mode.rule.ts deleted file mode 100644 index dda280791e1..00000000000 --- a/packages/cli/src/modules/breaking-changes/rules/v3/execute-workflow-each-mode.rule.ts +++ /dev/null @@ -1,60 +0,0 @@ -import type { BreakingChangeAffectedWorkflow, BreakingChangeRecommendation } from '@n8n/api-types'; -import type { WorkflowEntity } from '@n8n/db'; -import { BreakingChangeRule } from '@n8n/decorators'; -import type { INode } from 'n8n-workflow'; - -import { reportAffectedNodes } from '../../detection-report'; -import type { - BreakingChangeRuleMetadata, - IBreakingChangeWorkflowRule, - WorkflowDetectionReport, -} from '../../types'; -import { BreakingChangeCategory } from '../../types'; - -const EXECUTE_WORKFLOW_NODE_TYPE = 'n8n-nodes-base.executeWorkflow'; - -@BreakingChangeRule({ version: 'v3' }) -export class ExecuteWorkflowEachModeRule implements IBreakingChangeWorkflowRule { - id = 'execute-workflow-each-mode-v3'; - - getMetadata(): BreakingChangeRuleMetadata { - return { - version: 'v3', - title: 'Execute Sub-workflow "Run once for each item" mode removed', - description: - 'The "Run once for each item" mode of the Execute Sub-workflow node is being removed. Use a Loop Over Items node before the node in "Run once with all items" mode instead.', - category: BreakingChangeCategory.workflow, - impact: 'executionsFail', - }; - } - - // eslint-disable-next-line @typescript-eslint/require-await - async getRecommendations( - _workflowResults: BreakingChangeAffectedWorkflow[], - ): Promise { - return [ - { - action: 'Replace with a Loop Over Items node', - description: - 'Add a Loop Over Items node before the flagged Execute Sub-workflow node and set its mode to "Run once with all items" to keep running the sub-workflow once per item.', - }, - ]; - } - - // eslint-disable-next-line @typescript-eslint/require-await - async detectWorkflow( - _workflow: WorkflowEntity, - nodesGroupedByType: Map, - ): Promise { - const affectedNodes = (nodesGroupedByType.get(EXECUTE_WORKFLOW_NODE_TYPE) ?? []).filter( - (node) => node.parameters.mode === 'each', - ); - - return reportAffectedNodes(affectedNodes, (node) => ({ - title: `Node '${node.name}' uses the removed "Run once for each item" mode`, - description: - 'The "Run once for each item" mode is being removed. Add a Loop Over Items node before this node and switch it to "Run once with all items" to keep running the sub-workflow once per item.', - level: 'error', - })); - } -} diff --git a/packages/nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflow/ExecuteWorkflow.node.ts b/packages/nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflow/ExecuteWorkflow.node.ts index abc13a00b96..c626d870472 100644 --- a/packages/nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflow/ExecuteWorkflow.node.ts +++ b/packages/nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflow/ExecuteWorkflow.node.ts @@ -327,18 +327,6 @@ workflowInputs: { ], default: 'once', }, - { - displayName: - '"Run once for each item" is deprecated and will be removed in a future version. To run the sub-workflow once per item, add a "Loop Over Items" node before this node and use "Run once with all items".', - name: 'eachModeDeprecationNotice', - type: 'notice', - default: '', - displayOptions: { - show: { - mode: ['each'], - }, - }, - }, { displayName: 'Options', name: 'options',