diff --git a/docs/TELEMETRY.md b/docs/TELEMETRY.md index ce949be44..06e47a8fe 100644 --- a/docs/TELEMETRY.md +++ b/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 diff --git a/src/agent/use-skill.ts b/src/agent/use-skill.ts index 15313b6a7..d4852f26b 100644 --- a/src/agent/use-skill.ts +++ b/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}`; }, }); diff --git a/src/telemetry/classify.ts b/src/telemetry/classify.ts index 6b24af546..8dce6be4a 100644 --- a/src/telemetry/classify.ts +++ b/src/telemetry/classify.ts @@ -71,6 +71,38 @@ const BUILT_IN_AGENT_NAMES: ReadonlySet = 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 = 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 = 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) diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index 7c7fa3a23..8bf73dbc6 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -165,9 +165,9 @@ const EVENT_PROPERTY_ALLOWLIST: Record = { // 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"], diff --git a/src/telemetry/product-events.ts b/src/telemetry/product-events.ts index 1c390cd08..1eff10728 100644 --- a/src/telemetry/product-events.ts +++ b/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; diff --git a/tests/unit/telemetry-product-events.test.ts b/tests/unit/telemetry-product-events.test.ts index 805503b69..8600e4827 100644 --- a/tests/unit/telemetry-product-events.test.ts +++ b/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 // ---------------------------------------------------------------------------