mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-28 07:17:04 +02:00
fix(core): Introduce batch workflow rule to fix subworkflow with wait node detection (#22447)
This commit is contained in:
Guillaume Jacquart
GitHub
parent
3edb952aae
commit
e0bc4416ea
|
|
@ -35,7 +35,7 @@ const affectedWorkflowSchema = z.object({
|
|||
active: z.boolean(),
|
||||
numberOfExecutions: z.number(),
|
||||
lastUpdatedAt: z.date(),
|
||||
lastExecutedAt: z.date().nullable(),
|
||||
lastExecutedAt: z.date().optional(),
|
||||
issues: z.array(workflowIssueSchema),
|
||||
});
|
||||
export type BreakingChangeAffectedWorkflow = z.infer<typeof affectedWorkflowSchema>;
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import { createNode, createWorkflow } from './test-helpers';
|
|||
import { FileAccessRule } from '../rules/v2/file-access.rule';
|
||||
import { ProcessEnvAccessRule } from '../rules/v2/process-env-access.rule';
|
||||
import { RemovedNodesRule } from '../rules/v2/removed-nodes.rule';
|
||||
import { WaitNodeSubworkflowRule } from '../rules/v2/wait-node-subworkflow.rule';
|
||||
|
||||
describe('BreakingChangeService', () => {
|
||||
const logger = mockLogger();
|
||||
|
|
@ -120,6 +121,38 @@ describe('BreakingChangeService', () => {
|
|||
expect(report.report).toHaveProperty('workflowResults');
|
||||
expect(Array.isArray(report.report.workflowResults)).toBe(true);
|
||||
});
|
||||
|
||||
it('should aggregate results from batch rules', async () => {
|
||||
// Register the batch rule
|
||||
const waitNodeRule = new WaitNodeSubworkflowRule();
|
||||
ruleRegistry.registerAll([waitNodeRule]);
|
||||
|
||||
// Create a sub-workflow with ExecuteWorkflowTrigger and Wait node
|
||||
const { workflow: subWorkflow } = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('Execute Workflow Trigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
// Create a parent workflow that calls the sub-workflow
|
||||
const { workflow: parentWorkflow } = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('Execute Workflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: 'sub-wf-1',
|
||||
}),
|
||||
]);
|
||||
|
||||
workflowRepository.find.mockResolvedValue([subWorkflow, parentWorkflow] as never);
|
||||
workflowRepository.count.mockResolvedValue(2);
|
||||
|
||||
const report = await service.detect('v2');
|
||||
|
||||
const waitNodeResult = report.report.workflowResults.find(
|
||||
(r) => r.ruleId === 'wait-node-subworkflow-v2',
|
||||
);
|
||||
expect(waitNodeResult).toBeDefined();
|
||||
expect(waitNodeResult?.affectedWorkflows).toHaveLength(1);
|
||||
expect(waitNodeResult?.affectedWorkflows[0].id).toBe('parent-wf-1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getDetectionResults()', () => {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import type { BreakingChangeAffectedWorkflow } from '@n8n/api-types';
|
||||
import {
|
||||
BreakingChangeAffectedWorkflow,
|
||||
BreakingChangeInstanceRuleResult,
|
||||
BreakingChangeReportResult,
|
||||
BreakingChangeVersion,
|
||||
|
|
@ -10,18 +10,31 @@ import { Time } from '@n8n/constants';
|
|||
import { WorkflowRepository } from '@n8n/db';
|
||||
import { Container, Service } from '@n8n/di';
|
||||
import { ErrorReporter } from 'n8n-core';
|
||||
import { INode } from 'n8n-workflow';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
|
||||
import { RuleRegistry } from './breaking-changes.rule-registry.service';
|
||||
import { allRules, RuleInstances } from './rules';
|
||||
import type { IBreakingChangeWorkflowRule, IBreakingChangeInstanceRule } from './types';
|
||||
import type {
|
||||
IBreakingChangeBatchWorkflowRule,
|
||||
IBreakingChangeInstanceRule,
|
||||
IBreakingChangeWorkflowRule,
|
||||
} from './types';
|
||||
import { N8N_VERSION } from '../../constants';
|
||||
|
||||
import { CacheService } from '@/services/cache/cache.service';
|
||||
|
||||
interface WorkflowMetadata {
|
||||
name: string;
|
||||
active: boolean;
|
||||
numberOfExecutions: number;
|
||||
lastExecutedAt?: Date;
|
||||
lastUpdatedAt: Date;
|
||||
}
|
||||
|
||||
@Service()
|
||||
export class BreakingChangeService {
|
||||
private readonly batchSize = 100;
|
||||
private static readonly REPORT_DURATION_CACHE_THRESHOLD = Time.seconds.toMilliseconds * 2;
|
||||
private static readonly CACHE_KEY_PREFIX = 'breaking-changes:results:';
|
||||
private readonly ongoingDetections = new Map<
|
||||
BreakingChangeVersion,
|
||||
|
|
@ -72,77 +85,28 @@ export class BreakingChangeService {
|
|||
return instanceLevelResults;
|
||||
}
|
||||
|
||||
private async getAllWorkflowRulesResults(
|
||||
workflowLevelRules: IBreakingChangeWorkflowRule[],
|
||||
totalWorkflows: number,
|
||||
): Promise<BreakingChangeWorkflowRuleResult[]> {
|
||||
const allAffectedWorkflowsByRule: Map<string, BreakingChangeAffectedWorkflow[]> = new Map();
|
||||
const allResults: BreakingChangeWorkflowRuleResult[] = [];
|
||||
|
||||
this.logger.debug('Processing workflows in batches', {
|
||||
totalWorkflows,
|
||||
batchSize: this.batchSize,
|
||||
});
|
||||
|
||||
// Process workflows in batches
|
||||
for (let skip = 0; skip < totalWorkflows; skip += this.batchSize) {
|
||||
const workflows = await this.workflowRepository.find({
|
||||
select: ['id', 'name', 'active', 'activeVersionId', 'nodes', 'updatedAt', 'statistics'],
|
||||
skip,
|
||||
take: this.batchSize,
|
||||
order: { id: 'ASC' },
|
||||
relations: {
|
||||
statistics: true,
|
||||
},
|
||||
});
|
||||
|
||||
this.logger.debug('Processing batch', {
|
||||
skip,
|
||||
workflowsInBatch: workflows.length,
|
||||
});
|
||||
|
||||
for (const workflow of workflows) {
|
||||
const nodesGroupedByType: Map<string, INode[]> = new Map();
|
||||
for (const node of workflow.nodes) {
|
||||
if (!nodesGroupedByType.has(node.type)) {
|
||||
nodesGroupedByType.set(node.type, []);
|
||||
}
|
||||
nodesGroupedByType.get(node.type)!.push(node);
|
||||
}
|
||||
for (const rule of workflowLevelRules) {
|
||||
const workflowDetectionResult = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
if (workflowDetectionResult.isAffected) {
|
||||
const affectedWorkflow: BreakingChangeAffectedWorkflow = {
|
||||
id: workflow.id,
|
||||
name: workflow.name,
|
||||
active: !!workflow.activeVersionId,
|
||||
issues: workflowDetectionResult.issues,
|
||||
numberOfExecutions: workflow.statistics.reduce(
|
||||
(acc, cur) => acc + (cur.count || 0),
|
||||
0,
|
||||
),
|
||||
lastExecutedAt: workflow.statistics.sort(
|
||||
(a, b) => b.latestEvent.getTime() - a.latestEvent.getTime(),
|
||||
)[0]?.latestEvent,
|
||||
lastUpdatedAt: workflow.updatedAt,
|
||||
};
|
||||
if (!allAffectedWorkflowsByRule.has(rule.id)) {
|
||||
allAffectedWorkflowsByRule.set(rule.id, [affectedWorkflow]);
|
||||
} else {
|
||||
allAffectedWorkflowsByRule.get(rule.id)!.push(affectedWorkflow);
|
||||
}
|
||||
}
|
||||
}
|
||||
private groupNodesByType(nodes: INode[]): Map<string, INode[]> {
|
||||
const nodesGroupedByType: Map<string, INode[]> = new Map();
|
||||
for (const node of nodes) {
|
||||
if (!nodesGroupedByType.has(node.type)) {
|
||||
nodesGroupedByType.set(node.type, []);
|
||||
}
|
||||
nodesGroupedByType.get(node.type)!.push(node);
|
||||
}
|
||||
return nodesGroupedByType;
|
||||
}
|
||||
|
||||
// Aggregate results
|
||||
private async aggregateRegularRuleResults(
|
||||
workflowLevelRules: IBreakingChangeWorkflowRule[],
|
||||
allAffectedWorkflowsByRule: Map<string, BreakingChangeAffectedWorkflow[]>,
|
||||
): Promise<BreakingChangeWorkflowRuleResult[]> {
|
||||
const results: BreakingChangeWorkflowRuleResult[] = [];
|
||||
for (const rule of workflowLevelRules) {
|
||||
const workflowResults = allAffectedWorkflowsByRule.get(rule.id) || [];
|
||||
const workflowResults = allAffectedWorkflowsByRule.get(rule.id) ?? [];
|
||||
const isAffected = workflowResults.some((wr) => wr.issues.length > 0);
|
||||
|
||||
if (isAffected) {
|
||||
allResults.push({
|
||||
results.push({
|
||||
ruleId: rule.id,
|
||||
ruleTitle: rule.getMetadata().title,
|
||||
ruleDescription: rule.getMetadata().description,
|
||||
|
|
@ -153,7 +117,130 @@ export class BreakingChangeService {
|
|||
});
|
||||
}
|
||||
}
|
||||
return allResults;
|
||||
return results;
|
||||
}
|
||||
|
||||
private async aggregateBatchRuleResults(
|
||||
batchRules: IBreakingChangeBatchWorkflowRule[],
|
||||
workflowMetadataMap: Map<string, WorkflowMetadata>,
|
||||
): Promise<BreakingChangeWorkflowRuleResult[]> {
|
||||
const results: BreakingChangeWorkflowRuleResult[] = [];
|
||||
for (const rule of batchRules) {
|
||||
const batchReport = await rule.produceReport();
|
||||
if (batchReport.affectedWorkflows.length === 0) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const affectedWorkflows: BreakingChangeAffectedWorkflow[] = [];
|
||||
for (const affected of batchReport.affectedWorkflows) {
|
||||
const metadata = workflowMetadataMap.get(affected.workflowId);
|
||||
if (!metadata) {
|
||||
this.logger.warn('Workflow metadata not found for batch rule result', {
|
||||
workflowId: affected.workflowId,
|
||||
ruleId: rule.id,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
affectedWorkflows.push({
|
||||
id: affected.workflowId,
|
||||
name: metadata.name,
|
||||
active: metadata.active,
|
||||
issues: affected.issues,
|
||||
numberOfExecutions: metadata.numberOfExecutions,
|
||||
lastExecutedAt: metadata.lastExecutedAt,
|
||||
lastUpdatedAt: metadata.lastUpdatedAt,
|
||||
});
|
||||
}
|
||||
|
||||
if (affectedWorkflows.length > 0) {
|
||||
results.push({
|
||||
ruleId: rule.id,
|
||||
ruleTitle: rule.getMetadata().title,
|
||||
ruleDescription: rule.getMetadata().description,
|
||||
ruleSeverity: rule.getMetadata().severity,
|
||||
ruleDocumentationUrl: rule.getMetadata().documentationUrl,
|
||||
affectedWorkflows,
|
||||
recommendations: await rule.getRecommendations(affectedWorkflows),
|
||||
});
|
||||
}
|
||||
}
|
||||
return results;
|
||||
}
|
||||
|
||||
private async getAllWorkflowRulesResults(
|
||||
workflowLevelRules: IBreakingChangeWorkflowRule[],
|
||||
batchRules: IBreakingChangeBatchWorkflowRule[],
|
||||
totalWorkflows: number,
|
||||
): Promise<BreakingChangeWorkflowRuleResult[]> {
|
||||
const allAffectedWorkflowsByRule: Map<string, BreakingChangeAffectedWorkflow[]> = new Map();
|
||||
const workflowMetadataMap: Map<string, WorkflowMetadata> = new Map();
|
||||
|
||||
// Reset batch rules internal state before processing
|
||||
batchRules.forEach((rule) => rule.reset());
|
||||
|
||||
this.logger.debug('Processing workflows in batches', {
|
||||
totalWorkflows,
|
||||
batchSize: this.batchSize,
|
||||
regularRulesCount: workflowLevelRules.length,
|
||||
batchRulesCount: batchRules.length,
|
||||
});
|
||||
|
||||
for (let skip = 0; skip < totalWorkflows; skip += this.batchSize) {
|
||||
const workflows = await this.workflowRepository.find({
|
||||
select: ['id', 'name', 'active', 'activeVersionId', 'nodes', 'updatedAt', 'statistics'],
|
||||
skip,
|
||||
take: this.batchSize,
|
||||
order: { id: 'ASC' },
|
||||
relations: { statistics: true },
|
||||
});
|
||||
|
||||
this.logger.debug('Processing batch', { skip, workflowsInBatch: workflows.length });
|
||||
|
||||
for (const workflow of workflows) {
|
||||
const nodesGroupedByType = this.groupNodesByType(workflow.nodes);
|
||||
|
||||
const workflowMetadata: WorkflowMetadata = {
|
||||
name: workflow.name,
|
||||
active: !!workflow.activeVersionId,
|
||||
numberOfExecutions: workflow.statistics.reduce((acc, cur) => acc + (cur.count || 0), 0),
|
||||
lastExecutedAt: workflow.statistics.sort(
|
||||
(a, b) => b.latestEvent.getTime() - a.latestEvent.getTime(),
|
||||
)[0]?.latestEvent,
|
||||
lastUpdatedAt: workflow.updatedAt,
|
||||
};
|
||||
workflowMetadataMap.set(workflow.id, workflowMetadata);
|
||||
|
||||
for (const rule of workflowLevelRules) {
|
||||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
if (result.isAffected) {
|
||||
const affectedWorkflow: BreakingChangeAffectedWorkflow = {
|
||||
id: workflow.id,
|
||||
issues: result.issues,
|
||||
...workflowMetadata,
|
||||
};
|
||||
const existing = allAffectedWorkflowsByRule.get(rule.id);
|
||||
if (existing) {
|
||||
existing.push(affectedWorkflow);
|
||||
} else {
|
||||
allAffectedWorkflowsByRule.set(rule.id, [affectedWorkflow]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for (const rule of batchRules) {
|
||||
await rule.collectWorkflowData(workflow, nodesGroupedByType);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const regularResults = await this.aggregateRegularRuleResults(
|
||||
workflowLevelRules,
|
||||
allAffectedWorkflowsByRule,
|
||||
);
|
||||
const batchResults = await this.aggregateBatchRuleResults(batchRules, workflowMetadataMap);
|
||||
|
||||
return regularResults.concat(batchResults);
|
||||
}
|
||||
|
||||
async refreshDetectionResults(
|
||||
|
|
@ -208,7 +295,7 @@ export class BreakingChangeService {
|
|||
}
|
||||
|
||||
private shouldCacheDetection(durationMs: number): boolean {
|
||||
return durationMs > Time.seconds.toMilliseconds * 10;
|
||||
return durationMs > BreakingChangeService.REPORT_DURATION_CACHE_THRESHOLD;
|
||||
}
|
||||
|
||||
async detect(targetVersion: BreakingChangeVersion): Promise<BreakingChangeReportResult> {
|
||||
|
|
@ -217,14 +304,21 @@ export class BreakingChangeService {
|
|||
|
||||
const rules = this.ruleRegistry.getRules(targetVersion);
|
||||
|
||||
const workflowLevelRules = rules.filter((rule) => 'detectWorkflow' in rule);
|
||||
const instanceLevelRules = rules.filter((rule) => 'detect' in rule);
|
||||
const workflowLevelRules = rules.filter(
|
||||
(rule): rule is IBreakingChangeWorkflowRule => 'detectWorkflow' in rule,
|
||||
);
|
||||
const batchWorkflowRules = rules.filter(
|
||||
(rule): rule is IBreakingChangeBatchWorkflowRule => 'collectWorkflowData' in rule,
|
||||
);
|
||||
const instanceLevelRules = rules.filter(
|
||||
(rule): rule is IBreakingChangeInstanceRule => 'detect' in rule,
|
||||
);
|
||||
|
||||
const totalWorkflows = await this.workflowRepository.count();
|
||||
|
||||
const [instanceLevelResults, workflowLevelResults] = await Promise.all([
|
||||
this.getAllInstanceRulesResults(instanceLevelRules),
|
||||
this.getAllWorkflowRulesResults(workflowLevelRules, totalWorkflows),
|
||||
this.getAllWorkflowRulesResults(workflowLevelRules, batchWorkflowRules, totalWorkflows),
|
||||
]);
|
||||
|
||||
const report = this.createDetectionReport(
|
||||
|
|
@ -253,9 +347,13 @@ export class BreakingChangeService {
|
|||
return undefined;
|
||||
}
|
||||
|
||||
const totalWorkflows = await this.workflowRepository.count();
|
||||
|
||||
if ('detectWorkflow' in rule) {
|
||||
const totalWorkflows = await this.workflowRepository.count();
|
||||
return (await this.getAllWorkflowRulesResults([rule], totalWorkflows))[0];
|
||||
return (await this.getAllWorkflowRulesResults([rule], [], totalWorkflows))[0];
|
||||
}
|
||||
if ('collectWorkflowData' in rule) {
|
||||
return (await this.getAllWorkflowRulesResults([], [rule], totalWorkflows))[0];
|
||||
}
|
||||
return (await this.getAllInstanceRulesResults([rule]))[0];
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ describe('WaitNodeSubworkflowRule', () => {
|
|||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
rule = new WaitNodeSubworkflowRule();
|
||||
rule.reset();
|
||||
});
|
||||
|
||||
describe('getMetadata()', () => {
|
||||
|
|
@ -16,9 +17,8 @@ describe('WaitNodeSubworkflowRule', () => {
|
|||
|
||||
expect(metadata).toMatchObject({
|
||||
version: 'v2',
|
||||
title: 'Waiting node behavior change in sub-workflows',
|
||||
description:
|
||||
'Waiting nodes (Wait, Form, and HITL nodes) in sub-workflows now return data from the last node instead of the node before the waiting node',
|
||||
title: 'Sub-workflow waiting node output behavior change',
|
||||
description: expect.stringContaining('Parent workflows calling sub-workflows'),
|
||||
category: BreakingChangeCategory.workflow,
|
||||
severity: 'medium',
|
||||
});
|
||||
|
|
@ -29,252 +29,393 @@ describe('WaitNodeSubworkflowRule', () => {
|
|||
it('should return recommendations', async () => {
|
||||
const recommendations = await rule.getRecommendations([]);
|
||||
|
||||
expect(recommendations).toHaveLength(3);
|
||||
expect(recommendations).toEqual([
|
||||
{
|
||||
action: 'Review sub-workflow output handling',
|
||||
description:
|
||||
'Check workflows that use Execute Workflow node to call sub-workflows containing waiting nodes (Wait, Form, or HITL nodes). The output data structure may have changed.',
|
||||
},
|
||||
{
|
||||
action: 'Update downstream logic',
|
||||
description:
|
||||
'Adjust any logic in parent workflows that depends on the data returned from sub-workflows with waiting nodes, as it now returns the last node data instead of the node before the waiting node.',
|
||||
},
|
||||
{
|
||||
action: 'Test affected workflows',
|
||||
description:
|
||||
'Test all workflows with Execute Workflow nodes calling sub-workflows that contain waiting nodes to ensure the new behavior works as expected.',
|
||||
},
|
||||
]);
|
||||
expect(recommendations).toHaveLength(2);
|
||||
expect(recommendations[0].action).toBe('Review Execute Workflow node outputs');
|
||||
expect(recommendations[1].action).toBe('Test affected parent workflows');
|
||||
});
|
||||
});
|
||||
|
||||
describe('detectWorkflow()', () => {
|
||||
it.each([
|
||||
{
|
||||
description: 'workflow has no waiting nodes',
|
||||
nodes: [
|
||||
createNode('HTTP', 'n8n-nodes-base.httpRequest'),
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
],
|
||||
},
|
||||
{
|
||||
description: 'workflow has waiting nodes but no Execute Workflow Trigger',
|
||||
nodes: [
|
||||
createNode('HTTP', 'n8n-nodes-base.httpRequest'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
],
|
||||
},
|
||||
{
|
||||
description: 'workflow has Execute Workflow Trigger but no waiting nodes',
|
||||
nodes: [
|
||||
createNode('HTTP', 'n8n-nodes-base.httpRequest'),
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
],
|
||||
},
|
||||
])('should return no issues when $description', async ({ nodes }) => {
|
||||
const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', nodes);
|
||||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
|
||||
expect(result).toEqual({
|
||||
isAffected: false,
|
||||
issues: [],
|
||||
});
|
||||
});
|
||||
|
||||
it('should detect sub-workflow with Wait nodes', async () => {
|
||||
const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [
|
||||
describe('batch workflow detection', () => {
|
||||
it('should flag parent workflow when it calls a sub-workflow with waiting nodes', async () => {
|
||||
// Create a sub-workflow with waiting nodes
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
createNode('HTTP', 'n8n-nodes-base.httpRequest'),
|
||||
]);
|
||||
|
||||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
|
||||
expect(result.isAffected).toBe(true);
|
||||
expect(result.issues).toHaveLength(1);
|
||||
expect(result.issues[0].nodeId).toBeDefined();
|
||||
expect(result.issues[0].nodeName).toBe('Wait');
|
||||
});
|
||||
|
||||
it('should detect sub-workflow with multiple Wait nodes', async () => {
|
||||
const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait1', 'n8n-nodes-base.wait'),
|
||||
createNode('Wait2', 'n8n-nodes-base.wait'),
|
||||
createNode('HTTP', 'n8n-nodes-base.httpRequest'),
|
||||
// Create a parent workflow that calls the sub-workflow
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('Start', 'n8n-nodes-base.manualTrigger'),
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
}),
|
||||
]);
|
||||
|
||||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
// Collect data from both workflows
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
|
||||
expect(result.isAffected).toBe(true);
|
||||
expect(result.issues).toHaveLength(2);
|
||||
expect(result.issues[0].nodeId).toBeDefined();
|
||||
expect(result.issues[0].nodeName).toBe('Wait1');
|
||||
expect(result.issues[1].nodeId).toBeDefined();
|
||||
expect(result.issues[1].nodeName).toBe('Wait2');
|
||||
// Produce report
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(1);
|
||||
expect(report.affectedWorkflows[0].workflowId).toBe('parent-wf-1');
|
||||
expect(report.affectedWorkflows[0].issues).toHaveLength(1);
|
||||
expect(report.affectedWorkflows[0].issues[0].nodeName).toBe('ExecuteWorkflow');
|
||||
expect(report.affectedWorkflows[0].issues[0].description).toContain('sub-wf-1');
|
||||
});
|
||||
|
||||
it('should detect complex sub-workflow with Wait among other nodes', async () => {
|
||||
const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [
|
||||
it('should NOT flag sub-workflow itself', async () => {
|
||||
// Create a sub-workflow with waiting nodes (no parent calling it)
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
// Sub-workflow should NOT be in the affected list - only parents should be flagged
|
||||
expect(report.affectedWorkflows).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should NOT flag parent when waitForSubWorkflow is false', async () => {
|
||||
// Create a sub-workflow with waiting nodes
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
// Create a parent workflow that does NOT wait for sub-workflow completion
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('Start', 'n8n-nodes-base.manualTrigger'),
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
options: { waitForSubWorkflow: false },
|
||||
}),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should flag parent when waitForSubWorkflow is true (explicit)', async () => {
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
options: { waitForSubWorkflow: true },
|
||||
}),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(1);
|
||||
expect(report.affectedWorkflows[0].workflowId).toBe('parent-wf-1');
|
||||
});
|
||||
|
||||
it('should flag parent when waitForSubWorkflow is not set (defaults to true)', async () => {
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
// No options set - waitForSubWorkflow defaults to true
|
||||
}),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should flag parent when waitForSubWorkflow is an expression (treated as true)', async () => {
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
options: { waitForSubWorkflow: '={{ $json.shouldWait }}' },
|
||||
}),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
// Expression is treated as true to avoid false negatives
|
||||
expect(report.affectedWorkflows).toHaveLength(1);
|
||||
expect(report.affectedWorkflows[0].workflowId).toBe('parent-wf-1');
|
||||
});
|
||||
|
||||
it('should NOT flag parent when sub-workflow has no waiting nodes', async () => {
|
||||
// Create a sub-workflow WITHOUT waiting nodes
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('HTTP', 'n8n-nodes-base.httpRequest'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
createNode('Code', 'n8n-nodes-base.code'),
|
||||
createNode('Set', 'n8n-nodes-base.set'),
|
||||
]);
|
||||
|
||||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
|
||||
expect(result.isAffected).toBe(true);
|
||||
expect(result.issues).toHaveLength(1);
|
||||
expect(result.issues[0].level).toBe('warning');
|
||||
expect(result.issues[0].nodeId).toBeDefined();
|
||||
expect(result.issues[0].nodeName).toBe('Wait');
|
||||
});
|
||||
|
||||
it('should not detect regular workflow with Wait and Execute Workflow nodes', async () => {
|
||||
const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [
|
||||
createNode('HTTP', 'n8n-nodes-base.httpRequest'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow'),
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
}),
|
||||
]);
|
||||
|
||||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(result.isAffected).toBe(false);
|
||||
expect(result.issues).toHaveLength(0);
|
||||
expect(report.affectedWorkflows).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should detect sub-workflow with Form node', async () => {
|
||||
const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [
|
||||
it('should NOT flag parent when sub-workflow has waiting nodes but no ExecuteWorkflowTrigger', async () => {
|
||||
// A workflow with waiting nodes but NOT a sub-workflow (no trigger)
|
||||
const regularWorkflow = createWorkflow('regular-wf', 'Regular Workflow', [
|
||||
createNode('ManualTrigger', 'n8n-nodes-base.manualTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'regular-wf' },
|
||||
}),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(regularWorkflow.workflow, regularWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should handle workflowId as string', async () => {
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: 'sub-wf-1', // String instead of object
|
||||
}),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should flag when workflowId is an expression (dynamic call)', async () => {
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: '={{ $json.workflowId }}', // Expression - can't evaluate statically
|
||||
}),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
// Should flag with "may call" warning since we can't determine the workflow ID
|
||||
expect(report.affectedWorkflows).toHaveLength(1);
|
||||
expect(report.affectedWorkflows[0].workflowId).toBe('parent-wf-1');
|
||||
expect(report.affectedWorkflows[0].issues[0].title).toContain('may call');
|
||||
expect(report.affectedWorkflows[0].issues[0].description).toContain('dynamically');
|
||||
});
|
||||
|
||||
it('should flag when source is not database (dynamic call)', async () => {
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'parameter', // JSON parameter source
|
||||
workflowJson: '{}',
|
||||
}),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
// Should flag with "may call" warning since we can't determine the workflow ID
|
||||
expect(report.affectedWorkflows).toHaveLength(1);
|
||||
expect(report.affectedWorkflows[0].workflowId).toBe('parent-wf-1');
|
||||
expect(report.affectedWorkflows[0].issues[0].title).toContain('may call');
|
||||
});
|
||||
|
||||
it('should flag parent when sub-workflow has Form node', async () => {
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Form', 'n8n-nodes-base.form'),
|
||||
createNode('HTTP', 'n8n-nodes-base.httpRequest'),
|
||||
]);
|
||||
|
||||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(result.isAffected).toBe(true);
|
||||
expect(result.issues).toHaveLength(1);
|
||||
expect(result.issues[0].description).toContain('form');
|
||||
expect(result.issues[0].nodeId).toBeDefined();
|
||||
expect(result.issues[0].nodeName).toBe('Form');
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(1);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
nodeName: 'Slack',
|
||||
nodeType: 'n8n-nodes-base.slack',
|
||||
operation: 'sendAndWait',
|
||||
expectedInDescription: 'slack',
|
||||
},
|
||||
])(
|
||||
'should detect sub-workflow with $nodeName HITL node using $operation operation',
|
||||
async ({ nodeName, nodeType, operation, expectedInDescription }) => {
|
||||
const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode(nodeName, nodeType, { operation }),
|
||||
createNode('HTTP', 'n8n-nodes-base.httpRequest'),
|
||||
]);
|
||||
|
||||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
|
||||
expect(result.isAffected).toBe(true);
|
||||
expect(result.issues).toHaveLength(1);
|
||||
expect(result.issues[0].description).toContain(expectedInDescription);
|
||||
expect(result.issues[0].nodeId).toBeDefined();
|
||||
expect(result.issues[0].nodeName).toBe(nodeName);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
{
|
||||
nodeName: 'Slack',
|
||||
nodeType: 'n8n-nodes-base.slack',
|
||||
nonWaitingOperation: 'sendMessage',
|
||||
},
|
||||
{
|
||||
nodeName: 'Telegram',
|
||||
nodeType: 'n8n-nodes-base.telegram',
|
||||
nonWaitingOperation: 'sendMessage',
|
||||
},
|
||||
{
|
||||
nodeName: 'GitHub',
|
||||
nodeType: 'n8n-nodes-base.github',
|
||||
nonWaitingOperation: 'getIssue',
|
||||
},
|
||||
])(
|
||||
'should NOT detect sub-workflow with $nodeName node without $waitingOperation operation',
|
||||
async ({ nodeName, nodeType, nonWaitingOperation }) => {
|
||||
const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode(nodeName, nodeType, { operation: nonWaitingOperation }),
|
||||
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 detect sub-workflow with multiple HITL node types', async () => {
|
||||
const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [
|
||||
it('should flag parent when sub-workflow has HITL node with sendAndWait operation', async () => {
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
createNode('Form', 'n8n-nodes-base.form'),
|
||||
createNode('Slack', 'n8n-nodes-base.slack', { operation: 'sendAndWait' }),
|
||||
createNode('HTTP', 'n8n-nodes-base.httpRequest'),
|
||||
]);
|
||||
|
||||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(result.isAffected).toBe(true);
|
||||
expect(result.issues).toHaveLength(3);
|
||||
expect(result.issues[0].description).toContain('wait');
|
||||
expect(result.issues[0].nodeId).toBeDefined();
|
||||
expect(result.issues[0].nodeName).toBe('Wait');
|
||||
expect(result.issues[1].description).toContain('form');
|
||||
expect(result.issues[1].nodeId).toBeDefined();
|
||||
expect(result.issues[1].nodeName).toBe('Form');
|
||||
expect(result.issues[2].description).toContain('slack');
|
||||
expect(result.issues[2].nodeId).toBeDefined();
|
||||
expect(result.issues[2].nodeName).toBe('Slack');
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(1);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ nodeName: 'Telegram', nodeType: 'n8n-nodes-base.telegram', operation: 'sendAndWait' },
|
||||
{ nodeName: 'EmailSend', nodeType: 'n8n-nodes-base.emailSend', operation: 'sendAndWait' },
|
||||
{
|
||||
nodeName: 'MicrosoftTeams',
|
||||
nodeType: 'n8n-nodes-base.microsoftTeams',
|
||||
operation: 'sendAndWait',
|
||||
},
|
||||
{
|
||||
nodeName: 'MicrosoftOutlook',
|
||||
nodeType: 'n8n-nodes-base.microsoftOutlook',
|
||||
operation: 'sendAndWait',
|
||||
},
|
||||
{ nodeName: 'Discord', nodeType: 'n8n-nodes-base.discord', operation: 'sendAndWait' },
|
||||
{ nodeName: 'GitHub', nodeType: 'n8n-nodes-base.github', operation: 'dispatchAndWait' },
|
||||
])(
|
||||
'should detect sub-workflow with $nodeName HITL node using $operation',
|
||||
async ({ nodeName, nodeType, operation }) => {
|
||||
const { workflow, nodesGroupedByType } = createWorkflow('wf-1', 'Test Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode(nodeName, nodeType, { operation }),
|
||||
]);
|
||||
it('should NOT flag when HITL node does not use waiting operation', async () => {
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Slack', 'n8n-nodes-base.slack', { operation: 'sendMessage' }), // Not sendAndWait
|
||||
]);
|
||||
|
||||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(result.isAffected).toBe(true);
|
||||
expect(result.issues).toHaveLength(1);
|
||||
expect(result.issues[0].nodeId).toBeDefined();
|
||||
expect(result.issues[0].nodeName).toBe(nodeName);
|
||||
},
|
||||
);
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should flag parent calling multiple affected sub-workflows', async () => {
|
||||
const subWorkflow1 = createWorkflow('sub-wf-1', 'Sub Workflow 1', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
const subWorkflow2 = createWorkflow('sub-wf-2', 'Sub Workflow 2', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Form', 'n8n-nodes-base.form'),
|
||||
]);
|
||||
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow1', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
}),
|
||||
createNode('ExecuteWorkflow2', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-2' },
|
||||
}),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(subWorkflow1.workflow, subWorkflow1.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(subWorkflow2.workflow, subWorkflow2.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(1);
|
||||
expect(report.affectedWorkflows[0].workflowId).toBe('parent-wf-1');
|
||||
expect(report.affectedWorkflows[0].issues).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('should flag multiple parents calling the same affected sub-workflow', async () => {
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
const parentWorkflow1 = createWorkflow('parent-wf-1', 'Parent Workflow 1', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
}),
|
||||
]);
|
||||
|
||||
const parentWorkflow2 = createWorkflow('parent-wf-2', 'Parent Workflow 2', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
}),
|
||||
]);
|
||||
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow1.workflow, parentWorkflow1.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow2.workflow, parentWorkflow2.nodesGroupedByType);
|
||||
const report = await rule.produceReport();
|
||||
|
||||
expect(report.affectedWorkflows).toHaveLength(2);
|
||||
const workflowIds = report.affectedWorkflows.map((w) => w.workflowId);
|
||||
expect(workflowIds).toContain('parent-wf-1');
|
||||
expect(workflowIds).toContain('parent-wf-2');
|
||||
});
|
||||
|
||||
it('should reset state correctly', async () => {
|
||||
const subWorkflow = createWorkflow('sub-wf-1', 'Sub Workflow', [
|
||||
createNode('ExecuteWorkflowTrigger', 'n8n-nodes-base.executeWorkflowTrigger'),
|
||||
createNode('Wait', 'n8n-nodes-base.wait'),
|
||||
]);
|
||||
|
||||
const parentWorkflow = createWorkflow('parent-wf-1', 'Parent Workflow', [
|
||||
createNode('ExecuteWorkflow', 'n8n-nodes-base.executeWorkflow', {
|
||||
source: 'database',
|
||||
workflowId: { value: 'sub-wf-1' },
|
||||
}),
|
||||
]);
|
||||
|
||||
// First run
|
||||
await rule.collectWorkflowData(subWorkflow.workflow, subWorkflow.nodesGroupedByType);
|
||||
await rule.collectWorkflowData(parentWorkflow.workflow, parentWorkflow.nodesGroupedByType);
|
||||
const report1 = await rule.produceReport();
|
||||
expect(report1.affectedWorkflows).toHaveLength(1);
|
||||
|
||||
// Reset
|
||||
rule.reset();
|
||||
|
||||
// Second run should be empty without collecting data again
|
||||
const report2 = await rule.produceReport();
|
||||
expect(report2.affectedWorkflows).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,20 +1,34 @@
|
|||
import type { BreakingChangeAffectedWorkflow, BreakingChangeRecommendation } from '@n8n/api-types';
|
||||
import type {
|
||||
BreakingChangeAffectedWorkflow,
|
||||
BreakingChangeRecommendation,
|
||||
BreakingChangeWorkflowIssue,
|
||||
} from '@n8n/api-types';
|
||||
import type { WorkflowEntity } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import type { INode, INodeParameters } from 'n8n-workflow';
|
||||
import { SEND_AND_WAIT_OPERATION } from 'n8n-workflow';
|
||||
|
||||
import type {
|
||||
BatchWorkflowDetectionReport,
|
||||
BreakingChangeRuleMetadata,
|
||||
IBreakingChangeWorkflowRule,
|
||||
WorkflowDetectionReport,
|
||||
IBreakingChangeBatchWorkflowRule,
|
||||
} from '../../types';
|
||||
import { BreakingChangeCategory } from '../../types';
|
||||
|
||||
interface ParentWorkflowInfo {
|
||||
parentWorkflowId: string;
|
||||
executeWorkflowNode: INode;
|
||||
calledWorkflowId?: string;
|
||||
}
|
||||
|
||||
@Service()
|
||||
export class WaitNodeSubworkflowRule implements IBreakingChangeWorkflowRule {
|
||||
export class WaitNodeSubworkflowRule implements IBreakingChangeBatchWorkflowRule {
|
||||
id: string = 'wait-node-subworkflow-v2';
|
||||
|
||||
// Internal state for batch processing
|
||||
private subWorkflowsWithWaitingNodes: Map<string, string> = new Map(); // workflowId -> workflowName
|
||||
private parentWorkflowsCalling: ParentWorkflowInfo[] = [];
|
||||
|
||||
// Configuration for node types and their waiting conditions
|
||||
private readonly waitingNodeConfig: Array<{ nodeTypes: string[]; operation?: string }> = [
|
||||
{
|
||||
|
|
@ -51,13 +65,13 @@ export class WaitNodeSubworkflowRule implements IBreakingChangeWorkflowRule {
|
|||
getMetadata(): BreakingChangeRuleMetadata {
|
||||
return {
|
||||
version: 'v2',
|
||||
title: 'Waiting node behavior change in sub-workflows',
|
||||
title: 'Sub-workflow waiting node output behavior change',
|
||||
description:
|
||||
'Waiting nodes (Wait, Form, and HITL nodes) in sub-workflows now return data from the last node instead of the node before the waiting node',
|
||||
'Parent workflows calling sub-workflows with waiting nodes (Wait, Form, HITL) now receive correct data. Previously, incorrect results were returned when the sub-workflow entered a waiting state.',
|
||||
category: BreakingChangeCategory.workflow,
|
||||
severity: 'medium',
|
||||
documentationUrl:
|
||||
'https://docs.n8n.io/2-0-breaking-changes/#return-expected-sub-workflow-data-when-it-contains-a-wait-node',
|
||||
'https://docs.n8n.io/2-0-breaking-changes/#return-expected-sub-workflow-data-when-the-sub-workflow-resumes-from-waiting-waiting-for-webhook-forms-hitl-etc',
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -66,80 +80,155 @@ export class WaitNodeSubworkflowRule implements IBreakingChangeWorkflowRule {
|
|||
): Promise<BreakingChangeRecommendation[]> {
|
||||
return [
|
||||
{
|
||||
action: 'Review sub-workflow output handling',
|
||||
action: 'Review Execute Workflow node outputs',
|
||||
description:
|
||||
'Check workflows that use Execute Workflow node to call sub-workflows containing waiting nodes (Wait, Form, or HITL nodes). The output data structure may have changed.',
|
||||
'Check the Execute Workflow nodes flagged above. If your workflow logic depends on the specific data returned from sub-workflows containing waiting nodes, verify the data structure is correct.',
|
||||
},
|
||||
{
|
||||
action: 'Update downstream logic',
|
||||
action: 'Test affected parent workflows',
|
||||
description:
|
||||
'Adjust any logic in parent workflows that depends on the data returned from sub-workflows with waiting nodes, as it now returns the last node data instead of the node before the waiting node.',
|
||||
},
|
||||
{
|
||||
action: 'Test affected workflows',
|
||||
description:
|
||||
'Test all workflows with Execute Workflow nodes calling sub-workflows that contain waiting nodes to ensure the new behavior works as expected.',
|
||||
'Run the affected parent workflows to verify the data returned from sub-workflows with waiting nodes is now correct and matches your expectations.',
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
private hasWaitingOperation(node: INode, requiredOperation: string): boolean {
|
||||
const operation = node.parameters.operation;
|
||||
return operation === requiredOperation;
|
||||
reset(): void {
|
||||
this.subWorkflowsWithWaitingNodes.clear();
|
||||
this.parentWorkflowsCalling = [];
|
||||
}
|
||||
|
||||
async detectWorkflow(
|
||||
_workflow: WorkflowEntity,
|
||||
async collectWorkflowData(
|
||||
workflow: WorkflowEntity,
|
||||
nodesGroupedByType: Map<string, INode[]>,
|
||||
): Promise<WorkflowDetectionReport> {
|
||||
// Check if the workflow contains any waiting nodes (Wait, Form, or HITL nodes)
|
||||
const foundWaitingNodes: Array<{ node: INode; nodeTypeName: string }> = [];
|
||||
): Promise<void> {
|
||||
// Check if this workflow IS a sub-workflow with waiting nodes
|
||||
const hasExecuteWorkflowTrigger =
|
||||
(nodesGroupedByType.get('n8n-nodes-base.executeWorkflowTrigger')?.length ?? 0) > 0;
|
||||
const hasWaitingNodes = this.findWaitingNodes(nodesGroupedByType).length > 0;
|
||||
|
||||
if (hasExecuteWorkflowTrigger && hasWaitingNodes) {
|
||||
this.subWorkflowsWithWaitingNodes.set(workflow.id, workflow.name);
|
||||
}
|
||||
|
||||
// Check if this workflow CALLS sub-workflows with waitForSubWorkflow enabled
|
||||
const executeWorkflowNodes = nodesGroupedByType.get('n8n-nodes-base.executeWorkflow') ?? [];
|
||||
for (const node of executeWorkflowNodes) {
|
||||
// Check if waitForSubWorkflow is enabled (default is true)
|
||||
const options = node.parameters.options as INodeParameters | undefined;
|
||||
const waitForSubWorkflowValue = options?.waitForSubWorkflow;
|
||||
// If waitForSubWorkflow is explicitly false, skip. Otherwise treat as true (default).
|
||||
// Expressions (strings starting with =) are treated as true to avoid false negatives.
|
||||
const isExplicitlyFalse = waitForSubWorkflowValue === false;
|
||||
if (isExplicitlyFalse) {
|
||||
continue; // Skip if not waiting for sub-workflow completion
|
||||
}
|
||||
|
||||
const calledWorkflowId = this.extractCalledWorkflowId(node);
|
||||
|
||||
this.parentWorkflowsCalling.push({
|
||||
parentWorkflowId: workflow.id,
|
||||
executeWorkflowNode: node,
|
||||
calledWorkflowId,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
async produceReport(): Promise<BatchWorkflowDetectionReport> {
|
||||
// Group issues by parent workflow ID (a parent may call multiple affected sub-workflows)
|
||||
const issuesByParentWorkflow = new Map<string, BreakingChangeWorkflowIssue[]>();
|
||||
|
||||
// For each parent workflow calling a sub-workflow, check if it calls an affected sub-workflow
|
||||
for (const parent of this.parentWorkflowsCalling) {
|
||||
const isUnknownWorkflow = parent.calledWorkflowId === undefined;
|
||||
const subWorkflowName =
|
||||
parent.calledWorkflowId !== undefined
|
||||
? this.subWorkflowsWithWaitingNodes.get(parent.calledWorkflowId)
|
||||
: undefined;
|
||||
const isKnownAffectedWorkflow = subWorkflowName !== undefined;
|
||||
|
||||
if (!isUnknownWorkflow && !isKnownAffectedWorkflow) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const issue: BreakingChangeWorkflowIssue = {
|
||||
title: isUnknownWorkflow
|
||||
? 'Execute Workflow node may call sub-workflow with changed output behavior'
|
||||
: 'Execute Workflow node calls sub-workflow with changed output behavior',
|
||||
description: isUnknownWorkflow
|
||||
? `The "${parent.executeWorkflowNode.name}" node calls a sub-workflow dynamically (via expression or parameter). If the called sub-workflow contains waiting nodes (Wait, Form, Human-in-the-loop), the data returned has changed in v2 - it now returns the correct data instead of the previously incorrect results.`
|
||||
: `The "${parent.executeWorkflowNode.name}" node calls sub-workflow "${subWorkflowName}" (ID: ${parent.calledWorkflowId}) which contains waiting nodes. The data returned from this sub-workflow has changed in v2 - it now returns the correct data instead of the previously incorrect results.`,
|
||||
level: 'warning',
|
||||
nodeId: parent.executeWorkflowNode.id,
|
||||
nodeName: parent.executeWorkflowNode.name,
|
||||
};
|
||||
|
||||
const existingIssues = issuesByParentWorkflow.get(parent.parentWorkflowId);
|
||||
if (existingIssues) {
|
||||
existingIssues.push(issue);
|
||||
} else {
|
||||
issuesByParentWorkflow.set(parent.parentWorkflowId, [issue]);
|
||||
}
|
||||
}
|
||||
|
||||
// Convert to the expected format
|
||||
const affectedWorkflows: BatchWorkflowDetectionReport['affectedWorkflows'] = [];
|
||||
for (const [workflowId, issues] of issuesByParentWorkflow) {
|
||||
affectedWorkflows.push({ workflowId, issues });
|
||||
}
|
||||
|
||||
return { affectedWorkflows };
|
||||
}
|
||||
|
||||
private findWaitingNodes(nodesGroupedByType: Map<string, INode[]>): INode[] {
|
||||
const waitingNodes: INode[] = [];
|
||||
|
||||
// Check all configured node types
|
||||
for (const { nodeTypes, operation } of this.waitingNodeConfig) {
|
||||
for (const nodeType of nodeTypes) {
|
||||
const nodes = nodesGroupedByType.get(nodeType) ?? [];
|
||||
|
||||
// If no operation is specified, all nodes of this type wait
|
||||
// Otherwise, filter for nodes with the specific operation
|
||||
const waitingNodes = operation
|
||||
? nodes.filter((node) => this.hasWaitingOperation(node, operation))
|
||||
const matchingNodes = operation
|
||||
? nodes.filter((node) => node.parameters.operation === operation)
|
||||
: nodes;
|
||||
|
||||
for (const node of waitingNodes) {
|
||||
const nodeTypeName = nodeType.split('.').pop() ?? nodeType;
|
||||
foundWaitingNodes.push({ node, nodeTypeName });
|
||||
}
|
||||
waitingNodes.push(...matchingNodes);
|
||||
}
|
||||
}
|
||||
|
||||
if (foundWaitingNodes.length === 0) {
|
||||
return { isAffected: false, issues: [] };
|
||||
return waitingNodes;
|
||||
}
|
||||
|
||||
private extractCalledWorkflowId(node: INode): string | undefined {
|
||||
const source = node.parameters.source as string | undefined;
|
||||
|
||||
// Only handle database source - for other sources we can't determine the workflow ID statically
|
||||
if (source !== 'database' && source !== undefined) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Check if this workflow IS a subworkflow by looking for Execute Workflow Trigger
|
||||
const executeWorkflowTriggerNodes =
|
||||
nodesGroupedByType.get('n8n-nodes-base.executeWorkflowTrigger') ?? [];
|
||||
// Default source is 'database', so also handle when source is undefined
|
||||
const workflowId = node.parameters.workflowId;
|
||||
|
||||
if (executeWorkflowTriggerNodes.length === 0) {
|
||||
return { isAffected: false, issues: [] };
|
||||
if (typeof workflowId === 'string') {
|
||||
// Check if it's an expression (starts with =)
|
||||
if (workflowId.startsWith('=')) {
|
||||
return undefined; // Can't evaluate expressions statically
|
||||
}
|
||||
return workflowId;
|
||||
}
|
||||
|
||||
// This workflow is a subworkflow (has Execute Workflow Trigger) and contains waiting nodes
|
||||
// The output behavior has changed
|
||||
// Create one issue per waiting node
|
||||
if (typeof workflowId === 'object' && workflowId !== null && 'value' in workflowId) {
|
||||
const value = workflowId.value;
|
||||
if (typeof value === 'string') {
|
||||
// Check if it's an expression (starts with =)
|
||||
if (value.startsWith('=')) {
|
||||
return undefined; // Can't evaluate expressions statically
|
||||
}
|
||||
return value;
|
||||
}
|
||||
}
|
||||
|
||||
const issues = foundWaitingNodes.map(({ node, nodeTypeName }) => ({
|
||||
title: 'Sub-workflow with waiting node has changed output behavior',
|
||||
description: `This workflow is a sub-workflow (contains Execute Workflow Trigger) with a waiting node (${nodeTypeName}). The data returned to the parent workflow from sub-workflows containing waiting nodes has changed. Previously, the child workflow returned data from the node before the waiting node. Now they return data from the last node in the workflow.`,
|
||||
level: 'warning' as const,
|
||||
nodeId: node.id,
|
||||
nodeName: node.name,
|
||||
}));
|
||||
|
||||
return {
|
||||
isAffected: true,
|
||||
issues,
|
||||
};
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,3 +14,14 @@ export interface InstanceDetectionReport {
|
|||
instanceIssues: BreakingChangeInstanceIssue[];
|
||||
recommendations: BreakingChangeRecommendation[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Report returned by batch workflow rules after processing all workflows.
|
||||
* Used when a rule needs to correlate data across multiple workflows before producing results.
|
||||
*/
|
||||
export interface BatchWorkflowDetectionReport {
|
||||
affectedWorkflows: Array<{
|
||||
workflowId: string;
|
||||
issues: BreakingChangeWorkflowIssue[];
|
||||
}>;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,7 +7,11 @@ import type {
|
|||
import type { WorkflowEntity } from '@n8n/db';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
|
||||
import type { InstanceDetectionReport, WorkflowDetectionReport } from './detection.types';
|
||||
import type {
|
||||
BatchWorkflowDetectionReport,
|
||||
InstanceDetectionReport,
|
||||
WorkflowDetectionReport,
|
||||
} from './detection.types';
|
||||
|
||||
export const enum BreakingChangeCategory {
|
||||
workflow = 'workflow',
|
||||
|
|
@ -45,4 +49,43 @@ export interface IBreakingChangeWorkflowRule {
|
|||
): Promise<WorkflowDetectionReport>;
|
||||
}
|
||||
|
||||
export type IBreakingChangeRule = IBreakingChangeInstanceRule | IBreakingChangeWorkflowRule;
|
||||
/**
|
||||
* Interface for batch-based workflow rules that need to correlate data across multiple workflows.
|
||||
* Unlike IBreakingChangeWorkflowRule which processes each workflow independently,
|
||||
* batch rules collect data from all workflows first, then produce a final report.
|
||||
*
|
||||
* Use case example: Detecting parent workflows that call sub-workflows with specific characteristics,
|
||||
* where the rule needs to identify both the sub-workflows and their callers before reporting.
|
||||
*/
|
||||
export interface IBreakingChangeBatchWorkflowRule {
|
||||
id: string;
|
||||
getMetadata(): BreakingChangeRuleMetadata;
|
||||
getRecommendations(
|
||||
workflowResults: BreakingChangeAffectedWorkflow[],
|
||||
): Promise<BreakingChangeRecommendation[]>;
|
||||
|
||||
/**
|
||||
* Called for each workflow during the scanning phase.
|
||||
* The rule should collect and store any relevant data internally.
|
||||
*/
|
||||
collectWorkflowData(
|
||||
workflow: WorkflowEntity,
|
||||
nodesGroupedByType: Map<string, INode[]>,
|
||||
): Promise<void>;
|
||||
|
||||
/**
|
||||
* Called after all workflows have been scanned to produce the final report.
|
||||
* The rule should correlate the collected data and return the affected workflows.
|
||||
*/
|
||||
produceReport(): Promise<BatchWorkflowDetectionReport>;
|
||||
|
||||
/**
|
||||
* Called to reset internal state before a new detection run.
|
||||
*/
|
||||
reset(): void;
|
||||
}
|
||||
|
||||
export type IBreakingChangeRule =
|
||||
| IBreakingChangeInstanceRule
|
||||
| IBreakingChangeWorkflowRule
|
||||
| IBreakingChangeBatchWorkflowRule;
|
||||
|
|
|
|||
|
|
@ -40,7 +40,6 @@ const mockWorkflowWithMultipleNodes = {
|
|||
active: false,
|
||||
numberOfExecutions: 50,
|
||||
lastUpdatedAt: new Date('2024-01-10'),
|
||||
lastExecutedAt: null,
|
||||
issues: [
|
||||
{
|
||||
nodeId: 'node-2',
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user