Browse Source

Add roo cloud settings (#4117)

* Add roo cloud settings

First setting controls logging task messages.

* Update packages/cloud/src/TelemetryClient.ts

Co-authored-by: Matt Rubens <[email protected]>

* Fix typo

---------

Co-authored-by: Matt Rubens <[email protected]>
John Richmond 9 months ago
parent
commit
838256a7c1

+ 1 - 1
packages/cloud/src/CloudService.ts

@@ -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)

+ 17 - 0
packages/cloud/src/TelemetryClient.ts

@@ -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(
@@ -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() {}
 }

+ 182 - 11
packages/cloud/src/__tests__/TelemetryClient.test.ts

@@ -17,6 +17,7 @@ describe("TelemetryClient", () => {
 	}
 
 	let mockAuthService: any
+	let mockSettingsService: any
 
 	beforeEach(() => {
 		vi.clearAllMocks()
@@ -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({}),
@@ -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,
@@ -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,
@@ -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({
@@ -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")),
@@ -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>>
@@ -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.
@@ -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,
@@ -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",
@@ -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"))
 
@@ -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()
 		})

+ 5 - 0
packages/types/src/cloud.ts

@@ -43,6 +43,11 @@ export const organizationSettingsSchema = z.object({
 			fuzzyMatchThreshold: z.number().optional(),
 		})
 		.optional(),
+	cloudSettings: z
+		.object({
+			recordTaskMessages: z.boolean().optional(),
+		})
+		.optional(),
 	allowList: organizationAllowListSchema,
 })