Merge 49ad8aee2c into 3e202ebf5b

packages/cloud/src/TelemetryClient.ts

@@ -8,6 +8,7 @@ import {
 	rooCodeTelemetryEventSchema,
 	TelemetryPropertiesProvider,
 	TelemetryEventSubscription,
+	type StaticAppProperties,
 } from "@roo-code/types"
 
 import { getRooCodeApiUrl } from "./config.js"
@@ -16,6 +17,12 @@ import type { RetryQueue } from "./retry-queue/index.js"
 abstract class BaseTelemetryClient implements TelemetryClient {
 	protected providerRef: WeakRef<TelemetryPropertiesProvider> | null = null
 	protected telemetryEnabled: boolean = false
+	/**
+	 * Cached static app properties captured when the provider is set.
+	 * These are used as fallback when the provider is no longer available
+	 * (e.g., after ClineProvider is disposed).
+	 */
+	protected cachedStaticProperties: StaticAppProperties | null = null
 
 	constructor(
 		public readonly subscription?: TelemetryEventSubscription,
@@ -54,6 +61,10 @@ abstract class BaseTelemetryClient implements TelemetryClient {
 					`Error getting telemetry properties: ${error instanceof Error ? error.message : String(error)}`,
 				)
 			}
+		} else if (this.cachedStaticProperties) {
+			// Provider is no longer available (e.g., ClineProvider was disposed).
+			// Use cached static properties to ensure required telemetry fields are present.
+			providerProperties = { ...this.cachedStaticProperties }
 		}
 
 		// Merge provider properties with event-specific properties.
@@ -73,6 +84,16 @@ abstract class BaseTelemetryClient implements TelemetryClient {
 
 	public setProvider(provider: TelemetryPropertiesProvider): void {
 		this.providerRef = new WeakRef(provider)
+
+		// Capture static app properties immediately so they remain available
+		// even after the provider is garbage collected.
+		// This is especially important for ClineProvider which may be disposed
+		// when the webview is closed, but telemetry events may still be in flight.
+		if ("appProperties" in provider) {
+			this.cachedStaticProperties = (
+				provider as TelemetryPropertiesProvider & { appProperties: StaticAppProperties }
+			).appProperties
+		}
 	}
 
 	public abstract updateTelemetryState(didUserOptIn: boolean): void

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

@@ -195,6 +195,77 @@ describe("TelemetryClient", () => {
 
 			expect(result).toEqual({ customProp: "value" })
 		})
+
+		it("should use cached static properties when provider is no longer available", async () => {
+			const client = new TelemetryClient(mockAuthService, mockSettingsService)
+
+			const staticProperties = {
+				appName: "roo-code",
+				appVersion: "1.0.0",
+				vscodeVersion: "1.60.0",
+				platform: "darwin",
+				editorName: "vscode",
+			}
+
+			// Create a mock provider with appProperties
+			const mockProvider: TelemetryPropertiesProvider & { appProperties: any } = {
+				appProperties: staticProperties,
+				getTelemetryProperties: vi.fn().mockResolvedValue({
+					...staticProperties,
+					language: "en",
+					mode: "code",
+				}),
+			}
+
+			// Set the provider (this should cache the static properties)
+			client.setProvider(mockProvider)
+
+			// Verify cached properties were set
+			const cachedProps = getPrivateProperty<any>(client, "cachedStaticProperties")
+			expect(cachedProps).toEqual(staticProperties)
+
+			// Now simulate the provider being garbage collected by setting providerRef to return undefined
+			// We do this by clearing the providerRef directly
+			;(client as any).providerRef = new WeakRef({} as any) // WeakRef to an empty object that will be different
+			// Force the WeakRef to return undefined by overriding deref
+			;(client as any).providerRef = { deref: () => undefined }
+
+			const getEventProperties = getPrivateProperty<
+				(event: { event: TelemetryEventName; properties?: Record<string, any> }) => Promise<Record<string, any>>
+			>(client, "getEventProperties").bind(client)
+
+			const result = await getEventProperties({
+				event: TelemetryEventName.TASK_CREATED,
+				properties: { customProp: "value" },
+			})
+
+			// Should include cached static properties when provider is unavailable
+			expect(result).toEqual({
+				...staticProperties,
+				customProp: "value",
+			})
+		})
+
+		it("should not cache properties when provider does not have appProperties", async () => {
+			const client = new TelemetryClient(mockAuthService, mockSettingsService)
+
+			// Create a mock provider WITHOUT appProperties
+			const mockProvider: TelemetryPropertiesProvider = {
+				getTelemetryProperties: vi.fn().mockResolvedValue({
+					appName: "roo-code",
+					appVersion: "1.0.0",
+					language: "en",
+					mode: "code",
+				}),
+			}
+
+			// Set the provider
+			client.setProvider(mockProvider)
+
+			// Verify cached properties were NOT set (provider doesn't have appProperties)
+			const cachedProps = getPrivateProperty<any>(client, "cachedStaticProperties")
+			expect(cachedProps).toBeNull()
+		})
 	})
 
 	describe("capture", () => {