From 1b80da2f3a4349fc91839870f32ea7ce6c5ee71b Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sun, 5 Jul 2026 16:57:39 +0000 Subject: [PATCH 01/14] fix(telemetry): reduce PostHog event volume, fix consent dark pattern --- PRIVACY.md | 12 +- .../telemetry/src/PostHogTelemetryClient.ts | 10 +- packages/telemetry/src/TelemetryService.ts | 108 ++++++++++++++- .../__tests__/PostHogTelemetryClient.test.ts | 12 ++ .../TelemetryService.circuit-breaker.test.ts | 124 ++++++++++++++++++ .../TelemetryService.shutdown.test.ts | 64 +++++++++ .../TelemetryService.task-completed.test.ts | 50 +++++++ .../telemetry.isTelemetryOptedIn.test.ts | 21 +++ .../telemetry.taskProperties.test.ts | 38 ++++++ packages/types/src/telemetry.ts | 21 +++ src/__tests__/extension.spec.ts | 103 +++++++++++++-- .../fetchers/__tests__/modelCache.spec.ts | 117 +++++++++++++++++ src/api/providers/fetchers/modelCache.ts | 37 ++++-- ...resentAssistantMessage-custom-tool.spec.ts | 6 - ...esentAssistantMessage-unknown-tool.spec.ts | 20 +++ .../presentAssistantMessage.ts | 39 +++--- src/core/task/Task.ts | 23 ++-- .../task/__tests__/messageCounting.spec.ts | 75 +++++++++++ src/core/task/messageCounting.ts | 20 +++ src/core/tools/AttemptCompletionTool.ts | 2 +- .../__tests__/attemptCompletionTool.spec.ts | 40 +++++- .../telemetrySettingsTracking.spec.ts | 45 ++++--- src/core/webview/webviewMessageHandler.ts | 11 +- src/extension.ts | 20 ++- .../ChatView.scroll-debug-repro.spec.tsx | 3 + .../src/components/common/TelemetryBanner.tsx | 26 +++- .../common/__tests__/TelemetryBanner.spec.tsx | 65 +++++++++ webview-ui/src/components/settings/About.tsx | 4 +- .../settings/__tests__/About.spec.tsx | 30 +++++ webview-ui/src/i18n/locales/ca/welcome.json | 4 +- webview-ui/src/i18n/locales/de/welcome.json | 4 +- webview-ui/src/i18n/locales/en/welcome.json | 4 +- webview-ui/src/i18n/locales/es/welcome.json | 4 +- webview-ui/src/i18n/locales/fr/welcome.json | 4 +- webview-ui/src/i18n/locales/hi/welcome.json | 4 +- webview-ui/src/i18n/locales/id/welcome.json | 4 +- webview-ui/src/i18n/locales/it/welcome.json | 4 +- webview-ui/src/i18n/locales/ja/welcome.json | 4 +- webview-ui/src/i18n/locales/ko/welcome.json | 4 +- webview-ui/src/i18n/locales/nl/welcome.json | 4 +- webview-ui/src/i18n/locales/pl/welcome.json | 4 +- .../src/i18n/locales/pt-BR/welcome.json | 4 +- webview-ui/src/i18n/locales/ru/welcome.json | 4 +- webview-ui/src/i18n/locales/tr/welcome.json | 4 +- webview-ui/src/i18n/locales/vi/welcome.json | 4 +- .../src/i18n/locales/zh-CN/welcome.json | 4 +- .../src/i18n/locales/zh-TW/welcome.json | 4 +- webview-ui/src/utils/TelemetryClient.ts | 4 +- 48 files changed, 1103 insertions(+), 119 deletions(-) create mode 100644 packages/telemetry/src/__tests__/TelemetryService.circuit-breaker.test.ts create mode 100644 packages/telemetry/src/__tests__/TelemetryService.shutdown.test.ts create mode 100644 packages/telemetry/src/__tests__/TelemetryService.task-completed.test.ts create mode 100644 packages/types/src/__tests__/telemetry.isTelemetryOptedIn.test.ts create mode 100644 packages/types/src/__tests__/telemetry.taskProperties.test.ts create mode 100644 src/core/task/__tests__/messageCounting.spec.ts create mode 100644 src/core/task/messageCounting.ts create mode 100644 webview-ui/src/components/common/__tests__/TelemetryBanner.spec.tsx diff --git a/PRIVACY.md b/PRIVACY.md index 2f589cf3ea..741a7bdc06 100644 --- a/PRIVACY.md +++ b/PRIVACY.md @@ -32,14 +32,14 @@ go—and, importantly, where they don't. - **API Keys & Credentials**: If you enter an API key (e.g., to connect an AI model), it is stored locally on your device and never sent to us or any third party, except the provider you have chosen. -- **Telemetry (Usage Data)**: We collect feature usage and error data to help - us improve Zoo Code. This telemetry is powered by PostHog and includes your - VS Code machine ID, feature usage patterns, and exception reports. The VS Code +- **Telemetry (Usage Data)**: We collect feature usage and error data to help us + improve Zoo Code. This telemetry is powered by PostHog and includes your VS + Code machine ID, feature usage patterns, and exception reports. The VS Code machine ID is a persistent identifier and may be considered personal data in some jurisdictions; we use it only for product analytics and error grouping. - We retain telemetry only as long as needed for product analytics and debugging. - Telemetry does **not** collect your code or AI prompts, and you can opt out at - any time through the settings. + We retain telemetry only as long as needed for product analytics and + debugging. This PostHog-based telemetry does **not** collect your code or AI + prompts, and you can opt out at any time through the settings. - **Marketplace Requests**: When you browse or search the Marketplace for Model Configuration Profiles (MCPs) or Custom Modes, Zoo Code makes a secure API call to Zoo Code's backend servers to retrieve listing information. These diff --git a/packages/telemetry/src/PostHogTelemetryClient.ts b/packages/telemetry/src/PostHogTelemetryClient.ts index 77f0bd2aa5..3f7dd810f0 100644 --- a/packages/telemetry/src/PostHogTelemetryClient.ts +++ b/packages/telemetry/src/PostHogTelemetryClient.ts @@ -31,7 +31,15 @@ export class PostHogTelemetryClient extends BaseTelemetryClient { super( { type: "exclude", - events: [TelemetryEventName.TASK_MESSAGE, TelemetryEventName.LLM_COMPLETION], + events: [ + TelemetryEventName.TASK_MESSAGE, + TelemetryEventName.LLM_COMPLETION, + // Per-turn events superseded by the toolsUsed/messageCount summary on + // Task Completed (see TelemetryService.captureTaskCompleted). Excluded + // here as a backstop in case any call site still fires them directly. + TelemetryEventName.TASK_CONVERSATION_MESSAGE, + TelemetryEventName.TOOL_USED, + ], }, debug, ) diff --git a/packages/telemetry/src/TelemetryService.ts b/packages/telemetry/src/TelemetryService.ts index 8eb1ed0ab6..62691ccf0c 100644 --- a/packages/telemetry/src/TelemetryService.ts +++ b/packages/telemetry/src/TelemetryService.ts @@ -5,16 +5,53 @@ import { type TelemetryPropertiesProvider, TelemetryEventName, type TelemetrySetting, + type ToolUsage, } from "@roo-code/types" +/** + * Events prone to retry-storm-style repetition (e.g. a broken embedder config + * re-triggering on every file-system event). Guarded by a circuit breaker in + * `captureEvent` so a single broken install can't flood the Product Analytics + * quota. Tracked per-event via a sliding time window, independent of any + * other telemetry the same install may also be sending. + */ +const CIRCUIT_BREAKER_GUARDED_EVENTS = new Set([TelemetryEventName.CODE_INDEX_ERROR]) + +/** Captures of a guarded event within the counting window allowed before the breaker trips. */ +const CIRCUIT_BREAKER_MAX_IN_WINDOW = 50 + +/** Rolling window over which guarded-event occurrences are counted. */ +const CIRCUIT_BREAKER_WINDOW_MS = 10 * 60 * 1000 + +/** How long a tripped breaker stays tripped before allowing captures again. */ +const CIRCUIT_BREAKER_COOLDOWN_MS = 10 * 60 * 1000 + /** * TelemetryService wrapper class that defers initialization. * This ensures that we only create the various clients after environment * variables are loaded. */ export class TelemetryService { + // Timestamps of recent guarded-event occurrences, per event name, oldest first. + private guardedEventOccurrences = new Map() + private trippedUntil = new Map() + + // In-flight client.capture()/captureException() promises. captureEvent/captureException are + // synchronous (void-returning) for callers, but the underlying client calls are async (e.g. + // PostHogTelemetryClient awaits property enrichment before enqueueing). Tracked here so + // shutdown() can drain them before flushing/closing the clients -- otherwise a capture that's + // still mid-flight when shutdown() runs could be lost entirely. + private pendingClientCalls = new Set>() + constructor(private clients: TelemetryClient[]) {} + private trackPendingClientCall(promise: Promise): void { + // Never let a rejected client call surface as an unhandled rejection or block shutdown. + const tracked = promise.catch(() => undefined) + this.pendingClientCalls.add(tracked) + void tracked.finally(() => this.pendingClientCalls.delete(tracked)) + } + public register(client: TelemetryClient): void { this.clients.push(client) } @@ -51,6 +88,44 @@ export class TelemetryService { this.clients.forEach((client) => client.updateTelemetryState(isOptedIn)) } + /** + * Checks whether a guarded event should be dropped by the circuit breaker, + * updating the breaker's internal state as a side effect. Tracked entirely + * independently of other event names -- unrelated telemetry from the same + * install must never mask (or count towards) a guarded-event burst. + */ + private shouldDropForCircuitBreaker(eventName: TelemetryEventName): boolean { + if (!CIRCUIT_BREAKER_GUARDED_EVENTS.has(eventName)) { + return false + } + + const now = Date.now() + + const trippedUntil = this.trippedUntil.get(eventName) + if (trippedUntil !== undefined) { + if (now < trippedUntil) { + return true + } + + // Cooldown elapsed - reset and allow this capture through. + this.trippedUntil.delete(eventName) + this.guardedEventOccurrences.delete(eventName) + } + + const windowStart = now - CIRCUIT_BREAKER_WINDOW_MS + const occurrences = (this.guardedEventOccurrences.get(eventName) ?? []).filter((ts) => ts > windowStart) + occurrences.push(now) + this.guardedEventOccurrences.set(eventName, occurrences) + + if (occurrences.length > CIRCUIT_BREAKER_MAX_IN_WINDOW) { + this.trippedUntil.set(eventName, now + CIRCUIT_BREAKER_COOLDOWN_MS) + this.guardedEventOccurrences.delete(eventName) + return true + } + + return false + } + /** * Generic method to capture any type of event with specified properties * @param eventName The event name to capture @@ -62,7 +137,11 @@ export class TelemetryService { return } - this.clients.forEach((client) => client.capture({ event: eventName, properties })) + if (this.shouldDropForCircuitBreaker(eventName)) { + return + } + + this.clients.forEach((client) => this.trackPendingClientCall(client.capture({ event: eventName, properties }))) } /** @@ -75,7 +154,9 @@ export class TelemetryService { return } - this.clients.forEach((client) => client.captureException(error, additionalProperties)) + this.clients.forEach((client) => + this.trackPendingClientCall(client.captureException(error, additionalProperties)), + ) } public captureTaskCreated(taskId: string): void { @@ -86,8 +167,21 @@ export class TelemetryService { this.captureEvent(TelemetryEventName.TASK_RESTARTED, { taskId }) } - public captureTaskCompleted(taskId: string): void { - this.captureEvent(TelemetryEventName.TASK_COMPLETED, { taskId }) + /** + * Captures task completion, optionally summarizing the per-task tool and + * message counts that were previously reported as separate per-turn events + * (`Tool Used`, `Conversation Message`) to reduce Product Analytics volume. + */ + public captureTaskCompleted( + taskId: string, + toolsUsed?: ToolUsage, + messageCount?: { user: number; assistant: number }, + ): void { + this.captureEvent(TelemetryEventName.TASK_COMPLETED, { + taskId, + ...(toolsUsed !== undefined && { toolsUsed }), + ...(messageCount !== undefined && { messageCount }), + }) } public captureConversationMessage(taskId: string, source: "user" | "assistant"): void { @@ -263,7 +357,11 @@ export class TelemetryService { return } - this.clients.forEach((client) => client.shutdown()) + // Drain any in-flight capture/captureException calls first, so a client's shutdown() + // (which flushes its queue) can't run ahead of a capture that hasn't been enqueued yet. + await Promise.all(this.pendingClientCalls) + + await Promise.all(this.clients.map((client) => client.shutdown())) } private static _instance: TelemetryService | null = null diff --git a/packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts b/packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts index 1feeb1688f..47c03911fd 100644 --- a/packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts +++ b/packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts @@ -71,6 +71,18 @@ describe("PostHogTelemetryClient", () => { expect(isEventCapturable(TelemetryEventName.LLM_COMPLETION)).toBe(false) }) + + it("should exclude per-turn Conversation Message and Tool Used events (superseded by Task Completed summaries)", () => { + const client = new PostHogTelemetryClient() + + const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>( + client, + "isEventCapturable", + ).bind(client) + + expect(isEventCapturable(TelemetryEventName.TASK_CONVERSATION_MESSAGE)).toBe(false) + expect(isEventCapturable(TelemetryEventName.TOOL_USED)).toBe(false) + }) }) describe("isPropertyCapturable", () => { diff --git a/packages/telemetry/src/__tests__/TelemetryService.circuit-breaker.test.ts b/packages/telemetry/src/__tests__/TelemetryService.circuit-breaker.test.ts new file mode 100644 index 0000000000..53ebaaebb1 --- /dev/null +++ b/packages/telemetry/src/__tests__/TelemetryService.circuit-breaker.test.ts @@ -0,0 +1,124 @@ +// pnpm --filter @roo-code/telemetry test src/__tests__/TelemetryService.circuit-breaker.test.ts + +import { TelemetryEventName, type TelemetryClient } from "@roo-code/types" + +import { TelemetryService } from "../TelemetryService" + +describe("TelemetryService circuit breaker", () => { + let mockClient: TelemetryClient + + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(0) + + mockClient = { + setProvider: vi.fn(), + capture: vi.fn().mockResolvedValue(undefined), + captureException: vi.fn().mockResolvedValue(undefined), + updateTelemetryState: vi.fn(), + isTelemetryEnabled: vi.fn().mockReturnValue(true), + shutdown: vi.fn().mockResolvedValue(undefined), + } + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it("passes through captures under the trip threshold", () => { + const service = new TelemetryService([mockClient]) + + for (let i = 0; i < 49; i++) { + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i }) + } + + expect(mockClient.capture).toHaveBeenCalledTimes(49) + }) + + it("trips after 50 CODE_INDEX_ERROR captures within the window and drops further ones", () => { + const service = new TelemetryService([mockClient]) + + for (let i = 0; i < 50; i++) { + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i }) + } + expect(mockClient.capture).toHaveBeenCalledTimes(50) + + // 51st capture should be dropped - breaker has tripped. + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i: 50 }) + expect(mockClient.capture).toHaveBeenCalledTimes(50) + + // Keeps dropping while tripped. + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i: 51 }) + expect(mockClient.capture).toHaveBeenCalledTimes(50) + }) + + it("re-allows captures after the cooldown window elapses", () => { + const service = new TelemetryService([mockClient]) + + for (let i = 0; i < 50; i++) { + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i }) + } + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i: 50 }) + expect(mockClient.capture).toHaveBeenCalledTimes(50) + + // Just under 10 minutes - still tripped. + vi.setSystemTime(10 * 60 * 1000 - 1) + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i: 51 }) + expect(mockClient.capture).toHaveBeenCalledTimes(50) + + // Cooldown elapsed - one more error gets through. + vi.setSystemTime(10 * 60 * 1000) + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i: 52 }) + expect(mockClient.capture).toHaveBeenCalledTimes(51) + }) + + it("does not reset the guarded count when unrelated events are interleaved", () => { + // A real broken install still does normal things (creates/completes other tasks) + // while a subsystem like code-index is stuck in a retry loop. Unrelated telemetry + // must not mask the CODE_INDEX_ERROR burst by resetting its count. + const service = new TelemetryService([mockClient]) + + for (let i = 0; i < 25; i++) { + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i }) + service.captureEvent(TelemetryEventName.TASK_CREATED, { taskId: `task-${i}` }) + } + // 25 CODE_INDEX_ERROR so far - still under the threshold of 50. + expect(mockClient.capture).toHaveBeenCalledTimes(25 + 25) + + for (let i = 25; i < 50; i++) { + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i }) + service.captureEvent(TelemetryEventName.TASK_CREATED, { taskId: `task-${i}` }) + } + // 50th CODE_INDEX_ERROR trips the breaker; TASK_CREATED events are never guarded. + expect(mockClient.capture).toHaveBeenCalledTimes(50 + 50) + + // Further CODE_INDEX_ERROR captures are dropped even though unrelated events keep flowing. + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i: 50 }) + service.captureEvent(TelemetryEventName.TASK_CREATED, { taskId: "task-50" }) + expect(mockClient.capture).toHaveBeenCalledTimes(50 + 51) + }) + + it("expires old occurrences outside the counting window instead of trapping the breaker open forever", () => { + // A slow trickle of CODE_INDEX_ERROR (below the burst rate) should never trip the + // breaker, since old occurrences age out of the window rather than accumulating forever. + const service = new TelemetryService([mockClient]) + + for (let i = 0; i < 60; i++) { + service.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { i }) + // Advance well past the counting window between each one. + vi.setSystemTime(Date.now() + 60 * 1000) + } + + expect(mockClient.capture).toHaveBeenCalledTimes(60) + }) + + it("does not guard other event names", () => { + const service = new TelemetryService([mockClient]) + + for (let i = 0; i < 200; i++) { + service.captureEvent(TelemetryEventName.TOOL_USED, { tool: "read_file" }) + } + + expect(mockClient.capture).toHaveBeenCalledTimes(200) + }) +}) diff --git a/packages/telemetry/src/__tests__/TelemetryService.shutdown.test.ts b/packages/telemetry/src/__tests__/TelemetryService.shutdown.test.ts new file mode 100644 index 0000000000..cbfede00b2 --- /dev/null +++ b/packages/telemetry/src/__tests__/TelemetryService.shutdown.test.ts @@ -0,0 +1,64 @@ +// pnpm --filter @roo-code/telemetry test src/__tests__/TelemetryService.shutdown.test.ts + +import { TelemetryEventName, type TelemetryClient } from "@roo-code/types" + +import { TelemetryService } from "../TelemetryService" + +describe("TelemetryService.shutdown draining", () => { + it("awaits in-flight captureEvent calls before shutting down clients", async () => { + let resolveCapture!: () => void + const capturePromise = new Promise((resolve) => { + resolveCapture = resolve + }) + + const captureOrder: string[] = [] + + const mockClient: TelemetryClient = { + setProvider: vi.fn(), + capture: vi.fn().mockImplementation(async () => { + await capturePromise + captureOrder.push("captured") + }), + captureException: vi.fn(), + updateTelemetryState: vi.fn(), + isTelemetryEnabled: vi.fn().mockReturnValue(true), + shutdown: vi.fn().mockImplementation(async () => { + captureOrder.push("shutdown") + }), + } + + const service = new TelemetryService([mockClient]) + + // Fire a capture whose underlying client.capture() promise hasn't resolved yet. + service.captureEvent(TelemetryEventName.TASK_CREATED, { taskId: "abc" }) + + const shutdownPromise = service.shutdown() + + // Capture is still pending - shutdown must not have run yet. + expect(captureOrder).toEqual([]) + + resolveCapture() + await shutdownPromise + + // The in-flight capture must complete before the client is shut down. + expect(captureOrder).toEqual(["captured", "shutdown"]) + }) + + it("does not let a rejected capture prevent shutdown from completing", async () => { + const mockClient: TelemetryClient = { + setProvider: vi.fn(), + capture: vi.fn().mockRejectedValue(new Error("capture failed")), + captureException: vi.fn(), + updateTelemetryState: vi.fn(), + isTelemetryEnabled: vi.fn().mockReturnValue(true), + shutdown: vi.fn().mockResolvedValue(undefined), + } + + const service = new TelemetryService([mockClient]) + + service.captureEvent(TelemetryEventName.TASK_CREATED, { taskId: "abc" }) + + await expect(service.shutdown()).resolves.toBeUndefined() + expect(mockClient.shutdown).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/telemetry/src/__tests__/TelemetryService.task-completed.test.ts b/packages/telemetry/src/__tests__/TelemetryService.task-completed.test.ts new file mode 100644 index 0000000000..a637b550fd --- /dev/null +++ b/packages/telemetry/src/__tests__/TelemetryService.task-completed.test.ts @@ -0,0 +1,50 @@ +// pnpm --filter @roo-code/telemetry test src/__tests__/TelemetryService.task-completed.test.ts + +import { TelemetryEventName, type TelemetryClient } from "@roo-code/types" + +import { TelemetryService } from "../TelemetryService" + +describe("TelemetryService.captureTaskCompleted", () => { + let mockClient: TelemetryClient + + beforeEach(() => { + mockClient = { + setProvider: vi.fn(), + capture: vi.fn().mockResolvedValue(undefined), + captureException: vi.fn().mockResolvedValue(undefined), + updateTelemetryState: vi.fn(), + isTelemetryEnabled: vi.fn().mockReturnValue(true), + shutdown: vi.fn().mockResolvedValue(undefined), + } + }) + + it("captures Task Completed with the taskId when no summary is provided", () => { + const service = new TelemetryService([mockClient]) + + service.captureTaskCompleted("task_1") + + expect(mockClient.capture).toHaveBeenCalledWith({ + event: TelemetryEventName.TASK_COMPLETED, + properties: { taskId: "task_1" }, + }) + }) + + it("includes toolsUsed and messageCount summaries when provided", () => { + const service = new TelemetryService([mockClient]) + + service.captureTaskCompleted( + "task_1", + { read_file: { attempts: 3, failures: 0 }, apply_diff: { attempts: 1, failures: 1 } }, + { user: 4, assistant: 5 }, + ) + + expect(mockClient.capture).toHaveBeenCalledWith({ + event: TelemetryEventName.TASK_COMPLETED, + properties: { + taskId: "task_1", + toolsUsed: { read_file: { attempts: 3, failures: 0 }, apply_diff: { attempts: 1, failures: 1 } }, + messageCount: { user: 4, assistant: 5 }, + }, + }) + }) +}) diff --git a/packages/types/src/__tests__/telemetry.isTelemetryOptedIn.test.ts b/packages/types/src/__tests__/telemetry.isTelemetryOptedIn.test.ts new file mode 100644 index 0000000000..06f0dd0f36 --- /dev/null +++ b/packages/types/src/__tests__/telemetry.isTelemetryOptedIn.test.ts @@ -0,0 +1,21 @@ +// pnpm --filter @roo-code/types test src/__tests__/telemetry.isTelemetryOptedIn.test.ts + +import { isTelemetryOptedIn } from "../telemetry.js" + +describe("isTelemetryOptedIn", () => { + it("returns true only for an explicit 'enabled' setting", () => { + expect(isTelemetryOptedIn("enabled")).toBe(true) + }) + + it("returns false for an explicit 'disabled' setting", () => { + expect(isTelemetryOptedIn("disabled")).toBe(false) + }) + + it("returns false for 'unset' (no explicit consent yet)", () => { + expect(isTelemetryOptedIn("unset")).toBe(false) + }) + + it("returns false for undefined (treated the same as unset)", () => { + expect(isTelemetryOptedIn(undefined)).toBe(false) + }) +}) diff --git a/packages/types/src/__tests__/telemetry.taskProperties.test.ts b/packages/types/src/__tests__/telemetry.taskProperties.test.ts new file mode 100644 index 0000000000..8b01c96d5a --- /dev/null +++ b/packages/types/src/__tests__/telemetry.taskProperties.test.ts @@ -0,0 +1,38 @@ +// pnpm --filter @roo-code/types test src/__tests__/telemetry.taskProperties.test.ts + +import { taskPropertiesSchema } from "../telemetry.js" + +describe("taskPropertiesSchema", () => { + it("accepts a payload with no message/tool summary", () => { + const result = taskPropertiesSchema.safeParse({ taskId: "task_1" }) + + expect(result.success).toBe(true) + }) + + it("accepts an optional toolsUsed map", () => { + const result = taskPropertiesSchema.safeParse({ + taskId: "task_1", + toolsUsed: { read_file: { attempts: 3, failures: 0 } }, + }) + + expect(result.success).toBe(true) + }) + + it("accepts an optional messageCount summary", () => { + const result = taskPropertiesSchema.safeParse({ + taskId: "task_1", + messageCount: { user: 4, assistant: 5 }, + }) + + expect(result.success).toBe(true) + }) + + it("rejects a messageCount missing a required field", () => { + const result = taskPropertiesSchema.safeParse({ + taskId: "task_1", + messageCount: { user: 4 }, + }) + + expect(result.success).toBe(false) + }) +}) diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index 402cd571c8..c357943da6 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -13,6 +13,18 @@ export const telemetrySettingsSchema = z.enum(telemetrySettings) export type TelemetrySetting = z.infer +/** + * Whether the user has explicitly opted into telemetry. + * + * Only an explicit "enabled" counts as consent. "unset" (no choice made yet) and + * "disabled" both mean telemetry must not be captured -- this is what makes a + * neutral dismiss of the consent banner (which leaves the setting "unset") + * actually neutral, rather than silently opting the user in. + */ +export function isTelemetryOptedIn(telemetrySetting: TelemetrySetting | undefined): boolean { + return telemetrySetting === "enabled" +} + /** * TelemetryEventName */ @@ -128,6 +140,15 @@ export const taskPropertiesSchema = z.object({ pending: z.number(), }) .optional(), + // Per-task tool/message summaries, captured once on Task Completed instead + // of as separate per-turn events (reduces Product Analytics volume). + toolsUsed: z.record(z.string(), z.object({ attempts: z.number(), failures: z.number() })).optional(), + messageCount: z + .object({ + user: z.number(), + assistant: z.number(), + }) + .optional(), }) export type TaskProperties = z.infer diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index 689e35307d..2fe114e636 100644 --- a/src/__tests__/extension.spec.ts +++ b/src/__tests__/extension.spec.ts @@ -36,6 +36,8 @@ vi.mock("vscode", () => ({ }, env: { language: "en", + isTelemetryEnabled: true, + onDidChangeTelemetryEnabled: vi.fn(), }, ExtensionMode: { Production: 1, @@ -72,19 +74,18 @@ vi.mock("@roo-code/cloud", () => ({ getRooCodeApiUrl: vi.fn().mockReturnValue("https://app.roocode.com"), })) +const mockTelemetryServiceInstance = { + register: vi.fn(), + setProvider: vi.fn(), + shutdown: vi.fn(), + updateTelemetryState: vi.fn(), +} + vi.mock("@roo-code/telemetry", () => ({ TelemetryService: { - createInstance: vi.fn().mockReturnValue({ - register: vi.fn(), - setProvider: vi.fn(), - shutdown: vi.fn(), - }), + createInstance: vi.fn().mockReturnValue(mockTelemetryServiceInstance), get instance() { - return { - register: vi.fn(), - setProvider: vi.fn(), - shutdown: vi.fn(), - } + return mockTelemetryServiceInstance }, }, PostHogTelemetryClient: vi.fn(), @@ -114,6 +115,7 @@ vi.mock("../core/config/ContextProxy", () => ({ setValue: vi.fn(), getValues: vi.fn().mockReturnValue({}), getProviderSettings: vi.fn().mockReturnValue({}), + getGlobalState: vi.fn().mockReturnValue("enabled"), }), }, })) @@ -317,4 +319,85 @@ describe("extension.ts", () => { await expect(activate(mockContext)).resolves.toBeDefined() }) }) + + describe("telemetry level reactivity", () => { + beforeEach(() => { + vi.resetModules() + }) + + test("registers a listener for vscode.env.onDidChangeTelemetryEnabled", async () => { + const vscode = await import("vscode") + + const { activate } = await import("../extension") + await activate(mockContext) + + expect(vscode.env.onDidChangeTelemetryEnabled).toHaveBeenCalledTimes(1) + expect(vscode.env.onDidChangeTelemetryEnabled).toHaveBeenCalledWith(expect.any(Function)) + }) + + test("re-evaluates telemetry state from stored settings when VS Code's global toggle changes", async () => { + const vscode = await import("vscode") + const { TelemetryService } = await import("@roo-code/telemetry") + const { ContextProxy } = await import("../core/config/ContextProxy") + + const mockContextProxyInstance = await (ContextProxy.getInstance as any)() + vi.mocked(mockContextProxyInstance.getGlobalState).mockReturnValue("enabled") + + const { activate } = await import("../extension") + await activate(mockContext) + + const updateTelemetryStateMock = vi.mocked(TelemetryService.instance.updateTelemetryState) + updateTelemetryStateMock.mockClear() + + const onDidChangeHandler = vi.mocked(vscode.env.onDidChangeTelemetryEnabled).mock.calls[0][0] + onDidChangeHandler(false) + + expect(updateTelemetryStateMock).toHaveBeenCalledWith(true) + }) + + test("treats a disabled stored setting as opted out even when VS Code telemetry is re-enabled", async () => { + const vscode = await import("vscode") + const { TelemetryService } = await import("@roo-code/telemetry") + const { ContextProxy } = await import("../core/config/ContextProxy") + + const mockContextProxyInstance = await (ContextProxy.getInstance as any)() + vi.mocked(mockContextProxyInstance.getGlobalState).mockReturnValue("disabled") + + const { activate } = await import("../extension") + await activate(mockContext) + + const updateTelemetryStateMock = vi.mocked(TelemetryService.instance.updateTelemetryState) + updateTelemetryStateMock.mockClear() + + const onDidChangeHandler = vi.mocked(vscode.env.onDidChangeTelemetryEnabled).mock.calls[0][0] + onDidChangeHandler(true) + + expect(updateTelemetryStateMock).toHaveBeenCalledWith(false) + }) + }) + + describe("deactivate", () => { + beforeEach(() => { + vi.resetModules() + }) + + test("still runs terminal cleanup when telemetry shutdown rejects", async () => { + const { TelemetryService } = await import("@roo-code/telemetry") + const { Terminal } = await import("../integrations/terminal/Terminal") + const { TerminalRegistry } = await import("../integrations/terminal/TerminalRegistry") + + vi.mocked(TelemetryService.instance.shutdown).mockRejectedValue(new Error("shutdown failed")) + const setTerminalProfileSpy = vi.spyOn(Terminal, "setTerminalProfile") + + const { activate, deactivate } = await import("../extension") + await activate(mockContext) + + await expect(deactivate()).resolves.toBeUndefined() + + expect(setTerminalProfileSpy).toHaveBeenCalledWith(undefined) + expect(TerminalRegistry.cleanup).toHaveBeenCalledTimes(1) + + setTerminalProfileSpy.mockRestore() + }) + }) }) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 5b771f2bda..8033b3ad30 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -59,6 +59,7 @@ vi.mock("../../../core/config/ContextProxy", () => ({ import type { Mock } from "vitest" import * as fsSync from "fs" import NodeCache from "node-cache" +import { TelemetryService } from "@roo-code/telemetry" import { getModels, getModelsFromCache } from "../modelCache" import { getLiteLLMModels } from "../litellm" import { getOpenRouterModels } from "../openrouter" @@ -481,6 +482,122 @@ describe("empty cache protection", () => { }) }) +describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { + type ModelCacheModule = typeof import("../modelCache") + + let freshGetModels: ModelCacheModule["getModels"] + let freshRefreshModels: ModelCacheModule["refreshModels"] + let freshMockGetOpenRouterModels: Mock + let freshMockGetLiteLLMModels: Mock + + beforeEach(async () => { + // The empty-response throttle is deliberately module-level, persistent state (once per + // provider per session). Reset modules per test so each test starts with a clean gate. + vi.resetModules() + vi.clearAllMocks() + + const modelCacheModule: ModelCacheModule = await import("../modelCache") + const openRouterModule = await import("../openrouter") + const liteLLMModule = await import("../litellm") + + freshGetModels = modelCacheModule.getModels + freshRefreshModels = modelCacheModule.refreshModels + freshMockGetOpenRouterModels = openRouterModule.getOpenRouterModels as Mock + freshMockGetLiteLLMModels = liteLLMModule.getLiteLLMModels as Mock + + const NodeCacheModule = await import("node-cache") + const MockedNodeCache = vi.mocked(NodeCacheModule.default) + const mockCache: any = new MockedNodeCache() + mockCache.get.mockReturnValue(undefined) + }) + + it("fires MODEL_CACHE_EMPTY_RESPONSE only once for repeated empty getModels responses from the same provider", async () => { + freshMockGetOpenRouterModels.mockResolvedValue({}) + + await freshGetModels({ provider: "openrouter" }) + await freshGetModels({ provider: "openrouter" }) + await freshGetModels({ provider: "openrouter" }) + + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledWith( + "Model Cache Empty Response", + expect.objectContaining({ provider: "openrouter", context: "getModels" }), + ) + }) + + it("fires again after a non-empty response resets the throttle for that provider", async () => { + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetOpenRouterModels.mockResolvedValue({}) + await freshGetModels({ provider: "openrouter" }) + await freshGetModels({ provider: "openrouter" }) + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + + freshMockGetOpenRouterModels.mockResolvedValue({ + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + description: "OpenRouter model", + }, + }) + await freshGetModels({ provider: "openrouter" }) + + freshMockGetOpenRouterModels.mockResolvedValue({}) + await freshGetModels({ provider: "openrouter" }) + + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) + + it("throttles independently per provider", async () => { + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetOpenRouterModels.mockResolvedValue({}) + freshMockGetLiteLLMModels.mockResolvedValue({}) + + await freshGetModels({ provider: "openrouter" }) + await freshGetModels({ provider: "litellm", apiKey: "key", baseUrl: "http://localhost:4000" }) + + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) + + it("throttles empty responses from refreshModels using the same per-provider gate", async () => { + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetOpenRouterModels.mockResolvedValue({}) + + await freshRefreshModels({ provider: "openrouter" }) + await freshRefreshModels({ provider: "openrouter" }) + + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledWith( + "Model Cache Empty Response", + expect.objectContaining({ + provider: "openrouter", + context: "refreshModels", + hasExistingCache: false, + existingCacheSize: 0, + }), + ) + }) + + it("throttles independently per distinct endpoint, not just per provider name", async () => { + // Two different LiteLLM servers share the "litellm" provider name but are a different + // cache identity (see getCacheKey) -- an empty response from one must not suppress the + // signal for the other. + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetLiteLLMModels.mockResolvedValue({}) + + await freshGetModels({ provider: "litellm", apiKey: "key-a", baseUrl: "http://server-a:4000" }) + await freshGetModels({ provider: "litellm", apiKey: "key-a", baseUrl: "http://server-a:4000" }) + await freshGetModels({ provider: "litellm", apiKey: "key-b", baseUrl: "http://server-b:4000" }) + + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) +}) + describe("key-scoped cache key derivation", () => { // Exercises the per-API-key cache discriminator that all KEY_SCOPED_PROVIDERS share. // Requesty is used only because it is a key-scoped provider with a mocked fetcher; the diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 312f4f9382..00f262a38b 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -40,6 +40,26 @@ const modelRecordSchema = z.record(z.string(), modelInfoSchema) // deduplicate each other's in-flight refreshes. const inFlightRefresh = new Map>() +// Cache keys (see getCacheKey) for which we've already reported an empty model response this +// session. A persistently-empty endpoint (e.g. misconfigured server) would otherwise re-fire this +// event on every cache refresh; gate it to at most once per distinct provider+server+key identity +// until a non-empty response is seen -- the same identity dimensions the model cache itself uses, +// so two different endpoints for the same provider can never suppress each other's signal. +const reportedEmptyModelResponse = new Set() + +function captureModelCacheEmptyResponseOnce( + provider: RouterName, + cacheKey: string, + properties: Record, +): void { + if (reportedEmptyModelResponse.has(cacheKey)) { + return + } + + reportedEmptyModelResponse.add(cacheKey) + TelemetryService.instance.captureEvent(TelemetryEventName.MODEL_CACHE_EMPTY_RESPONSE, { provider, ...properties }) +} + // Providers whose model lists are scoped to the signed-in user (e.g. per-account // allowlists or org policies). For these we MUST NOT cache results on disk or // in memory: a sign-in/out cycle could otherwise serve a previous user's model @@ -265,12 +285,10 @@ export const getModels = async (options: GetModelsOptions): Promise await writeModels(cacheKey, models).catch((err) => console.error(`[MODEL_CACHE] Error writing ${cacheKey} models to file cache:`, err), ) + + reportedEmptyModelResponse.delete(cacheKey) } else if (modelCount === 0) { - TelemetryService.instance.captureEvent(TelemetryEventName.MODEL_CACHE_EMPTY_RESPONSE, { - provider, - context: "getModels", - hasExistingCache: false, - }) + captureModelCacheEmptyResponseOnce(provider, cacheKey, { context: "getModels", hasExistingCache: false }) } return models @@ -328,8 +346,7 @@ export const refreshModels = async (options: GetModelsOptions): Promise 0, existingCacheSize: existingCount, @@ -341,6 +358,8 @@ export const refreshModels = async (options: GetModelsOptions): Promise ({ vi.mock("@roo-code/telemetry", () => ({ TelemetryService: { instance: { - captureToolUsage: vi.fn(), captureConsecutiveMistakeError: vi.fn(), }, }, })) -import { TelemetryService } from "@roo-code/telemetry" import { customToolRegistry } from "@roo-code/core" describe("presentAssistantMessage - Custom Tool Recording", () => { @@ -118,7 +116,6 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { // Should record as "custom_tool", not "my_custom_tool" expect(mockTask.recordToolUsage).toHaveBeenCalledWith("custom_tool") - expect(TelemetryService.instance.captureToolUsage).toHaveBeenCalledWith(mockTask.taskId, "custom_tool") }) }) @@ -171,7 +168,6 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { // Should record as "read_file", not "custom_tool" expect(mockTask.recordToolUsage).toHaveBeenCalledWith("read_file") - expect(TelemetryService.instance.captureToolUsage).toHaveBeenCalledWith(mockTask.taskId, "read_file") }) it("should record MCP tool usage as 'use_mcp_tool' (not custom_tool)", async () => { @@ -213,7 +209,6 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { // Should record as "use_mcp_tool", not "custom_tool" expect(mockTask.recordToolUsage).toHaveBeenCalledWith("use_mcp_tool") - expect(TelemetryService.instance.captureToolUsage).toHaveBeenCalledWith(mockTask.taskId, "use_mcp_tool") }) }) @@ -355,7 +350,6 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { // Should not record usage for partial blocks expect(mockTask.recordToolUsage).not.toHaveBeenCalled() - expect(TelemetryService.instance.captureToolUsage).not.toHaveBeenCalled() }) }) }) diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts index 8e6c8d9d9e..1b5dd7c785 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts @@ -214,6 +214,26 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => { expect(mockTask.userMessageContentReady).toBe(true) }) + it("does not record raw model-supplied tool names in tool usage analytics", async () => { + // block.name comes straight from the model's tool-call output and is only checked + // against isValidToolName() *after* recordToolUsage() would otherwise be called. + // An arbitrary/malicious model-supplied name must never become a toolsUsed key. + const toolCallId = "tool_call_analytics_test" + mockTask.assistantMessageContent = [ + { + type: "tool_use", + id: toolCallId, + name: "'; DROP TABLE users; --", + params: {}, + partial: false, + }, + ] + + await presentAssistantMessage(mockTask) + + expect(mockTask.recordToolUsage).not.toHaveBeenCalledWith("'; DROP TABLE users; --") + }) + it("should still work with didRejectTool flag for unknown tool", async () => { const toolCallId = "tool_call_rejected_test" mockTask.assistantMessageContent = [ diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index f71b5cc1bd..4d4fde73b7 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -234,8 +234,9 @@ export async function presentAssistantMessage(cline: Task) { } if (!mcpBlock.partial) { - cline.recordToolUsage("use_mcp_tool") // Record as use_mcp_tool for analytics - TelemetryService.instance.captureToolUsage(cline.taskId, "use_mcp_tool") + // Recorded on the task and summarized once on Task Completed + // instead of emitting a separate telemetry event per tool call. + cline.recordToolUsage("use_mcp_tool") } // Resolve sanitized server name back to original server name @@ -553,23 +554,6 @@ export async function presentAssistantMessage(cline: Task) { pushToolResult(formatResponse.toolError(errorString)) } - if (!block.partial) { - // Check if this is a custom tool - if so, record as "custom_tool" (like MCP tools) - const isCustomTool = stateExperiments?.customTools && customToolRegistry.has(block.name) - const recordName = isCustomTool ? "custom_tool" : block.name - cline.recordToolUsage(recordName) - TelemetryService.instance.captureToolUsage(cline.taskId, recordName) - - // Track legacy format usage for read_file tool (for migration monitoring) - if (block.name === "read_file" && block.usedLegacyFormat) { - const modelInfo = cline.api.getModel() - TelemetryService.instance.captureEvent(TelemetryEventName.READ_FILE_LEGACY_FORMAT_USED, { - taskId: cline.taskId, - model: modelInfo?.id, - }) - } - } - // Validate tool use before execution - ONLY for complete (non-partial) blocks. // Validating partial blocks would cause validation errors to be thrown repeatedly // during streaming, pushing multiple tool_results for the same tool_use_id and @@ -621,6 +605,23 @@ export async function presentAssistantMessage(cline: Task) { break } + + // Record tool usage only for known tool names -- block.name is model-controlled + // and must never reach analytics (recordToolUsage/toolsUsed) unvalidated, since it + // becomes a nested property key on the Task Completed event. + const isCustomTool = stateExperiments?.customTools && customToolRegistry.has(block.name) + if (isCustomTool || isValidToolName(block.name, stateExperiments)) { + const recordName = isCustomTool ? "custom_tool" : block.name + cline.recordToolUsage(recordName) + } + + // Track legacy format usage for read_file tool (for migration monitoring) + if (block.name === "read_file" && block.usedLegacyFormat) { + TelemetryService.instance.captureEvent(TelemetryEventName.READ_FILE_LEGACY_FORMAT_USED, { + taskId: cline.taskId, + model: modelInfo?.id, + }) + } } // Check for identical consecutive tool calls. diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 7bee3c5e14..36a43dabb7 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -134,6 +134,7 @@ import { MessageManager } from "../message-manager" import { validateAndFixToolResultIds } from "./validateToolResultIds" import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages" import { prepareApiConversationMessage } from "./apiConversationHistory" +import { shouldAddUserMessageToHistory } from "./messageCounting" const MAX_EXPONENTIAL_BACKOFF_SECONDS = 600 // 10 minutes const DEFAULT_USAGE_COLLECTION_TIMEOUT_MS = 5000 // 5 seconds @@ -321,6 +322,10 @@ export class Task extends EventEmitter implements TaskLike { consecutiveNoAssistantMessagesCount: number = 0 toolUsage: ToolUsage = {} + // Conversation message counts, summarized once on Task Completed instead + // of emitting a separate telemetry event per turn. + messageCounts: { user: number; assistant: number } = { user: 0, assistant: 0 } + // Checkpoints enableCheckpoints: boolean checkpointTimeout: number @@ -2591,18 +2596,16 @@ export class Task extends EventEmitter implements TaskLike { // Add environment details as its own text block, separate from tool // results. const finalUserContent = [...contentWithoutEnvDetails, { type: "text" as const, text: environmentDetails }] - // Only add user message to conversation history if: - // 1. This is the first attempt (retryAttempt === 0), AND - // 2. The original userContent was not empty (empty signals delegation resume where - // the user message with tool_result and env details is already in history), OR - // 3. The message was removed in a previous iteration (userMessageWasRemoved === true) - // This prevents consecutive user messages while allowing re-add when needed + // See shouldAddUserMessageToHistory for the full add/skip rules (retry/empty/removed). const isEmptyUserContent = currentUserContent.length === 0 - const shouldAddUserMessage = - ((currentItem.retryAttempt ?? 0) === 0 && !isEmptyUserContent) || currentItem.userMessageWasRemoved + const shouldAddUserMessage = shouldAddUserMessageToHistory({ + retryAttempt: currentItem.retryAttempt, + isEmptyUserContent, + userMessageWasRemoved: currentItem.userMessageWasRemoved, + }) if (shouldAddUserMessage) { await this.addToApiConversationHistory({ role: "user", content: finalUserContent }) - TelemetryService.instance.captureConversationMessage(this.taskId, "user") + this.messageCounts.user++ } // Since we sent off a placeholder api_req_started message to update the @@ -3519,7 +3522,7 @@ export class Task extends EventEmitter implements TaskLike { ) this.assistantMessageSavedToHistory = true - TelemetryService.instance.captureConversationMessage(this.taskId, "assistant") + this.messageCounts.assistant++ } // Present any partial blocks that were just completed. diff --git a/src/core/task/__tests__/messageCounting.spec.ts b/src/core/task/__tests__/messageCounting.spec.ts new file mode 100644 index 0000000000..6e49473201 --- /dev/null +++ b/src/core/task/__tests__/messageCounting.spec.ts @@ -0,0 +1,75 @@ +// npx vitest run core/task/__tests__/messageCounting.spec.ts + +import { shouldAddUserMessageToHistory } from "../messageCounting" + +describe("shouldAddUserMessageToHistory", () => { + it("adds the message on a first attempt with non-empty content", () => { + expect( + shouldAddUserMessageToHistory({ + retryAttempt: 0, + isEmptyUserContent: false, + userMessageWasRemoved: false, + }), + ).toBe(true) + }) + + it("adds the message when retryAttempt is undefined (treated as first attempt) with non-empty content", () => { + expect( + shouldAddUserMessageToHistory({ + retryAttempt: undefined, + isEmptyUserContent: false, + userMessageWasRemoved: undefined, + }), + ).toBe(true) + }) + + it("skips an empty-content first attempt (delegation resume - already in history)", () => { + expect( + shouldAddUserMessageToHistory({ + retryAttempt: 0, + isEmptyUserContent: true, + userMessageWasRemoved: false, + }), + ).toBe(false) + }) + + it("skips a retry attempt (retryAttempt > 0) with non-empty content", () => { + expect( + shouldAddUserMessageToHistory({ + retryAttempt: 1, + isEmptyUserContent: false, + userMessageWasRemoved: false, + }), + ).toBe(false) + }) + + it("re-adds the message on a retry attempt if it was previously removed", () => { + expect( + shouldAddUserMessageToHistory({ + retryAttempt: 2, + isEmptyUserContent: false, + userMessageWasRemoved: true, + }), + ).toBe(true) + }) + + it("re-adds an empty-content message if it was previously removed", () => { + expect( + shouldAddUserMessageToHistory({ + retryAttempt: 0, + isEmptyUserContent: true, + userMessageWasRemoved: true, + }), + ).toBe(true) + }) + + it("skips a retry attempt with empty content that was not removed", () => { + expect( + shouldAddUserMessageToHistory({ + retryAttempt: 3, + isEmptyUserContent: true, + userMessageWasRemoved: false, + }), + ).toBe(false) + }) +}) diff --git a/src/core/task/messageCounting.ts b/src/core/task/messageCounting.ts new file mode 100644 index 0000000000..11a886d619 --- /dev/null +++ b/src/core/task/messageCounting.ts @@ -0,0 +1,20 @@ +/** + * Whether the current turn's user message should be added to API conversation + * history (and counted towards the per-task message-count telemetry summary). + * + * Only added when: + * 1. This is the first attempt (retryAttempt === 0) AND the content is non-empty, OR + * 2. The message was removed in a previous iteration (userMessageWasRemoved === true) + * + * Empty content on a first attempt signals a delegation resume, where the user message + * with tool_result and env details is already in history -- adding it again would create + * a duplicate (and inflate the message count). + */ +export function shouldAddUserMessageToHistory(params: { + retryAttempt: number | undefined + isEmptyUserContent: boolean + userMessageWasRemoved: boolean | undefined +}): boolean { + const { retryAttempt, isEmptyUserContent, userMessageWasRemoved } = params + return ((retryAttempt ?? 0) === 0 && !isEmptyUserContent) || Boolean(userMessageWasRemoved) +} diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index 39feae0d9b..836a5a1f58 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -222,7 +222,7 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { // This ensures the latest stats are captured regardless of throttle timer. task.emitFinalTokenUsageUpdate() - TelemetryService.instance.captureTaskCompleted(task.taskId) + TelemetryService.instance.captureTaskCompleted(task.taskId, task.toolUsage, task.messageCounts) task.emit(RooCodeEventName.TaskCompleted, task.taskId, task.getTokenUsage(), task.toolUsage) } } diff --git a/src/core/tools/__tests__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index 86ff112585..719b63ac24 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/src/core/tools/__tests__/attemptCompletionTool.spec.ts @@ -81,6 +81,7 @@ describe("attemptCompletionTool", () => { emit: vi.fn(), getTokenUsage: vi.fn().mockReturnValue({}), toolUsage: {}, + messageCounts: { user: 0, assistant: 0 }, taskId: "task_1", apiConfiguration: { apiProvider: "test" } as any, api: { getModel: vi.fn().mockReturnValue({ id: "test-model", info: {} }) } as any, @@ -633,7 +634,7 @@ describe("attemptCompletionTool", () => { expect(mockProvider.reopenParentFromDelegation).not.toHaveBeenCalled() expect(mockProvider.log).toHaveBeenCalledWith(expect.stringContaining("Skipping delegation")) expect(mockTask.ask).toHaveBeenCalledWith("completion_result", "", false) - expect(mockCaptureTaskCompleted).toHaveBeenCalledWith("child-1") + expect(mockCaptureTaskCompleted).toHaveBeenCalledWith("child-1", {}, { user: 0, assistant: 0 }) }) it("delegates an interrupted subtask completion when the parent is still delegated and awaiting that child", async () => { @@ -732,7 +733,7 @@ describe("attemptCompletionTool", () => { expect(mockProvider.reopenParentFromDelegation).not.toHaveBeenCalled() expect(mockProvider.log).toHaveBeenCalledWith(expect.stringContaining("Skipping delegation")) expect(mockTask.ask).toHaveBeenCalledWith("completion_result", "", false) - expect(mockCaptureTaskCompleted).toHaveBeenCalledWith("child-1") + expect(mockCaptureTaskCompleted).toHaveBeenCalledWith("child-1", {}, { user: 0, assistant: 0 }) }) it("emits TaskCompleted only when completion is accepted", async () => { @@ -757,7 +758,7 @@ describe("attemptCompletionTool", () => { await attemptCompletionTool.handle(mockTask as Task, block, callbacks) expect(mockHandleError).not.toHaveBeenCalled() - expect(mockCaptureTaskCompleted).toHaveBeenCalledWith("task_1") + expect(mockCaptureTaskCompleted).toHaveBeenCalledWith("task_1", {}, { user: 0, assistant: 0 }) expect(mockTask.emit).toHaveBeenCalledWith( RooCodeEventName.TaskCompleted, "task_1", @@ -766,6 +767,39 @@ describe("attemptCompletionTool", () => { ) }) + it("summarizes accumulated tool usage and message counts on completion", async () => { + const block: AttemptCompletionToolUse = { + type: "tool_use", + name: "attempt_completion", + params: { result: "2" }, + nativeArgs: { result: "2" }, + partial: false, + } + + mockTask.ask = vi.fn().mockResolvedValue({ response: "yesButtonClicked", text: "", images: [] }) + mockTask.toolUsage = { + read_file: { attempts: 3, failures: 0 }, + apply_diff: { attempts: 1, failures: 1 }, + } + mockTask.messageCounts = { user: 4, assistant: 5 } + + const callbacks: AttemptCompletionCallbacks = { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + askFinishSubTaskApproval: mockAskFinishSubTaskApproval, + toolDescription: mockToolDescription, + } + + await attemptCompletionTool.handle(mockTask as Task, block, callbacks) + + expect(mockCaptureTaskCompleted).toHaveBeenCalledWith( + "task_1", + { read_file: { attempts: 3, failures: 0 }, apply_diff: { attempts: 1, failures: 1 } }, + { user: 4, assistant: 5 }, + ) + }) + it("does not emit TaskCompleted when user provides follow-up feedback", async () => { const block: AttemptCompletionToolUse = { type: "tool_use", diff --git a/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts b/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts index a99c8862a3..109069defd 100644 --- a/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts +++ b/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach } from "vitest" import { TelemetryService } from "@roo-code/telemetry" -import { TelemetryEventName, type TelemetrySetting } from "@roo-code/types" +import { TelemetryEventName, type TelemetrySetting, isTelemetryOptedIn } from "@roo-code/types" describe("Telemetry Settings Tracking", () => { let mockTelemetryService: { @@ -31,8 +31,8 @@ describe("Telemetry Settings Tracking", () => { const newSetting = "disabled" as TelemetrySetting // Simulate the logic from webviewMessageHandler - const isOptedIn = newSetting !== "disabled" - const wasPreviouslyOptedIn = previousSetting !== "disabled" + const isOptedIn = isTelemetryOptedIn(newSetting) + const wasPreviouslyOptedIn = isTelemetryOptedIn(previousSetting) // If turning telemetry OFF, fire event BEFORE disabling if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { @@ -50,12 +50,12 @@ describe("Telemetry Settings Tracking", () => { expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(false) }) - it("should fire event when going from unset to disabled", () => { + it("should not fire an opt-out event when going from unset to disabled (was never opted in)", () => { const previousSetting = "unset" as TelemetrySetting const newSetting = "disabled" as TelemetrySetting - const isOptedIn = newSetting !== "disabled" - const wasPreviouslyOptedIn = previousSetting !== "disabled" + const isOptedIn = isTelemetryOptedIn(newSetting) + const wasPreviouslyOptedIn = isTelemetryOptedIn(previousSetting) if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) @@ -63,7 +63,9 @@ describe("Telemetry Settings Tracking", () => { TelemetryService.instance.updateTelemetryState(isOptedIn) - expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledWith("unset", "disabled") + // "unset" was never opted in, so there is no opt-out transition to report. + expect(mockTelemetryService.captureTelemetrySettingsChanged).not.toHaveBeenCalled() + expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(false) }) }) @@ -72,8 +74,8 @@ describe("Telemetry Settings Tracking", () => { const previousSetting = "disabled" as TelemetrySetting const newSetting = "enabled" as TelemetrySetting - const isOptedIn = newSetting !== "disabled" - const wasPreviouslyOptedIn = previousSetting !== "disabled" + const isOptedIn = isTelemetryOptedIn(newSetting) + const wasPreviouslyOptedIn = isTelemetryOptedIn(previousSetting) // Update the telemetry state first TelemetryService.instance.updateTelemetryState(isOptedIn) @@ -95,8 +97,8 @@ describe("Telemetry Settings Tracking", () => { const previousSetting = "enabled" as TelemetrySetting const newSetting = "enabled" as TelemetrySetting - const isOptedIn = newSetting !== "disabled" - const wasPreviouslyOptedIn = previousSetting !== "disabled" + const isOptedIn = isTelemetryOptedIn(newSetting) + const wasPreviouslyOptedIn = isTelemetryOptedIn(previousSetting) // Neither condition should be met if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { @@ -114,14 +116,13 @@ describe("Telemetry Settings Tracking", () => { expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(true) }) - it("should fire event when going from unset to enabled (telemetry banner close)", () => { + it("should fire an opt-in event when going from unset to enabled (explicit Accept)", () => { const previousSetting = "unset" as TelemetrySetting const newSetting = "enabled" as TelemetrySetting - const isOptedIn = newSetting !== "disabled" - const wasPreviouslyOptedIn = previousSetting !== "disabled" + const isOptedIn = isTelemetryOptedIn(newSetting) + const wasPreviouslyOptedIn = isTelemetryOptedIn(previousSetting) - // For unset -> enabled, both are opted in, so no event should fire if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) } @@ -132,8 +133,18 @@ describe("Telemetry Settings Tracking", () => { TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) } - // unset is treated as opted-in, so no event should fire - expect(mockTelemetryService.captureTelemetrySettingsChanged).not.toHaveBeenCalled() + // "unset" is not opted in, so unset -> enabled is a genuine opt-in transition. + expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledWith("unset", "enabled") + expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(true) + }) + }) + + describe("neutral banner dismiss ('unset' left as-is)", () => { + it("does not report telemetry as opted in while the setting remains unset", () => { + // A neutral dismiss of the consent banner sends no telemetrySetting message at + // all, so the stored setting stays "unset". Confirm "unset" alone -- with no + // transition -- is not treated as consent. + expect(isTelemetryOptedIn("unset" as TelemetrySetting)).toBe(false) }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index fdcc53ddc1..cd02684325 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -22,6 +22,7 @@ import { checkoutDiffPayloadSchema, checkoutRestorePayloadSchema, getCompletionCheckpoint, + isTelemetryOptedIn, } from "@roo-code/types" import { customToolRegistry } from "@roo-code/core" import { CloudService } from "@roo-code/cloud" @@ -628,11 +629,11 @@ export const webviewMessageHandler = async ( ), ) - // Enable telemetry by default (when unset) or when explicitly enabled + // Only capture telemetry once the user has explicitly opted in; "unset" (no + // choice made yet) and "disabled" are both treated as not opted in. provider.getStateToPostToWebview().then((state) => { const { telemetrySetting } = state - const isOptedIn = telemetrySetting !== "disabled" - TelemetryService.instance.updateTelemetryState(isOptedIn) + TelemetryService.instance.updateTelemetryState(isTelemetryOptedIn(telemetrySetting)) }) provider.isViewLaunched = true @@ -2461,8 +2462,8 @@ export const webviewMessageHandler = async ( case "telemetrySetting": { const telemetrySetting = message.text as TelemetrySetting const previousSetting = getGlobalState("telemetrySetting") || "unset" - const isOptedIn = telemetrySetting !== "disabled" - const wasPreviouslyOptedIn = previousSetting !== "disabled" + const isOptedIn = isTelemetryOptedIn(telemetrySetting) + const wasPreviouslyOptedIn = isTelemetryOptedIn(previousSetting) // If turning telemetry OFF, fire event BEFORE disabling if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { diff --git a/src/extension.ts b/src/extension.ts index e326509c3d..e24b341a52 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -18,6 +18,7 @@ if (fs.existsSync(envPath)) { } import type { CloudUserInfo, AuthState } from "@roo-code/types" +import { isTelemetryOptedIn } from "@roo-code/types" import { CloudService } from "@roo-code/cloud" import { TelemetryService, PostHogTelemetryClient } from "@roo-code/telemetry" import { customToolRegistry } from "@roo-code/core" @@ -173,6 +174,15 @@ export async function activate(context: vscode.ExtensionContext) { const contextProxy = await ContextProxy.getInstance(context) + // React live to VS Code's global telemetry toggle (recommended over only reading + // telemetry.telemetryLevel, which PostHogTelemetryClient still checks as a secondary gate). + context.subscriptions.push( + vscode.env.onDidChangeTelemetryEnabled(() => { + const telemetrySetting = contextProxy.getGlobalState("telemetrySetting") ?? "unset" + TelemetryService.instance.updateTelemetryState(isTelemetryOptedIn(telemetrySetting)) + }), + ) + // Initialize code index managers for all workspace folders. const codeIndexManagers: CodeIndexManager[] = [] @@ -387,7 +397,15 @@ export async function deactivate() { } await McpServerManager.cleanup(extensionContext) - TelemetryService.instance.shutdown() + + try { + await TelemetryService.instance.shutdown() + } catch (error) { + outputChannel.appendLine( + `Failed to shut down telemetry service: ${error instanceof Error ? error.message : String(error)}`, + ) + } + Terminal.setTerminalProfile(undefined) TerminalRegistry.cleanup() } diff --git a/webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx index 12f98898f4..5faf4a69fd 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx @@ -92,6 +92,9 @@ vi.mock("./WorktreeSelector", () => ({ WorktreeSelector: () => null })) vi.mock("@vscode/webview-ui-toolkit/react", () => ({ VSCodeLink: ({ children }: { children: React.ReactNode }) => <>{children}, + VSCodeButton: ({ children, onClick }: { children: React.ReactNode; onClick?: () => void }) => ( + + ), })) vi.mock("@/components/ui", async (importOriginal) => { diff --git a/webview-ui/src/components/common/TelemetryBanner.tsx b/webview-ui/src/components/common/TelemetryBanner.tsx index 3d39b17115..f9c1ecfe92 100644 --- a/webview-ui/src/components/common/TelemetryBanner.tsx +++ b/webview-ui/src/components/common/TelemetryBanner.tsx @@ -1,6 +1,6 @@ import { memo, useState } from "react" import { Trans } from "react-i18next" -import { VSCodeLink } from "@vscode/webview-ui-toolkit/react" +import { VSCodeButton, VSCodeLink } from "@vscode/webview-ui-toolkit/react" import type { TelemetrySetting } from "@roo-code/types" @@ -11,11 +11,23 @@ const TelemetryBanner = () => { const { t } = useAppTranslation() const [isDismissed, setIsDismissed] = useState(false) + // A neutral dismiss ("x") intentionally sends no message, leaving the + // setting as "unset" (routed to disabled for actual capture). Only the + // explicit Accept/Decline actions record the user's choice. const handleClose = () => { setIsDismissed(true) + } + + const handleAccept = () => { + setIsDismissed(true) vscode.postMessage({ type: "telemetrySetting", text: "enabled" satisfies TelemetrySetting }) } + const handleDecline = () => { + setIsDismissed(true) + vscode.postMessage({ type: "telemetrySetting", text: "disabled" satisfies TelemetrySetting }) + } + const handleOpenSettings = () => { window.postMessage({ type: "action", @@ -30,7 +42,7 @@ const TelemetryBanner = () => { return (
- {/* Close button (X) */} + {/* Close button (X) - neutral dismiss, does not opt in */}
{t("welcome:telemetry.helpImprove")}
-
+
{ }} />
+
+ + {t("welcome:telemetry.accept")} + + + {t("welcome:telemetry.decline")} + +
) } diff --git a/webview-ui/src/components/common/__tests__/TelemetryBanner.spec.tsx b/webview-ui/src/components/common/__tests__/TelemetryBanner.spec.tsx new file mode 100644 index 0000000000..309e09f2a9 --- /dev/null +++ b/webview-ui/src/components/common/__tests__/TelemetryBanner.spec.tsx @@ -0,0 +1,65 @@ +import { render, screen, fireEvent } from "@testing-library/react" +import { describe, it, expect, vi, beforeEach } from "vitest" + +import TelemetryBanner from "../TelemetryBanner" + +const mockPostMessage = vi.fn() +vi.mock("@src/utils/vscode", () => ({ + vscode: { + postMessage: (message: any) => mockPostMessage(message), + }, +})) + +vi.mock("@src/i18n/TranslationContext", () => ({ + useAppTranslation: () => ({ + t: (key: string) => { + const translations: Record = { + "welcome:telemetry.helpImprove": "Help Improve Zoo Code", + "welcome:telemetry.helpImproveMessage": "Zoo Code collects error and usage data...", + "welcome:telemetry.accept": "Accept", + "welcome:telemetry.decline": "Decline", + } + return translations[key] || key + }, + }), +})) + +describe("TelemetryBanner", () => { + beforeEach(() => { + mockPostMessage.mockClear() + }) + + it("renders explicit Accept and Decline actions", () => { + render() + + expect(screen.getByRole("button", { name: "Accept" })).toBeInTheDocument() + expect(screen.getByRole("button", { name: "Decline" })).toBeInTheDocument() + }) + + it("sends an enabled setting and dismisses when Accept is clicked", () => { + const { container } = render() + + fireEvent.click(screen.getByRole("button", { name: "Accept" })) + + expect(mockPostMessage).toHaveBeenCalledWith({ type: "telemetrySetting", text: "enabled" }) + expect(container.firstChild).toBeNull() + }) + + it("sends a disabled setting and dismisses when Decline is clicked", () => { + const { container } = render() + + fireEvent.click(screen.getByRole("button", { name: "Decline" })) + + expect(mockPostMessage).toHaveBeenCalledWith({ type: "telemetrySetting", text: "disabled" }) + expect(container.firstChild).toBeNull() + }) + + it("dismisses without sending any message when the close (x) button is clicked", () => { + const { container } = render() + + fireEvent.click(screen.getByRole("button", { name: /close/i })) + + expect(mockPostMessage).not.toHaveBeenCalled() + expect(container.firstChild).toBeNull() + }) +}) diff --git a/webview-ui/src/components/settings/About.tsx b/webview-ui/src/components/settings/About.tsx index 7459d5b8df..f9075cd558 100644 --- a/webview-ui/src/components/settings/About.tsx +++ b/webview-ui/src/components/settings/About.tsx @@ -4,7 +4,7 @@ import { Trans } from "react-i18next" import { ArrowRightLeft, Download, Upload, TriangleAlert, Bug, Lightbulb, Shield, MessagesSquare } from "lucide-react" import { VSCodeButton, VSCodeCheckbox, VSCodeLink } from "@vscode/webview-ui-toolkit/react" -import type { ExtensionMessage, TelemetrySetting } from "@roo-code/types" +import { type ExtensionMessage, type TelemetrySetting, isTelemetryOptedIn } from "@roo-code/types" import { Package } from "@roo/package" @@ -108,7 +108,7 @@ export const About = ({ telemetrySetting, setTelemetrySetting, debug, setDebug, section="about" label={t("settings:footer.telemetry.label")}> { const checked = e.target.checked === true setTelemetrySetting(checked ? "enabled" : "disabled") diff --git a/webview-ui/src/components/settings/__tests__/About.spec.tsx b/webview-ui/src/components/settings/__tests__/About.spec.tsx index 5266be8bae..e23767d736 100644 --- a/webview-ui/src/components/settings/__tests__/About.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/About.spec.tsx @@ -151,6 +151,36 @@ describe("About", () => { ) }) + it("shows the telemetry checkbox as checked when the setting is explicitly enabled", () => { + render( + + + , + ) + + expect(screen.getByRole("checkbox", { name: /telemetry/i })).toBeChecked() + }) + + it("does not show the telemetry checkbox as checked when the setting is unset (no consent given yet)", () => { + render( + + + , + ) + + expect(screen.getByRole("checkbox", { name: /telemetry/i })).not.toBeChecked() + }) + + it("does not show the telemetry checkbox as checked when the setting is disabled", () => { + render( + + + , + ) + + expect(screen.getByRole("checkbox", { name: /telemetry/i })).not.toBeChecked() + }) + it("renders export, import, and reset buttons", () => { render( diff --git a/webview-ui/src/i18n/locales/ca/welcome.json b/webview-ui/src/i18n/locales/ca/welcome.json index 06b3fca6ba..a4c83a5c27 100644 --- a/webview-ui/src/i18n/locales/ca/welcome.json +++ b/webview-ui/src/i18n/locales/ca/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Ajuda a millorar Zoo Code", - "helpImproveMessage": "Zoo Code recopila dades d'errors i ús per ajudar-nos a corregir errors i millorar l'extensió. Aquesta telemetria no recopila codi, indicacions o informació personal. Pots desactivar-la a configuració." + "helpImproveMessage": "Zoo Code recopila dades d'errors i ús per ajudar-nos a corregir errors i millorar l'extensió. Aquesta telemetria no recopila codi, indicacions o informació personal. Pots desactivar-la a configuració.", + "accept": "Accepta", + "decline": "Rebutja" }, "importSettings": "Importa la configuració" } diff --git a/webview-ui/src/i18n/locales/de/welcome.json b/webview-ui/src/i18n/locales/de/welcome.json index 99eef8b9e7..e4b514b947 100644 --- a/webview-ui/src/i18n/locales/de/welcome.json +++ b/webview-ui/src/i18n/locales/de/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Hilf, Zoo Code zu verbessern", - "helpImproveMessage": "Zoo Code sammelt Fehler- und Nutzungsdaten, um uns dabei zu helfen, Fehler zu beheben und die Erweiterung zu verbessern. Diese Telemetrie sammelt keine Code-, Prompt- oder persönliche Informationen. Du kannst diese in den Einstellungen deaktivieren." + "helpImproveMessage": "Zoo Code sammelt Fehler- und Nutzungsdaten, um uns dabei zu helfen, Fehler zu beheben und die Erweiterung zu verbessern. Diese Telemetrie sammelt keine Code-, Prompt- oder persönliche Informationen. Du kannst diese in den Einstellungen deaktivieren.", + "accept": "Akzeptieren", + "decline": "Ablehnen" }, "importSettings": "Einstellungen importieren" } diff --git a/webview-ui/src/i18n/locales/en/welcome.json b/webview-ui/src/i18n/locales/en/welcome.json index f6bd95ee1c..f5239f93cc 100644 --- a/webview-ui/src/i18n/locales/en/welcome.json +++ b/webview-ui/src/i18n/locales/en/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Help Improve Zoo Code", - "helpImproveMessage": "Zoo Code collects error and usage data to help us fix bugs and improve the extension. This telemetry does not collect code, prompts or personal information. You can turn this off in settings." + "helpImproveMessage": "Zoo Code collects error and usage data to help us fix bugs and improve the extension. This telemetry does not collect code, prompts or personal information. You can turn this off in settings.", + "accept": "Accept", + "decline": "Decline" }, "importSettings": "Import Settings" } diff --git a/webview-ui/src/i18n/locales/es/welcome.json b/webview-ui/src/i18n/locales/es/welcome.json index ddc8b358fe..9947a2ca73 100644 --- a/webview-ui/src/i18n/locales/es/welcome.json +++ b/webview-ui/src/i18n/locales/es/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Ayuda a mejorar Zoo Code", - "helpImproveMessage": "Zoo Code recopila datos de errores y uso para ayudarnos a corregir errores y mejorar la extensión. Esta telemetría no recopila código, prompts o información personal. Puedes desactivar esto en la configuración." + "helpImproveMessage": "Zoo Code recopila datos de errores y uso para ayudarnos a corregir errores y mejorar la extensión. Esta telemetría no recopila código, prompts o información personal. Puedes desactivar esto en la configuración.", + "accept": "Aceptar", + "decline": "Rechazar" }, "importSettings": "Importar configuración" } diff --git a/webview-ui/src/i18n/locales/fr/welcome.json b/webview-ui/src/i18n/locales/fr/welcome.json index b9af536a6b..791c21ab2b 100644 --- a/webview-ui/src/i18n/locales/fr/welcome.json +++ b/webview-ui/src/i18n/locales/fr/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Aide à améliorer Zoo Code", - "helpImproveMessage": "Zoo Code collecte des données d'erreurs et d'utilisation pour nous aider à corriger les bugs et améliorer l'extension. Cette télémétrie ne collecte pas de code, de prompts ou d'informations personnelles. Tu peux désactiver ceci dans les paramètres." + "helpImproveMessage": "Zoo Code collecte des données d'erreurs et d'utilisation pour nous aider à corriger les bugs et améliorer l'extension. Cette télémétrie ne collecte pas de code, de prompts ou d'informations personnelles. Tu peux désactiver ceci dans les paramètres.", + "accept": "Accepter", + "decline": "Refuser" }, "importSettings": "Importer les paramètres" } diff --git a/webview-ui/src/i18n/locales/hi/welcome.json b/webview-ui/src/i18n/locales/hi/welcome.json index 3c3028fc0c..c8013a9849 100644 --- a/webview-ui/src/i18n/locales/hi/welcome.json +++ b/webview-ui/src/i18n/locales/hi/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Zoo Code को बेहतर बनाने में मदद करें", - "helpImproveMessage": "Zoo Code बग ठीक करने और एक्सटेंशन को बेहतर बनाने में हमारी मदद करने के लिए त्रुटि और उपयोग डेटा एकत्र करता है। यह दूरसंचार कोड, प्रॉम्प्ट या व्यक्तिगत जानकारी एकत्र नहीं करता है। आप इसे सेटिंग्स में अक्षम कर सकते हैं।" + "helpImproveMessage": "Zoo Code बग ठीक करने और एक्सटेंशन को बेहतर बनाने में हमारी मदद करने के लिए त्रुटि और उपयोग डेटा एकत्र करता है। यह दूरसंचार कोड, प्रॉम्प्ट या व्यक्तिगत जानकारी एकत्र नहीं करता है। आप इसे सेटिंग्स में अक्षम कर सकते हैं।", + "accept": "स्वीकार करें", + "decline": "अस्वीकार करें" }, "importSettings": "सेटिंग्स आयात करें" } diff --git a/webview-ui/src/i18n/locales/id/welcome.json b/webview-ui/src/i18n/locales/id/welcome.json index 9b7acd777f..b30f742636 100644 --- a/webview-ui/src/i18n/locales/id/welcome.json +++ b/webview-ui/src/i18n/locales/id/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Bantu Tingkatkan Zoo Code", - "helpImproveMessage": "Zoo Code mengumpulkan data kesalahan dan penggunaan untuk membantu kami memperbaiki bug dan meningkatkan ekstensi. Telemetri ini tidak mengumpulkan kode, prompt, atau informasi pribadi. Anda dapat menonaktifkannya di pengaturan." + "helpImproveMessage": "Zoo Code mengumpulkan data kesalahan dan penggunaan untuk membantu kami memperbaiki bug dan meningkatkan ekstensi. Telemetri ini tidak mengumpulkan kode, prompt, atau informasi pribadi. Anda dapat menonaktifkannya di pengaturan.", + "accept": "Terima", + "decline": "Tolak" }, "importSettings": "Impor Pengaturan" } diff --git a/webview-ui/src/i18n/locales/it/welcome.json b/webview-ui/src/i18n/locales/it/welcome.json index c5b4066964..040635f84a 100644 --- a/webview-ui/src/i18n/locales/it/welcome.json +++ b/webview-ui/src/i18n/locales/it/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Aiuta a migliorare Zoo Code", - "helpImproveMessage": "Zoo Code raccoglie dati di errore e utilizzo per aiutarci a correggere i bug e migliorare l'estensione. Questa telemetria non raccoglie codice, prompt o informazioni personali. Puoi disabilitare questa funzione in impostazioni." + "helpImproveMessage": "Zoo Code raccoglie dati di errore e utilizzo per aiutarci a correggere i bug e migliorare l'estensione. Questa telemetria non raccoglie codice, prompt o informazioni personali. Puoi disabilitare questa funzione in impostazioni.", + "accept": "Accetta", + "decline": "Rifiuta" }, "importSettings": "Importa impostazioni" } diff --git a/webview-ui/src/i18n/locales/ja/welcome.json b/webview-ui/src/i18n/locales/ja/welcome.json index 55e8b4e3dd..c6a68c3dca 100644 --- a/webview-ui/src/i18n/locales/ja/welcome.json +++ b/webview-ui/src/i18n/locales/ja/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Zoo Codeの改善にご協力ください", - "helpImproveMessage": "Zoo Codeは、バグを修正し、拡張機能を改善するために、エラーおよび使用データを収集します。このテレメトリはコード、プロンプト、または個人情報を収集しません。これは設定で無効にできます。" + "helpImproveMessage": "Zoo Codeは、バグを修正し、拡張機能を改善するために、エラーおよび使用データを収集します。このテレメトリはコード、プロンプト、または個人情報を収集しません。これは設定で無効にできます。", + "accept": "同意する", + "decline": "拒否する" }, "importSettings": "設定のインポート" } diff --git a/webview-ui/src/i18n/locales/ko/welcome.json b/webview-ui/src/i18n/locales/ko/welcome.json index db9407a20a..8d3f98dfaf 100644 --- a/webview-ui/src/i18n/locales/ko/welcome.json +++ b/webview-ui/src/i18n/locales/ko/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Zoo Code 개선에 도움주세요", - "helpImproveMessage": "Zoo Code는 버그를 수정하고 확장을 개선하기 위해 오류 및 사용 데이터를 수집합니다. 이 원격 분석은 코드, 프롬프트 또는 개인 정보를 수집하지 않습니다. 설정에서 이를 비활성화할 수 있습니다." + "helpImproveMessage": "Zoo Code는 버그를 수정하고 확장을 개선하기 위해 오류 및 사용 데이터를 수집합니다. 이 원격 분석은 코드, 프롬프트 또는 개인 정보를 수집하지 않습니다. 설정에서 이를 비활성화할 수 있습니다.", + "accept": "수락", + "decline": "거부" }, "importSettings": "설정 가져오기" } diff --git a/webview-ui/src/i18n/locales/nl/welcome.json b/webview-ui/src/i18n/locales/nl/welcome.json index d62f761496..ead6831052 100644 --- a/webview-ui/src/i18n/locales/nl/welcome.json +++ b/webview-ui/src/i18n/locales/nl/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Help Zoo Code te verbeteren", - "helpImproveMessage": "Zoo Code verzamelt fout- en gebruiksgegevens om ons te helpen bugs op te lossen en de extensie te verbeteren. Deze telemetrie verzamelt geen code, prompts of persoonlijke informatie. Je kunt dit in instellingen uitschakelen." + "helpImproveMessage": "Zoo Code verzamelt fout- en gebruiksgegevens om ons te helpen bugs op te lossen en de extensie te verbeteren. Deze telemetrie verzamelt geen code, prompts of persoonlijke informatie. Je kunt dit in instellingen uitschakelen.", + "accept": "Accepteren", + "decline": "Weigeren" }, "importSettings": "Instellingen importeren" } diff --git a/webview-ui/src/i18n/locales/pl/welcome.json b/webview-ui/src/i18n/locales/pl/welcome.json index 32037e759e..37e41e2921 100644 --- a/webview-ui/src/i18n/locales/pl/welcome.json +++ b/webview-ui/src/i18n/locales/pl/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Pomóż ulepszyć Zoo Code", - "helpImproveMessage": "Zoo Code zbiera dane o błędach i użytkowaniu, aby pomóc nam naprawiać błędy i ulepszać rozszerzenie. Ta telemetria nie zbiera kodu, podpowiedzi ani danych osobowych. Możesz je wyłączyć w ustawieniach." + "helpImproveMessage": "Zoo Code zbiera dane o błędach i użytkowaniu, aby pomóc nam naprawiać błędy i ulepszać rozszerzenie. Ta telemetria nie zbiera kodu, podpowiedzi ani danych osobowych. Możesz je wyłączyć w ustawieniach.", + "accept": "Akceptuj", + "decline": "Odrzuć" }, "importSettings": "Importuj ustawienia" } diff --git a/webview-ui/src/i18n/locales/pt-BR/welcome.json b/webview-ui/src/i18n/locales/pt-BR/welcome.json index c444ff4374..34b920a074 100644 --- a/webview-ui/src/i18n/locales/pt-BR/welcome.json +++ b/webview-ui/src/i18n/locales/pt-BR/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Ajude a melhorar o Zoo Code", - "helpImproveMessage": "Zoo Code coleta dados de erro e uso para nos ajudar a corrigir bugs e melhorar a extensão. Esta telemetria não coleta código, prompts ou informações pessoais. Você pode desabilitar isso nas configurações." + "helpImproveMessage": "Zoo Code coleta dados de erro e uso para nos ajudar a corrigir bugs e melhorar a extensão. Esta telemetria não coleta código, prompts ou informações pessoais. Você pode desabilitar isso nas configurações.", + "accept": "Aceitar", + "decline": "Recusar" }, "importSettings": "Importar configurações" } diff --git a/webview-ui/src/i18n/locales/ru/welcome.json b/webview-ui/src/i18n/locales/ru/welcome.json index dade1e8ece..d576bd8eee 100644 --- a/webview-ui/src/i18n/locales/ru/welcome.json +++ b/webview-ui/src/i18n/locales/ru/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Помогите улучшить Zoo Code", - "helpImproveMessage": "Zoo Code собирает данные об ошибках и использовании, чтобы помочь нам исправлять ошибки и улучшать расширение. Эта телеметрия не собирает код, приглашения или личную информацию. Вы можете отключить это в настройках." + "helpImproveMessage": "Zoo Code собирает данные об ошибках и использовании, чтобы помочь нам исправлять ошибки и улучшать расширение. Эта телеметрия не собирает код, приглашения или личную информацию. Вы можете отключить это в настройках.", + "accept": "Принять", + "decline": "Отклонить" }, "importSettings": "Импортировать настройки" } diff --git a/webview-ui/src/i18n/locales/tr/welcome.json b/webview-ui/src/i18n/locales/tr/welcome.json index 0216342ed9..a4a2528321 100644 --- a/webview-ui/src/i18n/locales/tr/welcome.json +++ b/webview-ui/src/i18n/locales/tr/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Zoo Code'u iyileştirmeye yardımcı ol", - "helpImproveMessage": "Zoo Code, hataları düzeltmemize ve uzantıyı iyileştirmemize yardımcı olmak için hata ve kullanım verilerini toplar. Bu telemetri kod, komutlar veya kişisel bilgileri toplamaz. Bunu ayarlardan devre dışı bırakabilirsin." + "helpImproveMessage": "Zoo Code, hataları düzeltmemize ve uzantıyı iyileştirmemize yardımcı olmak için hata ve kullanım verilerini toplar. Bu telemetri kod, komutlar veya kişisel bilgileri toplamaz. Bunu ayarlardan devre dışı bırakabilirsin.", + "accept": "Kabul et", + "decline": "Reddet" }, "importSettings": "Ayarları İçe Aktar" } diff --git a/webview-ui/src/i18n/locales/vi/welcome.json b/webview-ui/src/i18n/locales/vi/welcome.json index 93756a444e..c23df3aa89 100644 --- a/webview-ui/src/i18n/locales/vi/welcome.json +++ b/webview-ui/src/i18n/locales/vi/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "Giúp cải thiện Zoo Code", - "helpImproveMessage": "Zoo Code thu thập dữ liệu lỗi và sử dụng để giúp chúng tôi sửa lỗi và cải thiện tiện ích mở rộng. Dữ liệu từ xa này không thu thập mã, lời nhắc hoặc thông tin cá nhân. Bạn có thể vô hiệu hóa nó trong cài đặt." + "helpImproveMessage": "Zoo Code thu thập dữ liệu lỗi và sử dụng để giúp chúng tôi sửa lỗi và cải thiện tiện ích mở rộng. Dữ liệu từ xa này không thu thập mã, lời nhắc hoặc thông tin cá nhân. Bạn có thể vô hiệu hóa nó trong cài đặt.", + "accept": "Chấp nhận", + "decline": "Từ chối" }, "importSettings": "Nhập Cài đặt" } diff --git a/webview-ui/src/i18n/locales/zh-CN/welcome.json b/webview-ui/src/i18n/locales/zh-CN/welcome.json index 1506c0c959..562cc228c6 100644 --- a/webview-ui/src/i18n/locales/zh-CN/welcome.json +++ b/webview-ui/src/i18n/locales/zh-CN/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "帮助改进 Zoo Code", - "helpImproveMessage": "Zoo Code 收集错误和使用数据以帮助我们修复 Bug 并改进扩展。此遥测不收集代码、提示或个人信息。你可以在设置中将其关闭。" + "helpImproveMessage": "Zoo Code 收集错误和使用数据以帮助我们修复 Bug 并改进扩展。此遥测不收集代码、提示或个人信息。你可以在设置中将其关闭。", + "accept": "接受", + "decline": "拒绝" }, "importSettings": "导入设置" } diff --git a/webview-ui/src/i18n/locales/zh-TW/welcome.json b/webview-ui/src/i18n/locales/zh-TW/welcome.json index 4ba8548a62..e61aa69e9a 100644 --- a/webview-ui/src/i18n/locales/zh-TW/welcome.json +++ b/webview-ui/src/i18n/locales/zh-TW/welcome.json @@ -12,7 +12,9 @@ }, "telemetry": { "helpImprove": "幫助改進 Zoo Code", - "helpImproveMessage": "Zoo Code 會收集錯誤和使用情況資料以幫助我們修復錯誤並改進擴充功能。此遙測不會收集程式碼、提示詞或個人資訊。您可以在 設定 中關閉。" + "helpImproveMessage": "Zoo Code 會收集錯誤和使用情況資料以幫助我們修復錯誤並改進擴充功能。此遙測不會收集程式碼、提示詞或個人資訊。您可以在 設定 中關閉。", + "accept": "接受", + "decline": "拒絕" }, "importSettings": "匯入設定" } diff --git a/webview-ui/src/utils/TelemetryClient.ts b/webview-ui/src/utils/TelemetryClient.ts index 3e28a4f234..3c14ae0f18 100644 --- a/webview-ui/src/utils/TelemetryClient.ts +++ b/webview-ui/src/utils/TelemetryClient.ts @@ -1,6 +1,6 @@ import posthog from "posthog-js" -import type { TelemetrySetting } from "@roo-code/types" +import { type TelemetrySetting, isTelemetryOptedIn } from "@roo-code/types" class TelemetryClient { private static instance: TelemetryClient @@ -9,7 +9,7 @@ class TelemetryClient { public updateTelemetryState(telemetrySetting: TelemetrySetting, apiKey?: string, distinctId?: string) { posthog.reset() - if (telemetrySetting !== "disabled" && apiKey && distinctId) { + if (isTelemetryOptedIn(telemetrySetting) && apiKey && distinctId) { TelemetryClient.telemetryEnabled = true posthog.init(apiKey, { From 5bec0440aad4718c6ab5d35d9fa56aeb137f06cc Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 11 Jul 2026 20:03:52 +0000 Subject: [PATCH 02/14] fix(EditTool): double-counted tool usage --- src/core/tools/ApplyPatchTool.ts | 1 - src/core/tools/EditFileTool.ts | 3 +-- src/core/tools/EditTool.ts | 3 +-- src/core/tools/GenerateImageTool.ts | 2 -- src/core/tools/SearchReplaceTool.ts | 3 +-- src/core/tools/__tests__/editFileTool.spec.ts | 1 - src/core/tools/__tests__/editTool.spec.ts | 1 - src/core/tools/__tests__/searchReplaceTool.spec.ts | 1 - 8 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/core/tools/ApplyPatchTool.ts b/src/core/tools/ApplyPatchTool.ts index 3f3295404b..56b2bf8909 100644 --- a/src/core/tools/ApplyPatchTool.ts +++ b/src/core/tools/ApplyPatchTool.ts @@ -131,7 +131,6 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { } task.consecutiveMistakeCount = 0 - task.recordToolUsage("apply_patch") } catch (error) { await handleError("apply patch", error as Error) await task.diffViewProvider.reset() diff --git a/src/core/tools/EditFileTool.ts b/src/core/tools/EditFileTool.ts index 2495a372bc..0d273f6398 100644 --- a/src/core/tools/EditFileTool.ts +++ b/src/core/tools/EditFileTool.ts @@ -463,8 +463,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { pushToolResult(message + replacementInfo) - // Record successful tool usage and cleanup - task.recordToolUsage("edit_file") + // Cleanup (tool usage is recorded centrally in presentAssistantMessage) await task.diffViewProvider.reset() this.resetPartialState() diff --git a/src/core/tools/EditTool.ts b/src/core/tools/EditTool.ts index 79338c17a6..45a233a9aa 100644 --- a/src/core/tools/EditTool.ts +++ b/src/core/tools/EditTool.ts @@ -229,8 +229,7 @@ export class EditTool extends BaseTool<"edit"> { const message = await task.diffViewProvider.pushToolWriteResult(task, task.cwd, false) pushToolResult(message) - // Record successful tool usage and cleanup - task.recordToolUsage("edit") + // Cleanup (tool usage is recorded centrally in presentAssistantMessage) await task.diffViewProvider.reset() this.resetPartialState() diff --git a/src/core/tools/GenerateImageTool.ts b/src/core/tools/GenerateImageTool.ts index c32fc85bf1..b036a71977 100644 --- a/src/core/tools/GenerateImageTool.ts +++ b/src/core/tools/GenerateImageTool.ts @@ -242,8 +242,6 @@ export class GenerateImageTool extends BaseTool<"generate_image"> { task.didEditFile = true - task.recordToolUsage("generate_image") - const fullImagePath = path.join(task.cwd, finalPath) let imageUri = provider?.convertToWebviewUri?.(fullImagePath) ?? vscode.Uri.file(fullImagePath).toString() diff --git a/src/core/tools/SearchReplaceTool.ts b/src/core/tools/SearchReplaceTool.ts index 2d8817364f..2d4a60f935 100644 --- a/src/core/tools/SearchReplaceTool.ts +++ b/src/core/tools/SearchReplaceTool.ts @@ -225,8 +225,7 @@ export class SearchReplaceTool extends BaseTool<"search_replace"> { const message = await task.diffViewProvider.pushToolWriteResult(task, task.cwd, false) pushToolResult(message) - // Record successful tool usage and cleanup - task.recordToolUsage("search_replace") + // Cleanup (tool usage is recorded centrally in presentAssistantMessage) await task.diffViewProvider.reset() this.resetPartialState() diff --git a/src/core/tools/__tests__/editFileTool.spec.ts b/src/core/tools/__tests__/editFileTool.spec.ts index 0e7343905e..8b46cb5b2f 100644 --- a/src/core/tools/__tests__/editFileTool.spec.ts +++ b/src/core/tools/__tests__/editFileTool.spec.ts @@ -560,7 +560,6 @@ describe("editFileTool", () => { expect(mockTask.diffViewProvider.saveChanges).toHaveBeenCalled() expect(mockTask.didEditFile).toBe(true) - expect(mockTask.recordToolUsage).toHaveBeenCalledWith("edit_file") }) it("reverts changes when user rejects", async () => { diff --git a/src/core/tools/__tests__/editTool.spec.ts b/src/core/tools/__tests__/editTool.spec.ts index cbf635554c..b72844e92c 100644 --- a/src/core/tools/__tests__/editTool.spec.ts +++ b/src/core/tools/__tests__/editTool.spec.ts @@ -345,7 +345,6 @@ describe("editTool", () => { expect(mockTask.diffViewProvider.saveChanges).toHaveBeenCalled() expect(mockTask.didEditFile).toBe(true) - expect(mockTask.recordToolUsage).toHaveBeenCalledWith("edit") }) it("reverts changes when user rejects", async () => { diff --git a/src/core/tools/__tests__/searchReplaceTool.spec.ts b/src/core/tools/__tests__/searchReplaceTool.spec.ts index ed08f9acbd..6392f98bcc 100644 --- a/src/core/tools/__tests__/searchReplaceTool.spec.ts +++ b/src/core/tools/__tests__/searchReplaceTool.spec.ts @@ -314,7 +314,6 @@ describe("searchReplaceTool", () => { expect(mockCline.diffViewProvider.saveChanges).toHaveBeenCalled() expect(mockCline.didEditFile).toBe(true) - expect(mockCline.recordToolUsage).toHaveBeenCalledWith("search_replace") }) it("reverts changes when user rejects", async () => { From 47e294d12cade053ea10c81d96f610eafe2e0d62 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 11 Jul 2026 20:05:06 +0000 Subject: [PATCH 03/14] feat(Tools): Bucket invalid tool names under a static analytics key --- packages/types/src/tool.ts | 1 + .../presentAssistantMessage-unknown-tool.spec.ts | 15 ++++++++++----- .../assistant-message/presentAssistantMessage.ts | 15 ++++++--------- src/shared/tools.ts | 1 + 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/types/src/tool.ts b/packages/types/src/tool.ts index 4f90b63e9f..d89a8107c1 100644 --- a/packages/types/src/tool.ts +++ b/packages/types/src/tool.ts @@ -46,6 +46,7 @@ export const toolNames = [ "skill", "generate_image", "custom_tool", + "invalid_tool_call", ] as const export const toolNamesSchema = z.enum(toolNames) diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts index 1b5dd7c785..e9bdbcaf3e 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts @@ -101,11 +101,13 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => { // Verify consecutiveMistakeCount was incremented expect(mockTask.consecutiveMistakeCount).toBe(1) - // Verify recordToolError was called + // Verify recordToolError was called, bucketed under a static key rather than the raw + // model-supplied tool name (which must never become an arbitrary analytics property key). expect(mockTask.recordToolError).toHaveBeenCalledWith( - "nonexistent_tool", + "invalid_tool_call", expect.stringContaining("Unknown tool"), ) + expect(mockTask.recordToolError).not.toHaveBeenCalledWith("nonexistent_tool", expect.anything()) // Verify error message was shown to user (uses i18n key) expect(mockTask.say).toHaveBeenCalledWith("error", "unknownToolError") @@ -217,13 +219,15 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => { it("does not record raw model-supplied tool names in tool usage analytics", async () => { // block.name comes straight from the model's tool-call output and is only checked // against isValidToolName() *after* recordToolUsage() would otherwise be called. - // An arbitrary/malicious model-supplied name must never become a toolsUsed key. + // An arbitrary/malicious model-supplied name must never become a toolsUsed key, + // whether via recordToolUsage (success path) or recordToolError (failure path). const toolCallId = "tool_call_analytics_test" + const maliciousName = "'; DROP TABLE users; --" mockTask.assistantMessageContent = [ { type: "tool_use", id: toolCallId, - name: "'; DROP TABLE users; --", + name: maliciousName, params: {}, partial: false, }, @@ -231,7 +235,8 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => { await presentAssistantMessage(mockTask) - expect(mockTask.recordToolUsage).not.toHaveBeenCalledWith("'; DROP TABLE users; --") + expect(mockTask.recordToolUsage).not.toHaveBeenCalledWith(maliciousName) + expect(mockTask.recordToolError).not.toHaveBeenCalledWith(maliciousName, expect.anything()) }) it("should still work with didRejectTool flag for unknown tool", async () => { diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 4d4fde73b7..07782e0a49 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -303,14 +303,10 @@ export async function presentAssistantMessage(cline: Task) { if (!toolCallId) { const errorMessage = "Invalid tool call: missing tool_use.id. XML tool calls are no longer supported. Remove any XML tool markup (e.g. ...) and use native tool calling instead." - // Record a tool error for visibility/telemetry. Use the reported tool name if present. + // Record a tool error for visibility/telemetry. block.name is model-controlled and + // must never reach analytics unvalidated, so bucket it under a static key. try { - if ( - typeof (cline as any).recordToolError === "function" && - typeof (block as any).name === "string" - ) { - ;(cline as any).recordToolError((block as any).name as ToolName, errorMessage) - } + cline.recordToolError("invalid_tool_call", errorMessage) } catch { // Best-effort only } @@ -901,10 +897,11 @@ export async function presentAssistantMessage(cline: Task) { break } - // Not a custom tool - handle as unknown tool error + // Not a custom tool - handle as unknown tool error. block.name is model-controlled + // and must never reach analytics unvalidated, so bucket it under a static key. const errorMessage = `Unknown tool "${block.name}". This tool does not exist. Please use one of the available tools.` cline.consecutiveMistakeCount++ - cline.recordToolError(block.name as ToolName, errorMessage) + cline.recordToolError("invalid_tool_call", errorMessage) await cline.say("error", t("tools:unknownToolError", { toolName: block.name })) // Push tool_result directly WITHOUT setting didAlreadyUseTool // This prevents the stream from being interrupted with "Response interrupted by tool use result" diff --git a/src/shared/tools.ts b/src/shared/tools.ts index d2dd9907b1..3fcd516109 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -290,6 +290,7 @@ export const TOOL_DISPLAY_NAMES: Record = { skill: "load skill", generate_image: "generate images", custom_tool: "use custom tools", + invalid_tool_call: "use an unrecognized tool", } as const // Define available tool groups. From 4c9a6f97f464c7e856ccf191165ba0c727514aed Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 11 Jul 2026 20:05:50 +0000 Subject: [PATCH 04/14] fix(Telemetry): Honor live VS Code telemetry-disable signal --- src/__tests__/extension.spec.ts | 40 +++++++++++++++++++++++++++++---- src/extension.ts | 6 ++++- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index 2fe114e636..617014c619 100644 --- a/src/__tests__/extension.spec.ts +++ b/src/__tests__/extension.spec.ts @@ -321,8 +321,10 @@ describe("extension.ts", () => { }) describe("telemetry level reactivity", () => { - beforeEach(() => { + beforeEach(async () => { vi.resetModules() + const vscode = await import("vscode") + ;(vscode.env as any).isTelemetryEnabled = true }) test("registers a listener for vscode.env.onDidChangeTelemetryEnabled", async () => { @@ -342,6 +344,7 @@ describe("extension.ts", () => { const mockContextProxyInstance = await (ContextProxy.getInstance as any)() vi.mocked(mockContextProxyInstance.getGlobalState).mockReturnValue("enabled") + ;(vscode.env as any).isTelemetryEnabled = true const { activate } = await import("../extension") await activate(mockContext) @@ -349,19 +352,22 @@ describe("extension.ts", () => { const updateTelemetryStateMock = vi.mocked(TelemetryService.instance.updateTelemetryState) updateTelemetryStateMock.mockClear() + // The real vscode.env.onDidChangeTelemetryEnabled event carries no payload; the handler + // must read the current vscode.env.isTelemetryEnabled value, not any argument it's called with. const onDidChangeHandler = vi.mocked(vscode.env.onDidChangeTelemetryEnabled).mock.calls[0][0] - onDidChangeHandler(false) + onDidChangeHandler(undefined as any) expect(updateTelemetryStateMock).toHaveBeenCalledWith(true) }) - test("treats a disabled stored setting as opted out even when VS Code telemetry is re-enabled", async () => { + test("treats a disabled stored setting as opted out even when VS Code telemetry is enabled", async () => { const vscode = await import("vscode") const { TelemetryService } = await import("@roo-code/telemetry") const { ContextProxy } = await import("../core/config/ContextProxy") const mockContextProxyInstance = await (ContextProxy.getInstance as any)() vi.mocked(mockContextProxyInstance.getGlobalState).mockReturnValue("disabled") + ;(vscode.env as any).isTelemetryEnabled = true const { activate } = await import("../extension") await activate(mockContext) @@ -370,7 +376,33 @@ describe("extension.ts", () => { updateTelemetryStateMock.mockClear() const onDidChangeHandler = vi.mocked(vscode.env.onDidChangeTelemetryEnabled).mock.calls[0][0] - onDidChangeHandler(true) + onDidChangeHandler(undefined as any) + + expect(updateTelemetryStateMock).toHaveBeenCalledWith(false) + }) + + test("treats VS Code's live telemetry-disabled signal as opted out even when the stored setting is enabled", async () => { + const vscode = await import("vscode") + const { TelemetryService } = await import("@roo-code/telemetry") + const { ContextProxy } = await import("../core/config/ContextProxy") + + const mockContextProxyInstance = await (ContextProxy.getInstance as any)() + vi.mocked(mockContextProxyInstance.getGlobalState).mockReturnValue("enabled") + ;(vscode.env as any).isTelemetryEnabled = true + + const { activate } = await import("../extension") + await activate(mockContext) + + const updateTelemetryStateMock = vi.mocked(TelemetryService.instance.updateTelemetryState) + updateTelemetryStateMock.mockClear() + + // Simulate the user turning off VS Code's global telemetry toggle: the live env value + // flips before the event fires, and the handler must honor it rather than only the + // stored extension setting. + ;(vscode.env as any).isTelemetryEnabled = false + + const onDidChangeHandler = vi.mocked(vscode.env.onDidChangeTelemetryEnabled).mock.calls[0][0] + onDidChangeHandler(undefined as any) expect(updateTelemetryStateMock).toHaveBeenCalledWith(false) }) diff --git a/src/extension.ts b/src/extension.ts index e24b341a52..5162142e58 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -176,10 +176,14 @@ export async function activate(context: vscode.ExtensionContext) { // React live to VS Code's global telemetry toggle (recommended over only reading // telemetry.telemetryLevel, which PostHogTelemetryClient still checks as a secondary gate). + // vscode.env.isTelemetryEnabled is ANDed in directly because the deprecated + // telemetry.telemetryLevel setting the client checks doesn't reflect this live event. context.subscriptions.push( vscode.env.onDidChangeTelemetryEnabled(() => { const telemetrySetting = contextProxy.getGlobalState("telemetrySetting") ?? "unset" - TelemetryService.instance.updateTelemetryState(isTelemetryOptedIn(telemetrySetting)) + TelemetryService.instance.updateTelemetryState( + isTelemetryOptedIn(telemetrySetting) && vscode.env.isTelemetryEnabled, + ) }), ) From 8c53d864dc059203a983a27e1ab078f87ac26290 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 11 Jul 2026 20:07:07 +0000 Subject: [PATCH 05/14] fix(ModelCache): Re-arm empty-model-response throttle for auth-scoped providers + privacy doc fix --- PRIVACY.md | 4 +-- .../fetchers/__tests__/modelCache.spec.ts | 36 +++++++++++++++++++ src/api/providers/fetchers/modelCache.ts | 18 ++++++---- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/PRIVACY.md b/PRIVACY.md index 741a7bdc06..eb7cf3950a 100644 --- a/PRIVACY.md +++ b/PRIVACY.md @@ -56,8 +56,8 @@ go—and, importantly, where they don't. ## **Your Choices & Control** - You can run models locally to prevent data being sent to third-parties. -- Telemetry collection is enabled by default to help us improve Zoo Code, but - you can opt out at any time through the settings. +- Telemetry collection is off by default and requires your explicit opt-in. + You can change your choice at any time through the settings. - You can delete Zoo Code to stop all data collection. ## **Security & Updates** diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 8033b3ad30..c342d11887 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -43,6 +43,7 @@ vi.mock("fs", () => ({ vi.mock("../litellm") vi.mock("../openrouter") vi.mock("../requesty") +vi.mock("../zoo-gateway") // Mock ContextProxy with a simple static instance vi.mock("../../../core/config/ContextProxy", () => ({ @@ -64,10 +65,12 @@ import { getModels, getModelsFromCache } from "../modelCache" import { getLiteLLMModels } from "../litellm" import { getOpenRouterModels } from "../openrouter" import { getRequestyModels } from "../requesty" +import { getZooGatewayModels } from "../zoo-gateway" const mockGetLiteLLMModels = getLiteLLMModels as Mock const mockGetOpenRouterModels = getOpenRouterModels as Mock const mockGetRequestyModels = getRequestyModels as Mock +const mockGetZooGatewayModels = getZooGatewayModels as Mock const DUMMY_REQUESTY_KEY = "requesty-key-for-testing" @@ -296,6 +299,39 @@ describe("empty cache protection", () => { expect(result).toEqual(mockModels) expect(mockSet).toHaveBeenCalledWith("openrouter", mockModels) }) + + it("re-arms the empty-response throttle after a non-empty response from an auth-scoped provider", async () => { + // zoo-gateway is auth-scoped and skips caching entirely, but a non-empty response + // must still clear the empty-response throttle so a later empty response is reported again. + mockGetZooGatewayModels.mockResolvedValueOnce({}) + + await getModels({ provider: "zoo-gateway", apiKey: "test-key" }) + + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + + const mockModels = { + "zoo-gateway/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + description: "Zoo Gateway model", + }, + } + mockGetZooGatewayModels.mockResolvedValueOnce(mockModels) + + await getModels({ provider: "zoo-gateway", apiKey: "test-key" }) + + // Auth-scoped providers never populate the cache. + expect(mockSet).not.toHaveBeenCalled() + + mockGetZooGatewayModels.mockResolvedValueOnce({}) + + await getModels({ provider: "zoo-gateway", apiKey: "test-key" }) + + // The throttle should have been re-armed by the non-empty response above, so this + // second empty response is reported again instead of being suppressed. + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) }) describe("refreshModels", () => { diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 00f262a38b..0dcdd135e0 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -279,15 +279,19 @@ export const getModels = async (options: GetModelsOptions): Promise // Only cache non-empty results so a failed API response doesn't get persisted // as if the provider had no models. Auth-scoped providers skip caching entirely. - if (modelCount > 0 && !shouldSkipCache) { - memoryCache.set(cacheKey, models) + if (modelCount > 0) { + // Clear the empty-response throttle for any non-empty response, including from + // auth-scoped providers that skip caching, so a later empty response is reported again. + reportedEmptyModelResponse.delete(cacheKey) - await writeModels(cacheKey, models).catch((err) => - console.error(`[MODEL_CACHE] Error writing ${cacheKey} models to file cache:`, err), - ) + if (!shouldSkipCache) { + memoryCache.set(cacheKey, models) - reportedEmptyModelResponse.delete(cacheKey) - } else if (modelCount === 0) { + await writeModels(cacheKey, models).catch((err) => + console.error(`[MODEL_CACHE] Error writing ${cacheKey} models to file cache:`, err), + ) + } + } else { captureModelCacheEmptyResponseOnce(provider, cacheKey, { context: "getModels", hasExistingCache: false }) } From 413021a408f45dc2fe84ff1b1e6ce1006cad03a1 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sun, 12 Jul 2026 02:43:53 +0000 Subject: [PATCH 06/14] fix(telemetry): make telemetry opt-out by disclosed default --- PRIVACY.md | 4 ++-- .../telemetry.isTelemetryOptedIn.test.ts | 10 ++++----- packages/types/src/telemetry.ts | 12 +++++----- .../telemetrySettingsTracking.spec.ts | 22 +++++++++++-------- src/core/webview/webviewMessageHandler.ts | 4 ++-- .../src/__tests__/TelemetryClient.spec.ts | 21 +++++++++++++++++- .../src/components/common/TelemetryBanner.tsx | 9 ++++---- .../settings/__tests__/About.spec.tsx | 4 ++-- 8 files changed, 55 insertions(+), 31 deletions(-) diff --git a/PRIVACY.md b/PRIVACY.md index eb7cf3950a..741a7bdc06 100644 --- a/PRIVACY.md +++ b/PRIVACY.md @@ -56,8 +56,8 @@ go—and, importantly, where they don't. ## **Your Choices & Control** - You can run models locally to prevent data being sent to third-parties. -- Telemetry collection is off by default and requires your explicit opt-in. - You can change your choice at any time through the settings. +- Telemetry collection is enabled by default to help us improve Zoo Code, but + you can opt out at any time through the settings. - You can delete Zoo Code to stop all data collection. ## **Security & Updates** diff --git a/packages/types/src/__tests__/telemetry.isTelemetryOptedIn.test.ts b/packages/types/src/__tests__/telemetry.isTelemetryOptedIn.test.ts index 06f0dd0f36..671f88cc67 100644 --- a/packages/types/src/__tests__/telemetry.isTelemetryOptedIn.test.ts +++ b/packages/types/src/__tests__/telemetry.isTelemetryOptedIn.test.ts @@ -3,7 +3,7 @@ import { isTelemetryOptedIn } from "../telemetry.js" describe("isTelemetryOptedIn", () => { - it("returns true only for an explicit 'enabled' setting", () => { + it("returns true for an explicit 'enabled' setting", () => { expect(isTelemetryOptedIn("enabled")).toBe(true) }) @@ -11,11 +11,11 @@ describe("isTelemetryOptedIn", () => { expect(isTelemetryOptedIn("disabled")).toBe(false) }) - it("returns false for 'unset' (no explicit consent yet)", () => { - expect(isTelemetryOptedIn("unset")).toBe(false) + it("returns true for 'unset' (disclosed opt-out default applies)", () => { + expect(isTelemetryOptedIn("unset")).toBe(true) }) - it("returns false for undefined (treated the same as unset)", () => { - expect(isTelemetryOptedIn(undefined)).toBe(false) + it("returns true for undefined (treated the same as unset)", () => { + expect(isTelemetryOptedIn(undefined)).toBe(true) }) }) diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index c357943da6..5bc9b1ebe9 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -14,15 +14,15 @@ export const telemetrySettingsSchema = z.enum(telemetrySettings) export type TelemetrySetting = z.infer /** - * Whether the user has explicitly opted into telemetry. + * Whether telemetry should be captured for this install. * - * Only an explicit "enabled" counts as consent. "unset" (no choice made yet) and - * "disabled" both mean telemetry must not be captured -- this is what makes a - * neutral dismiss of the consent banner (which leaves the setting "unset") - * actually neutral, rather than silently opting the user in. + * Telemetry is on by default (disclosed opt-out): "unset" (no choice made yet) and + * "enabled" both mean telemetry may be captured. Only an explicit "disabled" opts out. + * The consent banner's dismiss/close action never writes a setting, so it stays neutral -- + * it just leaves the default in effect rather than recording an affirmative choice either way. */ export function isTelemetryOptedIn(telemetrySetting: TelemetrySetting | undefined): boolean { - return telemetrySetting === "enabled" + return telemetrySetting !== "disabled" } /** diff --git a/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts b/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts index 109069defd..0f1e0e1315 100644 --- a/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts +++ b/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts @@ -50,7 +50,7 @@ describe("Telemetry Settings Tracking", () => { expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(false) }) - it("should not fire an opt-out event when going from unset to disabled (was never opted in)", () => { + it("should fire an opt-out event when going from unset to disabled (explicit Decline)", () => { const previousSetting = "unset" as TelemetrySetting const newSetting = "disabled" as TelemetrySetting @@ -63,8 +63,9 @@ describe("Telemetry Settings Tracking", () => { TelemetryService.instance.updateTelemetryState(isOptedIn) - // "unset" was never opted in, so there is no opt-out transition to report. - expect(mockTelemetryService.captureTelemetrySettingsChanged).not.toHaveBeenCalled() + // "unset" is opted in under the disclosed opt-out default, so unset -> disabled + // is a genuine opt-out transition. + expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledWith("unset", "disabled") expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(false) }) }) @@ -116,7 +117,7 @@ describe("Telemetry Settings Tracking", () => { expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(true) }) - it("should fire an opt-in event when going from unset to enabled (explicit Accept)", () => { + it("should not fire an event when going from unset to enabled (already opted in by default)", () => { const previousSetting = "unset" as TelemetrySetting const newSetting = "enabled" as TelemetrySetting @@ -133,18 +134,21 @@ describe("Telemetry Settings Tracking", () => { TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) } - // "unset" is not opted in, so unset -> enabled is a genuine opt-in transition. - expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledWith("unset", "enabled") + // "unset" is already opted in under the disclosed opt-out default, so explicit + // Accept (unset -> enabled) is a no-op transition, not a new opt-in. + expect(mockTelemetryService.captureTelemetrySettingsChanged).not.toHaveBeenCalled() expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(true) }) }) describe("neutral banner dismiss ('unset' left as-is)", () => { - it("does not report telemetry as opted in while the setting remains unset", () => { + it("leaves the disclosed opt-out default in effect while the setting remains unset", () => { // A neutral dismiss of the consent banner sends no telemetrySetting message at // all, so the stored setting stays "unset". Confirm "unset" alone -- with no - // transition -- is not treated as consent. - expect(isTelemetryOptedIn("unset" as TelemetrySetting)).toBe(false) + // transition, and no affirmative choice recorded either way -- resolves to the + // disclosed default (telemetry on) rather than silently opting the user in via + // dismissal itself. + expect(isTelemetryOptedIn("unset" as TelemetrySetting)).toBe(true) }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index cd02684325..d0abee43a9 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -629,8 +629,8 @@ export const webviewMessageHandler = async ( ), ) - // Only capture telemetry once the user has explicitly opted in; "unset" (no - // choice made yet) and "disabled" are both treated as not opted in. + // Telemetry is on by disclosed default: "unset" (no choice made yet) leaves that + // default in effect, same as "enabled". Only an explicit "disabled" opts out. provider.getStateToPostToWebview().then((state) => { const { telemetrySetting } = state TelemetryService.instance.updateTelemetryState(isTelemetryOptedIn(telemetrySetting)) diff --git a/webview-ui/src/__tests__/TelemetryClient.spec.ts b/webview-ui/src/__tests__/TelemetryClient.spec.ts index 968de4fdd6..5d71deb9a0 100644 --- a/webview-ui/src/__tests__/TelemetryClient.spec.ts +++ b/webview-ui/src/__tests__/TelemetryClient.spec.ts @@ -77,7 +77,26 @@ describe("TelemetryClient", () => { expect(posthog.init).not.toHaveBeenCalled() }) - it("doesn't initialize PostHog when telemetry is unset", () => { + it("initializes PostHog when telemetry is unset and credentials are present (disclosed opt-out default)", () => { + // Arrange + const API_KEY = "test-api-key" + const DISTINCT_ID = "test-user-id" + + // Act + telemetryClient.updateTelemetryState("unset", API_KEY, DISTINCT_ID) + + // Assert + expect(posthog.init).toHaveBeenCalledWith( + API_KEY, + expect.objectContaining({ + api_host: "https://us.i.posthog.com", + persistence: "localStorage", + loaded: expect.any(Function), + }), + ) + }) + + it("doesn't initialize PostHog when telemetry is unset but credentials are missing", () => { // Act telemetryClient.updateTelemetryState("unset") diff --git a/webview-ui/src/components/common/TelemetryBanner.tsx b/webview-ui/src/components/common/TelemetryBanner.tsx index f9c1ecfe92..99d7ba329f 100644 --- a/webview-ui/src/components/common/TelemetryBanner.tsx +++ b/webview-ui/src/components/common/TelemetryBanner.tsx @@ -11,9 +11,10 @@ const TelemetryBanner = () => { const { t } = useAppTranslation() const [isDismissed, setIsDismissed] = useState(false) - // A neutral dismiss ("x") intentionally sends no message, leaving the - // setting as "unset" (routed to disabled for actual capture). Only the - // explicit Accept/Decline actions record the user's choice. + // A neutral dismiss ("x") intentionally sends no message, leaving the setting + // "unset" so the disclosed opt-out default (telemetry on) stays in effect. + // Only an explicit Decline (-> "disabled") opts out. Only the explicit + // Accept/Decline actions record the user's choice. const handleClose = () => { setIsDismissed(true) } @@ -42,7 +43,7 @@ const TelemetryBanner = () => { return (
- {/* Close button (X) - neutral dismiss, does not opt in */} + {/* Close button (X) - neutral dismiss, does not record a choice */}