Report first-party skill names on skill_used, bucket the rest as custom (#959)

* Report first-party skill names on skill_used, bucket the rest as custom

* Qualify skill-name privacy bullet and narrow first-party skill comment

* Correct the skill_used allowlist rationale to reportable-by-name

user-invocable false opts out of slash synthesis, not name reporting: seven flagged skills stay allowlisted as use_skill recipes; only the four bake-only background skills are excluded.

Author: TheGreatAxios

docs/TELEMETRY.md

@@ -18,7 +18,7 @@ Each event carries a small set of properties:
 | `$ai_generation`     | Once per completed turn (may be sampled); always on turn failure                    | `$ai_trace_id`, `$ai_provider`, `$ai_model`, `$ai_input_tokens`, `$ai_output_tokens`, `$ai_latency`, `$ai_is_error`, `$ai_error`, `$ai_cache_read_input_tokens`, `$ai_cache_creation_input_tokens`, `$ai_reasoning_tokens`, `tool_call_count`, `tool_error_count`, `subagent_call_count` |
 | `$ai_span`           | Opt-in only — once per top-level tool call when `CORBITS_TELEMETRY_AI_SPANS` is set | `$ai_trace_id`, `$ai_span_id`, `$ai_parent_id`, `$ai_span_name`, `$ai_is_error`                                                                                                                                                                                                          |
 | `slash_command`      | A slash command is dispatched (shared product-event path)                           | `command_name`                                                                                                                                                                                                                                                                           |
-| `skill_used`         | `use_skill` loads a skill that resolved                                             | (none beyond common properties)                                                                                                                                                                                                                                                          |
+| `skill_used`         | `use_skill` loads a skill that resolved                                             | `skill_name`                                                                                                                                                                                                                                                                             |
 | `plugin_loaded`      | First successful load of a plugin identity in this process                          | `origin`                                                                                                                                                                                                                                                                                 |
 | `subagent_start`     | A `spawn_agent` dispatch begins                                                     | `agent_name`                                                                                                                                                                                                                                                                             |
 | `subagent_end`       | A `spawn_agent` dispatch finishes                                                   | `agent_name`, `status`, `duration_ms`, `model`, `turn_count`, `input_tokens`, `output_tokens`, `cache_read_tokens`, `cache_write_tokens`, `reasoning_tokens`, `tool_call_count`, `tool_error_count`, `stop_reason`, `parent_trace_id`                                                    |
@@ -76,13 +76,27 @@ share of prompts driven by MCP stays visible without the server key coming
 with it. `agent_name` on `subagent_*` is the same pattern: first-party
 director ids from `DIRECTOR_IDS` (and the legacy `worker` alias) are reported
 by id; project-defined or marketplace profile ids become `custom`.
-`skill_used` and `plugin_loaded` go further: there is no first-party list of
-skills or plugins to match against, so `skill_used` carries no name at all and
-`plugin_loaded` carries only `origin`, the discovery tier (`repo`, `user`,
-`project`, `path`). Enabled telemetry reports the same plugin identity at most
-once per runtime reporter, including across reloads. Disabled/no-op loads do not
-consume that identity, so enabling telemetry later can report the first real
-load.
+`skill_used` carries `skill_name`: a first-party skill name reportable by
+name from the closed `corbits-skills` allowlist (`ast-grep`, `create-issue`,
+`git-rebase`, `git-worktrees`, `implement`, `interview`,
+`linear-issue-workflow`, `opsh`, `philosophy`, `plan`,
+`pull-request-review`, `refactor`, `review`, `scribe`, `style`,
+`typescript`), or `custom` for anything else. `user-invocable: false` opts a
+skill out of slash synthesis, not out of name reporting: eleven bundled
+skills carry the flag, and seven of them (`git-rebase`, `git-worktrees`,
+`linear-issue-workflow`, `opsh`, `philosophy`, `style`, `typescript`)
+remain real `use_skill` recipes, so they stay on the allowlist — the names
+are ours either way. Excluded are the four bake-only background skills
+(`idiot-proof`, `native-integration`, `native-runtime`, `ponytail`), which
+are baked into agent prompts rather than invoked as skills. Unknown,
+project-local, and plugin-authored skill names are never transmitted —
+`skill_name` is the only identifying-adjacent property the event can carry.
+`plugin_loaded` goes further: there is no first-party list of plugins to
+match against, so it carries only `origin`, the discovery tier (`repo`,
+`user`, `project`, `path`). Enabled telemetry reports the same plugin
+identity at most once per runtime reporter, including across reloads.
+Disabled/no-op loads do not consume that identity, so enabling telemetry later
+can report the first real load.
 
 `error_class` is bucketed the same way: only the error types defined by the
 language are reported by name, because an error subclass defined in
@@ -187,8 +201,10 @@ retry paths.
 - Prompts, model output, or any conversation content (except intentional
   free-text the operator types into `/feedback` — see below)
 - File paths, file contents, or repo/project names
-- Names anyone but this project chose: MCP servers, skills, plugins, agent
-  profiles, plugin-registered slash commands, error subclasses (see above)
+- Names anyone but this project chose: MCP servers, skills (other than
+  first-party `corbits-skills` names, which are sent by name per the table
+  above), plugins, agent profiles, plugin-registered slash commands, error
+  subclasses (see above)
 - Shell commands, tool arguments, or tool results
 - API keys, tokens, or any other credential
 - Anything not in the allowlist above

src/agent/use-skill.ts

@@ -5,6 +5,7 @@ import { type } from "arktype";
 
 import { resolveSkillBody } from "../extensions/skills.js";
 import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js";
+import { captureSkillUsed } from "../telemetry/product-events.js";
 
 // Lazy skill loading: names are listed in the system prompt; details come from
 // skill_search; this tool pulls the full instructions into context when the
@@ -50,10 +51,10 @@ export function createUseSkillTool(
       }
       const body = await resolveSkillBody(cwd, name, skillDirs);
       if (body === undefined) return `No skill named "${name}" is available.`;
-      // Skills are project- or plugin-authored, so the name is as identifying
-      // as any other user-written string and never leaves the process; the
-      // event records only that a skill was loaded.
-      telemetry.capture("skill_used");
+      // Skill names are project- or plugin-authored, so an unrecognised
+      // name never leaves the process: first-party `corbits-skills` names
+      // are reported by name, everything else as `custom`.
+      captureSkillUsed(telemetry, name);
       return `Skill "${name}" — follow these instructions for this task:\n\n${body}`;
     },
   });

