From 9f16b13843e075b3dcf28d29408e19b0ae2b5d4f Mon Sep 17 00:00:00 2001 From: Antoine Moreaux Date: Mon, 25 Aug 2025 10:30:38 +0200 Subject: [PATCH] =?UTF-8?q?feat(ai):=20add=20integration=20tests=20for=20M?= =?UTF-8?q?CP=20controller=20and=20improve=20JSON-R=E2=80=A6=20(#14047)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …PC validation - Introduced full integration test suite for MCP controller, testing `POST /mcp` with valid and invalid payloads. - Added `@IsDefined` validation to ensure the `method` field is required in JSON-RPC requests. - Applied `RestApiExceptionFilter` to MCP controller for consistent error handling. - Enhanced validation pipe in MCP controller to whitelist and reject non-whitelisted properties. - Consolidated exception filters in SSOAuthController. --- .../ai/controllers/mcp.controller.spec.ts | 7 + .../ai/controllers/mcp.controller.ts | 13 +- .../engine/core-modules/ai/dtos/json-rpc.ts | 2 + .../auth/controllers/sso-auth.controller.ts | 7 +- .../suites/mcp.controller.integration-spec.ts | 169 ++++++++++++++++++ 5 files changed, 190 insertions(+), 8 deletions(-) create mode 100644 packages/twenty-server/test/integration/ai/suites/mcp.controller.integration-spec.ts diff --git a/packages/twenty-server/src/engine/core-modules/ai/controllers/mcp.controller.spec.ts b/packages/twenty-server/src/engine/core-modules/ai/controllers/mcp.controller.spec.ts index 6da63c16cb..89d3a7881b 100644 --- a/packages/twenty-server/src/engine/core-modules/ai/controllers/mcp.controller.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/ai/controllers/mcp.controller.spec.ts @@ -6,6 +6,7 @@ import { type Workspace } from 'src/engine/core-modules/workspace/workspace.enti import { MCP_SERVER_METADATA } from 'src/engine/core-modules/ai/constants/mcp.const'; import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/access-token.service'; import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; +import { HttpExceptionHandlerService } from 'src/engine/core-modules/exception-handler/http-exception-handler.service'; import { McpController } from './mcp.controller'; @@ -33,6 +34,12 @@ describe('McpController', () => { provide: WorkspaceCacheStorageService, useValue: jest.fn(), }, + { + provide: HttpExceptionHandlerService, + useValue: { + handleError: jest.fn(), + }, + }, ], }).compile(); diff --git a/packages/twenty-server/src/engine/core-modules/ai/controllers/mcp.controller.ts b/packages/twenty-server/src/engine/core-modules/ai/controllers/mcp.controller.ts index 34b9564bb3..b5ea753372 100644 --- a/packages/twenty-server/src/engine/core-modules/ai/controllers/mcp.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/ai/controllers/mcp.controller.ts @@ -2,6 +2,7 @@ import { Body, Controller, Post, + UseFilters, UseGuards, UsePipes, ValidationPipe, @@ -15,21 +16,29 @@ import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-worksp import { JsonRpc } from 'src/engine/core-modules/ai/dtos/json-rpc'; import { McpService } from 'src/engine/core-modules/ai/services/mcp.service'; import { JwtAuthGuard } from 'src/engine/guards/jwt-auth.guard'; +import { RestApiExceptionFilter } from 'src/engine/api/rest/rest-api-exception.filter'; @Controller('mcp') @UseGuards(JwtAuthGuard, WorkspaceAuthGuard) +@UseFilters(RestApiExceptionFilter) export class McpController { constructor(private readonly mcpService: McpService) {} @Post() - @UsePipes(new ValidationPipe({ transform: true })) + @UsePipes( + new ValidationPipe({ + transform: true, + whitelist: true, + forbidNonWhitelisted: true, + }), + ) async handleMcpCore( @Body() body: JsonRpc, @AuthWorkspace() workspace: Workspace, @AuthApiKey() apiKey: string | undefined, @AuthUserWorkspaceId() userWorkspaceId: string | undefined, ) { - return this.mcpService.handleMCPCoreQuery(body, { + return await this.mcpService.handleMCPCoreQuery(body, { workspace, userWorkspaceId, apiKey, diff --git a/packages/twenty-server/src/engine/core-modules/ai/dtos/json-rpc.ts b/packages/twenty-server/src/engine/core-modules/ai/dtos/json-rpc.ts index 12ef0a0b27..2c6d50d8bf 100644 --- a/packages/twenty-server/src/engine/core-modules/ai/dtos/json-rpc.ts +++ b/packages/twenty-server/src/engine/core-modules/ai/dtos/json-rpc.ts @@ -1,4 +1,5 @@ import { + IsDefined, IsNotEmpty, IsObject, IsOptional, @@ -14,6 +15,7 @@ export class JsonRpc { @Matches(/^2\.0$/, { message: 'jsonrpc must be exactly "2.0"' }) jsonrpc = '2.0'; + @IsDefined({ message: 'method is required' }) @IsString() @IsNotEmpty() method: string; diff --git a/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts b/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts index ec24e1f32d..3f9244ee39 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts @@ -19,7 +19,6 @@ import { AuthException, AuthExceptionCode, } from 'src/engine/core-modules/auth/auth.exception'; -import { AuthOAuthExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-oauth-exception.filter'; import { AuthRestApiExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-rest-api-exception.filter'; import { EnterpriseFeaturesEnabledGuard } from 'src/engine/core-modules/auth/guards/enterprise-features-enabled.guard'; import { OIDCAuthGuard } from 'src/engine/core-modules/auth/guards/oidc-auth.guard'; @@ -42,6 +41,7 @@ import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace. import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; @Controller('auth') +@UseFilters(AuthRestApiExceptionFilter) export class SSOAuthController { constructor( private readonly loginTokenService: LoginTokenService, @@ -58,7 +58,6 @@ export class SSOAuthController { @Get('saml/metadata/:identityProviderId') @UseGuards(EnterpriseFeaturesEnabledGuard, PublicEndpointGuard) - @UseFilters(AuthRestApiExceptionFilter) // eslint-disable-next-line @typescript-eslint/no-explicit-any async generateMetadata(@Req() req: any): Promise { return generateServiceProviderMetadata({ @@ -76,7 +75,6 @@ export class SSOAuthController { @Get('oidc/login/:identityProviderId') @UseGuards(EnterpriseFeaturesEnabledGuard, OIDCAuthGuard, PublicEndpointGuard) - @UseFilters(AuthRestApiExceptionFilter) async oidcAuth() { // As this method is protected by OIDC Auth guard, it will trigger OIDC SSO flow return; @@ -84,7 +82,6 @@ export class SSOAuthController { @Get('saml/login/:identityProviderId') @UseGuards(EnterpriseFeaturesEnabledGuard, SAMLAuthGuard, PublicEndpointGuard) - @UseFilters(AuthRestApiExceptionFilter) async samlAuth() { // As this method is protected by SAML Auth guard, it will trigger SAML SSO flow return; @@ -92,14 +89,12 @@ export class SSOAuthController { @Get('oidc/callback') @UseGuards(EnterpriseFeaturesEnabledGuard, OIDCAuthGuard, PublicEndpointGuard) - @UseFilters(AuthOAuthExceptionFilter) async oidcAuthCallback(@Req() req: OIDCRequest, @Res() res: Response) { return await this.authCallback(req, res); } @Post('saml/callback/:identityProviderId') @UseGuards(EnterpriseFeaturesEnabledGuard, SAMLAuthGuard, PublicEndpointGuard) - @UseFilters(AuthOAuthExceptionFilter) async samlAuthCallback(@Req() req: SAMLRequest, @Res() res: Response) { try { return await this.authCallback(req, res); 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 new file mode 100644 index 0000000000..e13692b7c2 --- /dev/null +++ b/packages/twenty-server/test/integration/ai/suites/mcp.controller.integration-spec.ts @@ -0,0 +1,169 @@ +import request from 'supertest'; + +/** + * Integration tests for MCP core controller + * + * These tests hit the real Nest app bootstrapped by test/integration/utils/setup-test.ts + * and exercise the guarded POST /mcp endpoint using valid/invalid JSON-RPC payloads. + */ + +describe('MCP Controller (integration)', () => { + const baseUrl = `http://localhost:${APP_PORT}`; + const endpoint = '/mcp'; + + const postMcp = (body: any, bearer: string = API_KEY_ACCESS_TOKEN) => { + return request(baseUrl) + .post(endpoint) + .set('Authorization', `Bearer ${bearer}`) + .set('Content-Type', 'application/json') + .send(JSON.stringify(body)); + }; + + it('should respond to ping with a JSON-RPC result envelope', async () => { + await postMcp({ jsonrpc: '2.0', method: 'ping', id: '1' }) + .expect(201) + .expect((res) => { + expect(res.body).toMatchObject({ + id: '1', + jsonrpc: '2.0', + result: {}, + }); + expect(res.body.error).toBeUndefined(); + }); + }); + + it('should respond to initialize with server metadata and capabilities', async () => { + await postMcp({ jsonrpc: '2.0', method: 'initialize', id: 123 }) + .expect(201) + .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.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); + }); + }); + + it('should validate request body and return 400 for invalid payload (missing method)', async () => { + await postMcp({}) + .expect(400) + .expect((res) => { + expect(res.body.error).toBe('BadRequestException'); + }); + }); + + describe('MCP Tools', () => { + it('should list available tools with schemas', async () => { + const res = await postMcp({ + jsonrpc: '2.0', + method: 'tools/list', + id: 'tools-list-1', + }).expect(201); + + 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); + // If tools exist, they should have name, description and inputSchema + const first = res.body.result.tools[0]; + + if (first) { + expect(first.name).toBeDefined(); + expect(first.description).toBeDefined(); + expect(first.inputSchema).toBeDefined(); + } + }); + + it('should return empty result for tools/call without params', async () => { + const res = await postMcp({ + jsonrpc: '2.0', + method: 'tools/call', + id: 'tools-call-empty', + }).expect(201); + + expect(res.body).toMatchObject({ + id: 'tools-call-empty', + jsonrpc: '2.0', + result: {}, + }); + }); + + it('should return error when calling a non-existent tool', async () => { + const res = await postMcp({ + jsonrpc: '2.0', + method: 'tools/call', + id: 'tools-call-missing', + params: { name: 'non_existent_tool', arguments: {} }, + }).expect(201); + + 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(String(res.body.error.message)).toMatch(/non_existent_tool/); + }); + + it('should exercise each listed tool with a call and receive a JSON-RPC envelope', async () => { + const list = await postMcp({ + jsonrpc: '2.0', + method: 'tools/list', + id: 'tools-list-for-calls', + }).expect(201); + + const tools: Array<{ name: string }> = list.body.result.tools || []; + + // For each tool, attempt a call with empty arguments. We only assert we get a valid envelope + // with either a result or an error. This ensures the MCP controller routes tool calls correctly + // for every tool exposed in the workspace, without relying on specific seeded data. + for (let i = 0; i < tools.length; i++) { + const tool = tools[i]; + const id = `tool-call-${i}`; + const res = await postMcp({ + jsonrpc: '2.0', + method: 'tools/call', + id, + params: { name: tool.name, arguments: {} }, + }).expect(201); + + expect(res.body.jsonrpc).toBe('2.0'); + expect(res.body.id).toBe(id); + // Either a structured result or an error is acceptable depending on validation/business rules + const hasResultContent = !!res.body?.result?.content; + const hasError = !!res.body?.error; + + expect(hasResultContent || hasError).toBe(true); + } + }); + + it('should list prompts and resources as empty arrays with listChanged=false', async () => { + const prompts = await postMcp({ + jsonrpc: '2.0', + method: 'prompts/list', + id: 'prompts-1', + }).expect(201); + + 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(resources.body.result.capabilities?.resources?.listChanged).toBe( + false, + ); + expect(Array.isArray(resources.body.result.resources)).toBe(true); + }); + }); +});