code review

Author: liwilliam2021

src/core/webview/__tests__/messageEnhancer.test.ts

@@ -259,23 +259,46 @@ describe("MessageEnhancer", () => {
 	describe("captureTelemetry", () => {
 		it("should capture telemetry when TelemetryService is available", () => {
 			const mockTaskId = "task-123"
-			MessageEnhancer.captureTelemetry(mockTaskId)
+			const mockCaptureEvent = vi.fn()
+			vi.mocked(TelemetryService.instance).captureEvent = mockCaptureEvent
+
+			MessageEnhancer.captureTelemetry(mockTaskId, true)
 
 			expect(TelemetryService.hasInstance).toHaveBeenCalled()
-			expect(TelemetryService.instance.capturePromptEnhanced).toHaveBeenCalledWith(mockTaskId)
+			expect(mockCaptureEvent).toHaveBeenCalledWith(expect.any(String), {
+				taskId: mockTaskId,
+				includeTaskHistory: true,
+			})
 		})
 
 		it("should handle missing TelemetryService gracefully", () => {
 			vi.mocked(TelemetryService).hasInstance = vi.fn().mockReturnValue(false)
 
 			// Should not throw
-			expect(() => MessageEnhancer.captureTelemetry("task-123")).not.toThrow()
+			expect(() => MessageEnhancer.captureTelemetry("task-123", true)).not.toThrow()
 		})
 
 		it("should work without task ID", () => {
-			MessageEnhancer.captureTelemetry()
+			const mockCaptureEvent = vi.fn()
+			vi.mocked(TelemetryService.instance).captureEvent = mockCaptureEvent
 
-			expect(TelemetryService.instance.capturePromptEnhanced).toHaveBeenCalledWith(undefined)
+			MessageEnhancer.captureTelemetry(undefined, false)
+
+			expect(mockCaptureEvent).toHaveBeenCalledWith(expect.any(String), {
+				includeTaskHistory: false,
+			})
+		})
+
+		it("should default includeTaskHistory to false when not provided", () => {
+			const mockCaptureEvent = vi.fn()
+			vi.mocked(TelemetryService.instance).captureEvent = mockCaptureEvent
+
+			MessageEnhancer.captureTelemetry("task-123")
+
+			expect(mockCaptureEvent).toHaveBeenCalledWith(expect.any(String), {
+				taskId: "task-123",
+				includeTaskHistory: false,
+			})
 		})
 	})
 
@@ -299,5 +322,44 @@ describe("MessageEnhancer", () => {
 			expect(history).not.toContain("Tool use")
 			expect(history.split("\n").length).toBe(3) // Only 3 valid messages
 		})
+
+		it("should handle malformed messages gracefully", () => {
+			const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {})
+
+			// Create messages that will cause errors when accessed
+			const malformedMessages = [
+				null,
+				undefined,
+				{ type: "ask" }, // Missing required properties
+				"not an object",
+			] as any
+
+			// Access private method through any type assertion for testing
+			const history = (MessageEnhancer as any).extractTaskHistory(malformedMessages)
+
+			// Should return empty string and log error
+			expect(history).toBe("")
+			expect(consoleSpy).toHaveBeenCalledWith("Failed to extract task history:", expect.any(Error))
+
+			consoleSpy.mockRestore()
+		})
+
+		it("should handle messages with circular references", () => {
+			const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {})
+
+			// Create a message with circular reference
+			const circularMessage: any = { type: "ask", text: "Test" }
+			circularMessage.self = circularMessage
+
+			const messages = [circularMessage] as ClineMessage[]
+
+			// Access private method through any type assertion for testing
+			const history = (MessageEnhancer as any).extractTaskHistory(messages)
+
+			// Should handle gracefully
+			expect(history).toBe("User: Test")
+
+			consoleSpy.mockRestore()
+		})
 	})
 })

src/core/webview/messageEnhancer.ts

