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/apps/vscode-e2e/src/bedrock-mock-server.ts b/apps/vscode-e2e/src/bedrock-mock-server.ts index 059031cf90..d966d54fc7 100644 --- a/apps/vscode-e2e/src/bedrock-mock-server.ts +++ b/apps/vscode-e2e/src/bedrock-mock-server.ts @@ -6,6 +6,8 @@ export interface BedrockMockServer { url: string /** Headers from the most recent converse-stream request (populated after first call). */ lastRequestHeaders: http2.IncomingHttpHeaders | undefined + /** Parsed JSON bodies of every converse-stream request received so far, oldest first. */ + requestBodies: unknown[] close(): Promise } @@ -59,7 +61,7 @@ function encodeFrame(eventType: string, payload: object): Buffer { return frame } -function buildToolCallFrames(toolName: string, toolUseId: string, argsJson: string): Buffer[] { +export function buildToolCallFrames(toolName: string, toolUseId: string, argsJson: string): Buffer[] { const frames: Buffer[] = [] frames.push(encodeFrame("messageStart", { role: "assistant" })) frames.push( @@ -88,7 +90,40 @@ function buildToolCallFrames(toolName: string, toolUseId: string, argsJson: stri return frames } -export async function startBedrockMockServer(): Promise { +// A response with no content blocks at all -- no text, no tool_use. Exercises the +// "no assistant messages" retry path (Task#recursivelyMakeClineRequests), which is +// otherwise unreachable from a real model (a real model asked to return nothing +// still typically returns some text) and untestable at the unit level without +// deeply mocking the streaming loop. +export function buildEmptyResponseFrames(): Buffer[] { + const frames: Buffer[] = [] + frames.push(encodeFrame("messageStart", { role: "assistant" })) + frames.push(encodeFrame("messageStop", { stopReason: "end_turn" })) + frames.push( + encodeFrame("metadata", { + metrics: { latencyMs: 1 }, + usage: { inputTokens: 100, outputTokens: 0, totalTokens: 100, serverToolUsage: {} }, + }), + ) + return frames +} + +export interface BedrockMockServerOptions { + /** + * Frame sequences to serve, one per converse-stream request, in order. The last + * entry repeats for any request beyond the queue's length. Defaults to always + * returning the attempt_completion("4") tool call (the pre-existing behavior). + */ + responses?: Buffer[][] +} + +export async function startBedrockMockServer(options: BedrockMockServerOptions = {}): Promise { + const responses = options.responses ?? [ + buildToolCallFrames("attempt_completion", "tooluse_bedrock_mock_001", JSON.stringify({ result: "4" })), + ] + let requestCount = 0 + const requestBodies: unknown[] = [] + // HTTP/2 cleartext (h2c) — matches what @aws-sdk/client-bedrock-runtime uses by default. const server = http2.createServer() let lastRequestHeaders: http2.IncomingHttpHeaders | undefined @@ -111,18 +146,22 @@ export async function startBedrockMockServer(): Promise { lastRequestHeaders = headers - // Drain the request body before responding (AWS SDK sends the full request before reading). - stream.resume() + // Capture the request body (AWS SDK sends the full request before reading the response). + const bodyChunks: Buffer[] = [] + stream.on("data", (chunk: Buffer) => bodyChunks.push(chunk)) stream.on("end", () => { + try { + requestBodies.push(JSON.parse(Buffer.concat(bodyChunks).toString("utf8"))) + } catch { + requestBodies.push(undefined) + } + stream.respond({ ":status": 200, "content-type": "application/vnd.amazon.eventstream", }) - const frames = buildToolCallFrames( - "attempt_completion", - "tooluse_bedrock_mock_001", - JSON.stringify({ result: "4" }), - ) + const frames = responses[Math.min(requestCount, responses.length - 1)] ?? [] + requestCount++ for (const frame of frames) { stream.write(frame) } @@ -139,6 +178,9 @@ export async function startBedrockMockServer(): Promise { get lastRequestHeaders() { return lastRequestHeaders }, + get requestBodies() { + return requestBodies + }, close: () => { // Destroy all open HTTP/2 sessions first so server.close() resolves immediately // instead of waiting for the extension's retry loop to exhaust itself. diff --git a/apps/vscode-e2e/src/suite/providers/bedrock-empty-response-retry.test.ts b/apps/vscode-e2e/src/suite/providers/bedrock-empty-response-retry.test.ts new file mode 100644 index 0000000000..3beec8ffba --- /dev/null +++ b/apps/vscode-e2e/src/suite/providers/bedrock-empty-response-retry.test.ts @@ -0,0 +1,146 @@ +import * as assert from "assert" + +import { RooCodeEventName, type ClineMessage } from "@roo-code/types" + +import { + startBedrockMockServer, + buildEmptyResponseFrames, + buildToolCallFrames, + type BedrockMockServer, +} from "../../bedrock-mock-server" +import { setDefaultSuiteTimeout } from "../test-utils" +import { waitFor, waitUntilCompleted } from "../utils" + +const BEDROCK_MODEL_ID = "us.anthropic.claude-haiku-4-5-20251001-v1:0" +const USER_PROMPT = "bedrock-empty-response-retry-smoke: what is 2+2? Reply with only the number." + +suite("Bedrock provider — empty assistant response retry", function () { + setDefaultSuiteTimeout(this) + this.timeout(3 * 60_000) + + let mockServer: BedrockMockServer | undefined + + suiteTeardown(async () => { + const aimockUrl = process.env.AIMOCK_URL + const isRecord = process.env.AIMOCK_RECORD === "true" + await globalThis.api.setConfiguration({ + apiProvider: "openrouter" as const, + openRouterApiKey: aimockUrl && !isRecord ? "mock-key" : process.env.OPENROUTER_API_KEY!, + openRouterModelId: "openai/gpt-4.1", + ...(aimockUrl && { openRouterBaseUrl: `${aimockUrl}/v1` }), + }) + + if (mockServer) { + await new Promise((resolve) => setTimeout(resolve, 500)) + await mockServer.close() + mockServer = undefined + } + }) + + // Regression test for a bug where the manual-retry path (autoApprovalEnabled: false, + // user clicks "retry" on the api_req_failed prompt after the model returns no assistant + // content) failed to mark userMessageWasRemoved on the retried stack item. That caused + // shouldAddUserMessageToHistory to skip re-adding the user's message entirely on retry, + // so the retried request went out without it -- silently dropping the user's turn. + test("re-sends the user message and completes after a manual retry following an empty assistant response", async () => { + mockServer = await startBedrockMockServer({ + responses: [ + buildEmptyResponseFrames(), + buildToolCallFrames("attempt_completion", "tooluse_bedrock_mock_002", JSON.stringify({ result: "4" })), + ], + }) + + await globalThis.api.setConfiguration({ + apiProvider: "bedrock" as const, + awsUseApiKey: true, + awsApiKey: "mock-key", + awsRegion: "us-east-1", + apiModelId: BEDROCK_MODEL_ID, + awsBedrockEndpoint: mockServer.url, + awsBedrockEndpointEnabled: true, + }) + + const api = globalThis.api + // Keyed by taskId: this suite runs after bedrock.test.ts's tests in the same + // extension host, and RooCodeEventName.Message is a global event stream, so a + // leftover ask from a prior test's task could otherwise be mistaken for this + // test's own "api_req_failed"/"completion_result" prompts. + const asksByTaskId: Record = {} + let ourTaskId: string | undefined + + const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => { + // Only track the final, non-partial ask -- approving a still-streaming/partial + // ask (e.g. mid-tool-execution) can interrupt the in-flight tool call, which the + // framework then reports as an error and retries, inflating the request count. + if (message.type === "ask" && message.partial !== true) { + ;(asksByTaskId[taskId] ??= []).push(message) + } + } + api.on(RooCodeEventName.Message, messageHandler) + + try { + await waitUntilCompleted({ + api, + start: async () => { + const taskId = await api.startNewTask({ + // autoApprovalEnabled: false is required so the empty-response retry + // goes through the manual ask("api_req_failed", ...) prompt path + // (the buggy branch) rather than the auto-retry backoff path. + configuration: { mode: "ask", autoApprovalEnabled: false }, + text: USER_PROMPT, + }) + ourTaskId = taskId + + // Wait for the manual retry prompt, then approve it (equivalent to + // clicking the primary "Retry" button -- response: "yesButtonClicked"). + await waitFor(() => asksByTaskId[taskId]?.some(({ ask }) => ask === "api_req_failed") ?? false) + await api.approveCurrentAsk() + + // After the retry succeeds, the model calls attempt_completion, which + // prompts a separate "completion_result" ask -- approve that too so + // RooCodeEventName.TaskCompleted (what waitUntilCompleted waits on) fires. + await waitFor(() => asksByTaskId[taskId]?.some(({ ask }) => ask === "completion_result") ?? false) + await api.approveCurrentAsk() + + return taskId + }, + }) + } finally { + api.off(RooCodeEventName.Message, messageHandler) + } + + const asks = ourTaskId ? (asksByTaskId[ourTaskId] ?? []) : [] + + assert.ok( + asks.some(({ ask }) => ask === "api_req_failed"), + "Should have prompted for retry after the empty assistant response", + ) + + // The task must have completed at all -- if the user message was dropped on + // retry (the bug), the retried request would still be missing the user's turn, + // and depending on API validation this could hang, error, or produce a + // nonsensical response instead of reaching completion via waitUntilCompleted above. + // + // The request count itself is intentionally >= 2 rather than exactly 2: after the + // retry succeeds, the framework's own tool_result-interruption recovery + // (validateAndFixToolResultIds, unrelated to this fix) can occasionally inject an + // extra self-correcting exchange if the attempt_completion tool result hasn't been + // recorded by the time the next turn is built. That's expected, independently + // tested framework behavior -- what this test cares about is specifically the + // *first retry request* (index 1, immediately after the empty response), which is + // exactly what the userMessageWasRemoved fix governs. + assert.ok( + mockServer.requestBodies.length >= 2, + `Should have made at least 2 requests (initial + retry), got ${mockServer.requestBodies.length}`, + ) + + const retryRequestBody = mockServer.requestBodies[1] as { messages?: Array<{ content?: unknown[] }> } + const retryRequestJson = JSON.stringify(retryRequestBody) + + assert.ok( + retryRequestJson.includes("bedrock-empty-response-retry-smoke"), + "The retried request must still include the user's original message text " + + "(regression check: userMessageWasRemoved must be set so the message is re-added before retrying)", + ) + }) +}) 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..f372edca74 100644 --- a/packages/telemetry/src/TelemetryService.ts +++ b/packages/telemetry/src/TelemetryService.ts @@ -5,16 +5,66 @@ 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 + +/** + * Upper bound on how long shutdown() will wait for in-flight capture calls to drain. + * deactivate() awaits shutdown() before terminal cleanup, so an unbounded wait here + * (e.g. a capture stuck on network I/O that never resolves/rejects) would block the + * extension host from ever finishing deactivation. Losing an in-flight capture on + * timeout is an acceptable tradeoff against blocking shutdown indefinitely. + */ +const SHUTDOWN_DRAIN_TIMEOUT_MS = 3000 + /** * 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>() + + // Set at the start of shutdown() so new captureEvent/captureException calls stop being + // tracked (and, once clients are closing, stop being sent) instead of racing the drain. + private isShuttingDown = false + 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 +101,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 @@ -58,11 +146,15 @@ export class TelemetryService { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any public captureEvent(eventName: TelemetryEventName, properties?: Record): void { - if (!this.isReady) { + if (!this.isReady || this.isShuttingDown) { + return + } + + if (this.shouldDropForCircuitBreaker(eventName)) { return } - this.clients.forEach((client) => client.capture({ event: eventName, properties })) + this.clients.forEach((client) => this.trackPendingClientCall(client.capture({ event: eventName, properties }))) } /** @@ -71,11 +163,13 @@ export class TelemetryService { * @param additionalProperties Additional properties to include with the exception */ public captureException(error: Error, additionalProperties?: Record): void { - if (!this.isReady) { + if (!this.isReady || this.isShuttingDown) { 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 +180,39 @@ 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. + * + * A single task may emit this more than once over its lifetime (e.g. an + * "idle" or "shutdown" installment followed later by a final + * "attempt_completion" one) -- toolsUsed/messageCount are always the delta + * since the previous emission for that task, not a running total, so + * summing installments for a taskId reconstructs the full-task counts + * without double-counting. + * + * Note "attempt_completion" means the model called that tool, not that the + * user accepted the result -- it fires the same way whether the user goes + * on to accept, decline, or give feedback instead. + * + * IMPORTANT for anyone querying this event (e.g. a PostHog dashboard/funnel): + * "one row" no longer means "one finished task". Group by taskId and sum + * toolsUsed/messageCount across completionReason installments -- do not + * treat `count()` of raw events as a count of completed tasks. + */ + public captureTaskCompleted( + taskId: string, + toolsUsed?: ToolUsage, + messageCount?: { user: number; assistant: number }, + completionReason: "attempt_completion" | "idle" | "shutdown" = "attempt_completion", + ): void { + this.captureEvent(TelemetryEventName.TASK_COMPLETED, { + taskId, + completionReason, + ...(toolsUsed !== undefined && { toolsUsed }), + ...(messageCount !== undefined && { messageCount }), + }) } public captureConversationMessage(taskId: string, source: "user" | "assistant"): void { @@ -263,7 +388,26 @@ export class TelemetryService { return } - this.clients.forEach((client) => client.shutdown()) + // Stop accepting new captures immediately, before draining -- otherwise a steady trickle + // of new calls (e.g. from a teardown-time error handler) could keep pendingClientCalls + // non-empty indefinitely and the drain loop below would never terminate on its own. + this.isShuttingDown = true + + // 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. + // Loop rather than a single snapshot: a call already in flight when draining started may + // itself still be tracked by the time we check again. Bounded by a timeout so a capture + // stuck on network I/O that never resolves/rejects can't block deactivate() forever -- + // losing that one capture is an acceptable tradeoff against hanging terminal cleanup. + const drainStart = Date.now() + while (this.pendingClientCalls.size > 0 && Date.now() - drainStart < SHUTDOWN_DRAIN_TIMEOUT_MS) { + await Promise.race([ + Promise.all(this.pendingClientCalls), + new Promise((resolve) => setTimeout(resolve, SHUTDOWN_DRAIN_TIMEOUT_MS - (Date.now() - drainStart))), + ]) + } + + 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..3e765cadc1 --- /dev/null +++ b/packages/telemetry/src/__tests__/TelemetryService.shutdown.test.ts @@ -0,0 +1,195 @@ +// 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("drains a call already queued before shutdown() started, even across multiple drain passes", async () => { + // Regression test: shutdown() must not take a single Promise.all snapshot of + // pendingClientCalls. A promise added to pendingClientCalls *after* Promise.all(set) + // has already been called is never awaited by that call, even if it never resolves + // -- Promise.all takes its list of promises to await synchronously, at call time. So a + // capture that was already in flight (tracked in pendingClientCalls) when shutdown() + // took its first Promise.all snapshot, but whose *own* async chain enqueues more work + // tracked via a fresh pendingClientCalls entry, must still be drained by a later pass + // of the loop rather than being silently dropped by a single-pass drain. + let resolveFirstCapture!: () => void + const firstCapturePromise = new Promise((resolve) => { + resolveFirstCapture = resolve + }) + + let resolveSecondCapture!: () => void + const secondCapturePromise = new Promise((resolve) => { + resolveSecondCapture = resolve + }) + + const captureOrder: string[] = [] + let firstCaptureStarted = false + + const mockClient: TelemetryClient = { + setProvider: vi.fn(), + capture: vi.fn().mockImplementation(async () => { + if (!firstCaptureStarted) { + firstCaptureStarted = true + await firstCapturePromise + captureOrder.push("first-captured") + return + } + + await secondCapturePromise + captureOrder.push("second-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]) + + // Both captures are fired *before* shutdown() is called, so both are legitimately + // in flight (tracked in pendingClientCalls) at the moment shutdown() takes its first + // snapshot -- unlike a capture fired after shutdown() has started, which is expected + // to be gated out instead (see the "stops accepting new captures" test below). + service.captureEvent(TelemetryEventName.TASK_CREATED, { taskId: "first" }) + service.captureEvent(TelemetryEventName.TASK_CREATED, { taskId: "second" }) + + const shutdownPromise = service.shutdown() + + resolveFirstCapture() + // Flush several microtask ticks so a buggy single-pass drain has every opportunity + // to run client.shutdown() before we check -- a couple of ticks isn't enough since + // the mocked capture/shutdown chain itself spans a few microtask hops. + for (let i = 0; i < 4; i++) { + await Promise.resolve() + } + + // The second capture is still pending (its own resolver hasn't fired) -- shutdown + // must not have completed yet, otherwise it dropped the second capture. + expect(captureOrder).not.toContain("shutdown") + + resolveSecondCapture() + await shutdownPromise + + expect(mockClient.capture).toHaveBeenCalledTimes(2) + expect(captureOrder).toEqual(["first-captured", "second-captured", "shutdown"]) + }) + + it("stops accepting new captures once shutdown() has started", async () => { + // Finding #4: shutdown() must mark itself as shutting down before draining, so a + // steady trickle of new captures firing after shutdown() has begun (e.g. from a + // teardown-time error handler) can't keep pendingClientCalls non-empty forever and + // prevent the drain loop from ever terminating on its own. + const mockClient: TelemetryClient = { + setProvider: vi.fn(), + capture: vi.fn().mockResolvedValue(undefined), + captureException: vi.fn(), + updateTelemetryState: vi.fn(), + isTelemetryEnabled: vi.fn().mockReturnValue(true), + shutdown: vi.fn().mockResolvedValue(undefined), + } + + const service = new TelemetryService([mockClient]) + + const shutdownPromise = service.shutdown() + + // Fired after shutdown() has already started -- must be dropped, not tracked/drained. + service.captureEvent(TelemetryEventName.TASK_CREATED, { taskId: "late" }) + + await shutdownPromise + + expect(mockClient.capture).not.toHaveBeenCalled() + }) + + it("does not hang forever when a capture never resolves, bounded by the drain timeout", async () => { + vi.useFakeTimers() + try { + const mockClient: TelemetryClient = { + setProvider: vi.fn(), + // Never resolves -- simulates a capture stuck on network I/O. + capture: vi.fn().mockImplementation(() => new Promise(() => {})), + 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: "stuck" }) + + const shutdownPromise = service.shutdown() + let settled = false + void shutdownPromise.then(() => { + settled = true + }) + + await vi.advanceTimersByTimeAsync(3000) + + expect(settled).toBe(true) + expect(mockClient.shutdown).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + 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..270fc8f83a --- /dev/null +++ b/packages/telemetry/src/__tests__/TelemetryService.task-completed.test.ts @@ -0,0 +1,66 @@ +// 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 and a default 'attempt_completion' completionReason 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", completionReason: "attempt_completion" }, + }) + }) + + 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", + completionReason: "attempt_completion", + toolsUsed: { read_file: { attempts: 3, failures: 0 }, apply_diff: { attempts: 1, failures: 1 } }, + messageCount: { user: 4, assistant: 5 }, + }, + }) + }) + + it("includes the given completionReason for idle/shutdown installments", () => { + const service = new TelemetryService([mockClient]) + + service.captureTaskCompleted("task_1", { read_file: { attempts: 1, failures: 0 } }, undefined, "idle") + + expect(mockClient.capture).toHaveBeenCalledWith({ + event: TelemetryEventName.TASK_COMPLETED, + properties: { + taskId: "task_1", + completionReason: "idle", + toolsUsed: { read_file: { attempts: 1, failures: 0 } }, + }, + }) + }) +}) 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..671f88cc67 --- /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 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 true for 'unset' (disclosed opt-out default applies)", () => { + expect(isTelemetryOptedIn("unset")).toBe(true) + }) + + it("returns true for undefined (treated the same as unset)", () => { + expect(isTelemetryOptedIn(undefined)).toBe(true) + }) +}) 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..4fb4939634 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 telemetry should be captured for this install. + * + * 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 !== "disabled" +} + /** * TelemetryEventName */ @@ -128,6 +140,25 @@ export const taskPropertiesSchema = z.object({ pending: z.number(), }) .optional(), + // Per-task tool/message summaries, captured once per Task Completed + // installment instead of as separate per-turn events (reduces Product + // Analytics volume). A single task may emit more than one installment + // (see completionReason); each installment's counts are the delta since + // the previous installment for that taskId, not a running total. + toolsUsed: z.record(z.string(), z.object({ attempts: z.number(), failures: z.number() })).optional(), + messageCount: z + .object({ + user: z.number(), + assistant: z.number(), + }) + .optional(), + // Why this Task Completed installment was emitted: the model called + // attempt_completion ("attempt_completion" -- regardless of whether the + // user went on to accept, decline, or give feedback; this is NOT a signal + // that the user accepted the result), the task went idle with unreported + // activity ("idle"), or the extension/task was shut down with unreported + // activity still pending ("shutdown"). + completionReason: z.enum(["attempt_completion", "idle", "shutdown"]).optional(), }) export type TaskProperties = z.infer 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/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 75a72accde..250e28c8e3 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -359,6 +359,11 @@ export type ExtensionState = Pick< telemetrySetting: TelemetrySetting telemetryKey?: string machineId?: string + // Live vscode.env.isTelemetryEnabled, so the webview's own PostHog client can respect + // the VS Code global telemetry toggle the same way the extension-side gate does -- + // without this, an explicit user Accept can still send events while VS Code's global + // telemetry is disabled. + vscodeTelemetryEnabled?: boolean renderContext: "sidebar" | "editor" settingsImportedAt?: number diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index 689e35307d..95e418dc57 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,133 @@ describe("extension.ts", () => { await expect(activate(mockContext)).resolves.toBeDefined() }) }) + + describe("telemetry level reactivity", () => { + beforeEach(async () => { + vi.resetModules() + const vscode = await import("vscode") + ;(vscode.env as any).isTelemetryEnabled = true + }) + + 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") + ;(vscode.env as any).isTelemetryEnabled = true + + const { activate } = await import("../extension") + await activate(mockContext) + + 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(undefined as any) + + expect(updateTelemetryStateMock).toHaveBeenCalledWith(true) + }) + + 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) + + const updateTelemetryStateMock = vi.mocked(TelemetryService.instance.updateTelemetryState) + updateTelemetryStateMock.mockClear() + + const onDidChangeHandler = vi.mocked(vscode.env.onDidChangeTelemetryEnabled).mock.calls[0][0] + 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) + }) + + test("pushes a state update to the webview so its own PostHog client picks up the new vscode.env.isTelemetryEnabled value", async () => { + const vscode = await import("vscode") + const { ClineProvider } = await import("../core/webview/ClineProvider") + + const { activate } = await import("../extension") + await activate(mockContext) + + const visibleInstance = (ClineProvider as any).getVisibleInstance() + vi.mocked(visibleInstance.postStateToWebviewWithoutClineMessages).mockClear() + + const onDidChangeHandler = vi.mocked(vscode.env.onDidChangeTelemetryEnabled).mock.calls[0][0] + onDidChangeHandler(undefined as any) + + expect(visibleInstance.postStateToWebviewWithoutClineMessages).toHaveBeenCalled() + }) + }) + + 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/__tests__/nested-delegation-resume.spec.ts b/src/__tests__/nested-delegation-resume.spec.ts index 35341f935c..e364c82a3c 100644 --- a/src/__tests__/nested-delegation-resume.spec.ts +++ b/src/__tests__/nested-delegation-resume.spec.ts @@ -199,10 +199,12 @@ describe("Nested delegation resume (A → B → C)", () => { emit: vi.fn(), getTokenUsage: vi.fn(() => ({})), toolUsage: {}, + messageCounts: { user: 0, assistant: 0 }, clineMessages: [], userMessageContent: [], consecutiveMistakeCount: 0, emitFinalTokenUsageUpdate: vi.fn(), + flushTelemetryInstallment: vi.fn(), } as unknown as Task const blockC = { @@ -246,10 +248,12 @@ describe("Nested delegation resume (A → B → C)", () => { emit: vi.fn(), getTokenUsage: vi.fn(() => ({})), toolUsage: {}, + messageCounts: { user: 0, assistant: 0 }, clineMessages: [], userMessageContent: [], consecutiveMistakeCount: 0, emitFinalTokenUsageUpdate: vi.fn(), + flushTelemetryInstallment: vi.fn(), } as unknown as Task const blockB = { diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 5b771f2bda..d413818dd2 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", () => ({ @@ -59,14 +60,17 @@ 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" 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" @@ -295,6 +299,106 @@ describe("empty cache protection", () => { expect(result).toEqual(mockModels) expect(mockSet).toHaveBeenCalledWith("openrouter", mockModels) }) + + it("reuses an in-flight fetch for concurrent getModels() calls to the same provider", async () => { + // Finding #11: getModels() previously had no de-duplication at all -- two concurrent + // cache-miss calls would each independently fire their own provider fetch. It now + // shares the same inFlightRefresh single-flight coordinator refreshModels() uses. + const mockModels = { + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + description: "OpenRouter model", + }, + } + + let resolvePromise: (value: typeof mockModels) => void + const delayedPromise = new Promise((resolve) => { + resolvePromise = resolve + }) + mockGetOpenRouterModels.mockReturnValue(delayedPromise) + mockGet.mockReturnValue(undefined) + + const promise1 = getModels({ provider: "openrouter" }) + const promise2 = getModels({ provider: "openrouter" }) + + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(1) + + resolvePromise!(mockModels) + + const [result1, result2] = await Promise.all([promise1, promise2]) + expect(result1).toEqual(mockModels) + expect(result2).toEqual(mockModels) + }) + + it("shares a single in-flight fetch between getModels() and refreshModels() for the same key", async () => { + // The two entry points must converge on the same coordinator so a getModels() cache + // miss racing a concurrent refreshModels() call can't produce two unordered writes to + // the same cache key -- whichever fetch happened to finish last previously won, + // regardless of which one was actually more current. + const mockModels = { + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + description: "OpenRouter model", + }, + } + + let resolvePromise: (value: typeof mockModels) => void + const delayedPromise = new Promise((resolve) => { + resolvePromise = resolve + }) + mockGetOpenRouterModels.mockReturnValue(delayedPromise) + mockGet.mockReturnValue(undefined) + + const { refreshModels } = await import("../modelCache") + + const getPromise = getModels({ provider: "openrouter" }) + const refreshPromise = refreshModels({ provider: "openrouter" }) + + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(1) + + resolvePromise!(mockModels) + + const [getResult, refreshResult] = await Promise.all([getPromise, refreshPromise]) + expect(getResult).toEqual(mockModels) + expect(refreshResult).toEqual(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", () => { @@ -481,6 +585,156 @@ 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 + let freshMockGetZooGatewayModels: 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") + const zooGatewayModule = await import("../zoo-gateway") + + freshGetModels = modelCacheModule.getModels + freshRefreshModels = modelCacheModule.refreshModels + freshMockGetOpenRouterModels = openRouterModule.getOpenRouterModels as Mock + freshMockGetLiteLLMModels = liteLLMModule.getLiteLLMModels as Mock + freshMockGetZooGatewayModels = zooGatewayModule.getZooGatewayModels 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) + }) + + it("throttles zoo-gateway independently per session token, even though caching itself is skipped", async () => { + // zoo-gateway is auth-scoped (see AUTH_SCOPED_PROVIDERS) and never persists to the + // memory/disk cache, but the empty-response throttle must still discriminate by + // identity: a sign-out/sign-in cycle to a different account carries a different + // session token (apiKey) on the same gateway URL, and must not have its empty-response + // signal suppressed by the previous account's throttle entry. + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetZooGatewayModels.mockResolvedValue({}) + + await freshGetModels({ provider: "zoo-gateway", apiKey: "account-a-token" }) + await freshGetModels({ provider: "zoo-gateway", apiKey: "account-a-token" }) + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + + await freshGetModels({ provider: "zoo-gateway", apiKey: "account-b-token" }) + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) + + it("throttles zoo-gateway independently per gateway baseUrl", async () => { + // Same session token, different gateway endpoint (e.g. staging vs. production) -- + // must also be treated as a distinct identity for throttle purposes. + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetZooGatewayModels.mockResolvedValue({}) + + await freshGetModels({ provider: "zoo-gateway", apiKey: "token", baseUrl: "https://gateway-a.example.com" }) + await freshGetModels({ provider: "zoo-gateway", apiKey: "token", baseUrl: "https://gateway-b.example.com" }) + + 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..5da790e4fa 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 @@ -48,7 +68,10 @@ const AUTH_SCOPED_PROVIDERS: ReadonlySet = new Set(["zoo-gateway"]) // Providers whose model list is determined by the server URL, not just by the provider name. // Each unique baseUrl must be cached independently so that switching endpoints never serves -// stale results from a previously-cached server. +// stale results from a previously-cached server. zoo-gateway is included here too: although +// it's auth-scoped and never actually persisted (see shouldSkipCache), getCacheKey() is also +// used to key the empty-response throttle (reportedEmptyModelResponse) and in-flight refresh +// map, both of which must still discriminate by endpoint even when caching itself is skipped. const URL_SCOPED_PROVIDERS: ReadonlySet = new Set([ "litellm", "poe", @@ -56,15 +79,20 @@ const URL_SCOPED_PROVIDERS: ReadonlySet = new Set([ "ollama", "lmstudio", "requesty", + "zoo-gateway", ]) // Providers where the API key itself determines which models are visible (e.g. per-key // allowlists). For these the cache key also includes a short hash of // the API key so that two different keys on the same server never share a cache entry. +// zoo-gateway is included so a sign-out/sign-in cycle to a different account (same gateway +// URL, different session token) doesn't collapse into the same throttle/in-flight identity -- +// see the URL_SCOPED_PROVIDERS comment above for why this matters despite caching being skipped. const KEY_SCOPED_PROVIDERS: ReadonlySet = new Set([ "litellm", // Per-key model allowlists are a first-class LiteLLM proxy feature "poe", // Per-account model availability "requesty", // Per-account custom model policies + "zoo-gateway", // Per-session-token account identity ]) function isAuthScopedProvider(provider: RouterName): boolean { @@ -247,39 +275,70 @@ export const getModels = async (options: GetModelsOptions): Promise const shouldSkipCache = isAuthScopedProvider(provider) - let models = shouldSkipCache ? undefined : getModelsFromCache(options) + const models = shouldSkipCache ? undefined : getModelsFromCache(options) if (models) { return models } - try { - models = await fetchModelsFromProvider(options) - const modelCount = Object.keys(models).length - - // 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) - - await writeModels(cacheKey, models).catch((err) => - console.error(`[MODEL_CACHE] Error writing ${cacheKey} models to file cache:`, err), - ) - } else if (modelCount === 0) { - TelemetryService.instance.captureEvent(TelemetryEventName.MODEL_CACHE_EMPTY_RESPONSE, { - provider, - context: "getModels", - hasExistingCache: false, - }) + // Route the cache-miss fetch through the same single-flight coordinator refreshModels() + // uses (inFlightRefresh), keyed on the same compound cacheKey. Without this, concurrent + // getModels() calls for the same key each independently miss the cache and fire their own + // redundant provider fetch, and a getModels() fetch racing a refreshModels() fetch for the + // same key has no ordering guarantee -- whichever call's memoryCache.set() lands last wins, + // even if it started (and thus reflects) an earlier, staler request. Sharing the map means + // every caller for a given key -- get or refresh -- converges on one in-flight fetch. + if (!shouldSkipCache) { + const existingRequest = inFlightRefresh.get(cacheKey) + if (existingRequest) { + return existingRequest } + } - return models - } catch (error) { - // Log the error and re-throw it so the caller can handle it (e.g., show a UI message). - console.error(`[getModels] Failed to fetch models in modelCache for ${provider}:`, error) + const fetchPromise = (async (): Promise => { + try { + const fetched = await fetchModelsFromProvider(options) + const modelCount = Object.keys(fetched).length + + // 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) { + // 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) + + if (!shouldSkipCache) { + memoryCache.set(cacheKey, fetched) + + await writeModels(cacheKey, fetched).catch((err) => + console.error(`[MODEL_CACHE] Error writing ${cacheKey} models to file cache:`, err), + ) + } + } else { + captureModelCacheEmptyResponseOnce(provider, cacheKey, { + context: "getModels", + hasExistingCache: false, + }) + } + + return fetched + } catch (error) { + // Log the error and re-throw it so the caller can handle it (e.g., show a UI message). + console.error(`[getModels] Failed to fetch models in modelCache for ${provider}:`, error) + + throw error // Re-throw the original error to be handled by the caller. + } finally { + if (!shouldSkipCache) { + inFlightRefresh.delete(cacheKey) + } + } + })() - throw error // Re-throw the original error to be handled by the caller. + if (!shouldSkipCache) { + inFlightRefresh.set(cacheKey, fetchPromise) } + + return fetchPromise } /** @@ -328,8 +387,7 @@ export const refreshModels = async (options: GetModelsOptions): Promise 0, existingCacheSize: existingCount, @@ -341,6 +399,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..ec4da6590a 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") @@ -214,6 +216,52 @@ 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, + // 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: maliciousName, + params: {}, + partial: false, + }, + ] + + await presentAssistantMessage(mockTask) + + expect(mockTask.recordToolUsage).not.toHaveBeenCalledWith(maliciousName) + expect(mockTask.recordToolError).not.toHaveBeenCalledWith(maliciousName, expect.anything()) + }) + + it("does not record a raw mcp_-prefixed model-supplied name in tool usage analytics either", async () => { + // isValidToolName() (mocked here to always return false, i.e. this suite's "unknown + // tool" path) would, in production, treat ANY "mcp_"-prefixed string as a valid + // dynamic MCP tool name. Exercise that prefix specifically so a crafted + // "mcp_" name is also confirmed to never reach analytics as a raw key. + const toolCallId = "tool_call_mcp_analytics_test" + const maliciousMcpName = "mcp_'; DROP TABLE users; --" + mockTask.assistantMessageContent = [ + { + type: "tool_use", + id: toolCallId, + name: maliciousMcpName, + params: {}, + partial: false, + }, + ] + + await presentAssistantMessage(mockTask) + + expect(mockTask.recordToolUsage).not.toHaveBeenCalledWith(maliciousMcpName) + expect(mockTask.recordToolError).not.toHaveBeenCalledWith(maliciousMcpName, expect.anything()) + }) + 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/__tests__/presentAssistantMessage-validation-error.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-validation-error.spec.ts new file mode 100644 index 0000000000..34fe48aaab --- /dev/null +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-validation-error.spec.ts @@ -0,0 +1,106 @@ +// npx vitest run src/core/assistant-message/__tests__/presentAssistantMessage-validation-error.spec.ts + +import { describe, it, expect, beforeEach, vi } from "vitest" +import { presentAssistantMessage } from "../presentAssistantMessage" + +// Mock dependencies +vi.mock("../../task/Task") +vi.mock("../../tools/validateToolUse", () => ({ + // isValidToolName: true means this is a *known* tool name, so it reaches recordToolUsage's + // gate -- but validateToolUse itself still throws (e.g. mode-disallowed), which is the gap + // this suite covers: that thrown error must not leave the attempt unrecorded in telemetry. + validateToolUse: vi.fn(() => { + throw new Error('Tool "read_file" is not allowed in ask mode.') + }), + isValidToolName: vi.fn(() => true), +})) +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + captureToolUsage: vi.fn(), + captureConsecutiveMistakeError: vi.fn(), + }, + }, +})) + +describe("presentAssistantMessage - validateToolUse throws", () => { + let mockTask: any + + beforeEach(() => { + mockTask = { + taskId: "test-task-id", + instanceId: "test-instance", + abort: false, + presentAssistantMessageLocked: false, + presentAssistantMessageHasPendingUpdates: false, + currentStreamingContentIndex: 0, + assistantMessageContent: [], + userMessageContent: [], + didCompleteReadingStream: false, + didRejectTool: false, + didAlreadyUseTool: false, + consecutiveMistakeCount: 0, + clineMessages: [], + api: { + getModel: () => ({ id: "test-model", info: {} }), + }, + recordToolUsage: vi.fn(), + recordToolError: vi.fn(), + toolRepetitionDetector: { + check: vi.fn().mockReturnValue({ allowExecution: true }), + }, + providerRef: { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "ask", + customModes: [], + }), + }), + }, + say: vi.fn().mockResolvedValue(undefined), + ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }), + } + + mockTask.pushToolResultToUserContent = vi.fn().mockImplementation((toolResult: any) => { + const existingResult = mockTask.userMessageContent.find( + (block: any) => block.type === "tool_result" && block.tool_use_id === toolResult.tool_use_id, + ) + if (existingResult) { + return false + } + mockTask.userMessageContent.push(toolResult) + return true + }) + }) + + it("records a tool error when a known tool fails validateToolUse validation", async () => { + const toolCallId = "tool_call_mode_disallowed" + mockTask.assistantMessageContent = [ + { + type: "tool_use", + id: toolCallId, + name: "read_file", + params: { path: "foo.ts" }, + nativeArgs: { path: "foo.ts" }, + partial: false, + }, + ] + + await presentAssistantMessage(mockTask) + + // The failed attempt must not vanish from telemetry: neither recordToolUsage (the tool + // never actually ran) nor silence -- it must show up via recordToolError instead. + expect(mockTask.recordToolUsage).not.toHaveBeenCalled() + expect(mockTask.recordToolError).toHaveBeenCalledWith( + "read_file", + expect.stringContaining("not allowed in ask mode"), + ) + + // The tool_result error is still sent to the model as before. + const toolResult = mockTask.userMessageContent.find( + (item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId, + ) + expect(toolResult).toBeDefined() + expect(toolResult.is_error).toBe(true) + }) +}) diff --git a/src/core/assistant-message/__tests__/toTelemetryToolName.spec.ts b/src/core/assistant-message/__tests__/toTelemetryToolName.spec.ts new file mode 100644 index 0000000000..e92401724c --- /dev/null +++ b/src/core/assistant-message/__tests__/toTelemetryToolName.spec.ts @@ -0,0 +1,42 @@ +// npx vitest run src/core/assistant-message/__tests__/toTelemetryToolName.spec.ts + +import { describe, it, expect } from "vitest" +import { toTelemetryToolName } from "../presentAssistantMessage" + +describe("toTelemetryToolName", () => { + it("records a statically known tool name as-is", () => { + expect(toTelemetryToolName("read_file", false)).toBe("read_file") + }) + + it("records a registered custom tool as the static 'custom_tool' key regardless of its actual name", () => { + expect(toTelemetryToolName("my_custom_tool", true)).toBe("custom_tool") + }) + + // Regression coverage: isValidToolName() accepts ANY string starting with "mcp_" as a + // dynamic MCP tool (the "mcp_serverName_toolName" convention), including + // model-controlled/malicious strings that merely happen to match the prefix. Recording + // block.name directly would let a crafted tool name become an arbitrary toolsUsed + // property key -- these must all bucket under the static "use_mcp_tool" key, matching + // how the dedicated mcp_tool_use block type always records. + it("records a well-formed dynamic MCP tool name as the static 'use_mcp_tool' key", () => { + expect(toTelemetryToolName("mcp_myServer_myTool", false)).toBe("use_mcp_tool") + }) + + it("records a malicious mcp_-prefixed name as 'use_mcp_tool', never as the raw string", () => { + const maliciousName = "mcp_'; DROP TABLE users; --" + expect(toTelemetryToolName(maliciousName, false)).toBe("use_mcp_tool") + expect(toTelemetryToolName(maliciousName, false)).not.toBe(maliciousName) + }) + + it("records an unknown/invalid tool name as 'invalid_tool_call'", () => { + const maliciousName = "'; DROP TABLE users; --" + expect(toTelemetryToolName(maliciousName, false)).toBe("invalid_tool_call") + }) + + it("prefers the mcp_ prefix bucket over isCustomTool=false unknown-name fallback", () => { + // Even without experiments/customTools context, an "mcp_" prefix always wins over + // falling through to "invalid_tool_call", since isValidToolName() treats any such + // name as valid. + expect(toTelemetryToolName("mcp_", false)).toBe("use_mcp_tool") + }) +}) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index f71b5cc1bd..313ef7bef2 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -41,6 +41,39 @@ import { codebaseSearchTool } from "../tools/CodebaseSearchTool" import { formatResponse } from "../prompts/responses" import { sanitizeToolUseId } from "../../utils/tool-id" +/** + * Maps a (possibly model-controlled) tool name to the key that's safe to record for + * telemetry (recordToolUsage/recordToolError). block.name must never reach analytics + * unvalidated, since it becomes a nested property key on the Task Completed event: + * - A registered custom tool records as "custom_tool". + * - A statically known tool name (including "use_mcp_tool" itself) records as-is. + * - A dynamic MCP tool name (the "mcp_serverName_toolName" convention accepted by + * isValidToolName) records as "use_mcp_tool", matching how the dedicated + * mcp_tool_use block type above always records -- the serverName/toolName portion + * is model-controlled and must never become a raw toolsUsed key itself. + * - Anything else (including a name that merely starts with "mcp_" but isn't a real + * dynamic MCP tool, or any other invalid/malformed name) records as "invalid_tool_call". + */ +export function toTelemetryToolName( + toolName: string, + isCustomTool: boolean, + experiments?: Record, +): ToolName { + if (isCustomTool) { + return "custom_tool" + } + + if (toolName.startsWith("mcp_")) { + return "use_mcp_tool" + } + + if (isValidToolName(toolName, experiments)) { + return toolName + } + + return "invalid_tool_call" +} + /** * Processes and presents assistant message content to the user interface. * @@ -234,8 +267,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 @@ -302,14 +336,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 } @@ -425,7 +455,10 @@ export async function presentAssistantMessage(cline: Task) { cline.consecutiveMistakeCount++ try { - cline.recordToolError(block.name as ToolName, errorMessage) + // isKnownTool is already true here, but block.name may still be a + // model-controlled "mcp_..." string rather than a real MCP tool -- + // bucket dynamic MCP names the same way as the recordToolUsage path. + cline.recordToolError(toTelemetryToolName(block.name, false, stateExperiments), errorMessage) } catch { // Best-effort only } @@ -553,23 +586,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 @@ -619,8 +635,40 @@ export async function presentAssistantMessage(cline: Task) { is_error: true, }) + // Record the failed attempt so it isn't silently missing from toolsUsed -- + // otherwise a tool that's disallowed for the current mode (or fails other + // validation) leaves no telemetry trace at all, unlike every other failure + // path in this function. Bucketed the same way as the success path above so + // block.name (model-controlled) never reaches analytics unvalidated. + const isCustomToolAttempt = Boolean( + stateExperiments?.customTools && customToolRegistry.has(block.name), + ) + cline.recordToolError( + toTelemetryToolName(block.name, isCustomToolAttempt, stateExperiments), + error.message, + ) + 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. Dynamic MCP tool + // names are bucketed under the static "use_mcp_tool" key by toTelemetryToolName, + // same as the mcp_tool_use block type above -- block.name here could otherwise be + // any model-controlled "mcp_..." string, not necessarily a real MCP tool. + const isCustomTool = Boolean(stateExperiments?.customTools && customToolRegistry.has(block.name)) + if (isCustomTool || isValidToolName(block.name, stateExperiments)) { + cline.recordToolUsage(toTelemetryToolName(block.name, isCustomTool, stateExperiments)) + } + + // 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. @@ -900,10 +948,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/core/task/Task.ts b/src/core/task/Task.ts index 7bee3c5e14..716cd9cd74 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,25 @@ export class Task extends EventEmitter implements TaskLike { consecutiveNoAssistantMessagesCount: number = 0 toolUsage: ToolUsage = {} + // Conversation message counts, summarized once per Task Completed + // installment instead of emitting a separate telemetry event per turn. + messageCounts: { user: number; assistant: number } = { user: 0, assistant: 0 } + + // Idle/shutdown telemetry flush: reports toolUsage/messageCounts for tasks that + // go quiet or get torn down without the model ever calling attempt_completion + // (or without the user accepting it), so long-running/abandoned tasks aren't + // invisible to telemetry. Each flush reports only what changed since the previous + // one, tracked via telemetryToolUsageBaseline/telemetryMessageCountsBaseline -- + // task.toolUsage/messageCounts themselves are never mutated by this, since they're + // also read as running totals by the public TaskCompleted API event and the UI. + // Checked on an interval rather than hooked into every say()/ask() call site. + private static readonly IDLE_TELEMETRY_CHECK_INTERVAL_MS = 5 * 60 * 1000 + private static readonly IDLE_TELEMETRY_THRESHOLD_MS = 30 * 60 * 1000 + private idleTelemetryCheckInterval?: NodeJS.Timeout + private lastTelemetryFlushAt: number = Date.now() + private telemetryToolUsageBaseline: ToolUsage = {} + private telemetryMessageCountsBaseline: { user: number; assistant: number } = { user: 0, assistant: 0 } + // Checkpoints enableCheckpoints: boolean checkpointTimeout: number @@ -593,6 +613,8 @@ export class Task extends EventEmitter implements TaskLike { { leading: true, trailing: true, maxWait: this.TOKEN_USAGE_EMIT_INTERVAL_MS }, ) + this.startIdleTelemetryCheck() + onCreated?.(this) if (startTask) { @@ -2232,6 +2254,17 @@ export class Task extends EventEmitter implements TaskLike { public dispose(): void { console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`) + // Stop the idle telemetry check and report any unflushed activity as a + // shutdown installment, so a task torn down mid-work (panel closed, task + // switched, extension deactivated) isn't invisible to telemetry. + try { + clearInterval(this.idleTelemetryCheckInterval) + this.idleTelemetryCheckInterval = undefined + this.flushTelemetryInstallment("shutdown") + } catch (error) { + console.error("Error flushing shutdown telemetry:", error) + } + // Cancel any in-progress HTTP request try { this.cancelCurrentRequest() @@ -2591,18 +2624,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 +3550,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. @@ -3623,8 +3654,13 @@ export class Task extends EventEmitter implements TaskLike { if (this.apiConversationHistory.length > 0) { const lastMessage = this.apiConversationHistory[this.apiConversationHistory.length - 1] if (lastMessage.role === "user") { - // Remove the last user message that we added earlier + // Remove the last user message that we added earlier. Decrement + // messageCounts.user to match -- both retry branches below mark + // userMessageWasRemoved so the message (and its count) is restored + // exactly once when the retry succeeds, keeping the total symmetric + // regardless of how many empty-response cycles occur first. this.apiConversationHistory.pop() + this.messageCounts.user-- } } @@ -3668,32 +3704,45 @@ export class Task extends EventEmitter implements TaskLike { if (response === "yesButtonClicked") { await this.say("api_req_retried") - // Push the same content back to retry + // Push the same content back to retry. Mark that user message was + // removed (same as the auto-retry path above) so it gets re-added -- + // and messageCounts.user re-incremented -- on the retried attempt; + // otherwise shouldAddUserMessageToHistory sees retryAttempt > 0 with + // userMessageWasRemoved unset and skips re-adding it entirely. stack.push({ userContent: currentUserContent, includeFileDetails: false, retryAttempt: (currentItem.retryAttempt ?? 0) + 1, + userMessageWasRemoved: true, }) // Continue to retry the request continue } else { // User declined to retry - // Re-add the user message we removed. + // Re-add the user message we removed (see messageCounts.user-- above) + // and increment messageCounts.user to match, same as the normal + // add-to-history path -- otherwise this abandoned-task path + // permanently undercounts by one. await this.addToApiConversationHistory({ role: "user", content: currentUserContent, }) + this.messageCounts.user++ await this.say( "error", "Unexpected API Response: The language model did not provide any assistant messages. This may indicate an issue with the API or the model's output.", ) + // Synthetic assistant message recording the failure -- increment + // messageCounts.assistant to match, same as the normal + // assistant-message-saved path. await this.addToApiConversationHistory({ role: "assistant", content: [{ type: "text", text: "Failure: I did not provide a response." }], }) + this.messageCounts.assistant++ } } } @@ -4641,6 +4690,70 @@ export class Task extends EventEmitter implements TaskLike { } } + /** + * Emits a Task Completed installment for whatever toolUsage/messageCounts have + * changed since the previous installment (from any reason), then advances the + * telemetry baseline so a later installment reports only its own delta. Does + * NOT touch task.toolUsage/messageCounts themselves -- those stay running totals + * for the public TaskCompleted API event and the UI. No-ops if nothing changed + * since the last installment, so idle/shutdown checks don't emit empty events + * for tasks that were already fully reported (e.g. right after attempt_completion). + */ + public flushTelemetryInstallment(reason: "attempt_completion" | "idle" | "shutdown"): void { + const toolUsageDelta: ToolUsage = {} + + for (const [toolName, usage] of Object.entries(this.toolUsage) as [ToolName, ToolUsage[ToolName]][]) { + if (!usage) { + continue + } + + const baseline = this.telemetryToolUsageBaseline[toolName] + const attempts = usage.attempts - (baseline?.attempts ?? 0) + const failures = usage.failures - (baseline?.failures ?? 0) + + if (attempts > 0 || failures > 0) { + toolUsageDelta[toolName] = { attempts, failures } + } + } + + const messageCountDelta = { + user: this.messageCounts.user - this.telemetryMessageCountsBaseline.user, + assistant: this.messageCounts.assistant - this.telemetryMessageCountsBaseline.assistant, + } + + const hasToolUsageDelta = Object.keys(toolUsageDelta).length > 0 + const hasMessageDelta = messageCountDelta.user > 0 || messageCountDelta.assistant > 0 + + if (!hasToolUsageDelta && !hasMessageDelta) { + return + } + + this.emitFinalTokenUsageUpdate() + TelemetryService.instance.captureTaskCompleted(this.taskId, toolUsageDelta, messageCountDelta, reason) + + this.telemetryToolUsageBaseline = JSON.parse(JSON.stringify(this.toolUsage)) + this.telemetryMessageCountsBaseline = { ...this.messageCounts } + this.lastTelemetryFlushAt = Date.now() + } + + private startIdleTelemetryCheck(): void { + this.idleTelemetryCheckInterval = setInterval(() => { + // lastMessageTs only moves forward on activity, so comparing it against the + // last flush tells us whether anything happened since that flush -- if the + // task has been quiet since well before the last flush, there's nothing new + // to report and flushTelemetryInstallment's own empty-check would no-op anyway, + // but skipping here avoids waking up to do that check needlessly. + const idleForMs = Date.now() - (this.lastMessageTs ?? this.lastTelemetryFlushAt) + + if (idleForMs >= Task.IDLE_TELEMETRY_THRESHOLD_MS) { + this.flushTelemetryInstallment("idle") + } + }, Task.IDLE_TELEMETRY_CHECK_INTERVAL_MS) + + // Don't hold the process open just for this timer. + this.idleTelemetryCheckInterval?.unref?.() + } + // Getters public get taskStatus(): TaskStatus { diff --git a/src/core/task/__tests__/Task.persistence.spec.ts b/src/core/task/__tests__/Task.persistence.spec.ts index 1761db5bc3..ba3edda81c 100644 --- a/src/core/task/__tests__/Task.persistence.spec.ts +++ b/src/core/task/__tests__/Task.persistence.spec.ts @@ -304,6 +304,10 @@ describe("Task persistence", () => { task: "test task", startTask: false, }) + // Dispose the idle-telemetry-check interval before running all timers, so + // runAllTimersAsync doesn't treat it as an infinite loop (it's unrelated + // to what this test exercises). + task.dispose() const promise = task.retrySaveApiConversationHistory() await vi.runAllTimersAsync() @@ -326,6 +330,10 @@ describe("Task persistence", () => { task: "test task", startTask: false, }) + // Dispose the idle-telemetry-check interval before running all timers, so + // runAllTimersAsync doesn't treat it as an infinite loop (it's unrelated + // to what this test exercises). + task.dispose() const promise = task.retrySaveApiConversationHistory() await vi.runAllTimersAsync() diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 27ba5ce8ff..0789a3948b 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -2720,6 +2720,233 @@ describe("Cline", () => { }) }) +describe("Telemetry installments (idle/shutdown flush)", () => { + let mockProvider: any + let mockApiConfig: ProviderSettings + let mockExtensionContext: vscode.ExtensionContext + let captureTaskCompletedSpy: ReturnType + + beforeEach(() => { + if (!TelemetryService.hasInstance()) { + TelemetryService.createInstance([]) + } + + captureTaskCompletedSpy = vi.spyOn(TelemetryService.instance, "captureTaskCompleted") + + const storageUri = { fsPath: path.join(os.tmpdir(), "test-storage") } + + mockExtensionContext = { + globalState: { + get: vi.fn().mockReturnValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + globalStorageUri: storageUri, + workspaceState: { + get: vi.fn().mockReturnValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + secrets: { + get: vi.fn().mockResolvedValue(undefined), + store: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + }, + extensionUri: { fsPath: "/mock/extension/path" }, + extension: { packageJSON: { version: "1.0.0" } }, + } as unknown as vscode.ExtensionContext + + mockProvider = new ClineProvider( + mockExtensionContext, + { + appendLine: vi.fn(), + append: vi.fn(), + clear: vi.fn(), + show: vi.fn(), + hide: vi.fn(), + dispose: vi.fn(), + } as unknown as vscode.OutputChannel, + "sidebar", + new ContextProxy(mockExtensionContext), + ) as any + mockProvider.postMessageToWebview = vi.fn().mockResolvedValue(undefined) + mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined) + + mockApiConfig = { + apiProvider: "anthropic", + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-api-key", + } + }) + + afterEach(() => { + vi.useRealTimers() + captureTaskCompletedSpy.mockRestore() + }) + + function createTask() { + return new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + } + + describe("flushTelemetryInstallment", () => { + it("reports nothing and does not call captureTaskCompleted when there is no new activity", () => { + const task = createTask() + + task.flushTelemetryInstallment("idle") + + expect(captureTaskCompletedSpy).not.toHaveBeenCalled() + }) + + it("reports the current toolUsage/messageCounts as the delta on the first flush", () => { + const task = createTask() + task.recordToolUsage("read_file") + task.recordToolUsage("read_file") + task.messageCounts = { user: 2, assistant: 3 } + + task.flushTelemetryInstallment("idle") + + expect(captureTaskCompletedSpy).toHaveBeenCalledWith( + task.taskId, + { read_file: { attempts: 2, failures: 0 } }, + { user: 2, assistant: 3 }, + "idle", + ) + }) + + it("does not mutate task.toolUsage/messageCounts (they stay running totals for the public API/UI)", () => { + const task = createTask() + task.recordToolUsage("read_file") + task.messageCounts = { user: 1, assistant: 1 } + + task.flushTelemetryInstallment("idle") + + expect(task.toolUsage).toEqual({ read_file: { attempts: 1, failures: 0 } }) + expect(task.messageCounts).toEqual({ user: 1, assistant: 1 }) + }) + + it("reports only the delta since the previous installment on a second flush", () => { + const task = createTask() + task.recordToolUsage("read_file") + task.messageCounts = { user: 1, assistant: 1 } + task.flushTelemetryInstallment("idle") + captureTaskCompletedSpy.mockClear() + + task.recordToolUsage("read_file") + task.recordToolUsage("write_to_file") + task.messageCounts = { user: 3, assistant: 2 } + task.flushTelemetryInstallment("shutdown") + + expect(captureTaskCompletedSpy).toHaveBeenCalledWith( + task.taskId, + { read_file: { attempts: 1, failures: 0 }, write_to_file: { attempts: 1, failures: 0 } }, + { user: 2, assistant: 1 }, + "shutdown", + ) + }) + + it("does not emit an empty second installment when nothing changed since the first flush", () => { + const task = createTask() + task.recordToolUsage("read_file") + task.flushTelemetryInstallment("idle") + captureTaskCompletedSpy.mockClear() + + task.flushTelemetryInstallment("shutdown") + + expect(captureTaskCompletedSpy).not.toHaveBeenCalled() + }) + + it("includes failure deltas alongside attempt deltas", () => { + const task = createTask() + task.recordToolUsage("read_file") + task.flushTelemetryInstallment("idle") + captureTaskCompletedSpy.mockClear() + + task.recordToolError("read_file") + + task.flushTelemetryInstallment("shutdown") + + expect(captureTaskCompletedSpy).toHaveBeenCalledWith( + task.taskId, + { read_file: { attempts: 0, failures: 1 } }, + { user: 0, assistant: 0 }, + "shutdown", + ) + }) + }) + + describe("idle flush timer", () => { + it("flushes once activity has been quiet for the idle threshold", () => { + vi.useFakeTimers() + const task = createTask() + task.recordToolUsage("read_file") + + vi.advanceTimersByTime(31 * 60 * 1000) + + expect(captureTaskCompletedSpy).toHaveBeenCalledWith( + task.taskId, + { read_file: { attempts: 1, failures: 0 } }, + { user: 0, assistant: 0 }, + "idle", + ) + }) + + it("does not flush before the idle threshold has elapsed", () => { + vi.useFakeTimers() + const task = createTask() + task.recordToolUsage("read_file") + + vi.advanceTimersByTime(10 * 60 * 1000) + + expect(captureTaskCompletedSpy).not.toHaveBeenCalled() + }) + }) + + describe("dispose", () => { + it("flushes unreported activity as a shutdown installment", () => { + const task = createTask() + task.recordToolUsage("read_file") + task.messageCounts = { user: 1, assistant: 1 } + + task.dispose() + + expect(captureTaskCompletedSpy).toHaveBeenCalledWith( + task.taskId, + { read_file: { attempts: 1, failures: 0 } }, + { user: 1, assistant: 1 }, + "shutdown", + ) + }) + + it("does not flush again if everything was already reported before dispose", () => { + const task = createTask() + task.recordToolUsage("read_file") + task.flushTelemetryInstallment("attempt_completion") + captureTaskCompletedSpy.mockClear() + + task.dispose() + + expect(captureTaskCompletedSpy).not.toHaveBeenCalled() + }) + + it("stops the idle timer so a disposed task never flushes again", () => { + vi.useFakeTimers() + const task = createTask() + task.recordToolUsage("read_file") + task.dispose() + captureTaskCompletedSpy.mockClear() + + vi.advanceTimersByTime(60 * 60 * 1000) + + expect(captureTaskCompletedSpy).not.toHaveBeenCalled() + }) + }) +}) + describe("Queued message processing after condense", () => { function createProvider(): any { const storageUri = { fsPath: path.join(os.tmpdir(), "test-storage") } diff --git a/src/core/task/__tests__/messageCounting.retrySymmetry.spec.ts b/src/core/task/__tests__/messageCounting.retrySymmetry.spec.ts new file mode 100644 index 0000000000..bedc42d8bb --- /dev/null +++ b/src/core/task/__tests__/messageCounting.retrySymmetry.spec.ts @@ -0,0 +1,108 @@ +// npx vitest run core/task/__tests__/messageCounting.retrySymmetry.spec.ts + +import { shouldAddUserMessageToHistory } from "../messageCounting" + +/** + * Regression coverage for the empty-assistant-response retry cycle in + * Task#recursivelyMakeClineRequests: the user message is added once (incrementing + * messageCounts.user), popped when the assistant fails to respond (decrementing + * messageCounts.user to match), then re-added on retry via userMessageWasRemoved + * (incrementing messageCounts.user again). This exercises that increment/decrement/ + * increment sequence end-to-end using the same primitives Task.ts calls, without + * needing to drive the full streaming loop -- see + * apps/vscode-e2e/src/suite/providers/bedrock-empty-response-retry.test.ts for the + * end-to-end proof that the retried request actually includes the user's message. + */ +describe("empty-assistant-response retry keeps messageCounts.user symmetric", () => { + function simulateAttempt( + messageCounts: { user: number; assistant: number }, + params: { + retryAttempt: number | undefined + isEmptyUserContent: boolean + userMessageWasRemoved: boolean | undefined + }, + ) { + if (shouldAddUserMessageToHistory(params)) { + messageCounts.user++ + } + } + + function simulatePopOnEmptyResponse(messageCounts: { user: number; assistant: number }) { + // Mirrors Task.ts: popping the just-added user message from apiConversationHistory + // is paired with decrementing messageCounts.user to match. + messageCounts.user-- + } + + function simulateDeclineRetry(messageCounts: { user: number; assistant: number }) { + // Mirrors Task.ts's "User declined to retry" branch: the popped user message is + // re-added directly (not via shouldAddUserMessageToHistory) and a synthetic failure + // assistant message is appended, so both increments are paired explicitly here + // rather than going through simulateAttempt. + messageCounts.user++ + messageCounts.assistant++ + } + + it("ends at 1 after one empty-response retry that then succeeds (not 2)", () => { + const messageCounts = { user: 0, assistant: 0 } + + // First attempt: message added, count incremented. + simulateAttempt(messageCounts, { retryAttempt: 0, isEmptyUserContent: false, userMessageWasRemoved: false }) + expect(messageCounts.user).toBe(1) + + // Assistant returns nothing -- Task.ts pops the message it just added. + simulatePopOnEmptyResponse(messageCounts) + expect(messageCounts.user).toBe(0) + + // Retry (either auto or manual-approved branch) re-adds it via userMessageWasRemoved. + simulateAttempt(messageCounts, { retryAttempt: 1, isEmptyUserContent: false, userMessageWasRemoved: true }) + + // Exactly one logical user turn occurred -- the count must reflect that, not 2. + expect(messageCounts.user).toBe(1) + }) + + it("stays symmetric across multiple consecutive empty-response retries", () => { + const messageCounts = { user: 0, assistant: 0 } + + simulateAttempt(messageCounts, { retryAttempt: 0, isEmptyUserContent: false, userMessageWasRemoved: false }) + simulatePopOnEmptyResponse(messageCounts) + + simulateAttempt(messageCounts, { retryAttempt: 1, isEmptyUserContent: false, userMessageWasRemoved: true }) + simulatePopOnEmptyResponse(messageCounts) + + simulateAttempt(messageCounts, { retryAttempt: 2, isEmptyUserContent: false, userMessageWasRemoved: true }) + simulatePopOnEmptyResponse(messageCounts) + + // Final successful attempt. + simulateAttempt(messageCounts, { retryAttempt: 3, isEmptyUserContent: false, userMessageWasRemoved: true }) + + expect(messageCounts.user).toBe(1) + }) + + it("never goes negative when a message is popped and correctly re-added", () => { + const messageCounts = { user: 0, assistant: 0 } + + simulateAttempt(messageCounts, { retryAttempt: 0, isEmptyUserContent: false, userMessageWasRemoved: false }) + simulatePopOnEmptyResponse(messageCounts) + simulateAttempt(messageCounts, { retryAttempt: 1, isEmptyUserContent: false, userMessageWasRemoved: true }) + + expect(messageCounts.user).toBeGreaterThanOrEqual(0) + }) + + it("stays symmetric when the user declines to retry after an empty response", () => { + const messageCounts = { user: 0, assistant: 0 } + + // First attempt: user message added, count incremented. + simulateAttempt(messageCounts, { retryAttempt: 0, isEmptyUserContent: false, userMessageWasRemoved: false }) + expect(messageCounts).toEqual({ user: 1, assistant: 0 }) + + // Assistant returns nothing -- popped, decremented. + simulatePopOnEmptyResponse(messageCounts) + expect(messageCounts).toEqual({ user: 0, assistant: 0 }) + + // User declines to retry: message is re-added directly, plus a synthetic failure + // assistant message -- exactly one logical user turn and one assistant turn + // occurred overall, so both counters must end at 1, not 0. + simulateDeclineRetry(messageCounts) + expect(messageCounts).toEqual({ user: 1, assistant: 1 }) + }) +}) 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/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/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index 39feae0d9b..3239439ba1 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -1,7 +1,6 @@ import * as vscode from "vscode" import { RooCodeEventName, type HistoryItem } from "@roo-code/types" -import { TelemetryService } from "@roo-code/telemetry" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" @@ -81,6 +80,19 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { await task.say("completion_result", result, undefined, false) + // Whether this attempt_completion call is a stale replay of an already-completed + // subtask (user revisiting it from history) rather than a live model-initiated + // completion. Determined below, before telemetry is flushed, so a replay -- which + // runs this handler again on a fresh Task instance with a zero telemetry baseline + // -- doesn't produce a duplicate "attempt_completion" installment for work that + // was already reported when the subtask first completed. + let isStaleHistoryReplay = false + // Whether the delegation branch below already flushed telemetry (it needs to + // flush before delegateToParent, which may return early) -- prevents the shared + // fallthrough flush from double-reporting when delegation falls through to + // "continue" instead of returning. + let hasFlushedTelemetry = false + // Check for subtask using parentTaskId (metadata-driven delegation) if (task.parentTaskId) { // Check if this subtask has already completed and returned to parent @@ -97,6 +109,7 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { // Fall through to normal completion ask flow below (outside this if block) // This shows the user the completion result and waits for acceptance // without injecting another tool_result to the parent + isStaleHistoryReplay = true } else if (status === "active" || status === "interrupted") { historyLookupTaskId = task.parentTaskId const { historyItem: parentHistory } = await provider.getTaskWithId(task.parentTaskId) @@ -105,6 +118,15 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { (parentHistory?.status === "delegated" || parentHistory?.status === "active") && parentHistory?.awaitingChildId === task.taskId ) { + // Known not to be a stale history replay (status was "active", not + // "completed"), so flush telemetry before the delegation call, which + // may return early below. hasFlushedTelemetry prevents the shared + // fallthrough flush further down from double-reporting if delegation + // falls through to "continue" instead of returning. + task.emitFinalTokenUsageUpdate() + task.flushTelemetryInstallment("attempt_completion") + hasFlushedTelemetry = true + const delegation = await this.delegateToParent( task, result, @@ -113,7 +135,7 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { pushToolResult, ) if (delegation === "delegated") { - this.emitTaskCompleted(task) + this.emitPublicTaskCompleted(task) } if (delegation !== "continue") return } else { @@ -147,10 +169,30 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { } } + // PostHog telemetry: report here, once per model-initiated attempt_completion + // call, regardless of whether the user goes on to accept, decline, or give + // feedback. Gating this on user acceptance previously meant a task that never + // got an explicit "yes" (declined, abandoned mid-review, etc.) reported nothing + // at all. This is independent of the public TaskCompleted API event, which still + // only fires once the task is genuinely finished. Skipped for a stale history + // replay (revisiting an already-completed subtask) since that reruns this handler + // on a fresh Task instance and would otherwise double-report work already flushed + // when the subtask first completed, and skipped if the delegation branch above + // already flushed. + if (!isStaleHistoryReplay && !hasFlushedTelemetry) { + task.emitFinalTokenUsageUpdate() + task.flushTelemetryInstallment("attempt_completion") + } + const { response, text, images } = await task.ask("completion_result", "", false) if (response === "yesButtonClicked") { - this.emitTaskCompleted(task) + // A stale history replay reruns this handler on a fresh Task instance for a + // subtask that already completed (and already emitted TaskCompleted) the first + // time through -- re-acknowledging it from history must not emit it again. + if (!isStaleHistoryReplay) { + this.emitPublicTaskCompleted(task) + } return } @@ -217,12 +259,17 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { } } - private emitTaskCompleted(task: Task): void { + /** + * Emits the public RooCodeEventName.TaskCompleted API event. Only called once the + * task is genuinely finished (user accepted, or a subtask was successfully delegated + * back to its parent) -- unlike the PostHog telemetry flush, which reports on every + * model-initiated attempt_completion call regardless of outcome. + */ + private emitPublicTaskCompleted(task: Task): void { // Force final token usage update before emitting TaskCompleted. // This ensures the latest stats are captured regardless of throttle timer. task.emitFinalTokenUsageUpdate() - TelemetryService.instance.captureTaskCompleted(task.taskId) task.emit(RooCodeEventName.TaskCompleted, task.taskId, task.getTokenUsage(), task.toolUsage) } } 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__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index 86ff112585..a31c993d6f 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/src/core/tools/__tests__/attemptCompletionTool.spec.ts @@ -81,9 +81,13 @@ 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, + flushTelemetryInstallment: vi.fn((reason: "attempt_completion" | "idle" | "shutdown") => { + mockCaptureTaskCompleted(mockTask.taskId, mockTask.toolUsage, mockTask.messageCounts, reason) + }), } }) @@ -585,9 +589,108 @@ describe("attemptCompletionTool", () => { }) expect(mockTask.ask).toHaveBeenCalledWith("completion_result", "", false) expect(mockPushToolResult).not.toHaveBeenCalledWith("") + // Emission now happens once per validated attempt_completion call, before + // delegation is attempted -- independent of whether delegation succeeds. + expect(mockCaptureTaskCompleted).toHaveBeenCalledTimes(1) + }) + + it("does not emit a duplicate telemetry installment when replaying an already-completed subtask from history", async () => { + // Simulates the user revisiting a subtask from history: the child's + // historyItem.status is already "completed", so this handler runs again + // on a fresh Task instance (zero telemetry baseline). It must not report + // toolUsage/messageCounts as a "new" installment for work that was already + // flushed when the subtask first completed. + const block: AttemptCompletionToolUse = { + type: "tool_use", + name: "attempt_completion", + params: { result: "9" }, + nativeArgs: { result: "9" }, + partial: false, + } + const mockProvider = { + log: vi.fn(), + getTaskWithId: vi.fn().mockImplementation((id: string) => { + if (id === "child-1") { + return Promise.resolve({ historyItem: { id, status: "completed" } }) + } + throw new Error(`unexpected task id ${id}`) + }), + reopenParentFromDelegation: vi.fn(), + } + + Object.assign(mockTask, { + taskId: "child-1", + parentTaskId: "parent-1", + providerRef: { deref: () => mockProvider }, + toolUsage: { read_file: { attempts: 5, failures: 0 } }, + messageCounts: { user: 3, assistant: 4 }, + }) + + const callbacks: AttemptCompletionCallbacks = { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + askFinishSubTaskApproval: mockAskFinishSubTaskApproval, + toolDescription: mockToolDescription, + } + + await attemptCompletionTool.handle(mockTask as Task, block, callbacks) + + expect(mockAskFinishSubTaskApproval).not.toHaveBeenCalled() + expect(mockProvider.reopenParentFromDelegation).not.toHaveBeenCalled() + expect(mockTask.ask).toHaveBeenCalledWith("completion_result", "", false) expect(mockCaptureTaskCompleted).not.toHaveBeenCalled() }) + it("does not emit a duplicate public TaskCompleted event when replaying an already-completed subtask from history", async () => { + // Same stale-replay scenario as above, but the user then clicks the completion + // result's "yes" button again while revisiting -- this reruns the handler on a + // fresh Task instance, so the public RooCodeEventName.TaskCompleted API event + // (task.emit) must not fire a second time for a subtask that already completed + // (and already emitted it) the first time through. + const block: AttemptCompletionToolUse = { + type: "tool_use", + name: "attempt_completion", + params: { result: "9" }, + nativeArgs: { result: "9" }, + partial: false, + } + const mockProvider = { + log: vi.fn(), + getTaskWithId: vi.fn().mockImplementation((id: string) => { + if (id === "child-1") { + return Promise.resolve({ historyItem: { id, status: "completed" } }) + } + throw new Error(`unexpected task id ${id}`) + }), + reopenParentFromDelegation: vi.fn(), + } + + Object.assign(mockTask, { + taskId: "child-1", + parentTaskId: "parent-1", + providerRef: { deref: () => mockProvider }, + ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked", text: "", images: [] }), + }) + + const callbacks: AttemptCompletionCallbacks = { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + askFinishSubTaskApproval: mockAskFinishSubTaskApproval, + toolDescription: mockToolDescription, + } + + await attemptCompletionTool.handle(mockTask as Task, block, callbacks) + + expect(mockTask.emit).not.toHaveBeenCalledWith( + RooCodeEventName.TaskCompleted, + expect.anything(), + expect.anything(), + expect.anything(), + ) + }) + it("does not resume the parent when the parent is no longer awaiting this child", async () => { const block: AttemptCompletionToolUse = { type: "tool_use", @@ -633,7 +736,12 @@ 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 }, + "attempt_completion", + ) }) it("delegates an interrupted subtask completion when the parent is still delegated and awaiting that child", async () => { @@ -732,10 +840,15 @@ 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 }, + "attempt_completion", + ) }) - it("emits TaskCompleted only when completion is accepted", async () => { + it("emits the public TaskCompleted API event only when completion is accepted, but reports telemetry either way", async () => { const block: AttemptCompletionToolUse = { type: "tool_use", name: "attempt_completion", @@ -757,7 +870,12 @@ 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 }, + "attempt_completion", + ) expect(mockTask.emit).toHaveBeenCalledWith( RooCodeEventName.TaskCompleted, "task_1", @@ -766,7 +884,44 @@ describe("attemptCompletionTool", () => { ) }) - it("does not emit TaskCompleted when user provides follow-up feedback", async () => { + 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 }, + "attempt_completion", + ) + }) + + it("still reports telemetry for a model-initiated completion even when the user provides follow-up feedback instead of accepting", async () => { + // A task that's declined/given feedback and never explicitly accepted previously + // reported nothing to telemetry at all -- this call is what a long-running or + // ultimately-abandoned task relies on to be visible in aggregate metrics. const block: AttemptCompletionToolUse = { type: "tool_use", name: "attempt_completion", @@ -792,7 +947,14 @@ describe("attemptCompletionTool", () => { await attemptCompletionTool.handle(mockTask as Task, block, callbacks) expect(mockHandleError).not.toHaveBeenCalled() - expect(mockCaptureTaskCompleted).not.toHaveBeenCalled() + expect(mockCaptureTaskCompleted).toHaveBeenCalledWith( + "task_1", + {}, + { user: 0, assistant: 0 }, + "attempt_completion", + ) + // The public RooCodeEventName.TaskCompleted API event is unaffected by this + // change -- it still only fires once the user actually accepts. expect(mockTask.emit).not.toHaveBeenCalledWith( RooCodeEventName.TaskCompleted, expect.anything(), 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 () => { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 6e5545a1f2..e36da99b28 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2361,6 +2361,7 @@ export class ClineProvider const telemetryKey = process.env.POSTHOG_API_KEY const machineId = vscode.env.machineId + const vscodeTelemetryEnabled = vscode.env.isTelemetryEnabled const mergedAllowedCommands = this.mergeAllowedCommands(allowedCommands) const mergedDeniedCommands = this.mergeDeniedCommands(deniedCommands) const cwd = this.cwd @@ -2461,6 +2462,7 @@ export class ClineProvider telemetrySetting, telemetryKey, machineId, + vscodeTelemetryEnabled, showRooIgnoredFiles: showRooIgnoredFiles ?? false, enableSubfolderRules: enableSubfolderRules ?? false, language: language ?? formatLanguage(vscode.env.language), diff --git a/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts b/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts index a99c8862a3..0f1e0e1315 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 fire an opt-out event when going from unset to disabled (explicit Decline)", () => { 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,10 @@ describe("Telemetry Settings Tracking", () => { TelemetryService.instance.updateTelemetryState(isOptedIn) + // "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) }) }) @@ -72,8 +75,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 +98,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 +117,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 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 - 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 +134,21 @@ describe("Telemetry Settings Tracking", () => { TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) } - // unset is treated as opted-in, so no event should fire + // "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("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, 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/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 335911a7c7..2b3001257d 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -11,6 +11,10 @@ vi.mock("../../../api/providers/fetchers/lmstudio", () => ({ getLMStudioModels: vi.fn(), })) +vi.mock("../../../integrations/theme/getTheme", () => ({ + getTheme: vi.fn().mockResolvedValue({}), +})) + vi.mock("../../../integrations/openai-codex/oauth", () => ({ openAiCodexOAuthManager: { getAccessToken: vi.fn(), @@ -51,6 +55,16 @@ vi.mock("../rulesMessageHandler", () => ({ handleOpenRulesDirectory: vi.fn(), })) +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + hasInstance: vi.fn().mockReturnValue(false), + instance: { + updateTelemetryState: vi.fn(), + captureTelemetrySettingsChanged: vi.fn(), + }, + }, +})) + import type { ModelRecord } from "@roo-code/types" import { webviewMessageHandler } from "../webviewMessageHandler" @@ -127,6 +141,9 @@ vi.mock("vscode", () => { commands: { executeCommand: vi.fn().mockResolvedValue(undefined), }, + env: { + isTelemetryEnabled: true, + }, } }) @@ -1428,3 +1445,198 @@ describe("zooCodeSignOut", () => { ) }) }) + +describe("webviewMessageHandler - telemetrySetting", () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(vscode.env).isTelemetryEnabled = true + }) + + // Regression test: TelemetryService.updateTelemetryState must be gated on + // vscode.env.isTelemetryEnabled in addition to the stored setting, matching + // extension.ts's onDidChangeTelemetryEnabled listener. Without this AND, a user + // clicking Accept in the webview could re-enable telemetry even while VS Code's + // global telemetry toggle is off. + it("does not enable telemetry when the user accepts but VS Code's global telemetry toggle is off", async () => { + const { TelemetryService } = await import("@roo-code/telemetry") + vi.mocked(TelemetryService.hasInstance).mockReturnValue(true) + vi.mocked(vscode.env).isTelemetryEnabled = false + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + + await webviewMessageHandler(mockClineProvider, { type: "telemetrySetting", text: "enabled" }) + + expect(TelemetryService.instance.updateTelemetryState).toHaveBeenCalledWith(false) + }) + + it("enables telemetry when the user accepts and VS Code's global telemetry toggle is on", async () => { + const { TelemetryService } = await import("@roo-code/telemetry") + vi.mocked(TelemetryService.hasInstance).mockReturnValue(true) + vi.mocked(vscode.env).isTelemetryEnabled = true + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + + await webviewMessageHandler(mockClineProvider, { type: "telemetrySetting", text: "enabled" }) + + expect(TelemetryService.instance.updateTelemetryState).toHaveBeenCalledWith(true) + }) + + it("keeps telemetry disabled when the user declines, regardless of VS Code's global toggle", async () => { + const { TelemetryService } = await import("@roo-code/telemetry") + vi.mocked(TelemetryService.hasInstance).mockReturnValue(true) + vi.mocked(vscode.env).isTelemetryEnabled = true + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + + await webviewMessageHandler(mockClineProvider, { type: "telemetrySetting", text: "disabled" }) + + expect(TelemetryService.instance.updateTelemetryState).toHaveBeenCalledWith(false) + }) + + // Finding #12 regression: without serialization, two concurrent "telemetrySetting" messages + // each capture their own isOptedIn in a closure and apply it to TelemetryService whenever + // their own persistence write resolves -- with no ordering guarantee between the two + // invocations. A slow first write racing a fast second write could let the *first* + // message's (now-stale) intent win the live telemetry state, even though the *second* + // message reflects the user's actual final choice. + it("applies the most recently sent telemetrySetting last, even if an earlier message's write is slower", async () => { + const { TelemetryService } = await import("@roo-code/telemetry") + vi.mocked(TelemetryService.hasInstance).mockReturnValue(true) + vi.mocked(vscode.env).isTelemetryEnabled = true + + // Track the "stored" setting so the second call's getGlobalState read reflects + // whatever the first call has (or hasn't yet) written -- mirrors ContextProxy's real + // synchronous stateCache update inside setValue. + let storedSetting: string | undefined + vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation(() => storedSetting) + + let resolveSlowWrite!: () => void + const slowWrite = new Promise((resolve) => { + resolveSlowWrite = resolve + }) + + vi.mocked(mockClineProvider.contextProxy.setValue).mockImplementation(async (_key, value) => { + if (value === "disabled") { + // First message's write is slow -- resolves only after we explicitly release it + // below, once the second (fast) message has already been sent. + await slowWrite + } + storedSetting = value as string + }) + + // First message: turn telemetry off (slow write). + const first = webviewMessageHandler(mockClineProvider, { type: "telemetrySetting", text: "disabled" }) + + // Second message: turn telemetry back on (fast write), sent immediately after. + const second = webviewMessageHandler(mockClineProvider, { type: "telemetrySetting", text: "enabled" }) + + // Now let the first message's write proceed. + resolveSlowWrite() + + await Promise.all([first, second]) + + // The user's final, most-recently-sent choice was "enabled" -- the live telemetry + // state must reflect that, not "disabled" from the stale, slower first message. + const calls = vi.mocked(TelemetryService.instance.updateTelemetryState).mock.calls + expect(calls.at(-1)).toEqual([true]) + }) + + // CodeRabbit follow-up on the finding #12 fix: webviewDidLaunch's telemetry init read state + // via an async provider.getStateToPostToWebview().then(...) continuation, outside + // telemetrySettingQueue -- so it could resolve after a concurrent "telemetrySetting" message + // and clobber that message's queued (correct) update with a stale value. webviewDidLaunch now + // reads getGlobalState synchronously and is routed through the same queue. + it("does not let webviewDidLaunch's telemetry init race and clobber a concurrent telemetrySetting message", async () => { + const { TelemetryService } = await import("@roo-code/telemetry") + vi.mocked(TelemetryService.hasInstance).mockReturnValue(true) + vi.mocked(vscode.env).isTelemetryEnabled = true + + // webviewDidLaunch starts out "unset" (disclosed opt-out default -- opted in). Scoped to + // the "telemetrySetting" key specifically -- webviewDidLaunch also calls + // updateGlobalState("customModes", ...) through the same contextProxy mock, which must + // not clobber storedSetting. + let storedSetting: string | undefined = "unset" + vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation((key: string) => + key === "telemetrySetting" ? storedSetting : undefined, + ) + vi.mocked(mockClineProvider.contextProxy.setValue).mockImplementation(async (key: string, value) => { + if (key === "telemetrySetting") { + storedSetting = value as string + } + }) + + vi.mocked(mockClineProvider.customModesManager.getCustomModes).mockResolvedValue([]) + ;(mockClineProvider as any).getMcpHub = vi.fn().mockReturnValue(undefined) + ;(mockClineProvider as any).providerSettingsManager = { + listConfig: vi.fn().mockResolvedValue(undefined), + } + + // Deferred-promise handshake instead of setTimeout delays, so ordering is enforced + // explicitly rather than by racing real clock delays. Signals when webviewDidLaunch has + // taken its (pre-fix) state snapshot -- only fires under the *old* code path + // (provider.getStateToPostToWebview().then(...)); the fix never calls it at all. + let snapshotTaken!: () => void + const snapshotTakenPromise = new Promise((resolve) => { + snapshotTaken = resolve + }) + let releaseSnapshot!: () => void + const snapshotReleased = new Promise((resolve) => { + releaseSnapshot = resolve + }) + + // Snapshots storedSetting at call time (mirroring the real ClineProvider building its + // state object synchronously before any internal awaits), signals it was taken, then + // waits until the test explicitly releases it -- by which point the concurrent + // telemetrySetting write below has already landed, making the snapshot genuinely stale + // once its .then() callback finally runs. + ;(mockClineProvider as any).getStateToPostToWebview = vi.fn().mockImplementation(async () => { + const snapshot = storedSetting + snapshotTaken() + await snapshotReleased + return { telemetrySetting: snapshot } + }) + + // webviewDidLaunch fires first (e.g. webview reload) -- its telemetry init is now queued + // behind telemetrySettingQueue rather than resolving independently. + const launch = webviewMessageHandler(mockClineProvider, { type: "webviewDidLaunch" } as any) + + // Wait for webviewDidLaunch to either take its (pre-fix) snapshot, or flush a fixed + // number of microtask turns as a same-tick fallback for the fixed code path (which never + // triggers that signal) -- enough for its synchronous prefix (await getCustomModes(), + // await updateGlobalState()) to run, without relying on a wall-clock timer. + await Promise.race([ + snapshotTakenPromise, + (async () => { + for (let i = 0; i < 10; i++) { + await Promise.resolve() + } + })(), + ]) + + // A concurrent "telemetrySetting" message turns telemetry off, and is awaited to + // completion -- including its own updateTelemetryState(false) call -- *before* the + // deferred (pre-fix-only) snapshot below is released. Against the pre-fix code, this + // proves the snapshot it captured earlier ("unset") is genuinely stale by the time its + // .then() callback finally runs: the user's real, later choice already landed. + const disable = webviewMessageHandler(mockClineProvider, { type: "telemetrySetting", text: "disabled" }) + await disable + + // Now release the deferred snapshot so a getStateToPostToWebview() call, if the old code + // path is exercised, resolves (with its already-captured, now-stale value) only after + // the disable write above has fully landed. + const snapshotResolved = vi.mocked((mockClineProvider as any).getStateToPostToWebview).mock.results[0] + ?.value as Promise | undefined + releaseSnapshot() + + await Promise.all([launch, snapshotResolved]) + + // webviewDidLaunch's telemetry init is fire-and-forget from the handler's own point of + // view (the "webviewDidLaunch" case doesn't await it), so even awaiting + // getStateToPostToWebview() directly isn't enough to observe its .then() callback -- + // flush one more microtask turn for that callback to run. + await Promise.resolve() + + // The user's explicit "disabled" choice must be the final state -- webviewDidLaunch's + // queued re-application of the (by-then-stale) "unset"/opted-in state must not run after + // and override it. + const calls = vi.mocked(TelemetryService.instance.updateTelemetryState).mock.calls + expect(calls.at(-1)).toEqual([false]) + }) +}) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index fdcc53ddc1..ee687b7e85 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" @@ -86,6 +87,15 @@ import { getLMStudioModels } from "../../api/providers/fetchers/lmstudio" const ALLOWED_VSCODE_SETTINGS = new Set(["terminal.integrated.inheritEnv"]) +// Serializes handling of "telemetrySetting" messages. Each invocation reads the previous +// setting, awaits a persistence write, then applies the new live telemetry state -- with no +// serialization, two rapid messages (e.g. a fast toggle) can interleave across those awaits: +// each invocation captures its own isOptedIn in a closure, so whichever invocation's tail end +// (TelemetryService.instance.updateTelemetryState) happens to resolve *last* wins, regardless +// of which message the user sent last. Chaining onto this promise ensures a given invocation's +// entire read-write-apply sequence completes before the next one starts. +let telemetrySettingQueue: Promise = Promise.resolve() + import { MarketplaceManager, MarketplaceItemType } from "../../services/marketplace" import { setPendingTodoList } from "../tools/UpdateTodoListTool" import { @@ -628,12 +638,27 @@ export const webviewMessageHandler = async ( ), ) - // Enable telemetry by default (when unset) or when explicitly enabled - provider.getStateToPostToWebview().then((state) => { - const { telemetrySetting } = state - const isOptedIn = telemetrySetting !== "disabled" - TelemetryService.instance.updateTelemetryState(isOptedIn) - }) + // 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. + // vscode.env.isTelemetryEnabled is ANDed in (matching extension.ts's + // onDidChangeTelemetryEnabled listener) so a webview reload can't re-enable + // telemetry while VS Code's global toggle is off. + // + // Read the setting synchronously via getGlobalState (same as the "telemetrySetting" + // handler below) rather than awaiting provider.getStateToPostToWebview() -- that + // async gap let this continuation resolve after a concurrent "telemetrySetting" + // message's queued update and clobber it with a stale value, the same interleaving + // class of bug telemetrySettingQueue exists to prevent. Routing through the queue + // here too means webviewDidLaunch can't race a concurrent telemetrySetting message + // either. + telemetrySettingQueue = telemetrySettingQueue + .catch(() => undefined) + .then(async () => { + const telemetrySetting = getGlobalState("telemetrySetting") || "unset" + TelemetryService.instance.updateTelemetryState( + isTelemetryOptedIn(telemetrySetting) && vscode.env.isTelemetryEnabled, + ) + }) provider.isViewLaunched = true break @@ -2459,29 +2484,46 @@ export const webviewMessageHandler = async ( } break case "telemetrySetting": { - const telemetrySetting = message.text as TelemetrySetting - const previousSetting = getGlobalState("telemetrySetting") || "unset" - const isOptedIn = telemetrySetting !== "disabled" - const wasPreviouslyOptedIn = previousSetting !== "disabled" + // Chain onto the shared queue so a concurrent "telemetrySetting" message (e.g. a + // rapid toggle) can't interleave its read-write-apply sequence with this one -- see + // the telemetrySettingQueue comment above for why that matters. Swallow a prior + // link's rejection before chaining (rather than letting .then() propagate it) so one + // failed update can't permanently poison every subsequent telemetrySetting message + // for the rest of the session. + const thisUpdate = telemetrySettingQueue + .catch(() => undefined) + .then(async () => { + const telemetrySetting = message.text as TelemetrySetting + const previousSetting = getGlobalState("telemetrySetting") || "unset" + const isOptedIn = isTelemetryOptedIn(telemetrySetting) + const wasPreviouslyOptedIn = isTelemetryOptedIn(previousSetting) + + // If turning telemetry OFF, fire event BEFORE disabling + if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, telemetrySetting) + } - // If turning telemetry OFF, fire event BEFORE disabling - if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { - TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, telemetrySetting) - } + // Update the telemetry state. vscode.env.isTelemetryEnabled is ANDed in + // (matching extension.ts's onDidChangeTelemetryEnabled listener) so this can't + // re-enable telemetry while VS Code's global toggle is off -- the + // captureTelemetrySettingsChanged calls above/below still track the user's + // stored preference transition on its own, independent of that live toggle. + await updateGlobalState("telemetrySetting", telemetrySetting) - // Update the telemetry state - await updateGlobalState("telemetrySetting", telemetrySetting) + if (TelemetryService.hasInstance()) { + TelemetryService.instance.updateTelemetryState(isOptedIn && vscode.env.isTelemetryEnabled) + } - if (TelemetryService.hasInstance()) { - TelemetryService.instance.updateTelemetryState(isOptedIn) - } + // If turning telemetry ON, fire event AFTER enabling + if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, telemetrySetting) + } - // If turning telemetry ON, fire event AFTER enabling - if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) { - TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, telemetrySetting) - } + await provider.postStateToWebview() + }) + telemetrySettingQueue = thisUpdate - await provider.postStateToWebview() + await thisUpdate break } case "debugSetting": { diff --git a/src/extension.ts b/src/extension.ts index e326509c3d..b4e31f6b49 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,24 @@ 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). + // 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) && vscode.env.isTelemetryEnabled, + ) + + // Push the new vscode.env.isTelemetryEnabled value to the webview too, so its + // own PostHog client (gated separately in TelemetryClient.ts) can't keep + // sending events after the global toggle flips off mid-session. + void ClineProvider.getVisibleInstance()?.postStateToWebviewWithoutClineMessages() + }), + ) + // Initialize code index managers for all workspace folders. const codeIndexManagers: CodeIndexManager[] = [] @@ -387,7 +406,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/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. diff --git a/webview-ui/src/App.tsx b/webview-ui/src/App.tsx index 0521499dbb..9f1897e2af 100644 --- a/webview-ui/src/App.tsx +++ b/webview-ui/src/App.tsx @@ -59,6 +59,7 @@ const App = () => { telemetrySetting, telemetryKey, machineId, + vscodeTelemetryEnabled, renderContext, mdmCompliant, } = useExtensionState() @@ -186,9 +187,18 @@ const App = () => { useEffect(() => { if (didHydrateState) { - telemetryClient.updateTelemetryState(telemetrySetting, telemetryKey, machineId) + // vscodeTelemetryEnabled defaults to true when the extension hasn't sent a value + // yet (e.g. an old extension build during a mixed rollout), matching VS Code's own + // default -- it must never silently disable telemetry just because the field is + // momentarily undefined during hydration. + telemetryClient.updateTelemetryState( + telemetrySetting, + telemetryKey, + machineId, + vscodeTelemetryEnabled ?? true, + ) } - }, [telemetrySetting, telemetryKey, machineId, didHydrateState]) + }, [telemetrySetting, telemetryKey, machineId, vscodeTelemetryEnabled, didHydrateState]) // Tell the extension that we are ready to receive messages. useEffect(() => vscode.postMessage({ type: "webviewDidLaunch" }), []) diff --git a/webview-ui/src/__tests__/TelemetryClient.spec.ts b/webview-ui/src/__tests__/TelemetryClient.spec.ts index 968de4fdd6..0e1617a65e 100644 --- a/webview-ui/src/__tests__/TelemetryClient.spec.ts +++ b/webview-ui/src/__tests__/TelemetryClient.spec.ts @@ -77,13 +77,72 @@ 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") // Assert expect(posthog.init).not.toHaveBeenCalled() }) + + it("doesn't initialize PostHog when vscode.env.isTelemetryEnabled is false, even with an explicit Accept", () => { + // Regression test: the webview's own PostHog client must respect the live VS + // Code global telemetry toggle the same way the extension-side gate does -- + // otherwise a user's explicit Accept can still send events while VS Code's + // global telemetry is disabled. + const API_KEY = "test-api-key" + const DISTINCT_ID = "test-user-id" + + telemetryClient.updateTelemetryState("enabled", API_KEY, DISTINCT_ID, false) + + expect(posthog.init).not.toHaveBeenCalled() + }) + + it("initializes PostHog when vscode.env.isTelemetryEnabled is true and the user opted in", () => { + const API_KEY = "test-api-key" + const DISTINCT_ID = "test-user-id" + + telemetryClient.updateTelemetryState("enabled", API_KEY, DISTINCT_ID, true) + + expect(posthog.init).toHaveBeenCalledWith( + API_KEY, + expect.objectContaining({ + api_host: "https://us.i.posthog.com", + persistence: "localStorage", + loaded: expect.any(Function), + }), + ) + }) + + it("defaults vscodeTelemetryEnabled to true when the argument is omitted", () => { + // Callers on an old build (or during a state-hydration race) that don't yet + // pass this argument must not have telemetry silently disabled as a side effect. + const API_KEY = "test-api-key" + const DISTINCT_ID = "test-user-id" + + telemetryClient.updateTelemetryState("enabled", API_KEY, DISTINCT_ID) + + expect(posthog.init).toHaveBeenCalled() + }) }) /** 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..99d7ba329f 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,24 @@ const TelemetryBanner = () => { const { t } = useAppTranslation() const [isDismissed, setIsDismissed] = useState(false) + // 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) + } + + 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 +43,7 @@ const TelemetryBanner = () => { return (
- {/* Close button (X) */} + {/* Close button (X) - neutral dismiss, does not record a choice */}
{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..e9ca86be37 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("shows the telemetry checkbox as checked when the setting is unset (disclosed opt-out default)", () => { + render( + + + , + ) + + expect(screen.getByRole("checkbox", { name: /telemetry/i })).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/settings.json b/webview-ui/src/i18n/locales/ca/settings.json index 2f390b3213..7ed12658b9 100644 --- a/webview-ui/src/i18n/locales/ca/settings.json +++ b/webview-ui/src/i18n/locales/ca/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Permetre informes anònims d'errors i ús", - "description": "Ajuda a millorar Zoo Code enviant dades d'ús anònimes i informes d'error. Aquesta telemetria no recull codi, prompts o informació personal. Consulta la nostra política de privacitat per a més detalls. Pots desactivar-ho en qualsevol moment." + "label": "Permetre informes d'errors i ús", + "description": "Ajuda a millorar Zoo Code enviant dades d'ús i informes d'error, vinculades a un identificador d'instal·lació en lloc del teu nom o compte. Aquesta telemetria no recull el teu codi ni les teves indicacions. Consulta la nostra política de privacitat per a més detalls. Pots desactivar-ho en qualsevol moment." }, "settings": { "import": "Importar", diff --git a/webview-ui/src/i18n/locales/ca/welcome.json b/webview-ui/src/i18n/locales/ca/welcome.json index 06b3fca6ba..6e631fd1fb 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, vinculades a un identificador d'instal·lació, per ajudar-nos a corregir errors i millorar l'extensió. Aquesta telemetria no recopila el teu codi ni les teves indicacions. Pots desactivar-la a configuració.", + "accept": "Accepta", + "decline": "Rebutja" }, "importSettings": "Importa la configuració" } diff --git a/webview-ui/src/i18n/locales/de/settings.json b/webview-ui/src/i18n/locales/de/settings.json index 76a492a067..bf4f3e2102 100644 --- a/webview-ui/src/i18n/locales/de/settings.json +++ b/webview-ui/src/i18n/locales/de/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Anonyme Fehler- und Nutzungsberichte zulassen", - "description": "Hilf mit, Zoo Code zu verbessern, indem du anonyme Nutzungsdaten und Fehlerberichte sendest. Diese Telemetrie sammelt keine Code-, Prompt- oder persönliche Informationen. Weitere Einzelheiten findest du in unserer Datenschutzrichtlinie." + "label": "Fehler- und Nutzungsberichte zulassen", + "description": "Hilf mit, Zoo Code zu verbessern, indem du Nutzungsdaten und Fehlerberichte sendest, die mit einer installationsbezogenen Kennung statt mit deinem Namen oder Konto verknüpft sind. Diese Telemetrie sammelt nicht deinen Code oder deine Prompts. Weitere Einzelheiten findest du in unserer Datenschutzrichtlinie." }, "settings": { "import": "Importieren", diff --git a/webview-ui/src/i18n/locales/de/welcome.json b/webview-ui/src/i18n/locales/de/welcome.json index 99eef8b9e7..5e1107bb05 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, verknüpft mit einer installationsbezogenen Kennung, um uns dabei zu helfen, Fehler zu beheben und die Erweiterung zu verbessern. Diese Telemetrie sammelt nicht deinen Code oder deine Prompts. Du kannst dies in den Einstellungen deaktivieren.", + "accept": "Akzeptieren", + "decline": "Ablehnen" }, "importSettings": "Einstellungen importieren" } diff --git a/webview-ui/src/i18n/locales/en/settings.json b/webview-ui/src/i18n/locales/en/settings.json index 72c78bf065..cc0812aab3 100644 --- a/webview-ui/src/i18n/locales/en/settings.json +++ b/webview-ui/src/i18n/locales/en/settings.json @@ -1043,8 +1043,8 @@ }, "footer": { "telemetry": { - "label": "Allow anonymous error and usage reporting", - "description": "Help improve Zoo Code by sending anonymous usage data and error reports. This telemetry does not collect code, prompts or personal information. See our privacy policy for more details." + "label": "Allow error and usage reporting", + "description": "Help improve Zoo Code by sending usage data and error reports, linked to a per-install identifier rather than your name or account. This telemetry does not collect your code or prompts. See our privacy policy for more details." }, "settings": { "import": "Import", diff --git a/webview-ui/src/i18n/locales/en/welcome.json b/webview-ui/src/i18n/locales/en/welcome.json index f6bd95ee1c..6a6e977ab7 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, linked to a per-install identifier, to help us fix bugs and improve the extension. This telemetry does not collect your code or prompts. You can turn this off in settings.", + "accept": "Accept", + "decline": "Decline" }, "importSettings": "Import Settings" } diff --git a/webview-ui/src/i18n/locales/es/settings.json b/webview-ui/src/i18n/locales/es/settings.json index 7de02e6cf6..d2608abfec 100644 --- a/webview-ui/src/i18n/locales/es/settings.json +++ b/webview-ui/src/i18n/locales/es/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Permitir informes anónimos de errores y uso", - "description": "Ayuda a mejorar Zoo Code enviando datos de uso anónimos e informes de errores. Esta telemetría no recopila código, prompts o información personal. Consulta nuestra política de privacidad para más detalles." + "label": "Permitir informes de errores y uso", + "description": "Ayuda a mejorar Zoo Code enviando datos de uso e informes de errores, vinculados a un identificador de instalación en lugar de tu nombre o cuenta. Esta telemetría no recopila tu código ni tus prompts. Consulta nuestra política de privacidad para más detalles." }, "settings": { "import": "Importar", diff --git a/webview-ui/src/i18n/locales/es/welcome.json b/webview-ui/src/i18n/locales/es/welcome.json index ddc8b358fe..40820ca451 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, vinculados a un identificador de instalación, para ayudarnos a corregir errores y mejorar la extensión. Esta telemetría no recopila tu código ni tus prompts. Puedes desactivar esto en la configuración.", + "accept": "Aceptar", + "decline": "Rechazar" }, "importSettings": "Importar configuración" } diff --git a/webview-ui/src/i18n/locales/fr/settings.json b/webview-ui/src/i18n/locales/fr/settings.json index 13dbb025ce..c56d42b99a 100644 --- a/webview-ui/src/i18n/locales/fr/settings.json +++ b/webview-ui/src/i18n/locales/fr/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Autoriser les rapports anonymes d'erreurs et d'utilisation", - "description": "Aidez à améliorer Zoo Code en envoyant des données d'utilisation anonymes et des rapports d'erreurs. Cette télémétrie ne collecte pas de code, de prompts ou d'informations personnelles. Consultez notre politique de confidentialité pour plus de détails." + "label": "Autoriser les rapports d'erreurs et d'utilisation", + "description": "Aidez à améliorer Zoo Code en envoyant des données d'utilisation et des rapports d'erreurs, associés à un identifiant lié à l'installation plutôt qu'à votre nom ou votre compte. Cette télémétrie ne collecte pas votre code ni vos prompts. Consultez notre politique de confidentialité pour plus de détails." }, "settings": { "import": "Importer", diff --git a/webview-ui/src/i18n/locales/fr/welcome.json b/webview-ui/src/i18n/locales/fr/welcome.json index b9af536a6b..7a0f14b7df 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, associées à un identifiant lié à l'installation, pour nous aider à corriger les bugs et améliorer l'extension. Cette télémétrie ne collecte pas ton code ni tes prompts. 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/settings.json b/webview-ui/src/i18n/locales/hi/settings.json index 1f155c9740..17963fd948 100644 --- a/webview-ui/src/i18n/locales/hi/settings.json +++ b/webview-ui/src/i18n/locales/hi/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "गुमनाम त्रुटि और उपयोग रिपोर्टिंग की अनुमति दें", - "description": "गुमनाम उपयोग डेटा और त्रुटि रिपोर्ट भेजकर Zoo Code को बेहतर बनाने में मदद करें। यह टेलीमेट्री कोड, प्रॉम्प्ट या व्यक्तिगत जानकारी एकत्र नहीं करती। अधिक विवरण के लिए हमारी गोपनीयता नीति देखें। आप इसे कभी भी बंद कर सकते हैं।" + "label": "त्रुटि और उपयोग रिपोर्टिंग की अनुमति दें", + "description": "आपके नाम या खाते के बजाय एक इंस्टॉल-लिंक्ड पहचानकर्ता से जुड़े उपयोग डेटा और त्रुटि रिपोर्ट भेजकर Zoo Code को बेहतर बनाने में मदद करें। यह टेलीमेट्री आपका कोड या प्रॉम्प्ट एकत्र नहीं करती। अधिक विवरण के लिए हमारी गोपनीयता नीति देखें। आप इसे कभी भी बंद कर सकते हैं।" }, "settings": { "import": "इम्पोर्ट", diff --git a/webview-ui/src/i18n/locales/hi/welcome.json b/webview-ui/src/i18n/locales/hi/welcome.json index 3c3028fc0c..c683ca9818 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/settings.json b/webview-ui/src/i18n/locales/id/settings.json index 5c90f85103..12d91cf82e 100644 --- a/webview-ui/src/i18n/locales/id/settings.json +++ b/webview-ui/src/i18n/locales/id/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Izinkan pelaporan error dan penggunaan anonim", - "description": "Bantu tingkatkan Zoo Code dengan mengirimkan data penggunaan anonim dan laporan error. Telemetri ini tidak mengumpulkan kode, prompt, atau informasi pribadi. Lihat kebijakan privasi kami untuk detail lebih lanjut. Anda dapat menonaktifkannya kapan saja." + "label": "Izinkan pelaporan error dan penggunaan", + "description": "Bantu tingkatkan Zoo Code dengan mengirimkan data penggunaan dan laporan error, yang terkait dengan pengenal instalasi, bukan nama atau akun Anda. Telemetri ini tidak mengumpulkan kode atau prompt Anda. Lihat kebijakan privasi kami untuk detail lebih lanjut. Anda dapat menonaktifkannya kapan saja." }, "settings": { "import": "Impor", diff --git a/webview-ui/src/i18n/locales/id/welcome.json b/webview-ui/src/i18n/locales/id/welcome.json index 9b7acd777f..430c1ae89f 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, yang terkait dengan pengenal instalasi, untuk membantu kami memperbaiki bug dan meningkatkan ekstensi. Telemetri ini tidak mengumpulkan kode atau prompt Anda. Anda dapat menonaktifkannya di pengaturan.", + "accept": "Terima", + "decline": "Tolak" }, "importSettings": "Impor Pengaturan" } diff --git a/webview-ui/src/i18n/locales/it/settings.json b/webview-ui/src/i18n/locales/it/settings.json index 50bb0e20d6..2b12f301a0 100644 --- a/webview-ui/src/i18n/locales/it/settings.json +++ b/webview-ui/src/i18n/locales/it/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Consenti segnalazioni anonime di errori e utilizzo", - "description": "Aiuta a migliorare Zoo Code inviando dati di utilizzo anonimi e segnalazioni di errori. Questa telemetria non raccoglie codice, prompt o informazioni personali. Consulta la nostra informativa sulla privacy per maggiori dettagli." + "label": "Consenti segnalazioni di errori e utilizzo", + "description": "Aiuta a migliorare Zoo Code inviando dati di utilizzo e segnalazioni di errori, collegati a un identificatore di installazione anziché al tuo nome o account. Questa telemetria non raccoglie il tuo codice o i tuoi prompt. Consulta la nostra informativa sulla privacy per maggiori dettagli. Puoi disattivarla in qualsiasi momento." }, "settings": { "import": "Importa", diff --git a/webview-ui/src/i18n/locales/it/welcome.json b/webview-ui/src/i18n/locales/it/welcome.json index c5b4066964..cd7d418f76 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, collegati a un identificatore di installazione, per aiutarci a correggere i bug e migliorare l'estensione. Questa telemetria non raccoglie il tuo codice o i tuoi prompt. Puoi disabilitare questa funzione in impostazioni.", + "accept": "Accetta", + "decline": "Rifiuta" }, "importSettings": "Importa impostazioni" } diff --git a/webview-ui/src/i18n/locales/ja/settings.json b/webview-ui/src/i18n/locales/ja/settings.json index 12455c8b9d..93b341ff05 100644 --- a/webview-ui/src/i18n/locales/ja/settings.json +++ b/webview-ui/src/i18n/locales/ja/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "匿名のエラーと使用状況レポートを許可", - "description": "匿名の使用データとエラーレポートを送信してZoo Codeの改善にご協力ください。このテレメトリはコード、プロンプト、個人情報を収集しません。詳細についてはプライバシーポリシーをご覧ください。" + "label": "エラーと使用状況レポートを許可", + "description": "お名前やアカウントではなく、インストールに紐づく識別子と関連付けた使用データとエラーレポートを送信してZoo Codeの改善にご協力ください。このテレメトリはあなたのコードやプロンプトを収集しません。詳細についてはプライバシーポリシーをご覧ください。これはいつでも無効にできます。" }, "settings": { "import": "インポート", diff --git a/webview-ui/src/i18n/locales/ja/welcome.json b/webview-ui/src/i18n/locales/ja/welcome.json index 55e8b4e3dd..675a73a2eb 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/settings.json b/webview-ui/src/i18n/locales/ko/settings.json index 91b84dd41b..8a37afca44 100644 --- a/webview-ui/src/i18n/locales/ko/settings.json +++ b/webview-ui/src/i18n/locales/ko/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "익명 오류 및 사용 보고 허용", - "description": "익명 사용 데이터 및 오류 보고서를 전송하여 Zoo Code 개선에 도움을 주세요. 이 텔레메트리는 코드, 프롬프트 또는 개인 정보를 수집하지 않습니다. 자세한 내용은 개인정보 보호정책을 참조하세요." + "label": "오류 및 사용 보고 허용", + "description": "사용자의 이름이나 계정이 아닌 설치 식별자와 연결된 사용 데이터 및 오류 보고서를 전송하여 Zoo Code 개선에 도움을 주세요. 이 텔레메트리는 사용자의 코드나 프롬프트를 수집하지 않습니다. 자세한 내용은 개인정보 보호정책을 참조하세요. 언제든지 비활성화할 수 있습니다." }, "settings": { "import": "가져오기", diff --git a/webview-ui/src/i18n/locales/ko/welcome.json b/webview-ui/src/i18n/locales/ko/welcome.json index db9407a20a..9c7fe439dd 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/settings.json b/webview-ui/src/i18n/locales/nl/settings.json index add5237373..aa9ec46bcf 100644 --- a/webview-ui/src/i18n/locales/nl/settings.json +++ b/webview-ui/src/i18n/locales/nl/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Anonieme fout- en gebruiksrapportage toestaan", - "description": "Help Zoo Code te verbeteren door anonieme gebruiksgegevens en foutmeldingen te versturen. Deze telemetrie verzamelt geen code, prompts of persoonlijke informatie. Zie ons privacybeleid voor meer details." + "label": "Fout- en gebruiksrapportage toestaan", + "description": "Help Zoo Code te verbeteren door gebruiksgegevens en foutmeldingen te versturen, gekoppeld aan een installatie-identifier in plaats van je naam of account. Deze telemetrie verzamelt niet je code of prompts. Zie ons privacybeleid voor meer details." }, "settings": { "import": "Importeren", diff --git a/webview-ui/src/i18n/locales/nl/welcome.json b/webview-ui/src/i18n/locales/nl/welcome.json index d62f761496..6662db31c3 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, gekoppeld aan een installatie-identifier, om ons te helpen bugs op te lossen en de extensie te verbeteren. Deze telemetrie verzamelt niet je code of prompts. Je kunt dit in instellingen uitschakelen.", + "accept": "Accepteren", + "decline": "Weigeren" }, "importSettings": "Instellingen importeren" } diff --git a/webview-ui/src/i18n/locales/pl/settings.json b/webview-ui/src/i18n/locales/pl/settings.json index 021b8a6075..de32da3a1b 100644 --- a/webview-ui/src/i18n/locales/pl/settings.json +++ b/webview-ui/src/i18n/locales/pl/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Zezwól na anonimowe raportowanie błędów i użycia", - "description": "Pomóż ulepszyć Zoo Code, wysyłając anonimowe dane użytkowania i raporty błędów. Ta telemetria nie zbiera kodu, promptów ani danych osobowych. Zobacz naszą politykę prywatności, aby uzyskać więcej szczegółów. Możesz to wyłączyć w dowolnym momencie." + "label": "Zezwól na raportowanie błędów i użycia", + "description": "Pomóż ulepszyć Zoo Code, wysyłając dane użytkowania i raporty błędów powiązane z identyfikatorem instalacji, a nie z Twoim imieniem czy kontem. Ta telemetria nie zbiera Twojego kodu ani podpowiedzi. Zobacz naszą politykę prywatności, aby uzyskać więcej szczegółów. Możesz to wyłączyć w dowolnym momencie." }, "settings": { "import": "Importuj", diff --git a/webview-ui/src/i18n/locales/pl/welcome.json b/webview-ui/src/i18n/locales/pl/welcome.json index 32037e759e..5f342c5860 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, powiązane z identyfikatorem instalacji, aby pomóc nam naprawiać błędy i ulepszać rozszerzenie. Ta telemetria nie zbiera Twojego kodu ani podpowiedzi. Możesz ją wyłączyć w ustawieniach.", + "accept": "Akceptuj", + "decline": "Odrzuć" }, "importSettings": "Importuj ustawienia" } diff --git a/webview-ui/src/i18n/locales/pt-BR/settings.json b/webview-ui/src/i18n/locales/pt-BR/settings.json index 15203c1d31..97fd1c497f 100644 --- a/webview-ui/src/i18n/locales/pt-BR/settings.json +++ b/webview-ui/src/i18n/locales/pt-BR/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Permitir relatórios anônimos de erros e uso", - "description": "Ajude a melhorar o Zoo Code enviando dados de uso anônimos e relatórios de erros. Esta telemetria não coleta código, prompts ou informações pessoais. Consulte nossa política de privacidade para mais detalhes." + "label": "Permitir relatórios de erros e uso", + "description": "Ajude a melhorar o Zoo Code enviando dados de uso e relatórios de erros, vinculados a um identificador de instalação em vez do seu nome ou conta. Esta telemetria não coleta seu código ou seus prompts. Consulte nossa política de privacidade para mais detalhes." }, "settings": { "import": "Importar", diff --git a/webview-ui/src/i18n/locales/pt-BR/welcome.json b/webview-ui/src/i18n/locales/pt-BR/welcome.json index c444ff4374..29ecf56e7e 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, vinculados a um identificador de instalação, para nos ajudar a corrigir bugs e melhorar a extensão. Esta telemetria não coleta seu código ou seus prompts. Você pode desabilitar isso nas configurações.", + "accept": "Aceitar", + "decline": "Recusar" }, "importSettings": "Importar configurações" } diff --git a/webview-ui/src/i18n/locales/ru/settings.json b/webview-ui/src/i18n/locales/ru/settings.json index bc2b7b4398..1611efafd5 100644 --- a/webview-ui/src/i18n/locales/ru/settings.json +++ b/webview-ui/src/i18n/locales/ru/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Разрешить анонимную отправку ошибок и статистики использования", - "description": "Помогите улучшить Zoo Code, отправляя анонимные данные об использовании и отчеты об ошибках. Эта телеметрия не собирает код, промпты или личную информацию. Смотрите нашу политику конфиденциальности для получения подробной информации." + "label": "Разрешить отправку ошибок и статистики использования", + "description": "Помогите улучшить Zoo Code, отправляя данные об использовании и отчеты об ошибках, связанные с идентификатором установки, а не с вашим именем или учетной записью. Эта телеметрия не собирает ваш код или запросы. Смотрите нашу политику конфиденциальности для получения подробной информации." }, "settings": { "import": "Импорт", diff --git a/webview-ui/src/i18n/locales/ru/welcome.json b/webview-ui/src/i18n/locales/ru/welcome.json index dade1e8ece..62a40077d8 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/settings.json b/webview-ui/src/i18n/locales/tr/settings.json index 0f831af080..d3cc7f0d19 100644 --- a/webview-ui/src/i18n/locales/tr/settings.json +++ b/webview-ui/src/i18n/locales/tr/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Anonim hata ve kullanım raporlamaya izin ver", - "description": "Anonim kullanım verileri ve hata raporları göndererek Zoo Code'u geliştirmeye yardım edin. Bu telemetri kod, prompt veya kişisel bilgi toplamaz. Daha fazla ayrıntı için gizlilik politikamıza bakın. Bunu istediğiniz zaman kapatabilirsiniz." + "label": "Hata ve kullanım raporlamasına izin ver", + "description": "Adınız veya hesabınız yerine kuruluma bağlı bir tanımlayıcıyla ilişkilendirilmiş kullanım verileri ve hata raporları göndererek Zoo Code'u geliştirmeye yardım edin. Bu telemetri kodunuzu veya istemlerinizi toplamaz. Daha fazla ayrıntı için gizlilik politikamıza bakın. Bunu istediğiniz zaman kapatabilirsiniz." }, "settings": { "import": "İçe Aktar", diff --git a/webview-ui/src/i18n/locales/tr/welcome.json b/webview-ui/src/i18n/locales/tr/welcome.json index 0216342ed9..0f83de0f30 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, kuruluma bağlı bir tanımlayıcıyla ilişkilendirilmiş hata ve kullanım verilerini toplar. Bu telemetri kodunu veya istemlerini 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/settings.json b/webview-ui/src/i18n/locales/vi/settings.json index 424f17734a..4c96eb509a 100644 --- a/webview-ui/src/i18n/locales/vi/settings.json +++ b/webview-ui/src/i18n/locales/vi/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "Cho phép báo cáo lỗi và sử dụng ẩn danh", - "description": "Giúp cải thiện Zoo Code bằng cách gửi dữ liệu sử dụng ẩn danh và báo cáo lỗi. Telemetry này không thu thập mã, prompt hoặc thông tin cá nhân. Xem chính sách bảo mật của chúng tôi để biết thêm chi tiết. Bạn có thể tắt tính năng này bất cứ lúc nào." + "label": "Cho phép báo cáo lỗi và dữ liệu sử dụng", + "description": "Giúp cải thiện Zoo Code bằng cách gửi dữ liệu sử dụng và báo cáo lỗi, được liên kết với mã định danh cài đặt thay vì tên hoặc tài khoản của bạn. Telemetry này không thu thập mã nguồn hoặc lời nhắc của bạn. Xem chính sách bảo mật của chúng tôi để biết thêm chi tiết. Bạn có thể tắt tính năng này bất cứ lúc nào." }, "settings": { "import": "Nhập", diff --git a/webview-ui/src/i18n/locales/vi/welcome.json b/webview-ui/src/i18n/locales/vi/welcome.json index 93756a444e..76be9d097c 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, được liên kết với một mã định danh cài đặt, để 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ã nguồn hoặc lời nhắc của bạ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/settings.json b/webview-ui/src/i18n/locales/zh-CN/settings.json index 4e677eb329..00a5d71da0 100644 --- a/webview-ui/src/i18n/locales/zh-CN/settings.json +++ b/webview-ui/src/i18n/locales/zh-CN/settings.json @@ -963,8 +963,8 @@ }, "footer": { "telemetry": { - "label": "允许匿名数据收集", - "description": "通过发送匿名使用数据和错误报告来帮助改进 Zoo Code。此遥测不会收集代码、提示 或个人信息。详细信息请参阅我们的隐私政策。您可以随时关闭此功能。" + "label": "允许发送错误和使用情况报告", + "description": "通过发送与安装标识符（而非您的姓名或帐户）关联的使用数据和错误报告来帮助改进 Zoo Code。此遥测不会收集您的代码或提示词。详细信息请参阅我们的隐私政策。您可以随时关闭此功能。" }, "settings": { "import": "导入", diff --git a/webview-ui/src/i18n/locales/zh-CN/welcome.json b/webview-ui/src/i18n/locales/zh-CN/welcome.json index 1506c0c959..17740e8302 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/settings.json b/webview-ui/src/i18n/locales/zh-TW/settings.json index 34e7001b86..df4f0fd629 100644 --- a/webview-ui/src/i18n/locales/zh-TW/settings.json +++ b/webview-ui/src/i18n/locales/zh-TW/settings.json @@ -990,8 +990,8 @@ }, "footer": { "telemetry": { - "label": "允許匿名錯誤與使用情況回報", - "description": "透過發送匿名使用資料和錯誤回報來協助改善 Zoo Code。此遙測不會收集程式碼、提示詞或個人資訊。查看我們的 隱私權政策 以了解更多資訊。" + "label": "允許錯誤與使用情況回報", + "description": "透過發送與安裝識別碼（而非您的姓名或帳戶）相關聯的使用資料和錯誤回報來協助改善 Zoo Code。此遙測不會收集您的程式碼或提示詞。查看我們的 隱私權政策 以了解更多資訊。您可以隨時關閉此功能。" }, "settings": { "import": "匯入", diff --git a/webview-ui/src/i18n/locales/zh-TW/welcome.json b/webview-ui/src/i18n/locales/zh-TW/welcome.json index 4ba8548a62..eba96dc00a 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..f3a70da29f 100644 --- a/webview-ui/src/utils/TelemetryClient.ts +++ b/webview-ui/src/utils/TelemetryClient.ts @@ -1,15 +1,20 @@ 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 private static telemetryEnabled: boolean = false - public updateTelemetryState(telemetrySetting: TelemetrySetting, apiKey?: string, distinctId?: string) { + public updateTelemetryState( + telemetrySetting: TelemetrySetting, + apiKey?: string, + distinctId?: string, + vscodeTelemetryEnabled: boolean = true, + ) { posthog.reset() - if (telemetrySetting !== "disabled" && apiKey && distinctId) { + if (isTelemetryOptedIn(telemetrySetting) && apiKey && distinctId && vscodeTelemetryEnabled) { TelemetryClient.telemetryEnabled = true posthog.init(apiKey, {