fix: address skill telemetry review

Co-authored-by: Paperclip <noreply@paperclip.ing>

server/src/__tests__/company-skills-routes.test.ts

@@ -73,7 +73,11 @@ const mockCatalogService = vi.hoisted(() => ({
 }));
 
 const mockLogActivity = vi.hoisted(() => vi.fn());
+const mockTrackSkillCreated = vi.hoisted(() => vi.fn());
+const mockTrackSkillForked = vi.hoisted(() => vi.fn());
 const mockTrackSkillImported = vi.hoisted(() => vi.fn());
+const mockTrackSkillTestRun = vi.hoisted(() => vi.fn());
+const mockTrackSkillVersionSaved = vi.hoisted(() => vi.fn());
 const mockGetTelemetryClient = vi.hoisted(() => vi.fn());
 const mockReflectionCoachMutationGate = vi.hoisted(() => ({
   assertConsented: vi.fn(),
@@ -107,7 +111,11 @@ function registerModuleMocks() {
   vi.doMock("../routes/authz.js", async () => vi.importActual("../routes/authz.js"));
 
   vi.doMock("@paperclipai/shared/telemetry", () => ({
+    trackSkillCreated: mockTrackSkillCreated,
+    trackSkillForked: mockTrackSkillForked,
     trackSkillImported: mockTrackSkillImported,
+    trackSkillTestRun: mockTrackSkillTestRun,
+    trackSkillVersionSaved: mockTrackSkillVersionSaved,
     trackErrorHandlerCrash: vi.fn(),
   }));
 
@@ -694,6 +702,13 @@ describe("company skill mutation permissions", () => {
     }));
     expect(mockAccessService.canUser).not.toHaveBeenCalledWith("company-1", "board-user", "agents:create");
     expect(mockCompanySkillService.createLocalSkill).toHaveBeenCalled();
+    expect(mockTrackSkillCreated).toHaveBeenCalledWith(expect.anything(), {
+      skill_id: "skill-1",
+      creation_source: "blank",
+      sharing_scope: "company",
+      category_count: 0,
+      file_count: 1,
+    });
     expect(mockCompanySkillService.importFromSource).toHaveBeenCalled();
     expect(mockCompanySkillService.installFromCatalog).toHaveBeenCalled();
     expect(mockCompanySkillService.updateSkill).toHaveBeenCalled();
@@ -1080,6 +1095,33 @@ describe("company skill mutation permissions", () => {
       entityType: "company_skill_version",
       entityId: "version-1",
     }));
+    expect(mockTrackSkillVersionSaved).toHaveBeenCalledWith(expect.anything(), {
+      skill_id: "skill-1",
+      revision_number: 1,
+      file_type: "skill",
+    });
+  });
+
+  it("tracks Skill Studio file saves with the current version and file kind", async () => {
+    mockCompanySkillService.detail.mockResolvedValueOnce({
+      currentVersion: { revisionNumber: 7 },
+    });
+    const app = await createApp({ type: "board", source: "local_implicit", userId: "user-1" });
+
+    await request(app)
+      .patch("/api/companies/company-1/skills/skill-1/files")
+      .send({ path: "SKILL.md", content: "# Updated" })
+      .expect(200);
+
+    expect(mockCompanySkillService.updateFile).toHaveBeenCalledWith("company-1", "skill-1", "SKILL.md", "# Updated", {
+      type: "user",
+      userId: "user-1",
+    });
+    expect(mockTrackSkillVersionSaved).toHaveBeenCalledWith(expect.anything(), {
+      skill_id: "skill-1",
+      revision_number: 7,
+      file_type: "skill",
+    });
   });
 
   it("deletes skill files and logs the mutation", async () => {
@@ -1158,6 +1200,13 @@ describe("company skill mutation permissions", () => {
       action: "company.skill_forked",
       entityId: "skill-fork",
     }));
+    expect(mockTrackSkillForked).toHaveBeenCalledWith(expect.anything(), {
+      skill_id: "skill-fork",
+      fork_from_skill_id: "skill-1",
+      source_type: "github",
+      sharing_scope: "company",
+      reassign_agent_count: 0,
+    });
     expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
       action: "company.skill_comment_created",
       entityId: "comment-1",
@@ -1556,6 +1605,13 @@ describe("company skill mutation permissions", () => {
       reason: "skill_test_run_created",
       payload: expect.objectContaining({ issueId: "44444444-4444-4444-8444-444444444444", skillId: "skill-1" }),
     }));