@@ -1,4 +1,4 @@
-import { ProviderSettings, ClineMessage, GlobalState } from "@roo-code/types"
+import { ProviderSettings, ClineMessage, GlobalState, TelemetryEventName } from "@roo-code/types"
 import { TelemetryService } from "@roo-code/telemetry"
 import { supportPrompt } from "../../shared/support-prompt"
 import { singleCompletionHandler } from "../../utils/single-completion-handler"
@@ -97,36 +97,47 @@ export class MessageEnhancer {
 	 * @returns Formatted task history string
 	 */
 	private static extractTaskHistory(messages: ClineMessage[]): string {
-		const relevantMessages = messages
-			.filter((msg) => {
-				// Include user messages (type: "ask" with text) and assistant messages (type: "say" with say: "text")
-				if (msg.type === "ask" && msg.text) {
-					return true
-				}
-				if (msg.type === "say" && msg.say === "text" && msg.text) {
-					return true
-				}
-				return false
-			})
-			.slice(-10) // Limit to last 10 messages to avoid context explosion
+		try {
+			const relevantMessages = messages
+				.filter((msg) => {
+					// Include user messages (type: "ask" with text) and assistant messages (type: "say" with say: "text")
+					if (msg.type === "ask" && msg.text) {
+						return true
+					}
+					if (msg.type === "say" && msg.say === "text" && msg.text) {
+						return true
+					}
+					return false
+				})
+				.slice(-10) // Limit to last 10 messages to avoid context explosion
 
-		return relevantMessages
-			.map((msg) => {
-				const role = msg.type === "ask" ? "User" : "Assistant"
-				const content = msg.text || ""
-				// Truncate long messages
-				return `${role}: ${content.slice(0, 500)}${content.length > 500 ? "..." : ""}`
-			})
-			.join("\n")
+			return relevantMessages
+				.map((msg) => {
+					const role = msg.type === "ask" ? "User" : "Assistant"
+					const content = msg.text || ""
+					// Truncate long messages
+					return `${role}: ${content.slice(0, 500)}${content.length > 500 ? "..." : ""}`
+				})
+				.join("\n")
+		} catch (error) {
+			// Log error but don't fail the enhancement
+			console.error("Failed to extract task history:", error)
+			return ""
+		}
 	}
 
 	/**
 	 * Captures telemetry for prompt enhancement
 	 * @param taskId Optional task ID for telemetry tracking
+	 * @param includeTaskHistory Whether task history was included in the enhancement
 	 */
-	static captureTelemetry(taskId?: string): void {
+	static captureTelemetry(taskId?: string, includeTaskHistory?: boolean): void {
 		if (TelemetryService.hasInstance()) {
-			TelemetryService.instance.capturePromptEnhanced(taskId)
+			// Use captureEvent directly to include the includeTaskHistory property
+			TelemetryService.instance.captureEvent(TelemetryEventName.PROMPT_ENHANCED, {
+				...(taskId && { taskId }),
+				includeTaskHistory: includeTaskHistory ?? false,
+			})
 		}
 	}
 }

src/core/webview/webviewMessageHandler.ts

@@ -1340,8 +1340,13 @@ export const webviewMessageHandler = async (
 			if (message.text) {
 				try {
 					const state = await provider.getState()
-					const { apiConfiguration, customSupportPrompts, listApiConfigMeta, enhancementApiConfigId } = state
-					const includeTaskHistoryInEnhance = (state as any).includeTaskHistoryInEnhance
+					const {
+						apiConfiguration,
+						customSupportPrompts,
+						listApiConfigMeta,
+						enhancementApiConfigId,
+						includeTaskHistoryInEnhance,
+					} = state
 
 					const currentCline = provider.getCurrentCline()
 					const result = await MessageEnhancer.enhanceMessage({
@@ -1357,7 +1362,7 @@ export const webviewMessageHandler = async (
 
 					if (result.success && result.enhancedText) {
 						// Capture telemetry for prompt enhancement
-						MessageEnhancer.captureTelemetry(currentCline?.taskId)
+						MessageEnhancer.captureTelemetry(currentCline?.taskId, includeTaskHistoryInEnhance)
 						await provider.postMessageToWebview({ type: "enhancedPrompt", text: result.enhancedText })
 					} else {
 						throw new Error(result.error || "Unknown error")