From e00dce45c65a8e2e535ce29b99f3f09a90e60048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= Date: Thu, 4 Jun 2026 21:54:28 +0200 Subject: [PATCH] feat(core): Add telemetry for mcp skills use (no-changelog) (#31625) Co-authored-by: Ricardo Espinoza --- .../create-workflow-from-code.tool.test.ts | 52 ++++++++++- .../modules/mcp/__tests__/skills-used.test.ts | 93 +++++++++++++++++++ .../__tests__/update-workflow.tool.test.ts | 62 ++++++++++++- .../create-workflow-from-code.tool.ts | 13 ++- .../mcp/tools/workflow-builder/skills-used.ts | 22 +++++ .../workflow-builder/update-workflow.tool.ts | 13 ++- 6 files changed, 251 insertions(+), 4 deletions(-) create mode 100644 packages/cli/src/modules/mcp/__tests__/skills-used.test.ts create mode 100644 packages/cli/src/modules/mcp/tools/workflow-builder/skills-used.ts diff --git a/packages/cli/src/modules/mcp/__tests__/create-workflow-from-code.tool.test.ts b/packages/cli/src/modules/mcp/__tests__/create-workflow-from-code.tool.test.ts index 021fb90e7e6..e580b919ad6 100644 --- a/packages/cli/src/modules/mcp/__tests__/create-workflow-from-code.tool.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/create-workflow-from-code.tool.test.ts @@ -162,6 +162,7 @@ describe('create-workflow-from-code MCP tool', () => { const callHandler = async ( input: { code: string; + skillsUsed?: string[]; name?: string; description?: string; projectId?: string; @@ -172,6 +173,7 @@ describe('create-workflow-from-code MCP tool', () => { await tool.handler( { code: input.code, + skillsUsed: input.skillsUsed, name: input.name as string, description: input.description as string, projectId: input.projectId as string, @@ -404,13 +406,16 @@ describe('create-workflow-from-code MCP tool', () => { }); test('tracks telemetry on success', async () => { - await callHandler({ code: 'const wf = ...' }); + await callHandler({ code: 'const wf = ...', skillsUsed: ['workflow-builder'] }); expect(telemetry.track).toHaveBeenCalledWith( 'User called mcp tool', expect.objectContaining({ user_id: 'user-1', tool_name: 'create_workflow_from_code', + parameters: expect.objectContaining({ + skillsUsed: ['workflow-builder'], + }), results: expect.objectContaining({ success: true, data: expect.objectContaining({ @@ -422,6 +427,51 @@ describe('create-workflow-from-code MCP tool', () => { ); }); + test('omits skillsUsed from telemetry when not provided', async () => { + await callHandler({ code: 'const wf = ...' }); + + const trackedPayload = (telemetry.track as jest.Mock).mock.calls[0][1] as { + parameters: Record; + }; + expect(trackedPayload.parameters).not.toHaveProperty('skillsUsed'); + }); + + test('omits skillsUsed from telemetry when an empty array is passed', async () => { + await callHandler({ code: 'const wf = ...', skillsUsed: [] }); + + const trackedPayload = (telemetry.track as jest.Mock).mock.calls[0][1] as { + parameters: Record; + }; + expect(trackedPayload.parameters).not.toHaveProperty('skillsUsed'); + }); + + test('normalizes skillsUsed before tracking telemetry', async () => { + await callHandler({ + code: 'const wf = ...', + skillsUsed: [' Workflow-Builder ', 'workflow-builder', 'has spaces', 'NODE-SELECTION'], + }); + + expect(telemetry.track).toHaveBeenCalledWith( + 'User called mcp tool', + expect.objectContaining({ + parameters: expect.objectContaining({ + skillsUsed: ['workflow-builder', 'node-selection'], + }), + }), + ); + }); + + test('does not reject the call when skillsUsed overflows the cap', async () => { + const oversized = Array.from({ length: 60 }, (_, i) => `skill-${i}`); + const result = await callHandler({ code: 'const wf = ...', skillsUsed: oversized }); + + expect(result.isError).toBeUndefined(); + const trackedPayload = (telemetry.track as jest.Mock).mock.calls[0][1] as { + parameters: { skillsUsed: string[] }; + }; + expect(trackedPayload.parameters.skillsUsed).toHaveLength(50); + }); + test('assigns webhookId to webhook nodes before saving', async () => { nodeTypes.getByNameAndVersion.mockImplementation(((type: string) => { if (type === 'n8n-nodes-base.webhook') { diff --git a/packages/cli/src/modules/mcp/__tests__/skills-used.test.ts b/packages/cli/src/modules/mcp/__tests__/skills-used.test.ts new file mode 100644 index 00000000000..8178996cd31 --- /dev/null +++ b/packages/cli/src/modules/mcp/__tests__/skills-used.test.ts @@ -0,0 +1,93 @@ +import { sanitizeSkillsUsed } from '../tools/workflow-builder/skills-used'; + +describe('sanitizeSkillsUsed', () => { + test('returns undefined for non-array input', () => { + expect(sanitizeSkillsUsed(undefined)).toBeUndefined(); + expect(sanitizeSkillsUsed(null)).toBeUndefined(); + expect(sanitizeSkillsUsed('workflow-builder')).toBeUndefined(); + expect(sanitizeSkillsUsed(42)).toBeUndefined(); + expect(sanitizeSkillsUsed({ skill: 'workflow-builder' })).toBeUndefined(); + }); + + test('returns undefined for an empty array', () => { + expect(sanitizeSkillsUsed([])).toBeUndefined(); + }); + + test('returns undefined when every entry is invalid', () => { + expect(sanitizeSkillsUsed(['', ' ', 'UPPER CASE', 'has spaces', '!!!'])).toBeUndefined(); + }); + + test('keeps valid kebab-case identifiers', () => { + expect(sanitizeSkillsUsed(['workflow-builder', 'node-selection'])).toEqual([ + 'workflow-builder', + 'node-selection', + ]); + }); + + test('keeps identifiers with underscores and dots', () => { + expect(sanitizeSkillsUsed(['snake_case_skill', 'dotted.skill.name'])).toEqual([ + 'snake_case_skill', + 'dotted.skill.name', + ]); + }); + + test('keeps identifiers starting with a digit', () => { + expect(sanitizeSkillsUsed(['1-step-skill'])).toEqual(['1-step-skill']); + }); + + test('trims and lowercases entries', () => { + expect(sanitizeSkillsUsed([' Workflow-Builder ', 'NODE-SELECTION'])).toEqual([ + 'workflow-builder', + 'node-selection', + ]); + }); + + test('drops entries with disallowed characters', () => { + expect( + sanitizeSkillsUsed([ + 'workflow-builder', + 'has spaces', + 'has/slash', + 'has:colon', + 'unicode-‑hyphen', + 'contains "quote"', + '-starts-with-dash', + '.starts-with-dot', + ]), + ).toEqual(['workflow-builder']); + }); + + test('drops non-string entries', () => { + expect(sanitizeSkillsUsed(['workflow-builder', 42, null, undefined, { x: 1 }])).toEqual([ + 'workflow-builder', + ]); + }); + + test('drops entries longer than 64 chars', () => { + const tooLong = 'a'.repeat(65); + const justRight = 'a'.repeat(64); + expect(sanitizeSkillsUsed([tooLong, justRight, 'workflow-builder'])).toEqual([ + justRight, + 'workflow-builder', + ]); + }); + + test('deduplicates after normalization', () => { + expect( + sanitizeSkillsUsed([ + 'workflow-builder', + 'Workflow-Builder', + ' workflow-builder ', + 'node-selection', + ]), + ).toEqual(['workflow-builder', 'node-selection']); + }); + + test('caps the result at 50 entries without rejecting the input', () => { + const input = Array.from({ length: 60 }, (_, i) => `skill-${i}`); + const result = sanitizeSkillsUsed(input); + expect(result).toHaveLength(50); + expect(result?.[0]).toBe('skill-0'); + expect(result?.[49]).toBe('skill-49'); + }); +}); diff --git a/packages/cli/src/modules/mcp/__tests__/update-workflow.tool.test.ts b/packages/cli/src/modules/mcp/__tests__/update-workflow.tool.test.ts index cf5d327c9fc..4650292a18c 100644 --- a/packages/cli/src/modules/mcp/__tests__/update-workflow.tool.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/update-workflow.tool.test.ts @@ -139,12 +139,13 @@ describe('update-workflow MCP tool', () => { ); const callHandler = async ( - input: { workflowId: string; operations: unknown[] }, + input: { workflowId: string; skillsUsed?: string[]; operations: unknown[] }, tool = createTool(), ) => await tool.handler( { workflowId: input.workflowId, + skillsUsed: input.skillsUsed, operations: input.operations as never, }, {} as never, @@ -400,6 +401,7 @@ describe('update-workflow MCP tool', () => { test('tracks telemetry on success with op metadata', async () => { await callHandler({ workflowId: 'wf-1', + skillsUsed: ['workflow-builder', 'node-selection'], operations: [ { type: 'setWorkflowMetadata', name: 'Renamed' }, { type: 'updateNodeParameters', nodeName: 'B', parameters: { url: 'https://new' } }, @@ -413,6 +415,7 @@ describe('update-workflow MCP tool', () => { tool_name: 'update_workflow', parameters: expect.objectContaining({ workflowId: 'wf-1', + skillsUsed: ['workflow-builder', 'node-selection'], opCount: 2, opTypes: ['setWorkflowMetadata', 'updateNodeParameters'], }), @@ -421,6 +424,63 @@ describe('update-workflow MCP tool', () => { ); }); + test('omits skillsUsed from telemetry when not provided', async () => { + await callHandler({ + workflowId: 'wf-1', + operations: [{ type: 'setWorkflowMetadata', name: 'Renamed' }], + }); + + const trackedPayload = (telemetry.track as jest.Mock).mock.calls[0][1] as { + parameters: Record; + }; + expect(trackedPayload.parameters).not.toHaveProperty('skillsUsed'); + }); + + test('omits skillsUsed from telemetry when an empty array is passed', async () => { + await callHandler({ + workflowId: 'wf-1', + skillsUsed: [], + operations: [{ type: 'setWorkflowMetadata', name: 'Renamed' }], + }); + + const trackedPayload = (telemetry.track as jest.Mock).mock.calls[0][1] as { + parameters: Record; + }; + expect(trackedPayload.parameters).not.toHaveProperty('skillsUsed'); + }); + + test('normalizes skillsUsed before tracking telemetry', async () => { + await callHandler({ + workflowId: 'wf-1', + skillsUsed: [' Workflow-Builder ', 'workflow-builder', 'has spaces', 'NODE-SELECTION'], + operations: [{ type: 'setWorkflowMetadata', name: 'Renamed' }], + }); + + expect(telemetry.track).toHaveBeenCalledWith( + 'User called mcp tool', + expect.objectContaining({ + parameters: expect.objectContaining({ + skillsUsed: ['workflow-builder', 'node-selection'], + }), + }), + ); + }); + + test('does not reject the call when skillsUsed overflows the cap', async () => { + const oversized = Array.from({ length: 60 }, (_, i) => `skill-${i}`); + const result = await callHandler({ + workflowId: 'wf-1', + skillsUsed: oversized, + operations: [{ type: 'setWorkflowMetadata', name: 'Renamed' }], + }); + + expect(result.isError).toBeUndefined(); + const trackedPayload = (telemetry.track as jest.Mock).mock.calls[0][1] as { + parameters: { skillsUsed: string[] }; + }; + expect(trackedPayload.parameters.skillsUsed).toHaveLength(50); + }); + test('tracks telemetry on failure', async () => { const result = await callHandler({ workflowId: 'wf-1', diff --git a/packages/cli/src/modules/mcp/tools/workflow-builder/create-workflow-from-code.tool.ts b/packages/cli/src/modules/mcp/tools/workflow-builder/create-workflow-from-code.tool.ts index adde0cc190a..873101af6fc 100644 --- a/packages/cli/src/modules/mcp/tools/workflow-builder/create-workflow-from-code.tool.ts +++ b/packages/cli/src/modules/mcp/tools/workflow-builder/create-workflow-from-code.tool.ts @@ -5,6 +5,7 @@ import { buildInvalidAiToolSourceErrorResponse } from './connection-structure-ch import { MCP_CREATE_WORKFLOW_FROM_CODE_TOOL, CODE_BUILDER_VALIDATE_TOOL } from './constants'; import { autoPopulateNodeCredentials, stripNullCredentialStubs } from './credentials-auto-assign'; import { validateDataTableReferencesForWorkflow } from './data-table-validation'; +import { sanitizeSkillsUsed } from './skills-used'; import { USER_CALLED_MCP_TOOL_EVENT } from '../../mcp.constants'; import type { ToolDefinition, UserCalledMCPToolEventPayload } from '../../mcp.types'; import { getSdkReferenceHint } from '../workflow-validation.utils'; @@ -25,6 +26,12 @@ const inputSchema = { .describe( `Full TypeScript/JavaScript workflow code using the n8n Workflow SDK. Must be validated first with ${CODE_BUILDER_VALIDATE_TOOL.toolName}.`, ), + skillsUsed: z + .array(z.string()) + .optional() + .describe( + 'Names of n8n skills (lowercase kebab-case identifiers) used by the MCP client to produce this workflow create call. Server-side normalization will trim, lowercase, dedupe, and drop entries that are not valid skill identifiers.', + ), name: z .string() .max(128) @@ -105,7 +112,7 @@ export const createCreateWorkflowFromCodeTool = ( ): ToolDefinition => ({ name: MCP_CREATE_WORKFLOW_FROM_CODE_TOOL.toolName, config: { - description: `Create a workflow in n8n from validated SDK code. This tool expects code that already follows the n8n Workflow SDK patterns and has passed ${CODE_BUILDER_VALIDATE_TOOL.toolName}. If code fails to parse, call get_sdk_reference, rewrite the code using the reference, validate again, then retry creation. If the user named a target project, resolve it via search_projects before calling this tool; when projectId is omitted, the workflow is created in the user's personal project. After creation, always tell the user which project the workflow landed in (see the targetProject field in the response).`, + description: `Create a workflow in n8n from validated SDK code. This tool expects code that already follows the n8n Workflow SDK patterns and has passed ${CODE_BUILDER_VALIDATE_TOOL.toolName}. If code fails to parse, call get_sdk_reference, rewrite the code using the reference, validate again, then retry creation. If the user named a target project, resolve it via search_projects before calling this tool; when projectId is omitted, the workflow is created in the user's personal project. If you used n8n skills while preparing this workflow, pass their identifiers in skillsUsed. After creation, always tell the user which project the workflow landed in (see the targetProject field in the response).`, inputSchema, outputSchema, annotations: { @@ -118,22 +125,26 @@ export const createCreateWorkflowFromCodeTool = ( }, handler: async ({ code, + skillsUsed, name, description, projectId, folderId, }: { code: string; + skillsUsed?: string[]; name?: string; description?: string; projectId?: string; folderId?: string; }) => { + const sanitizedSkillsUsed = sanitizeSkillsUsed(skillsUsed); const telemetryPayload: UserCalledMCPToolEventPayload = { user_id: user.id, tool_name: MCP_CREATE_WORKFLOW_FROM_CODE_TOOL.toolName, parameters: { codeLength: code.length, + ...(sanitizedSkillsUsed !== undefined ? { skillsUsed: sanitizedSkillsUsed } : {}), hasName: !!name, hasProjectId: !!projectId, hasFolderId: !!folderId, diff --git a/packages/cli/src/modules/mcp/tools/workflow-builder/skills-used.ts b/packages/cli/src/modules/mcp/tools/workflow-builder/skills-used.ts new file mode 100644 index 00000000000..667432c4da7 --- /dev/null +++ b/packages/cli/src/modules/mcp/tools/workflow-builder/skills-used.ts @@ -0,0 +1,22 @@ +import { RUNTIME_SKILL_NAME_PATTERN } from '@n8n/agents'; + +const MAX_SKILLS_LOGGED = 50; + +export function sanitizeSkillsUsed(input: unknown): string[] | undefined { + if (!Array.isArray(input)) return undefined; + + const seen = new Set(); + const out: string[] = []; + + for (const raw of input) { + if (typeof raw !== 'string') continue; + const normalized = raw.trim().toLowerCase(); + if (!RUNTIME_SKILL_NAME_PATTERN.test(normalized)) continue; + if (seen.has(normalized)) continue; + seen.add(normalized); + out.push(normalized); + if (out.length >= MAX_SKILLS_LOGGED) break; + } + + return out.length > 0 ? out : undefined; +} diff --git a/packages/cli/src/modules/mcp/tools/workflow-builder/update-workflow.tool.ts b/packages/cli/src/modules/mcp/tools/workflow-builder/update-workflow.tool.ts index 8e713aae5ea..84ad8f44440 100644 --- a/packages/cli/src/modules/mcp/tools/workflow-builder/update-workflow.tool.ts +++ b/packages/cli/src/modules/mcp/tools/workflow-builder/update-workflow.tool.ts @@ -9,6 +9,7 @@ import { MCP_UPDATE_WORKFLOW_TOOL } from './constants'; import { validateCredentialReferences } from './credential-validation'; import { autoPopulateNodeCredentials } from './credentials-auto-assign'; import { validateDataTableReferencesForUpdate } from './data-table-validation'; +import { sanitizeSkillsUsed } from './skills-used'; import { applyOperations, partialUpdateOperationSchema, @@ -60,6 +61,12 @@ function collectTouchedNodes(operations: PartialUpdateOperation[]): Map { + const sanitizedSkillsUsed = sanitizeSkillsUsed(skillsUsed); const telemetryPayload: UserCalledMCPToolEventPayload = { user_id: user.id, tool_name: MCP_UPDATE_WORKFLOW_TOOL.toolName, parameters: { workflowId, + ...(sanitizedSkillsUsed !== undefined ? { skillsUsed: sanitizedSkillsUsed } : {}), opCount: operations.length, opTypes: operations.map((op) => op.type), },