diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index d9f546d463..6a65c858f9 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -398,95 +398,71 @@ describe("Cline", () => { describe("getEnvironmentDetails", () => { describe("API conversation handling", () => { - it.skip("should clean conversation history before sending to API", async () => { - // Cline.create will now use our mocked getEnvironmentDetails - const [cline, task] = Task.create({ + beforeEach(() => { + Task.resetGlobalApiRequestTime() + }) + + afterEach(() => { + Task.resetGlobalApiRequestTime() + }) + + it("should strip non-protocol fields from API conversation history before sending to the API", async () => { + const cline = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, task: "test task", + startTask: false, }) + vi.spyOn(cline as any, "getSystemPrompt").mockResolvedValue("mock system prompt") - cline.abandoned = true - await task - - // Set up mock stream. - const mockStreamForClean = (async function* () { - yield { type: "text", text: "test response" } - })() - - // Set up spy. - const cleanMessageSpy = vi.fn().mockReturnValue(mockStreamForClean) - vi.spyOn(cline.api, "createMessage").mockImplementation(cleanMessageSpy) + const mockStream = { + async *[Symbol.asyncIterator]() { + yield { type: "text", text: "response" } + }, + async next() { + return { done: true, value: { type: "text", text: "response" } } + }, + async return() { + return { done: true, value: undefined } + }, + async throw(error: any) { + throw error + }, + async [Symbol.asyncDispose]() { + // Cleanup + }, + } as AsyncGenerator + const createMessageSpy = vi.spyOn(cline.api, "createMessage").mockReturnValue(mockStream) - // Add test message to conversation history. cline.apiConversationHistory = [ { role: "user" as const, content: [{ type: "text" as const, text: "test message" }], ts: Date.now(), + extraProp: "should be removed", }, - ] + ] as any - // Mock abort state - Object.defineProperty(cline, "abort", { - get: () => false, - set: () => {}, - configurable: true, - }) - - // Add a message with extra properties to the conversation history - const messageWithExtra = { - role: "user" as const, - content: [{ type: "text" as const, text: "test message" }], - ts: Date.now(), - extraProp: "should be removed", - } - - cline.apiConversationHistory = [messageWithExtra] - - // Trigger an API request - await cline.recursivelyMakeClineRequests([{ type: "text", text: "test request" }], false) - - // Get the conversation history from the first API call - expect(cleanMessageSpy.mock.calls.length).toBeGreaterThan(0) - const history = cleanMessageSpy.mock.calls[0]?.[1] - expect(history).toBeDefined() - expect(history.length).toBeGreaterThan(0) + const iterator = cline.attemptApiRequest(0) + await iterator.next() - // Find our test message - const cleanedMessage = history.find((msg: { content?: Array<{ text: string }> }) => - msg.content?.some((content) => content.text === "test message"), - ) - expect(cleanedMessage).toBeDefined() - expect(cleanedMessage).toEqual({ - role: "user", - content: [{ type: "text", text: "test message" }], - }) + const [, cleanConversationHistory] = createMessageSpy.mock.calls[0]! - // Verify extra properties were removed - expect(Object.keys(cleanedMessage!)).toEqual(["role", "content"]) + expect(cleanConversationHistory).toEqual([ + { + role: "user", + content: [{ type: "text", text: "test message" }], + }, + ]) + expect(Object.keys(cleanConversationHistory[0]!)).toEqual(["role", "content"]) }) - it.skip("should handle image blocks based on model capabilities", async () => { - // Create two configurations - one with image support, one without - const configWithImages = { - ...mockApiConfig, - apiModelId: "claude-3-sonnet", - } - const configWithoutImages = { - ...mockApiConfig, - apiModelId: "gpt-3.5-turbo", - } - - // Create test conversation history with mixed content - const conversationHistory: (Anthropic.MessageParam & { ts?: number })[] = [ + it("should shape image blocks for API compatibility before request construction", async () => { + const conversationHistory = [ { role: "user" as const, content: [ - { - type: "text" as const, - text: "Here is an image", - } satisfies Anthropic.TextBlockParam, + { type: "text" as const, text: "Here is an image" }, { type: "image" as const, source: { @@ -494,29 +470,23 @@ describe("Cline", () => { media_type: "image/jpeg", data: "base64data", }, - } satisfies Anthropic.ImageBlockParam, - ], - }, - { - role: "assistant" as const, - content: [ - { - type: "text" as const, - text: "I see the image", - } satisfies Anthropic.TextBlockParam, + }, ], }, ] - // Test with model that supports images - const [clineWithImages, taskWithImages] = Task.create({ + const withImages = new Task({ provider: mockProvider, - apiConfiguration: configWithImages, + apiConfiguration: { + ...mockApiConfig, + apiModelId: "claude-3-sonnet", + }, task: "test task", + startTask: false, }) + vi.spyOn(withImages as any, "getSystemPrompt").mockResolvedValue("mock system prompt") - // Mock the model info to indicate image support - vi.spyOn(clineWithImages.api, "getModel").mockReturnValue({ + vi.spyOn(withImages.api, "getModel").mockReturnValue({ id: "claude-3-sonnet", info: { supportsImages: true, @@ -528,17 +498,18 @@ describe("Cline", () => { } as ModelInfo, }) - clineWithImages.apiConversationHistory = conversationHistory - - // Test with model that doesn't support images - const [clineWithoutImages, taskWithoutImages] = Task.create({ + const withoutImages = new Task({ provider: mockProvider, - apiConfiguration: configWithoutImages, + apiConfiguration: { + ...mockApiConfig, + apiModelId: "gpt-3.5-turbo", + }, task: "test task", + startTask: false, }) + vi.spyOn(withoutImages as any, "getSystemPrompt").mockResolvedValue("mock system prompt") - // Mock the model info to indicate no image support - vi.spyOn(clineWithoutImages.api, "getModel").mockReturnValue({ + vi.spyOn(withoutImages.api, "getModel").mockReturnValue({ id: "gpt-3.5-turbo", info: { supportsImages: false, @@ -550,88 +521,72 @@ describe("Cline", () => { } as ModelInfo, }) - clineWithoutImages.apiConversationHistory = conversationHistory - - // Mock abort state for both instances - Object.defineProperty(clineWithImages, "abort", { - get: () => false, - set: () => {}, - configurable: true, - }) - - Object.defineProperty(clineWithoutImages, "abort", { - get: () => false, - set: () => {}, - configurable: true, - }) - - // Set up mock streams - const mockStreamWithImages = (async function* () { - yield { type: "text", text: "test response" } - })() + const mockStream = { + async *[Symbol.asyncIterator]() { + yield { type: "text", text: "response" } + }, + async next() { + return { done: true, value: { type: "text", text: "response" } } + }, + async return() { + return { done: true, value: undefined } + }, + async throw(error: any) { + throw error + }, + async [Symbol.asyncDispose]() { + // Cleanup + }, + } as AsyncGenerator + const withImagesSpy = vi.spyOn(withImages.api, "createMessage").mockReturnValue(mockStream) + const withoutImagesSpy = vi.spyOn(withoutImages.api, "createMessage").mockReturnValue(mockStream) - const mockStreamWithoutImages = (async function* () { - yield { type: "text", text: "test response" } - })() + withImages.apiConversationHistory = conversationHistory as any + withoutImages.apiConversationHistory = conversationHistory as any - // Set up spies - const imagesSpy = vi.fn().mockReturnValue(mockStreamWithImages) - const noImagesSpy = vi.fn().mockReturnValue(mockStreamWithoutImages) + const withImagesIterator = withImages.attemptApiRequest(0) + await withImagesIterator.next() + const withoutImagesIterator = withoutImages.attemptApiRequest(0) + await withoutImagesIterator.next() - vi.spyOn(clineWithImages.api, "createMessage").mockImplementation(imagesSpy) - vi.spyOn(clineWithoutImages.api, "createMessage").mockImplementation(noImagesSpy) + const [, historyWithImages] = withImagesSpy.mock.calls[0]! + const [, historyWithoutImages] = withoutImagesSpy.mock.calls[0]! - // Set up conversation history with images - clineWithImages.apiConversationHistory = [ + expect(historyWithImages).toEqual([ { role: "user", content: [ { type: "text", text: "Here is an image" }, - { type: "image", source: { type: "base64", media_type: "image/jpeg", data: "base64data" } }, + { + type: "image", + source: { + type: "base64", + media_type: "image/jpeg", + data: "base64data", + }, + }, ], }, - ] - - clineWithImages.abandoned = true - await taskWithImages.catch(() => {}) - - clineWithoutImages.abandoned = true - await taskWithoutImages.catch(() => {}) - - // Trigger API requests - await clineWithImages.recursivelyMakeClineRequests([{ type: "text", text: "test request" }]) - await clineWithoutImages.recursivelyMakeClineRequests([{ type: "text", text: "test request" }]) - - // Get the calls - const imagesCalls = imagesSpy.mock.calls - const noImagesCalls = noImagesSpy.mock.calls - - // Verify model with image support preserves image blocks - expect(imagesCalls.length).toBeGreaterThan(0) - if (imagesCalls[0]?.[1]?.[0]?.content) { - expect(imagesCalls[0][1][0].content).toHaveLength(2) - expect(imagesCalls[0][1][0].content[0]).toEqual({ type: "text", text: "Here is an image" }) - expect(imagesCalls[0][1][0].content[1]).toHaveProperty("type", "image") - } - - // Verify model without image support converts image blocks to text - expect(noImagesCalls.length).toBeGreaterThan(0) - if (noImagesCalls[0]?.[1]?.[0]?.content) { - expect(noImagesCalls[0][1][0].content).toHaveLength(2) - expect(noImagesCalls[0][1][0].content[0]).toEqual({ type: "text", text: "Here is an image" }) - expect(noImagesCalls[0][1][0].content[1]).toEqual({ - type: "text", - text: "[Referenced image in conversation]", - }) - } + ]) + expect(historyWithoutImages).toEqual([ + { + role: "user", + content: [ + { type: "text", text: "Here is an image" }, + { type: "text", text: "[Referenced image in conversation]" }, + ], + }, + ]) }) - it.skip("should handle API retry with countdown", async () => { - const [cline, task] = Task.create({ + it("should handle API retry with countdown", async () => { + const cline = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, task: "test task", + startTask: false, }) + vi.spyOn(cline as any, "getSystemPrompt").mockResolvedValue("mock system prompt") // Mock delay to track countdown timing const mockDelay = vi.fn().mockResolvedValue(undefined) @@ -689,70 +644,37 @@ describe("Cline", () => { } return mockSuccessStream }) - - // Set up mock state - mockProvider.getState = vi.fn().mockResolvedValue({}) - - // Mock previous API request message - cline.clineMessages = [ - { - ts: Date.now(), - type: "say", - say: "api_req_started", - text: JSON.stringify({ - tokensIn: 100, - tokensOut: 50, - cacheWrites: 0, - cacheReads: 0, - }), - }, - ] + const providerState = await mockProvider.getState() + vi.spyOn(mockProvider, "getState").mockResolvedValue({ + ...providerState, + apiConfiguration: mockApiConfig, + autoApprovalEnabled: true, + requestDelaySeconds: 3, + }) // Trigger API request const iterator = cline.attemptApiRequest(0) await iterator.next() - // Calculate expected delay for first retry - const baseDelay = 3 // test retry delay - - // Verify countdown messages - for (let i = baseDelay; i > 0; i--) { - expect(saySpy).toHaveBeenCalledWith( - "api_req_retry_delayed", - expect.stringContaining(`Retrying in ${i} seconds`), - undefined, - true, - ) - } - - expect(saySpy).toHaveBeenCalledWith( - "api_req_retry_delayed", - expect.stringContaining("Retrying now"), - undefined, - false, - ) - - // Calculate expected delay calls for countdown - const totalExpectedDelays = baseDelay // One delay per second for countdown - expect(mockDelay).toHaveBeenCalledTimes(totalExpectedDelays) + const retryMessages = saySpy.mock.calls.filter((call) => call[0] === "api_req_retry_delayed") + expect(retryMessages).toEqual([ + ["api_req_retry_delayed", "API Error\n3", undefined, true], + ["api_req_retry_delayed", "API Error\n2", undefined, true], + ["api_req_retry_delayed", "API Error\n1", undefined, true], + ["api_req_retry_delayed", "API Error\n", undefined, false], + ]) + expect(mockDelay).toHaveBeenCalledTimes(3) expect(mockDelay).toHaveBeenCalledWith(1000) - - // Verify error message content - const errorMessage = saySpy.mock.calls.find((call) => call[1]?.includes(mockError.message))?.[1] - expect(errorMessage).toBe( - `${mockError.message}\n\nRetry attempt 1\nRetrying in ${baseDelay} seconds...`, - ) - - await cline.abortTask(true) - await task.catch(() => {}) }) - it.skip("should not apply retry delay twice", async () => { - const [cline, task] = Task.create({ + it("should not apply retry delay twice", async () => { + const cline = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, task: "test task", + startTask: false, }) + vi.spyOn(cline as any, "getSystemPrompt").mockResolvedValue("mock system prompt") // Mock delay to track countdown timing const mockDelay = vi.fn().mockResolvedValue(undefined) @@ -810,61 +732,30 @@ describe("Cline", () => { } return mockSuccessStream }) - - // Set up mock state - mockProvider.getState = vi.fn().mockResolvedValue({}) - - // Mock previous API request message - cline.clineMessages = [ - { - ts: Date.now(), - type: "say", - say: "api_req_started", - text: JSON.stringify({ - tokensIn: 100, - tokensOut: 50, - cacheWrites: 0, - cacheReads: 0, - }), - }, - ] + const providerState = await mockProvider.getState() + vi.spyOn(mockProvider, "getState").mockResolvedValue({ + ...providerState, + apiConfiguration: mockApiConfig, + autoApprovalEnabled: true, + requestDelaySeconds: 3, + }) // Trigger API request const iterator = cline.attemptApiRequest(0) await iterator.next() - // Verify delay is only applied for the countdown - const baseDelay = 3 // test retry delay - const expectedDelayCount = baseDelay // One delay per second for countdown - expect(mockDelay).toHaveBeenCalledTimes(expectedDelayCount) + expect(mockDelay).toHaveBeenCalledTimes(3) expect(mockDelay).toHaveBeenCalledWith(1000) // Each delay should be 1 second // Verify countdown messages were only shown once const retryMessages = saySpy.mock.calls.filter( - (call) => call[0] === "api_req_retry_delayed" && call[1]?.includes("Retrying in"), - ) - expect(retryMessages).toHaveLength(baseDelay) - - // Verify the retry message sequence - for (let i = baseDelay; i > 0; i--) { - expect(saySpy).toHaveBeenCalledWith( - "api_req_retry_delayed", - expect.stringContaining(`Retrying in ${i} seconds`), - undefined, - true, - ) - } - - // Verify final retry message - expect(saySpy).toHaveBeenCalledWith( - "api_req_retry_delayed", - expect.stringContaining("Retrying now"), - undefined, - false, + (call) => call[0] === "api_req_retry_delayed" && call[3] === true, ) - - await cline.abortTask(true) - await task.catch(() => {}) + expect(retryMessages).toEqual([ + ["api_req_retry_delayed", "API Error\n3", undefined, true], + ["api_req_retry_delayed", "API Error\n2", undefined, true], + ["api_req_retry_delayed", "API Error\n1", undefined, true], + ]) }) describe("processUserContentMentions", () => { diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index df8858ec78..d25c6971d2 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1,5 +1,7 @@ // pnpm --filter roo-cline test core/webview/__tests__/ClineProvider.spec.ts +import * as path from "path" + import Anthropic from "@anthropic-ai/sdk" import * as vscode from "vscode" import axios from "axios" @@ -51,6 +53,14 @@ vi.mock("axios", () => ({ vi.mock("../../../utils/safeWriteJson") +vi.mock("../../../utils/path", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + getWorkspacePath: vi.fn().mockReturnValue(""), + } +}) + vi.mock("../../../utils/storage", () => ({ getSettingsDirectoryPath: vi.fn().mockResolvedValue("/test/settings/path"), getTaskDirectoryPath: vi.fn().mockResolvedValue("/test/task/path"), @@ -115,6 +125,11 @@ vi.mock("vscode", () => ({ ExtensionContext: vi.fn(), OutputChannel: vi.fn(), WebviewView: vi.fn(), + EventEmitter: vi.fn().mockImplementation(() => ({ + event: vi.fn(), + fire: vi.fn(), + dispose: vi.fn(), + })), Uri: { joinPath: vi.fn(), file: vi.fn(), @@ -130,6 +145,7 @@ vi.mock("vscode", () => ({ showInformationMessage: vi.fn(), showWarningMessage: vi.fn(), showErrorMessage: vi.fn(), + activeTextEditor: undefined, onDidChangeActiveTextEditor: vi.fn(() => ({ dispose: vi.fn() })), }, workspace: { @@ -137,6 +153,7 @@ vi.mock("vscode", () => ({ get: vi.fn().mockReturnValue([]), update: vi.fn(), }), + getWorkspaceFolder: vi.fn(), onDidChangeConfiguration: vi.fn().mockImplementation(() => ({ dispose: vi.fn(), })), @@ -2022,8 +2039,10 @@ describe("Project MCP Settings", () => { let mockWebviewView: vscode.WebviewView let mockPostMessage: any - beforeEach(() => { + beforeEach(async () => { vi.clearAllMocks() + const pathUtils = await import("../../../utils/path") + vi.mocked(pathUtils.getWorkspacePath).mockReturnValue("") mockContext = { extensionPath: "/test/path", @@ -2072,13 +2091,16 @@ describe("Project MCP Settings", () => { onDidDispose: vi.fn(), onDidChangeVisibility: vi.fn(), } as unknown as vscode.WebviewView - + ;(vscode.window as any).activeTextEditor = undefined + ;(vscode.workspace.getWorkspaceFolder as any).mockReset() provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) }) - test.skip("handles openProjectMcpSettings message", async () => { + test("handles openProjectMcpSettings message", async () => { // Mock workspace folders first ;(vscode.workspace as any).workspaceFolders = [{ uri: { fsPath: "/test/workspace" } }] + const pathUtils = await import("../../../utils/path") + vi.mocked(pathUtils.getWorkspacePath).mockReturnValue("/test/workspace") // Mock fs functions const fs = await import("fs/promises") @@ -2109,14 +2131,18 @@ describe("Project MCP Settings", () => { type: "openProjectMcpSettings", }) + const expectedRooDir = path.join("/test/workspace", ".roo") + const expectedMcpPath = path.join(expectedRooDir, "mcp.json") + // Check that fs.mkdir was called with the correct path - expect(mockedFs.mkdir).toHaveBeenCalledWith("/test/workspace/.roo", { recursive: true }) + expect(mockedFs.mkdir).toHaveBeenCalledWith(expectedRooDir, { recursive: true }) + expect(pathUtils.getWorkspacePath).toHaveBeenCalled() // Verify file was created with default content - expect(safeWriteJson).toHaveBeenCalledWith("/test/workspace/.roo/mcp.json", { mcpServers: {} }) + expect(safeWriteJson).toHaveBeenCalledWith(expectedMcpPath, { mcpServers: {} }, { prettyPrint: true }) // Check that openFile was called - expect(openFileSpy).toHaveBeenCalledWith("/test/workspace/.roo/mcp.json") + expect(openFileSpy).toHaveBeenCalledWith(expectedMcpPath) }) test("handles openProjectMcpSettings when workspace is not open", async () => { @@ -2133,86 +2159,27 @@ describe("Project MCP Settings", () => { expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.no_workspace") }) - test.skip("handles openProjectMcpSettings file creation error", async () => { + test("handles openProjectMcpSettings file creation error", async () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] // Mock workspace folders ;(vscode.workspace as any).workspaceFolders = [{ uri: { fsPath: "/test/workspace" } }] + const pathUtils = await import("../../../utils/path") + vi.mocked(pathUtils.getWorkspacePath).mockReturnValue("/test/workspace") // Mock fs functions to fail - const fs = require("fs/promises") - fs.mkdir.mockRejectedValue(new Error("Failed to create directory")) + const fs = await import("fs/promises") + const mockedFs = vi.mocked(fs) + mockedFs.mkdir.mockRejectedValue(new Error("Failed to create directory")) // Trigger openProjectMcpSettings await messageHandler({ type: "openProjectMcpSettings", }) - // Verify error message was shown - expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( - expect.stringContaining("Failed to create or open .roo/mcp.json"), - ) - }) -}) - -describe.skip("ContextProxy integration", () => { - let provider: ClineProvider - let mockContext: vscode.ExtensionContext - let mockOutputChannel: vscode.OutputChannel - let mockContextProxy: any - - beforeEach(() => { - // Reset mocks - vi.clearAllMocks() - - // Setup basic mocks - mockContext = { - globalState: { - get: vi.fn(), - update: vi.fn(), - keys: vi.fn().mockReturnValue([]), - }, - workspaceState: { - get: vi.fn().mockReturnValue(undefined), - update: vi.fn().mockResolvedValue(undefined), - keys: vi.fn().mockReturnValue([]), - }, - secrets: { get: vi.fn(), store: vi.fn(), delete: vi.fn() }, - extensionUri: { fsPath: "/test/path" } as vscode.Uri, - globalStorageUri: { fsPath: "/test/path" }, - extension: { packageJSON: { version: "1.0.0" } }, - } as unknown as vscode.ExtensionContext - - mockOutputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel - mockContextProxy = new ContextProxy(mockContext) - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", mockContextProxy) - }) - - test("updateGlobalState uses contextProxy", async () => { - await provider.setValue("currentApiConfigName", "testValue") - expect(mockContextProxy.updateGlobalState).toHaveBeenCalledWith("currentApiConfigName", "testValue") - }) - - test("getGlobalState uses contextProxy", async () => { - mockContextProxy.getGlobalState.mockResolvedValueOnce("testValue") - const result = await provider.getValue("currentApiConfigName") - expect(mockContextProxy.getGlobalState).toHaveBeenCalledWith("currentApiConfigName") - expect(result).toBe("testValue") - }) - - test("storeSecret uses contextProxy", async () => { - await provider.setValue("apiKey", "test-secret") - expect(mockContextProxy.storeSecret).toHaveBeenCalledWith("apiKey", "test-secret") - }) - - test("contextProxy methods are available", () => { - // Verify the contextProxy has all the required methods - expect(mockContextProxy.getGlobalState).toBeDefined() - expect(mockContextProxy.updateGlobalState).toBeDefined() - expect(mockContextProxy.storeSecret).toBeDefined() - expect(mockContextProxy.setValue).toBeDefined() - expect(mockContextProxy.setValues).toBeDefined() + // Verify the translated error key is surfaced in test mode + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.create_json") }) })