mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-01 15:47:41 +08:00
fix(core): Run node-type-gated validators in MCP workflow build tools (#31247)
This commit is contained in:
+16
-2
@@ -9,6 +9,7 @@
|
||||
import type { Logger } from '@n8n/backend-common';
|
||||
import { parseWorkflowCodeToBuilder, validateWorkflow, workflow } from '@n8n/workflow-sdk';
|
||||
import type { WorkflowJSON } from '@n8n/workflow-sdk';
|
||||
import type { INodeTypes } from 'n8n-workflow';
|
||||
|
||||
import type { ParseAndValidateResult, ValidationWarning } from '../types';
|
||||
import { stripImportStatements } from '../utils/extract-code';
|
||||
@@ -31,6 +32,13 @@ export interface ParseValidateHandlerConfig {
|
||||
logger?: Logger;
|
||||
/** Whether to generate pin data for new nodes. Defaults to true. */
|
||||
generatePinData?: boolean;
|
||||
/**
|
||||
* Optional node-type provider used to unlock the provider-gated validators
|
||||
* in `validateWorkflow` (input-index checks, AI input-type support checks,
|
||||
* main-output index checks, etc.). Without it those validators silently
|
||||
* skip — agent-built workflows then pass validation despite real defects.
|
||||
*/
|
||||
nodeTypesProvider?: INodeTypes;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -51,10 +59,12 @@ interface ValidationIssue {
|
||||
export class ParseValidateHandler {
|
||||
private logger?: Logger;
|
||||
private generatePinData: boolean;
|
||||
private nodeTypesProvider?: INodeTypes;
|
||||
|
||||
constructor(config: ParseValidateHandlerConfig = {}) {
|
||||
this.logger = config.logger;
|
||||
this.generatePinData = config.generatePinData ?? true;
|
||||
this.nodeTypesProvider = config.nodeTypesProvider;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -153,7 +163,9 @@ export class ParseValidateHandler {
|
||||
'info',
|
||||
);
|
||||
|
||||
const jsonValidation = validateWorkflow(json);
|
||||
const jsonValidation = validateWorkflow(json, {
|
||||
nodeTypesProvider: this.nodeTypesProvider,
|
||||
});
|
||||
this.collectValidationIssues(
|
||||
jsonValidation.errors,
|
||||
allWarnings,
|
||||
@@ -224,7 +236,9 @@ export class ParseValidateHandler {
|
||||
const json = builder.toJSON();
|
||||
|
||||
// Run JSON-based validation for additional checks
|
||||
const validationResult = validateWorkflow(json);
|
||||
const validationResult = validateWorkflow(json, {
|
||||
nodeTypesProvider: this.nodeTypesProvider,
|
||||
});
|
||||
|
||||
// Collect JSON validation errors as warnings for agent self-correction
|
||||
this.collectValidationIssues(
|
||||
|
||||
+3
-1
@@ -561,7 +561,9 @@ describe('ParseValidateHandler', () => {
|
||||
|
||||
expect(result.map((w) => w.code)).toEqual(['GRAPH_ERR', 'JSON_ERR']);
|
||||
expect(mockFromJSON).toHaveBeenCalledWith(nonEmptyJson);
|
||||
expect(mockValidateWorkflow).toHaveBeenCalledWith(nonEmptyJson);
|
||||
expect(mockValidateWorkflow).toHaveBeenCalledWith(nonEmptyJson, {
|
||||
nodeTypesProvider: undefined,
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -115,7 +115,7 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
return { description: {} };
|
||||
}) as typeof nodeTypes.getByNameAndVersion);
|
||||
|
||||
mockParseAndValidate.mockResolvedValue({ workflow: mockWorkflowJson });
|
||||
mockParseAndValidate.mockResolvedValue({ workflow: mockWorkflowJson, warnings: [] });
|
||||
mockStripImportStatements.mockImplementation((code: string) => code);
|
||||
mockAutoPopulateNodeCredentials.mockResolvedValue({ assignments: [], skippedHttpNodes: [] });
|
||||
|
||||
@@ -225,6 +225,30 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
expect(result.isError).toBeUndefined();
|
||||
});
|
||||
|
||||
test('surfaces validation warnings in the response', async () => {
|
||||
const warning = {
|
||||
code: 'INVALID_OUTPUT_INDEX',
|
||||
message: "'Fetch Google' has a connection from its error output (index 1).",
|
||||
nodeName: 'Fetch Google',
|
||||
};
|
||||
mockParseAndValidate.mockResolvedValue({ workflow: mockWorkflowJson, warnings: [warning] });
|
||||
|
||||
const result = await callHandler({ code: 'const wf = ...' });
|
||||
|
||||
const response = parseResult(result);
|
||||
expect(response.warnings).toEqual([warning]);
|
||||
expect(result.isError).toBeUndefined();
|
||||
});
|
||||
|
||||
test('omits the warnings field when validation produced none', async () => {
|
||||
mockParseAndValidate.mockResolvedValue({ workflow: mockWorkflowJson, warnings: [] });
|
||||
|
||||
const result = await callHandler({ code: 'const wf = ...' });
|
||||
|
||||
const response = parseResult(result);
|
||||
expect(response).not.toHaveProperty('warnings');
|
||||
});
|
||||
|
||||
test('sets correct workflow entity defaults', async () => {
|
||||
await callHandler({ code: 'const wf = ...' });
|
||||
|
||||
@@ -257,6 +281,7 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
|
||||
test('falls back to "Untitled Workflow" when neither name nor code name exists', async () => {
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
warnings: [],
|
||||
workflow: { ...mockWorkflowJson, name: undefined },
|
||||
});
|
||||
|
||||
@@ -530,6 +555,7 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
|
||||
test('rejects workflow whose data table id does not exist', async () => {
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
warnings: [],
|
||||
workflow: {
|
||||
...mockWorkflowJson,
|
||||
nodes: [dataTableNode(dataTableLocator('id', 'missing'))],
|
||||
@@ -547,6 +573,7 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
|
||||
test('rejects workflow whose data table name does not exist', async () => {
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
warnings: [],
|
||||
workflow: {
|
||||
...mockWorkflowJson,
|
||||
nodes: [dataTableNode(dataTableLocator('name', 'missing-table'))],
|
||||
@@ -567,6 +594,7 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
count: 1,
|
||||
});
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
warnings: [],
|
||||
workflow: {
|
||||
...mockWorkflowJson,
|
||||
nodes: [dataTableNode(dataTableLocator('id', 'dt-existing'))],
|
||||
@@ -591,6 +619,7 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
count: 1,
|
||||
});
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
warnings: [],
|
||||
workflow: {
|
||||
...mockWorkflowJson,
|
||||
nodes: [dataTableNode(dataTableLocator('id', 'dt-existing'))],
|
||||
@@ -615,6 +644,7 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
|
||||
test('skips validation when dataTableId is an expression', async () => {
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
warnings: [],
|
||||
workflow: {
|
||||
...mockWorkflowJson,
|
||||
nodes: [dataTableNode(dataTableLocator('id', '={{ $json.id }}'))],
|
||||
@@ -661,6 +691,7 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
});
|
||||
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
warnings: [],
|
||||
workflow: {
|
||||
...mockWorkflowJson,
|
||||
nodes: [httpNodeWithGithub('6CoUMkVOJRNsbmr2')],
|
||||
@@ -686,6 +717,7 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
);
|
||||
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
warnings: [],
|
||||
workflow: {
|
||||
...mockWorkflowJson,
|
||||
nodes: [httpNodeWithGithub('ghost')],
|
||||
@@ -706,6 +738,7 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
]);
|
||||
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
warnings: [],
|
||||
workflow: {
|
||||
...mockWorkflowJson,
|
||||
nodes: [httpNodeWithGithub('in-project-cred')],
|
||||
|
||||
+23
-2
@@ -114,6 +114,22 @@ const outputSchema = {
|
||||
.describe(
|
||||
'Actionable hint for recovering from the error. When present, follow the suggested action before retrying.',
|
||||
),
|
||||
warnings: z
|
||||
.array(
|
||||
z.object({
|
||||
code: z.string().describe('The warning code identifying the type of warning'),
|
||||
message: z.string().describe('The warning message'),
|
||||
nodeName: z.string().optional().describe('The node that triggered the warning'),
|
||||
parameterPath: z
|
||||
.string()
|
||||
.optional()
|
||||
.describe('The parameter path that triggered the warning'),
|
||||
}),
|
||||
)
|
||||
.optional()
|
||||
.describe(
|
||||
'Validation warnings emitted while parsing the submitted code. Surface these to the user so they can correct the workflow.',
|
||||
),
|
||||
error: z
|
||||
.string()
|
||||
.optional()
|
||||
@@ -195,7 +211,10 @@ export const createCreateWorkflowFromCodeTool = (
|
||||
'@n8n/ai-workflow-builder'
|
||||
);
|
||||
|
||||
const handler = new ParseValidateHandler({ generatePinData: false });
|
||||
const handler = new ParseValidateHandler({
|
||||
generatePinData: false,
|
||||
nodeTypesProvider: nodeTypes,
|
||||
});
|
||||
const strippedCode = stripImportStatements(code);
|
||||
const result = await handler.parseAndValidate(strippedCode);
|
||||
|
||||
@@ -297,7 +316,7 @@ export const createCreateWorkflowFromCodeTool = (
|
||||
: undefined,
|
||||
].filter((note): note is string => note !== undefined);
|
||||
|
||||
const output = {
|
||||
const baseOutput = {
|
||||
workflowId: savedWorkflow.id,
|
||||
name: savedWorkflow.name,
|
||||
nodeCount: savedWorkflow.nodes.length,
|
||||
@@ -310,6 +329,8 @@ export const createCreateWorkflowFromCodeTool = (
|
||||
},
|
||||
note: notes.length ? notes.join(' ') : undefined,
|
||||
};
|
||||
const output =
|
||||
result.warnings.length > 0 ? { ...baseOutput, warnings: result.warnings } : baseOutput;
|
||||
|
||||
return {
|
||||
content: [{ type: 'text', text: JSON.stringify(output, null, 2) }],
|
||||
|
||||
@@ -414,7 +414,10 @@ export const createUpdateWorkflowTool = (
|
||||
}
|
||||
|
||||
const { ParseValidateHandler } = await import('@n8n/ai-workflow-builder');
|
||||
const validator = new ParseValidateHandler({ generatePinData: false });
|
||||
const validator = new ParseValidateHandler({
|
||||
generatePinData: false,
|
||||
nodeTypesProvider: nodeTypes,
|
||||
});
|
||||
const validationWarnings = validator.validateJSON({
|
||||
name: workflowUpdateData.name,
|
||||
nodes: workflowUpdateData.nodes,
|
||||
|
||||
@@ -80,7 +80,10 @@ export const createValidateWorkflowCodeTool = (
|
||||
const { ParseValidateHandler, stripImportStatements } = await import(
|
||||
'@n8n/ai-workflow-builder'
|
||||
);
|
||||
const handler = new ParseValidateHandler({ generatePinData: false });
|
||||
const handler = new ParseValidateHandler({
|
||||
generatePinData: false,
|
||||
nodeTypesProvider: nodeTypes,
|
||||
});
|
||||
const strippedCode = stripImportStatements(code);
|
||||
const result = await handler.parseAndValidate(strippedCode);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user