From 1aa325a64f26a8b377cee341df7486dcedd535b2 Mon Sep 17 00:00:00 2001 From: Daria Date: Mon, 8 Dec 2025 15:47:55 +0200 Subject: [PATCH] feat: Validate nodes before activating (#22916) --- .../__tests__/active-workflow-manager.test.ts | 54 +------------- packages/cli/src/active-workflow-manager.ts | 51 ++----------- .../workflow-validation.error.ts | 13 ++++ .../workflows/workflow-validation.service.ts | 29 ++++++++ .../cli/src/workflows/workflow.service.ts | 51 +++++++++++-- .../active-workflow-manager.test.ts | 31 +++++--- .../workflows/workflow.service.test.ts | 57 ++++++++++++++- .../workflows/workflows.controller.test.ts | 8 ++- .../app/composables/useWorkflowActivate.ts | 5 +- packages/workflow/src/index.ts | 1 + packages/workflow/src/workflow-validation.ts | 59 +++++++++++++++ .../workflow/test/workflow-validation.test.ts | 71 +++++++++++++++++++ 12 files changed, 312 insertions(+), 118 deletions(-) create mode 100644 packages/cli/src/errors/response-errors/workflow-validation.error.ts create mode 100644 packages/cli/src/workflows/workflow-validation.service.ts create mode 100644 packages/workflow/src/workflow-validation.ts create mode 100644 packages/workflow/test/workflow-validation.test.ts diff --git a/packages/cli/src/__tests__/active-workflow-manager.test.ts b/packages/cli/src/__tests__/active-workflow-manager.test.ts index 1bb3bce9943..bebddf12fc1 100644 --- a/packages/cli/src/__tests__/active-workflow-manager.test.ts +++ b/packages/cli/src/__tests__/active-workflow-manager.test.ts @@ -2,14 +2,7 @@ import { mockLogger } from '@n8n/backend-test-utils'; import type { WorkflowEntity, WorkflowHistory, WorkflowRepository } from '@n8n/db'; import { mock } from 'jest-mock-extended'; import type { InstanceSettings } from 'n8n-core'; -import type { - WorkflowParameters, - INode, - INodeType, - INodeTypeDescription, - WorkflowActivateMode, -} from 'n8n-workflow'; -import { Workflow } from 'n8n-workflow'; +import type { WorkflowActivateMode } from 'n8n-workflow'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; import type { NodeTypes } from '@/node-types'; @@ -43,49 +36,6 @@ describe('ActiveWorkflowManager', () => { ); }); - describe('checkIfWorkflowCanBeActivated', () => { - const disabledNode = mock({ type: 'triggerNode', disabled: true }); - const unknownNode = mock({ type: 'unknownNode' }); - const noTriggersNode = mock({ type: 'noTriggersNode' }); - const pollNode = mock({ type: 'pollNode' }); - const triggerNode = mock({ type: 'triggerNode' }); - const webhookNode = mock({ type: 'webhookNode' }); - - nodeTypes.getByNameAndVersion.mockImplementation((type) => { - // TODO: getByNameAndVersion signature needs to be updated to allow returning undefined - if (type === 'unknownNode') return undefined as unknown as INodeType; - const partial: Partial = { - poll: undefined, - trigger: undefined, - webhook: undefined, - description: mock({ - properties: [], - }), - }; - if (type === 'pollNode') partial.poll = jest.fn(); - if (type === 'triggerNode') partial.trigger = jest.fn(); - if (type === 'webhookNode') partial.webhook = jest.fn(); - return mock(partial); - }); - - test.each([ - ['should skip disabled nodes', disabledNode, [], false], - ['should skip nodes marked as ignored', triggerNode, ['triggerNode'], false], - ['should skip unknown nodes', unknownNode, [], false], - ['should skip nodes with no trigger method', noTriggersNode, [], false], - ['should activate if poll method exists', pollNode, [], true], - ['should activate if trigger method exists', triggerNode, [], true], - ['should activate if webhook method exists', webhookNode, [], true], - ])('%s', async (_, node, ignoredNodes, expected) => { - const workflow = new Workflow(mock({ nodeTypes, nodes: [node] })); - const canBeActivated = activeWorkflowManager.checkIfWorkflowCanBeActivated( - workflow, - ignoredNodes, - ); - expect(canBeActivated).toBe(expected); - }); - }); - describe('shouldAddWebhooks', () => { describe('if leader', () => { beforeAll(() => { @@ -130,7 +80,6 @@ describe('ActiveWorkflowManager', () => { test.each<[WorkflowActivateMode]>([['init'], ['leadershipChange']])( 'should skip inactive workflow in `%s` activation mode', async (mode) => { - const checkSpy = jest.spyOn(activeWorkflowManager, 'checkIfWorkflowCanBeActivated'); const addWebhooksSpy = jest.spyOn(activeWorkflowManager, 'addWebhooks'); const addTriggersAndPollersSpy = jest.spyOn( activeWorkflowManager, @@ -142,7 +91,6 @@ describe('ActiveWorkflowManager', () => { const added = await activeWorkflowManager.add('some-id', mode); - expect(checkSpy).not.toHaveBeenCalled(); expect(addWebhooksSpy).not.toHaveBeenCalled(); expect(addTriggersAndPollersSpy).not.toHaveBeenCalled(); expect(added).toEqual({ triggersAndPollers: false, webhooks: false }); diff --git a/packages/cli/src/active-workflow-manager.ts b/packages/cli/src/active-workflow-manager.ts index 55eeab64ef4..cc7fcbb64d4 100644 --- a/packages/cli/src/active-workflow-manager.ts +++ b/packages/cli/src/active-workflow-manager.ts @@ -42,6 +42,7 @@ import { UnexpectedError, ensureError, createRunExecutionData, + validateWorkflowHasTriggerLikeNode, } from 'n8n-workflow'; import { strict } from 'node:assert'; @@ -626,9 +627,13 @@ export class ActiveWorkflowManager { settings: dbWorkflow.settings, }); - const canBeActivated = this.checkIfWorkflowCanBeActivated(workflow, STARTING_NODES); + const validation = validateWorkflowHasTriggerLikeNode( + workflow.nodes, + this.nodeTypes, + STARTING_NODES, + ); - if (!canBeActivated) { + if (!validation.isValid) { throw new WorkflowActivationError( `Workflow ${formatWorkflow(dbWorkflow)} has no node to start the workflow - at least one trigger, poller or webhook node is required`, { level: 'warning' }, @@ -741,48 +746,6 @@ export class ActiveWorkflowManager { } } - /** - * A workflow can only be activated if it has a node which has either triggers - * or webhooks defined. - * - * @param {string[]} [ignoreNodeTypes] Node-types to ignore in the check - */ - checkIfWorkflowCanBeActivated(workflow: Workflow, ignoreNodeTypes?: string[]): boolean { - let node: INode; - let nodeType: INodeType | undefined; - - for (const nodeName of Object.keys(workflow.nodes)) { - node = workflow.nodes[nodeName]; - - if (node.disabled === true) { - // Deactivated nodes can not trigger a run so ignore - continue; - } - - if (ignoreNodeTypes !== undefined && ignoreNodeTypes.includes(node.type)) { - continue; - } - - nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion); - - if (nodeType === undefined) { - // Type is not known so check is not possible - continue; - } - - if ( - nodeType.poll !== undefined || - nodeType.trigger !== undefined || - nodeType.webhook !== undefined - ) { - // Is a trigger node. So workflow can be activated. - return true; - } - } - - return false; - } - /** * Count all triggers in the workflow, excluding Manual Trigger and other n8n-internal triggers. */ diff --git a/packages/cli/src/errors/response-errors/workflow-validation.error.ts b/packages/cli/src/errors/response-errors/workflow-validation.error.ts new file mode 100644 index 00000000000..be4dc971413 --- /dev/null +++ b/packages/cli/src/errors/response-errors/workflow-validation.error.ts @@ -0,0 +1,13 @@ +import { BadRequestError } from './bad-request.error'; + +/** + * Error thrown when a workflow fails validation before activation. + */ +export class WorkflowValidationError extends BadRequestError { + readonly meta = { validationError: true as const }; + + constructor(message: string) { + super(message); + this.name = 'WorkflowValidationError'; + } +} diff --git a/packages/cli/src/workflows/workflow-validation.service.ts b/packages/cli/src/workflows/workflow-validation.service.ts new file mode 100644 index 00000000000..507d559421a --- /dev/null +++ b/packages/cli/src/workflows/workflow-validation.service.ts @@ -0,0 +1,29 @@ +import { Service } from '@n8n/di'; +import { validateWorkflowHasTriggerLikeNode } from 'n8n-workflow'; +import type { INodes } from 'n8n-workflow'; + +import { STARTING_NODES } from '@/constants'; +import type { NodeTypes } from '@/node-types'; + +export interface WorkflowValidationResult { + isValid: boolean; + error?: string; +} + +@Service() +export class WorkflowValidationService { + validateForActivation(nodes: INodes, nodeTypes: NodeTypes): WorkflowValidationResult { + const triggerValidation = validateWorkflowHasTriggerLikeNode(nodes, nodeTypes, STARTING_NODES); + + if (!triggerValidation.isValid) { + return { + isValid: false, + error: + triggerValidation.error ?? + 'Workflow cannot be activated because it has no trigger node. At least one trigger, webhook, or polling node is required.', + }; + } + + return { isValid: true }; + } +} diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index e0ffbde1b9d..5c9067f502e 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -6,6 +6,7 @@ import type { ListQueryDb, WorkflowFolderUnionFull, WorkflowHistoryUpdate, + WorkflowHistory, } from '@n8n/db'; import { SharedWorkflow, @@ -26,6 +27,7 @@ import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPar import isEqual from 'lodash/isEqual'; import pick from 'lodash/pick'; import { FileLocation, BinaryDataService } from 'n8n-core'; +import type { INode, INodes } from 'n8n-workflow'; import { NodeApiError, PROJECT_ROOT, assert, calculateWorkflowChecksum } from 'n8n-workflow'; import { v4 as uuid } from 'uuid'; @@ -37,10 +39,12 @@ import { ActiveWorkflowManager } from '@/active-workflow-manager'; import { FolderNotFoundError } from '@/errors/folder-not-found.error'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import { WorkflowValidationError } from '@/errors/response-errors/workflow-validation.error'; import { WorkflowHistoryVersionNotFoundError } from '@/errors/workflow-history-version-not-found.error'; import { EventService } from '@/events/event.service'; import { ExternalHooks } from '@/external-hooks'; import { validateEntity } from '@/generic-helpers'; +import { NodeTypes } from '@/node-types'; import type { ListQuery } from '@/requests'; import { hasSharing } from '@/requests'; import { OwnershipService } from '@/services/ownership.service'; @@ -49,6 +53,8 @@ import { RoleService } from '@/services/role.service'; import { TagService } from '@/services/tag.service'; import * as WorkflowHelpers from '@/workflow-helpers'; +import { WorkflowValidationService } from './workflow-validation.service'; + @Service() export class WorkflowService { constructor( @@ -71,6 +77,8 @@ export class WorkflowService { private readonly folderRepository: FolderRepository, private readonly workflowFinderService: WorkflowFinderService, private readonly workflowPublishHistoryRepository: WorkflowPublishHistoryRepository, + private readonly workflowValidationService: WorkflowValidationService, + private readonly nodeTypes: NodeTypes, ) {} async getMany( @@ -519,13 +527,19 @@ export class WorkflowService { ); } - const versionToActivate = options?.versionId ?? workflow.versionId; + const versionIdToActivate = options?.versionId ?? workflow.versionId; const wasActive = workflow.activeVersionId !== null; + let versionToActivate: WorkflowHistory; try { - await this.workflowHistoryService.getVersion(user, workflow.id, versionToActivate, { - includePublishHistory: false, - }); + versionToActivate = await this.workflowHistoryService.getVersion( + user, + workflow.id, + versionIdToActivate, + { + includePublishHistory: false, + }, + ); } catch (error) { if (error instanceof WorkflowHistoryVersionNotFoundError) { throw new NotFoundError('Version not found'); @@ -537,6 +551,8 @@ export class WorkflowService { await this._detectConflicts(workflow, options.expectedChecksum); } + this._validateNodes(workflowId, versionToActivate.nodes); + if (wasActive) { await this.activeWorkflowManager.remove(workflowId); } @@ -544,7 +560,7 @@ export class WorkflowService { const activationMode = wasActive ? 'update' : 'activate'; await this.workflowRepository.update(workflowId, { - activeVersionId: versionToActivate, + activeVersionId: versionIdToActivate, active: true, // workflow content did not change, so we keep updatedAt as is updatedAt: workflow.updatedAt, @@ -578,7 +594,11 @@ export class WorkflowService { const updateFields: WorkflowHistoryUpdate = {}; if (options.name !== undefined) updateFields.name = options.name; if (options.description !== undefined) updateFields.description = options.description; - await this.workflowHistoryService.updateVersion(versionToActivate, workflowId, updateFields); + await this.workflowHistoryService.updateVersion( + versionIdToActivate, + workflowId, + updateFields, + ); } // Fetch workflow again with workflowPublishHistory after activation to include the new entry @@ -890,4 +910,23 @@ export class WorkflowService { ); } } + + _validateNodes(workflowId: string, nodes: INode[]) { + const nodesToValidate = nodes.reduce((acc, node) => { + acc[node.name] = node; + return acc; + }, {}); + const validation = this.workflowValidationService.validateForActivation( + nodesToValidate, + this.nodeTypes, + ); + + if (!validation.isValid) { + this.logger.warn('Workflow activation failed validation', { + workflowId, + error: validation.error, + }); + throw new WorkflowValidationError(validation.error ?? 'Workflow validation failed'); + } + } } diff --git a/packages/cli/test/integration/active-workflow-manager.test.ts b/packages/cli/test/integration/active-workflow-manager.test.ts index f65b7c73639..1b00906357e 100644 --- a/packages/cli/test/integration/active-workflow-manager.test.ts +++ b/packages/cli/test/integration/active-workflow-manager.test.ts @@ -12,6 +12,7 @@ import { InstanceSettings, ExternalSecretsProxy } from 'n8n-core'; import { FormTrigger } from 'n8n-nodes-base/nodes/Form/FormTrigger.node'; import { ScheduleTrigger } from 'n8n-nodes-base/nodes/Schedule/ScheduleTrigger.node'; import { NodeApiError, Workflow } from 'n8n-workflow'; +import type * as N8nWorkflow from 'n8n-workflow'; import type { IWebhookData, IWorkflowBase, @@ -19,9 +20,6 @@ import type { INodeTypeData, } from 'n8n-workflow'; -import { createOwner } from './shared/db/users'; -import * as utils from './shared/utils/'; - import { ActiveExecutions } from '@/active-executions'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; import { ExecutionService } from '@/executions/execution.service'; @@ -34,6 +32,9 @@ import { WebhookService } from '@/webhooks/webhook.service'; import * as AdditionalData from '@/workflow-execute-additional-data'; import { WorkflowService } from '@/workflows/workflow.service'; +import { createOwner } from './shared/db/users'; +import * as utils from './shared/utils/'; + mockInstance(ActiveExecutions); mockInstance(Push); mockInstance(ExternalSecretsProxy); @@ -54,6 +55,23 @@ let createActiveWorkflow: ( let createInactiveWorkflow: () => Promise; let owner: User; +jest.mock('n8n-workflow', () => { + const actual = jest.requireActual('n8n-workflow'); + return { + ...actual, + validateWorkflowHasTriggerLikeNode: jest.fn( + (...args: Parameters) => + actual.validateWorkflowHasTriggerLikeNode(...args), + ), + }; +}); + +const { validateWorkflowHasTriggerLikeNode } = jest.requireMock('n8n-workflow'); +const validateWorkflowHasTriggerLikeNodeSpy = + validateWorkflowHasTriggerLikeNode as jest.MockedFunction< + typeof N8nWorkflow.validateWorkflowHasTriggerLikeNode + >; + beforeAll(async () => { await testDb.init(); @@ -113,13 +131,9 @@ describe('init()', () => { it('should check that workflow can be activated', async () => { await Promise.all([createActiveWorkflow(), createActiveWorkflow()]); - const checkSpy = jest - .spyOn(activeWorkflowManager, 'checkIfWorkflowCanBeActivated') - .mockReturnValue(true); - await activeWorkflowManager.init(); - expect(checkSpy).toHaveBeenCalledTimes(2); + expect(validateWorkflowHasTriggerLikeNodeSpy).toHaveBeenCalledTimes(2); }); }); @@ -342,7 +356,6 @@ describe('addWebhooks()', () => { const [node] = dbWorkflow.nodes; jest.spyOn(Workflow.prototype, 'getNode').mockReturnValue(node); - jest.spyOn(activeWorkflowManager, 'checkIfWorkflowCanBeActivated').mockReturnValue(true); webhookService.createWebhookIfNotExists.mockResolvedValue(undefined); await activeWorkflowManager.addWebhooks(workflow, additionalData, 'trigger', 'init'); diff --git a/packages/cli/test/integration/workflows/workflow.service.test.ts b/packages/cli/test/integration/workflows/workflow.service.test.ts index cc3b490a4ef..57cf67c4c28 100644 --- a/packages/cli/test/integration/workflows/workflow.service.test.ts +++ b/packages/cli/test/integration/workflows/workflow.service.test.ts @@ -1,4 +1,9 @@ -import { createWorkflowWithHistory, testDb, mockInstance } from '@n8n/backend-test-utils'; +import { + createWorkflowWithHistory, + testDb, + mockInstance, + createActiveWorkflow, +} from '@n8n/backend-test-utils'; import { GlobalConfig } from '@n8n/config'; import { SharedWorkflowRepository, @@ -12,19 +17,24 @@ import { v4 as uuid } from 'uuid'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; +import { NodeTypes } from '@/node-types'; import { Telemetry } from '@/telemetry'; import { WorkflowFinderService } from '@/workflows/workflow-finder.service'; import { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service'; +import { WorkflowValidationService } from '@/workflows/workflow-validation.service'; import { WorkflowService } from '@/workflows/workflow.service'; import { createOwner } from '../shared/db/users'; import { createWorkflowHistoryItem } from '../shared/db/workflow-history'; let globalConfig: GlobalConfig; +let workflowRepository: WorkflowRepository; let workflowService: WorkflowService; let workflowPublishHistoryRepository: WorkflowPublishHistoryRepository; +let workflowHistoryService: WorkflowHistoryService; const activeWorkflowManager = mockInstance(ActiveWorkflowManager); -const workflowHistoryService = mockInstance(WorkflowHistoryService); +const workflowValidationService = mockInstance(WorkflowValidationService); +const nodeTypes = mockInstance(NodeTypes); mockInstance(MessageEventBus); mockInstance(Telemetry); @@ -32,11 +42,13 @@ beforeAll(async () => { await testDb.init(); globalConfig = Container.get(GlobalConfig); + workflowRepository = Container.get(WorkflowRepository); workflowPublishHistoryRepository = Container.get(WorkflowPublishHistoryRepository); + workflowHistoryService = Container.get(WorkflowHistoryService); workflowService = new WorkflowService( mock(), Container.get(SharedWorkflowRepository), - Container.get(WorkflowRepository), + workflowRepository, mock(), mock(), mock(), @@ -53,9 +65,15 @@ beforeAll(async () => { mock(), Container.get(WorkflowFinderService), workflowPublishHistoryRepository, + workflowValidationService, + nodeTypes, ); }); +beforeEach(() => { + workflowValidationService.validateForActivation.mockReturnValue({ isValid: true }); +}); + afterEach(async () => { await testDb.truncate(['WorkflowEntity', 'WorkflowHistory', 'WorkflowPublishHistory']); jest.restoreAllMocks(); @@ -190,4 +208,37 @@ describe('activateWorkflow()', () => { userId: owner.id, }); }); + + test('should not activate workflow if validation fails and keep old active version', async () => { + const owner = await createOwner(); + const workflow = await createActiveWorkflow({}, owner); + + const oldActiveVersionId = workflow.activeVersionId; + + const addRecordSpy = jest.spyOn(workflowPublishHistoryRepository, 'addRecord'); + + // Create a new version to try to activate + const newVersionId = uuid(); + await createWorkflowHistoryItem(workflow.id, { versionId: newVersionId }); + + // Mock validation to fail + workflowValidationService.validateForActivation.mockReturnValue({ + isValid: false, + error: 'Workflow cannot be activated because it has no trigger node.', + }); + + await expect( + workflowService.activateWorkflow(owner, workflow.id, { + versionId: newVersionId, + }), + ).rejects.toThrow('Workflow cannot be activated because it has no trigger node.'); + + // Verify no publish history was added + expect(addRecordSpy).not.toBeCalled(); + + // Verify the workflow still has the old active version + const workflowAfter = await workflowRepository.findOne({ where: { id: workflow.id } }); + expect(workflowAfter?.activeVersionId).toBe(oldActiveVersionId); + expect(workflowAfter?.active).toBe(true); + }); }); diff --git a/packages/cli/test/integration/workflows/workflows.controller.test.ts b/packages/cli/test/integration/workflows/workflows.controller.test.ts index 6697e375c3c..5b424f93c9f 100644 --- a/packages/cli/test/integration/workflows/workflows.controller.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.test.ts @@ -29,6 +29,7 @@ import { } from '@n8n/db'; import { Container } from '@n8n/di'; import type { Scope } from '@n8n/permissions'; +import { WorkflowValidationService } from '@/workflows/workflow-validation.service'; import { createFolder } from '@test-integration/db/folders'; import { DateTime } from 'luxon'; import { PROJECT_ROOT, type INode, type IPinData, type IWorkflowBase } from 'n8n-workflow'; @@ -67,6 +68,7 @@ const testServer = utils.setupTestServer({ const { objectContaining, arrayContaining, any } = expect; const activeWorkflowManagerLike = mockInstance(ActiveWorkflowManager); +const workflowValidationService = mockInstance(WorkflowValidationService); let projectRepository: ProjectRepository; let workflowRepository: WorkflowRepository; @@ -98,6 +100,8 @@ beforeEach(async () => { authMemberAgent = testServer.authAgentFor(member); anotherMember = await createMember(); + workflowValidationService.validateForActivation.mockReturnValue({ isValid: true }); + folderListMissingRole = await createCustomRoleWithScopeSlugs(['workflow:read', 'workflow:list'], { roleType: 'project', displayName: 'Workflow Read-Only', @@ -3113,14 +3117,14 @@ describe('POST /workflows/:workflowId/activate', () => { const emitSpy = jest.spyOn(eventService, 'emit'); - activeWorkflowManagerLike.add.mockRejectedValueOnce(new Error('Validation failed')); + activeWorkflowManagerLike.add.mockRejectedValueOnce(new Error('Activation failed')); const response = await authOwnerAgent .post(`/workflows/${workflow.id}/activate`) .send({ versionId: newVersionId }); expect(response.statusCode).toBe(400); - expect(response.body.message).toBe('Validation failed'); + expect(response.body.message).toBe('Activation failed'); const updatedWorkflow = await workflowRepository.findOne({ where: { id: workflow.id }, diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowActivate.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowActivate.ts index de98495428b..5a71f39c42e 100644 --- a/packages/frontend/editor-ui/src/app/composables/useWorkflowActivate.ts +++ b/packages/frontend/editor-ui/src/app/composables/useWorkflowActivate.ts @@ -214,7 +214,10 @@ export function useWorkflowActivate() { interpolate: { newStateName: 'published' }, }) + ':', ); - workflowsStore.setWorkflowInactive(workflowId); + // Only update workflow state to inactive if this is not a validation error + if (!error.meta?.validationError) { + workflowsStore.setWorkflowInactive(workflowId); + } return false; } finally { updatingWorkflowActivation.value = false; diff --git a/packages/workflow/src/index.ts b/packages/workflow/src/index.ts index b0a8970493e..03cd6d7c6f6 100644 --- a/packages/workflow/src/index.ts +++ b/packages/workflow/src/index.ts @@ -27,6 +27,7 @@ export * from './workflow'; export * from './workflow-checksum'; export * from './workflow-data-proxy'; export * from './workflow-data-proxy-env-provider'; +export * from './workflow-validation'; export * from './versioned-node-type'; export * from './type-validation'; export * from './result'; diff --git a/packages/workflow/src/workflow-validation.ts b/packages/workflow/src/workflow-validation.ts new file mode 100644 index 00000000000..d1b6c06ac2a --- /dev/null +++ b/packages/workflow/src/workflow-validation.ts @@ -0,0 +1,59 @@ +import type { INode, INodes, INodeType } from './interfaces'; + +export interface INodeTypesGetter { + getByNameAndVersion(nodeType: string, version?: number): INodeType | undefined; +} + +/** + * Validates that a workflow has at least one trigger-like node (trigger, webhook, or polling node). + * A workflow can only be activated if it has a node that can start the workflow execution. + * + * @param nodes - The workflow nodes to validate + * @param nodeTypes - Node types getter to retrieve node definitions + * @param ignoreNodeTypes - Optional array of node types to ignore (e.g., manual trigger, start node) + * @returns Object with isValid flag and error message if invalid + */ +export function validateWorkflowHasTriggerLikeNode( + nodes: INodes, + nodeTypes: INodeTypesGetter, + ignoreNodeTypes?: string[], +): { isValid: boolean; error?: string } { + let node: INode; + let nodeType: INodeType | undefined; + + for (const nodeName of Object.keys(nodes)) { + node = nodes[nodeName]; + + // Skip disabled nodes - they cannot trigger a run + if (node.disabled === true) { + continue; + } + + // Skip ignored node types (e.g., manual trigger, start node) + if (ignoreNodeTypes?.includes(node.type)) { + continue; + } + + nodeType = nodeTypes.getByNameAndVersion(node.type, node.typeVersion); + + if (nodeType === undefined) { + // Type is not known, skip validation + continue; + } + + if ( + nodeType.poll !== undefined || + nodeType.trigger !== undefined || + nodeType.webhook !== undefined + ) { + // Is a trigger node. So workflow can be activated. + return { isValid: true }; + } + } + + return { + isValid: false, + error: + 'Workflow cannot be activated because it has no trigger node. At least one trigger, webhook, or polling node is required.', + }; +} diff --git a/packages/workflow/test/workflow-validation.test.ts b/packages/workflow/test/workflow-validation.test.ts new file mode 100644 index 00000000000..263b91c3d61 --- /dev/null +++ b/packages/workflow/test/workflow-validation.test.ts @@ -0,0 +1,71 @@ +import { vi } from 'vitest'; + +import type { INode, INodes, INodeType, INodeTypeDescription } from '../src/interfaces'; +import type { INodeTypesGetter } from '../src/workflow-validation'; +import { validateWorkflowHasTriggerLikeNode } from '../src/workflow-validation'; + +describe('validateWorkflowHasTriggerLikeNode', () => { + const disabledNode = { type: 'triggerNode', disabled: true } as INode; + const unknownNode = { type: 'unknownNode' } as INode; + const noTriggersNode = { type: 'noTriggersNode' } as INode; + const pollNode = { type: 'pollNode' } as INode; + const triggerNode = { type: 'triggerNode' } as INode; + const webhookNode = { type: 'webhookNode' } as INode; + + const nodeTypes: INodeTypesGetter = { + getByNameAndVersion: vi.fn(), + }; + + beforeEach(() => { + vi.mocked(nodeTypes.getByNameAndVersion).mockImplementation((type): INodeType | undefined => { + if (type === 'unknownNode') return undefined; + + const nodeType: Partial = { + poll: undefined, + trigger: undefined, + webhook: undefined, + description: {} as INodeTypeDescription, + }; + + if (type === 'pollNode') nodeType.poll = vi.fn(); + if (type === 'triggerNode') nodeType.trigger = vi.fn(); + if (type === 'webhookNode') nodeType.webhook = vi.fn(); + + return nodeType as INodeType; + }); + }); + + test.each([ + ['should skip disabled nodes', { disabledNode }, [], false], + ['should skip nodes marked as ignored', { triggerNode }, ['triggerNode'], false], + ['should skip unknown nodes', { unknownNode }, [], false], + ['should skip nodes with no trigger method', { noTriggersNode }, [], false], + ['should activate if poll method exists', { pollNode }, [], true], + ['should activate if trigger method exists', { triggerNode }, [], true], + ['should activate if webhook method exists', { webhookNode }, [], true], + [ + 'should ignore multiple node types', + { triggerNode, webhookNode, pollNode }, + ['triggerNode', 'webhookNode', 'pollNode'], + false, + ], + ])('%s', (_, nodes: INodes, ignoredNodes: string[], expectedValid: boolean) => { + const result = validateWorkflowHasTriggerLikeNode(nodes, nodeTypes, ignoredNodes); + + expect(result.isValid).toBe(expectedValid); + if (!expectedValid) { + expect(result.error).toBeDefined(); + expect(result.error).toContain('no trigger node'); + } + }); + + test('should return error message when no trigger nodes found', () => { + const nodes: INodes = { noTriggersNode }; + const result = validateWorkflowHasTriggerLikeNode(nodes, nodeTypes); + + expect(result.isValid).toBe(false); + expect(result.error).toBe( + 'Workflow cannot be activated because it has no trigger node. At least one trigger, webhook, or polling node is required.', + ); + }); +});