mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-12 02:19:43 +00:00
fix(core): Remove the Execute Sub-workflow each-mode breaking-change rule and migration (no-changelog) (#40753)
Co-authored-by: Claude Fable 5.1 <[email protected]>
This commit is contained in:
mfsiega
co-authored by
Claude Fable 5.1
Claude Fable 5.1
parent
19337237ec
commit
d3173d3893
+56
-62
@@ -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<WorkflowEntity>: 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<WorkflowEntity>({ 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<WorkflowEntity>: 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<WorkflowEntity>({ 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();
|
||||
});
|
||||
});
|
||||
|
||||
-272
@@ -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"'),
|
||||
);
|
||||
});
|
||||
});
|
||||
-213
@@ -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>): 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,
|
||||
};
|
||||
},
|
||||
};
|
||||
@@ -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];
|
||||
|
||||
@@ -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';
|
||||
|
||||
-82
@@ -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');
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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<BreakingChangeRecommendation[]> {
|
||||
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<string, INode[]>,
|
||||
): Promise<WorkflowDetectionReport> {
|
||||
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',
|
||||
}));
|
||||
}
|
||||
}
|
||||
@@ -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',
|
||||
|
||||
Reference in New Issue
Block a user