[STG-2280] feat(cli): emit skill_id telemetry property

.changeset/telemetry-skill-id-first-success.md

@@ -0,0 +1,7 @@
+---
+"browse": patch
+---
+
+Emit a `skill_id` property on `cli.command_completed` telemetry.
+
+The validated, catalog-public skill id (e.g. `yelp.com/extract-reviews`, or `bundled/browse` for `skills install`) is attached to the completion event for `browse skills add`/`install`, covering both successful installs and every downstream failure path (`skill_not_found`, `skill_install_failed`, ...). Only the parsed, regex-validated id is ever attached — never the raw argument.

packages/cli/src/lib/run-telemetry.ts

@@ -2,6 +2,7 @@ export interface RunTelemetryState {
   resultCode?: string;
   httpStatus?: number;
   requestHadHttpResponse?: boolean;
+  skillId?: string;
 }
 
 let currentRunTelemetry: RunTelemetryState = {};

packages/cli/src/lib/skills/install.ts

@@ -13,6 +13,7 @@ import { delimiter, dirname, join } from "node:path";
 import { fileURLToPath } from "node:url";
 
 import { fail } from "../errors.js";
+import { setRunTelemetryCompletion } from "../run-telemetry.js";
 import { defaultSkillsApiBaseUrl, isRecord, responseDetail } from "./shared.js";
 
 const defaultBlobBaseUrl =
@@ -105,7 +106,11 @@ export function isBlobSkillId(skillId: ParsedSkillId): boolean {
 }
 
 export async function installSkill(rawSkillId: string): Promise<void> {
-  const skillId = parseSkillId(rawSkillId);
+  const skill = parseSkillId(rawSkillId);
+  // Attach only the validated, catalog-public id to telemetry — never the raw
+  // argument, which can contain arbitrary user input. Setting it right after
+  // parsing covers success and every downstream failure path.
+  setRunTelemetryCompletion({ skillId: skill.id });
   const npxPath = await findExecutable("npx");
   if (!npxPath) {
     fail(
@@ -115,9 +120,9 @@ export async function installSkill(rawSkillId: string): Promise<void> {
     );
   }
 
-  const files = await fetchSkillFiles(skillId);
+  const files = await fetchSkillFiles(skill);
   if (files.status === "found") {
-    const result = await downloadBlobSkill(skillId, files.files);
+    const result = await downloadBlobSkill(skill, files.files);
     process.stdout.write(
       `Downloaded ${result.fileCount} skill file${result.fileCount === 1 ? "" : "s"} to ${result.installPath}\n`,
     );
@@ -132,7 +137,7 @@ export async function installSkill(rawSkillId: string): Promise<void> {
 
   if (files.status === "not_found") {
     fail(
-      `Skill "${skillId.id}" not found in the catalog. Run \`browse skills find ${skillId.domain}\` to discover available skills, or \`browse skills list\` to browse them.`,
+      `Skill "${skill.id}" not found in the catalog. Run \`browse skills find ${skill.domain}\` to discover available skills, or \`browse skills list\` to browse them.`,
       1,
       { resultCode: "skill_not_found" },
     );
@@ -146,11 +151,12 @@ export async function installSkill(rawSkillId: string): Promise<void> {
     "add",
     "browserbase/browse.sh",
     "--skill",
-    skillId.id,
+    skill.id,
   ]);
 }
 
 export async function installBundledCliSkill(): Promise<void> {
+  setRunTelemetryCompletion({ skillId: "bundled/browse" });
   const npxPath = await findExecutable("npx");
   if (!npxPath) {
     fail(

packages/cli/src/lib/telemetry.ts

@@ -255,6 +255,7 @@ function commandCompletedProperties(
       failureTelemetry?.requestHadHttpResponse ??
       runTelemetry.requestHadHttpResponse ??
       null,
+    skill_id: runTelemetry.skillId ?? null,
   };
 }
 

packages/cli/tests/cli-telemetry.test.ts

@@ -64,6 +64,7 @@ describe("CLI telemetry", () => {
       expect(completedPayload.properties.result_code).toBe("ok");
       expect(completedPayload.properties.http_status).toBe(null);
       expect(completedPayload.properties.request_had_http_response).toBe(null);
+      expect(completedPayload.properties.skill_id).toBe(null);
     } finally {
       await telemetryServer.close();
     }
@@ -220,6 +221,72 @@ describe("CLI telemetry", () => {
     }
   });
 
+  it("attaches the validated skill id to completion telemetry for skills add", async () => {
+    const telemetryServer = await startTelemetryServer();
+    const skillsApiServer = await startFakeBrowserbaseServer(
+      (_request, response) => {
+        jsonResponse(response, 404, { error: "not found" });
+      },
+    );
+
+    try {
+      const installIdFile = await tempInstallIdFile(
+        "browse-telemetry-skill-id-",
+      );
+      const result = await runCli(
+        ["skills", "add", "example.com/extract-reviews"],
+        {
+          env: {
+            ...telemetryEnv(telemetryServer, installIdFile),
+            BROWSE_SKILLS_API_BASE_URL: skillsApiServer.baseUrl,
+          },
+        },
+      );
+
+      expect(result.exitCode).toBe(1);
+      expect(result.stderr).toContain("not found in the catalog");
+
+      const payloads = telemetryPayloads(telemetryServer);
+      const completedPayload = findPayload(payloads, "cli.command_completed");
+      expect(completedPayload.properties).toMatchObject({
+        command_path: "skills.add",
+        skill_id: "example.com/extract-reviews",
+        result_code: "skill_not_found",
+        success: false,
+      });
+
+      const invokedPayload = findPayload(payloads, "cli.command_invoked");
+      expect(invokedPayload.properties).not.toHaveProperty("skill_id");
+    } finally {
+      await skillsApiServer.close();
+      await telemetryServer.close();
+    }
+  });
+
+  it("never attaches unvalidated skill arguments to telemetry", async () => {
+    const telemetryServer = await startTelemetryServer();
+
+    try {
+      const installIdFile = await tempInstallIdFile(
+        "browse-telemetry-skill-raw-",
+      );
+      const result = await runCli(
+        ["skills", "add", "../secret-local-path/oops"],
+        { env: telemetryEnv(telemetryServer, installIdFile) },
+      );
+
+      expect(result.exitCode).toBe(1);
+
+      const payloads = telemetryPayloads(telemetryServer);
+      const completedPayload = findPayload(payloads, "cli.command_completed");
+      expect(completedPayload.properties.skill_id).toBe(null);
+      expect(completedPayload.properties.result_code).toBe("invalid_skill_id");
+      expect(JSON.stringify(payloads)).not.toContain("secret-local-path");
+    } finally {
+      await telemetryServer.close();
+    }
+  });
+
   it("does not emit telemetry for help and topic-help paths", async () => {
     const telemetryServer = await startTelemetryServer();