From 92dd8e36da2e44d74e3df494b0f9e222957cad29 Mon Sep 17 00:00:00 2001 From: cannuri <91494156+cannuri@users.noreply.github.com> Date: Mon, 24 Mar 2025 19:48:36 +0100 Subject: [PATCH] Fix browser tool visibility in system prompt preview (#1840) fix sys prompt browser visibility --- src/core/webview/ClineProvider.ts | 24 +++- .../webview/__tests__/ClineProvider.test.ts | 126 ++++++++++++------ 2 files changed, 107 insertions(+), 43 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 82790aac6e..78c1f76dc5 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -35,7 +35,7 @@ import { import { HistoryItem } from "../../shared/HistoryItem" import { ApiConfigMeta, ExtensionMessage } from "../../shared/ExtensionMessage" import { checkoutDiffPayloadSchema, checkoutRestorePayloadSchema, WebviewMessage } from "../../shared/WebviewMessage" -import { Mode, PromptComponent, defaultModeSlug, ModeConfig } from "../../shared/modes" +import { Mode, PromptComponent, defaultModeSlug, ModeConfig, getModeBySlug, getGroupName } from "../../shared/modes" import { checkExistKey } from "../../shared/checkExistApiConfig" import { EXPERIMENT_IDS, experiments as Experiments, experimentDefault, ExperimentId } from "../../shared/experiments" import { formatLanguage } from "../../shared/language" @@ -2060,9 +2060,25 @@ export class ClineProvider extends EventEmitter implements const rooIgnoreInstructions = this.getCurrentCline()?.rooIgnoreController?.getInstructions() - // Determine if browser tools can be used based on model support and user settings - const modelSupportsComputerUse = this.getCurrentCline()?.api.getModel().info.supportsComputerUse ?? false - const canUseBrowserTool = modelSupportsComputerUse && (browserToolEnabled ?? true) + // Determine if browser tools can be used based on model support, mode, and user settings + let modelSupportsComputerUse = false + + // Create a temporary API handler to check if the model supports computer use + // This avoids relying on an active Cline instance which might not exist during preview + try { + const tempApiHandler = buildApiHandler(apiConfiguration) + modelSupportsComputerUse = tempApiHandler.getModel().info.supportsComputerUse ?? false + } catch (error) { + console.error("Error checking if model supports computer use:", error) + } + + // Check if the current mode includes the browser tool group + const modeConfig = getModeBySlug(mode, customModes) + const modeSupportsBrowser = modeConfig?.groups.some((group) => getGroupName(group) === "browser") ?? false + + // Only enable browser tools if the model supports it, the mode includes browser tools, + // and browser tools are enabled in settings + const canUseBrowserTool = modelSupportsComputerUse && modeSupportsBrowser && (browserToolEnabled ?? true) const systemPrompt = await SYSTEM_PROMPT( this.context, diff --git a/src/core/webview/__tests__/ClineProvider.test.ts b/src/core/webview/__tests__/ClineProvider.test.ts index 3df38a469f..08f9b9f4b5 100644 --- a/src/core/webview/__tests__/ClineProvider.test.ts +++ b/src/core/webview/__tests__/ClineProvider.test.ts @@ -1344,29 +1344,27 @@ describe("ClineProvider", () => { }) // Tests for browser tool support - test("correctly extracts modelSupportsComputerUse from Cline instance", async () => { - // Setup Cline instance with mocked api.getModel() - const { Cline } = require("../../Cline") - const mockCline = new Cline() - mockCline.api = { + test("correctly determines model support for computer use without Cline instance", async () => { + // Mock buildApiHandler to return an API handler with supportsComputerUse: true + const { buildApiHandler } = require("../../../api") + ;(buildApiHandler as jest.Mock).mockImplementation(() => ({ getModel: jest.fn().mockReturnValue({ id: "claude-3-sonnet", info: { supportsComputerUse: true }, }), - } - await provider.addClineToStack(mockCline) + })) // Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly const systemPromptModule = require("../../prompts/system") const systemPromptSpy = jest.spyOn(systemPromptModule, "SYSTEM_PROMPT") - // Mock getState to return browserToolEnabled: true + // Mock getState to return browserToolEnabled: true and a mode that supports browser jest.spyOn(provider, "getState").mockResolvedValue({ apiConfiguration: { apiProvider: "openrouter", }, browserToolEnabled: true, - mode: "code", + mode: "code", // code mode includes browser tool group experiments: experimentDefault, } as any) @@ -1385,16 +1383,14 @@ describe("ClineProvider", () => { }) test("correctly handles when model doesn't support computer use", async () => { - // Setup Cline instance with mocked api.getModel() that doesn't support computer use - const { Cline } = require("../../Cline") - const mockCline = new Cline() - mockCline.api = { + // Mock buildApiHandler to return an API handler with supportsComputerUse: false + const { buildApiHandler } = require("../../../api") + ;(buildApiHandler as jest.Mock).mockImplementation(() => ({ getModel: jest.fn().mockReturnValue({ id: "non-computer-use-model", info: { supportsComputerUse: false }, }), - } - await provider.addClineToStack(mockCline) + })) // Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly const systemPromptModule = require("../../prompts/system") @@ -1426,16 +1422,14 @@ describe("ClineProvider", () => { }) test("correctly handles when browserToolEnabled is false", async () => { - // Setup Cline instance with mocked api.getModel() that supports computer use - const { Cline } = require("../../Cline") - const mockCline = new Cline() - mockCline.api = { + // Mock buildApiHandler to return an API handler with supportsComputerUse: true + const { buildApiHandler } = require("../../../api") + ;(buildApiHandler as jest.Mock).mockImplementation(() => ({ getModel: jest.fn().mockReturnValue({ id: "claude-3-sonnet", info: { supportsComputerUse: true }, }), - } - await provider.addClineToStack(mockCline) + })) // Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly const systemPromptModule = require("../../prompts/system") @@ -1466,38 +1460,92 @@ describe("ClineProvider", () => { expect(callArgs[2]).toBe(false) }) - test("correctly calculates canUseBrowserTool as combination of model support and setting", async () => { - // Setup Cline instance with mocked api.getModel() - const { Cline } = require("../../Cline") - const mockCline = new Cline() - mockCline.api = { + test("correctly handles when mode doesn't include browser tool group", async () => { + // Mock buildApiHandler to return an API handler with supportsComputerUse: true + const { buildApiHandler } = require("../../../api") + ;(buildApiHandler as jest.Mock).mockImplementation(() => ({ getModel: jest.fn().mockReturnValue({ id: "claude-3-sonnet", info: { supportsComputerUse: true }, }), - } - await provider.addClineToStack(mockCline) + })) + + // Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly + const systemPromptModule = require("../../prompts/system") + const systemPromptSpy = jest.spyOn(systemPromptModule, "SYSTEM_PROMPT") + + // Mock getState to return a mode that doesn't include browser tool group + jest.spyOn(provider, "getState").mockResolvedValue({ + apiConfiguration: { + apiProvider: "openrouter", + }, + browserToolEnabled: true, + mode: "custom-mode-without-browser", // Custom mode without browser tool group + experiments: experimentDefault, + } as any) + + // Mock getModeBySlug to return a mode without browser tool group + const modesModule = require("../../../shared/modes") + jest.spyOn(modesModule, "getModeBySlug").mockReturnValue({ + slug: "custom-mode-without-browser", + name: "Custom Mode", + roleDefinition: "Custom role", + groups: ["read", "edit"], // No browser group + }) + + // Trigger getSystemPrompt + const handler = getMessageHandler() + await handler({ type: "getSystemPrompt", mode: "custom-mode-without-browser" }) + + // Verify SYSTEM_PROMPT was called + expect(systemPromptSpy).toHaveBeenCalled() + + // Get the actual arguments passed to SYSTEM_PROMPT + const callArgs = systemPromptSpy.mock.calls[0] + + // Verify the supportsComputerUse parameter (3rd parameter, index 2) + // Even though model supports it and browserToolEnabled is true, the mode doesn't include browser tool group + expect(callArgs[2]).toBe(false) + }) + + test("correctly calculates canUseBrowserTool based on all three conditions", async () => { + // Mock buildApiHandler + const { buildApiHandler } = require("../../../api") // Mock SYSTEM_PROMPT const systemPromptModule = require("../../prompts/system") const systemPromptSpy = jest.spyOn(systemPromptModule, "SYSTEM_PROMPT") - // Test all combinations of model support and browserToolEnabled + // Mock getModeBySlug + const modesModule = require("../../../shared/modes") + + // Test all combinations of model support, mode support, and browserToolEnabled const testCases = [ - { modelSupports: true, settingEnabled: true, expected: true }, - { modelSupports: true, settingEnabled: false, expected: false }, - { modelSupports: false, settingEnabled: true, expected: false }, - { modelSupports: false, settingEnabled: false, expected: false }, + { modelSupports: true, modeSupports: true, settingEnabled: true, expected: true }, + { modelSupports: true, modeSupports: true, settingEnabled: false, expected: false }, + { modelSupports: true, modeSupports: false, settingEnabled: true, expected: false }, + { modelSupports: false, modeSupports: true, settingEnabled: true, expected: false }, + { modelSupports: false, modeSupports: false, settingEnabled: false, expected: false }, ] for (const testCase of testCases) { // Reset mocks systemPromptSpy.mockClear() - // Update mock Cline instance - mockCline.api.getModel = jest.fn().mockReturnValue({ - id: "test-model", - info: { supportsComputerUse: testCase.modelSupports }, + // Mock buildApiHandler to return appropriate model support + ;(buildApiHandler as jest.Mock).mockImplementation(() => ({ + getModel: jest.fn().mockReturnValue({ + id: "test-model", + info: { supportsComputerUse: testCase.modelSupports }, + }), + })) + + // Mock getModeBySlug to return appropriate mode support + jest.spyOn(modesModule, "getModeBySlug").mockReturnValue({ + slug: "test-mode", + name: "Test Mode", + roleDefinition: "Test role", + groups: testCase.modeSupports ? ["read", "browser"] : ["read"], }) // Mock getState @@ -1506,13 +1554,13 @@ describe("ClineProvider", () => { apiProvider: "openrouter", }, browserToolEnabled: testCase.settingEnabled, - mode: "code", + mode: "test-mode", experiments: experimentDefault, } as any) // Trigger getSystemPrompt const handler = getMessageHandler() - await handler({ type: "getSystemPrompt", mode: "code" }) + await handler({ type: "getSystemPrompt", mode: "test-mode" }) // Verify SYSTEM_PROMPT was called expect(systemPromptSpy).toHaveBeenCalled()