mirror of
https://github.com/n8n-io/n8n.git
synced 2026-06-19 07:36:52 +00:00
fix(core): Fix error path schema mismatch in mcp tools (#32560)
This commit is contained in:
committed by
GitHub
parent
09f588857a
commit
c91250f2a7
@@ -776,17 +776,47 @@ describe('create-workflow-from-code MCP tool', () => {
|
||||
};
|
||||
|
||||
const envelopeShape = tool.config.outputSchema as z.ZodRawShape;
|
||||
const itemsField = envelopeShape.autoAssignedCredentials as z.ZodArray<
|
||||
z.ZodObject<z.ZodRawShape>
|
||||
>;
|
||||
// `autoAssignedCredentials` is optional in the schema, so unwrap the
|
||||
// ZodOptional to reach the inner array before tightening its items.
|
||||
const itemsField = (
|
||||
envelopeShape.autoAssignedCredentials as z.ZodOptional<
|
||||
z.ZodArray<z.ZodObject<z.ZodRawShape>>
|
||||
>
|
||||
).unwrap();
|
||||
const strictSchema = z
|
||||
.object({
|
||||
...envelopeShape,
|
||||
autoAssignedCredentials: z.array(itemsField.element.strict()),
|
||||
autoAssignedCredentials: z.array(itemsField.element.strict()).optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
expect(() => strictSchema.parse(result.structuredContent)).not.toThrow();
|
||||
});
|
||||
|
||||
test('error-path structuredContent conforms to declared outputSchema', async () => {
|
||||
// Regression for ADO-5448 / GH #32503: a thrown handler error returned
|
||||
// `structuredContent: { error }`, which violated the declared
|
||||
// outputSchema (additionalProperties: false + required success fields)
|
||||
// and made strict MCP clients reject the response with an opaque
|
||||
// `-32602` schema mismatch that masked the real error.
|
||||
mockParseAndValidate.mockRejectedValue(new Error('boom: invalid SDK code'));
|
||||
|
||||
const tool = createTool();
|
||||
const result = (await tool.handler({ code: 'const wf = ...' } as never, {} as never)) as {
|
||||
isError?: boolean;
|
||||
structuredContent: unknown;
|
||||
};
|
||||
|
||||
// The real, previously-masked error is now surfaced...
|
||||
expect(result.isError).toBe(true);
|
||||
const structured = result.structuredContent as { error?: string };
|
||||
expect(structured.error).toContain('boom: invalid SDK code');
|
||||
expect(createWorkflowMock).not.toHaveBeenCalled();
|
||||
|
||||
// ...and the error envelope validates against the published schema,
|
||||
// so strict clients no longer reject it with -32602.
|
||||
const strictSchema = z.object(tool.config.outputSchema as z.ZodRawShape).strict();
|
||||
expect(() => strictSchema.parse(result.structuredContent)).not.toThrow();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@ import { mockInstance } from '@n8n/backend-test-utils';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import { SharedWorkflowRepository, User, WorkflowEntity } from '@n8n/db';
|
||||
import { NodeConnectionTypes, type IConnections, type INode } from 'n8n-workflow';
|
||||
import { z } from 'zod';
|
||||
|
||||
import { createUpdateWorkflowTool } from '../tools/workflow-builder/update-workflow.tool';
|
||||
|
||||
@@ -191,6 +192,65 @@ describe('update-workflow MCP tool', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('output schema conformance', () => {
|
||||
// Regression for ADO-5448 / GH #32503: the error path returned
|
||||
// `structuredContent: { error }`, which failed validation against the
|
||||
// declared outputSchema (the MCP SDK publishes it with
|
||||
// additionalProperties: false and required success fields). Strict MCP
|
||||
// clients then rejected the response with an opaque `-32602` schema
|
||||
// mismatch that masked the real error. Both the error and success
|
||||
// envelopes must validate against the published schema.
|
||||
const buildStrictOutputSchema = (tool: ReturnType<typeof createTool>) =>
|
||||
z.object(tool.config.outputSchema as z.ZodRawShape).strict();
|
||||
|
||||
test('error-path structuredContent conforms to declared outputSchema', async () => {
|
||||
// A JSON Pointer path without a leading "/" passes input validation but
|
||||
// fails at apply time — the exact repro from the ticket.
|
||||
const tool = createTool();
|
||||
const result = (await callHandler(
|
||||
{
|
||||
workflowId: 'wf-1',
|
||||
operations: [
|
||||
{
|
||||
type: 'setNodeParameter',
|
||||
nodeName: 'B',
|
||||
path: 'parameters.url',
|
||||
value: 'https://new',
|
||||
},
|
||||
],
|
||||
},
|
||||
tool,
|
||||
)) as { isError?: boolean; structuredContent: unknown };
|
||||
|
||||
// The real, previously-masked error is now surfaced...
|
||||
expect(result.isError).toBe(true);
|
||||
const structured = result.structuredContent as { error?: string };
|
||||
expect(structured.error).toContain('Operation 0 failed');
|
||||
expect(structured.error).toContain('is invalid or contains unsafe segments');
|
||||
|
||||
// ...and the error envelope validates against the published schema,
|
||||
// so strict clients no longer reject it with -32602.
|
||||
expect(() => buildStrictOutputSchema(tool).parse(result.structuredContent)).not.toThrow();
|
||||
expect(workflowService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('success-path structuredContent conforms to declared outputSchema', async () => {
|
||||
const tool = createTool();
|
||||
const result = (await callHandler(
|
||||
{
|
||||
workflowId: 'wf-1',
|
||||
operations: [
|
||||
{ type: 'updateNodeParameters', nodeName: 'B', parameters: { url: 'https://new' } },
|
||||
],
|
||||
},
|
||||
tool,
|
||||
)) as { isError?: boolean; structuredContent: unknown };
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
expect(() => buildStrictOutputSchema(tool).parse(result.structuredContent)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('handler', () => {
|
||||
test('applies updateNodeParameters and saves the workflow', async () => {
|
||||
const result = await callHandler({
|
||||
|
||||
+17
-4
@@ -70,11 +70,18 @@ const inputSchema = {
|
||||
),
|
||||
} satisfies z.ZodRawShape;
|
||||
|
||||
// The MCP SDK publishes this schema with `additionalProperties: false` and
|
||||
// validates `structuredContent` against it on every response. Success returns
|
||||
// the full payload below; the error path returns only `{ error }` (optionally
|
||||
// with `hint`). To keep both shapes valid under strict clients, the success
|
||||
// fields are optional and `error` is a declared, optional property — otherwise
|
||||
// a thrown handler error surfaces as an opaque `-32602` schema mismatch
|
||||
// instead of the real message.
|
||||
const outputSchema = {
|
||||
workflowId: z.string().describe('The ID of the created workflow'),
|
||||
name: z.string().describe('The name of the created workflow'),
|
||||
nodeCount: z.number().describe('The number of nodes in the workflow'),
|
||||
url: z.string().describe('The URL to open the workflow in n8n'),
|
||||
workflowId: z.string().optional().describe('The ID of the created workflow'),
|
||||
name: z.string().optional().describe('The name of the created workflow'),
|
||||
nodeCount: z.number().optional().describe('The number of nodes in the workflow'),
|
||||
url: z.string().optional().describe('The URL to open the workflow in n8n'),
|
||||
autoAssignedCredentials: z
|
||||
.array(
|
||||
z.object({
|
||||
@@ -83,6 +90,7 @@ const outputSchema = {
|
||||
credentialType: z.string().describe('The credential type that was auto-assigned'),
|
||||
}),
|
||||
)
|
||||
.optional()
|
||||
.describe('List of credentials that were automatically assigned to nodes'),
|
||||
targetProject: z
|
||||
.object({
|
||||
@@ -92,6 +100,7 @@ const outputSchema = {
|
||||
.enum(['personal', 'team'])
|
||||
.describe('Whether the workflow landed in a personal or team project'),
|
||||
})
|
||||
.optional()
|
||||
.describe('The project the workflow was actually created in.'),
|
||||
note: z
|
||||
.string()
|
||||
@@ -105,6 +114,10 @@ const outputSchema = {
|
||||
.describe(
|
||||
'Actionable hint for recovering from the error. When present, follow the suggested action before retrying.',
|
||||
),
|
||||
error: z
|
||||
.string()
|
||||
.optional()
|
||||
.describe('Error message explaining why the creation failed. Present only on failure.'),
|
||||
} satisfies z.ZodRawShape;
|
||||
|
||||
/**
|
||||
|
||||
@@ -184,12 +184,18 @@ const inputSchema: z.ZodRawShape = {
|
||||
),
|
||||
};
|
||||
|
||||
// The MCP SDK publishes this schema with `additionalProperties: false` and
|
||||
// validates `structuredContent` against it on every response. Success returns
|
||||
// the full payload below; the error path returns only `{ error }`. To keep
|
||||
// both shapes valid under strict clients, the success fields are optional and
|
||||
// `error` is a declared, optional property — otherwise a thrown handler error
|
||||
// surfaces as an opaque `-32602` schema mismatch instead of the real message.
|
||||
const outputSchema = {
|
||||
workflowId: z.string(),
|
||||
name: z.string(),
|
||||
nodeCount: z.number(),
|
||||
url: z.string(),
|
||||
appliedOperations: z.number().describe('Number of operations applied.'),
|
||||
workflowId: z.string().optional(),
|
||||
name: z.string().optional(),
|
||||
nodeCount: z.number().optional(),
|
||||
url: z.string().optional(),
|
||||
appliedOperations: z.number().optional().describe('Number of operations applied.'),
|
||||
autoAssignedCredentials: z
|
||||
.array(
|
||||
z.object({
|
||||
@@ -198,6 +204,7 @@ const outputSchema = {
|
||||
credentialType: z.string(),
|
||||
}),
|
||||
)
|
||||
.optional()
|
||||
.describe('Credentials auto-assigned to nodes that were added in this update.'),
|
||||
validationWarnings: z
|
||||
.array(
|
||||
@@ -207,10 +214,15 @@ const outputSchema = {
|
||||
nodeName: z.string().optional(),
|
||||
}),
|
||||
)
|
||||
.optional()
|
||||
.describe(
|
||||
'Graph and JSON validation warnings on the resulting workflow. Use these to self-correct on the next call.',
|
||||
),
|
||||
note: z.string().optional(),
|
||||
error: z
|
||||
.string()
|
||||
.optional()
|
||||
.describe('Error message explaining why the update failed. Present only on failure.'),
|
||||
} satisfies z.ZodRawShape;
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user