src/telemetry/classify.ts

@@ -71,6 +71,38 @@ const BUILT_IN_AGENT_NAMES: ReadonlySet<string> = new Set([
   "worker",
 ]);
 
+// First-party skills reportable by name: the bundled `corbits-skills`
+// skills (plugins/corbits-skills/skills) whose names we ship ourselves, so
+// reporting one cannot identify the operator. The manifest carries only the
+// plugin id and kind — no skill list — so the closed set is spelled out here
+// and pinned by tests/unit/telemetry-product-events.test.ts.
+// `user-invocable: false` is a slash-surface flag, not a telemetry flag:
+// eleven bundled skills carry it, and seven of them (git-rebase,
+// git-worktrees, linear-issue-workflow, opsh, philosophy, style, typescript)
+// opt out of slash synthesis yet remain real `use_skill` recipes, so they
+// stay reportable by name. Excluded are the four bake-only background skills
+// (idiot-proof, native-integration, native-runtime, ponytail), which are
+// baked into agent prompts rather than invoked as skills. Project- or
+// plugin-authored skills are never reported by name.
+const FIRST_PARTY_SKILL_NAMES: ReadonlySet<string> = new Set([
+  "ast-grep",
+  "create-issue",
+  "git-rebase",
+  "git-worktrees",
+  "implement",
+  "interview",
+  "linear-issue-workflow",
+  "opsh",
+  "philosophy",
+  "plan",
+  "pull-request-review",
+  "refactor",
+  "review",
+  "scribe",
+  "style",
+  "typescript",
+]);
+
 // Error constructors defined by the language. A subclass name is application
 // or plugin code and can be as identifying as any other author-chosen string.
 const STANDARD_ERROR_NAMES: ReadonlySet<string> = new Set([
@@ -100,6 +132,10 @@ export function classifyAgentName(agentName: string): string {
   return BUILT_IN_AGENT_NAMES.has(agentName) ? agentName : CUSTOM;
 }
 
+export function classifySkillName(skillName: string): string {
+  return FIRST_PARTY_SKILL_NAMES.has(skillName) ? skillName : CUSTOM;
+}
+
 export function classifyErrorClass(error: unknown): string {
   if (!(error instanceof Error)) return "non_error";
   return STANDARD_ERROR_NAMES.has(error.constructor.name)

src/telemetry/index.ts

@@ -165,9 +165,9 @@ const EVENT_PROPERTY_ALLOWLIST: Record<TelemetryEvent, readonly string[]> = {
   // allowlist bounds which keys travel; the classifiers bound which values
   // can, and the two are independent guards on purpose.
   slash_command: ["command_name"],
-  // Skill names are project- or plugin-authored with no first-party set to
-  // match against, so the event counts skill use and carries nothing else.
-  skill_used: [],
+  // skill_name is a first-party corbits-skills name (see classifySkillName)
+  // or "custom" — project- or plugin-authored names never leave the process.
+  skill_used: ["skill_name"],
   // origin is the discovery tier (repo/user/project/path); the manifest id is
   // author-chosen free text and is not sent.
   plugin_loaded: ["origin"],

src/telemetry/product-events.ts

@@ -6,7 +6,7 @@ import type {
   SubAgentTerminalReason,
 } from "../subagent/types.js";
 import type { Telemetry } from "./index.js";
-import { classifyCommandName } from "./classify.js";
+import { classifyCommandName, classifySkillName } from "./classify.js";
 
 /** Emit slash_command with a classified first-party (or `custom`) name. */
 export function captureSlashCommand(
@@ -18,6 +18,16 @@ export function captureSlashCommand(
   });
 }
 
+/** Emit skill_used with a classified first-party (or `custom`) skill name. */
+export function captureSkillUsed(
+  telemetry: Telemetry,
+  skillName: string,
+): void {
+  telemetry.capture("skill_used", {
+    skill_name: classifySkillName(skillName),
+  });
+}
+
 export interface CaptureSubagentEndArgs {
   agentName: string;
   status: string;

tests/unit/telemetry-product-events.test.ts

@@ -28,6 +28,7 @@ import {
   classifyAgentName,
   classifyErrorClass,
   classifyPermissionKind,
+  classifySkillName,
 } from "../../src/telemetry/classify.js";
 
 import {
@@ -37,6 +38,7 @@ import {
 } from "../../src/telemetry/index.js";
 import {
   buildSubagentEndProperties,
+  captureSkillUsed,
   captureSlashCommand,
   createPluginLoadReporter,
 } from "../../src/telemetry/product-events.js";
@@ -167,7 +169,7 @@ test("permission_prompt reports built-in tool ids by name", () => {
 // 2. skill_name — a project-local skill can be named after the employer
 // ---------------------------------------------------------------------------
 
-test("skill_used carries no skill name, so an employer-named skill cannot leak", async () => {
+test("skill_used buckets an employer-named skill to custom so it cannot leak", async () => {
   const { telemetry, wire, events } = harness();
   const cwd = await tempDir("corbits-skill-");
   const skillDir = join(cwd, ".agents", "skills", "acme-internal-deploy");
@@ -190,10 +192,81 @@ test("skill_used carries no skill name, so an employer-named skill cannot leak",
   expect(result).toContain("Deploy the internal service");
   const [event] = await events();
   expect(event?.event).toBe("skill_used");
-  expect(event?.properties.skill_name).toBeUndefined();
+  expect(event?.properties.skill_name).toBe("custom");
   expect(await wire()).not.toContain("acme-internal");
 });
 
+test("skill_used reports a first-party skill by name", async () => {
+  const { telemetry, events } = harness();
+  const cwd = await tempDir("corbits-skill-");
+  const skillDir = join(cwd, ".agents", "skills", "review");
+  await mkdir(skillDir, { recursive: true });
+  await writeFile(
+    join(skillDir, "SKILL.md"),
+    "---\nname: review\n---\n\nReview the branch.\n",
+  );
+
+  const tool = createUseSkillTool(cwd, [], telemetry);
+  if (tool.kind !== "string")
+    throw new Error(`expected string tool, got ${tool.kind}`);
+  const result = await tool.handler(
+    { name: "review" },
+    new AbortController().signal,
+  );
+
+  expect(result).toContain("Review the branch");
+  const [event] = await events();
+  expect(event?.event).toBe("skill_used");
+  expect(event?.properties.skill_name).toBe("review");
+});
+
+test("first-party skill names are reported by name; everything else stays custom", () => {
+  for (const name of [
+    "ast-grep",
+    "create-issue",
+    "git-rebase",
+    "git-worktrees",
+    "implement",
+    "interview",
+    "linear-issue-workflow",
+    "opsh",
+    "philosophy",
+    "plan",
+    "pull-request-review",
+    "refactor",
+    "review",
+    "scribe",
+    "style",
+    "typescript",
+  ]) {
+    expect(classifySkillName(name)).toBe(name);
+  }
+  expect(classifySkillName("acme-internal-deploy")).toBe("custom");
+  // Bundled catalog skills outside the closed allowlist are not reported by
+  // name either — the allowlist is the closed set, not the skills directory.
+  // All four bake-only background skills (user-invocable: false, baked into
+  // agent prompts rather than invoked as skills) stay custom, while the
+  // seven flagged-but-allowlisted use_skill-only recipes assert by name above.
+  expect(classifySkillName("idiot-proof")).toBe("custom");
+  expect(classifySkillName("native-integration")).toBe("custom");
+  expect(classifySkillName("native-runtime")).toBe("custom");
+  expect(classifySkillName("ponytail")).toBe("custom");
+  expect(classifySkillName("Review")).toBe("custom");
+  expect(classifySkillName("")).toBe("custom");
+});
+
+test('skill_used buckets a plugin-authored skill to "custom"', async () => {
+  const { telemetry, wire, events } = harness();
+
+  captureSkillUsed(telemetry, "acmecorp-deploy");
+  captureSkillUsed(telemetry, "plan");
+
+  const captured = await events();
+  expect(captured[0]?.properties.skill_name).toBe("custom");
+  expect(captured[1]?.properties.skill_name).toBe("plan");
+  expect(await wire()).not.toContain("acmecorp");
+});
+
 // ---------------------------------------------------------------------------
 // 3. plugin_id — an author-chosen manifest id on a private local plugin
 // ---------------------------------------------------------------------------