fix(core): Show the policy block before node issues on execute and publish (#40290)

packages/cli/src/__tests__/workflow-runner.test.ts

@@ -15,7 +15,7 @@ import { createExecution } from '@test-integration/db/executions';
 import { createUser } from '@test-integration/db/users';
 import { setupTestServer } from '@test-integration/utils';
 import type { Response } from 'express';
-import { DirectedGraph, StorageConfig, WorkflowExecute, WorkflowHasIssuesError } from 'n8n-core';
+import { DirectedGraph, StorageConfig, WorkflowExecute } from 'n8n-core';
 import * as core from 'n8n-core';
 import {
 	type IExecuteData,
@@ -1108,8 +1108,9 @@ describe('run', () => {
 		});
 	});
 
-	describe('workflow issues pre-flight failure', () => {
-		function arrangeFailingRunDeps(error: Error) {
+	describe('startup failure', () => {
+		it('rejects on startup errors', async () => {
+			const error = new Error('boom');
 			const activeExecutions = Container.get(ActiveExecutions);
 			vi.spyOn(activeExecutions, 'add').mockResolvedValue('1');
 			vi.spyOn(Container.get(CredentialsPermissionChecker), 'check').mockResolvedValueOnce();
@@ -1119,7 +1120,7 @@ describe('run', () => {
 			vi.spyOn(WorkflowExecuteAdditionalData, 'getBase').mockResolvedValue(
 				mock<IWorkflowExecuteAdditionalData>(),
 			);
-
+			const processError = vi.spyOn(runner, 'processError').mockResolvedValueOnce();
 			const data = mock<IWorkflowExecutionDataProcess>({
 				workflowData: { nodes: [], id: 'workflow-id', settings: undefined, staticData: {} },
 				executionData: createRunExecutionData({}),
@@ -1128,34 +1129,10 @@ describe('run', () => {
 				destinationNode: undefined,
 				userId: 'mock-user-id',
 			});
-			return { data };
-		}
-
-		it('surfaces a WorkflowHasIssuesError as a failed run instead of rejecting', async () => {
-			const error = new WorkflowHasIssuesError(
-				{ node1: { parameters: { field: ['is missing'] } } },
-				{},
-			);
-			const { data } = arrangeFailingRunDeps(error);
-			const failExecution = vi.spyOn(runner, 'failExecution').mockResolvedValueOnce();
-			const processError = vi.spyOn(runner, 'processError').mockResolvedValueOnce();
-
-			await expect(runner.run(data)).resolves.toBe('1');
-
-			expect(failExecution).toHaveBeenCalledWith(data, '1', error);
-			expect(processError).not.toHaveBeenCalled();
-		});
-
-		it('still rejects on other startup errors', async () => {
-			const error = new Error('boom');
-			const { data } = arrangeFailingRunDeps(error);
-			const failExecution = vi.spyOn(runner, 'failExecution').mockResolvedValueOnce();
-			const processError = vi.spyOn(runner, 'processError').mockResolvedValueOnce();
 
 			await expect(runner.run(data)).rejects.toThrowError(error);
 
 			expect(processError).toHaveBeenCalled();
-			expect(failExecution).not.toHaveBeenCalled();
 		});
 	});
 });

packages/cli/src/scaling/job-processor.ts

@@ -6,13 +6,7 @@ import { ExecutionsConfig } from '@n8n/config';
 import { MAX_INTEGER_32BITS_SIGNED } from '@n8n/constants';
 import { ExecutionRepository, WorkflowRepository } from '@n8n/db';
 import { Service } from '@n8n/di';
-import {
-	WorkflowHasIssuesError,
-	InstanceSettings,
-	WorkflowExecute,
-	SupplyDataContext,
-	StructuredToolkit,
-} from 'n8n-core';
+import { InstanceSettings, WorkflowExecute, SupplyDataContext, StructuredToolkit } from 'n8n-core';
 import {
 	ManualExecutionCancelledError,
 	NodeConnectionTypes,
@@ -20,7 +14,6 @@ import {
 	TimeoutExecutionCancelledError,
 	Workflow,
 	UnexpectedError,
-	createRunExecutionData,
 	runDataAttemptedDynamicCredentials,
 	runDataUsedDynamicCredentials,
 } from 'n8n-workflow';
@@ -341,35 +334,14 @@ export class JobProcessor {
 				agentRequest: manualData?.agentRequest,
 			};
 
-			try {
-				workflowRun = this.manualExecutionService.runManually(
-					data,
-					workflow,
-					additionalData,
-					executionId,
-					resultData.pinData,
-					execution.storedAt,
-				);
-			} catch (error) {
-				if (error instanceof WorkflowHasIssuesError) {
-					// execution did not even start, but we call `workflowExecuteAfter` to notify main
-
-					const now = new Date();
-					const runData: IRun = {
-						mode: 'manual',
-						status: 'error',
-						finished: false,
-						startedAt: now,
-						stoppedAt: now,
-						data: createRunExecutionData({ resultData: { error, runData: {} } }),
-						storedAt: execution.storedAt,
-					};
-
-					await lifecycleHooks.runHook('workflowExecuteAfter', [runData]);
-					return { success: false };
-				}
-				throw error;
-			}
+			workflowRun = this.manualExecutionService.runManually(
+				data,
+				workflow,
+				additionalData,
+				executionId,
+				resultData.pinData,
+				execution.storedAt,
+			);
 		}
 
 		const runningJob: RunningJob = {

packages/cli/src/workflow-runner.ts

@@ -17,7 +17,6 @@ import {
 	InstanceSettings,
 	StorageConfig,
 	WorkflowExecute,
-	WorkflowHasIssuesError,
 } from 'n8n-core';
 import type {
 	ExecutionError,
@@ -720,11 +719,6 @@ export class WorkflowRunner {
 						),
 				);
 		} catch (error) {
-			if (error instanceof WorkflowHasIssuesError) {
-				await this.failExecution(data, executionId, error);
-				return;
-			}
-
 			await this.processError(
 				error,
 				new Date(),

packages/core/src/execution-engine/__tests__/workflow-execute-process-process-run-execution-data.test.ts

@@ -9,12 +9,7 @@ import type {
 	INodeType,
 	IRunExecutionData,
 } from 'n8n-workflow';
-import {
-	BaseError,
-	NodeConnectionTypes,
-	UnexpectedError,
-	createRunExecutionData,
-} from 'n8n-workflow';
+import { NodeConnectionTypes, UnexpectedError, createRunExecutionData } from 'n8n-workflow';
 import { mock } from 'vitest-mock-extended';
 
 import { NodeTypes } from '@test/helpers';
@@ -53,6 +48,8 @@ describe('processRunExecutionData', () => {
 		runHook.mockResolvedValue(undefined);
 	});
 
+	const hookNames = () => (runHook.mock.calls as Array<[string]>).map(([name]) => name);
+
 	test('throws if execution-data is missing', () => {
 		// ARRANGE
 		const node = createNodeData({ name: 'passThrough', type: types.passThrough });
@@ -137,7 +134,7 @@ describe('processRunExecutionData', () => {
 			.addNodes(node)
 			.toWorkflow({ name: '', active: false, nodeTypes, settings: { executionOrder: 'v1' } });
 
-		const runBlocked = async (executionData: IRunExecutionData) => {
+		const runBlocked = async (executionData: IRunExecutionData, workflowToRun = workflow) => {
 			runHook.mockImplementation(async (hookName: string) => {
 				if (hookName === 'workflowExecuteBefore') throw blocked;
 			});
@@ -146,9 +143,9 @@ describe('processRunExecutionData', () => {
 				additionalData,
 				executionMode,
 				executionData,
-			).processRunExecutionData(workflow);
+			).processRunExecutionData(workflowToRun);
 
-			return { result, hooks: (runHook.mock.calls as Array<[string]>).map(([name]) => name) };
+			return { result, hooks: hookNames() };
 		};
 
 		test('aborts the execution and keeps the error it was given', async () => {
@@ -174,6 +171,28 @@ describe('processRunExecutionData', () => {
 			expect(result.data.resultData.error?.message).toBe(blocked.message);
 			expect(hooks).toEqual(['workflowExecuteBefore', 'workflowExecuteAfter']);
 		});
+
+		test('is reported ahead of the workflow issues', async () => {
+			const nodeWithIssues = createNodeData({
+				name: 'node',
+				type: types.testNodeWithRequiredProperty,
+			});
+			const { result } = await runBlocked(
+				createRunExecutionData({
+					startData: { startNodes: [{ name: nodeWithIssues.name, sourceData: null }] },
+					executionData: {
+						nodeExecutionStack: [
+							{ data: { main: [[{ json: { foo: 1 } }]] }, node: nodeWithIssues, source: null },
+						],
+					},
+				}),
+				new DirectedGraph()
+					.addNodes(nodeWithIssues)
+					.toWorkflow({ name: '', active: false, nodeTypes, settings: { executionOrder: 'v1' } }),
+			);
+
+			expect(result.data.resultData.error?.message).toBe(blocked.message);
+		});
 	});
 
 	test('agent node emits nodeExecuteBefore only once when resuming after tool execution', async () => {
@@ -387,35 +406,36 @@ describe('processRunExecutionData', () => {
 	});
 
 	describe('workflow issues', () => {
-		test('throws if workflow contains nodes with missing required properties', () => {
-			// ARRANGE
-			const node = createNodeData({ name: 'node', type: types.testNodeWithRequiredProperty });
-			const workflow = new DirectedGraph()
-				.addNodes(node)
-				.toWorkflow({ name: '', active: false, nodeTypes, settings: { executionOrder: 'v1' } });
+		const node = createNodeData({ name: 'node', type: types.testNodeWithRequiredProperty });
+		const workflow = new DirectedGraph()
+			.addNodes(node)
+			.toWorkflow({ name: '', active: false, nodeTypes, settings: { executionOrder: 'v1' } });
 
-			const taskDataConnection = { main: [[{ json: { foo: 1 } }]] };
+		test('fails the run if workflow contains nodes with missing required properties', async () => {
+			// ARRANGE
 			const executionData = createRunExecutionData({
 				startData: { startNodes: [{ name: node.name, sourceData: null }] },
 				executionData: {
-					nodeExecutionStack: [{ data: taskDataConnection, node, source: null }],
+					nodeExecutionStack: [{ data: { main: [[{ json: { foo: 1 } }]] }, node, source: null }],
 				},
 			});
 
-			const workflowExecute = new WorkflowExecute(additionalData, executionMode, executionData);
+			// ACT
+			const result = await new WorkflowExecute(
+				additionalData,
+				executionMode,
+				executionData,
+			).processRunExecutionData(workflow);
 
-			// ACT & ASSERT
-			// The function returns a Promise, but throws synchronously, so we can't await it.
-			// eslint-disable-next-line @typescript-eslint/promise-function-async
-			const execution = () => workflowExecute.processRunExecutionData(workflow);
-
-			expect(execution).toThrow(BaseError);
-			expect(execution).toThrow(
+			// ASSERT
+			expect(result.data.resultData.error?.name).toBe('WorkflowHasIssuesError');
+			expect(result.data.resultData.error?.message).toMatch(
 				/^The 'node' node has issues:\n- Parameter "Required Text" is required\.$/,
 			);
+			expect(hookNames()).toEqual(['workflowExecuteBefore', 'workflowExecuteAfter']);
 		});
 
-		test('does not complain about nodes with issue past the destination node', () => {
+		test('does not complain about nodes with issue past the destination node', async () => {
 			// ARRANGE
 			const node1 = createNodeData({ name: 'node1', type: types.passThrough });
 			const node2 = createNodeData({ name: 'node2', type: types.testNodeWithRequiredProperty });
@@ -435,12 +455,15 @@ describe('processRunExecutionData', () => {
 				},
 			});
 
-			const workflowExecute = new WorkflowExecute(additionalData, executionMode, executionData);
+			// ACT
+			const result = await new WorkflowExecute(
+				additionalData,
+				executionMode,
+				executionData,
+			).processRunExecutionData(workflow);
 
-			// ACT & ASSERT
-			// The function returns a Promise, but throws synchronously, so we can't await it.
-			// eslint-disable-next-line @typescript-eslint/promise-function-async
-			expect(() => workflowExecute.processRunExecutionData(workflow)).not.toThrowError();
+			// ASSERT
+			expect(result.data.resultData.error).toBeUndefined();
 		});
 	});
 

packages/core/src/execution-engine/workflow-execute.ts

@@ -2260,7 +2260,12 @@ export class WorkflowExecute {
 	processRunExecutionData(workflow: Workflow): PCancelable<IRun> {
 		Logger.debug('Workflow execution started', { workflowId: workflow.id });
 		const { startedAt, hooks } = this.setupExecution();
-		this.checkForWorkflowIssues(workflow);
+		assertExecutionDataExists(
+			this.runExecutionData.executionData,
+			workflow,
+			this.additionalData,
+			this.mode,
+		);
 		this.handleWaitingState(workflow);
 
 		// Variables which hold temporary data for each node-execution
@@ -2279,6 +2284,7 @@ export class WorkflowExecute {
 			// eslint-disable-next-line complexity
 			const returnPromise = (async () => {
 				await this.initializeExecution(workflow, hooks);
+				this.checkForWorkflowIssues(workflow);
 
 				executionLoop: while (this.isExecutionStackNotEmpty()) {
 					if (this.shouldStopExecuting()) {

packages/frontend/editor-ui/src/app/components/MainHeader/WorkflowHeaderDraftPublishActions.test.ts

@@ -25,6 +25,7 @@ import {
 	createWorkflowDocumentId,
 } from '@/app/stores/workflowDocument.store';
 import { useNodeTypesStore } from '@/app/stores/nodeTypes.store';
+import { mockRestrictedNodeTypes } from '@n8n/frontend-module-type-availability-policies/__tests__/mocks';
 import { nodeViewEventBus } from '@/app/event-bus';
 import { useReviewRequiredStore } from '@/features/workflow-reviews/reviewRequired.store';
 import { useWorkflowReviewStatusStore } from '@/features/workflow-reviews/reviewStatus.store';
@@ -899,6 +900,34 @@ describe('WorkflowHeaderDraftPublishActions', () => {
 			expect(getByTestId('workflow-open-publish-modal-button')).toBeDisabled();
 		});
 
+		it('should name the restricted node before its parameter issues', () => {
+			const restrictedNodeWithIssues: INodeUi = {
+				id: 'http-1',
+				name: 'HTTP Request',
+				type: 'n8n-nodes-base.httpRequest',
+				typeVersion: 4.2,
+				position: [200, 0],
+				parameters: {},
+				disabled: false,
+				issues: { parameters: { url: ['Parameter "URL" is required.'] } },
+			};
+			workflowDocumentStore.setNodes([triggerNode, restrictedNodeWithIssues]);
+			workflowDocumentStore.setConnections({
+				[triggerNode.name]: {
+					main: [[{ node: restrictedNodeWithIssues.name, type: 'main', index: 0 }]],
+				},
+			});
+			mockRestrictedNodeTypes({ [restrictedNodeWithIssues.type]: 'instance' });
+
+			const { getByTestId, getByText, queryByText } = renderComponent();
+
+			expect(getByTestId('workflow-open-publish-modal-button')).toBeDisabled();
+			expect(
+				getByText(/An administrator blocked 'HTTP Request' on this instance/),
+			).toBeInTheDocument();
+			expect(queryByText(/node has issues/)).not.toBeInTheDocument();
+		});
+
 		it('should show publish button enabled when there are unpublished changes (versionId mismatch)', () => {
 			workflowDocumentStore.setNodes([triggerNode]);
 

packages/frontend/editor-ui/src/app/components/MainHeader/WorkflowHeaderDraftPublishActions.vue

@@ -38,6 +38,10 @@ import { ProjectTypes } from '@/features/collaboration/projects/projects.types';
 import { useProjectsStore } from '@/features/collaboration/projects/projects.store';
 import { useWorkflowActivate } from '@/app/composables/useWorkflowActivate';
 import { useToast } from '@n8n/composables/useToast';
+import {
+	describeNodeTypeRestriction,
+	getNodeTypeRestriction,
+} from '@n8n/frontend-module-type-availability-policies';
 import { createEventBus } from '@n8n/utils/event-bus';
 import type { WorkflowVersionFormModalEventBusEvents } from '@/features/workflows/workflowHistory/components/WorkflowVersionFormModal.vue';
 import { useWorkflowHistoryStore } from '@/features/workflows/workflowHistory/workflowHistory.store';
@@ -180,16 +184,20 @@ const hasNodeIssues = computed(() => workflowDocumentStore.value.hasPublishBlock
 
 const isWorkflowPublishable = computed(() => containsTrigger.value && !hasNodeIssues.value);
 
-/** Why publishing is blocked, or '' when it is not. Same copy as the Publish tooltip. */
+/** Why publishing is blocked, or '' when it is not. Also the Publish tooltip. */
 const publishBlockedReason = computed(() => {
 	if (isWorkflowPublishable.value) return '';
+	if (!containsTrigger.value) return i18n.baseText('workflows.publishModal.noTriggerMessage');
 
-	return !containsTrigger.value
-		? i18n.baseText('workflows.publishModal.noTriggerMessage')
-		: i18n.baseText('workflowActivator.showMessage.activeChangedNodesIssuesExistTrue.title', {
-				interpolate: { count: nodesWithValidationIssues.value.length },
-				adjustToNumber: nodesWithValidationIssues.value.length,
-			});
+	for (const node of nodesWithValidationIssues.value) {
+		const restriction = getNodeTypeRestriction(node.type);
+		if (restriction) return describeNodeTypeRestriction(node.name, restriction.scope, 'replace');
+	}
+
+	return i18n.baseText('workflowActivator.showMessage.activeChangedNodesIssuesExistTrue.title', {
+		interpolate: { count: nodesWithValidationIssues.value.length },
+		adjustToNumber: nodesWithValidationIssues.value.length,
+	});
 });
 
 type WorkflowPublishState =
@@ -457,14 +465,7 @@ const publishButtonConfig = computed(() => {
 			loading: false,
 			showIndicator: false,
 			indicatorClass: '',
-			tooltip: !containsTrigger.value
-				? i18n.baseText('workflows.publishModal.noTriggerMessage')
-				: hasNodeIssues.value
-					? i18n.baseText('workflowActivator.showMessage.activeChangedNodesIssuesExistTrue.title', {
-							interpolate: { count: nodesWithValidationIssues.value.length },
-							adjustToNumber: nodesWithValidationIssues.value.length,
-						})
-					: '',
+			tooltip: publishBlockedReason.value,
 			showVersionInfo: false,
 		};
 	}
@@ -477,12 +478,7 @@ const publishButtonConfig = computed(() => {
 			loading: false,
 			showIndicator: false,
 			indicatorClass: '',
-			tooltip: !containsTrigger.value
-				? i18n.baseText('workflows.publishModal.noTriggerMessage')
-				: i18n.baseText('workflowActivator.showMessage.activeChangedNodesIssuesExistTrue.title', {
-						interpolate: { count: nodesWithValidationIssues.value.length },
-						adjustToNumber: nodesWithValidationIssues.value.length,
-					}),
+			tooltip: publishBlockedReason.value,
 			showVersionInfo: false,
 		},
 		'not-published-eligible': {
@@ -518,13 +514,7 @@ const publishButtonConfig = computed(() => {
 			loading: false,
 			showIndicator: true,
 			indicatorClass: 'error',
-			tooltip: i18n.baseText(
-				'workflowActivator.showMessage.activeChangedNodesIssuesExistTrue.title',
-				{
-					interpolate: { count: nodesWithValidationIssues.value.length },
-					adjustToNumber: nodesWithValidationIssues.value.length,
-				},
-			),
+			tooltip: publishBlockedReason.value,
 			showVersionInfo: true,
 		},
 		'published-invalid-trigger': {