diff --git a/packages/twenty-server/src/engine/api/mcp/constants/json-rpc-error-code.const.ts b/packages/twenty-server/src/engine/api/mcp/constants/json-rpc-error-code.const.ts new file mode 100644 index 0000000000..8b4fc5743d --- /dev/null +++ b/packages/twenty-server/src/engine/api/mcp/constants/json-rpc-error-code.const.ts @@ -0,0 +1,9 @@ +export const JSON_RPC_ERROR_CODE = { + PARSE_ERROR: -32700, + INVALID_REQUEST: -32600, + METHOD_NOT_FOUND: -32601, + INVALID_PARAMS: -32602, + INTERNAL_ERROR: -32603, + // -32000 to -32099: reserved for implementation-defined server errors + SERVER_ERROR: -32000, +} as const; diff --git a/packages/twenty-server/src/engine/api/mcp/constants/mcp-protocol-version.const.ts b/packages/twenty-server/src/engine/api/mcp/constants/mcp-protocol-version.const.ts new file mode 100644 index 0000000000..f41cb83a6c --- /dev/null +++ b/packages/twenty-server/src/engine/api/mcp/constants/mcp-protocol-version.const.ts @@ -0,0 +1 @@ +export const MCP_PROTOCOL_VERSION = '2024-11-05'; diff --git a/packages/twenty-server/src/engine/api/mcp/constants/mcp-server-info.const.ts b/packages/twenty-server/src/engine/api/mcp/constants/mcp-server-info.const.ts new file mode 100644 index 0000000000..db9fffb5bf --- /dev/null +++ b/packages/twenty-server/src/engine/api/mcp/constants/mcp-server-info.const.ts @@ -0,0 +1,4 @@ +export const MCP_SERVER_INFO = { + name: 'Twenty MCP Server', + version: '0.1.0', +}; diff --git a/packages/twenty-server/src/engine/api/mcp/constants/mcp-server-instructions.const.ts b/packages/twenty-server/src/engine/api/mcp/constants/mcp-server-instructions.const.ts new file mode 100644 index 0000000000..e69c4bc451 --- /dev/null +++ b/packages/twenty-server/src/engine/api/mcp/constants/mcp-server-instructions.const.ts @@ -0,0 +1,2 @@ +export const MCP_SERVER_INSTRUCTIONS = + 'Twenty CRM MCP Server. Follow this workflow: (1) get_tool_catalog to discover tools, (2) learn_tools to get input schemas, (3) execute_tool to run them. Never guess tool names — always start with get_tool_catalog. Use load_skills for guidance on complex tasks like workflow or dashboard building.'; diff --git a/packages/twenty-server/src/engine/api/mcp/constants/mcp.const.ts b/packages/twenty-server/src/engine/api/mcp/constants/mcp.const.ts deleted file mode 100644 index a3d48ae56b..0000000000 --- a/packages/twenty-server/src/engine/api/mcp/constants/mcp.const.ts +++ /dev/null @@ -1,10 +0,0 @@ -export const MCP_SERVER_METADATA = { - metadata: { - info: 'Twenty CRM MCP Server. Follow this workflow: (1) get_tool_catalog to discover tools, (2) learn_tools to get input schemas, (3) execute_tool to run them. Never guess tool names — always start with get_tool_catalog. Use load_skills for guidance on complex tasks like workflow or dashboard building.', - }, - protocolVersion: '2024-11-05', - serverInfo: { - name: 'Twenty MCP Server', - version: '0.1.0', - }, -}; diff --git a/packages/twenty-server/src/engine/api/mcp/controllers/__tests__/mcp-core.controller.spec.ts b/packages/twenty-server/src/engine/api/mcp/controllers/__tests__/mcp-core.controller.spec.ts index 96fe5d0bee..e216d9281d 100644 --- a/packages/twenty-server/src/engine/api/mcp/controllers/__tests__/mcp-core.controller.spec.ts +++ b/packages/twenty-server/src/engine/api/mcp/controllers/__tests__/mcp-core.controller.spec.ts @@ -2,7 +2,9 @@ import { Test, type TestingModule } from '@nestjs/testing'; import { DEFAULT_TOOL_INPUT_SCHEMA } from 'twenty-shared/logic-function'; -import { MCP_SERVER_METADATA } from 'src/engine/api/mcp/constants/mcp.const'; +import { MCP_PROTOCOL_VERSION } from 'src/engine/api/mcp/constants/mcp-protocol-version.const'; +import { MCP_SERVER_INFO } from 'src/engine/api/mcp/constants/mcp-server-info.const'; +import { MCP_SERVER_INSTRUCTIONS } from 'src/engine/api/mcp/constants/mcp-server-instructions.const'; import { McpCoreController } from 'src/engine/api/mcp/controllers/mcp-core.controller'; import { type JsonRpc } from 'src/engine/api/mcp/dtos/json-rpc'; import { McpAuthGuard } from 'src/engine/api/mcp/guards/mcp-auth.guard'; @@ -71,6 +73,13 @@ describe('McpCoreController', () => { const mockUser = { id: 'user-1' } as UserEntity; const mockUserWorkspaceId = 'user-workspace-1'; const mockApiKey = { id: 'api-key-1' } as ApiKeyEntity; + const mockRes = { + status: jest.fn().mockReturnThis(), + } as unknown as import('express').Response; + + beforeEach(() => { + (mockRes.status as jest.Mock).mockClear(); + }); it('should call mcpProtocolService.handleMCPCoreQuery with correct parameters', async () => { const mockRequest: JsonRpc = { @@ -97,6 +106,7 @@ describe('McpCoreController', () => { mockApiKey, mockUser, mockUserWorkspaceId, + mockRes, ); expect(mcpProtocolService.handleMCPCoreQuery).toHaveBeenCalledWith( @@ -122,12 +132,14 @@ describe('McpCoreController', () => { id: '123', jsonrpc: '2.0', result: { - ...MCP_SERVER_METADATA, + protocolVersion: MCP_PROTOCOL_VERSION, capabilities: { tools: { listChanged: false }, resources: { listChanged: false }, prompts: { listChanged: false }, }, + serverInfo: MCP_SERVER_INFO, + instructions: MCP_SERVER_INSTRUCTIONS, }, }; @@ -139,6 +151,7 @@ describe('McpCoreController', () => { mockApiKey, mockUser, mockUserWorkspaceId, + mockRes, ); expect(mcpProtocolService.handleMCPCoreQuery).toHaveBeenCalledWith( @@ -164,10 +177,6 @@ describe('McpCoreController', () => { id: '123', jsonrpc: '2.0', result: { - ...MCP_SERVER_METADATA, - capabilities: { - tools: { listChanged: false }, - }, tools: [ { name: 'testTool', @@ -186,6 +195,7 @@ describe('McpCoreController', () => { mockApiKey, mockUser, mockUserWorkspaceId, + mockRes, ); expect(mcpProtocolService.handleMCPCoreQuery).toHaveBeenCalledWith( @@ -200,6 +210,27 @@ describe('McpCoreController', () => { expect(result).toEqual(mockResponse); }); + it('should return 202 with no body for notifications', async () => { + const mockRequest: JsonRpc = { + jsonrpc: '2.0', + method: 'notifications/initialized', + }; + + mcpProtocolService.handleMCPCoreQuery.mockResolvedValue(null); + + const result = await controller.handleMcpCore( + mockRequest, + mockWorkspace, + mockApiKey, + mockUser, + mockUserWorkspaceId, + mockRes, + ); + + expect(result).toBeUndefined(); + expect(mockRes.status).toHaveBeenCalledWith(202); + }); + it('should handle API key auth without user', async () => { const mockRequest: JsonRpc = { jsonrpc: '2.0', @@ -225,6 +256,7 @@ describe('McpCoreController', () => { mockApiKey, undefined, undefined, + mockRes, ); expect(mcpProtocolService.handleMCPCoreQuery).toHaveBeenCalledWith( diff --git a/packages/twenty-server/src/engine/api/mcp/controllers/mcp-core.controller.ts b/packages/twenty-server/src/engine/api/mcp/controllers/mcp-core.controller.ts index a32709c7ef..5b1060dfc6 100644 --- a/packages/twenty-server/src/engine/api/mcp/controllers/mcp-core.controller.ts +++ b/packages/twenty-server/src/engine/api/mcp/controllers/mcp-core.controller.ts @@ -1,13 +1,19 @@ import { Body, Controller, + HttpCode, + HttpStatus, Post, + Res, UseFilters, UseGuards, UsePipes, ValidationPipe, } from '@nestjs/common'; +import { type Response } from 'express'; +import { isDefined } from 'twenty-shared/utils'; + import { JsonRpc } from 'src/engine/api/mcp/dtos/json-rpc'; import { McpAuthGuard } from 'src/engine/api/mcp/guards/mcp-auth.guard'; import { McpProtocolService } from 'src/engine/api/mcp/services/mcp-protocol.service'; @@ -16,8 +22,8 @@ import { ApiKeyEntity } from 'src/engine/core-modules/api-key/api-key.entity'; import { UserEntity } from 'src/engine/core-modules/user/user.entity'; import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { AuthApiKey } from 'src/engine/decorators/auth/auth-api-key.decorator'; -import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator'; import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-workspace-id.decorator'; +import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard'; import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; @@ -29,6 +35,7 @@ export class McpCoreController { constructor(private readonly mcpProtocolService: McpProtocolService) {} @Post() + @HttpCode(HttpStatus.OK) @UsePipes( new ValidationPipe({ transform: true, @@ -42,12 +49,22 @@ export class McpCoreController { @AuthApiKey() apiKey: ApiKeyEntity | undefined, @AuthUser({ allowUndefined: true }) user: UserEntity | undefined, @AuthUserWorkspaceId() userWorkspaceId: string | undefined, + @Res({ passthrough: true }) res: Response, ) { - return await this.mcpProtocolService.handleMCPCoreQuery(body, { + const result = await this.mcpProtocolService.handleMCPCoreQuery(body, { workspace, userId: user?.id, userWorkspaceId, apiKey, }); + + // JSON-RPC notifications (no id) expect no response body + if (!isDefined(result)) { + res.status(HttpStatus.ACCEPTED); + + return; + } + + return result; } } diff --git a/packages/twenty-server/src/engine/api/mcp/dtos/json-rpc.ts b/packages/twenty-server/src/engine/api/mcp/dtos/json-rpc.ts index ad9eb9526e..99ed0102da 100644 --- a/packages/twenty-server/src/engine/api/mcp/dtos/json-rpc.ts +++ b/packages/twenty-server/src/engine/api/mcp/dtos/json-rpc.ts @@ -29,5 +29,5 @@ export class JsonRpc { @IsOptional() @Validate(IsNumberOrString) - id: string | number; + id?: string | number; } diff --git a/packages/twenty-server/src/engine/api/mcp/services/__tests__/mcp-protocol.service.spec.ts b/packages/twenty-server/src/engine/api/mcp/services/__tests__/mcp-protocol.service.spec.ts index 53906ff923..dd235901bd 100644 --- a/packages/twenty-server/src/engine/api/mcp/services/__tests__/mcp-protocol.service.spec.ts +++ b/packages/twenty-server/src/engine/api/mcp/services/__tests__/mcp-protocol.service.spec.ts @@ -1,7 +1,10 @@ import { HttpException, HttpStatus } from '@nestjs/common'; import { Test, type TestingModule } from '@nestjs/testing'; -import { MCP_SERVER_METADATA } from 'src/engine/api/mcp/constants/mcp.const'; +import { JSON_RPC_ERROR_CODE } from 'src/engine/api/mcp/constants/json-rpc-error-code.const'; +import { MCP_PROTOCOL_VERSION } from 'src/engine/api/mcp/constants/mcp-protocol-version.const'; +import { MCP_SERVER_INFO } from 'src/engine/api/mcp/constants/mcp-server-info.const'; +import { MCP_SERVER_INSTRUCTIONS } from 'src/engine/api/mcp/constants/mcp-server-instructions.const'; import { type JsonRpc } from 'src/engine/api/mcp/dtos/json-rpc'; import { McpProtocolService } from 'src/engine/api/mcp/services/mcp-protocol.service'; import { McpToolExecutorService } from 'src/engine/api/mcp/services/mcp-tool-executor.service'; @@ -97,21 +100,23 @@ describe('McpProtocolService', () => { }); describe('handleInitialize', () => { - it('should return correct initialization response', () => { + it('should return spec-compliant initialization response', () => { const requestId = '123'; const result = service.handleInitialize(requestId); - expect(result).toMatchObject({ + expect(result).toEqual({ id: requestId, jsonrpc: '2.0', - result: expect.objectContaining({ - ...MCP_SERVER_METADATA, + result: { + protocolVersion: MCP_PROTOCOL_VERSION, capabilities: { tools: { listChanged: false }, resources: { listChanged: false }, prompts: { listChanged: false }, }, - }), + serverInfo: MCP_SERVER_INFO, + instructions: MCP_SERVER_INSTRUCTIONS, + }, }); }); }); @@ -176,20 +181,37 @@ describe('McpProtocolService', () => { apiKey: undefined, }); - expect(result).toMatchObject({ + expect(result).toEqual({ id: '123', jsonrpc: '2.0', - result: expect.objectContaining({ - ...MCP_SERVER_METADATA, + result: { + protocolVersion: MCP_PROTOCOL_VERSION, capabilities: { tools: { listChanged: false }, resources: { listChanged: false }, prompts: { listChanged: false }, }, - }), + serverInfo: MCP_SERVER_INFO, + instructions: MCP_SERVER_INSTRUCTIONS, + }, }); }); + it('should return null for notifications (no id)', async () => { + const mockRequest: JsonRpc = { + jsonrpc: '2.0', + method: 'notifications/initialized', + }; + + const result = await service.handleMCPCoreQuery(mockRequest, { + workspace: mockWorkspace, + userWorkspaceId: mockUserWorkspaceId, + apiKey: undefined, + }); + + expect(result).toBeNull(); + }); + it('should build a ToolSet with exactly 5 tools and pass it to executor for tools/call', async () => { userRoleService.getRoleIdForUserWorkspace.mockResolvedValue(mockRoleId); @@ -197,7 +219,6 @@ describe('McpProtocolService', () => { id: '123', jsonrpc: '2.0', result: { - ...MCP_SERVER_METADATA, content: [{ type: 'text', text: '{}' }], isError: false, }, @@ -276,6 +297,71 @@ describe('McpProtocolService', () => { ); }); + it('should return prompts list without role resolution', async () => { + const mockRequest: JsonRpc = { + jsonrpc: '2.0', + method: 'prompts/list', + id: '123', + }; + + const result = await service.handleMCPCoreQuery(mockRequest, { + workspace: mockWorkspace, + userWorkspaceId: mockUserWorkspaceId, + apiKey: undefined, + }); + + expect(result).toEqual({ + id: '123', + jsonrpc: '2.0', + result: { prompts: [] }, + }); + expect(userRoleService.getRoleIdForUserWorkspace).not.toHaveBeenCalled(); + }); + + it('should return resources list without role resolution', async () => { + const mockRequest: JsonRpc = { + jsonrpc: '2.0', + method: 'resources/list', + id: '123', + }; + + const result = await service.handleMCPCoreQuery(mockRequest, { + workspace: mockWorkspace, + userWorkspaceId: mockUserWorkspaceId, + apiKey: undefined, + }); + + expect(result).toEqual({ + id: '123', + jsonrpc: '2.0', + result: { resources: [] }, + }); + expect(userRoleService.getRoleIdForUserWorkspace).not.toHaveBeenCalled(); + }); + + it('should return method not found for unknown methods', async () => { + const mockRequest: JsonRpc = { + jsonrpc: '2.0', + method: 'unknown/method', + id: '123', + }; + + const result = await service.handleMCPCoreQuery(mockRequest, { + workspace: mockWorkspace, + userWorkspaceId: mockUserWorkspaceId, + apiKey: undefined, + }); + + expect(result).toEqual({ + id: '123', + jsonrpc: '2.0', + error: { + code: JSON_RPC_ERROR_CODE.METHOD_NOT_FOUND, + message: "Method 'unknown/method' not found", + }, + }); + }); + it('should handle tools/call with apiKey authentication', async () => { const mockToolCallResponse = { id: '123', @@ -306,20 +392,17 @@ describe('McpProtocolService', () => { ); }); - it('should handle error when tool execution fails', async () => { + it('should wrap unexpected errors with INTERNAL_ERROR code', async () => { userRoleService.getRoleIdForUserWorkspace.mockResolvedValue(mockRoleId); mcpToolExecutorService.handleToolCall.mockRejectedValue( - new HttpException( - "Tool 'nonExistentTool' not found", - HttpStatus.NOT_FOUND, - ), + new Error('Something went wrong'), ); const mockRequest: JsonRpc = { jsonrpc: '2.0', method: 'tools/call', - params: { name: 'nonExistentTool', arguments: {} }, + params: { name: 'execute_tool', arguments: {} }, id: '123', }; @@ -333,9 +416,36 @@ describe('McpProtocolService', () => { id: '123', jsonrpc: '2.0', error: { - ...MCP_SERVER_METADATA, - code: HttpStatus.NOT_FOUND, - message: "Tool 'nonExistentTool' not found", + code: JSON_RPC_ERROR_CODE.INTERNAL_ERROR, + message: 'Something went wrong', + }, + }); + }); + + it('should wrap HttpException errors with SERVER_ERROR code', async () => { + userRoleService.getRoleIdForUserWorkspace.mockRejectedValue( + new HttpException('Role ID missing', HttpStatus.FORBIDDEN), + ); + + const mockRequest: JsonRpc = { + jsonrpc: '2.0', + method: 'tools/call', + params: { name: 'execute_tool', arguments: {} }, + id: '123', + }; + + const result = await service.handleMCPCoreQuery(mockRequest, { + workspace: mockWorkspace, + userWorkspaceId: mockUserWorkspaceId, + apiKey: undefined, + }); + + expect(result).toEqual({ + id: '123', + jsonrpc: '2.0', + error: { + code: JSON_RPC_ERROR_CODE.SERVER_ERROR, + message: 'Role ID missing', }, }); }); diff --git a/packages/twenty-server/src/engine/api/mcp/services/__tests__/mcp-tool-executor.service.spec.ts b/packages/twenty-server/src/engine/api/mcp/services/__tests__/mcp-tool-executor.service.spec.ts new file mode 100644 index 0000000000..b65fa4626e --- /dev/null +++ b/packages/twenty-server/src/engine/api/mcp/services/__tests__/mcp-tool-executor.service.spec.ts @@ -0,0 +1,146 @@ +import { JSON_RPC_ERROR_CODE } from 'src/engine/api/mcp/constants/json-rpc-error-code.const'; +import { McpToolExecutorService } from 'src/engine/api/mcp/services/mcp-tool-executor.service'; + +describe('McpToolExecutorService', () => { + let service: McpToolExecutorService; + + beforeEach(() => { + service = new McpToolExecutorService(); + }); + + describe('handleToolsListing', () => { + it('should return only tools array in result', () => { + const toolSet = { + test_tool: { + description: 'A test tool', + inputSchema: { + jsonSchema: { + type: 'object', + properties: { query: { type: 'string' } }, + required: ['query'], + }, + }, + }, + } as any; + + const result = service.handleToolsListing('123', toolSet); + + expect(result).toEqual({ + id: '123', + jsonrpc: '2.0', + result: { + tools: [ + { + name: 'test_tool', + description: 'A test tool', + inputSchema: { + type: 'object', + properties: { query: { type: 'string' } }, + required: ['query'], + }, + }, + ], + }, + }); + }); + + it('should keep inputSchema unchanged when it is already a plain schema', () => { + const toolSet = { + plain_tool: { + description: 'A plain schema tool', + inputSchema: { + type: 'object', + properties: { name: { type: 'string' } }, + }, + }, + } as any; + + const result = service.handleToolsListing('456', toolSet); + + expect(result).toEqual({ + id: '456', + jsonrpc: '2.0', + result: { + tools: [ + { + name: 'plain_tool', + description: 'A plain schema tool', + inputSchema: { + type: 'object', + properties: { name: { type: 'string' } }, + }, + }, + ], + }, + }); + }); + }); + + describe('handleToolCall', () => { + it('should return JSON-RPC error with INVALID_PARAMS for unknown tools', async () => { + const toolSet = {} as any; + + const result = await service.handleToolCall('123', toolSet, { + name: 'nonexistent_tool', + arguments: {}, + }); + + expect(result).toEqual({ + id: '123', + jsonrpc: '2.0', + error: { + code: JSON_RPC_ERROR_CODE.INVALID_PARAMS, + message: 'Unknown tool: nonexistent_tool', + }, + }); + }); + + it('should return result with isError: false on success', async () => { + const toolSet = { + my_tool: { + execute: jest.fn().mockResolvedValue({ data: 'ok' }), + description: 'My tool', + inputSchema: { type: 'object' }, + }, + } as any; + + const result = await service.handleToolCall('123', toolSet, { + name: 'my_tool', + arguments: {}, + }); + + expect(result).toEqual({ + id: '123', + jsonrpc: '2.0', + result: { + content: [{ type: 'text', text: '{"data":"ok"}' }], + isError: false, + }, + }); + }); + + it('should return result with isError: true when tool execution throws', async () => { + const toolSet = { + failing_tool: { + execute: jest.fn().mockRejectedValue(new Error('API rate limited')), + description: 'A tool that fails', + inputSchema: { type: 'object' }, + }, + } as any; + + const result = await service.handleToolCall('123', toolSet, { + name: 'failing_tool', + arguments: {}, + }); + + expect(result).toEqual({ + id: '123', + jsonrpc: '2.0', + result: { + content: [{ type: 'text', text: 'API rate limited' }], + isError: true, + }, + }); + }); + }); +}); diff --git a/packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts b/packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts index b6b0586bf0..aba1a31278 100644 --- a/packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts +++ b/packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts @@ -3,6 +3,10 @@ import { HttpException, HttpStatus, Injectable } from '@nestjs/common'; import { type ToolSet, zodSchema } from 'ai'; import { isDefined } from 'twenty-shared/utils'; +import { JSON_RPC_ERROR_CODE } from 'src/engine/api/mcp/constants/json-rpc-error-code.const'; +import { MCP_PROTOCOL_VERSION } from 'src/engine/api/mcp/constants/mcp-protocol-version.const'; +import { MCP_SERVER_INFO } from 'src/engine/api/mcp/constants/mcp-server-info.const'; +import { MCP_SERVER_INSTRUCTIONS } from 'src/engine/api/mcp/constants/mcp-server-instructions.const'; import { type JsonRpc } from 'src/engine/api/mcp/dtos/json-rpc'; import { McpToolExecutorService } from 'src/engine/api/mcp/services/mcp-tool-executor.service'; import { wrapJsonRpcResponse } from 'src/engine/api/mcp/utils/wrap-jsonrpc-response.util'; @@ -51,14 +55,14 @@ export class McpProtocolService { handleInitialize(requestId: string | number) { return wrapJsonRpcResponse(requestId, { result: { + protocolVersion: MCP_PROTOCOL_VERSION, capabilities: { tools: { listChanged: false }, resources: { listChanged: false }, prompts: { listChanged: false }, }, - tools: [], - resources: [], - prompts: [], + serverInfo: MCP_SERVER_INFO, + instructions: MCP_SERVER_INSTRUCTIONS, }, }); } @@ -152,6 +156,7 @@ export class McpProtocolService { }; } + // Returns null for JSON-RPC notifications (no id), which require no response body async handleMCPCoreQuery( { id, method, params }: JsonRpc, { @@ -165,20 +170,40 @@ export class McpProtocolService { userWorkspaceId?: string; apiKey: ApiKeyEntity | undefined; }, - ): Promise> { + ): Promise | null> { try { + // JSON-RPC notifications have no id and expect no response + if (!isDefined(id)) { + return null; + } + if (method === 'initialize') { return this.handleInitialize(id); } if (method === 'ping') { - return wrapJsonRpcResponse( - id, - { - result: {}, + return wrapJsonRpcResponse(id, { result: {} }); + } + + if (method === 'prompts/list') { + return wrapJsonRpcResponse(id, { + result: { prompts: [] }, + }); + } + + if (method === 'resources/list') { + return wrapJsonRpcResponse(id, { + result: { resources: [] }, + }); + } + + if (method !== 'tools/list' && method !== 'tools/call') { + return wrapJsonRpcResponse(id, { + error: { + code: JSON_RPC_ERROR_CODE.METHOD_NOT_FOUND, + message: `Method '${method}' not found`, }, - true, - ); + }); } const roleId = await this.getRoleId( @@ -197,7 +222,16 @@ export class McpProtocolService { userWorkspaceId, }); - if (method === 'tools/call' && params) { + if (method === 'tools/call') { + if (!params) { + return wrapJsonRpcResponse(id, { + error: { + code: JSON_RPC_ERROR_CODE.INVALID_PARAMS, + message: 'tools/call requires params with name and arguments', + }, + }); + } + return await this.mcpToolExecutorService.handleToolCall( id, toolSet, @@ -205,40 +239,22 @@ export class McpProtocolService { ); } - if (method === 'tools/list') { - return this.mcpToolExecutorService.handleToolsListing(id, toolSet); - } - - if (method === 'prompts/list') { - return wrapJsonRpcResponse(id, { - result: { - capabilities: { - prompts: { listChanged: false }, - }, - prompts: [], - }, - }); - } - - if (method === 'resources/list') { - return wrapJsonRpcResponse(id, { - result: { - capabilities: { - resources: { listChanged: false }, - }, - resources: [], - }, - }); - } - - return wrapJsonRpcResponse(id, { - result: {}, - }); + return this.mcpToolExecutorService.handleToolsListing(id, toolSet); } catch (error) { - return wrapJsonRpcResponse(id, { + if (error instanceof HttpException) { + return wrapJsonRpcResponse(id ?? 0, { + error: { + code: JSON_RPC_ERROR_CODE.SERVER_ERROR, + message: error.message || 'Request failed', + }, + }); + } + + return wrapJsonRpcResponse(id ?? 0, { error: { - code: error.status || HttpStatus.INTERNAL_SERVER_ERROR, - message: error.message || 'Failed to execute tool', + code: JSON_RPC_ERROR_CODE.INTERNAL_ERROR, + message: + error instanceof Error ? error.message : 'Internal server error', }, }); } diff --git a/packages/twenty-server/src/engine/api/mcp/services/mcp-tool-executor.service.ts b/packages/twenty-server/src/engine/api/mcp/services/mcp-tool-executor.service.ts index 297361f63e..bc14645ce3 100644 --- a/packages/twenty-server/src/engine/api/mcp/services/mcp-tool-executor.service.ts +++ b/packages/twenty-server/src/engine/api/mcp/services/mcp-tool-executor.service.ts @@ -1,8 +1,9 @@ -import { HttpException, HttpStatus, Injectable } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { type ToolSet } from 'ai'; import { isDefined } from 'twenty-shared/utils'; +import { JSON_RPC_ERROR_CODE } from 'src/engine/api/mcp/constants/json-rpc-error-code.const'; import { wrapJsonRpcResponse } from 'src/engine/api/mcp/utils/wrap-jsonrpc-response.util'; @Injectable() @@ -15,29 +16,43 @@ export class McpToolExecutorService { const toolName = params.name as keyof typeof toolSet; const tool = toolSet[toolName]; - if (isDefined(tool) && isDefined(tool.execute)) { + if (!isDefined(tool) || !isDefined(tool.execute)) { + return wrapJsonRpcResponse(id, { + error: { + code: JSON_RPC_ERROR_CODE.INVALID_PARAMS, + message: `Unknown tool: ${String(params.name)}`, + }, + }); + } + + try { + const result = await tool.execute(params.arguments, { + toolCallId: '1', + messages: [], + }); + + return wrapJsonRpcResponse(id, { + result: { + content: [{ type: 'text', text: JSON.stringify(result) }], + isError: false, + }, + }); + } catch (executionError) { return wrapJsonRpcResponse(id, { result: { content: [ { type: 'text', - text: JSON.stringify( - await tool.execute(params.arguments, { - toolCallId: '1', - messages: [], - }), - ), + text: + executionError instanceof Error + ? executionError.message + : 'Tool execution failed', }, ], - isError: false, + isError: true, }, }); } - - throw new HttpException( - `Tool '${params.name}' not found`, - HttpStatus.NOT_FOUND, - ); } handleToolsListing(id: string | number, toolSet: ToolSet) { @@ -63,12 +78,7 @@ export class McpToolExecutorService { return wrapJsonRpcResponse(id, { result: { - capabilities: { - tools: { listChanged: false }, - }, tools: toolsArray, - resources: [], - prompts: [], }, }); } diff --git a/packages/twenty-server/src/engine/api/mcp/utils/wrap-jsonrpc-response.util.ts b/packages/twenty-server/src/engine/api/mcp/utils/wrap-jsonrpc-response.util.ts index 78a44a0641..4eaea1f97f 100644 --- a/packages/twenty-server/src/engine/api/mcp/utils/wrap-jsonrpc-response.util.ts +++ b/packages/twenty-server/src/engine/api/mcp/utils/wrap-jsonrpc-response.util.ts @@ -1,28 +1,12 @@ -import { MCP_SERVER_METADATA } from 'src/engine/api/mcp/constants/mcp.const'; - export const wrapJsonRpcResponse = ( id: string | number, payload: | Record<'result', Record> | Record<'error', Record>, - omitMetadata = false, ) => { - const body = - 'result' in payload - ? { - result: omitMetadata - ? payload.result - : { ...payload.result, ...MCP_SERVER_METADATA }, - } - : { - error: omitMetadata - ? payload.error - : { ...payload.error, ...MCP_SERVER_METADATA }, - }; - return { id, jsonrpc: '2.0', - ...body, + ...payload, }; }; diff --git a/packages/twenty-server/test/integration/ai/suites/mcp.controller.integration-spec.ts b/packages/twenty-server/test/integration/ai/suites/mcp.controller.integration-spec.ts index e13692b7c2..401ab608a9 100644 --- a/packages/twenty-server/test/integration/ai/suites/mcp.controller.integration-spec.ts +++ b/packages/twenty-server/test/integration/ai/suites/mcp.controller.integration-spec.ts @@ -21,7 +21,7 @@ describe('MCP Controller (integration)', () => { it('should respond to ping with a JSON-RPC result envelope', async () => { await postMcp({ jsonrpc: '2.0', method: 'ping', id: '1' }) - .expect(201) + .expect(200) .expect((res) => { expect(res.body).toMatchObject({ id: '1', @@ -32,18 +32,26 @@ describe('MCP Controller (integration)', () => { }); }); - it('should respond to initialize with server metadata and capabilities', async () => { + it('should respond to initialize with spec-compliant InitializeResult', async () => { await postMcp({ jsonrpc: '2.0', method: 'initialize', id: 123 }) - .expect(201) + .expect(200) .expect((res) => { expect(res.body.id).toBe(123); expect(res.body.jsonrpc).toBe('2.0'); expect(res.body.result).toBeDefined(); - // Should include capabilities and server metadata fields merged by wrapJsonRpcResponse + expect(res.body.result.protocolVersion).toBe('2024-11-05'); expect(res.body.result.capabilities).toBeDefined(); - expect(Array.isArray(res.body.result.tools)).toBe(true); - expect(Array.isArray(res.body.result.resources)).toBe(true); - expect(Array.isArray(res.body.result.prompts)).toBe(true); + expect(res.body.result.serverInfo).toBeDefined(); + expect(res.body.result.serverInfo.name).toBe('Twenty MCP Server'); + expect(typeof res.body.result.instructions).toBe('string'); + }); + }); + + it('should return 202 with no body for notifications/initialized (no id)', async () => { + await postMcp({ jsonrpc: '2.0', method: 'notifications/initialized' }) + .expect(202) + .expect((res) => { + expect(res.body).toEqual({}); }); }); @@ -61,12 +69,11 @@ describe('MCP Controller (integration)', () => { jsonrpc: '2.0', method: 'tools/list', id: 'tools-list-1', - }).expect(201); + }).expect(200); expect(res.body.id).toBe('tools-list-1'); expect(res.body.jsonrpc).toBe('2.0'); expect(res.body.result).toBeDefined(); - expect(res.body.result.capabilities?.tools?.listChanged).toBe(false); expect(Array.isArray(res.body.result.tools)).toBe(true); // In a seeded workspace, there should be at least one tool expect(res.body.result.tools.length).toBeGreaterThanOrEqual(0); @@ -80,18 +87,17 @@ describe('MCP Controller (integration)', () => { } }); - it('should return empty result for tools/call without params', async () => { + it('should return invalid params error for tools/call without params', async () => { const res = await postMcp({ jsonrpc: '2.0', method: 'tools/call', id: 'tools-call-empty', - }).expect(201); + }).expect(200); - expect(res.body).toMatchObject({ - id: 'tools-call-empty', - jsonrpc: '2.0', - result: {}, - }); + expect(res.body.id).toBe('tools-call-empty'); + expect(res.body.jsonrpc).toBe('2.0'); + expect(res.body.error).toBeDefined(); + expect(res.body.error.code).toBe(-32602); }); it('should return error when calling a non-existent tool', async () => { @@ -100,13 +106,12 @@ describe('MCP Controller (integration)', () => { method: 'tools/call', id: 'tools-call-missing', params: { name: 'non_existent_tool', arguments: {} }, - }).expect(201); + }).expect(200); expect(res.body.id).toBe('tools-call-missing'); expect(res.body.jsonrpc).toBe('2.0'); expect(res.body.error).toBeDefined(); - // From McpService error wrapper: code = HttpStatus.NOT_FOUND (404) and message containing tool name - expect(res.body.error.code).toBe(404); + expect(res.body.error.code).toBe(-32602); expect(String(res.body.error.message)).toMatch(/non_existent_tool/); }); @@ -115,7 +120,7 @@ describe('MCP Controller (integration)', () => { jsonrpc: '2.0', method: 'tools/list', id: 'tools-list-for-calls', - }).expect(201); + }).expect(200); const tools: Array<{ name: string }> = list.body.result.tools || []; @@ -130,7 +135,7 @@ describe('MCP Controller (integration)', () => { method: 'tools/call', id, params: { name: tool.name, arguments: {} }, - }).expect(201); + }).expect(200); expect(res.body.jsonrpc).toBe('2.0'); expect(res.body.id).toBe(id); @@ -142,27 +147,21 @@ describe('MCP Controller (integration)', () => { } }); - it('should list prompts and resources as empty arrays with listChanged=false', async () => { + it('should list prompts and resources as empty arrays', async () => { const prompts = await postMcp({ jsonrpc: '2.0', method: 'prompts/list', id: 'prompts-1', - }).expect(201); + }).expect(200); - expect(prompts.body.result.capabilities?.prompts?.listChanged).toBe( - false, - ); expect(Array.isArray(prompts.body.result.prompts)).toBe(true); const resources = await postMcp({ jsonrpc: '2.0', method: 'resources/list', id: 'resources-1', - }).expect(201); + }).expect(200); - expect(resources.body.result.capabilities?.resources?.listChanged).toBe( - false, - ); expect(Array.isArray(resources.body.result.resources)).toBe(true); }); });