From 986b82fa2808595c56179555afb286395cba385c Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Fri, 16 Oct 2026 14:12:37 +0000 Subject: [PATCH] feat: Add telemetry events to the skills hub (no-changelog) (#41234) --- .../src/agents/agent-skill.schema.ts | 10 ++ .../api-types/src/agents/skills.schema.ts | 3 + packages/@n8n/telemetry/src/events/context.ts | 89 ++++++++++++ .../__tests__/agent-skills.service.test.ts | 56 ++++++++ .../__tests__/skills-api.service.test.ts | 54 ++++++++ .../agents/__tests__/skills-telemetry.test.ts | 116 ++++++++++++++++ .../modules/agents/agent-skills.service.ts | 42 ++++++ .../agents/agents-skills.controller.ts | 3 +- .../agents/skills/skills-api.service.ts | 24 +++- .../modules/agents/skills/skills-telemetry.ts | 127 ++++++++++++++++++ .../agents/__tests__/AgentSkillModal.test.ts | 3 + .../agents/__tests__/useAgentApi.test.ts | 11 ++ .../useAgentCapabilitiesActions.test.ts | 5 +- .../agents/components/AgentSkillModal.vue | 8 ++ .../agents/composables/useAgentApi.ts | 4 +- .../useAgentCapabilitiesActions.ts | 4 + .../context/views/SettingsSkillsView.test.ts | 43 +++++- .../context/views/SettingsSkillsView.vue | 28 +++- 18 files changed, 622 insertions(+), 8 deletions(-) create mode 100644 packages/cli/src/modules/agents/__tests__/skills-telemetry.test.ts create mode 100644 packages/cli/src/modules/agents/skills/skills-telemetry.ts diff --git a/packages/@n8n/api-types/src/agents/agent-skill.schema.ts b/packages/@n8n/api-types/src/agents/agent-skill.schema.ts index 18d4a02c06d..8b75da768c7 100644 --- a/packages/@n8n/api-types/src/agents/agent-skill.schema.ts +++ b/packages/@n8n/api-types/src/agents/agent-skill.schema.ts @@ -92,6 +92,16 @@ export const agentSkillAvailabilitySchema = z.enum(['project', 'agent', 'instanc export type AgentSkillAvailability = z.infer; +/** + * How a person made a new skill in the skill modal: typed it, or uploaded a SKILL.md file + * or folder. The browser parses an upload, so only the client knows. Telemetry reads it + * and the server does not store it. + */ +export const skillCreateMethodSchema = z.enum(['manual', 'upload']); + +export type SkillCreateMethod = z.infer; + export const createAgentSkillSchema = agentSkillSchema.extend({ availableTo: agentSkillAvailabilitySchema.optional(), + method: skillCreateMethodSchema.optional(), }); diff --git a/packages/@n8n/api-types/src/agents/skills.schema.ts b/packages/@n8n/api-types/src/agents/skills.schema.ts index 0798e966423..36cc852f59f 100644 --- a/packages/@n8n/api-types/src/agents/skills.schema.ts +++ b/packages/@n8n/api-types/src/agents/skills.schema.ts @@ -4,6 +4,7 @@ import { AGENT_SKILL_NAME_MAX_LENGTH, agentSkillSchema, agentSkillShape, + skillCreateMethodSchema, } from './agent-skill.schema'; import type { AgentSkill } from './types'; import type { ProjectType } from '../schemas/project.schema'; @@ -149,6 +150,8 @@ export class CreateSkillDto extends Z.class({ agentId: z.string().max(36).optional(), /** Hub skills ignore `allowedTools`, so a create that sends it is refused. */ skill: agentSkillSchema.omit({ allowedTools: true }).extend({ name: skillNameSchema }), + /** For telemetry only. Absent counts as `manual`. */ + method: skillCreateMethodSchema.optional(), }) {} /** diff --git a/packages/@n8n/telemetry/src/events/context.ts b/packages/@n8n/telemetry/src/events/context.ts index 485c4d2f8b3..b57e74342a5 100644 --- a/packages/@n8n/telemetry/src/events/context.ts +++ b/packages/@n8n/telemetry/src/events/context.ts @@ -36,6 +36,34 @@ const optionalUserId = z 'The acting user. Absent on the events the editor fires, which identify the user already', ); +/** Where a skill lives. An agent skill takes its project from its agent. */ +const skillScope = z + .enum(['project', 'agent', 'instance']) + .describe('Where the skill lives: one project, one agent, or the whole instance'); + +const skillId = z.string().describe('The skill. Its name and text are never reported'); + +const skillProjectId = z + .string() + .optional() + .describe( + 'The project of a project skill, or the project of the agent for an agent skill. Absent for an instance skill', + ); + +/** The surface that wrote the skill. Kept apart from `skill.source`, which stores no MCP value. */ +const skillSource = z + .enum(['ui', 'agent', 'mcp']) + .describe( + '`ui` is the Skills settings page. `agent` is the agent builder: a person in its skill modal, or n8n Assistant when it builds the agent. `mcp` is an MCP client', + ); + +/** Whether a person or an assistant wrote the text. */ +const skillMethod = z + .enum(['manual', 'upload', 'ai']) + .describe( + '`manual` is text typed in the skill modal. `upload` is a SKILL.md file or folder parsed in the browser. `ai` is text that n8n Assistant or an MCP client wrote. Filter out `ai` to count the skills people write by hand', + ); + export const CONTEXT_TELEMETRY = defineTelemetryEvents({ USER_VIEWED_PREFERENCES: { name: 'User viewed preferences', @@ -271,4 +299,65 @@ export const CONTEXT_TELEMETRY = defineTelemetryEvents({ text_length: textLength.optional(), }), }, + // ─── Skills hub ─── + + USER_VIEWED_SKILLS: { + name: 'User viewed skills', + description: + 'A user opened the Skills page in the Context settings area. Fires on every visit, so it counts how often the page is used, also when the user changes nothing.', + properties: z.object({ + count: z + .number() + .describe('Skills the user can see in total, not the number the first page showed'), + }), + }, + USER_OPENED_SKILL: { + name: 'User opened skill', + description: + 'A user opened one skill from the Skills settings page, in the editor or, without the edit permission, in the viewer. Fires once the skill loads. Kept apart from `User opened agent skill`, which is the agent builder.', + properties: z.object({ + skill_id: skillId, + scope: skillScope, + project_id: skillProjectId, + read_only: z.boolean().describe('Whether the user may only read the skill'), + }), + }, + USER_CREATED_SKILL: { + name: 'User created skill', + description: + 'A skill was created in the skills hub, from the settings page, the agent builder or an MCP client. Fires once the create commits. A copy that the server makes on its own, such as the startup copy or an agent duplicate, reports nothing.', + properties: z.object({ + user_id: userId, + skill_id: skillId, + scope: skillScope, + project_id: skillProjectId, + source: skillSource, + method: skillMethod, + }), + }, + USER_UPDATED_SKILL: { + name: 'User updated skill', + description: + 'A save stored a new version of a skill in the skills hub. A save with unchanged content stores no version and reports nothing, also when it moves the skill. Reports the scope after the save.', + properties: z.object({ + user_id: userId, + skill_id: skillId, + scope: skillScope, + project_id: skillProjectId, + source: skillSource, + version: z.number().describe('The number of the version this save stored'), + scope_changed: z.boolean().describe('Whether the same save moved the skill to another scope'), + }), + }, + USER_DELETED_SKILL: { + name: 'User deleted skill', + description: + 'A user deleted a skill from the skills hub on the settings page. Fires once the delete commits. Removing a skill from one agent keeps the hub skill and reports nothing here.', + properties: z.object({ + user_id: userId, + skill_id: skillId, + scope: skillScope, + project_id: skillProjectId, + }), + }, }); diff --git a/packages/cli/src/modules/agents/__tests__/agent-skills.service.test.ts b/packages/cli/src/modules/agents/__tests__/agent-skills.service.test.ts index cad6059b556..fe2c72f7bd6 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-skills.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-skills.service.test.ts @@ -19,6 +19,7 @@ import type { Agent } from '../entities/agent.entity'; import { composeJsonConfig } from '../json-config/agent-config-composition'; import type { AgentRepository } from '../repositories/agent.repository'; import type { SkillSaveResult, SkillService } from '../skills/skill.service'; +import type { SkillsTelemetry } from '../skills/skills-telemetry'; import { getAgentConfigHash, getAgentSkillHash } from '../utils/agent-config-hash'; import { type AgentFixtureOverrides, @@ -80,6 +81,7 @@ describe('AgentSkillsService', () => { let runtimeCacheService: Mocked; let modificationTelemetry: Mocked; let agentUpdateBroadcaster: Mocked; + let skillsTelemetry: Mocked; function givenAgent(overrides: AgentFixtureOverrides = {}) { const agent = makeAgent(overrides); @@ -102,6 +104,7 @@ describe('AgentSkillsService', () => { transactionRunner = mock(); transactionRunner.run.mockImplementation(async (ctx, fn) => await fn(ctx)); vi.mocked(userHasScopes).mockResolvedValue(true); + skillsTelemetry = mock(); service = new AgentSkillsService( mockLogger(), agentRepository, @@ -113,6 +116,7 @@ describe('AgentSkillsService', () => { skillRefs, skillService, transactionRunner, + skillsTelemetry, ); }); @@ -164,6 +168,39 @@ describe('AgentSkillsService', () => { expect(result.configHash).toBe(getAgentConfigHash(composeJsonConfig(agent, refs))); }); + it('reports a skill that a person made in the agent builder, with the method sent', async () => { + givenAgent({ schema: baseConfig }); + + await service.createAndAttachSkill(agentId, projectId, skill, context, 'project', 'upload'); + + expect(skillsTelemetry.created).toHaveBeenCalledWith( + 'user-1', + ['skill_new1'], + 'agent', + 'upload', + ); + }); + + it.each([ + ['builder', 'agent'], + ['mcp', 'mcp'], + ] as const)('reports a skill that %s wrote as written with AI', async (modifiedBy, source) => { + givenAgent({ schema: baseConfig }); + + await service.createAndAttachSkills(agentId, projectId, [skill], { user, modifiedBy }); + + expect(skillsTelemetry.created).toHaveBeenCalledWith('user-1', ['skill_new1'], source, 'ai'); + }); + + it('reports nothing with the skills hub off', async () => { + Object.assign(skillRefs, { usesSkillsHub: false }); + givenAgent({ schema: baseConfig }); + + await service.createAndAttachSkill(agentId, projectId, skill, context); + + expect(skillsTelemetry.created).not.toHaveBeenCalled(); + }); + it('keeps allowed tools on the agent row when the skills hub is off', async () => { Object.assign(skillRefs, { usesSkillsHub: false }); givenAgent({ schema: baseConfig }); @@ -499,6 +536,7 @@ describe('AgentSkillsService', () => { ); expect(skillService.assertCanEdit).toHaveBeenCalledWith(user, ['skill_a']); + expect(skillsTelemetry.updated).toHaveBeenCalledWith('user-1', 'skill_a', 'agent', 2, false); expect(skillService.save).toHaveBeenCalledWith( 'skill_a', { @@ -527,6 +565,24 @@ describe('AgentSkillsService', () => { }); }); + it('reports nothing for a save that stored no new version', async () => { + givenAgent({ + schema: { ...baseConfig, skills: [{ type: 'skill', id: 'skill_a' }] }, + skills: { skill_a: skill }, + }); + skillService.save.mockResolvedValue(saveResult({ created: false })); + + await service.updateSkill( + agentId, + projectId, + 'skill_a', + { instructions: 'New instructions.' }, + { user, modifiedBy: 'mcp' }, + ); + + expect(skillsTelemetry.updated).not.toHaveBeenCalled(); + }); + it('clears the pin of this agent when the pin is the latest version', async () => { const agent = givenAgent({ schema: { ...baseConfig, skills: [{ type: 'skill', id: 'skill_a', versionId: pinId }] }, diff --git a/packages/cli/src/modules/agents/__tests__/skills-api.service.test.ts b/packages/cli/src/modules/agents/__tests__/skills-api.service.test.ts index be78e9699fc..b7b8047ba9f 100644 --- a/packages/cli/src/modules/agents/__tests__/skills-api.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/skills-api.service.test.ts @@ -14,6 +14,7 @@ import type { Skill } from '../entities/skill.entity'; import type { ResolvedSkillRow, SkillRepository } from '../repositories/skill.repository'; import { SkillsApiService } from '../skills/skills-api.service'; import type { SkillService } from '../skills/skill.service'; +import type { SkillsTelemetry } from '../skills/skills-telemetry'; import { getAgentSkillHash } from '../utils/agent-config-hash'; vi.mock('@/permissions.ee/check-access', () => ({ userHasScopes: vi.fn() })); @@ -77,12 +78,14 @@ describe('SkillsApiService', () => { const projectScopeService = mock(); const txRunner = mock(); const userRepository = mock(); + const skillsTelemetry = mock(); const service = new SkillsApiService( skillService, repository, projectScopeService, txRunner, userRepository, + skillsTelemetry, ); const TX_CTX = { trx: {} } as OperationContext; let visible: Skill[]; @@ -376,6 +379,38 @@ describe('SkillsApiService', () => { expect(detail).toMatchObject({ id: 'skill_new', name: 'Skill', canEdit: false }); }); + it('reports the create with the method the client sent', async () => { + vi.mocked(hasGlobalScope).mockReturnValue(true); + + await service.create(user, payload({ method: 'upload' })); + + expect(skillsTelemetry.created).toHaveBeenCalledWith( + user.id, + [{ id: 'skill_new', projectId: null, agentId: null }], + 'ui', + 'upload', + ); + }); + + it('reports a create without a method as manual', async () => { + vi.mocked(hasGlobalScope).mockReturnValue(true); + + await service.create(user, payload({})); + + expect(skillsTelemetry.created).toHaveBeenCalledWith( + user.id, + [{ id: 'skill_new', projectId: null, agentId: null }], + 'ui', + 'manual', + ); + }); + + it('reports nothing for a refused create', async () => { + await expect(service.create(user, payload({}))).rejects.toThrow(ForbiddenError); + + expect(skillsTelemetry.created).not.toHaveBeenCalled(); + }); + it('creates an agent skill with projectSkill:create on the project of the agent', async () => { vi.mocked(userHasScopes).mockResolvedValue(true); @@ -482,6 +517,7 @@ describe('SkillsApiService', () => { user.id, ); expect(skillService.canAccess).toHaveBeenCalledWith(user, visible[0], 'update'); + expect(skillsTelemetry.updated).toHaveBeenCalledWith(user.id, visible[0], 'ui', 3, false); const { followingAgentIds: _following, ...response } = saved; expect(result).toEqual({ id: 'skill_a', @@ -536,12 +572,28 @@ describe('SkillsApiService', () => { TX_CTX, ); expect(skillService.refreshFollowingAgents).toHaveBeenCalledWith(['agent-1']); + // The place the move wrote, not a read after the commit. + expect(skillsTelemetry.updated).toHaveBeenCalledWith( + user.id, + { id: 'skill_a', projectId: 'project-1', agentId: null }, + 'ui', + 3, + true, + ); // The move runs first, so the save checks a rename in the new scope. expect(skillService.changeTarget.mock.invocationCallOrder[0]).toBeLessThan( skillService.save.mock.invocationCallOrder[0], ); }); + it('reports nothing for a save that stored no new version', async () => { + skillService.save.mockResolvedValue({ ...saved, created: false, followingAgentIds: [] }); + + await service.update(user, 'skill_a', update({ instructions: 'Do it.', baseSkillHash: 'b' })); + + expect(skillsTelemetry.updated).not.toHaveBeenCalled(); + }); + it('does not move a skill that is already in the requested scope', async () => { vi.mocked(hasGlobalScope).mockReturnValue(true); skillService.save.mockResolvedValue(saved); @@ -573,6 +625,7 @@ describe('SkillsApiService', () => { expect(skillService.canAccess).toHaveBeenCalledWith(user, visible[0], 'delete'); expect(skillService.deleteSkill).toHaveBeenCalledWith('skill_a', ['project-1']); + expect(skillsTelemetry.deleted).toHaveBeenCalledWith(user.id, visible[0]); }); it('names the agents of the projects the user may read in a conflict', async () => { @@ -594,6 +647,7 @@ describe('SkillsApiService', () => { await expect(service.delete(user, 'skill_a')).rejects.toThrow(ForbiddenError); expect(skillService.deleteSkill).not.toHaveBeenCalled(); + expect(skillsTelemetry.deleted).not.toHaveBeenCalled(); }); }); }); diff --git a/packages/cli/src/modules/agents/__tests__/skills-telemetry.test.ts b/packages/cli/src/modules/agents/__tests__/skills-telemetry.test.ts new file mode 100644 index 00000000000..f9908b66394 --- /dev/null +++ b/packages/cli/src/modules/agents/__tests__/skills-telemetry.test.ts @@ -0,0 +1,116 @@ +import { mockLogger } from '@n8n/backend-test-utils'; +import { TELEMETRY_EVENT } from '@n8n/telemetry'; +import { mock } from 'vitest-mock-extended'; + +import type { Telemetry } from '@/telemetry'; + +import type { Skill } from '../entities/skill.entity'; +import type { SkillRepository } from '../repositories/skill.repository'; +import { SkillsTelemetry } from '../skills/skills-telemetry'; + +function skill(id: string, target: Partial> = {}): Skill { + return mock({ id, projectId: target.projectId ?? null, agentId: target.agentId ?? null }); +} + +describe('SkillsTelemetry', () => { + const telemetry = mock(); + const repository = mock(); + const service = new SkillsTelemetry(telemetry, repository, mockLogger()); + const skills = [ + skill('skill_instance'), + skill('skill_project', { projectId: 'project-1' }), + skill('skill_agent', { agentId: 'agent-1' }), + ]; + + beforeEach(() => { + vi.resetAllMocks(); + repository.findByIds.mockImplementation(async (ids) => + skills.filter((s) => ids.includes(s.id)), + ); + repository.findAgents.mockImplementation( + async (ids) => + new Map( + ids + .filter((id) => id === 'agent-1') + .map((id) => [id, { name: 'Support bot', projectId: 'project-2' }] as const), + ), + ); + }); + + it('reports each created skill with its scope, and an agent skill with the project of its agent', async () => { + await service.created( + 'user-1', + ['skill_instance', 'skill_project', 'skill_agent'], + 'ui', + 'upload', + ); + + const base = { user_id: 'user-1', source: 'ui', method: 'upload' }; + expect(telemetry.track.mock.calls).toEqual([ + [ + TELEMETRY_EVENT.CONTEXT.USER_CREATED_SKILL, + { ...base, skill_id: 'skill_instance', scope: 'instance' }, + ], + [ + TELEMETRY_EVENT.CONTEXT.USER_CREATED_SKILL, + { ...base, skill_id: 'skill_project', scope: 'project', project_id: 'project-1' }, + ], + [ + TELEMETRY_EVENT.CONTEXT.USER_CREATED_SKILL, + { ...base, skill_id: 'skill_agent', scope: 'agent', project_id: 'project-2' }, + ], + ]); + }); + + it('reports an update with the scope after the save', async () => { + await service.updated('user-1', 'skill_project', 'mcp', 4, true); + + expect(telemetry.track).toHaveBeenCalledWith(TELEMETRY_EVENT.CONTEXT.USER_UPDATED_SKILL, { + user_id: 'user-1', + skill_id: 'skill_project', + scope: 'project', + project_id: 'project-1', + source: 'mcp', + version: 4, + scope_changed: true, + }); + }); + + it('reports the place the caller passed without reading the skill back', async () => { + await service.updated( + 'user-1', + { id: 'skill_project', projectId: 'project-9', agentId: null }, + 'ui', + 2, + true, + ); + + expect(repository.findByIds).not.toHaveBeenCalled(); + expect(telemetry.track).toHaveBeenCalledWith( + TELEMETRY_EVENT.CONTEXT.USER_UPDATED_SKILL, + expect.objectContaining({ scope: 'project', project_id: 'project-9' }), + ); + }); + + it('reports a delete from the skill read before the delete', async () => { + repository.findByIds.mockResolvedValue([]); + + await service.deleted('user-1', skills[2]); + + expect(telemetry.track).toHaveBeenCalledWith(TELEMETRY_EVENT.CONTEXT.USER_DELETED_SKILL, { + user_id: 'user-1', + skill_id: 'skill_agent', + scope: 'agent', + project_id: 'project-2', + }); + }); + + it('never fails the write it reports', async () => { + repository.findByIds.mockRejectedValue(new Error('connection lost')); + + await expect(service.created('user-1', ['skill_project'], 'ui', 'manual')).resolves.toBe( + undefined, + ); + expect(telemetry.track).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/modules/agents/agent-skills.service.ts b/packages/cli/src/modules/agents/agent-skills.service.ts index b27df0170a3..5204b38da4c 100644 --- a/packages/cli/src/modules/agents/agent-skills.service.ts +++ b/packages/cli/src/modules/agents/agent-skills.service.ts @@ -4,6 +4,7 @@ import { type AgentSkillAvailability, type AgentSkill, type AgentSkillMutationResponse, + type SkillCreateMethod, } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import { TransactionRunner, type User } from '@n8n/db'; @@ -29,6 +30,11 @@ import { AgentRepository } from './repositories/agent.repository'; import { findIntroducedNameClashes, findNameClashGroups } from './skills/skill-names'; import type { SkillTarget } from './repositories/skill.repository'; import { SkillService, withTrimmedName } from './skills/skill.service'; +import { + SkillsTelemetry, + type SkillTelemetryMethod, + type SkillTelemetrySource, +} from './skills/skills-telemetry'; import { getAgentConfigHash, getAgentSkillHash } from './utils/agent-config-hash'; import { markAgentDraftDirty, saveAgentDraftFenced } from './utils/agent-draft.utils'; import { getAgentOrThrow } from './utils/get-agent-or-throw'; @@ -52,6 +58,7 @@ export class AgentSkillsService { private readonly agentSkillRefs: AgentSkillRefsService, private readonly skillService: SkillService, private readonly transactionRunner: TransactionRunner, + private readonly skillsTelemetry: SkillsTelemetry, ) {} /** The skills of the agent draft, keyed by skill id: the versions that the draft runs. */ @@ -105,6 +112,7 @@ export class AgentSkillsService { skill: AgentSkill, context: AgentMutationTelemetryContext, availableTo: AgentSkillAvailability = 'project', + method?: SkillCreateMethod, ): Promise { const { results } = await this.createSkillsBatch( agentId, @@ -112,6 +120,7 @@ export class AgentSkillsService { [skill], context, availableTo, + method, ); return results[0]; } @@ -122,6 +131,7 @@ export class AgentSkillsService { skills: AgentSkill[], context: AgentMutationTelemetryContext, availableTo: AgentSkillAvailability = 'project', + method?: SkillCreateMethod, ): Promise<{ results: AgentSkillMutationResponse[]; saved: Agent; skillRefs: AgentSkillRefs }> { if (skills.length === 0) { throw new UserError('At least one skill is required.'); @@ -178,6 +188,15 @@ export class AgentSkillsService { ); this.logger.debug('Created and attached agent skills', { agentId, projectId, skillIds: ids }); + // With the hub off, the skills stay on the agent row and the hub has nothing to report. + if (this.agentSkillRefs.usesSkillsHub) { + await this.skillsTelemetry.created( + context.user.id, + ids, + telemetrySourceOf(context), + telemetryMethodOf(context, method), + ); + } return { results: ids.map((id, index) => ({ @@ -332,6 +351,16 @@ export class AgentSkillsService { context.pushRef, ); + if (saved.created) { + await this.skillsTelemetry.updated( + context.user.id, + skillId, + telemetrySourceOf(context), + saved.version, + nextAvailability !== undefined, + ); + } + this.logger.debug('Updated agent skill', { agentId, projectId, @@ -485,3 +514,16 @@ function toStoredSkill(skill: AgentSkill): AgentSkill { ...(skill.references?.length ? { references: skill.references } : {}), }; } + +/** A person in the agent builder and n8n Assistant building the agent both report as `agent`. */ +function telemetrySourceOf(context: AgentMutationTelemetryContext): SkillTelemetrySource { + return context.modifiedBy === 'mcp' ? 'mcp' : 'agent'; +} + +/** Only a person in the skill modal types or uploads. n8n Assistant and MCP clients write with AI. */ +function telemetryMethodOf( + context: AgentMutationTelemetryContext, + method: SkillCreateMethod | undefined, +): SkillTelemetryMethod { + return context.modifiedBy === 'user' ? (method ?? 'manual') : 'ai'; +} diff --git a/packages/cli/src/modules/agents/agents-skills.controller.ts b/packages/cli/src/modules/agents/agents-skills.controller.ts index 0f257dcc80a..ddce12f1cba 100644 --- a/packages/cli/src/modules/agents/agents-skills.controller.ts +++ b/packages/cli/src/modules/agents/agents-skills.controller.ts @@ -58,7 +58,7 @@ export class AgentsSkillsController { agentId, 'create skill for', ); - const { availableTo, ...skill } = payload; + const { availableTo, method, ...skill } = payload; return await this.agentSkillsService.createAndAttachSkill( agentId, projectId, @@ -69,6 +69,7 @@ export class AgentsSkillsController { pushRef: req.headers?.['push-ref'], }, availableTo, + method, ); } diff --git a/packages/cli/src/modules/agents/skills/skills-api.service.ts b/packages/cli/src/modules/agents/skills/skills-api.service.ts index 6522ab12fc5..3aa3047ae67 100644 --- a/packages/cli/src/modules/agents/skills/skills-api.service.ts +++ b/packages/cli/src/modules/agents/skills/skills-api.service.ts @@ -23,6 +23,7 @@ import type { Skill } from '../entities/skill.entity'; import { SkillRepository, type SkillTarget } from '../repositories/skill.repository'; import { getAgentSkillHash } from '../utils/agent-config-hash'; import { SkillService, toAgentSkill } from './skill.service'; +import { SkillsTelemetry } from './skills-telemetry'; function scopeOf(skill: Pick): SkillScope { if (skill.agentId) return 'agent'; @@ -43,6 +44,7 @@ export class SkillsApiService { private readonly projectScopeService: ProjectScopeService, private readonly txRunner: TransactionRunner, private readonly userRepository: UserRepository, + private readonly skillsTelemetry: SkillsTelemetry, ) {} async list(user: User, query: ListSkillsQueryDto): Promise { @@ -84,6 +86,12 @@ export class SkillsApiService { source: 'ui', createdById: user.id, }); + await this.skillsTelemetry.created( + user.id, + [{ id, ...target }], + 'ui', + payload.method ?? 'manual', + ); // No second access check: a custom role can create skills it cannot read later. const [skill] = await this.skillRepository.findByIds([id]); if (!skill) throw new NotFoundError('Skill not found'); @@ -107,6 +115,10 @@ export class SkillsApiService { update, user.id, ); + if (saved.created) { + // No move, so the skill stays where `requireAccess` read it. + await this.skillsTelemetry.updated(user.id, skill, 'ui', saved.version, false); + } return { id: skillId, ...saved, skillHash: getAgentSkillHash(saved.skill) }; } @@ -122,14 +134,24 @@ export class SkillsApiService { }); // `save` leaves the refresh to the caller when it joins a transaction. await this.skillService.refreshFollowingAgents(followingAgentIds); + if (saved.created) { + await this.skillsTelemetry.updated( + user.id, + { id: skillId, ...target }, + 'ui', + saved.version, + true, + ); + } return { id: skillId, ...saved, skillHash: getAgentSkillHash(saved.skill) }; } async delete(user: User, skillId: string): Promise { - await this.requireAccess(user, skillId, 'delete'); + const skill = await this.requireAccess(user, skillId, 'delete'); // The conflict message names only the agents the user may read, as the detail does. const agentProjectIds = await this.projectScopeService.getProjectIds(user, ['agent:read']); await this.skillService.deleteSkill(skillId, agentProjectIds ?? 'all'); + await this.skillsTelemetry.deleted(user.id, skill); } private async toDetail(user: User, skill: Skill): Promise { diff --git a/packages/cli/src/modules/agents/skills/skills-telemetry.ts b/packages/cli/src/modules/agents/skills/skills-telemetry.ts new file mode 100644 index 00000000000..439551dfa00 --- /dev/null +++ b/packages/cli/src/modules/agents/skills/skills-telemetry.ts @@ -0,0 +1,127 @@ +import type { SkillScope } from '@n8n/api-types'; +import { Logger } from '@n8n/backend-common'; +import { Service } from '@n8n/di'; +import { TELEMETRY_EVENT } from '@n8n/telemetry'; +import { ensureError } from '@n8n/utils/errors/ensure-error'; + +import { Telemetry } from '@/telemetry'; + +import type { Skill } from '../entities/skill.entity'; +import { SkillRepository } from '../repositories/skill.repository'; + +/** The surface that wrote the skill. Not `skill.source`, which has no MCP value. */ +export type SkillTelemetrySource = 'ui' | 'agent' | 'mcp'; + +/** `manual` and `upload` come from the skill modal. `ai` is text an assistant wrote. */ +export type SkillTelemetryMethod = 'manual' | 'upload' | 'ai'; + +type SkillPlace = { scope: SkillScope; project_id?: string }; + +type SkillTelemetryTarget = Pick; + +/** + * A skill with the place the write put it. A bare id is read back after the write, and a + * concurrent move can land before that read, so pass the place when the caller has it. + */ +export type SkillTelemetryRef = SkillTelemetryTarget | string; + +/** + * Skills hub telemetry. Callers emit only once the write commits and only with the hub + * on. An emit never fails the write: an error is logged and dropped. + */ +@Service() +export class SkillsTelemetry { + constructor( + private readonly telemetry: Telemetry, + private readonly skillRepository: SkillRepository, + private readonly logger: Logger, + ) {} + + async created( + userId: string, + refs: SkillTelemetryRef[], + source: SkillTelemetrySource, + method: SkillTelemetryMethod, + ): Promise { + await this.safely(async () => { + const skills = await this.resolve(refs); + const placeOf = await this.placeResolver(skills); + for (const skill of skills) { + this.telemetry.track(TELEMETRY_EVENT.CONTEXT.USER_CREATED_SKILL, { + user_id: userId, + skill_id: skill.id, + ...placeOf(skill), + source, + method, + }); + } + }); + } + + /** Call only for a save that stored a new version. */ + async updated( + userId: string, + ref: SkillTelemetryRef, + source: SkillTelemetrySource, + version: number, + scopeChanged: boolean, + ): Promise { + await this.safely(async () => { + const [skill] = await this.resolve([ref]); + if (!skill) return; + const placeOf = await this.placeResolver([skill]); + this.telemetry.track(TELEMETRY_EVENT.CONTEXT.USER_UPDATED_SKILL, { + user_id: userId, + skill_id: skill.id, + ...placeOf(skill), + source, + version, + scope_changed: scopeChanged, + }); + }); + } + + /** Takes the skill as read before the delete, since the row is gone after it. */ + async deleted(userId: string, skill: SkillTelemetryTarget): Promise { + await this.safely(async () => { + const placeOf = await this.placeResolver([skill]); + this.telemetry.track(TELEMETRY_EVENT.CONTEXT.USER_DELETED_SKILL, { + user_id: userId, + skill_id: skill.id, + ...placeOf(skill), + }); + }); + } + + /** Reads back only the refs that came as a bare id, in one query. */ + private async resolve(refs: SkillTelemetryRef[]): Promise { + const ids = refs.filter((ref) => typeof ref === 'string'); + const read = ids.length > 0 ? await this.skillRepository.findByIds(ids) : []; + return [...refs.filter((ref) => typeof ref !== 'string'), ...read]; + } + + /** An agent skill has no projectId of its own, so its project comes from its agent. */ + private async placeResolver( + skills: SkillTelemetryTarget[], + ): Promise<(skill: SkillTelemetryTarget) => SkillPlace> { + const agents = await this.skillRepository.findAgents( + skills.flatMap((skill) => (skill.agentId ? [skill.agentId] : [])), + ); + return (skill) => { + if (skill.agentId) { + const projectId = agents.get(skill.agentId)?.projectId; + return { scope: 'agent', ...(projectId ? { project_id: projectId } : {}) }; + } + if (skill.projectId) return { scope: 'project', project_id: skill.projectId }; + return { scope: 'instance' }; + }; + } + + private async safely(emit: () => Promise): Promise { + try { + await emit(); + } catch (error) { + this.logger.warn('Could not send skills telemetry', { error: ensureError(error).message }); + } + } +} diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSkillModal.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSkillModal.test.ts index ef83ac57850..bf4c1255619 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSkillModal.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSkillModal.test.ts @@ -414,6 +414,7 @@ describe('AgentSkillModal', () => { description: 'Use for research', instructions: 'Main instructions', }, + method: 'upload', }); expect(uiStore.closeModal).toHaveBeenCalledWith(MODAL_NAME); }); @@ -450,6 +451,7 @@ describe('AgentSkillModal', () => { instructions: 'Read the guide', references: [{ path: 'references/guide.md', content: '# Guide' }], }, + method: 'manual', }); expect(uiStore.closeModal).toHaveBeenCalledWith(MODAL_NAME); }); @@ -1223,6 +1225,7 @@ describe('AgentSkillModal from Settings', () => { id: undefined, skill: { name: 'Research', description: 'Use it', instructions: 'Body' }, scope: 'project:personal-1', + method: 'upload', }); // Settings has no agent, so the builder's import event is not sent. expect(trackImportedSkill).not.toHaveBeenCalled(); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentApi.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentApi.test.ts index cbd2d40c369..7216668ad1c 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentApi.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentApi.test.ts @@ -91,6 +91,17 @@ describe('useAgentApi', () => { ); }); + it('sends how the user made the skill with the create request', async () => { + await createAgentSkill(restApiContext, 'project-1', 'agent-1', skill, undefined, 'upload'); + + expect(makeRestApiRequest).toHaveBeenCalledWith( + restApiContext, + 'POST', + '/projects/project-1/agents/v2/agent-1/skills', + { ...skill, method: 'upload' }, + ); + }); + it('saves the skill text and its target in one request', async () => { await updateAgentSkill( restApiContext, diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentCapabilitiesActions.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentCapabilitiesActions.test.ts index debd6732cfa..ff2f956dd57 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentCapabilitiesActions.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentCapabilitiesActions.test.ts @@ -184,6 +184,7 @@ type SkillModalData = { id?: string; skill: AgentSkill; availableTo?: 'project' | 'agent' | 'instance'; + method?: 'manual' | 'upload'; }) => unknown; onAttach?: (skill: SkillListItem) => Promise; onRemove?: (skillId: string) => void; @@ -803,6 +804,7 @@ describe('useAgentCapabilitiesActions — skills hub', () => { 'agent-1', sqlHelper, undefined, + undefined, ); expect(getHubSkill).not.toHaveBeenCalled(); @@ -901,7 +903,7 @@ describe('useAgentCapabilitiesActions — skills hub', () => { }); const { modalData } = setupHub(); - modalData.onConfirm({ skill: sqlHelper, availableTo }); + modalData.onConfirm({ skill: sqlHelper, availableTo, method: 'upload' }); await flushPromises(); expect(createAgentSkill).toHaveBeenCalledWith( @@ -910,6 +912,7 @@ describe('useAgentCapabilitiesActions — skills hub', () => { 'agent-1', sqlHelper, availableTo, + 'upload', ); }, ); diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentSkillModal.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentSkillModal.vue index 7391363d15c..e243d5842a7 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentSkillModal.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentSkillModal.vue @@ -8,6 +8,7 @@ import { AGENT_SKILL_REFERENCE_MAX_COUNT, skillNameKey, type AgentSkillAvailability, + type SkillCreateMethod, type SkillDetail, type SkillListItem, } from '@n8n/api-types'; @@ -94,6 +95,8 @@ export type AgentSkillModalData = { skill: AgentSkill; availableTo?: AgentSkillAvailability; scope?: string; + /** Set on create only: whether the user typed the skill or uploaded it. */ + method?: SkillCreateMethod; }) => AgentSkillConfirmResult | Promise; outdated?: boolean; onUseLatestVersion?: (id: string) => void; @@ -142,6 +145,8 @@ const submitted = ref(false); const formIsValid = ref(false); const selectedPath = ref(SKILL_FILE); const isEditing = computed(() => !!props.data.skillId); +// An upload lands on the edit step like a saved skill does, so the step alone cannot tell. +const createMethod = ref('manual'); const hasPicker = !props.data.skillId && !!props.data.hubSkillsEnabled && !!props.data.onAttach; const step = ref<'select' | 'preview' | 'upload' | 'manual' | 'edit'>( props.data.skill || props.data.skillId ? 'edit' : hasPicker ? 'select' : 'upload', @@ -181,6 +186,7 @@ async function refreshDefaultName() { } function onAddManually() { + createMethod.value = 'manual'; step.value = 'manual'; void refreshDefaultName(); } @@ -501,6 +507,7 @@ onMounted(() => { }); function onSkillUploaded(importedSkill: AgentSkill) { + createMethod.value = 'upload'; skill.value = normalizeSkill(importedSkill); serverNameError.value = ''; selectedPath.value = SKILL_FILE; @@ -612,6 +619,7 @@ async function onSave() { skill: payload, ...(showAvailabilitySelect.value && targetChanged ? { availableTo: availableTo.value } : {}), ...(canChangeScope.value ? { scope: scope.value } : {}), + ...(isEditing.value ? {} : { method: createMethod.value }), }); } catch { // The caller reports its own errors. Keep the modal open with the user's edits. diff --git a/packages/frontend/editor-ui/src/features/agents/composables/useAgentApi.ts b/packages/frontend/editor-ui/src/features/agents/composables/useAgentApi.ts index 577595a57bf..2603969d800 100644 --- a/packages/frontend/editor-ui/src/features/agents/composables/useAgentApi.ts +++ b/packages/frontend/editor-ui/src/features/agents/composables/useAgentApi.ts @@ -37,6 +37,7 @@ import type { SkillDetail, SkillListItem, SkillListResponse, + SkillCreateMethod, SkillNewNameQueryDto, SkillNewNameResponse, VectorStoreTestResult, @@ -710,12 +711,13 @@ export const createAgentSkill = async ( agentId: string, skill: AgentSkill, availableTo?: AgentSkillAvailability, + method?: SkillCreateMethod, ): Promise => { return await makeRestApiRequest( context, 'POST', `/projects/${projectId}/agents/v2/${agentId}/skills`, - availableTo ? { ...skill, availableTo } : skill, + { ...skill, ...(availableTo ? { availableTo } : {}), ...(method ? { method } : {}) }, ); }; diff --git a/packages/frontend/editor-ui/src/features/agents/composables/useAgentCapabilitiesActions.ts b/packages/frontend/editor-ui/src/features/agents/composables/useAgentCapabilitiesActions.ts index 6eb61b2e61e..3aa5e46b474 100644 --- a/packages/frontend/editor-ui/src/features/agents/composables/useAgentCapabilitiesActions.ts +++ b/packages/frontend/editor-ui/src/features/agents/composables/useAgentCapabilitiesActions.ts @@ -8,6 +8,7 @@ import { skillNameKey, type AgentConfigValidationIssue, type AgentSkillAvailability, + type SkillCreateMethod, type SkillDetail, type SkillListItem, type SkillNewNameQueryDto, @@ -522,10 +523,12 @@ export function useAgentCapabilitiesActions(deps: UseAgentCapabilitiesActionsDep onConfirm: async ({ skill, availableTo, + method, }: { id?: string; skill: AgentSkill; availableTo?: AgentSkillAvailability; + method?: SkillCreateMethod; }): Promise => { if (localSkills) { if (!isCurrentTarget()) return undefined; @@ -559,6 +562,7 @@ export function useAgentCapabilitiesActions(deps: UseAgentCapabilitiesActionsDep targetAgentId, sanitizedSkill, hubSkillsEnabled.value ? availableTo : undefined, + method, ); } catch (error) { if (isSkillNameTakenError(error)) { diff --git a/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.test.ts b/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.test.ts index 3180690a9d3..fbc62d68f5d 100644 --- a/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.test.ts +++ b/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.test.ts @@ -3,6 +3,7 @@ import { type MockedStore, mockedStore } from '@/__tests__/utils'; import { createTestingPinia } from '@pinia/testing'; import { SKILL_NAME_TAKEN_MESSAGE, type SkillDetail, type SkillListItem } from '@n8n/api-types'; import { ResponseError } from '@n8n/rest-api-client'; +import { TELEMETRY_EVENT } from '@n8n/telemetry'; import type { IUser } from '@n8n/rest-api-client/api/users'; import { useUsersStore } from '@n8n/stores/users.store'; import userEvent from '@testing-library/user-event'; @@ -22,6 +23,11 @@ const routeQuery: Record = {}; const confirmMock = vi.fn().mockResolvedValue('confirm'); const showMessageMock = vi.fn(); const showErrorMock = vi.fn(); +const trackMock = vi.fn(); + +vi.mock('@n8n/composables/useTelemetry', () => ({ + useTelemetry: () => ({ track: trackMock }), +})); vi.mock('@/app/composables/useMessage', () => ({ useMessage: () => ({ confirm: confirmMock, alert: vi.fn(), prompt: vi.fn() }), @@ -123,6 +129,7 @@ describe('SettingsSkillsView', () => { confirmMock.mockResolvedValue('confirm'); showMessageMock.mockClear(); showErrorMock.mockClear(); + trackMock.mockClear(); }); it('loads the first page on mount', async () => { @@ -138,6 +145,30 @@ describe('SettingsSkillsView', () => { expect(await findByTestId('skills-empty-state')).toHaveTextContent('No skills yet'); }); + it('reports the visit once, with the total count', async () => { + skillsStore.fetchSkills.mockImplementation(async () => { + skillsStore.count = 72; + return { count: 72, data: [] }; + }); + + renderView(); + await new Promise(process.nextTick); + + expect(trackMock).toHaveBeenCalledTimes(1); + expect(trackMock).toHaveBeenCalledWith(TELEMETRY_EVENT.CONTEXT.USER_VIEWED_SKILLS, { + count: 72, + }); + }); + + it('reports no visit when the load fails', async () => { + skillsStore.fetchSkills.mockRejectedValueOnce(new Error('offline')); + + renderView(); + await new Promise(process.nextTick); + + expect(trackMock).not.toHaveBeenCalled(); + }); + it('does not show the empty state when the load fails', async () => { skillsStore.fetchSkills.mockRejectedValueOnce(new Error('offline')); @@ -249,13 +280,18 @@ describe('SettingsSkillsView', () => { description: 'Write copy.', instructions: 'Be brief.', }; - lastModalData().onConfirm({ skill: content, scope: 'project:personal-1' }); + lastModalData().onConfirm({ + skill: content, + scope: 'project:personal-1', + method: 'upload', + }); await new Promise(process.nextTick); expect(skillsStore.createSkill).toHaveBeenCalledWith({ scope: 'project', projectId: 'personal-1', skill: content, + method: 'upload', }); // The new row is the feedback, as on the preferences page. expect(showMessageMock).not.toHaveBeenCalled(); @@ -309,6 +345,11 @@ describe('SettingsSkillsView', () => { await new Promise(process.nextTick); expect(lastModalData().readOnly).toBe(true); + expect(trackMock).toHaveBeenCalledWith(TELEMETRY_EVENT.CONTEXT.USER_OPENED_SKILL, { + skill_id: 's1', + scope: 'instance', + read_only: true, + }); }); it('locks the scope while agents use the skill', async () => { diff --git a/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.vue b/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.vue index 73a53fbc232..d3a78f5b8bb 100644 --- a/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.vue +++ b/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.vue @@ -2,10 +2,12 @@ import { computed, onMounted, ref } from 'vue'; import { useRoute, useRouter } from 'vue-router'; import { useDebounceFn } from '@vueuse/core'; -import type { AgentSkill, SkillListItem, SkillScope } from '@n8n/api-types'; +import type { AgentSkill, SkillCreateMethod, SkillListItem, SkillScope } from '@n8n/api-types'; +import { useTelemetry } from '@n8n/composables/useTelemetry'; import { useI18n } from '@n8n/i18n'; import { useToast } from '@n8n/composables/useToast'; import { getDebounceTime } from '@n8n/composables/useDebounce'; +import { TELEMETRY_EVENT } from '@n8n/telemetry'; import { N8nButton, N8nDropdownMenu, @@ -53,6 +55,7 @@ const message = useMessage(); const uiStore = useUIStore(); const projectsStore = useProjectsStore(); const skillsStore = useSkillsStore(); +const telemetry = useTelemetry(); const { showError, showMessage } = useToast(); const { scopeDisplay } = useSkillScopeDisplay(); @@ -191,7 +194,8 @@ function openCreateModal() { ...parseScope(scopeValue), name: i18n.baseText('agents.builder.skills.defaultName'), }), - onConfirm: async ({ skill, scope }) => await onCreate(skill, scope ?? options[0].value), + onConfirm: async ({ skill, scope, method }) => + await onCreate(skill, scope ?? options[0].value, method), }); } @@ -226,10 +230,17 @@ function saveFailed(error: unknown): AgentSkillConfirmFailure { async function onCreate( skill: AgentSkill, scopeValue: string, + method?: SkillCreateMethod, ): Promise { let createdId: string; try { - createdId = (await skillsStore.createSkill({ ...parseScope(scopeValue), skill })).id; + createdId = ( + await skillsStore.createSkill({ + ...parseScope(scopeValue), + skill, + ...(method ? { method } : {}), + }) + ).id; } catch (error) { return saveFailed(error); } @@ -270,6 +281,12 @@ function scopeLock( async function openEditModal(item: Pick) { try { const detail = await skillsStore.fetchSkill(item.id); + telemetry.track(TELEMETRY_EVENT.CONTEXT.USER_OPENED_SKILL, { + skill_id: detail.id, + scope: detail.scope, + ...(detail.projectId ? { project_id: detail.projectId } : {}), + read_only: !detail.canEdit, + }); const currentValue = scopeValueOf(detail); // The current scope is listed even when the user could not create a skill there, // such as an agent skill, which is only made from inside its agent. @@ -361,6 +378,11 @@ async function goBack() { onMounted(async () => { documentTitle.set(i18n.baseText('settings.context.skills.title')); await Promise.all([load(), projectsStore.getMyProjects().catch(() => undefined)]); + // Once for the visit, not once for each page. A failed load reports nothing, so a zero + // here is an empty list and never a broken one. + if (!loadFailed.value) { + telemetry.track(TELEMETRY_EVENT.CONTEXT.USER_VIEWED_SKILLS, { count: skillsStore.count }); + } // Deep links: one skill to open, or a new one to start. The skill may be on another // page, so it is read by id rather than from the list. const { skillId, create } = route.query;