fix(core): Fix workflow-sdk validation for plain workflow objects (#28416)

This commit is contained in:
Milorad FIlipović
2026-04-14 18:29:20 +02:00
committed by GitHub
parent 3f57f1cc19
commit 62dc073b3d
15 changed files with 401 additions and 7 deletions

View File

@@ -13,6 +13,17 @@ import type { WorkflowJSON } from '@n8n/workflow-sdk';
import type { ParseAndValidateResult, ValidationWarning } from '../types';
import { stripImportStatements } from '../utils/extract-code';
/**
* Error thrown when workflow code parsing fails.
* Used by MCP tools to distinguish parse errors from other failures.
*/
export class WorkflowCodeParseError extends Error {
constructor(message: string, options?: ErrorOptions) {
super(message, options);
this.name = 'WorkflowCodeParseError';
}
}
/**
* Configuration for ParseValidateHandler
*/
@@ -201,7 +212,7 @@ export class ParseValidateHandler {
code: code.substring(0, 500),
});
throw new Error(
throw new WorkflowCodeParseError(
`Failed to parse generated workflow code: ${error instanceof Error ? error.message : 'Unknown error'}`,
);
}

View File

@@ -19,7 +19,7 @@ export { generateCodeBuilderThreadId } from './utils/code-builder-session';
// Core utilities for MCP integration
export { NodeTypeParser } from './utils/node-type-parser';
export { ParseValidateHandler } from './handlers/parse-validate-handler';
export { ParseValidateHandler, WorkflowCodeParseError } from './handlers/parse-validate-handler';
export { createCodeBuilderSearchTool } from './tools/code-builder-search.tool';
export { createCodeBuilderGetTool } from './tools/code-builder-get.tool';
export type { CodeBuilderGetToolOptions } from './tools/code-builder-get.tool';

View File

