diff --git a/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts b/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts index 91a4b7c36aa..7a1d7bd850d 100644 --- a/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts +++ b/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts @@ -2,6 +2,9 @@ * Tests for MCP serve route auth propagation. */ +import { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js' +import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js' import { createRouteContext } from '@sim/testing/helpers/http' import { authInternalMock } from '@sim/testing/mocks/auth-internal.mock' import { @@ -18,7 +21,7 @@ import { import { hybridAuthMockFns } from '@sim/testing/mocks/hybrid-auth.mock' import { permissionsMock, permissionsMockFns } from '@sim/testing/mocks/permissions.mock' import { NextRequest } from 'next/server' -import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { mockExecuteWorkflowService, @@ -101,16 +104,14 @@ const SERVER_RESOURCE = 'http://localhost:3000/api/mcp/serve/server-1' const SERVER_RESOURCE_METADATA = 'http://localhost:3000/.well-known/oauth-protected-resource/api/mcp/serve/server-1' -function toolCallRequest(headers: Record) { +function toolCallRequest( + headers: Record, + params: Record = { name: 'tool_a', arguments: { q: 'test' } } +) { return new NextRequest(SERVER_RESOURCE, { method: 'POST', headers: { Accept: 'application/json', ...headers }, - body: JSON.stringify({ - jsonrpc: '2.0', - id: 1, - method: 'tools/call', - params: { name: 'tool_a', arguments: { q: 'test' } }, - }), + body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/call', params }), }) } @@ -1454,6 +1455,110 @@ describe('MCP Serve Route', () => { expect(body.error.message).toContain('tools/list response is too large') }) + describe('JSON-RPC errors answering a well-formed request', () => { + const PUBLIC_SERVER = { + id: 'server-1', + name: 'Public Server', + workspaceId: 'ws-1', + isPublic: true, + createdBy: 'owner-1', + } + const DEPLOYED_TOOL_LOOKUPS = [ + [PUBLIC_SERVER], + [{ toolName: 'tool_a', workflowId: 'wf-1' }], + [{ workspaceId: 'ws-1', deploymentVersionId: 'deployment-1' }], + ] + const sdkClients: Client[] = [] + + afterEach(async () => { + await Promise.all(sdkClients.splice(0).map((client) => client.close())) + }) + + /** Drives the route with the real SDK client, whose transport reads HTTP 400/404 as a lost session. */ + async function connectSdkClient() { + dbChainMockFns.limit.mockResolvedValue([PUBLIC_SERVER]) + const routeFetch = async (url: string | URL, init?: RequestInit) => { + const request = new NextRequest(new Request(url, init)) + const context = createRouteContext({ serverId: 'server-1' }) + return request.method === 'GET' ? GET(request, context) : POST(request, context) + } + const client = new Client({ name: 'route-test', version: '1.0.0' }) + sdkClients.push(client) + await client.connect( + new StreamableHTTPClientTransport(new URL(SERVER_RESOURCE), { fetch: routeFetch }) + ) + return client + } + + it('delivers MethodNotFound as a JSON-RPC error the SDK client can read', async () => { + const client = await connectSdkClient() + const failure = await client.listPrompts().catch((error: unknown) => error) + + expect(failure).toBeInstanceOf(McpError) + expect(failure).toMatchObject({ code: ErrorCode.MethodNotFound }) + }) + + it.each([ + { + failure: 'tools/call params without a tool name', + lookups: [[PUBLIC_SERVER]], + params: { name: '' }, + code: ErrorCode.InvalidParams, + }, + { + failure: 'an unknown tool', + lookups: [[PUBLIC_SERVER], []], + code: ErrorCode.InvalidParams, + }, + { + failure: 'an undeployed workflow', + lookups: [ + [PUBLIC_SERVER], + [{ toolName: 'tool_a', workflowId: 'wf-1' }], + [{ workspaceId: 'ws-1', deploymentVersionId: null }], + ], + code: ErrorCode.InternalError, + }, + { + failure: 'a workflow archived after the tool lookup', + lookups: DEPLOYED_TOOL_LOOKUPS, + serviceStatus: 404, + code: ErrorCode.InternalError, + }, + { + failure: 'tool arguments the workflow rejects', + lookups: DEPLOYED_TOOL_LOOKUPS, + serviceStatus: 400, + code: ErrorCode.InvalidParams, + }, + ])( + 'answers $failure over HTTP 200 to a JSON-only client', + async ({ lookups, params, serviceStatus, code }) => { + for (const rows of lookups) dbChainMockFns.limit.mockResolvedValueOnce(rows) + if (serviceStatus) { + mockExecuteWorkflowService.mockResolvedValueOnce({ + ok: false, + failure: { kind: 'precheck', statusCode: serviceStatus, message: 'Refused' }, + }) + } + + const response = await POST( + toolCallRequest({}, params), + createRouteContext({ serverId: 'server-1' }) + ) + const body = await response.json() + + expect(response.status).toBe(200) + expect(body.error.code).toBe(code) + if (serviceStatus) { + expect(body.error.data.httpStatus).toBe(serviceStatus) + } else { + expect(mockExecuteWorkflowService).not.toHaveBeenCalled() + } + } + ) + }) + describe('initialize protocol version negotiation', () => { async function callInitialize(protocolVersion?: string) { dbChainMockFns.limit.mockResolvedValueOnce([ diff --git a/apps/sim/app/api/mcp/serve/[serverId]/route.ts b/apps/sim/app/api/mcp/serve/[serverId]/route.ts index a6ae1898e2b..aa1464b421d 100644 --- a/apps/sim/app/api/mcp/serve/[serverId]/route.ts +++ b/apps/sim/app/api/mcp/serve/[serverId]/route.ts @@ -139,8 +139,25 @@ function createError( } } +const SESSION_SIGNAL_STATUSES = new Set([400, 401, 404]) + +/** + * Answers a well-formed request with a JSON-RPC error. Streamable HTTP clients read a 400, 401, + * or 404 as a lost or unauthenticated session and reconnect instead of surfacing the error, so + * those go out at 200, as the SDK server sends them; every other status keeps its limit, retry, + * or alarm meaning. A malformed message has no request to answer and keeps its 400. + */ +function jsonRpcErrorResponse( + error: JSONRPCError, + init: { status?: number; headers?: HeadersInit } = {} +): NextResponse { + const status = + init.status === undefined || SESSION_SIGNAL_STATUSES.has(init.status) ? 200 : init.status + return NextResponse.json(error, { status, headers: init.headers }) +} + function clientCancelledJsonRpcResponse(id: RequestId): NextResponse { - return NextResponse.json( + return jsonRpcErrorResponse( createError(id, ErrorCode.ConnectionClosed, 'Client cancelled request'), { status: 499, @@ -753,11 +770,8 @@ export const POST = withRouteHandler( const paramsValidation = mcpToolCallParamsSchema.safeParse(rpcParams) if (!paramsValidation.success) { - return NextResponse.json( - createError(id, ErrorCode.InvalidParams, 'Invalid tool call parameters'), - { - status: 400, - } + return jsonRpcErrorResponse( + createError(id, ErrorCode.InvalidParams, 'Invalid tool call parameters') ) } @@ -780,11 +794,8 @@ export const POST = withRouteHandler( } default: - return NextResponse.json( - createError(id, ErrorCode.MethodNotFound, `Method not found: ${method}`), - { - status: 404, - } + return jsonRpcErrorResponse( + createError(id, ErrorCode.MethodNotFound, `Method not found: ${method}`) ) } } catch (error) { @@ -804,7 +815,7 @@ async function handleToolsList( try { const duplicateToolName = await getDuplicateToolName(serverId) if (duplicateToolName) { - return NextResponse.json( + return jsonRpcErrorResponse( createError(id, ErrorCode.InvalidRequest, 'MCP server has duplicate tool names', { code: 'duplicate_tool_name', toolName: duplicateToolName, @@ -920,7 +931,7 @@ async function handleToolsList( maxBytes: error.maxBytes, observedBytes: error.observedBytes, }) - return NextResponse.json( + return jsonRpcErrorResponse( createError(id, ErrorCode.InternalError, 'MCP tools/list response is too large', { code: 'payload_too_large', maxBytes: error.maxBytes, @@ -931,7 +942,7 @@ async function handleToolsList( ) } logger.error('Error listing tools:', error) - return NextResponse.json(createError(id, ErrorCode.InternalError, 'Failed to list tools'), { + return jsonRpcErrorResponse(createError(id, ErrorCode.InternalError, 'Failed to list tools'), { status: 500, }) } @@ -941,7 +952,7 @@ async function handleToolsCall( id: RequestId, serverId: string, serverWorkspaceId: string, - params: { name: string; arguments?: Record } | undefined, + params: { name: string; arguments?: Record }, executeAuthContext?: ExecuteAuthContext | null, publicServerOwnerId?: string, simViaHeader?: string | null, @@ -949,11 +960,6 @@ async function handleToolsCall( ): Promise { let abortSignal: ManagedAbortSignal | null = null try { - if (!params?.name) { - return NextResponse.json(createError(id, ErrorCode.InvalidParams, 'Tool name required'), { - status: 400, - }) - } abortSignal = createManagedAbortSignal( requestSignal ?? new AbortController().signal, getMaxExecutionTimeout() @@ -979,7 +985,7 @@ async function handleToolsCall( const abortedAfterToolLookup = callerAbortedJsonRpcResponse(id, abortSignal) if (abortedAfterToolLookup) return abortedAfterToolLookup if (matchingTools.length > 1) { - return NextResponse.json( + return jsonRpcErrorResponse( createError(id, ErrorCode.InvalidRequest, `Duplicate tool name: ${params.name}`, { code: 'duplicate_tool_name', toolName: params.name, @@ -990,11 +996,8 @@ async function handleToolsCall( } const [tool] = matchingTools if (!tool) { - return NextResponse.json( - createError(id, ErrorCode.InvalidParams, `Tool not found: ${params.name}`), - { - status: 404, - } + return jsonRpcErrorResponse( + createError(id, ErrorCode.InvalidParams, `Tool not found: ${params.name}`) ) } @@ -1023,11 +1026,8 @@ async function handleToolsCall( * the deploy status GET route. */ if (!wf?.deploymentVersionId) { - return NextResponse.json( - createError(id, ErrorCode.InternalError, 'Workflow is not deployed'), - { - status: 400, - } + return jsonRpcErrorResponse( + createError(id, ErrorCode.InternalError, 'Workflow is not deployed') ) } @@ -1036,7 +1036,7 @@ async function handleToolsCall( serverId, workflowId: tool.workflowId, }) - return NextResponse.json( + return jsonRpcErrorResponse( createError(id, ErrorCode.InternalError, 'Workflow is unavailable for this MCP server'), { status: 403 } ) @@ -1103,7 +1103,7 @@ async function handleToolsCall( Math.ceil(failure.retryAfterMs / 1000) ).toString() } - return NextResponse.json( + return jsonRpcErrorResponse( createError( id, getWorkflowErrorCode(failure.statusCode, { code: failure.code }), @@ -1125,7 +1125,7 @@ async function handleToolsCall( } if (serviceResult.aborted === 'client') { - return NextResponse.json( + return jsonRpcErrorResponse( createError(id, ErrorCode.ConnectionClosed, 'Client cancelled request', { httpStatus: 499, retryable: false, @@ -1136,7 +1136,7 @@ async function handleToolsCall( } if (serviceResult.aborted === 'timeout') { - return NextResponse.json( + return jsonRpcErrorResponse( createError( id, ErrorCode.InternalError, @@ -1181,7 +1181,7 @@ async function handleToolsCall( ) } catch (error) { if (abortSignal?.isTimedOut()) { - return NextResponse.json( + return jsonRpcErrorResponse( createError(id, ErrorCode.InternalError, 'Tool execution timed out', { code: 'timeout', retryable: true, @@ -1199,7 +1199,7 @@ async function handleToolsCall( observedBytes: error.observedBytes, label: error.label, }) - return NextResponse.json( + return jsonRpcErrorResponse( createError( id, error.label === 'MCP workflow execution request body' @@ -1217,7 +1217,7 @@ async function handleToolsCall( ) } logger.error('Error calling tool:', error) - return NextResponse.json(createError(id, ErrorCode.InternalError, 'Tool execution failed'), { + return jsonRpcErrorResponse(createError(id, ErrorCode.InternalError, 'Tool execution failed'), { status: 500, }) } finally {