Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/cloud/src/CloudService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class CloudService {
this.callbacks.stateChanged?.(),
)

this.telemetryClient = new TelemetryClient(this.authService)
this.telemetryClient = new TelemetryClient(this.authService, this.settingsService)

try {
TelemetryService.instance.register(this.telemetryClient)
Expand Down
17 changes: 17 additions & 0 deletions packages/cloud/src/TelemetryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ import { BaseTelemetryClient } from "@roo-code/telemetry"

import { getRooCodeApiUrl } from "./Config"
import { AuthService } from "./AuthService"
import { SettingsService } from "./SettingsService"

export class TelemetryClient extends BaseTelemetryClient {
constructor(
private authService: AuthService,
private settingsService: SettingsService,
debug = false,
) {
super(
Expand Down Expand Up @@ -83,5 +85,20 @@ export class TelemetryClient extends BaseTelemetryClient {
return true
}

protected override isEventCapturable(eventName: TelemetryEventName): boolean {
// Ensure that this event type is supported by the telemetry client
if (!super.isEventCapturable(eventName)) {
return false
}

// Only record message telemetry if a cloud account is present and explicitly configured to record messages
if (eventName === TelemetryEventName.TASK_MESSAGE) {
return this.settingsService.getSettings()?.cloudSettings?.recordTaskMessages || false
}

// Other telemetry types are capturable at this point
return true
}

public override async shutdown() {}
}
193 changes: 182 additions & 11 deletions packages/cloud/src/__tests__/TelemetryClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe("TelemetryClient", () => {
}

let mockAuthService: any
let mockSettingsService: any

beforeEach(() => {
vi.clearAllMocks()
Expand All @@ -29,6 +30,15 @@ describe("TelemetryClient", () => {
hasActiveSession: vi.fn().mockReturnValue(true),
}

// Create a mock SettingsService
mockSettingsService = {
getSettings: vi.fn().mockReturnValue({
cloudSettings: {
recordTaskMessages: true,
},
}),
}

mockFetch.mockResolvedValue({
ok: true,
json: vi.fn().mockResolvedValue({}),
Expand All @@ -44,7 +54,7 @@ describe("TelemetryClient", () => {

describe("isEventCapturable", () => {
it("should return true for events not in exclude list", () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)

const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
client,
Expand All @@ -58,7 +68,7 @@ describe("TelemetryClient", () => {
})

it("should return false for events in exclude list", () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)

const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
client,
Expand All @@ -67,11 +77,86 @@ describe("TelemetryClient", () => {

expect(isEventCapturable(TelemetryEventName.TASK_CONVERSATION_MESSAGE)).toBe(false)
})

it("should return true for TASK_MESSAGE events when recordTaskMessages is true", () => {
mockSettingsService.getSettings.mockReturnValue({
cloudSettings: {
recordTaskMessages: true,
},
})

const client = new TelemetryClient(mockAuthService, mockSettingsService)

const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
client,
"isEventCapturable",
).bind(client)

expect(isEventCapturable(TelemetryEventName.TASK_MESSAGE)).toBe(true)
})

it("should return false for TASK_MESSAGE events when recordTaskMessages is false", () => {
mockSettingsService.getSettings.mockReturnValue({
cloudSettings: {
recordTaskMessages: false,
},
})

const client = new TelemetryClient(mockAuthService, mockSettingsService)

const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
client,
"isEventCapturable",
).bind(client)

expect(isEventCapturable(TelemetryEventName.TASK_MESSAGE)).toBe(false)
})

it("should return false for TASK_MESSAGE events when recordTaskMessages is undefined", () => {
mockSettingsService.getSettings.mockReturnValue({
cloudSettings: {},
})

const client = new TelemetryClient(mockAuthService, mockSettingsService)

const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
client,
"isEventCapturable",
).bind(client)

expect(isEventCapturable(TelemetryEventName.TASK_MESSAGE)).toBe(false)
})

it("should return false for TASK_MESSAGE events when cloudSettings is undefined", () => {
mockSettingsService.getSettings.mockReturnValue({})

const client = new TelemetryClient(mockAuthService, mockSettingsService)

const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
client,
"isEventCapturable",
).bind(client)

expect(isEventCapturable(TelemetryEventName.TASK_MESSAGE)).toBe(false)
})

it("should return false for TASK_MESSAGE events when getSettings returns undefined", () => {
mockSettingsService.getSettings.mockReturnValue(undefined)

const client = new TelemetryClient(mockAuthService, mockSettingsService)

const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
client,
"isEventCapturable",
).bind(client)

expect(isEventCapturable(TelemetryEventName.TASK_MESSAGE)).toBe(false)
})
})

describe("getEventProperties", () => {
it("should merge provider properties with event properties", async () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)

const mockProvider: TelemetryPropertiesProvider = {
getTelemetryProperties: vi.fn().mockResolvedValue({
Expand Down Expand Up @@ -112,7 +197,7 @@ describe("TelemetryClient", () => {
})

it("should handle errors from provider gracefully", async () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)

const mockProvider: TelemetryPropertiesProvider = {
getTelemetryProperties: vi.fn().mockRejectedValue(new Error("Provider error")),
Expand All @@ -138,7 +223,7 @@ describe("TelemetryClient", () => {
})

it("should return event properties when no provider is set", async () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)

const getEventProperties = getPrivateProperty<
(event: { event: TelemetryEventName; properties?: Record<string, any> }) => Promise<Record<string, any>>
Expand All @@ -155,7 +240,7 @@ describe("TelemetryClient", () => {

describe("capture", () => {
it("should not capture events that are not capturable", async () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)

await client.capture({
event: TelemetryEventName.TASK_CONVERSATION_MESSAGE, // In exclude list.
Expand All @@ -165,8 +250,56 @@ describe("TelemetryClient", () => {
expect(mockFetch).not.toHaveBeenCalled()
})

it("should not capture TASK_MESSAGE events when recordTaskMessages is false", async () => {
mockSettingsService.getSettings.mockReturnValue({
cloudSettings: {
recordTaskMessages: false,
},
})

const client = new TelemetryClient(mockAuthService, mockSettingsService)

await client.capture({
event: TelemetryEventName.TASK_MESSAGE,
properties: {
taskId: "test-task-id",
message: {
ts: 1,
type: "say",
say: "text",
text: "test message",
},
},
})

expect(mockFetch).not.toHaveBeenCalled()
})

it("should not capture TASK_MESSAGE events when recordTaskMessages is undefined", async () => {
mockSettingsService.getSettings.mockReturnValue({
cloudSettings: {},
})

const client = new TelemetryClient(mockAuthService, mockSettingsService)

await client.capture({
event: TelemetryEventName.TASK_MESSAGE,
properties: {
taskId: "test-task-id",
message: {
ts: 1,
type: "say",
say: "text",
text: "test message",
},
},
})

expect(mockFetch).not.toHaveBeenCalled()
})

it("should not send request when schema validation fails", async () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)

await client.capture({
event: TelemetryEventName.TASK_CREATED,
Expand All @@ -178,7 +311,7 @@ describe("TelemetryClient", () => {
})

it("should send request when event is capturable and validation passes", async () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)

const providerProperties = {
appName: "roo-code",
Expand Down Expand Up @@ -222,8 +355,46 @@ describe("TelemetryClient", () => {
)
})

it("should attempt to capture TASK_MESSAGE events when recordTaskMessages is true", async () => {
mockSettingsService.getSettings.mockReturnValue({
cloudSettings: {
recordTaskMessages: true,
},
})

const eventProperties = {
taskId: "test-task-id",
message: {
ts: 1,
type: "say",
say: "text",
text: "test message",
},
}

const mockValidatedData = {
type: TelemetryEventName.TASK_MESSAGE,
properties: eventProperties,
}

const client = new TelemetryClient(mockAuthService, mockSettingsService)

await client.capture({
event: TelemetryEventName.TASK_MESSAGE,
properties: eventProperties,
})

expect(mockFetch).toHaveBeenCalledWith(
"https://app.roocode.com/api/events",
expect.objectContaining({
method: "POST",
body: JSON.stringify(mockValidatedData),
}),
)
})

it("should handle fetch errors gracefully", async () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)

mockFetch.mockRejectedValue(new Error("Network error"))

Expand All @@ -238,12 +409,12 @@ describe("TelemetryClient", () => {

describe("telemetry state methods", () => {
it("should always return true for isTelemetryEnabled", () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)
expect(client.isTelemetryEnabled()).toBe(true)
})

it("should have empty implementations for updateTelemetryState and shutdown", async () => {
const client = new TelemetryClient(mockAuthService)
const client = new TelemetryClient(mockAuthService, mockSettingsService)
client.updateTelemetryState(true)
await client.shutdown()
})
Expand Down
5 changes: 5 additions & 0 deletions packages/types/src/cloud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ export const organizationSettingsSchema = z.object({
fuzzyMatchThreshold: z.number().optional(),
})
.optional(),
cloudSettings: z
.object({
recordTaskMessages: z.boolean().optional(),
})
.optional(),
allowList: organizationAllowListSchema,
})

Expand Down
Loading