@@ -16,6 +16,7 @@ export type {
export {
NodeTypeParser,
ParseValidateHandler,
WorkflowCodeParseError,
createCodeBuilderSearchTool,
createCodeBuilderGetTool,
createGetSuggestedNodesTool,

View File

@@ -0,0 +1,152 @@
import { parseWorkflowCodeToBuilder } from './parse-workflow-code';
describe('parseWorkflowCodeToBuilder', () => {
describe('SDK builder code', () => {
it('should return a WorkflowBuilder from SDK workflow() calls', () => {
const code = `export default workflow('test-id', 'My Workflow')
.add(trigger({ type: 'n8n-nodes-base.manualTrigger', version: 1, config: {} }))
`;
const builder = parseWorkflowCodeToBuilder(code);
expect(typeof builder.regenerateNodeIds).toBe('function');
expect(typeof builder.validate).toBe('function');
expect(typeof builder.toJSON).toBe('function');
const json = builder.toJSON();
expect(json.name).toBe('My Workflow');
expect(json.nodes).toHaveLength(1);
});
});
describe('plain object code (WorkflowJSON)', () => {
it('should convert a plain object with nodes array into a WorkflowBuilder', () => {
const code = `
const myFlow = {
name: 'TEST',
nodes: [
{
id: 'sticky-test',
name: 'Test Note',
type: 'n8n-nodes-base.stickyNote',
typeVersion: 1,
position: [100, 100],
parameters: { content: 'Hello', height: 200, width: 300, color: 3 }
}
],
connections: {}
};
export default myFlow;
`;
const builder = parseWorkflowCodeToBuilder(code);
expect(typeof builder.regenerateNodeIds).toBe('function');
expect(typeof builder.validate).toBe('function');
expect(typeof builder.toJSON).toBe('function');
builder.regenerateNodeIds();
const json = builder.toJSON();
expect(json.name).toBe('TEST');
expect(json.nodes).toHaveLength(1);
expect(json.nodes[0].type).toBe('n8n-nodes-base.stickyNote');
});
it('should convert a directly exported object literal', () => {
const code = `export default {
name: 'Direct Export',
nodes: [
{
id: 'node-1',
name: 'Manual Trigger',
type: 'n8n-nodes-base.manualTrigger',
typeVersion: 1,
position: [0, 0],
parameters: {}
}
],
connections: {}
}`;
const builder = parseWorkflowCodeToBuilder(code);
builder.regenerateNodeIds();
const json = builder.toJSON();
expect(json.name).toBe('Direct Export');
expect(json.nodes).toHaveLength(1);
});
it('should handle a plain object with multiple nodes', () => {
const code = `export default {
name: 'Multi Node',
nodes: [
{
id: 'trigger-1',
name: 'Manual Trigger',
type: 'n8n-nodes-base.manualTrigger',
typeVersion: 1,
position: [0, 0],
parameters: {}
},
{
id: 'set-1',
name: 'Set',
type: 'n8n-nodes-base.set',
typeVersion: 3.4,
position: [200, 0],
parameters: {}
}
],
connections: {
'Manual Trigger': {
main: [[{ node: 'Set', type: 'main', index: 0 }]]
}
}
}`;
const builder = parseWorkflowCodeToBuilder(code);
builder.regenerateNodeIds();
const json = builder.toJSON();
expect(json.nodes).toHaveLength(2);
expect(json.connections).toBeDefined();
});
it('should handle a plain object with empty nodes array', () => {
const code = "export default { name: 'Empty', nodes: [], connections: {} }";
const builder = parseWorkflowCodeToBuilder(code);
builder.regenerateNodeIds();
const json = builder.toJSON();
expect(json.name).toBe('Empty');
expect(json.nodes).toHaveLength(0);
});
});
describe('invalid exports', () => {
it('should throw for a number export', () => {
expect(() => parseWorkflowCodeToBuilder('export default 42')).toThrow(
'Code must export a workflow built with the workflow() SDK function.',
);
});
it('should throw for a string export', () => {
expect(() => parseWorkflowCodeToBuilder("export default 'hello'")).toThrow(
'Code must export a workflow built with the workflow() SDK function.',
);
});
it('should throw for an object without nodes', () => {
expect(() => parseWorkflowCodeToBuilder("export default { foo: 'bar' }")).toThrow(
'Code must export a workflow built with the workflow() SDK function.',
);
});
it('should throw for a boolean export', () => {
expect(() => parseWorkflowCodeToBuilder('export default true')).toThrow(
'Code must export a workflow built with the workflow() SDK function.',
);
});
});
});

View File

@@ -7,6 +7,7 @@
import { interpretSDKCode, InterpreterError, SecurityError } from '../ast-interpreter';
import type { SDKFunctions } from '../ast-interpreter';
import { expr as exprFn } from '../expression';
import { isWorkflowBuilder, isWorkflowJSON } from '../typeguards';
import type { WorkflowJSON, WorkflowBuilder } from '../types/base';
import { workflow as workflowFn } from '../workflow-builder';
import { nextBatch as nextBatchFn } from '../workflow-builder/control-flow-builders/next-batch';
@@ -631,7 +632,8 @@ export function parseWorkflowCodeToBuilder(code: string): WorkflowBuilder {
try {
// Use AST interpreter instead of new Function() for security
return interpretSDKCode(executableCode, sdkFunctions) as WorkflowBuilder;
const result = interpretSDKCode(executableCode, sdkFunctions);
return asWorkflowBuilder(result);
} catch (error) {
if (error instanceof SecurityError) {
throw new SyntaxError(
@@ -651,3 +653,22 @@ export function parseWorkflowCodeToBuilder(code: string): WorkflowBuilder {
throw error;
}
}
/**
* Coerce an interpreter result into a WorkflowBuilder.
*
* - If the result is already a WorkflowBuilder (produced by the SDK `workflow()` function), return it directly.
* - If the result is a plain object that looks like WorkflowJSON (has a `nodes` array), convert it via `workflow.fromJSON()`.
* - Otherwise, throw with a descriptive error.
*/
function asWorkflowBuilder(result: unknown): WorkflowBuilder {
if (isWorkflowBuilder(result)) {
return result;
}
if (isWorkflowJSON(result)) {
return workflowFn.fromJSON(result);
}
throw new SyntaxError('Code must export a workflow built with the workflow() SDK function.');
}

View File

@@ -0,0 +1,58 @@
import { isWorkflowBuilder, isWorkflowJSON } from './typeguards';
describe('isWorkflowBuilder', () => {
it('should return true for an object with regenerateNodeIds function', () => {
const builder = { regenerateNodeIds: () => {} };
expect(isWorkflowBuilder(builder)).toBe(true);
});
it('should return false for null', () => {
expect(isWorkflowBuilder(null)).toBe(false);
});
it('should return false for undefined', () => {
expect(isWorkflowBuilder(undefined)).toBe(false);
});
it('should return false for a string', () => {
expect(isWorkflowBuilder('workflow')).toBe(false);
});
it('should return false for an object without regenerateNodeIds', () => {
expect(isWorkflowBuilder({ nodes: [] })).toBe(false);
});
it('should return false when regenerateNodeIds is not a function', () => {
expect(isWorkflowBuilder({ regenerateNodeIds: 'not-a-function' })).toBe(false);
});
});
describe('isWorkflowJSON', () => {
it('should return true for an object with a nodes array', () => {
expect(isWorkflowJSON({ nodes: [] })).toBe(true);
});
it('should return true for a full WorkflowJSON-like object', () => {
expect(isWorkflowJSON({ name: 'Test', nodes: [{ id: '1' }], connections: {} })).toBe(true);
});
it('should return false for null', () => {
expect(isWorkflowJSON(null)).toBe(false);
});
it('should return false for undefined', () => {
expect(isWorkflowJSON(undefined)).toBe(false);
});
it('should return false for a string', () => {
expect(isWorkflowJSON('workflow')).toBe(false);
});
it('should return false for an object without nodes', () => {
expect(isWorkflowJSON({ name: 'Test' })).toBe(false);
});
it('should return false when nodes is not an array', () => {
expect(isWorkflowJSON({ nodes: 'not-an-array' })).toBe(false);
});
});

View File

@@ -0,0 +1,16 @@
import type { WorkflowBuilder, WorkflowJSON } from './types/base';
export function isWorkflowBuilder(value: unknown): value is WorkflowBuilder {
return (
typeof value === 'object' &&
value !== null &&
'regenerateNodeIds' in value &&
typeof value.regenerateNodeIds === 'function'
);
}
export function isWorkflowJSON(value: unknown): value is WorkflowJSON {
return (
typeof value === 'object' && value !== null && 'nodes' in value && Array.isArray(value.nodes)
);
}

View File

@@ -270,6 +270,26 @@ describe('create-workflow-from-code MCP tool', () => {
expect(response.error).toBe('Invalid syntax at line 5');
});
test('includes SDK reference hint only for parse errors', async () => {
const parseError = new Error('Failed to parse generated workflow code: unexpected token');
parseError.name = 'WorkflowCodeParseError';
mockParseAndValidate.mockRejectedValue(parseError);
const result = await callHandler({ code: 'bad code' });
const response = parseResult(result);
expect(response.hint).toContain('sdk_ref');
});
test('does not include SDK reference hint for non-parse errors', async () => {
mockParseAndValidate.mockRejectedValue(new Error('Permission denied'));
const result = await callHandler({ code: 'bad code' });
const response = parseResult(result);
expect(response.hint).toBeUndefined();
});
test('tracks telemetry on success', async () => {
await callHandler({ code: 'const wf = ...' });

View File

@@ -272,6 +272,26 @@ describe('update-workflow MCP tool', () => {
expect(response.error).toBe('Invalid syntax at line 5');
});
test('includes SDK reference hint only for parse errors', async () => {
const parseError = new Error('Failed to parse generated workflow code: unexpected token');
parseError.name = 'WorkflowCodeParseError';
mockParseAndValidate.mockRejectedValue(parseError);
const result = await callHandler({ workflowId: 'wf-1', code: 'bad code' });
const response = parseResult(result);
expect(response.hint).toContain('sdk_ref');
});
test('does not include SDK reference hint for non-parse errors', async () => {
mockParseAndValidate.mockRejectedValue(new Error('Service unavailable'));
const result = await callHandler({ workflowId: 'wf-1', code: 'bad code' });
const response = parseResult(result);
expect(response.hint).toBeUndefined();
});
test('tracks telemetry on success', async () => {
await callHandler({ workflowId: 'wf-1', code: 'const wf = ...' });

View File

@@ -143,6 +143,29 @@ describe('validate-workflow-code MCP tool', () => {
expect(result.isError).toBe(true);
});
test('includes SDK reference hint only for parse errors', async () => {
const parseError = new Error('Failed to parse generated workflow code: unexpected token');
parseError.name = 'WorkflowCodeParseError';
mockParseAndValidate.mockRejectedValue(parseError);
const tool = createTool();
const result = await tool.handler({ code: 'bad code' }, {} as never);
const response = parseResult(result);
expect(response.valid).toBe(false);
expect(response.hint).toContain('sdk_ref');
});
test('does not include SDK reference hint for non-parse errors', async () => {
mockParseAndValidate.mockRejectedValue(new Error('Some other error'));
const tool = createTool();
const result = await tool.handler({ code: 'bad code' }, {} as never);
const response = parseResult(result);
expect(response.hint).toBeUndefined();
});
test('tracks telemetry on success with nodeCount and warningCount', async () => {
mockParseAndValidate.mockResolvedValue({
workflow: { nodes: [{ id: '1' }, { id: '2' }] },

View File

@@ -5,7 +5,40 @@ import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
import { createWorkflow } from './mock.utils';
import { WorkflowAccessError } from '../mcp.errors';
import { getMcpWorkflow } from '../tools/workflow-validation.utils';
import { getMcpWorkflow, getSdkReferenceHint } from '../tools/workflow-validation.utils';
jest.mock('@n8n/ai-workflow-builder', () => ({
MCP_GET_SDK_REFERENCE_TOOL: { toolName: 'get_sdk_reference', displayTitle: 'SDK Ref' },
}));
describe('getSdkReferenceHint', () => {
test('returns hint for WorkflowCodeParseError', () => {
const error = new Error('parse failed');
error.name = 'WorkflowCodeParseError';
const hint = getSdkReferenceHint(error);
expect(hint).toContain('get_sdk_reference');
});
test('returns hint for SyntaxError', () => {
const hint = getSdkReferenceHint(
new SyntaxError('Code must export a workflow built with the workflow() SDK function.'),
);
expect(hint).toContain('get_sdk_reference');
});
test('returns undefined for generic Error', () => {
expect(getSdkReferenceHint(new Error('something else'))).toBeUndefined();
});
test('returns undefined for non-error values', () => {
expect(getSdkReferenceHint('string')).toBeUndefined();
expect(getSdkReferenceHint(null)).toBeUndefined();
expect(getSdkReferenceHint(undefined)).toBeUndefined();
});
});
describe('getMcpWorkflow', () => {
const user = Object.assign(new User(), { id: 'user-1' });

View File

@@ -5,6 +5,7 @@ import { MCP_CREATE_WORKFLOW_FROM_CODE_TOOL, CODE_BUILDER_VALIDATE_TOOL } from '
import { autoPopulateNodeCredentials, stripNullCredentialStubs } from './credentials-auto-assign';
import { USER_CALLED_MCP_TOOL_EVENT } from '../../mcp.constants';
import type { ToolDefinition, UserCalledMCPToolEventPayload } from '../../mcp.types';
import { getSdkReferenceHint } from '../workflow-validation.utils';
import type { CredentialsService } from '@/credentials/credentials.service';
import type { NodeTypes } from '@/node-types';
@@ -64,6 +65,12 @@ const outputSchema = {
.describe(
'Additional notes about the workflow creation, such as any nodes that were skipped during credential auto-assignment.',
),
hint: z
.string()
.optional()
.describe(
'Actionable hint for recovering from the error. When present, follow the suggested action before retrying.',
),
} satisfies z.ZodRawShape;
/**
@@ -209,7 +216,8 @@ export const createCreateWorkflowFromCodeTool = (
};
telemetry.track(USER_CALLED_MCP_TOOL_EVENT, telemetryPayload);
const output = { error: errorMessage };
const hint = getSdkReferenceHint(error);
const output = { error: errorMessage, ...(hint ? { hint } : {}) };
return {
content: [{ type: 'text', text: JSON.stringify(output, null, 2) }],

View File

@@ -14,7 +14,7 @@ import { resolveNodeWebhookIds } from '@/workflow-helpers';
import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
import type { WorkflowService } from '@/workflows/workflow.service';
import { getMcpWorkflow } from '../workflow-validation.utils';
import { getMcpWorkflow, getSdkReferenceHint } from '../workflow-validation.utils';
const inputSchema = {
workflowId: z.string().describe('The ID of the workflow to update'),
@@ -56,6 +56,12 @@ const outputSchema = {
.describe(
'Additional notes about the workflow update, such as any nodes that were skipped during credential auto-assignment.',
),
hint: z
.string()
.optional()
.describe(
'Actionable hint for recovering from the error. When present, follow the suggested action before retrying.',
),
} satisfies z.ZodRawShape;
/**
@@ -204,7 +210,8 @@ export const createUpdateWorkflowTool = (
};
telemetry.track(USER_CALLED_MCP_TOOL_EVENT, telemetryPayload);
const output = { error: errorMessage };
const hint = getSdkReferenceHint(error);
const output = { error: errorMessage, ...(hint ? { hint } : {}) };
return {
content: [{ type: 'text', text: JSON.stringify(output, null, 2) }],

View File

@@ -3,6 +3,7 @@ import z from 'zod';
import { USER_CALLED_MCP_TOOL_EVENT } from '../../mcp.constants';
import type { ToolDefinition, UserCalledMCPToolEventPayload } from '../../mcp.types';
import { getSdkReferenceHint } from '../workflow-validation.utils';
import type { Telemetry } from '@/telemetry';
@@ -34,6 +35,12 @@ const outputSchema = {
.optional()
.describe('Validation warnings (if any)'),
errors: z.array(z.string()).optional().describe('Validation errors (if invalid)'),
hint: z
.string()
.optional()
.describe(
'Actionable hint for recovering from the error. When present, follow the suggested action before retrying.',
),
} satisfies z.ZodRawShape;
/**
@@ -104,9 +111,12 @@ export const createValidateWorkflowCodeTool = (
};
telemetry.track(USER_CALLED_MCP_TOOL_EVENT, telemetryPayload);
const hint = getSdkReferenceHint(error);
const output = {
valid: false,
errors: [errorMessage],
...(hint ? { hint } : {}),
};
return {

View File

@@ -4,6 +4,20 @@ import type { Scope } from '@n8n/permissions';
import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
import { WorkflowAccessError } from '../mcp.errors';
import { MCP_GET_SDK_REFERENCE_TOOL } from './workflow-builder/constants';
/**
* Returns a hint nudging MCP clients to consult the SDK reference,
* but only when the error is a workflow code parse error.
*/
export function getSdkReferenceHint(error: unknown): string | undefined {
const isParseError =
error instanceof Error &&
(error.name === 'WorkflowCodeParseError' || error instanceof SyntaxError);
if (!isParseError) return undefined;
return `Make sure your code uses the n8n Workflow SDK syntax. Call ${MCP_GET_SDK_REFERENCE_TOOL.toolName} first to learn the correct patterns before writing workflow code.`;
}
export type FoundWorkflow = NonNullable<
Awaited<ReturnType<WorkflowFinderService['findWorkflowForUser']>>