fix: return method-specific MCP responses (#18671)
## Summary Fixes #18524 Fixes the MCP response contract for non-`initialize` methods. Previously, `/mcp` returned initialize-style metadata for methods like `tools/list`, which caused strict MCP clients to reject the response shape. The endpoint also returned `201 Created` for RPC calls even though no resource was being created. ## Changes - return only method-specific payloads for MCP list methods - `tools/list` -> `{ tools: [...] }` - `prompts/list` -> `{ prompts: [] }` - `resources/list` -> `{ resources: [] }` - keep MCP server metadata only on `initialize` - make `/mcp` return `200 OK` instead of `201 Created` - add regression tests for: - `tools/list` response shape - `prompts/list` response shape - `resources/list` response shape ## Why Strict MCP clients expect: - standard RPC transport semantics over HTTP - method-specific JSON-RPC result payloads Returning initialize metadata for non-`initialize` methods breaks that expectation and can cause client deserialization or protocol validation failures. ## Verification - reproduced the issue locally against `/mcp` - verified `tools/list` was previously returning initialize-style fields - verified `tools/list` now returns only `result.tools` - verified `/mcp` now returns `200 OK` - ran targeted Jest tests: ```bash cd /Users/apple/MyProjects/OpenSource/twenty/packages/twenty-server npx jest --runInBand src/engine/api/mcp/services/__tests__/mcp-protocol.service.spec.ts src/engine/api/mcp/services/__tests__/mcp-tool-executor.service.spec.ts --------- Co-authored-by: Félix Malfait <felix@twenty.com>
This commit is contained in:
committed by
GitHub
parent
6e36ad9fa2
commit
a07337fea0
+38
-6
@@ -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(
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user