From 42335b47701666b46120b6e0e6c14ce2724e1d2a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 15:44:41 -0700 Subject: [PATCH 1/5] Batch telemetry events through a single queued transport Two upcoming event catalogs turn telemetry volume from one event per turn into one per tool call, which the previous transport would have answered with an unbounded set of concurrent per-event POSTs. Queueing behind one request bounds both sockets and memory when the endpoint is unreachable. --- src/telemetry/index.ts | 127 ++++++++++--- tests/unit/telemetry-first-run.test.ts | 2 + tests/unit/telemetry-toggle.test.ts | 11 +- tests/unit/telemetry.test.ts | 240 ++++++++++++++++++++----- 4 files changed, 310 insertions(+), 70 deletions(-) diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index ba61adcc9..df9e56954 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -20,6 +20,22 @@ export const POSTHOG_API_KEY = process.env[TELEMETRY_KEY_ENV] ?? DEFAULT_POSTHOG // in flight past this is dropped. const FLUSH_DEADLINE_MS = 500; +// Batching defaults. A busy turn can emit an event per tool call, so events +// accumulate until either trigger fires rather than opening a socket each +// time. The queue limit bounds memory when the endpoint is unreachable — +// a captive portal or hung proxy would otherwise grow the queue for the +// whole session behind a single stuck request. +const DEFAULT_BATCH_SIZE = 20; +const DEFAULT_BATCH_INTERVAL_MS = 10_000; +const DEFAULT_QUEUE_LIMIT = 500; +const REQUEST_TIMEOUT_MS = 3000; + +export type BatchTuning = { + size?: number; + intervalMs?: number; + queueLimit?: number; +}; + // Shown once per installation, in whichever surface a new user reaches // first: the onboarding panel on a fresh install (so disclosure accompanies // the very first event), and the TUI banner otherwise. @@ -113,19 +129,26 @@ export type CreateTelemetryOptions = { fetchFn?: typeof fetch; host?: string; apiKey?: string; + batch?: BatchTuning; +}; + +type QueuedEvent = { + event: TelemetryEvent; + properties: Record; + timestamp: string; }; export type Telemetry = { enabled: boolean; capture(event: TelemetryEvent, properties?: Record): void; - // Waits briefly for captures currently in flight to settle, giving up + // Sends whatever is queued and waits briefly for it to settle, giving up // after a short deadline so a slow endpoint can never hold up process // exit. Callers use this to bound exit against dropped fire-and-forget // requests without ever making capture() itself blocking. flush(): Promise; }; -// Fire-and-forget PostHog capture client. Never throws, never blocks the +// Fire-and-forget PostHog batch client. Never throws, never blocks the // caller — errors (including timeouts) are swallowed silently since // telemetry must never affect product behavior. export function createTelemetry(options: CreateTelemetryOptions): Telemetry { @@ -136,18 +159,64 @@ export function createTelemetry(options: CreateTelemetryOptions): Telemetry { const fetchFn = options.fetchFn ?? fetch; const installationId = options.settings?.telemetry?.installationId ?? ""; - // Tracked so flush() can wait for in-flight requests without making - // capture() itself awaitable. - const pending = new Set>(); + const batchSize = options.batch?.size ?? DEFAULT_BATCH_SIZE; + const batchIntervalMs = options.batch?.intervalMs ?? DEFAULT_BATCH_INTERVAL_MS; + const queueLimit = options.batch?.queueLimit ?? DEFAULT_QUEUE_LIMIT; + + const queue: QueuedEvent[] = []; + let inFlight: Promise | null = null; + let timer: ReturnType | null = null; + + function cancelTimer(): void { + if (timer === null) return; + clearTimeout(timer); + timer = null; + } + + async function send(events: QueuedEvent[]): Promise { + const body = { + api_key: apiKey, + batch: events.map((queued) => ({ + event: queued.event, + timestamp: queued.timestamp, + properties: { ...queued.properties, distinct_id: installationId }, + })), + }; + try { + await fetchFn(`${host}/batch/`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), + }); + } catch { + // Swallow all errors — telemetry must never surface failures. + } + } + + // Returns the existing drain when one is running so at most one request is + // ever open: events captured mid-flight are picked up by that drain's next + // iteration instead of opening a second socket. + function drain(): Promise { + if (inFlight !== null) return inFlight; + const running = (async () => { + while (queue.length > 0) { + await send(queue.splice(0, batchSize)); + } + })().finally(() => { + inFlight = null; + }); + inFlight = running; + return running; + } function capture(event: TelemetryEvent, properties?: Record): void { if (!enabled) return; if (!(event === "cli_start" || event === "session_end" || event === "inference_turn")) return; - const body = { - api_key: apiKey, + queue.push({ event, - distinct_id: installationId, + timestamp: new Date().toISOString(), properties: { ...allowedProperties(event, properties), service_version: pkg.version, @@ -156,31 +225,37 @@ export function createTelemetry(options: CreateTelemetryOptions): Telemetry { schema_version: 1, session_id: SESSION_ID, }, - }; + }); - const request = fetchFn(`${host}/capture/`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify(body), - signal: AbortSignal.timeout(3000), - }) - .then(() => undefined) - .catch(() => { - // Swallow all errors — telemetry must never surface failures. - }); - pending.add(request); - void request.finally(() => pending.delete(request)); + // Oldest first: a stuck endpoint makes the head of the queue the least + // likely to still be worth reporting, and unbounded growth is never an + // acceptable alternative. + if (queue.length > queueLimit) queue.splice(0, queue.length - queueLimit); + + if (queue.length >= batchSize) { + cancelTimer(); + void drain(); + return; + } + if (timer === null) { + timer = setTimeout(() => { + timer = null; + void drain(); + }, batchIntervalMs); + timer.unref?.(); + } } async function flush(): Promise { - if (pending.size === 0) return; + cancelTimer(); + if (queue.length === 0 && inFlight === null) return; // Race against a short deadline: stragglers are dropped rather than - // allowed to delay exit for the full 3s per-request AbortSignal window. + // allowed to delay exit for the full per-request AbortSignal window. await Promise.race([ - Promise.allSettled(pending), + drain(), new Promise((resolve) => { - const timer = setTimeout(resolve, FLUSH_DEADLINE_MS); - timer.unref?.(); + const deadline = setTimeout(resolve, FLUSH_DEADLINE_MS); + deadline.unref?.(); }), ]); } diff --git a/tests/unit/telemetry-first-run.test.ts b/tests/unit/telemetry-first-run.test.ts index 33784a6e6..92cfac2c0 100644 --- a/tests/unit/telemetry-first-run.test.ts +++ b/tests/unit/telemetry-first-run.test.ts @@ -68,6 +68,7 @@ test("activation stamps the notice, swaps the singleton, and fires cli_start", a await activateHeldTelemetry("/fake/path", () => true, deps); expect(markCalls()).toBe(1); expect(getInstance()?.enabled).toBe(true); + await getInstance()?.flush(); expect(fetchCalls()).toBe(1); }); @@ -99,5 +100,6 @@ test("a failed notice stamp does not block activation", async () => { }); await activateHeldTelemetry("/fake/path", () => true, deps); expect(getInstance()?.enabled).toBe(true); + await getInstance()?.flush(); expect(fetchCalls()).toBe(1); }); diff --git a/tests/unit/telemetry-toggle.test.ts b/tests/unit/telemetry-toggle.test.ts index bd5e95f5a..b5e0926c5 100644 --- a/tests/unit/telemetry-toggle.test.ts +++ b/tests/unit/telemetry-toggle.test.ts @@ -180,7 +180,7 @@ test("toggle on re-enables after settings load/save resolve", async () => { }); test("session_id on captured payloads stays constant across an enable/disable/enable toggle cycle", async () => { - const capturedBodies: { properties: Record }[] = []; + const capturedBodies: { batch: { properties: Record }[] }[] = []; const fetchFn = ((_url: string, init: RequestInit) => { capturedBodies.push(JSON.parse(init.body as string)); return Promise.resolve(new Response("1", { status: 200 })); @@ -197,6 +197,9 @@ test("session_id on captured payloads stays constant across an enable/disable/en handler(true); await new Promise((resolve) => setTimeout(resolve, 10)); getInstance().capture("cli_start"); + // Toggling off discards the outgoing instance and its queue, so anything + // captured before it has to leave the process first. + await getInstance().flush(); handler(false); await new Promise((resolve) => setTimeout(resolve, 10)); @@ -205,12 +208,12 @@ test("session_id on captured payloads stays constant across an enable/disable/en handler(true); await new Promise((resolve) => setTimeout(resolve, 10)); getInstance().capture("cli_start"); + await getInstance().flush(); - await new Promise((resolve) => setTimeout(resolve, 0)); expect(capturedBodies.length).toBe(2); - const sessionId = capturedBodies[0].properties.session_id; + const sessionId = capturedBodies[0].batch[0].properties.session_id; expect(typeof sessionId).toBe("string"); expect((sessionId as string).length).toBeGreaterThan(0); - expect(capturedBodies[1].properties.session_id).toBe(sessionId); + expect(capturedBodies[1].batch[0].properties.session_id).toBe(sessionId); expect(sessionId).toBe(getSessionId()); }); diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index a34bfd48d..7e0ae3344 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -30,6 +30,20 @@ function fakeFetch(): { impl: typeof fetch; calls: () => number } { return { impl, calls: () => count }; } +type BatchBody = { + api_key: string; + batch: { event: string; timestamp: string; properties: Record }[]; +}; + +function recordingFetch() { + const bodies: BatchBody[] = []; + const impl = ((_url: string, init: RequestInit) => { + bodies.push(JSON.parse(init.body as string) as BatchBody); + return Promise.resolve(new Response("1", { status: 200 })); + }) as unknown as typeof fetch; + return { impl, bodies, events: () => bodies.flatMap((body) => body.batch) }; +} + test("resolveTelemetryEnabled is false when settings.telemetry.enabled is false", () => { expect(resolveTelemetryEnabled(settingsWith("id", false), {})).toBe(false); }); @@ -104,11 +118,7 @@ test("capture rejects unknown event names", () => { }); test("capture strips properties not in the event's allowlist", async () => { - const calls: unknown[] = []; - const impl = ((_url: string, init: RequestInit) => { - calls.push(JSON.parse(init.body as string)); - return Promise.resolve(new Response("1", { status: 200 })); - }) as unknown as typeof fetch; + const { impl, events } = recordingFetch(); const telemetry = createTelemetry({ settings: settingsWith("id"), env: {}, @@ -123,9 +133,9 @@ test("capture strips properties not in the event's allowlist", async () => { exit_reason: "done", secret_field: "should-not-appear", }); - await new Promise((resolve) => setTimeout(resolve, 0)); - expect(calls.length).toBe(1); - const body = calls[0] as { properties: Record }; + await telemetry.flush(); + expect(events().length).toBe(1); + const body = events()[0]; expect(body.properties.status).toBe("ok"); expect(body.properties.turn_count).toBe(3); expect(body.properties.duration_ms).toBe(100); @@ -135,11 +145,7 @@ test("capture strips properties not in the event's allowlist", async () => { }); test("capture strips properties not in inference_turn's allowlist", async () => { - const calls: unknown[] = []; - const impl = ((_url: string, init: RequestInit) => { - calls.push(JSON.parse(init.body as string)); - return Promise.resolve(new Response("1", { status: 200 })); - }) as unknown as typeof fetch; + const { impl, events } = recordingFetch(); const telemetry = createTelemetry({ settings: settingsWith("id"), env: {}, @@ -157,9 +163,9 @@ test("capture strips properties not in inference_turn's allowlist", async () => duration_ms: 400, prompt: "should-not-appear", }); - await new Promise((resolve) => setTimeout(resolve, 0)); - expect(calls.length).toBe(1); - const body = calls[0] as { event: string; properties: Record }; + await telemetry.flush(); + expect(events().length).toBe(1); + const body = events()[0]; expect(body.event).toBe("inference_turn"); expect(body.properties.provider_id).toBe("anthropic"); expect(body.properties.model_id).toBe("claude-x"); @@ -173,11 +179,7 @@ test("capture strips properties not in inference_turn's allowlist", async () => }); test("capture payload shape includes distinct_id and common props, with no client-side geoip flag", async () => { - const calls: unknown[] = []; - const impl = ((_url: string, init: RequestInit) => { - calls.push(JSON.parse(init.body as string)); - return Promise.resolve(new Response("1", { status: 200 })); - }) as unknown as typeof fetch; + const { impl, bodies, events } = recordingFetch(); const telemetry = createTelemetry({ settings: settingsWith("my-install-id"), env: {}, @@ -185,16 +187,13 @@ test("capture payload shape includes distinct_id and common props, with no clien apiKey: "test-key", }); telemetry.capture("cli_start"); - await new Promise((resolve) => setTimeout(resolve, 0)); - const body = calls[0] as { - api_key: string; - event: string; - distinct_id: string; - properties: Record; - }; - expect(body.api_key).toBe("test-key"); + await telemetry.flush(); + expect(bodies.length).toBe(1); + expect(bodies[0].api_key).toBe("test-key"); + const body = events()[0]; expect(body.event).toBe("cli_start"); - expect(body.distinct_id).toBe("my-install-id"); + expect(body.properties.distinct_id).toBe("my-install-id"); + expect(typeof body.timestamp).toBe("string"); expect(body.properties.$geoip_disable).toBeUndefined(); expect(body.properties.schema_version).toBe(1); expect(typeof body.properties.service_version).toBe("string"); @@ -246,11 +245,7 @@ test("flush resolves immediately when nothing is pending", async () => { }); test("capture attaches the same session_id across multiple events in one process", async () => { - const calls: unknown[] = []; - const impl = ((_url: string, init: RequestInit) => { - calls.push(JSON.parse(init.body as string)); - return Promise.resolve(new Response("1", { status: 200 })); - }) as unknown as typeof fetch; + const { impl, events } = recordingFetch(); const telemetry = createTelemetry({ settings: settingsWith("my-install-id"), env: {}, @@ -259,13 +254,13 @@ test("capture attaches the same session_id across multiple events in one process }); telemetry.capture("cli_start"); telemetry.capture("session_end", { status: "ok" }); - await new Promise((resolve) => setTimeout(resolve, 0)); - expect(calls.length).toBe(2); - const bodies = calls as { properties: Record }[]; - const sessionId = bodies[0].properties.session_id; + await telemetry.flush(); + const captured = events(); + expect(captured.length).toBe(2); + const sessionId = captured[0].properties.session_id; expect(typeof sessionId).toBe("string"); expect((sessionId as string).length).toBeGreaterThan(0); - expect(bodies[1].properties.session_id).toBe(sessionId); + expect(captured[1].properties.session_id).toBe(sessionId); expect(sessionId).toBe(getSessionId()); }); @@ -284,3 +279,168 @@ test("ensureTelemetrySettings called twice keeps installationId and enabled flag await rm(dir, { recursive: true, force: true }); } }); + +function gatedFetch() { + const releases: (() => void)[] = []; + const bodies: BatchBody[] = []; + let concurrent = 0; + let peakConcurrent = 0; + let open = false; + const impl = ((_url: string, init: RequestInit) => { + bodies.push(JSON.parse(init.body as string) as BatchBody); + concurrent++; + peakConcurrent = Math.max(peakConcurrent, concurrent); + return new Promise((resolve) => { + const release = () => { + concurrent--; + resolve(new Response("1", { status: 200 })); + }; + if (open) release(); + else releases.push(release); + }); + }) as unknown as typeof fetch; + return { + impl, + bodies, + openGate: () => { + open = true; + for (const release of releases.splice(0)) release(); + }, + peak: () => peakConcurrent, + }; +} + +const turnCounts = (body: BatchBody) => body.batch.map((entry) => entry.properties.turn_count); + +test("capture posts batches to the /batch/ endpoint", async () => { + const urls: string[] = []; + const impl = ((url: string) => { + urls.push(url); + return Promise.resolve(new Response("1", { status: 200 })); + }) as unknown as typeof fetch; + const telemetry = createTelemetry({ + settings: settingsWith("id"), + env: {}, + fetchFn: impl, + apiKey: "test-key", + host: "https://telemetry.example", + }); + telemetry.capture("cli_start"); + await telemetry.flush(); + expect(urls).toEqual(["https://telemetry.example/batch/"]); +}); + +test("reaching the batch size sends one request holding every queued event", async () => { + const { impl, bodies } = recordingFetch(); + const telemetry = createTelemetry({ + settings: settingsWith("id"), + env: {}, + fetchFn: impl, + apiKey: "test-key", + batch: { size: 3, intervalMs: 60_000 }, + }); + telemetry.capture("session_end", { turn_count: 1 }); + telemetry.capture("session_end", { turn_count: 2 }); + expect(bodies.length).toBe(0); + + telemetry.capture("session_end", { turn_count: 3 }); + await new Promise((resolve) => setTimeout(resolve, 5)); + expect(bodies.length).toBe(1); + expect(turnCounts(bodies[0])).toEqual([1, 2, 3]); +}); + +test("a partial batch is sent once the batch interval elapses", async () => { + const { impl, bodies } = recordingFetch(); + const telemetry = createTelemetry({ + settings: settingsWith("id"), + env: {}, + fetchFn: impl, + apiKey: "test-key", + batch: { size: 100, intervalMs: 10 }, + }); + telemetry.capture("session_end", { turn_count: 1 }); + expect(bodies.length).toBe(0); + + await new Promise((resolve) => setTimeout(resolve, 60)); + expect(bodies.length).toBe(1); + expect(turnCounts(bodies[0])).toEqual([1]); +}); + +test("overflowing the queue drops the oldest events", async () => { + const { impl, bodies } = recordingFetch(); + const telemetry = createTelemetry({ + settings: settingsWith("id"), + env: {}, + fetchFn: impl, + apiKey: "test-key", + batch: { size: 100, intervalMs: 60_000, queueLimit: 3 }, + }); + for (let turn = 1; turn <= 5; turn++) telemetry.capture("session_end", { turn_count: turn }); + await telemetry.flush(); + expect(bodies.length).toBe(1); + expect(turnCounts(bodies[0])).toEqual([3, 4, 5]); +}); + +test("captures during a request queue behind it instead of opening a second one", async () => { + const gate = gatedFetch(); + const telemetry = createTelemetry({ + settings: settingsWith("id"), + env: {}, + fetchFn: gate.impl, + apiKey: "test-key", + batch: { size: 1, intervalMs: 60_000 }, + }); + telemetry.capture("session_end", { turn_count: 1 }); + telemetry.capture("session_end", { turn_count: 2 }); + telemetry.capture("session_end", { turn_count: 3 }); + await new Promise((resolve) => setTimeout(resolve, 5)); + expect(gate.bodies.length).toBe(1); + + gate.openGate(); + await telemetry.flush(); + expect(gate.peak()).toBe(1); + expect(gate.bodies.map(turnCounts)).toEqual([[1], [2], [3]]); +}); + +test("flush drains a partially full queue within its deadline", async () => { + const { impl, bodies } = recordingFetch(); + const telemetry = createTelemetry({ + settings: settingsWith("id"), + env: {}, + fetchFn: impl, + apiKey: "test-key", + batch: { size: 100, intervalMs: 60_000 }, + }); + telemetry.capture("session_end", { turn_count: 1 }); + telemetry.capture("session_end", { turn_count: 2 }); + const start = Date.now(); + await telemetry.flush(); + expect(Date.now() - start).toBeLessThan(500); + expect(bodies.length).toBe(1); + expect(turnCounts(bodies[0])).toEqual([1, 2]); +}); + +test("a hung endpoint caps the queue and never opens a second request", async () => { + const gate = gatedFetch(); + const telemetry = createTelemetry({ + settings: settingsWith("id"), + env: {}, + fetchFn: gate.impl, + apiKey: "test-key", + batch: { size: 2, intervalMs: 60_000, queueLimit: 4 }, + }); + for (let turn = 1; turn <= 20; turn++) { + telemetry.capture("session_end", { turn_count: turn }); + await new Promise((resolve) => setTimeout(resolve, 1)); + } + expect(gate.bodies.length).toBe(1); + expect(gate.peak()).toBe(1); + expect(turnCounts(gate.bodies[0])).toEqual([1, 2]); + + gate.openGate(); + await telemetry.flush(); + expect(gate.peak()).toBe(1); + // Only the newest queueLimit events survived the overflow; everything + // between the in-flight batch and them was shed rather than buffered. + expect(gate.bodies.slice(1).flatMap(turnCounts)).toEqual([17, 18, 19, 20]); +}); From 8bebe1540cde4cb6575573c9b13c1103c31119d4 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 15:50:20 -0700 Subject: [PATCH 2/5] Document how telemetry batches and sheds events on the wire --- docs/TELEMETRY.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/TELEMETRY.md b/docs/TELEMETRY.md index 9133aaf55..13040c943 100644 --- a/docs/TELEMETRY.md +++ b/docs/TELEMETRY.md @@ -92,6 +92,27 @@ Events are sent to PostHog. PostHog derives an approximate country from the request IP server-side; the client sends no location data itself. No self-hosted or third-party analytics beyond PostHog are used. +## On the wire + +Events are not sent one at a time. Each captured event is stamped with its +capture time and held in an in-memory queue, which is posted to PostHog's +`/batch/` endpoint when it reaches the batch size or when the batch +interval elapses, whichever comes first. At most one request is ever in +flight: events captured while a request is open wait for it rather than +opening another connection. Exit paths flush the queue, bounded by a short +deadline so a slow endpoint cannot delay quitting. + +The queue has a hard depth limit. Once it is full — which in practice means +the endpoint is unreachable, as on a captive portal or behind a hung proxy +— the oldest queued events are dropped to make room for new ones. Telemetry +is therefore lossy by design: it never grows memory without bound, never +retries indefinitely, and never blocks or reports failures to the user. +Nothing is written to disk, so dropped events are gone rather than deferred +to a later run. + +See `src/telemetry/index.ts` for the batch size, interval, and queue limit +in force. + ## Not this document Local performance tracing and optional OpenTelemetry export to an operator-owned From fae60bb42c6763be7fffffd4ca599c5c045ef65f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 15:56:41 -0700 Subject: [PATCH 3/5] Discard queued telemetry events when the user opts out Dropping the singleton on opt-out left the outgoing instance's batch timer armed, so events captured before the toggle would still reach the network afterwards. Opting out speaks to activity already generated, not only to activity still to come. --- docs/TELEMETRY.md | 6 ++++ src/telemetry/index.ts | 15 +++++++++- src/telemetry/singleton.ts | 7 ++++- src/telemetry/toggle.ts | 12 +++++--- tests/unit/telemetry-toggle.test.ts | 44 ++++++++++++++++++++++++++++- 5 files changed, 77 insertions(+), 7 deletions(-) diff --git a/docs/TELEMETRY.md b/docs/TELEMETRY.md index 13040c943..d23cb9ce5 100644 --- a/docs/TELEMETRY.md +++ b/docs/TELEMETRY.md @@ -46,6 +46,12 @@ Any of the following disables telemetry entirely: - `CORBITS_TELEMETRY` set to any falsy value: `0`, `false`, `off`, `no`, or empty - `DO_NOT_TRACK=1` (the standard [Console Do Not Track](https://consoledonottrack.com/) convention) +Turning telemetry off also discards whatever is still queued and unsent. +Events captured earlier in the session but not yet transmitted are thrown +away at the moment you opt out, not sent on the way out — opting out covers +the activity you have already generated, not just the activity still to +come. + Re-enable from the same Telemetry tab or by removing the env var / settings override. While an env kill is active the Telemetry tab cannot re-enable — the env override always wins, and the attempt is refused rather than diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index df9e56954..406f9e5d8 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -146,6 +146,12 @@ export type Telemetry = { // exit. Callers use this to bound exit against dropped fire-and-forget // requests without ever making capture() itself blocking. flush(): Promise; + // Throws away everything queued and disarms the batch timer, so nothing + // captured before this call can ever reach the network. Opting out uses + // this: a user who says stop mid-session is saying they do not want the + // activity they have already generated sent, which makes discarding the + // queue the honest reading of that intent and flushing it a betrayal. + discard(): void; }; // Fire-and-forget PostHog batch client. Never throws, never blocks the @@ -260,5 +266,12 @@ export function createTelemetry(options: CreateTelemetryOptions): Telemetry { ]); } - return { enabled, capture, flush }; + // A request already on the wire cannot be unsent, but nothing still held + // in memory follows it. + function discard(): void { + cancelTimer(); + queue.length = 0; + } + + return { enabled, capture, flush, discard }; } diff --git a/src/telemetry/singleton.ts b/src/telemetry/singleton.ts index 740aa3dbc..797c9bb2e 100644 --- a/src/telemetry/singleton.ts +++ b/src/telemetry/singleton.ts @@ -5,7 +5,12 @@ import type { Telemetry } from "./index.js"; // than threading it through every intermediate call site. Defaults to a // disabled no-op so any code path that runs before index.ts sets it (or in // tests) never throws. -let instance: Telemetry = { enabled: false, capture: () => {}, flush: async () => {} }; +let instance: Telemetry = { + enabled: false, + capture: () => {}, + flush: async () => {}, + discard: () => {}, +}; export function setTelemetry(telemetry: Telemetry): void { instance = telemetry; diff --git a/src/telemetry/toggle.ts b/src/telemetry/toggle.ts index 39544f2cb..31961022e 100644 --- a/src/telemetry/toggle.ts +++ b/src/telemetry/toggle.ts @@ -57,10 +57,14 @@ export function createTelemetryToggleHandler( return; } if (!enabled) { - // Opt-out must be immediate and absolute: swap the in-memory singleton - // synchronously, before any await, so no capture in flight during the - // persistence step below can land on a still-enabled instance, and so - // an unhandled rejection from disk I/O can never leave telemetry on. + // Opt-out must be immediate and absolute: discard whatever the outgoing + // instance has queued (dropping the singleton alone would leave its + // batch timer armed to send it anyway), then swap the in-memory + // singleton synchronously, before any await, so no capture in flight + // during the persistence step below can land on a still-enabled + // instance, and so an unhandled rejection from disk I/O can never + // leave telemetry on. + deps.getTelemetry().discard(); deps.setTelemetry( deps.createTelemetry({ settings: { providers: {}, telemetry: { enabled: false } } }), ); diff --git a/tests/unit/telemetry-toggle.test.ts b/tests/unit/telemetry-toggle.test.ts index b5e0926c5..ed9fc641a 100644 --- a/tests/unit/telemetry-toggle.test.ts +++ b/tests/unit/telemetry-toggle.test.ts @@ -56,6 +56,43 @@ test("capture called immediately after toggle-off makes zero fetch calls", () => expect(fetchCalls()).toBe(0); }); +// Opting out is a statement about activity already generated, not only about +// activity to come: events captured before the toggle must never be sent +// afterwards. Dropping the singleton is not enough on its own — the outgoing +// instance's batch timer would still fire and post its queue — so this test +// guards the explicit discard. If a future change makes opt-out flush what it +// was holding, this fails, and that is the point. +test("opting out discards events captured before the toggle instead of sending them", async () => { + let sends = 0; + const fetchFn = (() => { + sends++; + return Promise.resolve(new Response("1", { status: 200 })); + }) as unknown as typeof fetch; + const { deps, getInstance } = fakeDeps({ + createTelemetry: (opts) => + createTelemetry({ + ...opts, + env: opts.env ?? {}, + apiKey: opts.apiKey ?? "test-key", + fetchFn, + // Short enough that an undiscarded queue would reach the network well + // inside this test's wait, rather than passing by outrunning a timer. + batch: { intervalMs: 20 }, + }), + }); + const handler = createTelemetryToggleHandler("/fake/path", deps); + + handler(true); + await new Promise((resolve) => setTimeout(resolve, 10)); + getInstance().capture("cli_start"); + expect(sends).toBe(0); + + handler(false); + await new Promise((resolve) => setTimeout(resolve, 150)); + expect(getInstance().enabled).toBe(false); + expect(sends).toBe(0); +}); + test("save rejection leaves the singleton disabled with no unhandled rejection", async () => { const { deps, getInstance } = fakeDeps({ saveGlobalSettings: async () => { @@ -90,7 +127,12 @@ test("load failure skips persistence entirely and stays disabled in memory", asy test("toggle on while env-killed writes nothing and swaps no instance", async () => { let ensureCalled = false; let saveCalled = false; - const initial: Telemetry = { enabled: false, capture: () => {}, flush: async () => {} }; + const initial: Telemetry = { + enabled: false, + capture: () => {}, + flush: async () => {}, + discard: () => {}, + }; let setInstance: Telemetry | undefined; const { deps } = fakeDeps({ getTelemetry: () => initial, From 942de52ba3fbd6ec7246025de099b64dde586341 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 14:39:32 -0700 Subject: [PATCH 4/5] Expand the anonymous product event catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds slash-command, skill, plugin, sub-agent, permission, compaction, and crash events to the telemetry catalog. Every identifier these events would naturally carry is named by someone other than us — an MCP server key is a settings key, a skill is a directory in the repo, a plugin id and an agent profile are author-chosen — so each is mapped to a fixed first-party enum at the emission site and reported as "custom" when it matches nothing. Emission takes Telemetry as an injected dependency rather than reading the process-wide handle, so a module built without one is silent by construction. --- docs/TELEMETRY.md | 47 ++- src/agent/tools.ts | 7 +- src/agent/use-skill.ts | 11 +- src/exec/runner.ts | 5 + src/index.ts | 7 + src/permission/gate.ts | 38 ++- src/plugins/loader.ts | 20 +- src/session/runtime-assembly.ts | 33 ++- src/subagent/run.ts | 3 + src/subagent/task-tool.ts | 23 ++ src/telemetry/classify.ts | 97 ++++++ src/telemetry/index.ts | 47 ++- src/telemetry/singleton.ts | 23 +- src/tui/runner.ts | 13 +- tests/unit/telemetry-product-events.test.ts | 313 ++++++++++++++++++++ 15 files changed, 653 insertions(+), 34 deletions(-) create mode 100644 src/telemetry/classify.ts create mode 100644 tests/unit/telemetry-product-events.test.ts diff --git a/docs/TELEMETRY.md b/docs/TELEMETRY.md index d23cb9ce5..c86e4d95a 100644 --- a/docs/TELEMETRY.md +++ b/docs/TELEMETRY.md @@ -6,13 +6,26 @@ includes prompts, code, file contents, or paths. ## What's collected -Three events, each with a small set of properties: +Each event carries a small set of properties: | Event | When | Properties | |---|---|---| | `cli_start` | Once per used session (see First-run disclosure) | (none beyond common properties) | | `session_end` | When a TUI session finishes | `status`, `turn_count`, `duration_ms`, `session_mode`, `exit_reason` | | `inference_turn` | Once per completed turn | `provider_id`, `model_id`, `input_tokens`, `output_tokens`, `cache_read_tokens`, `cache_write_tokens`, `thinking_tokens`, `duration_ms` | +| `slash_command` | A slash command is dispatched in the TUI | `command_name` | +| `skill_used` | `use_skill` loads a skill that resolved | (none beyond common properties) | +| `plugin_loaded` | A plugin is discovered and loaded at startup | `origin` | +| `subagent_start` | A `task` dispatch begins | `agent_name` | +| `subagent_end` | A `task` dispatch finishes | `agent_name`, `status`, `duration_ms` | +| `permission_prompt` | An approval prompt is answered (or abandoned) | `decision`, `permission_kind` | +| `compaction` | The compactor actually folds turns away | `mode`, `duration_ms`, `turns_before`, `turns_after` | +| `crash` | A fatal error reaches the process-level handler | `kind`, `error_class` | +| `auth_failure` | A provider rejects the stored credentials | `error_class` | + +`compaction` is deliberately silent on the runs where the compactor decides +there is nothing to compact — an event that also fires on no-ops makes its own +duration and turn-count averages meaningless. Common properties attached to every event: a random installation UUID (`distinct_id`), `session_id`, `service_version`, `os_type`, `os_arch`, and a @@ -30,10 +43,38 @@ onboarding or settings. `model_id` is the model identifier exactly as configured — it is the one user-entered string that is sent, so do not put anything identifying in a model name. +## Names are never sent, only categories + +Most of the things a usage event would naturally want to name are named by +someone other than us: an MCP server key is a key in your settings, a skill is +a directory in your repo, a plugin id is chosen by its author, an agent profile +and a plugin's slash commands are project-local. On a private repo those names +are your employer, your internal services, or fragments of your paths. + +So none of them are transmitted. Each is matched against a fixed list of names +this project itself ships and reported as that name, or as `custom` when it +matches nothing — with `mcp` as its own bucket for `permission_kind`, so the +share of prompts driven by MCP stays visible without the server key coming +with it. `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`). + +`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 +application or plugin code is as author-chosen as any other string. + +The mapping is `src/telemetry/classify.ts`, and the tests that feed each +emission site a deliberately identifying name and assert it reaches no part of +the payload are in `tests/unit/telemetry-product-events.test.ts`. + ## What's never collected - Prompts, model output, or any conversation content - 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) +- Shell commands, tool arguments, or tool results - API keys, tokens, or any other credential - Anything not in the allowlist above @@ -123,5 +164,5 @@ in force. Local performance tracing and optional OpenTelemetry export to an operator-owned collector (Phoenix, PostHog OTEL, Jaeger, generic OTLP) are documented in -`docs/PERFTRACE.md`. That pipe is separate: it does not expand these three -events, and product telemetry opt-out does not control OTEL export. +`docs/PERFTRACE.md`. That pipe is separate: it does not expand the events +above, and product telemetry opt-out does not control OTEL export. diff --git a/src/agent/tools.ts b/src/agent/tools.ts index e41377df4..f04a92e9a 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -16,6 +16,7 @@ import { type ShellTimeoutConfig, } from "../plugins/shell-guard-plugin.js"; import { advertiseEditFileLineRange } from "../plugins/edit-file-line-range.js"; +import type { Telemetry } from "../telemetry/index.js"; import type { PermissionGate } from "../permission/gate.js"; import { buildCorePosixToolPlugins } from "./posix-tool-plugins.js"; import { createLazyBlobReader } from "./lazy-blob-reader.js"; @@ -112,6 +113,9 @@ export type AgentToolsetArgs = { // Real sessions always pass their detected values — see tool-search.ts for // why these must be fixed for the session's life. toolAvailability?: ToolAvailability; + // Records skill loads and sub-agent dispatch. Omitted (tests, ad-hoc + // toolsets) means those events are never emitted. + telemetry?: Telemetry; // When provided, the agent gets a `task` tool that delegates to autonomous // sub-agents. Omitted in contexts that cannot spawn sub-agents (e.g. tests). subAgent?: { @@ -210,7 +214,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise): Promise => { @@ -34,6 +39,10 @@ export function createUseSkillTool(cwd: string, skillDirs: string[] = []): Agent if (name.length === 0) return "Error: use_skill requires a non-empty name."; 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"); return `Skill "${name}" — follow these instructions for this task:\n\n${body}`; }, }); diff --git a/src/exec/runner.ts b/src/exec/runner.ts index e8db9e3a9..08a1a5bf5 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -53,6 +53,7 @@ import type { PermissionRequest, } from "../permission/types.js"; import { createAgentToolset, type AgentToolset, type OperatorResult } from "../agent/tools.js"; +import { liveTelemetry } from "../telemetry/singleton.js"; import { collectToolPlugins, resolveToolPlugins } from "../plugins/tool-plugins.js"; import { expandExistingPluginMembers, @@ -262,6 +263,7 @@ export async function runExec(config: Config): Promise { isProjectPluginTrusted, isRegisteredPathTrusted, diagnostics: pluginLoadDiag, + telemetry: liveTelemetry, }); emitPluginWarningSummary(pluginLoadDiag, (line) => logger.warn(line)); // Metadata-only (untrusted) modules stay out of executable plugins. @@ -298,6 +300,7 @@ export async function runExec(config: Config): Promise { const permissionGate = createPermissionGate({ approvals: seededApprovals, + telemetry: liveTelemetry, cwd: config.cwd, rootsProvider: createWorktreeRootsProvider(config.cwd), providerName: config.providerName, @@ -326,6 +329,7 @@ export async function runExec(config: Config): Promise { cwd: config.cwd, permissionGate, skillDirs, + telemetry: liveTelemetry, ...(shellTimeout !== undefined ? { shellTimeout } : {}), ...(toolWatchdog !== undefined ? { toolWatchdog } : {}), ...(localSettingsForMode?.env !== undefined ? { shellEnv: localSettingsForMode.env } : {}), @@ -544,6 +548,7 @@ export async function runExec(config: Config): Promise { "pruning-compactor": createSessionPruningCompactor({ compactionMode: liveCompactionMode, summarize: summarizeForCompaction, + telemetry: liveTelemetry, }), }, }); diff --git a/src/index.ts b/src/index.ts index 35106d3d6..123f399a1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,6 +9,7 @@ import { ensureTelemetrySettings, globalSettingsPath } from "./config/settings.j import { installFileLogSink } from "./logging/sink.js"; import { flushPerfToOtel } from "./perf/index.js"; import { createTelemetry, telemetryDisabledByEnv } from "./telemetry/index.js"; +import { classifyErrorClass } from "./telemetry/classify.js"; import { getTelemetry, setTelemetry } from "./telemetry/singleton.js"; import { runExec } from "./exec/runner.js"; import { runOnboarding } from "./tui/onboarding.js"; @@ -154,6 +155,12 @@ export async function handleFatal(kind: CrashKind, error: unknown): Promise boolean) => void; + // Records that a prompt was shown and how it was answered. Injected rather + // than read from the process-wide handle so a gate built without one is + // silent by construction. + telemetry?: Telemetry; }; export type PermissionGate = { @@ -246,6 +269,7 @@ export type PermissionGate = { export function createPermissionGate(options: PermissionGateOptions): PermissionGate { const { requestApproval, persist, interactive, skipPermissions, providerName, model, cwd } = options; + const telemetry = options.telemetry ?? NOOP_TELEMETRY; const mcpTiers = options.mcpTiers ?? createMcpToolPermissionRegistry(); const resolvedCwd = cwd ?? process.cwd(); const rootsProvider = options.rootsProvider ?? createWorktreeRootsProvider(resolvedCwd); @@ -463,12 +487,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission try { outcome = await requestApproval(requestForOperator); } finally { - end( - waitSpanId, - outcome !== undefined - ? { decision: outcome.allow ? "allow" : "deny" } - : undefined, - ); + finishApprovalWait(telemetry, waitSpanId, request.tool, outcome); } if (outcome === undefined || !outcome.allow) { const suffix = @@ -516,12 +535,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission try { outcome = await requestApproval(request); } finally { - end( - waitSpanId, - outcome !== undefined - ? { decision: outcome.allow ? "allow" : "deny" } - : undefined, - ); + finishApprovalWait(telemetry, waitSpanId, request.tool, outcome); } if (outcome === undefined || !outcome.allow) { const suffix = diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index 6e4a5b988..3d5f463f1 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -7,6 +7,7 @@ import { SETTINGS_DIR_NAME } from "../branding.js"; import type { CommandPlugin } from "../tui/commands/registry.js"; import { pathIsInsideOrEqual } from "../util/path-contain.js"; import { parsePluginManifest, type PluginManifest } from "./manifest.js"; +import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js"; import { loadDataOnlyPlugin } from "./data-only.js"; import { resolvePluginWarningHandler, @@ -114,6 +115,7 @@ export async function loadPluginEntry( onWarning?: (msg: string) => void; diagnostics?: PluginLoadDiagnostics; origin?: PluginOrigin; + telemetry?: Telemetry; } = {}, ): Promise { const cwd = opts.cwd ?? process.cwd(); @@ -127,6 +129,7 @@ export async function loadPluginEntry( : { onWarning: stderrPluginWarning }, ); const origin = opts.origin; + const telemetry = opts.telemetry ?? NOOP_TELEMETRY; let target = entryPath; let pluginDir = entryPath; try { @@ -163,6 +166,7 @@ export async function loadPluginEntry( mod.origin = origin; mod.pluginPath = resolve(entryPath); } + if (origin !== undefined) telemetry.capture("plugin_loaded", { origin }); return mod; } return null; @@ -209,6 +213,7 @@ export async function loadPluginEntry( result.origin = origin; result.pluginPath = resolve(pluginDir); } + if (origin !== undefined) telemetry.capture("plugin_loaded", { origin }); return result; } catch (err) { // Route through the same sink as skill/load warnings so a diagnostics @@ -494,6 +499,7 @@ async function scanPluginsDir( origin: PluginOrigin, isTrusted?: (pluginPath: string) => boolean, diagnostics?: PluginLoadDiagnostics, + telemetry?: Telemetry, ): Promise { let entries: string[]; try { @@ -519,6 +525,7 @@ async function scanPluginsDir( cwd, origin, ...(diagnostics !== undefined ? { diagnostics } : {}), + ...(telemetry !== undefined ? { telemetry } : {}), }); if (plugin !== null) results.push(plugin); } @@ -537,13 +544,14 @@ export async function discoverUserPlugins( opts: { isPluginTrusted?: (pluginPath: string) => boolean; diagnostics?: PluginLoadDiagnostics; + telemetry?: Telemetry; } = {}, ): Promise { const projectDir = join(cwd, SETTINGS_DIR_NAME, "plugins"); const userDir = join(homedir(), SETTINGS_DIR_NAME, "plugins"); const [project, user] = await Promise.all([ - scanPluginsDir(projectDir, cwd, "project", opts.isPluginTrusted, opts.diagnostics), - scanPluginsDir(userDir, cwd, "user", undefined, opts.diagnostics), + scanPluginsDir(projectDir, cwd, "project", opts.isPluginTrusted, opts.diagnostics, opts.telemetry), + scanPluginsDir(userDir, cwd, "user", undefined, opts.diagnostics, opts.telemetry), ]); return [...project, ...user]; } @@ -584,6 +592,7 @@ export async function loadPluginsFromPaths( opts: { isPluginTrusted?: (pluginPath: string) => boolean; diagnostics?: PluginLoadDiagnostics; + telemetry?: Telemetry; } = {}, ): Promise { // A skipped member routes into `diagnostics` when the caller has one, same @@ -613,6 +622,7 @@ export async function loadPluginsFromPaths( cwd, origin: "path", ...(opts.diagnostics !== undefined ? { diagnostics: opts.diagnostics } : {}), + ...(opts.telemetry !== undefined ? { telemetry: opts.telemetry } : {}), }); }), ); @@ -626,11 +636,11 @@ export async function loadPluginsFromPaths( // different working directory. Product-shipped plugins are auto-trusted. export async function discoverRepoPlugins( cwd: string, - opts: { diagnostics?: PluginLoadDiagnostics } = {}, + opts: { diagnostics?: PluginLoadDiagnostics; telemetry?: Telemetry } = {}, ): Promise { const repoRoot = new URL("../../", import.meta.url).pathname; const pluginsDir = join(repoRoot, "plugins"); - return scanPluginsDir(pluginsDir, cwd, "repo", undefined, opts.diagnostics); + return scanPluginsDir(pluginsDir, cwd, "repo", undefined, opts.diagnostics, opts.telemetry); } // Claude Code records marketplace installs in @@ -650,6 +660,7 @@ export async function discoverClaudeInstalledPlugins( home?: string; onExpandSkip?: (skip: ExpandPluginPathSkip) => void; diagnostics?: PluginLoadDiagnostics; + telemetry?: Telemetry; } = {}, ): Promise { const home = opts.home ?? homedir(); @@ -769,6 +780,7 @@ export async function discoverClaudeInstalledPlugins( : plugin.manifest.name, }; } + opts.telemetry?.capture("plugin_loaded", { origin: "user" }); results.push(plugin); } } diff --git a/src/session/runtime-assembly.ts b/src/session/runtime-assembly.ts index 8dfa3e332..e0d65bf18 100644 --- a/src/session/runtime-assembly.ts +++ b/src/session/runtime-assembly.ts @@ -40,6 +40,7 @@ import type { Approval, GrantScope } from "../permission/types.js"; import type { ReasoningEffort } from "../provider/reasoning-effort.js"; import type { SubAgentProvider } from "../subagent/index.js"; import { COMPACTOR_KEEP_RECENT_TURNS, createPruningCompactor } from "./compactor.js"; +import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js"; // --------------------------------------------------------------------------- // 1. Sub-agent provider literal @@ -122,14 +123,17 @@ export type DiscoverSessionPluginsArgs = { isRegisteredPathTrusted: (pluginPath: string) => boolean; /** When set, skill/load warnings collect here for one end-of-batch summary. */ diagnostics?: PluginLoadDiagnostics; + telemetry?: Telemetry; }; /** Discover + dedupe plugins from repo, user, optional Claude, and registered paths. */ export async function discoverSessionPlugins( args: DiscoverSessionPluginsArgs, ): Promise { - const diag = - args.diagnostics !== undefined ? { diagnostics: args.diagnostics } : {}; + const diag = { + ...(args.diagnostics !== undefined ? { diagnostics: args.diagnostics } : {}), + ...(args.telemetry !== undefined ? { telemetry: args.telemetry } : {}), + }; const claudePlugins = args.discoverClaudePlugins === true ? await discoverClaudeInstalledPlugins(args.cwd, diag) @@ -246,15 +250,38 @@ const SESSION_COMPACTOR_SUMMARY_MAX_CHARS = 2500; export type SessionPruningCompactorArgs = { compactionMode: "llm" | "pruning"; summarize: (turns: ConversationTurn[]) => Promise; + telemetry?: Telemetry; }; /** Shared pruning-compactor defaults for the main session agent. */ export function createSessionPruningCompactor( args: SessionPruningCompactorArgs, ): Compactor { - return createPruningCompactor({ + const compactor = createPruningCompactor({ keepRecentTurns: COMPACTOR_KEEP_RECENT_TURNS, summaryMaxChars: SESSION_COMPACTOR_SUMMARY_MAX_CHARS, ...(args.compactionMode !== "pruning" ? { summarize: args.summarize } : {}), }); + const telemetry = args.telemetry ?? NOOP_TELEMETRY; + return { + ...compactor, + async apply(turns, ctx) { + const turnsBefore = turns.length; + const startedAt = Date.now(); + const result = await compactor.apply(turns, ctx); + // summarizedTurnCount is only set on the branch that actually folded + // turns away. The other branch is a no-op (or image aging alone), and + // reporting it as compaction would drag the duration and turn-count + // averages toward the runs where nothing happened. + if (result.record.decisions.summarizedTurnCount !== undefined) { + telemetry.capture("compaction", { + mode: args.compactionMode, + duration_ms: Date.now() - startedAt, + turns_before: turnsBefore, + turns_after: result.output.length, + }); + } + return result; + }, + }; } diff --git a/src/subagent/run.ts b/src/subagent/run.ts index c2e279cb8..7c8dc48a0 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -3,6 +3,8 @@ */ import { mkdir } from "node:fs/promises"; + +import { liveTelemetry } from "../telemetry/singleton.js"; import { join } from "node:path"; import { @@ -326,6 +328,7 @@ export async function runSubAgent(params: RunSubAgentParams): Promise { // Pass the public entry so nested workers still go through the outer // slot/refresh path; avoids task-tool importing runSubAgent (cycle). run: runSubAgent, + telemetry: liveTelemetry, ...(nd.onEvent !== undefined ? { onEvent: nd.onEvent } : {}), ...(nd.onProgress !== undefined ? { onProgress: nd.onProgress } : {}), ...(nd.sessions !== undefined ? { sessions: nd.sessions } : {}), diff --git a/src/subagent/task-tool.ts b/src/subagent/task-tool.ts index 0a481f28d..b9f2df055 100644 --- a/src/subagent/task-tool.ts +++ b/src/subagent/task-tool.ts @@ -43,6 +43,8 @@ import { cleanupSubAgentWorktree, createSubAgentWorktree, WorktreeError } from " import { generateSessionId } from "../session/index.js"; import { end, start } from "../perf/index.js"; import { currentTurnId } from "../perf/reactor-spans.js"; +import { classifyAgentName } from "../telemetry/classify.js"; +import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js"; import { join } from "node:path"; import type { NestedDispatchDeps, @@ -171,6 +173,9 @@ export type TaskToolDeps = SubAgentSandboxDeps & { * fails. Omit (default) to keep today's shared-cwd dispatch. */ useWorktree?: boolean; + // Records sub-agent starts and outcomes. Injected so the tool has no + // process-wide dependency; omitting it makes dispatch silent. + telemetry?: Telemetry; }; function taskToolResult(callId: string, content: string): ToolResult { @@ -180,6 +185,7 @@ function taskToolResult(callId: string, content: string): ToolResult { export function createTaskTool(deps: TaskToolDeps): AgentTool { const run = deps.run; + const telemetry = deps.telemetry ?? NOOP_TELEMETRY; // Session-scoped re-dispatch ledger: one per parent task tool instance. const briefLedger = createBriefDispatchLedger(); return tool({ @@ -484,6 +490,15 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { ...(turnId !== null && turnId.length > 0 ? { turn_id: turnId } : {}), }, }); + const subagentStartedAt = Date.now(); + // Profile ids come from project and plugin directories, so only the + // runtime's own "worker" fallback is reportable by name; anything else + // is bucketed. Sub-agents run in this process against the same session + // id, so there is no parent id worth sending — it would always equal + // the session_id already on the payload. + const agentName = classifyAgentName(agentLabel); + telemetry.capture("subagent_start", { agent_name: agentName }); + let subagentStatus: "completed" | "cancelled" | "failed" = "completed"; try { if (deps.useWorktree === true) { const worktreePath = join(deps.getWorkdirBase(), "worktrees", generateSessionId()); @@ -562,6 +577,7 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { turnBudgetStopAfterDispatches: TURN_BUDGET_STOP_AFTER_DISPATCHES, }; if (wasCancelled) { + subagentStatus = "cancelled"; if ( session !== undefined && deps.sessions?.get(session.id)?.status === "running" @@ -585,6 +601,7 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { (session !== undefined && deps.sessions?.get(session.id)?.status === "cancelled") ) { + subagentStatus = "cancelled"; briefLedger.recordOutcome(fingerprint, "cancelled"); if ( session !== undefined && @@ -594,6 +611,7 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { } return await finishWithWorktree(taskToolResult(call.id, cancelledSubAgentMessage(description))); } + subagentStatus = "failed"; // Run never produced a body — undo the admit so turn-budget retry budget // is not burned by auth/provider crashes. briefLedger.release(fingerprint); @@ -613,6 +631,11 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { } finally { end(subagentSpanId); + telemetry.capture("subagent_end", { + agent_name: agentName, + status: subagentStatus, + duration_ms: Date.now() - subagentStartedAt, + }); } }, }); diff --git a/src/telemetry/classify.ts b/src/telemetry/classify.ts new file mode 100644 index 000000000..ad39d4b0e --- /dev/null +++ b/src/telemetry/classify.ts @@ -0,0 +1,97 @@ +// Every identifier a product event would like to carry originates somewhere a +// user, a project, an MCP server, or a plugin author can name: an MCP server +// key is a settings key, a skill is a directory under the repo, a plugin id is +// author-chosen, an agent profile is project-local, a slash command can be +// registered by a plugin. On a private repo those names are the employer, an +// internal service, or a path fragment. +// +// So none of them are transmitted. Each is matched against a fixed list of +// names this repo itself ships and reported as that name, or as "custom" when +// it matches nothing. What leaves the process is a first-party enum: the fact +// that something unrecognised was used, never what it was called. + +import { isMcpToolName } from "../mcp/tool-name.js"; + +const CUSTOM = "custom"; + +// Built-in tool ids the gate can raise an approval prompt for. Deliberately +// spelled out here rather than derived from the advertised-tools list, which +// exists to keep the provider cache prefix stable and would silently widen +// this allowlist the day it starts including registered MCP or plugin tools. +const BUILT_IN_TOOL_NAMES: ReadonlySet = new Set([ + "ask_operator", + "delete_file", + "edit_file", + "grep", + "list_dir", + "lsp", + "manage_tasks", + "present", + "read_file", + "run_shell", + "search_agents", + "search_files", + "task", + "tool_search", + "use_skill", + "web_fetch", + "web_search", + "write_file", +]); + +// Slash commands registered by src/tui/commands/built-in.ts. Plugins register +// into the same registry, so an unlisted name is plugin-authored. +const BUILT_IN_COMMAND_NAMES: ReadonlySet = new Set([ + "changelog", + "clear", + "cost", + "goal", + "help", + "hooks", + "mcp", + "model", + "new", + "paste-image", + "permissions", + "plugins", + "rename", + "settings", +]); + +// The one agent label the runtime supplies itself; every other profile id +// comes from a project or plugin directory. +const BUILT_IN_AGENT_NAME = "worker"; + +// 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([ + "AggregateError", + "Error", + "EvalError", + "RangeError", + "ReferenceError", + "SyntaxError", + "TypeError", + "URIError", +]); + +// MCP tools collapse to a single bucket rather than "custom" so the share of +// prompts driven by MCP stays legible without the server key coming with it. +export function classifyPermissionKind(toolName: string): string { + if (BUILT_IN_TOOL_NAMES.has(toolName)) return toolName; + if (isMcpToolName(toolName)) return "mcp"; + return CUSTOM; +} + +export function classifyCommandName(commandName: string): string { + return BUILT_IN_COMMAND_NAMES.has(commandName) ? commandName : CUSTOM; +} + +export function classifyAgentName(agentName: string): string { + return agentName === BUILT_IN_AGENT_NAME ? BUILT_IN_AGENT_NAME : CUSTOM; +} + +export function classifyErrorClass(error: unknown): string { + if (!(error instanceof Error)) return "non_error"; + return STANDARD_ERROR_NAMES.has(error.constructor.name) ? error.constructor.name : CUSTOM; +} diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index 406f9e5d8..436106962 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -42,7 +42,19 @@ export type BatchTuning = { export const TELEMETRY_NOTICE = "Anonymous usage telemetry is enabled (no prompts, code, or paths collected). Disable in /settings > Telemetry. Docs: docs/TELEMETRY.md"; -export type TelemetryEvent = "cli_start" | "session_end" | "inference_turn"; +export type TelemetryEvent = + | "cli_start" + | "session_end" + | "inference_turn" + | "slash_command" + | "skill_used" + | "plugin_loaded" + | "subagent_start" + | "subagent_end" + | "permission_prompt" + | "compaction" + | "crash" + | "auth_failure"; // One id per interactive process (TUI session or CLI invocation), generated // once at module load and reused by every createTelemetry() instance for the @@ -73,8 +85,27 @@ const EVENT_PROPERTY_ALLOWLIST: Record = { "thinking_tokens", "duration_ms", ], + // Every identifier below is a first-party enum produced by + // src/telemetry/classify.ts, not the name the user or author wrote. The + // 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: [], + // origin is the discovery tier (repo/user/project/path); the manifest id is + // author-chosen free text and is not sent. + plugin_loaded: ["origin"], + subagent_start: ["agent_name"], + subagent_end: ["agent_name", "status", "duration_ms"], + permission_prompt: ["decision", "permission_kind"], + compaction: ["mode", "duration_ms", "turns_before", "turns_after"], + crash: ["kind", "error_class"], + auth_failure: ["error_class"], }; +const KNOWN_EVENTS: ReadonlySet = new Set(Object.keys(EVENT_PROPERTY_ALLOWLIST)); + const FALSY_ENV_FLAG_VALUES = new Set(["", "0", "false", "off", "no"]); // Trimmed so .env files and shell scripts that produce " 0" or "false\n" @@ -154,6 +185,18 @@ export type Telemetry = { discard(): void; }; +// Stand-in for callers that were constructed without a telemetry handle — +// tests, and any code path that runs before startup has built the real one. +// Modules take Telemetry as an injected dependency rather than reaching for a +// global, and this is what makes "not injected" mean "emits nothing" instead +// of "throws". +export const NOOP_TELEMETRY: Telemetry = { + enabled: false, + capture: () => {}, + flush: async () => {}, + discard: () => {}, +}; + // Fire-and-forget PostHog batch client. Never throws, never blocks the // caller — errors (including timeouts) are swallowed silently since // telemetry must never affect product behavior. @@ -218,7 +261,7 @@ export function createTelemetry(options: CreateTelemetryOptions): Telemetry { function capture(event: TelemetryEvent, properties?: Record): void { if (!enabled) return; - if (!(event === "cli_start" || event === "session_end" || event === "inference_turn")) return; + if (!KNOWN_EVENTS.has(event)) return; queue.push({ event, diff --git a/src/telemetry/singleton.ts b/src/telemetry/singleton.ts index 797c9bb2e..dc1fce6e2 100644 --- a/src/telemetry/singleton.ts +++ b/src/telemetry/singleton.ts @@ -1,16 +1,11 @@ -import type { Telemetry } from "./index.js"; +import { NOOP_TELEMETRY, type Telemetry } from "./index.js"; // Process-wide telemetry handle. index.ts constructs the real instance once // at startup; runner.ts and the /settings Telemetry tab read it from here rather // than threading it through every intermediate call site. Defaults to a // disabled no-op so any code path that runs before index.ts sets it (or in // tests) never throws. -let instance: Telemetry = { - enabled: false, - capture: () => {}, - flush: async () => {}, - discard: () => {}, -}; +let instance: Telemetry = NOOP_TELEMETRY; export function setTelemetry(telemetry: Telemetry): void { instance = telemetry; @@ -19,3 +14,17 @@ export function setTelemetry(telemetry: Telemetry): void { export function getTelemetry(): Telemetry { return instance; } + +// A stable handle to whatever the current instance is. Modules that take +// Telemetry as a constructor dependency hold this rather than the instance +// itself: the /settings toggle replaces the underlying client on enable and +// disable, and a captured instance would keep emitting into (or staying +// silent in) the client that existed at startup. +export const liveTelemetry: Telemetry = { + get enabled() { + return instance.enabled; + }, + capture: (event, properties) => instance.capture(event, properties), + flush: () => instance.flush(), + discard: () => instance.discard(), +}; diff --git a/src/tui/runner.ts b/src/tui/runner.ts index f6cfe105c..dc4a4af72 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -95,7 +95,8 @@ import { registerBuiltInCommands } from "./commands/built-in.js"; import type { PluginModule } from "../plugins/loader.js"; import { activateHeldTelemetry, telemetryFirstRunPending } from "../telemetry/first-run.js"; import { TELEMETRY_NOTICE } from "../telemetry/index.js"; -import { getTelemetry, setTelemetry } from "../telemetry/singleton.js"; +import { classifyCommandName } from "../telemetry/classify.js"; +import { getTelemetry, liveTelemetry, setTelemetry } from "../telemetry/singleton.js"; import { createTelemetryToggleHandler } from "../telemetry/toggle.js"; import { loadStartupChangelogMarkdown } from "../changelog/index.js"; import pkg from "../../package.json" with { type: "json" }; @@ -421,6 +422,7 @@ export async function runTUI(initialConfig: Config): Promise { isProjectPluginTrusted, isRegisteredPathTrusted, diagnostics: pluginLoadDiag, + telemetry: liveTelemetry, }); emitPluginWarningLog(pluginLoadDiag); // Fire-and-forget startup diagnostics (this + tool-plugin resolution below) @@ -652,6 +654,7 @@ export async function runTUI(initialConfig: Config): Promise { const seededApprovals = await loadSeededApprovals(config.cwd, sessionId); const permissionGate = createPermissionGate({ approvals: seededApprovals, + telemetry: liveTelemetry, cwd: config.cwd, rootsProvider: createWorktreeRootsProvider(config.cwd), providerName: config.providerName, @@ -1050,6 +1053,7 @@ export async function runTUI(initialConfig: Config): Promise { cwd: config.cwd, permissionGate, skillDirs, + telemetry: liveTelemetry, ...(shellTimeout !== undefined ? { shellTimeout } : {}), ...(localSettingsForMode?.env !== undefined ? { shellEnv: localSettingsForMode.env } : {}), toolWatchdog: liveToolWatchdog, @@ -1318,6 +1322,7 @@ export async function runTUI(initialConfig: Config): Promise { "pruning-compactor": createSessionPruningCompactor({ compactionMode: liveCompactionMode, summarize: summarizeForCompaction, + telemetry: liveTelemetry, }), }, }); @@ -1767,6 +1772,9 @@ export async function runTUI(initialConfig: Config): Promise { isCodexAuthError, isXaiAuthError, ); + if (kind === "codex_auth" || kind === "xai_auth") { + getTelemetry().capture("auth_failure", { error_class: kind }); + } if (!shouldSettleUiAfterSendFailure(kind)) return; recordRunError(err); systemNotice(err instanceof Error ? err.message : String(err)); @@ -1876,6 +1884,9 @@ export async function runTUI(initialConfig: Config): Promise { systemNotice(`Unknown command: ${name}`); return; } + // Plugins register into the same command registry as the built-ins, so an + // unrecognised name is plugin-authored and is bucketed rather than sent. + getTelemetry().capture("slash_command", { command_name: classifyCommandName(command.name) }); applyCommandResult(command.handler(args, commandContext)); }; diff --git a/tests/unit/telemetry-product-events.test.ts b/tests/unit/telemetry-product-events.test.ts new file mode 100644 index 000000000..8a83e5ff4 --- /dev/null +++ b/tests/unit/telemetry-product-events.test.ts @@ -0,0 +1,313 @@ +// Each test here feeds an emission site a name that identifies an employer, a +// service, or a path, then asserts that string appears NOWHERE in the bytes +// that would go to PostHog. Serializing the whole request body (not just the +// property the site meant to set) is the point: a comment claiming a value is +// a safe enum is not evidence, and a leak smuggled in under a different key +// would pass a property-by-property check. + +import { afterEach, expect, test } from "bun:test"; +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { createUseSkillTool } from "../../src/agent/use-skill.js"; +import type { Settings } from "../../src/config/settings.js"; +import { createPermissionGate } from "../../src/permission/gate.js"; +import { loadPluginEntry } from "../../src/plugins/loader.js"; +import { createSessionPruningCompactor } from "../../src/session/runtime-assembly.js"; +import { createTaskTool } from "../../src/subagent/task-tool.js"; +import { + classifyAgentName, + classifyCommandName, + classifyErrorClass, + classifyPermissionKind, +} from "../../src/telemetry/classify.js"; +import { createTelemetry, type Telemetry } from "../../src/telemetry/index.js"; + +type BatchBody = { + batch: { event: string; properties: Record }[]; +}; + +function harness(): { telemetry: Telemetry; wire: () => Promise; events: () => Promise } { + const bodies: BatchBody[] = []; + const fetchFn = ((_url: string, init: RequestInit) => { + bodies.push(JSON.parse(init.body as string) as BatchBody); + return Promise.resolve(new Response("1", { status: 200 })); + }) as unknown as typeof fetch; + const settings: Settings = { providers: {}, telemetry: { installationId: "install-id" } }; + const telemetry = createTelemetry({ settings, env: {}, fetchFn, apiKey: "test-key" }); + const wire = async (): Promise => { + await telemetry.flush(); + return JSON.stringify(bodies); + }; + return { + telemetry, + wire, + events: async () => { + await telemetry.flush(); + return bodies.flatMap((body) => body.batch); + }, + }; +} + +const tempDirs: string[] = []; + +afterEach(async () => { + while (tempDirs.length > 0) { + await rm(tempDirs.pop()!, { recursive: true, force: true }); + } +}); + +async function tempDir(prefix: string): Promise { + const dir = await mkdtemp(join(tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +// --------------------------------------------------------------------------- +// 1. permission_kind — an MCP tool id embeds the server key from settings +// --------------------------------------------------------------------------- + +test("permission_prompt buckets an MCP tool to \"mcp\" and never ships the server key", async () => { + const { telemetry, wire, events } = harness(); + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + requestApproval: async () => ({ allow: true }), + telemetry, + }); + + const verdict = await gate.evaluate({ + id: "call-1", + name: "mcp__acme-internal__deploy", + arguments: {}, + }); + + expect(verdict.allowed).toBe(true); + const [event] = await events(); + expect(event?.event).toBe("permission_prompt"); + expect(event?.properties.permission_kind).toBe("mcp"); + expect(event?.properties.decision).toBe("allow"); + expect(await wire()).not.toContain("acme-internal"); +}); + +test("permission_prompt buckets an unrecognised tool id to \"custom\"", async () => { + const { telemetry, wire, events } = harness(); + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + requestApproval: async () => ({ allow: false }), + telemetry, + }); + + await gate.evaluate({ id: "call-1", name: "acmecorp_payroll_export", arguments: {} }); + + const [event] = await events(); + expect(event?.properties.permission_kind).toBe("custom"); + expect(event?.properties.decision).toBe("deny"); + expect(await wire()).not.toContain("acmecorp"); +}); + +test("permission_prompt reports built-in tool ids by name", () => { + expect(classifyPermissionKind("run_shell")).toBe("run_shell"); + expect(classifyPermissionKind("edit_file")).toBe("edit_file"); +}); + +// --------------------------------------------------------------------------- +// 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 () => { + const { telemetry, wire, events } = harness(); + const cwd = await tempDir("corbits-skill-"); + const skillDir = join(cwd, ".agents", "skills", "acme-internal-deploy"); + await mkdir(skillDir, { recursive: true }); + await writeFile( + join(skillDir, "SKILL.md"), + "---\nname: acme-internal-deploy\n---\n\nDeploy the internal service.\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: "acme-internal-deploy" }); + + // Guard against the test passing because resolution failed: the event only + // fires on a resolved skill, so a silent miss would trivially "not 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(await wire()).not.toContain("acme-internal"); +}); + +// --------------------------------------------------------------------------- +// 3. plugin_id — an author-chosen manifest id on a private local plugin +// --------------------------------------------------------------------------- + +test("plugin_loaded carries only the discovery origin, never the manifest id", async () => { + const { telemetry, wire, events } = harness(); + const root = await tempDir("corbits-plugin-"); + const pluginDir = join(root, "plugin"); + await mkdir(join(pluginDir, "commands"), { recursive: true }); + await writeFile( + join(pluginDir, "plugin.json"), + JSON.stringify({ id: "acmecorp/internal-tools", name: "acmecorp internal tools", version: "1.0.0" }), + ); + await writeFile(join(pluginDir, "commands", "ship.md"), "---\ndescription: ship it\n---\n\nShip.\n"); + + const mod = await loadPluginEntry(pluginDir, { cwd: root, origin: "project", telemetry }); + + expect(mod).not.toBeNull(); + const [event] = await events(); + expect(event?.event).toBe("plugin_loaded"); + expect(event?.properties.origin).toBe("project"); + expect(event?.properties.plugin_id).toBeUndefined(); + expect(await wire()).not.toContain("acmecorp"); +}); + +// --------------------------------------------------------------------------- +// 4. agent_name — agent profiles are user-definable per project +// --------------------------------------------------------------------------- + +test("subagent events bucket a project-defined profile id to \"custom\"", async () => { + const { telemetry, wire, events } = harness(); + const cwd = await tempDir("corbits-agent-"); + const gate = createPermissionGate({ approvals: [], interactive: false, skipPermissions: true }); + + const tool = createTaskTool({ + cwd, + getWorkdirBase: () => cwd, + permissionGate: gate, + provider: { providerName: "test-provider", baseURL: "http://localhost", model: "test-model" }, + profiles: [{ id: "acmecorp-release-captain", description: "release", prompt: "release" }], + run: async () => "done", + telemetry, + }); + if (tool.kind !== "full") throw new Error(`expected full tool, got ${tool.kind}`); + await tool.handler( + { + id: "call-1", + name: "task", + arguments: { description: "Ship", prompt: "Ship it", agent: "acmecorp-release-captain" }, + }, + new AbortController().signal, + ); + + const captured = await events(); + const names = captured.map((e) => e.event); + expect(names).toContain("subagent_start"); + expect(names).toContain("subagent_end"); + for (const event of captured) { + expect(event.properties.agent_name).toBe("custom"); + // Sub-agents run in this process on the same session id, so a parent id + // would only ever restate session_id. + expect(event.properties.parent_session_id).toBeUndefined(); + } + expect(await wire()).not.toContain("acmecorp"); +}); + +test("the built-in worker label is reported by name", () => { + expect(classifyAgentName("worker")).toBe("worker"); + expect(classifyAgentName("acmecorp-release-captain")).toBe("custom"); +}); + +// --------------------------------------------------------------------------- +// 5. command_name — plugins register into the same slash-command registry +// --------------------------------------------------------------------------- + +test("slash_command buckets a plugin-registered command to \"custom\"", async () => { + const { telemetry, wire, events } = harness(); + + telemetry.capture("slash_command", { command_name: classifyCommandName("acmecorp-deploy") }); + telemetry.capture("slash_command", { command_name: classifyCommandName("settings") }); + + const captured = await events(); + expect(captured[0]?.properties.command_name).toBe("custom"); + expect(captured[1]?.properties.command_name).toBe("settings"); + expect(await wire()).not.toContain("acmecorp"); +}); + +// --------------------------------------------------------------------------- +// crash — an application or plugin error subclass is author-chosen text +// --------------------------------------------------------------------------- + +test("crash reports language error types by name and buckets everything else", async () => { + const { telemetry, wire, events } = harness(); + + class AcmeCorpVaultError extends Error {} + telemetry.capture("crash", { + kind: "uncaughtException", + error_class: classifyErrorClass(new AcmeCorpVaultError("boom")), + }); + telemetry.capture("crash", { + kind: "unhandledRejection", + error_class: classifyErrorClass(new TypeError("boom")), + }); + telemetry.capture("crash", { kind: "uncaughtException", error_class: classifyErrorClass("boom") }); + + const captured = await events(); + expect(captured.map((e) => e.properties.error_class)).toEqual([ + "custom", + "TypeError", + "non_error", + ]); + expect(await wire()).not.toContain("AcmeCorp"); +}); + +// --------------------------------------------------------------------------- +// compaction — must not fire when the compactor did nothing +// --------------------------------------------------------------------------- + +test("compaction fires only when turns were actually folded away", async () => { + const { telemetry, events } = harness(); + const compactor = createSessionPruningCompactor({ + compactionMode: "pruning", + summarize: async () => "summary", + telemetry, + }); + + const shortHistory = [ + { role: "user" as const, content: [{ type: "text" as const, text: "hi" }], timestamp: 1 }, + ]; + await compactor.apply(shortHistory, {} as never); + expect(await events()).toEqual([]); + + const longHistory = Array.from({ length: 60 }, (_, i) => ({ + role: (i % 2 === 0 ? "user" : "assistant") as "user" | "assistant", + content: [{ type: "text" as const, text: `turn ${i}` }], + timestamp: i, + })); + await compactor.apply(longHistory, {} as never); + + const captured = await events(); + expect(captured.length).toBe(1); + expect(captured[0]?.event).toBe("compaction"); + expect(captured[0]?.properties.mode).toBe("pruning"); + expect(captured[0]?.properties.turns_before).toBe(60); +}); + +// --------------------------------------------------------------------------- +// The allowlist itself: unknown events and unknown keys never reach the wire +// --------------------------------------------------------------------------- + +test("an unknown event name is dropped rather than sent", async () => { + const { telemetry, wire } = harness(); + telemetry.capture("not_a_real_event" as never, { anything: "acmecorp" }); + expect(await wire()).toBe("[]"); +}); + +test("keys outside an event's allowlist are stripped from the payload", async () => { + const { telemetry, wire, events } = harness(); + telemetry.capture("permission_prompt", { + decision: "allow", + permission_kind: "run_shell", + command: "rm -rf /Users/someone/acmecorp-secrets", + subject: "/Users/someone/acmecorp-secrets", + }); + + const [event] = await events(); + expect(event?.properties.command).toBeUndefined(); + expect(await wire()).not.toContain("acmecorp"); +}); From 33a7896b6f4e9db8c8ccf344f5f20b5e700f97f3 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 16:37:31 -0700 Subject: [PATCH 5/5] Report which provider rejected credentials on auth failure The auth_failure event reused error_class, which everywhere else means the JS error constructor name. One column carrying two incompatible meanings cannot be analysed, and it made the documented error_class guarantee false: the value shipped was a local send-failure kind that never passed through the classifier. --- docs/TELEMETRY.md | 11 ++++-- src/telemetry/index.ts | 4 ++- src/tui-opentui/session-chrome.ts | 15 ++++++++ src/tui/runner.ts | 5 ++- tests/unit/telemetry-product-events.test.ts | 39 +++++++++++++++++++++ 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/docs/TELEMETRY.md b/docs/TELEMETRY.md index c86e4d95a..42d3b8695 100644 --- a/docs/TELEMETRY.md +++ b/docs/TELEMETRY.md @@ -21,7 +21,7 @@ Each event carries a small set of properties: | `permission_prompt` | An approval prompt is answered (or abandoned) | `decision`, `permission_kind` | | `compaction` | The compactor actually folds turns away | `mode`, `duration_ms`, `turns_before`, `turns_after` | | `crash` | A fatal error reaches the process-level handler | `kind`, `error_class` | -| `auth_failure` | A provider rejects the stored credentials | `error_class` | +| `auth_failure` | A provider rejects the stored credentials | `auth_provider` | `compaction` is deliberately silent on the runs where the compactor decides there is nothing to compact — an event that also fires on no-ops makes its own @@ -62,7 +62,14 @@ all and `plugin_loaded` carries only `origin`, the discovery tier `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 -application or plugin code is as author-chosen as any other string. +application or plugin code is as author-chosen as any other string. It appears +on `crash` and nowhere else, so the column means one thing everywhere it is +recorded. + +`auth_provider` is a separate property for that reason: it names which +provider's sign-in was rejected (`codex`, `xai`), chosen from a fixed +first-party set in `src/tui-opentui/session-chrome.ts`. No part of the +provider's rejection message is sent. The mapping is `src/telemetry/classify.ts`, and the tests that feed each emission site a deliberately identifying name and assert it reaches no part of diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index 436106962..084475267 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -101,7 +101,9 @@ const EVENT_PROPERTY_ALLOWLIST: Record = { permission_prompt: ["decision", "permission_kind"], compaction: ["mode", "duration_ms", "turns_before", "turns_after"], crash: ["kind", "error_class"], - auth_failure: ["error_class"], + // Which provider rejected the credentials, not why — the rejection detail is + // provider-authored text and error_class means a JS constructor name. + auth_failure: ["auth_provider"], }; const KNOWN_EVENTS: ReadonlySet = new Set(Object.keys(EVENT_PROPERTY_ALLOWLIST)); diff --git a/src/tui-opentui/session-chrome.ts b/src/tui-opentui/session-chrome.ts index deb51e3d1..e2bec6236 100644 --- a/src/tui-opentui/session-chrome.ts +++ b/src/tui-opentui/session-chrome.ts @@ -5,6 +5,7 @@ * duplicating the state machine that produces it. */ +import type { Telemetry } from "../telemetry/index.js" import type { RampPhase } from "./ramp.js" /** Agent lifecycle status the progress label reads (mirrors the stream state). */ @@ -137,6 +138,20 @@ export function shouldSettleUiAfterSendFailure(kind: SendFailureKind): boolean { return kind === "codex_auth" || kind === "xai_auth" || kind === "error" } +// Send-failure kinds are first-party constants, so the provider each one names +// is a fixed mapping rather than a classification over author-chosen text. +const AUTH_FAILURE_PROVIDERS: Partial> = { + codex_auth: "codex", + xai_auth: "xai", +} + +/** Report which provider rejected the stored credentials; silent otherwise. */ +export function captureAuthFailure(telemetry: Telemetry, kind: SendFailureKind): void { + const provider = AUTH_FAILURE_PROVIDERS[kind] + if (provider === undefined) return + telemetry.capture("auth_failure", { auth_provider: provider }) +} + // The stream carries a failure as a bare message string, so the auth errors are // recognised by the profile phrase their constructors always produce // (`Codex profile "default" is not authorized. …`). diff --git a/src/tui/runner.ts b/src/tui/runner.ts index dc4a4af72..947bb97e3 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -145,6 +145,7 @@ import { surfaceSystemNotice, } from "../tui-opentui/shell.js"; import { + captureAuthFailure, classifyAgentSendFailure, shouldSettleUiAfterSendFailure, } from "../tui-opentui/session-chrome.js"; @@ -1772,9 +1773,7 @@ export async function runTUI(initialConfig: Config): Promise { isCodexAuthError, isXaiAuthError, ); - if (kind === "codex_auth" || kind === "xai_auth") { - getTelemetry().capture("auth_failure", { error_class: kind }); - } + captureAuthFailure(getTelemetry(), kind); if (!shouldSettleUiAfterSendFailure(kind)) return; recordRunError(err); systemNotice(err instanceof Error ? err.message : String(err)); diff --git a/tests/unit/telemetry-product-events.test.ts b/tests/unit/telemetry-product-events.test.ts index 8a83e5ff4..2df7341a4 100644 --- a/tests/unit/telemetry-product-events.test.ts +++ b/tests/unit/telemetry-product-events.test.ts @@ -23,6 +23,10 @@ import { classifyPermissionKind, } from "../../src/telemetry/classify.js"; import { createTelemetry, type Telemetry } from "../../src/telemetry/index.js"; +import { + captureAuthFailure, + classifyAgentSendFailure, +} from "../../src/tui-opentui/session-chrome.js"; type BatchBody = { batch: { event: string; properties: Record }[]; @@ -256,6 +260,41 @@ test("crash reports language error types by name and buckets everything else", a expect(await wire()).not.toContain("AcmeCorp"); }); +// --------------------------------------------------------------------------- +// auth_failure — the provider's rejection message names the profile +// --------------------------------------------------------------------------- + +test("auth_failure names the provider and never ships the rejection message", async () => { + const { telemetry, wire, events } = harness(); + const isCodexAuth = (e: unknown) => e instanceof Error && /codex profile/i.test(e.message); + const isXaiAuth = (e: unknown) => e instanceof Error && /xai profile/i.test(e.message); + + const codexRejection = new Error('Codex profile "acmecorp-eng" is not authorized.'); + const rejections = [ + codexRejection, + new Error('xai profile "acmecorp-eng" is not authorized.'), + new Error("connection reset by /Users/someone/acmecorp"), + ]; + for (const err of rejections) { + captureAuthFailure( + telemetry, + classifyAgentSendFailure(err, false, isCodexAuth, isXaiAuth), + ); + } + // An aborted send outranks the auth match, so it must emit nothing. + captureAuthFailure( + telemetry, + classifyAgentSendFailure(codexRejection, true, isCodexAuth, isXaiAuth), + ); + + const captured = await events(); + expect(captured.map((e) => e.event)).toEqual(["auth_failure", "auth_failure"]); + expect(captured.map((e) => e.properties.auth_provider)).toEqual(["codex", "xai"]); + const body = await wire(); + expect(body).not.toContain("acmecorp"); + expect(body).not.toContain("error_class"); +}); + // --------------------------------------------------------------------------- // compaction — must not fire when the compactor did nothing // ---------------------------------------------------------------------------