diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index adb9667c0be..84133cde173 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -113,3 +113,15 @@ export type { } from './schemas/external-secrets.schema'; export type { UsageState } from './schemas/usage.schema'; + +export type { + BreakingChangeRuleSeverity, + BreakingChangeRecommendation, + BreakingChangeAffectedWorkflow, + BreakingChangeInstanceIssue, + BreakingChangeWorkflowIssue, + BreakingChangeInstanceRuleResult, + BreakingChangeWorkflowRuleResult, + BreakingChangeReportResult, + BreakingChangeLightReportResult, +} from './schemas/breaking-changes.schema'; diff --git a/packages/@n8n/api-types/src/schemas/breaking-changes.schema.ts b/packages/@n8n/api-types/src/schemas/breaking-changes.schema.ts new file mode 100644 index 00000000000..5aeb9863380 --- /dev/null +++ b/packages/@n8n/api-types/src/schemas/breaking-changes.schema.ts @@ -0,0 +1,95 @@ +import { z } from 'zod'; + +// Enums +export const breakingChangeRuleSeveritySchema = z.enum(['low', 'medium', 'high', 'critical']); +export type BreakingChangeRuleSeverity = z.infer; + +export const breakingChangeIssueLevelSchema = z.enum(['info', 'warning', 'error']); + +// Common schemas +const recommendationSchema = z.object({ + action: z.string(), + description: z.string(), +}); +export type BreakingChangeRecommendation = z.infer; + +const instanceIssueSchema = z.object({ + title: z.string(), + description: z.string(), + level: breakingChangeIssueLevelSchema, +}); +export type BreakingChangeInstanceIssue = z.infer; + +const workflowIssueSchema = instanceIssueSchema.extend({ + nodeId: z.string().optional(), + nodeName: z.string().optional(), +}); +export type BreakingChangeWorkflowIssue = z.infer; + +const affectedWorkflowSchema = z.object({ + id: z.string(), + name: z.string(), + active: z.boolean(), + numberOfExecutions: z.number(), + lastUpdatedAt: z.date(), + lastExecutedAt: z.date().nullable(), + issues: z.array(workflowIssueSchema), +}); +export type BreakingChangeAffectedWorkflow = z.infer; + +const ruleResultBaseSchema = z.object({ + ruleId: z.string(), + ruleTitle: z.string(), + ruleDescription: z.string(), + ruleSeverity: breakingChangeRuleSeveritySchema, + ruleDocumentationUrl: z.string().optional(), + recommendations: z.array(recommendationSchema), +}); + +const instanceRuleResultsSchema = ruleResultBaseSchema.extend({ + instanceIssues: z.array(instanceIssueSchema), +}); +export type BreakingChangeInstanceRuleResult = z.infer; + +const workflowRuleResultsSchema = ruleResultBaseSchema.extend({ + affectedWorkflows: z.array(affectedWorkflowSchema), +}); +export type BreakingChangeWorkflowRuleResult = z.infer; + +const breakingChangeReportDataSchema = { + generatedAt: z.date(), + targetVersion: z.string(), + currentVersion: z.string(), + instanceResults: z.array(instanceRuleResultsSchema), + workflowResults: z.array(workflowRuleResultsSchema), +} as const; + +const breakingChangeReportSchema = z.object(breakingChangeReportDataSchema).strict(); + +const breakingChangeLightReportDataSchema = { + generatedAt: z.date(), + targetVersion: z.string(), + currentVersion: z.string(), + instanceResults: z.array(instanceRuleResultsSchema), + workflowResults: z.array( + workflowRuleResultsSchema.omit({ affectedWorkflows: true }).extend({ + nbAffectedWorkflows: z.number(), + }), + ), +} as const; + +const breakingChangeLightReportSchema = z.object(breakingChangeLightReportDataSchema).strict(); + +const breakingChangeReportResultDataSchema = z.object({ + report: breakingChangeReportSchema, + shouldCache: z.boolean(), +}); +export type BreakingChangeReportResult = z.infer; + +const breakingChangeLightReportResultDataSchema = z.object({ + report: breakingChangeLightReportSchema, + shouldCache: z.boolean(), +}); +export type BreakingChangeLightReportResult = z.infer< + typeof breakingChangeLightReportResultDataSchema +>; diff --git a/packages/cli/src/modules/breaking-changes/README.md b/packages/cli/src/modules/breaking-changes/README.md index e4b43364b68..7b69fe4795d 100644 --- a/packages/cli/src/modules/breaking-changes/README.md +++ b/packages/cli/src/modules/breaking-changes/README.md @@ -15,7 +15,6 @@ breaking-changes/ detection.types.ts # Report types index.ts rules/ - abstract-rule.ts # Base class for all rules v2/ # V2-specific rules removed-nodes.rule.ts process-env-access.rule.ts @@ -32,42 +31,98 @@ breaking-changes/ ### Detect Breaking Changes ``` -GET /breaking-changes?version=v2 +GET /breaking-changes/report?version=v2 ``` Returns: ```json { + "report": { "generatedAt": "2025-10-17T00:00:00.000Z", "targetVersion": "v2", "currentVersion": "1.x.x", - "totalIssues": 5, - "criticalIssues": 2, - "affectedWorkflowCount": 4, - "results": [ - { - "ruleId": "removed-nodes-v2", - "isAffected": true, - "affectedWorkflows": [ - { - "id": "wf-001", - "name": "Customer Onboarding", - "active": true, - "reason": "Uses removed nodes: n8n-nodes-base.spontit" - } - ], - "instanceIssues": [], - "recommendations": [ - { - "action": "Update affected workflows", - "description": "Replace removed nodes with alternatives" - } + "instanceResults": [ + { + "ruleId": "process-env-access-v2", + "ruleTitle": "Process Environment Access Restrictions", + "ruleDescription": "Access to process.env is now restricted", + "ruleSeverity": "high", + "instanceIssues": [ + { + "title": "Environment access detected", + "description": "Workflows using process.env may be affected", + "level": "warning" + } + ], + "recommendations": [ + { + "action": "Review environment variable usage", + "description": "Update workflows to use secure environment variable access" + } + ] + } + ], + "workflowResults": [ + { + "ruleId": "removed-nodes-v2", + "ruleTitle": "Removed Deprecated Nodes", + "ruleDescription": "Several deprecated nodes have been removed", + "ruleSeverity": "critical", + "affectedWorkflows": [ + { + "id": "wf-001", + "name": "Customer Onboarding", + "active": true, + "numberOfExecutions": 142, + "lastUpdatedAt": "2025-10-15T10:30:00.000Z", + "lastExecutedAt": "2025-10-16T14:22:00.000Z", + "issues": [ + { + "title": "Node 'n8n-nodes-base.spontit' with name 'Spontit' has been removed", + "description": "The node type 'n8n-nodes-base.spontit' is no longer available", + "level": "error", + "nodeId": "node-123", + "nodeName": "Spontit" + } ] - } + } + ], + "recommendations": [ + { + "action": "Update affected workflows", + "description": "Replace removed nodes with their updated versions or alternatives" + } + ] + } ] + }, + "shouldCache": false } ``` +## Rule Types + +The system supports two types of rules: + +### Workflow Rules (`IBreakingChangeWorkflowRule`) + +- **Purpose**: Check individual workflows for breaking changes +- **Methods**: + - `getMetadata()`: Returns rule metadata (version, title, description, severity, etc.) + - `detectWorkflow(workflow, nodesGroupedByType)`: Checks a single workflow and returns issues + - `getRecommendations(workflowResults)`: Returns recommendations based on detected issues +- **Returns**: `WorkflowDetectionReport` with workflow-specific issues +- **Example Use Cases**: Removed nodes, deprecated node features, changed node behavior + +### Instance Rules (`IBreakingChangeInstanceRule`) + +- **Purpose**: Check instance-level configuration and environment +- **Methods**: + - `getMetadata()`: Returns rule metadata (version, title, description, severity, etc.) + - `detect()`: Checks the entire instance and returns issues +- **Returns**: `InstanceDetectionReport` with instance-level issues and recommendations +- **Example Use Cases**: Environment variable requirements, database version checks, configuration changes + ## Adding Rules to the System ### Option 1: Adding a Rule to an Existing Version @@ -76,64 +131,131 @@ If you're adding a new breaking change rule for an existing version (e.g., v2), #### Step 1: Create the Rule Class -Create your rule file in the appropriate version directory (e.g., `rules/v2/my-new-rule.rule.ts`): +There are two types of rules you can create: + +##### A. Workflow Rules (for checking workflows) + +Create your rule file in the appropriate version directory (e.g., `rules/v2/my-workflow-rule.rule.ts`): ```typescript +import { BreakingChangeRecommendation } from '@n8n/api-types'; +import type { WorkflowEntity } from '@n8n/db'; import { Service } from '@n8n/di'; -import { Logger } from '@n8n/backend-common'; -import { ErrorReporter } from 'n8n-core'; +import { INode } from 'n8n-workflow'; -import type { DetectionResult, BreakingChangeMetadata } from '../../types'; -import { BreakingChangeSeverity, BreakingChangeCategory } from '../../types'; +import type { + BreakingChangeRuleMetadata, + IBreakingChangeWorkflowRule, + WorkflowDetectionReport, +} from '../../types'; +import { BreakingChangeCategory } from '../../types'; @Service() -export class MyNewRule extends IBreakingChangeInstanceRule { - constructor( - protected readonly logger: Logger, - protected errorReporter: ErrorReporter, - ) { - super(logger); - } +export class MyWorkflowRule implements IBreakingChangeWorkflowRule { + id: string = 'my-workflow-rule-v2'; - getMetadata(): BreakingChangeMetadata { - return { - id: 'my-rule-v2', - version: 'v2', - title: 'My Breaking Change', - description: 'Description of what changed', - category: BreakingChangeCategory.WORKFLOW, - severity: BreakingChangeSeverity.HIGH, - documentationUrl: 'https://docs.n8n.io/migration/v2/...', - }; - } + getMetadata(): BreakingChangeRuleMetadata { + return { + version: 'v2', + title: 'My Workflow Breaking Change', + description: 'Description of what changed in workflows', + category: BreakingChangeCategory.workflow, + severity: 'high', + documentationUrl: 'https://docs.n8n.io/migration/v2/...', + }; + } - async detect(): Promise { - const result = this.createEmptyResult(this.getMetadata().id); + async getRecommendations(): Promise { + return [ + { + action: 'Update affected workflows', + description: 'Replace or update the affected nodes', + }, + ]; + } - try { - // Instance-level issues - result.instanceIssues.push({ - type: 'configuration', - description: 'Database version XYZ is deprecated', - severity: BreakingChangeSeverity.HIGH, - }); + async detectWorkflow( + _workflow: WorkflowEntity, + nodesGroupedByType: Map, + ): Promise { + // Check if workflow uses specific node types + const affectedNodes = nodesGroupedByType.get('n8n-nodes-base.someNode') ?? []; - result.isAffected = - result.instanceIssues.length > 0; + if (affectedNodes.length === 0) { + return { isAffected: false, issues: [] }; + } - if (result.isAffected) { - result.recommendations.push({ - action: 'What to do', - description: 'How to fix it', - documentationUrl: 'https://docs.n8n.io/...', - }); - } - } catch (error) { - this.errorReporter.error(error, { shouldBeLogged: true}); - } + return { + isAffected: true, + issues: affectedNodes.map((node) => ({ + title: `Node '${node.type}' with name '${node.name}' is affected`, + description: 'This node requires updates for v2 compatibility', + level: 'warning', + nodeId: node.id, + nodeName: node.name, + })), + }; + } +} +``` - return result; - } +##### B. Instance Rules (for checking instance configuration) + +Create your rule file in the appropriate version directory (e.g., `rules/v2/my-instance-rule.rule.ts`): + +```typescript +import { BreakingChangeRecommendation } from '@n8n/api-types'; +import { Service } from '@n8n/di'; + +import type { + BreakingChangeRuleMetadata, + IBreakingChangeInstanceRule, + InstanceDetectionReport, +} from '../../types'; +import { BreakingChangeCategory } from '../../types'; + +@Service() +export class MyInstanceRule implements IBreakingChangeInstanceRule { + id: string = 'my-instance-rule-v2'; + + getMetadata(): BreakingChangeRuleMetadata { + return { + version: 'v2', + title: 'My Instance Breaking Change', + description: 'Description of what changed at instance level', + category: BreakingChangeCategory.instance, + severity: 'critical', + documentationUrl: 'https://docs.n8n.io/migration/v2/...', + }; + } + + async detect(): Promise { + const instanceIssues = []; + const recommendations = []; + + // Check instance-level configuration + // Example: check environment variables, database version, etc. + const hasIssue = false; // Your detection logic here + + if (hasIssue) { + instanceIssues.push({ + title: 'Configuration issue detected', + description: 'Database version XYZ is no longer supported', + level: 'error', + }); + + recommendations.push({ + action: 'Upgrade database', + description: 'Update to database version ABC or higher', + }); + } + + return { + isAffected: instanceIssues.length > 0, + instanceIssues, + recommendations, + }; + } } ``` @@ -148,10 +270,10 @@ import { RemovedNodesRule } from './removed-nodes.rule'; import { MyNewRule } from './my-new-rule.rule'; // Import your rule const v2Rules = [ - RemovedNodesRule, - ProcessEnvAccessRule, - FileAccessRule, - MyNewRule, // Add to array + RemovedNodesRule, + ProcessEnvAccessRule, + FileAccessRule, + MyNewRule, // Add to array ]; export { v2Rules }; @@ -187,9 +309,9 @@ import { MySecondV3Rule } from './my-second-v3-rule.rule'; // Import all v3 rules const v3Rules = [ - MyFirstV3Rule, - MySecondV3Rule, - // Add all v3 rules here + MyFirstV3Rule, + MySecondV3Rule, + // Add all v3 rules here ]; export { v3Rules }; @@ -211,24 +333,3 @@ type RuleInstances = InstanceType; export { allRules, type RuleInstances }; ``` - -## Rule Categories - -- **WORKFLOW**: Changes affecting workflow execution -- **INSTANCE**: Changes to instance configuration -- **ENVIRONMENT**: Changes to environment variables -- **DATABASE**: Changes to database requirements -- **INFRASTRUCTURE**: Changes to deployment/infrastructure - -## Severity Levels - -- **CRITICAL**: Will cause immediate workflow failures -- **HIGH**: May cause silent failures or require urgent action -- **MEDIUM**: Should be addressed but not urgent -- **LOW**: Optional improvements or deprecation notices - -## Permissions - -The `breakingChanges:list` scope is automatically granted to: -- **Global Owners** -- **Global Admins** diff --git a/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.rule-registry.service.test.ts b/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.rule-registry.service.test.ts index 041ba45205b..6d92f860a75 100644 --- a/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.rule-registry.service.test.ts +++ b/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.rule-registry.service.test.ts @@ -4,10 +4,10 @@ import { mock } from 'jest-mock-extended'; import { RuleRegistry } from '../breaking-changes.rule-registry.service'; import type { IBreakingChangeRule, - BreakingChangeMetadata, - InstanceDetectionResult, + BreakingChangeRuleMetadata, + InstanceDetectionReport, } from '../types'; -import { BreakingChangeCategory, BreakingChangeSeverity } from '../types'; +import { BreakingChangeCategory } from '../types'; describe('RuleRegistry', () => { const logger = mock({ @@ -26,12 +26,12 @@ describe('RuleRegistry', () => { version: string = 'v2', title: string = 'Test Rule', ): IBreakingChangeRule => { - const metadata: BreakingChangeMetadata = { + const metadata: BreakingChangeRuleMetadata = { version: version as 'v2', title, description: `Description for ${title}`, category: BreakingChangeCategory.workflow, - severity: BreakingChangeSeverity.medium, + severity: 'medium', }; const mockRule: IBreakingChangeRule = { @@ -42,7 +42,7 @@ describe('RuleRegistry', () => { isAffected: false, instanceIssues: [], recommendations: [], - } as InstanceDetectionResult); + } as InstanceDetectionReport); }), }; diff --git a/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.service.test.ts b/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.service.test.ts index 42e69b20737..a8fc2193139 100644 --- a/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.service.test.ts +++ b/packages/cli/src/modules/breaking-changes/__tests__/breaking-changes.service.test.ts @@ -1,8 +1,11 @@ +import type { BreakingChangeWorkflowRuleResult } from '@n8n/api-types'; import { mockLogger } from '@n8n/backend-test-utils'; import type { WorkflowRepository } from '@n8n/db'; import { mock } from 'jest-mock-extended'; import type { ErrorReporter } from 'n8n-core'; +import type { CacheService } from '@/services/cache/cache.service'; + import { N8N_VERSION } from '../../../constants'; import { RuleRegistry } from '../breaking-changes.rule-registry.service'; import { BreakingChangeService } from '../breaking-changes.service'; @@ -11,8 +14,6 @@ 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 type { CacheService } from '@/services/cache/cache.service'; - describe('BreakingChangeService', () => { const logger = mockLogger(); @@ -61,16 +62,13 @@ describe('BreakingChangeService', () => { const report = await service.detect('v2'); - expect(report).toMatchObject({ + expect(report.report).toMatchObject({ targetVersion: 'v2', currentVersion: N8N_VERSION, - summary: { - totalIssues: 0, - criticalIssues: 0, - }, - results: [], + instanceResults: [], + workflowResults: [], }); - expect(report.generatedAt).toBeInstanceOf(Date); + expect(report.report.generatedAt).toBeInstanceOf(Date); }); it('should aggregate results from multiple rules', async () => { @@ -89,57 +87,38 @@ describe('BreakingChangeService', () => { const report = await service.detect('v2'); // Verify report structure - expect(report.targetVersion).toBe('v2'); - expect(report.currentVersion).toBe(N8N_VERSION); - expect(report.generatedAt).toBeInstanceOf(Date); - - // Verify all three rules detected issues - expect(report.summary.totalIssues).toBe(3); - expect(report.summary.criticalIssues).toBe(1); // Only RemovedNodesRule is critical + expect(report.report.targetVersion).toBe('v2'); + expect(report.report.currentVersion).toBe(N8N_VERSION); + expect(report.report.generatedAt).toBeInstanceOf(Date); // Verify each rule's result is in the report - const removedNodesResult = report.results.find((r) => r.ruleId === 'removed-nodes-v2'); + const removedNodesResult = report.report.workflowResults.find( + (r) => r.ruleId === 'removed-nodes-v2', + ); expect(removedNodesResult?.affectedWorkflows).toHaveLength(1); - const processEnvResult = report.results.find((r) => r.ruleId === 'process-env-access-v2'); + const processEnvResult = report.report.workflowResults.find( + (r) => r.ruleId === 'process-env-access-v2', + ); expect(processEnvResult?.affectedWorkflows).toHaveLength(1); - const fileAccessResult = report.results.find( + const fileAccessResult = report.report.workflowResults.find( (r) => r.ruleId === 'file-access-restriction-v2', ); expect(fileAccessResult?.affectedWorkflows).toHaveLength(1); }); - it('should correctly count critical issues', async () => { - const { workflow: workflow1 } = createWorkflow('wf-1', 'Workflow 1', [ - createNode('Spontit Node', 'n8n-nodes-base.spontit'), // Critical severity - ]); - const { workflow: workflow2 } = createWorkflow('wf-2', 'Workflow 2', [ - createNode('File Node', 'n8n-nodes-base.readWriteFile'), // High severity (not critical) - ]); - workflowRepository.find.mockResolvedValue([workflow1, workflow2]); - workflowRepository.count.mockResolvedValue(2); - - const report = await service.detect('v2'); - - expect(report.summary.totalIssues).toBe(2); - expect(report.summary.criticalIssues).toBe(1); // Only removed nodes is critical - }); - it('should include all required fields in the report', async () => { workflowRepository.find.mockResolvedValue([]); workflowRepository.count.mockResolvedValue(0); const report = await service.detect('v2'); - expect(report).toHaveProperty('generatedAt'); - expect(report).toHaveProperty('targetVersion', 'v2'); - expect(report).toHaveProperty('currentVersion', N8N_VERSION); - expect(report).toHaveProperty('summary'); - expect(report.summary).toHaveProperty('totalIssues'); - expect(report.summary).toHaveProperty('criticalIssues'); - expect(report).toHaveProperty('results'); - expect(Array.isArray(report.results)).toBe(true); + expect(report.report).toHaveProperty('generatedAt'); + expect(report.report).toHaveProperty('targetVersion', 'v2'); + expect(report.report).toHaveProperty('currentVersion', N8N_VERSION); + expect(report.report).toHaveProperty('workflowResults'); + expect(Array.isArray(report.report.workflowResults)).toBe(true); }); }); @@ -182,4 +161,27 @@ describe('BreakingChangeService', () => { expect(detectSpy).toHaveBeenCalledTimes(2); }); }); + + describe('getDetectionReportForRule()', () => { + it('should return undefined for unknown rule ID', async () => { + const result = await service.getDetectionReportForRule('unknown-rule-id'); + expect(result).toBeUndefined(); + }); + + it('should return correct report for a known workflow-level rule', async () => { + const { workflow } = createWorkflow('wf-1', 'Test Workflow', [ + createNode('Spontit Node', 'n8n-nodes-base.spontit'), + ]); + + workflowRepository.find.mockResolvedValue([workflow as never]); + workflowRepository.count.mockResolvedValue(1); + + const result = (await service.getDetectionReportForRule( + 'removed-nodes-v2', + )) as BreakingChangeWorkflowRuleResult; + expect(result).toBeDefined(); + expect(result?.ruleId).toBe('removed-nodes-v2'); + expect(result?.affectedWorkflows).toHaveLength(1); + }); + }); }); diff --git a/packages/cli/src/modules/breaking-changes/__tests__/test-helpers.ts b/packages/cli/src/modules/breaking-changes/__tests__/test-helpers.ts index 442b6d495aa..86872fbc906 100644 --- a/packages/cli/src/modules/breaking-changes/__tests__/test-helpers.ts +++ b/packages/cli/src/modules/breaking-changes/__tests__/test-helpers.ts @@ -1,4 +1,4 @@ -import type { WorkflowEntity } from '@n8n/db'; +import type { WorkflowEntity, WorkflowStatistics } from '@n8n/db'; import type { INode } from 'n8n-workflow'; export const createWorkflow = (id: string, name: string, nodes: INode[], active = true) => ({ @@ -7,6 +7,13 @@ export const createWorkflow = (id: string, name: string, nodes: INode[], active name, active, nodes, + statistics: [ + { + count: 1, + rootCount: 1, + latestEvent: new Date(), + } as WorkflowStatistics, + ], } as WorkflowEntity, nodesGroupedByType: nodes.reduce((map, node) => { if (!map.has(node.type)) { diff --git a/packages/cli/src/modules/breaking-changes/breaking-changes.controller.ts b/packages/cli/src/modules/breaking-changes/breaking-changes.controller.ts index 8d461bd6862..1f3051641c2 100644 --- a/packages/cli/src/modules/breaking-changes/breaking-changes.controller.ts +++ b/packages/cli/src/modules/breaking-changes/breaking-changes.controller.ts @@ -1,4 +1,13 @@ -import { Get, RestController, GlobalScope, Query, Post } from '@n8n/decorators'; +import { + BreakingChangeInstanceRuleResult, + BreakingChangeLightReportResult, + BreakingChangeReportResult, + BreakingChangeWorkflowRuleResult, +} from '@n8n/api-types'; +import { AuthenticatedRequest } from '@n8n/db'; +import { Get, RestController, GlobalScope, Query, Post, Param } from '@n8n/decorators'; + +import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { BreakingChangeService } from './breaking-changes.service'; import { BreakingChangeVersion } from './types'; @@ -7,18 +16,59 @@ import { BreakingChangeVersion } from './types'; export class BreakingChangesController { constructor(private readonly service: BreakingChangeService) {} - /** - * Get all registered breaking change rules - */ - @Get('/') - @GlobalScope('breakingChanges:list') - async listRules(@Query query: { version?: BreakingChangeVersion }) { - return await this.service.getDetectionResults(query.version ?? 'v2'); + private getLightDetectionResults( + report: BreakingChangeReportResult['report'], + ): BreakingChangeLightReportResult['report'] { + return { + ...report, + workflowResults: report.workflowResults.map((r) => { + const { affectedWorkflows, ...otherFields } = r; + return { ...otherFields, nbAffectedWorkflows: affectedWorkflows.length }; + }), + }; } - @Post('/refresh') + /** + * Get all registered breaking change rules results + */ + @Get('/report') @GlobalScope('breakingChanges:list') - async refreshCache(@Query query: { version?: BreakingChangeVersion }) { - return await this.service.refreshDetectionResults(query.version ?? 'v2'); + async getDetectionReport( + @Query query: { version?: BreakingChangeVersion }, + ): Promise { + const report = await this.service.getDetectionResults(query.version ?? 'v2'); + return { + ...report, + report: this.getLightDetectionResults(report.report), + }; + } + + @Post('/report/refresh') + @GlobalScope('breakingChanges:list') + async refreshCache( + @Query query: { version?: BreakingChangeVersion }, + ): Promise { + const report = await this.service.refreshDetectionResults(query.version ?? 'v2'); + return { + ...report, + report: this.getLightDetectionResults(report.report), + }; + } + + /** + * Get specific breaking change rules + */ + @Get('/report/:ruleId') + @GlobalScope('breakingChanges:list') + async getDetectionReportForRule( + _req: AuthenticatedRequest, + _res: Response, + @Param('ruleId') ruleId: string, + ): Promise { + const result = await this.service.getDetectionReportForRule(ruleId); + if (!result) { + throw new NotFoundError(`Breaking change rule with ID '${ruleId}' not found.`); + } + return result; } } diff --git a/packages/cli/src/modules/breaking-changes/breaking-changes.service.ts b/packages/cli/src/modules/breaking-changes/breaking-changes.service.ts index 0054215a94a..d6b5c434737 100644 --- a/packages/cli/src/modules/breaking-changes/breaking-changes.service.ts +++ b/packages/cli/src/modules/breaking-changes/breaking-changes.service.ts @@ -1,31 +1,35 @@ +import { + BreakingChangeAffectedWorkflow, + BreakingChangeInstanceRuleResult, + BreakingChangeReportResult, + BreakingChangeWorkflowRuleResult, +} from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; +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 { strict as assert } from 'node:assert'; + +import { CacheService } from '@/services/cache/cache.service'; import { RuleRegistry } from './breaking-changes.rule-registry.service'; import { allRules, RuleInstances } from './rules'; import type { - DetectionReport, BreakingChangeVersion, - DetectionResult, - IBreakingChangeRule, - AffectedWorkflow, IBreakingChangeWorkflowRule, IBreakingChangeInstanceRule, } from './types'; -import { BreakingChangeSeverity } from './types'; import { N8N_VERSION } from '../../constants'; -import { CacheService } from '@/services/cache/cache.service'; - @Service() export class BreakingChangeService { private readonly batchSize = 100; private static readonly CACHE_KEY_PREFIX = 'breaking-changes:results:'; - private readonly ongoingDetections = new Map>(); + private readonly ongoingDetections = new Map< + BreakingChangeVersion, + Promise + >(); constructor( private readonly ruleRegistry: RuleRegistry, @@ -47,20 +51,25 @@ export class BreakingChangeService { async getAllInstanceRulesResults( instanceLevelRules: IBreakingChangeInstanceRule[], - ): Promise { - const instanceLevelResults: DetectionResult[] = []; + ): Promise { + const instanceLevelResults: BreakingChangeInstanceRuleResult[] = []; for (const rule of instanceLevelRules) { try { const ruleResult = await rule.detect(); + console.log('ruleResult', ruleResult); if (ruleResult.isAffected) { instanceLevelResults.push({ ruleId: rule.id, - affectedWorkflows: [], + ruleTitle: rule.getMetadata().title, + ruleDescription: rule.getMetadata().description, + ruleSeverity: rule.getMetadata().severity, + ruleDocumentationUrl: rule.getMetadata().documentationUrl, instanceIssues: ruleResult.instanceIssues, recommendations: ruleResult.recommendations, }); } } catch (error) { + console.log('error', error); this.errorReporter.error(error, { shouldBeLogged: true }); } } @@ -69,10 +78,10 @@ export class BreakingChangeService { private async getAllWorkflowRulesResults( workflowLevelRules: IBreakingChangeWorkflowRule[], - ): Promise { + ): Promise { const totalWorkflows = await this.workflowRepository.count(); - const allAffectedWorkflowsByRule: Map = new Map(); - const allResults: DetectionResult[] = []; + const allAffectedWorkflowsByRule: Map = new Map(); + const allResults: BreakingChangeWorkflowRuleResult[] = []; this.logger.info('Processing workflows in batches', { totalWorkflows, @@ -82,10 +91,13 @@ export class BreakingChangeService { // Process workflows in batches for (let skip = 0; skip < totalWorkflows; skip += this.batchSize) { const workflows = await this.workflowRepository.find({ - select: ['id', 'name', 'active', 'nodes'], + select: ['id', 'name', 'active', 'nodes', 'updatedAt', 'statistics'], skip, take: this.batchSize, order: { id: 'ASC' }, + relations: { + statistics: true, + }, }); this.logger.debug('Processing batch', { @@ -104,11 +116,19 @@ export class BreakingChangeService { for (const rule of workflowLevelRules) { const workflowDetectionResult = await rule.detectWorkflow(workflow, nodesGroupedByType); if (workflowDetectionResult.isAffected) { - const affectedWorkflow = { + const affectedWorkflow: BreakingChangeAffectedWorkflow = { id: workflow.id, name: workflow.name, active: workflow.active, 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]); @@ -128,8 +148,10 @@ export class BreakingChangeService { if (isAffected) { allResults.push({ ruleId: rule.id, + ruleTitle: rule.getMetadata().title, + ruleDescription: rule.getMetadata().description, + ruleSeverity: rule.getMetadata().severity, affectedWorkflows: workflowResults, - instanceIssues: [], recommendations: await rule.getRecommendations(workflowResults), }); } @@ -139,42 +161,60 @@ export class BreakingChangeService { async refreshDetectionResults( targetVersion: BreakingChangeVersion, - ): Promise { - await this.cacheService.deleteFromHash(BreakingChangeService.CACHE_KEY_PREFIX, targetVersion); + ): Promise { + await this.cacheService.delete(`${BreakingChangeService.CACHE_KEY_PREFIX}_${targetVersion}`); return await this.getDetectionResults(targetVersion); } async getDetectionResults( targetVersion: BreakingChangeVersion, - ): Promise { - return await this.cacheService.getHashValue( - BreakingChangeService.CACHE_KEY_PREFIX, - targetVersion, - { - refreshFn: async () => { - // Check if there's already an ongoing detection for this version - const existingDetection = this.ongoingDetections.get(targetVersion); - if (existingDetection) { - this.logger.debug('Reusing ongoing detection', { targetVersion }); - return await existingDetection; - } + ): Promise { + // Check if there's already an ongoing detection for this version + const existingDetection = this.ongoingDetections.get(targetVersion); + if (existingDetection) { + this.logger.debug('Reusing ongoing detection', { targetVersion }); + return await existingDetection; + } - // Start a new detection and store the promise - const detectionPromise = this.detect(targetVersion); - this.ongoingDetections.set(targetVersion, detectionPromise); + const cacheKey = `${BreakingChangeService.CACHE_KEY_PREFIX}_${targetVersion}`; - try { - return await detectionPromise; - } finally { - // Clean up the promise after completion (success or failure) - this.ongoingDetections.delete(targetVersion); - } - }, - }, - ); + // Start a new detection and store the promise + const detectionPromise: Promise = new Promise((resolve) => { + void (async () => { + // Check cache first + const cachedResult = await this.cacheService.get(cacheKey); + if (cachedResult) { + this.logger.info('Using cached breaking change detection results', { + targetVersion, + }); + return resolve(cachedResult); + } + + // Perform detection + const detectionResult = await this.detect(targetVersion); + return resolve(detectionResult); + })(); + }); + this.ongoingDetections.set(targetVersion, detectionPromise); + + try { + const result = await detectionPromise; + // Store in cache if detection took significant time + if (result.shouldCache) { + await this.cacheService.set(cacheKey, result); + } + return result; + } finally { + // Clean up the promise after completion (success or failure) + this.ongoingDetections.delete(targetVersion); + } } - async detect(targetVersion: BreakingChangeVersion): Promise { + private shouldCacheDetection(durationMs: number): boolean { + return durationMs > Time.seconds.toMilliseconds * 10; + } + + async detect(targetVersion: BreakingChangeVersion): Promise { const startTime = Date.now(); this.logger.info('Starting breaking change detection', { targetVersion }); @@ -190,41 +230,43 @@ export class BreakingChangeService { const report = this.createDetectionReport( targetVersion, - instanceLevelResults.concat(workflowLevelResults), - rules, + instanceLevelResults, + workflowLevelResults, ); const duration = Date.now() - startTime; this.logger.info('Breaking change detection completed', { duration, - totalIssues: report.summary.totalIssues, - criticalIssues: report.summary.criticalIssues, }); - return report; + return { report, shouldCache: this.shouldCacheDetection(duration) }; + } + + async getDetectionReportForRule( + ruleId: string, + ): Promise { + const rule = this.ruleRegistry.getRule(ruleId); + if (!rule) { + return undefined; + } + + if ('detectWorkflow' in rule) { + return (await this.getAllWorkflowRulesResults([rule]))[0]; + } + return (await this.getAllInstanceRulesResults([rule]))[0]; } private createDetectionReport( targetVersion: BreakingChangeVersion, - results: DetectionResult[], - rules: IBreakingChangeRule[], - ): DetectionReport { - const criticalIssues = results.filter((r) => { - const rule = rules.find((rule) => rule.id === r.ruleId); - assert(rule); - - return rule.getMetadata().severity === BreakingChangeSeverity.critical; - }).length; - + instanceResults: BreakingChangeInstanceRuleResult[], + workflowResults: BreakingChangeWorkflowRuleResult[], + ): BreakingChangeReportResult['report'] { return { generatedAt: new Date(), targetVersion, currentVersion: N8N_VERSION, - summary: { - totalIssues: results.length, - criticalIssues, - }, - results, + workflowResults, + instanceResults, }; } } diff --git a/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/file-access.rule.test.ts b/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/file-access.rule.test.ts index e8a3c87fd09..3ac9405dbaa 100644 --- a/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/file-access.rule.test.ts +++ b/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/file-access.rule.test.ts @@ -1,5 +1,5 @@ import { createNode, createWorkflow } from '../../../__tests__/test-helpers'; -import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../../types'; +import { BreakingChangeCategory } from '../../../types'; import { FileAccessRule } from '../file-access.rule'; describe('FileAccessRule', () => { @@ -19,7 +19,7 @@ describe('FileAccessRule', () => { title: 'File Access Restrictions', description: 'File access is now restricted to a default directory for security purposes', category: BreakingChangeCategory.workflow, - severity: BreakingChangeSeverity.high, + severity: 'high', }); }); }); @@ -61,7 +61,7 @@ describe('FileAccessRule', () => { expect(result.issues[0]).toMatchObject({ title: "File access node 'n8n-nodes-base.readWriteFile' with name 'Read File' affected", description: 'File access for this node is now restricted to configured directories.', - level: IssueLevel.warning, + level: 'warning', }); }); @@ -75,7 +75,7 @@ describe('FileAccessRule', () => { expect(result.isAffected).toBe(true); expect(result.issues[0]).toMatchObject({ title: "File access node 'n8n-nodes-base.readBinaryFiles' with name 'Read Binary' affected", - level: IssueLevel.warning, + level: 'warning', }); }); diff --git a/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/process-env-access.rule.test.ts b/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/process-env-access.rule.test.ts index aff11166884..5bd678d3513 100644 --- a/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/process-env-access.rule.test.ts +++ b/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/process-env-access.rule.test.ts @@ -1,5 +1,5 @@ import { createNode, createWorkflow } from '../../../__tests__/test-helpers'; -import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../../types'; +import { BreakingChangeCategory } from '../../../types'; import { ProcessEnvAccessRule } from '../process-env-access.rule'; describe('ProcessEnvAccessRule', () => { @@ -19,7 +19,7 @@ describe('ProcessEnvAccessRule', () => { title: 'Block process.env Access in Expressions and Code nodes', description: 'Direct access to process.env is blocked by default for security', category: BreakingChangeCategory.workflow, - severity: BreakingChangeSeverity.high, + severity: 'high', }); }); }); @@ -77,8 +77,8 @@ describe('ProcessEnvAccessRule', () => { expect(result.issues[0]).toMatchObject({ title: 'process.env access detected', description: - "The following nodes contain process.env access: 'Code'. This will be blocked by default in v2.0.0.", - level: IssueLevel.error, + "Node with name 'Code' accesses process.env which is blocked by default for security reasons.", + level: 'error', }); }); @@ -95,7 +95,7 @@ describe('ProcessEnvAccessRule', () => { expect(result.isAffected).toBe(true); expect(result.issues[0]).toMatchObject({ title: 'process.env access detected', - level: IssueLevel.error, + level: 'error', }); expect(result.issues[0].description).toContain('HTTP'); }); @@ -116,8 +116,8 @@ describe('ProcessEnvAccessRule', () => { const result = await rule.detectWorkflow(workflow, nodesGroupedByType); expect(result.issues[0].description).toContain('Code'); - expect(result.issues[0].description).toContain('HTTP'); - expect(result.issues[0].description).not.toContain('Set'); + expect(result.issues[1].description).toContain('HTTP'); + expect(result.issues).toHaveLength(2); }); it('should not detect false positives', async () => { diff --git a/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/removed-nodes.rule.test.ts b/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/removed-nodes.rule.test.ts index ae6d32d5d45..73e43bd54b2 100644 --- a/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/removed-nodes.rule.test.ts +++ b/packages/cli/src/modules/breaking-changes/rules/v2/__tests__/removed-nodes.rule.test.ts @@ -1,5 +1,5 @@ import { createNode, createWorkflow } from '../../../__tests__/test-helpers'; -import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../../types'; +import { BreakingChangeCategory } from '../../../types'; import { RemovedNodesRule } from '../removed-nodes.rule'; describe('RemovedNodesRule', () => { @@ -19,7 +19,7 @@ describe('RemovedNodesRule', () => { title: 'Removed Deprecated Nodes', description: 'Several deprecated nodes have been removed and will no longer work', category: BreakingChangeCategory.workflow, - severity: BreakingChangeSeverity.critical, + severity: 'critical', }); }); }); @@ -77,7 +77,7 @@ describe('RemovedNodesRule', () => { expect(result.issues[0].description).toBe( `The node type '${nodeType}' is no longer available. Please replace it with an alternative.`, ); - expect(result.issues[0].level).toBe(IssueLevel.error); + expect(result.issues[0].level).toBe('error'); }); it('should detect multiple removed nodes in the same workflow', async () => { diff --git a/packages/cli/src/modules/breaking-changes/rules/v2/file-access.rule.ts b/packages/cli/src/modules/breaking-changes/rules/v2/file-access.rule.ts index dafb3837d5c..681c6c29e1e 100644 --- a/packages/cli/src/modules/breaking-changes/rules/v2/file-access.rule.ts +++ b/packages/cli/src/modules/breaking-changes/rules/v2/file-access.rule.ts @@ -1,14 +1,14 @@ +import { BreakingChangeRecommendation } from '@n8n/api-types'; import { WorkflowEntity } from '@n8n/db'; import { Service } from '@n8n/di'; import { INode } from 'n8n-workflow'; import type { - BreakingChangeMetadata, - WorkflowDetectionResult, - Recommendation, + BreakingChangeRuleMetadata, IBreakingChangeWorkflowRule, + WorkflowDetectionReport, } from '../../types'; -import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../types'; +import { BreakingChangeCategory } from '../../types'; @Service() export class FileAccessRule implements IBreakingChangeWorkflowRule { @@ -16,23 +16,22 @@ export class FileAccessRule implements IBreakingChangeWorkflowRule { id: string = 'file-access-restriction-v2'; - getMetadata(): BreakingChangeMetadata { + getMetadata(): BreakingChangeRuleMetadata { return { version: 'v2', title: 'File Access Restrictions', description: 'File access is now restricted to a default directory for security purposes', category: BreakingChangeCategory.workflow, - severity: BreakingChangeSeverity.high, + severity: 'high', }; } - async getRecommendations(): Promise { + async getRecommendations(): Promise { return [ { action: 'Configure file access paths', description: 'Set N8N_RESTRICT_FILE_ACCESS_TO to a semicolon-separated list of allowed paths if workflows need to access files outside the default directory', - documentationUrl: this.getMetadata().documentationUrl, }, ]; } @@ -40,7 +39,7 @@ export class FileAccessRule implements IBreakingChangeWorkflowRule { async detectWorkflow( _workflow: WorkflowEntity, nodesGroupedByType: Map, - ): Promise { + ): Promise { const fileNodes = this.FILE_NODES.flatMap((nodeType) => nodesGroupedByType.get(nodeType) ?? []); if (fileNodes.length === 0) return { isAffected: false, issues: [] }; @@ -49,7 +48,9 @@ export class FileAccessRule implements IBreakingChangeWorkflowRule { issues: fileNodes.map((node) => ({ title: `File access node '${node.type}' with name '${node.name}' affected`, description: 'File access for this node is now restricted to configured directories.', - level: IssueLevel.warning, + level: 'warning', + nodeId: node.id, + nodeName: node.name, })), }; } diff --git a/packages/cli/src/modules/breaking-changes/rules/v2/process-env-access.rule.ts b/packages/cli/src/modules/breaking-changes/rules/v2/process-env-access.rule.ts index ca66b31c143..a844d7a640b 100644 --- a/packages/cli/src/modules/breaking-changes/rules/v2/process-env-access.rule.ts +++ b/packages/cli/src/modules/breaking-changes/rules/v2/process-env-access.rule.ts @@ -1,51 +1,51 @@ +import { BreakingChangeRecommendation } from '@n8n/api-types'; import { WorkflowEntity } from '@n8n/db'; import { Service } from '@n8n/di'; import { INode } from 'n8n-workflow'; import type { - BreakingChangeMetadata, + BreakingChangeRuleMetadata, IBreakingChangeWorkflowRule, - Recommendation, - WorkflowDetectionResult, + WorkflowDetectionReport, } from '../../types'; -import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../types'; +import { BreakingChangeCategory } from '../../types'; @Service() export class ProcessEnvAccessRule implements IBreakingChangeWorkflowRule { id: string = 'process-env-access-v2'; - getMetadata(): BreakingChangeMetadata { + getMetadata(): BreakingChangeRuleMetadata { return { version: 'v2', title: 'Block process.env Access in Expressions and Code nodes', description: 'Direct access to process.env is blocked by default for security', category: BreakingChangeCategory.workflow, - severity: BreakingChangeSeverity.high, + severity: 'high', }; } async detectWorkflow( workflow: WorkflowEntity, _nodesGroupedByType: Map, - ): Promise { + ): Promise { // Match process.env with optional whitespace, newlines, comments between 'process' and '.env' // This covers: process.env, process .env, process/* comment */.env, process\n.env, etc. // Also matches optional chaining: process?.env const processEnvPattern = /process\s*(?:\/\*[\s\S]*?\*\/)?\s*\??\.?\s*env\b/; - const affectedNodes: string[] = []; + const affectedNodes: Array<{ nodeId: string; nodeName: string }> = []; workflow.nodes.forEach((node) => { // Check in Code nodes if (node.type === 'n8n-nodes-base.code') { const code = typeof node.parameters?.code === 'string' ? node.parameters.code : undefined; if (code && processEnvPattern.test(code)) { - affectedNodes.push(node.name); + affectedNodes.push({ nodeId: node.id, nodeName: node.name }); } } else { // Check in expressions const nodeJson = JSON.stringify(node.parameters); - if (processEnvPattern.test(nodeJson) && !affectedNodes.includes(node.name)) { - affectedNodes.push(node.name); + if (processEnvPattern.test(nodeJson) && !affectedNodes.some((n) => n.nodeId === node.id)) { + affectedNodes.push({ nodeId: node.id, nodeName: node.name }); } } }); @@ -53,19 +53,17 @@ export class ProcessEnvAccessRule implements IBreakingChangeWorkflowRule { return { isAffected: affectedNodes.length > 0, issues: - affectedNodes.length > 0 - ? [ - { - title: 'process.env access detected', - description: `The following nodes contain process.env access: '${affectedNodes.join(', ')}'. This will be blocked by default in v2.0.0.`, - level: IssueLevel.error, - }, - ] - : [], + affectedNodes.map((n) => ({ + title: 'process.env access detected', + description: `Node with name '${n.nodeName}' accesses process.env which is blocked by default for security reasons.`, + level: 'error', + nodeId: n.nodeId, + nodeName: n.nodeName, + })) || [], }; } - async getRecommendations(): Promise { + async getRecommendations(): Promise { return [ { action: 'Remove process.env usage', diff --git a/packages/cli/src/modules/breaking-changes/rules/v2/removed-nodes.rule.ts b/packages/cli/src/modules/breaking-changes/rules/v2/removed-nodes.rule.ts index 43b0a8e8704..af084e0afe8 100644 --- a/packages/cli/src/modules/breaking-changes/rules/v2/removed-nodes.rule.ts +++ b/packages/cli/src/modules/breaking-changes/rules/v2/removed-nodes.rule.ts @@ -1,14 +1,14 @@ +import { BreakingChangeRecommendation } from '@n8n/api-types'; import type { WorkflowEntity } from '@n8n/db'; import { Service } from '@n8n/di'; import { INode } from 'n8n-workflow'; import type { - BreakingChangeMetadata, - WorkflowDetectionResult, - Recommendation, + BreakingChangeRuleMetadata, IBreakingChangeWorkflowRule, + WorkflowDetectionReport, } from '../../types'; -import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../types'; +import { BreakingChangeCategory } from '../../types'; @Service() export class RemovedNodesRule implements IBreakingChangeWorkflowRule { @@ -20,22 +20,21 @@ export class RemovedNodesRule implements IBreakingChangeWorkflowRule { id: string = 'removed-nodes-v2'; - getMetadata(): BreakingChangeMetadata { + getMetadata(): BreakingChangeRuleMetadata { return { version: 'v2', title: 'Removed Deprecated Nodes', description: 'Several deprecated nodes have been removed and will no longer work', category: BreakingChangeCategory.workflow, - severity: BreakingChangeSeverity.critical, + severity: 'critical', }; } - async getRecommendations(): Promise { + async getRecommendations(): Promise { return [ { action: 'Update affected workflows', description: 'Replace removed nodes with their updated versions or alternatives', - documentationUrl: this.getMetadata().documentationUrl, }, ]; } @@ -43,7 +42,7 @@ export class RemovedNodesRule implements IBreakingChangeWorkflowRule { async detectWorkflow( _workflow: WorkflowEntity, nodesGroupedByType: Map, - ): Promise { + ): Promise { const removedNodes = this.REMOVED_NODES.flatMap((type) => nodesGroupedByType.get(type) ?? []); if (removedNodes.length === 0) return { isAffected: false, issues: [] }; @@ -52,7 +51,9 @@ export class RemovedNodesRule implements IBreakingChangeWorkflowRule { issues: removedNodes.map((node) => ({ title: `Node '${node.type}' with name '${node.name}' has been removed`, description: `The node type '${node.type}' is no longer available. Please replace it with an alternative.`, - level: IssueLevel.error, + level: 'error', + nodeId: node.id, + nodeName: node.name, })), }; } diff --git a/packages/cli/src/modules/breaking-changes/types/detection.types.ts b/packages/cli/src/modules/breaking-changes/types/detection.types.ts index 5a69673ae91..6561c425fd5 100644 --- a/packages/cli/src/modules/breaking-changes/types/detection.types.ts +++ b/packages/cli/src/modules/breaking-changes/types/detection.types.ts @@ -1,15 +1,16 @@ -import type { DetectionResult } from './rule.types'; +import type { + BreakingChangeInstanceIssue, + BreakingChangeRecommendation, + BreakingChangeWorkflowIssue, +} from '@n8n/api-types'; -/** - * Detection report matching the UI structure - */ -export interface DetectionReport { - generatedAt: Date; - targetVersion: string; - currentVersion: string; - summary: { - totalIssues: number; - criticalIssues: number; - }; - results: DetectionResult[]; // Keep raw results for debugging +export interface WorkflowDetectionReport { + isAffected: boolean; + issues: BreakingChangeWorkflowIssue[]; // List of issues affecting this workflow +} + +export interface InstanceDetectionReport { + isAffected: boolean; + instanceIssues: BreakingChangeInstanceIssue[]; + recommendations: BreakingChangeRecommendation[]; } diff --git a/packages/cli/src/modules/breaking-changes/types/rule.types.ts b/packages/cli/src/modules/breaking-changes/types/rule.types.ts index a19b0b3391e..9c6277efef2 100644 --- a/packages/cli/src/modules/breaking-changes/types/rule.types.ts +++ b/packages/cli/src/modules/breaking-changes/types/rule.types.ts @@ -1,12 +1,12 @@ +import type { + BreakingChangeAffectedWorkflow, + BreakingChangeRecommendation, + BreakingChangeRuleSeverity, +} from '@n8n/api-types'; import type { WorkflowEntity } from '@n8n/db'; import type { INode } from 'n8n-workflow'; -export const enum BreakingChangeSeverity { - critical = 'critical', - high = 'high', - medium = 'medium', - low = 'low', -} +import type { InstanceDetectionReport, WorkflowDetectionReport } from './detection.types'; export const enum BreakingChangeCategory { workflow = 'workflow', @@ -16,74 +16,34 @@ export const enum BreakingChangeCategory { infrastructure = 'infrastructure', } -export const enum IssueLevel { - error = 'error', - warning = 'warning', - info = 'info', -} - export type BreakingChangeVersion = 'v2'; -export interface BreakingChangeMetadata { +export interface BreakingChangeRuleMetadata { version: BreakingChangeVersion; title: string; description: string; category: BreakingChangeCategory; - severity: BreakingChangeSeverity; - documentationUrl?: string; -} - -export interface WorkflowDetectionResult { - isAffected: boolean; - issues: DetectionIssue[]; // List of issues affecting this workflow -} - -export interface InstanceDetectionResult { - isAffected: boolean; - instanceIssues: DetectionIssue[]; - recommendations: Recommendation[]; -} - -export type AffectedWorkflow = Omit & { - id: string; - name: string; - active: boolean; -}; - -export interface DetectionResult { - ruleId: string; - affectedWorkflows: AffectedWorkflow[]; - instanceIssues: DetectionIssue[]; - recommendations: Recommendation[]; -} - -export interface DetectionIssue { - title: string; // e.g., "Environment Variables", "Database Configuration" - description: string; - level: IssueLevel; // error, warning, info -} - -export interface Recommendation { - action: string; - description: string; + severity: BreakingChangeRuleSeverity; documentationUrl?: string; } export interface IBreakingChangeInstanceRule { id: string; - getMetadata(): BreakingChangeMetadata; - detect(): Promise; + getMetadata(): BreakingChangeRuleMetadata; + detect(): Promise; } export interface IBreakingChangeWorkflowRule { id: string; - getMetadata(): BreakingChangeMetadata; - getRecommendations(workflowResults: AffectedWorkflow[]): Promise; + getMetadata(): BreakingChangeRuleMetadata; + getRecommendations( + workflowResults: BreakingChangeAffectedWorkflow[], + ): Promise; // The detectWorkflow function includes the nodes grouped by type for more efficient processing detectWorkflow( workflow: WorkflowEntity, nodesGroupedByType: Map, - ): Promise; + ): Promise; } export type IBreakingChangeRule = IBreakingChangeInstanceRule | IBreakingChangeWorkflowRule;