From 531410f64a8dbaeb3d8b39035ddf7838fd5ca3a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Thu, 28 May 2026 14:17:10 +0200 Subject: [PATCH] fix(ai): expose MORPH_RELATION join columns in AI/MCP tool schemas (#21012) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fixes a bug where `noteTarget` (and any other morph-relation join object) created via AI/MCP would land with `targetCompanyId` / `targetPersonId` / `targetOpportunityId` left null, even though the tool reported success. - Root cause: the Zod schema generators for the AI tools only branched on `FieldMetadataType.RELATION`. MORPH_RELATION fields fell through to the default case — for `create_*` they were exposed as `targetCompany: string` instead of `targetCompanyId: uuid`, and for `group_by_*` they were silently skipped entirely. Downstream (`data-arg-processor.service.ts` and the group-by arg processor) already accept the join-column form for both kinds of relations via `computeMorphOrRelationFieldJoinColumnName` and `isMorphOrRelationFlatFieldMetadata`, so the fix is purely in the schema generators. ## Changes - `record-properties.zod-schema.ts` — extend the existing RELATION MANY_TO_ONE / ONE_TO_MANY branches to also match MORPH_RELATION. - `group-by-tool.zod-schema.ts` — replace the silent MORPH_RELATION skip with the same treatment as RELATION MANY_TO_ONE (exposes `${name}Id` as a groupBy option). - `test/integration/ai/suites/mcp-tool-execution.integration-spec.ts` — new file. First integration test for tool execution end-to-end. Drives the real MCP JSON-RPC endpoint with the seeded API key (`learn_tools` for schema introspection, `execute_tool` for invocation): - asserts `create_note_target`'s schema exposes `targetCompanyId` / `targetPersonId` / `targetOpportunityId` as UUIDs and does **not** expose `targetCompany` / `targetPerson` / `targetOpportunity`. - creates a company + note + noteTarget via MCP, then queries the workspace schema to confirm `targetCompanyId` is actually persisted in the FK column. - asserts `group_by_note_targets` schema accepts `targetCompanyId` as a groupBy key. - sets up 3 noteTargets (2 → company A, 1 → company B), calls `group_by_note_targets` by `targetCompanyId`, and asserts the counts. Out of scope: `record-filter.zod-schema.ts` has the same pattern (only RELATION) — left for a follow-up so this PR stays focused on what was reported. ## Test plan - [x] `npx nx typecheck twenty-server` - [x] `npx oxlint --type-aware` on changed files — clean - [x] `npx oxfmt --check` on changed files — clean - [x] Integration tests pass (4/4) after `database:reset`: - `should expose the morph-relation join columns as \`${name}Id\` UUID parameters` - `should persist targetCompanyId when create_note_target is invoked via MCP` - `should expose targetCompanyId as a valid groupBy option` - `should group noteTargets by targetCompanyId via MCP` --- .../zod-schemas/group-by-tool.zod-schema.ts | 10 +- .../record-properties.zod-schema.ts | 8 +- .../mcp-tool-execution.integration-spec.ts | 360 ++++++++++++++++++ 3 files changed, 371 insertions(+), 7 deletions(-) create mode 100644 packages/twenty-server/test/integration/ai/suites/mcp-tool-execution.integration-spec.ts diff --git a/packages/twenty-server/src/engine/core-modules/record-crud/zod-schemas/group-by-tool.zod-schema.ts b/packages/twenty-server/src/engine/core-modules/record-crud/zod-schemas/group-by-tool.zod-schema.ts index 62a4d70253..24ecad95ce 100644 --- a/packages/twenty-server/src/engine/core-modules/record-crud/zod-schemas/group-by-tool.zod-schema.ts +++ b/packages/twenty-server/src/engine/core-modules/record-crud/zod-schemas/group-by-tool.zod-schema.ts @@ -66,7 +66,11 @@ const buildGroupByEntriesAndDescriptions = ( continue; } - if (isFieldMetadataEntityOfType(field, FieldMetadataType.RELATION)) { + const isRelationOrMorphRelation = + isFieldMetadataEntityOfType(field, FieldMetadataType.RELATION) || + isFieldMetadataEntityOfType(field, FieldMetadataType.MORPH_RELATION); + + if (isRelationOrMorphRelation) { if (field.settings?.relationType === RelationType.MANY_TO_ONE) { const relationFieldName = `${field.name}Id`; @@ -79,10 +83,6 @@ const buildGroupByEntriesAndDescriptions = ( continue; } - if (isFieldMetadataEntityOfType(field, FieldMetadataType.MORPH_RELATION)) { - continue; - } - if (isFieldMetadataDateKind(field.type)) { groupByEntries.push( z.object({ [field.name]: dateGroupBySchema }).strict(), diff --git a/packages/twenty-server/src/engine/core-modules/record-crud/zod-schemas/record-properties.zod-schema.ts b/packages/twenty-server/src/engine/core-modules/record-crud/zod-schemas/record-properties.zod-schema.ts index 75cfb4a91b..7dc8d9d72d 100644 --- a/packages/twenty-server/src/engine/core-modules/record-crud/zod-schemas/record-properties.zod-schema.ts +++ b/packages/twenty-server/src/engine/core-modules/record-crud/zod-schemas/record-properties.zod-schema.ts @@ -93,8 +93,12 @@ export const generateRecordPropertiesZodSchema = ( return; } + const isRelationOrMorphRelation = + isFieldMetadataEntityOfType(field, FieldMetadataType.RELATION) || + isFieldMetadataEntityOfType(field, FieldMetadataType.MORPH_RELATION); + if ( - isFieldMetadataEntityOfType(field, FieldMetadataType.RELATION) && + isRelationOrMorphRelation && field.settings?.relationType === RelationType.MANY_TO_ONE ) { const uuidSchema = z.uuidv4(); @@ -107,7 +111,7 @@ export const generateRecordPropertiesZodSchema = ( } if ( - isFieldMetadataEntityOfType(field, FieldMetadataType.RELATION) && + isRelationOrMorphRelation && field.settings?.relationType === RelationType.ONE_TO_MANY ) { return; diff --git a/packages/twenty-server/test/integration/ai/suites/mcp-tool-execution.integration-spec.ts b/packages/twenty-server/test/integration/ai/suites/mcp-tool-execution.integration-spec.ts new file mode 100644 index 0000000000..cd63e73b78 --- /dev/null +++ b/packages/twenty-server/test/integration/ai/suites/mcp-tool-execution.integration-spec.ts @@ -0,0 +1,360 @@ +import { randomUUID } from 'node:crypto'; + +import request from 'supertest'; + +import { deleteRecordsByIds } from 'test/integration/utils/delete-records-by-ids'; + +const TEST_WORKSPACE_SCHEMA = 'workspace_1wgvd1injqtife6y4rvfbu3h5'; + +const TOOL_NAMES = { + createCompany: 'create_company', + createNote: 'create_note', + createNoteTarget: 'create_note_target', + groupByNoteTargets: 'group_by_note_targets', +} as const; + +type McpToolCallResult = { + content?: Array<{ type: string; text: string }>; + isError?: boolean; +}; + +type DatabaseToolPayload = { + success: boolean; + message: string; + result: TResult; + error?: string; +}; + +type LearnToolsPayload = { + tools: Array<{ + name: string; + description?: string; + inputSchema?: Record; + }>; + notFound: string[]; + message: string; +}; + +type CreatedRecord = { id: string }; + +const baseUrl = `http://localhost:${APP_PORT}`; +const endpoint = '/mcp'; + +const postMcp = (body: Record) => + request(baseUrl) + .post(endpoint) + .set('Authorization', `Bearer ${API_KEY_ACCESS_TOKEN}`) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json') + .send(JSON.stringify(body)); + +const callMcpTool = async ( + name: string, + args: Record, + id: string = `call-${randomUUID()}`, +): Promise => { + const res = await postMcp({ + jsonrpc: '2.0', + method: 'tools/call', + id, + params: { name, arguments: args }, + }).expect(200); + + expect(res.body.id).toBe(id); + expect(res.body.jsonrpc).toBe('2.0'); + expect(res.body.error).toBeUndefined(); + + return res.body.result as McpToolCallResult; +}; + +const parseToolPayload = (result: McpToolCallResult): T => { + expect(result.isError).toBe(false); + expect(result.content?.[0]?.type).toBe('text'); + + const raw = result.content?.[0]?.text; + + expect(raw).toBeDefined(); + + return JSON.parse(raw as string) as T; +}; + +const executeWorkspaceTool = async ( + toolName: string, + args: Record, +): Promise => { + const mcpResult = await callMcpTool('execute_tool', { + toolName, + arguments: args, + }); + const payload = parseToolPayload>(mcpResult); + + expect(payload.success).toBe(true); + expect(payload.result).toBeDefined(); + + return payload.result; +}; + +const learnToolSchema = async ( + toolName: string, +): Promise> => { + const mcpResult = await callMcpTool('learn_tools', { + toolNames: [toolName], + aspects: ['schema'], + }); + const payload = parseToolPayload(mcpResult); + + expect(payload.notFound).toEqual([]); + expect(payload.tools).toHaveLength(1); + expect(payload.tools[0].name).toBe(toolName); + + const inputSchema = payload.tools[0].inputSchema; + + expect(inputSchema).toBeDefined(); + + return inputSchema as Record; +}; + +describe('MCP tool execution (integration)', () => { + describe('create_note_target (morph relation join)', () => { + let createdCompanyId: string | undefined; + let createdNoteId: string | undefined; + let createdNoteTargetId: string | undefined; + + afterAll(async () => { + if (createdNoteTargetId) { + await deleteRecordsByIds('noteTarget', [createdNoteTargetId]); + } + if (createdNoteId) { + await deleteRecordsByIds('note', [createdNoteId]); + } + if (createdCompanyId) { + await deleteRecordsByIds('company', [createdCompanyId]); + } + }); + + it('should expose the morph-relation join columns as `${name}Id` UUID parameters', async () => { + const inputSchema = await learnToolSchema(TOOL_NAMES.createNoteTarget); + + const properties = ( + inputSchema as { + properties?: Record; + } + ).properties; + + expect(properties).toBeDefined(); + + expect(properties?.noteId).toMatchObject({ + type: 'string', + format: 'uuid', + }); + + // Morph relations must be exposed as `${name}Id` UUIDs (the join column), + // not as the relation name typed as string — the data-arg-processor only + // accepts the join-column form for write operations. + expect(properties?.targetCompanyId).toMatchObject({ + type: 'string', + format: 'uuid', + }); + expect(properties?.targetPersonId).toMatchObject({ + type: 'string', + format: 'uuid', + }); + expect(properties?.targetOpportunityId).toMatchObject({ + type: 'string', + format: 'uuid', + }); + + expect(properties?.targetCompany).toBeUndefined(); + expect(properties?.targetPerson).toBeUndefined(); + expect(properties?.targetOpportunity).toBeUndefined(); + }); + + it('should persist targetCompanyId when create_note_target is invoked via MCP', async () => { + const company = await executeWorkspaceTool( + TOOL_NAMES.createCompany, + { name: `mcp-tool-exec-company-${randomUUID()}` }, + ); + + createdCompanyId = company.id; + expect(createdCompanyId).toBeDefined(); + + const note = await executeWorkspaceTool( + TOOL_NAMES.createNote, + { title: `mcp-tool-exec-note-${randomUUID()}` }, + ); + + createdNoteId = note.id; + expect(createdNoteId).toBeDefined(); + + const noteTarget = await executeWorkspaceTool( + TOOL_NAMES.createNoteTarget, + { + noteId: createdNoteId, + targetCompanyId: createdCompanyId, + }, + ); + + createdNoteTargetId = noteTarget.id; + expect(createdNoteTargetId).toBeDefined(); + + const rows = (await global.testDataSource.query( + `SELECT "noteId", "targetCompanyId", "targetPersonId", "targetOpportunityId" + FROM "${TEST_WORKSPACE_SCHEMA}"."noteTarget" WHERE id = $1`, + [createdNoteTargetId], + )) as Array<{ + noteId: string; + targetCompanyId: string | null; + targetPersonId: string | null; + targetOpportunityId: string | null; + }>; + + expect(rows).toHaveLength(1); + expect(rows[0].noteId).toBe(createdNoteId); + expect(rows[0].targetCompanyId).toBe(createdCompanyId); + expect(rows[0].targetPersonId).toBeNull(); + expect(rows[0].targetOpportunityId).toBeNull(); + }); + }); + + describe('group_by_note_targets (morph relation column)', () => { + let createdCompanyAId: string | undefined; + let createdCompanyBId: string | undefined; + let createdNoteId: string | undefined; + const createdNoteTargetIds: string[] = []; + + beforeAll(async () => { + const companyA = await executeWorkspaceTool( + TOOL_NAMES.createCompany, + { name: `mcp-group-by-company-A-${randomUUID()}` }, + ); + + createdCompanyAId = companyA.id; + + const companyB = await executeWorkspaceTool( + TOOL_NAMES.createCompany, + { name: `mcp-group-by-company-B-${randomUUID()}` }, + ); + + createdCompanyBId = companyB.id; + + const note = await executeWorkspaceTool( + TOOL_NAMES.createNote, + { title: `mcp-group-by-note-${randomUUID()}` }, + ); + + createdNoteId = note.id; + + // Two targets on company A, one on company B — the grouped counts + // we'll assert against later. + const targetCompanyIds = [ + createdCompanyAId, + createdCompanyAId, + createdCompanyBId, + ]; + + for (const targetCompanyId of targetCompanyIds) { + const noteTarget = await executeWorkspaceTool( + TOOL_NAMES.createNoteTarget, + { noteId: createdNoteId, targetCompanyId }, + ); + + createdNoteTargetIds.push(noteTarget.id); + } + }); + + afterAll(async () => { + if (createdNoteTargetIds.length > 0) { + await deleteRecordsByIds('noteTarget', createdNoteTargetIds); + } + if (createdNoteId) { + await deleteRecordsByIds('note', [createdNoteId]); + } + + const companyIds = [createdCompanyAId, createdCompanyBId].filter( + (id): id is string => typeof id === 'string', + ); + + if (companyIds.length > 0) { + await deleteRecordsByIds('company', companyIds); + } + }); + + it('should expose targetCompanyId as a valid groupBy option', async () => { + const inputSchema = await learnToolSchema(TOOL_NAMES.groupByNoteTargets); + const groupByItems = ( + inputSchema as { + properties?: { + groupBy?: { + items?: { + anyOf?: Array<{ properties?: Record }>; + properties?: Record; + }; + }; + }; + } + ).properties?.groupBy?.items; + + expect(groupByItems).toBeDefined(); + + // Each groupBy variant is { [columnName]: true }. Collect every column + // the schema offers so we can assert on the morph-relation columns. + const groupByColumns = new Set(); + + if (groupByItems?.anyOf) { + for (const variant of groupByItems.anyOf) { + for (const propertyName of Object.keys(variant.properties ?? {})) { + groupByColumns.add(propertyName); + } + } + } else if (groupByItems?.properties) { + for (const propertyName of Object.keys(groupByItems.properties)) { + groupByColumns.add(propertyName); + } + } + + expect(groupByColumns.has('noteId')).toBe(true); + expect(groupByColumns.has('targetCompanyId')).toBe(true); + expect(groupByColumns.has('targetPersonId')).toBe(true); + expect(groupByColumns.has('targetOpportunityId')).toBe(true); + }); + + it('should group noteTargets by targetCompanyId via MCP', async () => { + type GroupByGroup = { + dimensions: unknown[]; + value: string | number; + }; + type GroupByResult = { + groups: GroupByGroup[]; + dimensionLabels: string[]; + aggregation: string; + groupCount: number; + }; + + const result = await executeWorkspaceTool( + TOOL_NAMES.groupByNoteTargets, + { + groupBy: [{ targetCompanyId: true }], + aggregateOperation: 'COUNT', + // Scope to the noteTargets we created so other seeded rows don't + // leak into the counts. + noteId: { eq: createdNoteId }, + }, + ); + + expect(result.dimensionLabels).toEqual(['targetCompanyId']); + expect(result.aggregation).toBe('COUNT'); + expect(result.groupCount).toBe(2); + + // Dimensions are returned positionally, aligned with dimensionLabels. + const countsByCompany = Object.fromEntries( + result.groups.map((group) => [ + String(group.dimensions[0]), + Number(group.value), + ]), + ); + + expect(countsByCompany[createdCompanyAId as string]).toBe(2); + expect(countsByCompany[createdCompanyBId as string]).toBe(1); + }); + }); +});