+    expect(mockTrackSkillTestRun).toHaveBeenCalledWith(expect.anything(), {
+      skill_id: "skill-1",
+      status: "queued",
+      run_source: "run",
+      ad_hoc: false,
+      template_used: true,
+    });
 
     const cancelled = await request(app)
       .post("/api/companies/company-1/skills/skill-1/test-runs/22222222-2222-4222-8222-222222222222/cancel")

server/src/__tests__/shared-telemetry-events.test.ts

@@ -212,11 +212,11 @@ describe("shared Skill telemetry proposals", () => {
   it("emits primitive skill.created dimensions and omits user-authored identity", () => {
     const client = createClient();
     const dims = {
-      skillId: "11111111-1111-4111-8111-111111111111",
-      creationSource: "blank",
-      sharingScope: "company",
-      categoryCount: 2,
-      fileCount: 3,
+      skill_id: "11111111-1111-4111-8111-111111111111",
+      creation_source: "blank",
+      sharing_scope: "company",
+      category_count: 2,
+      file_count: 3,
       name: "Secret Skill",
       slug: "secret-skill",
       tagline: "keep this private",
@@ -242,9 +242,9 @@ describe("shared Skill telemetry proposals", () => {
   it("emits primitive skill.version_saved dimensions without file path or content", () => {
     const client = createClient();
     const dims = {
-      skillId: "22222222-2222-4222-8222-222222222222",
-      revisionNumber: 4,
-      fileType: "markdown",
+      skill_id: "22222222-2222-4222-8222-222222222222",
+      revision_number: 4,
+      file_type: "markdown",
       file_path: "customers/acme/SKILL.md",
       file_content: "keep this private",
       path: "customers/acme/SKILL.md",
@@ -269,11 +269,11 @@ describe("shared Skill telemetry proposals", () => {
   it("emits primitive skill.test_run dimensions without snapshots, prompts, or errors", () => {
     const client = createClient();
     const dims = {
-      skillId: "33333333-3333-4333-8333-333333333333",
+      skill_id: "33333333-3333-4333-8333-333333333333",
       status: "queued",
-      runSource: "run",
-      adHoc: true,
-      templateUsed: false,
+      run_source: "run",
+      ad_hoc: true,
+      template_used: false,
       inputSnapshot: "do not leak input",
       outputSnapshot: "do not leak output",
       harnessIssueDescription: "private harness issue",
@@ -302,11 +302,11 @@ describe("shared Skill telemetry proposals", () => {
   it("emits primitive skill.forked dimensions and passes opaque IDs through raw", () => {
     const client = createClient();
     const dims = {
-      skillId: "44444444-4444-4444-8444-444444444444",
-      forkFromSkillId: "55555555-5555-4555-8555-555555555555",
-      sourceType: "github",
-      sharingScope: "private",
-      reassignAgentCount: 1,
+      skill_id: "44444444-4444-4444-8444-444444444444",
+      fork_from_skill_id: "55555555-5555-4555-8555-555555555555",
+      source_type: "github",
+      sharing_scope: "private",
+      reassign_agent_count: 1,
       name: "Secret Skill",
       slug: "secret-skill",
       fork_from_name: "Secret Source",
@@ -332,7 +332,7 @@ describe("shared Skill telemetry proposals", () => {
   it("emits only sharing_scope for the deliberately rejectable share-link proposal", () => {
     const client = createClient();
     const dims = {
-      sharingScope: "public_link",
+      sharing_scope: "public_link",
       skill_id: "66666666-6666-4666-8666-666666666666",
       href: "https://paperclip.example/skills/secret-skill?token=secret-token",
       url: "https://paperclip.example/skills/secret-skill?token=secret-token",

server/src/routes/company-skills.ts

@@ -23,7 +23,17 @@ import {
   companySkillUpdateSchema,
   companySkillVersionCreateSchema,
 } from "@paperclipai/shared";
-import { trackSkillImported } from "@paperclipai/shared/telemetry";
+import type {
+  CompanySkillFileInventoryEntry,
+  CompanySkillSharingScope,
+} from "@paperclipai/shared";
+import {
+  trackSkillCreated,
+  trackSkillForked,
+  trackSkillImported,
+  trackSkillTestRun,
+  trackSkillVersionSaved,
+} from "@paperclipai/shared/telemetry";
 import { validate } from "../middleware/validate.js";
 import { accessService, agentService, companySkillService, heartbeatService, issueService, logActivity } from "../services/index.js";
 import {
@@ -79,6 +89,25 @@ export function companySkillRoutes(db: Db) {
     return skill.key;
   }
 
+  function emitSkillTelemetry(run: (client: NonNullable<ReturnType<typeof getTelemetryClient>>) => void) {
+    const telemetryClient = getTelemetryClient();
+    if (telemetryClient) run(telemetryClient);
+  }
+
+  function deriveSkillCreationSource(skill: { forkedFromSkillId?: string | null; metadata?: Record<string, unknown> | null }) {
+    if (skill.forkedFromSkillId) return "fork";
+    return asString(skill.metadata?.sourceKind) === "project_scan" ? "project_scan" : "blank";
+  }
+
+  function normalizeTelemetrySharingScope(value: CompanySkillSharingScope | string | null | undefined): CompanySkillSharingScope {
+    return value === "private" || value === "public_link" ? value : "company";
+  }
+
+  function deriveVersionFileType(fileInventory: Array<Pick<CompanySkillFileInventoryEntry, "kind">>) {
+    const kinds = new Set(fileInventory.map((entry) => entry.kind));
+    return kinds.size === 1 ? [...kinds][0]! : "other";
+  }
+
   function firstQueryString(value: unknown): string | undefined {
     if (typeof value === "string") return value;
     if (Array.isArray(value) && typeof value[0] === "string") return value[0];
@@ -554,6 +583,13 @@ export function companySkillRoutes(db: Db) {
           issueId: result.issueId,
         },
       });
+      emitSkillTelemetry((client) => trackSkillTestRun(client, {
+        skill_id: result.skillId,
+        status: result.status,
+        run_source: req.body.skillVersionId ? "rerun" : "run",
+        ad_hoc: result.inputId === null,
+        template_used: Boolean(result.templateId),
+      }));
       res.status(201).json(result);
     },
   );
@@ -657,6 +693,11 @@ export function companySkillRoutes(db: Db) {
           label: result.label,
         },
       });
+      emitSkillTelemetry((client) => trackSkillVersionSaved(client, {
+        skill_id: skillId,
+        revision_number: result.revisionNumber,
+        file_type: deriveVersionFileType(result.fileInventory),
+      }));
       res.status(201).json(result);
     },
   );
@@ -729,6 +770,13 @@ export function companySkillRoutes(db: Db) {
           reassignedAgentIds: result.reassignments.map((entry: { agentId: string }) => entry.agentId),
         },
       });
+      emitSkillTelemetry((client) => trackSkillForked(client, {
+        skill_id: result.skill.id,
+        fork_from_skill_id: result.original.id,
+        source_type: result.original.sourceType,
+        sharing_scope: normalizeTelemetrySharingScope(result.skill.sharingScope),
+        reassign_agent_count: result.reassignments.length,
+      }));
       res.status(201).json(result);
     },
   );
@@ -860,6 +908,13 @@ export function companySkillRoutes(db: Db) {
           name: result.name,
         },
       });
+      emitSkillTelemetry((client) => trackSkillCreated(client, {
+        skill_id: result.id,
+        creation_source: deriveSkillCreationSource(result),
+        sharing_scope: normalizeTelemetrySharingScope(result.sharingScope),
+        category_count: result.categories.length,
+        file_count: result.fileInventory.length,
+      }));
 
       res.status(201).json(result);
     },
@@ -925,6 +980,15 @@ export function companySkillRoutes(db: Db) {
           markdown: result.markdown,
         },
       });
+      const updatedSkill = await svc.detail(companyId, skillId, skillActor(req));
+      const currentVersion = updatedSkill?.currentVersion ?? null;
+      if (currentVersion) {
+        emitSkillTelemetry((client) => trackSkillVersionSaved(client, {
+          skill_id: skillId,
+          revision_number: currentVersion.revisionNumber,
+          file_type: result.kind,
+        }));
+      }
 
       res.json(result);
     },