test(server): strengthen MCP catalog and gating coverage (#23630)

The three MCP testing improvements discussed in #23613 (now merged; this branch has been rebased onto main). ## What **1. Name-set assertions in `mcp-protocol.service.spec.ts`.** The two "exactly 6 tools" tests only used `expect.objectContaining` subset matches, so the size claim in the titles was never enforced and a new meta-tool would pass silently. They now assert the exact sorted key set of the ToolSet handed to the executor. This immediately caught real drift: the toolset has seven tools, and `get_tool_catalog` was missing from the expected list. **2. Catalog contract integration test** (`test/integration/ai/suites/mcp-tool-catalog.integration-spec.ts`). Calls `get_tool_catalog` over real HTTP with an API-key bearer, then for every advertised category dispatches one read-only tool (`find_/list_/get_/search_` prefixed, up to 3 candidates) through `execute_tool` and asserts a success envelope. Any newly registered provider is covered the moment it appears in the catalog, with no new test code. Categories with no read-only tool are compared exactly against a deliberate exception list, currently empty since every advertised category ships a read-only tool, so drift in either direction fails loudly. **3. Permission gating integration test** (same suite). Creates two API keys: one bound to Admin, one bound to a freshly created role with `canUpdateAllSettings: false` and no settings flags. Asserts the ROLE category (from #23613) is present in the admin catalog and absent from the restricted one, while the restricted key still sees DATABASE_CRUD read tools, proving it is gating rather than a broken catalog. This locks the provider `isAvailable` contract at the real HTTP boundary, which the unit mocks cannot. ## Testing - `npx jest src/engine/api/mcp` — 43 tests pass - `test/integration/ai/suites` — 4 suites, 24 tests pass locally against a reset DB - `npx nx typecheck twenty-server` clean; oxlint and oxfmt clean on the touched files --- _Generated by [Claude Code](https://claude.ai/code/session_0131sLKVsRuaDoaKCxFM8g4Z)_ <a href="https://cubic.dev/pr/twentyhq/twenty/pull/23630?utm_source=github" rel="nofollow noreferrer noopener" target="_blank">``<img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg">``</a>
2026-07-31 14:10:30 +02:00
parent f74f71785e
commit fec266a5ae
2 changed files with 267 additions and 2 deletions
@@ -18,6 +18,7 @@ import { type McpToolAnnotations } from 'src/engine/api/mcp/types/mcp-tool-annot
 import { type FlatApiKey } from 'src/engine/core-modules/api-key/types/flat-api-key.type';
 import { ApiKeyRoleService } from 'src/engine/core-modules/api-key/services/api-key-role.service';
 import { EXECUTE_TOOL_TOOL_NAME } from 'src/engine/core-modules/tool-provider/tools/execute-tool.tool';
+import { GET_TOOL_CATALOG_TOOL_NAME } from 'src/engine/core-modules/tool-provider/tools/get-tool-catalog.tool';
 import { LEARN_TOOLS_TOOL_NAME } from 'src/engine/core-modules/tool-provider/tools/learn-tools.tool';
 import { LOAD_SKILL_TOOL_NAME } from 'src/engine/core-modules/tool-provider/tools/load-skill.tool';
 import { ToolRegistryService } from 'src/engine/core-modules/tool-provider/services/tool-registry.service';
@@ -46,6 +47,7 @@ describe('McpProtocolService', () => {
  const EXPECTED_MCP_TOOL_NAMES = [
    LEARN_TOOLS_TOOL_NAME,
    EXECUTE_TOOL_TOOL_NAME,
+    GET_TOOL_CATALOG_TOOL_NAME,
    LOAD_SKILL_TOOL_NAME,
    LIST_OBJECT_METADATA_NAMES_TOOL_NAME,
    LIST_SKILLS_TOOL_NAME,
@@ -58,6 +60,7 @@ describe('McpProtocolService', () => {
  > = {
    [LEARN_TOOLS_TOOL_NAME]: MCP_CLOSED_WORLD_READ_ONLY_TOOL_ANNOTATIONS,
    [EXECUTE_TOOL_TOOL_NAME]: MCP_EXECUTE_TOOL_ANNOTATIONS,
+    [GET_TOOL_CATALOG_TOOL_NAME]: MCP_CLOSED_WORLD_READ_ONLY_TOOL_ANNOTATIONS,
    [LOAD_SKILL_TOOL_NAME]: MCP_CLOSED_WORLD_READ_ONLY_TOOL_ANNOTATIONS,
    [LIST_OBJECT_METADATA_NAMES_TOOL_NAME]:
      MCP_CLOSED_WORLD_READ_ONLY_TOOL_ANNOTATIONS,
@@ -265,7 +268,7 @@ describe('McpProtocolService', () => {
      expect(result).toBeNull();
    });

-    it('should build a ToolSet with exactly 6 tools and pass it to executor for tools/call', async () => {
+    it('should build the meta-tool set by name and pass it to executor for tools/call', async () => {
      userRoleService.getRoleIdForUserWorkspace.mockResolvedValue(mockRoleId);

      const mockToolCallResponse = {
@@ -314,9 +317,15 @@ describe('McpProtocolService', () => {
        mockRequest.params,
        undefined,
      );
+
+      const [, toolSet] = mcpToolExecutorService.handleToolCall.mock.calls[0];
+
+      expect(Object.keys(toolSet).sort()).toEqual(
+        [...EXPECTED_MCP_TOOL_NAMES].sort(),
+      );
    });

-    it('should build a ToolSet with exactly 6 tools and pass it to executor for tools/list', async () => {
+    it('should build the meta-tool set by name and pass it to executor for tools/list', async () => {
      userRoleService.getRoleIdForUserWorkspace.mockResolvedValue(mockRoleId);

      mcpToolExecutorService.handleToolsListing.mockReturnValue({
@@ -351,6 +360,13 @@ describe('McpProtocolService', () => {
          ),
        ),
      );
+
+      const [, toolSet] =
+        mcpToolExecutorService.handleToolsListing.mock.calls[0];
+
+      expect(Object.keys(toolSet).sort()).toEqual(
+        [...EXPECTED_MCP_TOOL_NAMES].sort(),
+      );
    });

    it('should pass actorContext with FieldActorSource.AGENT to getToolsByName', async () => {
@@ -0,0 +1,249 @@
+import { gql } from 'graphql-tag';
+import request from 'supertest';
+import { generateApiKeyToken } from 'test/integration/graphql/utils/generate-api-key-token.util';
+import { createOneRole } from 'test/integration/metadata/suites/role/utils/create-one-role.util';
+import { deleteOneRole } from 'test/integration/metadata/suites/role/utils/delete-one-role.util';
+import { makeMetadataAPIRequest } from 'test/integration/metadata/suites/utils/make-metadata-api-request.util';
+import { jestExpectToBeDefined } from 'test/utils/jest-expect-to-be-defined.util.test';
+import { ToolCategory } from 'twenty-shared/ai';
+
+/**
+ * Contract tests for the MCP tool catalog.
+ *
+ * 1. Catalog contract: every category advertised by get_tool_catalog must be
+ *    dispatchable end to end through execute_tool. Scales automatically: a
+ *    newly registered provider is covered the moment it appears in the
+ *    catalog, with no new test code.
+ * 2. Permission gating: the catalog is role-dependent. An API key bound to a
+ *    role without settings permissions must not see settings-gated tools
+ *    (e.g. the ROLE category), while an admin-bound key must.
+ */
+
+const baseUrl = `http://localhost:${APP_PORT}`;
+
+const postMcp = (body: object, bearer: string) =>
+  request(baseUrl)
+    .post('/mcp')
+    .set('Authorization', `Bearer ${bearer}`)
+    .set('Content-Type', 'application/json')
+    .set('Accept', 'application/json')
+    .send(JSON.stringify(body));
+
+const callMcpTool = async (
+  bearer: string,
+  toolName: string,
+  toolArguments: object,
+) => {
+  const response = await postMcp(
+    {
+      jsonrpc: '2.0',
+      method: 'tools/call',
+      params: { name: toolName, arguments: toolArguments },
+      id: '1',
+    },
+    bearer,
+  ).expect(200);
+
+  return response.body.result as {
+    content: { type: string; text: string }[];
+    isError: boolean;
+  };
+};
+
+const getToolCatalog = async (
+  bearer: string,
+): Promise<Record<string, { name: string; description: string }[]>> => {
+  const result = await callMcpTool(bearer, 'get_tool_catalog', {});
+
+  expect(result.isError).toBe(false);
+
+  return JSON.parse(result.content[0].text).catalog;
+};
+
+const READ_ONLY_TOOL_NAME_PATTERN = /^(find_|list_|get_|search_)/;
+
+// Deliberate exceptions to the "every advertised category is dispatchable
+// through a read-only tool" contract. Currently none: every category the MCP
+// catalog advertises ships at least one read-only tool. Adding a category
+// here must be a conscious decision, not silent drift.
+const EXPECTED_CATEGORIES_WITHOUT_READ_ONLY_TOOLS: string[] = [];
+
+const createApiKeyToken = async (roleId: string): Promise<string> => {
+  const createResponse = await makeMetadataAPIRequest({
+    query: gql`
+      mutation CreateApiKey($input: CreateApiKeyInput!) {
+        createApiKey(input: $input) {
+          id
+        }
+      }
+    `,
+    variables: {
+      input: {
+        name: `MCP catalog test key ${roleId}`,
+        expiresAt: new Date(Date.now() + 60 * 60 * 1000).toISOString(),
+        roleId,
+      },
+    },
+  });
+
+  const apiKeyId = createResponse.body.data?.createApiKey?.id;
+
+  jestExpectToBeDefined(apiKeyId);
+  createdApiKeyIds.push(apiKeyId);
+
+  const tokenResponse = await generateApiKeyToken({
+    apiKeyId,
+    accessToken: APPLE_JANE_ADMIN_ACCESS_TOKEN,
+  });
+
+  const token = tokenResponse.body.data?.generateApiKeyToken?.token;
+
+  jestExpectToBeDefined(token);
+
+  return token;
+};
+
+const createdApiKeyIds: string[] = [];
+
+describe('MCP tool catalog (integration)', () => {
+  let adminApiKeyToken: string;
+  let restrictedApiKeyToken: string;
+  let restrictedRoleId: string;
+
+  beforeAll(async () => {
+    const rolesResponse = await makeMetadataAPIRequest({
+      query: gql`
+        query GetRoles {
+          getRoles {
+            id
+            label
+          }
+        }
+      `,
+    });
+
+    const adminRoleId = rolesResponse.body.data?.getRoles?.find(
+      (role: { label: string }) => role.label === 'Admin',
+    )?.id;
+
+    jestExpectToBeDefined(adminRoleId);
+
+    const { data: restrictedRoleData } = await createOneRole({
+      expectToFail: false,
+      input: {
+        label: 'MCP Catalog Restricted Role',
+        description: 'API-key role without settings permissions',
+        icon: 'IconKey',
+        canUpdateAllSettings: false,
+        canAccessAllTools: true,
+        canReadAllObjectRecords: true,
+        canUpdateAllObjectRecords: false,
+        canSoftDeleteAllObjectRecords: false,
+        canDestroyAllObjectRecords: false,
+        canBeAssignedToUsers: false,
+        canBeAssignedToAgents: false,
+        canBeAssignedToApiKeys: true,
+      },
+    });
+
+    restrictedRoleId = restrictedRoleData?.createOneRole?.id as string;
+    jestExpectToBeDefined(restrictedRoleId);
+
+    adminApiKeyToken = await createApiKeyToken(adminRoleId);
+    restrictedApiKeyToken = await createApiKeyToken(restrictedRoleId);
+  });
+
+  afterAll(async () => {
+    for (const apiKeyId of createdApiKeyIds) {
+      await testDataSource
+        .query('DELETE FROM core."apiKey" WHERE id = $1', [apiKeyId])
+        .catch(() => {});
+    }
+
+    if (restrictedRoleId) {
+      await deleteOneRole({
+        expectToFail: false,
+        input: { idToDelete: restrictedRoleId },
+      });
+    }
+  });
+
+  describe('catalog contract', () => {
+    it('should dispatch one read-only tool per advertised category through execute_tool', async () => {
+      const catalog = await getToolCatalog(adminApiKeyToken);
+      const categories = Object.keys(catalog);
+
+      expect(categories.length).toBeGreaterThan(0);
+
+      const categoriesWithoutReadOnlyTool: string[] = [];
+
+      for (const category of categories) {
+        const readOnlyCandidates = catalog[category]
+          .filter((tool) => READ_ONLY_TOOL_NAME_PATTERN.test(tool.name))
+          .slice(0, 3);
+
+        if (readOnlyCandidates.length === 0) {
+          // A write-only category has nothing safe to dispatch in CI; it is
+          // collected and checked against the deliberate exception list below.
+          categoriesWithoutReadOnlyTool.push(category);
+          continue;
+        }
+
+        let dispatched = false;
+
+        for (const candidate of readOnlyCandidates) {
+          const result = await callMcpTool(adminApiKeyToken, 'execute_tool', {
+            toolName: candidate.name,
+            arguments: {},
+          });
+
+          if (!result.isError) {
+            dispatched = true;
+            break;
+          }
+        }
+
+        expect({ category, dispatched }).toEqual({
+          category,
+          dispatched: true,
+        });
+      }
+
+      // Exact equality fails in both directions, so gaining or losing a
+      // skipped category forces a deliberate update of the exception list.
+      expect([...categoriesWithoutReadOnlyTool].sort()).toEqual(
+        EXPECTED_CATEGORIES_WITHOUT_READ_ONLY_TOOLS,
+      );
+    });
+  });
+
+  describe('permission gating', () => {
+    it('should expose role tools to an admin-bound API key', async () => {
+      const catalog = await getToolCatalog(adminApiKeyToken);
+
+      jestExpectToBeDefined(catalog[ToolCategory.ROLE]);
+
+      const roleToolNames = catalog[ToolCategory.ROLE].map((tool) => tool.name);
+
+      expect(roleToolNames).toEqual(
+        expect.arrayContaining(['list_roles', 'create_role', 'update_role']),
+      );
+    });
+
+    it('should hide role tools from an API key without settings permissions', async () => {
+      const catalog = await getToolCatalog(restrictedApiKeyToken);
+
+      expect(catalog[ToolCategory.ROLE]).toBeUndefined();
+
+      const allToolNames = Object.values(catalog)
+        .flat()
+        .map((tool) => tool.name);
+
+      expect(allToolNames).not.toEqual(expect.arrayContaining(['create_role']));
+
+      // The restricted role still sees record read tools, proving the empty
+      // ROLE category is gating rather than a broken catalog.
+      expect(catalog[ToolCategory.DATABASE_CRUD]?.length).toBeGreaterThan(0);
+    });
+  });
+});