Skip to content

Commit 838256a

Browse files
jrmrubens
jr
and
mrubens
authored
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]>
1 parent f3e124e commit 838256a

File tree

4 files changed

+205
-12
lines changed

4 files changed

+205
-12
lines changed

‎packages/cloud/src/CloudService.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export class CloudService {
4343
this.callbacks.stateChanged?.(),
4444
)
4545

46-
this.telemetryClient = new TelemetryClient(this.authService)
46+
this.telemetryClient = new TelemetryClient(this.authService, this.settingsService)
4747

4848
try {
4949
TelemetryService.instance.register(this.telemetryClient)

‎packages/cloud/src/TelemetryClient.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ import { BaseTelemetryClient } from "@roo-code/telemetry"
33

44
import { getRooCodeApiUrl } from "./Config"
55
import { AuthService } from "./AuthService"
6+
import { SettingsService } from "./SettingsService"
67

78
export class TelemetryClient extends BaseTelemetryClient {
89
constructor(
910
private authService: AuthService,
11+
private settingsService: SettingsService,
1012
debug = false,
1113
) {
1214
super(
@@ -83,5 +85,20 @@ export class TelemetryClient extends BaseTelemetryClient {
8385
return true
8486
}
8587

88+
protected override isEventCapturable(eventName: TelemetryEventName): boolean {
89+
// Ensure that this event type is supported by the telemetry client
90+
if (!super.isEventCapturable(eventName)) {
91+
return false
92+
}
93+
94+
// Only record message telemetry if a cloud account is present and explicitly configured to record messages
95+
if (eventName === TelemetryEventName.TASK_MESSAGE) {
96+
return this.settingsService.getSettings()?.cloudSettings?.recordTaskMessages || false
97+
}
98+
99+
// Other telemetry types are capturable at this point
100+
return true
101+
}
102+
86103
public override async shutdown() {}
87104
}

‎packages/cloud/src/__tests__/TelemetryClient.test.ts‎

Lines changed: 182 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ describe("TelemetryClient", () => {
1717
}
1818

1919
let mockAuthService: any
20+
let mockSettingsService: any
2021

2122
beforeEach(() => {
2223
vi.clearAllMocks()
@@ -29,6 +30,15 @@ describe("TelemetryClient", () => {
2930
hasActiveSession: vi.fn().mockReturnValue(true),
3031
}
3132

33+
// Create a mock SettingsService
34+
mockSettingsService = {
35+
getSettings: vi.fn().mockReturnValue({
36+
cloudSettings: {
37+
recordTaskMessages: true,
38+
},
39+
}),
40+
}
41+
3242
mockFetch.mockResolvedValue({
3343
ok: true,
3444
json: vi.fn().mockResolvedValue({}),
@@ -44,7 +54,7 @@ describe("TelemetryClient", () => {
4454

4555
describe("isEventCapturable", () => {
4656
it("should return true for events not in exclude list", () => {
47-
const client = new TelemetryClient(mockAuthService)
57+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
4858

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

6070
it("should return false for events in exclude list", () => {
61-
const client = new TelemetryClient(mockAuthService)
71+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
6272

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

6878
expect(isEventCapturable(TelemetryEventName.TASK_CONVERSATION_MESSAGE)).toBe(false)
6979
})
80+
81+
it("should return true for TASK_MESSAGE events when recordTaskMessages is true", () => {
82+
mockSettingsService.getSettings.mockReturnValue({
83+
cloudSettings: {
84+
recordTaskMessages: true,
85+
},
86+
})
87+
88+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
89+
90+
const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
91+
client,
92+
"isEventCapturable",
93+
).bind(client)
94+
95+
expect(isEventCapturable(TelemetryEventName.TASK_MESSAGE)).toBe(true)
96+
})
97+
98+
it("should return false for TASK_MESSAGE events when recordTaskMessages is false", () => {
99+
mockSettingsService.getSettings.mockReturnValue({
100+
cloudSettings: {
101+
recordTaskMessages: false,
102+
},
103+
})
104+
105+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
106+
107+
const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
108+
client,
109+
"isEventCapturable",
110+
).bind(client)
111+
112+
expect(isEventCapturable(TelemetryEventName.TASK_MESSAGE)).toBe(false)
113+
})
114+
115+
it("should return false for TASK_MESSAGE events when recordTaskMessages is undefined", () => {
116+
mockSettingsService.getSettings.mockReturnValue({
117+
cloudSettings: {},
118+
})
119+
120+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
121+
122+
const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
123+
client,
124+
"isEventCapturable",
125+
).bind(client)
126+
127+
expect(isEventCapturable(TelemetryEventName.TASK_MESSAGE)).toBe(false)
128+
})
129+
130+
it("should return false for TASK_MESSAGE events when cloudSettings is undefined", () => {
131+
mockSettingsService.getSettings.mockReturnValue({})
132+
133+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
134+
135+
const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
136+
client,
137+
"isEventCapturable",
138+
).bind(client)
139+
140+
expect(isEventCapturable(TelemetryEventName.TASK_MESSAGE)).toBe(false)
141+
})
142+
143+
it("should return false for TASK_MESSAGE events when getSettings returns undefined", () => {
144+
mockSettingsService.getSettings.mockReturnValue(undefined)
145+
146+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
147+
148+
const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>(
149+
client,
150+
"isEventCapturable",
151+
).bind(client)
152+
153+
expect(isEventCapturable(TelemetryEventName.TASK_MESSAGE)).toBe(false)
154+
})
70155
})
71156

72157
describe("getEventProperties", () => {
73158
it("should merge provider properties with event properties", async () => {
74-
const client = new TelemetryClient(mockAuthService)
159+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
75160

76161
const mockProvider: TelemetryPropertiesProvider = {
77162
getTelemetryProperties: vi.fn().mockResolvedValue({
@@ -112,7 +197,7 @@ describe("TelemetryClient", () => {
112197
})
113198

114199
it("should handle errors from provider gracefully", async () => {
115-
const client = new TelemetryClient(mockAuthService)
200+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
116201

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

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

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

156241
describe("capture", () => {
157242
it("should not capture events that are not capturable", async () => {
158-
const client = new TelemetryClient(mockAuthService)
243+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
159244

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

253+
it("should not capture TASK_MESSAGE events when recordTaskMessages is false", async () => {
254+
mockSettingsService.getSettings.mockReturnValue({
255+
cloudSettings: {
256+
recordTaskMessages: false,
257+
},
258+
})
259+
260+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
261+
262+
await client.capture({
263+
event: TelemetryEventName.TASK_MESSAGE,
264+
properties: {
265+
taskId: "test-task-id",
266+
message: {
267+
ts: 1,
268+
type: "say",
269+
say: "text",
270+
text: "test message",
271+
},
272+
},
273+
})
274+
275+
expect(mockFetch).not.toHaveBeenCalled()
276+
})
277+
278+
it("should not capture TASK_MESSAGE events when recordTaskMessages is undefined", async () => {
279+
mockSettingsService.getSettings.mockReturnValue({
280+
cloudSettings: {},
281+
})
282+
283+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
284+
285+
await client.capture({
286+
event: TelemetryEventName.TASK_MESSAGE,
287+
properties: {
288+
taskId: "test-task-id",
289+
message: {
290+
ts: 1,
291+
type: "say",
292+
say: "text",
293+
text: "test message",
294+
},
295+
},
296+
})
297+
298+
expect(mockFetch).not.toHaveBeenCalled()
299+
})
300+
168301
it("should not send request when schema validation fails", async () => {
169-
const client = new TelemetryClient(mockAuthService)
302+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
170303

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

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

183316
const providerProperties = {
184317
appName: "roo-code",
@@ -222,8 +355,46 @@ describe("TelemetryClient", () => {
222355
)
223356
})
224357

358+
it("should attempt to capture TASK_MESSAGE events when recordTaskMessages is true", async () => {
359+
mockSettingsService.getSettings.mockReturnValue({
360+
cloudSettings: {
361+
recordTaskMessages: true,
362+
},
363+
})
364+
365+
const eventProperties = {
366+
taskId: "test-task-id",
367+
message: {
368+
ts: 1,
369+
type: "say",
370+
say: "text",
371+
text: "test message",
372+
},
373+
}
374+
375+
const mockValidatedData = {
376+
type: TelemetryEventName.TASK_MESSAGE,
377+
properties: eventProperties,
378+
}
379+
380+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
381+
382+
await client.capture({
383+
event: TelemetryEventName.TASK_MESSAGE,
384+
properties: eventProperties,
385+
})
386+
387+
expect(mockFetch).toHaveBeenCalledWith(
388+
"https://app.roocode.com/api/events",
389+
expect.objectContaining({
390+
method: "POST",
391+
body: JSON.stringify(mockValidatedData),
392+
}),
393+
)
394+
})
395+
225396
it("should handle fetch errors gracefully", async () => {
226-
const client = new TelemetryClient(mockAuthService)
397+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
227398

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

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

239410
describe("telemetry state methods", () => {
240411
it("should always return true for isTelemetryEnabled", () => {
241-
const client = new TelemetryClient(mockAuthService)
412+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
242413
expect(client.isTelemetryEnabled()).toBe(true)
243414
})
244415

245416
it("should have empty implementations for updateTelemetryState and shutdown", async () => {
246-
const client = new TelemetryClient(mockAuthService)
417+
const client = new TelemetryClient(mockAuthService, mockSettingsService)
247418
client.updateTelemetryState(true)
248419
await client.shutdown()
249420
})

‎packages/types/src/cloud.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ export const organizationSettingsSchema = z.object({
4343
fuzzyMatchThreshold: z.number().optional(),
4444
})
4545
.optional(),
46+
cloudSettings: z
47+
.object({
48+
recordTaskMessages: z.boolean().optional(),
49+
})
50+
.optional(),
4651
allowList: organizationAllowListSchema,
4752
})
4853

0 commit comments

Comments
 (0)