fix(webview): serialize webviewDidLaunch telemetry init, polish locale copy

Author: edelauna

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(),
@@ -1533,4 +1537,64 @@ describe("webviewMessageHandler - telemetrySetting", () => {
 		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).
+		let storedSetting: string | undefined = "unset"
+		vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation(() => storedSetting)
+		vi.mocked(mockClineProvider.contextProxy.setValue).mockImplementation(async (_key, value) => {
+			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),
+		}
+		// Only exercised by the pre-fix code path (provider.getStateToPostToWebview().then(...));
+		// snapshots storedSetting at call time (mirroring the real ClineProvider building its
+		// state object synchronously before any internal awaits) and resolves slowly, so a
+		// concurrent write that lands during that delay is a genuinely stale read by the time
+		// the .then() callback runs -- reproducing the pre-fix race.
+		;(mockClineProvider as any).getStateToPostToWebview = vi.fn().mockImplementation(async () => {
+			const snapshot = storedSetting
+			await new Promise((resolve) => setTimeout(resolve, 10))
+			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)
+
+		// A concurrent "telemetrySetting" message turns telemetry off, arriving just after
+		// webviewDidLaunch has taken its (pre-fix) state snapshot but well before the slow
+		// getStateToPostToWebview() mock resolves -- reproducing the window where that snapshot
+		// is stale by the time its .then() callback actually runs.
+		const disable = new Promise((resolve) => setTimeout(resolve, 2)).then(() =>
+			webviewMessageHandler(mockClineProvider, { type: "telemetrySetting", text: "disabled" }),
+		)
+
+		await Promise.all([launch, disable])
+
+		// webviewDidLaunch's telemetry init is fire-and-forget from the handler's own point of
+		// view (the "webviewDidLaunch" case doesn't await it), so waiting on `launch` alone isn't
+		// enough to observe its trailing continuation -- give the slow getStateToPostToWebview()
+		// mock time to resolve too.
+		await new Promise((resolve) => setTimeout(resolve, 50))
+
+		// 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])
+	})
 })

src/core/webview/webviewMessageHandler.ts

@@ -643,12 +643,22 @@ export const webviewMessageHandler = async (
 			// 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.
-			provider.getStateToPostToWebview().then((state) => {
-				const { telemetrySetting } = state
-				TelemetryService.instance.updateTelemetryState(
-					isTelemetryOptedIn(telemetrySetting) && vscode.env.isTelemetryEnabled,
-				)
-			})
+			//
+			// 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