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

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

@@ -0,0 +1,8 @@
+---
+"browse": patch
+---
+
+Emit `skill_id` and `first_success` properties on `cli.command_completed` telemetry.
+
+- `skill_id`: 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.
+- `first_success: true` is emitted exactly once per anonymous install: the first time a browser-driver command completes successfully, tracked via a marker file stored alongside the existing anonymous install id. Best-effort like all CLI telemetry — it never affects command behavior.

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 =
@@ -106,6 +107,10 @@ export function isBlobSkillId(skillId: ParsedSkillId): boolean {
 
 export async function installSkill(rawSkillId: string): Promise<void> {
   const skillId = 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: skillId.id });
   const npxPath = await findExecutable("npx");
   if (!npxPath) {
     fail(
@@ -151,6 +156,7 @@ export async function installSkill(rawSkillId: string): Promise<void> {
 }
 
 export async function installBundledCliSkill(): Promise<void> {
+  setRunTelemetryCompletion({ skillId: "bundled/browse" });
   const npxPath = await findExecutable("npx");
   if (!npxPath) {
     fail(

packages/cli/src/lib/telemetry.ts

@@ -1,5 +1,5 @@
 import { randomUUID } from "node:crypto";
-import { mkdir, readFile, writeFile } from "node:fs/promises";
+import { access, mkdir, readFile, writeFile } from "node:fs/promises";
 import { homedir } from "node:os";
 import { dirname, join } from "node:path";
 
@@ -113,12 +113,14 @@ export async function captureCommandCompleted(
 
   const exitCode = resolveExitCode(error);
   const success = exitCode === 0;
+  const firstSuccess = await detectFirstSuccess(command, success);
   const completionCapture = getCliTelemetry(cliVersion)
     .capture(
       "cli.command_completed",
       commandCompletedProperties(command, {
         error,
         exitCode,
+        firstSuccess,
         recordedError: state.recordedError,
         success,
       }),
@@ -221,6 +223,7 @@ function commandCompletedProperties(
   completion: {
     error: Error | undefined;
     exitCode: number;
+    firstSuccess?: boolean;
     recordedError?: RecordedCommandError;
     success: boolean;
   },
@@ -236,7 +239,7 @@ function commandCompletedProperties(
     runTelemetry.resultCode ??
     fallbackResultCode(completion.success, errorType);
 
-  return {
+  const properties: TelemetryProperties = {
     command_path: invocation.commandPath,
     top_level_command: invocation.topLevelCommand,
     leaf_command: invocation.leafCommand,
@@ -255,7 +258,69 @@ function commandCompletedProperties(
       failureTelemetry?.requestHadHttpResponse ??
       runTelemetry.requestHadHttpResponse ??
       null,
+    skill_id: runTelemetry.skillId ?? null,
   };
+
+  if (completion.firstSuccess) {
+    properties.first_success = true;
+  }
+
+  return properties;
+}
+
+// Top-level command topics that are not browser-driver commands. Activation
+// (first_success) counts only successful driver commands, so new driver
+// commands are covered by default.
+const nonDriverTopLevelCommands = new Set([
+  "cloud",
+  "daemon",
+  "doctor",
+  "functions",
+  "skills",
+  "status",
+  "stop",
+  "templates",
+]);
+
+const activatedMarkerFileName = "activated";
+
+// Returns true exactly once per install: the first time a browser-driver
+// command completes successfully. Tracked via a marker file alongside the
+// anonymous install id. Best-effort — never throws, never blocks the command.
+async function detectFirstSuccess(
+  command: CommandInvocation,
+  success: boolean,
+  env: NodeJS.ProcessEnv = process.env,
+): Promise<boolean> {
+  if (
+    !success ||
+    nonDriverTopLevelCommands.has(command.topLevelCommand) ||
+    isTelemetryDisabled(env)
+  ) {
+    return false;
+  }
+
+  const markerPath = join(
+    dirname(resolveInstallIdPath(env)),
+    activatedMarkerFileName,
+  );
+
+  try {
+    await access(markerPath);
+    return false;
+  } catch {
+    // No marker yet: this is the first successful driver command.
+  }
+
+  try {
+    await mkdir(dirname(markerPath), { recursive: true });
+    await writeFile(markerPath, `${new Date().toISOString()}\n`, "utf8");
+  } catch {
+    // If persistence fails, a later success may report first_success again
+    // rather than affecting CLI behavior.
+  }
+
+  return true;
 }
 
 function fallbackResultCode(

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

@@ -1,9 +1,11 @@
 import { access, mkdtemp, rm } from "node:fs/promises";
+import net from "node:net";
 import { tmpdir } from "node:os";
-import { join } from "node:path";
+import { dirname, join } from "node:path";
 
 import { afterEach, describe, expect, it } from "vitest";
 
+import { getSocketPath } from "../src/lib/driver/daemon/paths.js";
 import {
   jsonResponse,
   startFakeBrowserbaseServer,
@@ -64,6 +66,11 @@ 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);
+      expect(completedPayload.properties).not.toHaveProperty("first_success");
+      await expect(
+        access(join(dirname(installIdFile), "activated")),
+      ).rejects.toThrow();
     } finally {
       await telemetryServer.close();
     }
@@ -220,6 +227,172 @@ 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.command_path).toBe("skills.add");
+      expect(completedPayload.properties.skill_id).toBe(
+        "example.com/extract-reviews",
+      );
+      expect(completedPayload.properties.result_code).toBe("skill_not_found");
+      expect(completedPayload.properties).not.toHaveProperty("first_success");
+
+      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("emits first_success exactly once for successful driver commands", async () => {
+    const telemetryServer = await startTelemetryServer();
+    const daemonDir = await mkdtemp(join(tmpdir(), "browse-telemetry-daemon-"));
+    cleanupPaths.push(daemonDir);
+    const previousDaemonDir = process.env.BROWSE_DAEMON_DIR;
+    process.env.BROWSE_DAEMON_DIR = daemonDir;
+    const session = "telemetry-first-success";
+    const server = net.createServer((socket) => {
+      let buffer = "";
+      socket.on("data", (chunk) => {
+        buffer += chunk.toString();
+        const newline = buffer.indexOf("\n");
+        if (newline === -1) return;
+
+        const request = JSON.parse(buffer.slice(0, newline)) as {
+          id: string;
+          type: string;
+        };
+
+        if (request.type === "status") {
+          socket.end(
+            JSON.stringify({
+              data: {
+                browserConnected: true,
+                initialized: true,
+                mode: "managed-local",
+                pages: [],
+                pid: process.pid,
+                session,
+                target: { headless: true, kind: "managed-local" },
+              },
+              id: request.id,
+              type: "success",
+            }) + "\n",
+          );
+          return;
+        }
+
+        socket.end(
+          JSON.stringify({
+            data: { ok: true },
+            id: request.id,
+            type: "success",
+          }) + "\n",
+        );
+      });
+    });
+
+    try {
+      await new Promise<void>((resolve, reject) => {
+        server.once("error", reject);
+        server.listen(getSocketPath(session), resolve);
+      });
+
+      const installIdFile = await tempInstallIdFile(
+        "browse-telemetry-first-success-",
+      );
+      const env = {
+        ...telemetryEnv(telemetryServer, installIdFile),
+        BROWSE_DAEMON_DIR: daemonDir,
+      };
+
+      const first = await runCli(
+        ["viewport", "1024", "768", "--session", session],
+        { env },
+      );
+      expect(first.exitCode).toBe(0);
+
+      const second = await runCli(
+        ["viewport", "800", "600", "--session", session],
+        { env },
+      );
+      expect(second.exitCode).toBe(0);
+
+      const completedPayloads = telemetryPayloads(telemetryServer).filter(
+        (payload) => payload.event === "cli.command_completed",
+      );
+      expect(completedPayloads).toHaveLength(2);
+      expect(completedPayloads[0]?.properties.first_success).toBe(true);
+      expect(completedPayloads[1]?.properties).not.toHaveProperty(
+        "first_success",
+      );
+
+      // The activation marker persists alongside the anonymous install id.
+      await expect(
+        access(join(dirname(installIdFile), "activated")),
+      ).resolves.toBeUndefined();
+    } finally {
+      if (previousDaemonDir === undefined) {
+        delete process.env.BROWSE_DAEMON_DIR;
+      } else {
+        process.env.BROWSE_DAEMON_DIR = previousDaemonDir;
+      }
+      await new Promise<void>((resolve, reject) => {
+        server.close((error) => (error ? reject(error) : resolve()));
+      });
+      await telemetryServer.close();
+    }
+  });
+
   it("does not emit telemetry for help and topic-help paths", async () => {
     const telemetryServer = await startTelemetryServer();