Skip to content

Commit cc0b0a7

Browse files
cannuricannuri
committed
fix sys prompt browser visibility
1 parent 891a55d commit cc0b0a7

File tree

2 files changed

+107
-43
lines changed

2 files changed

+107
-43
lines changed

‎src/core/webview/ClineProvider.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
import { HistoryItem } from "../../shared/HistoryItem"
2525
import { ApiConfigMeta, ExtensionMessage } from "../../shared/ExtensionMessage"
2626
import { checkoutDiffPayloadSchema, checkoutRestorePayloadSchema, WebviewMessage } from "../../shared/WebviewMessage"
27-
import { Mode, PromptComponent, defaultModeSlug, ModeConfig } from "../../shared/modes"
27+
import { Mode, PromptComponent, defaultModeSlug, ModeConfig, getModeBySlug, getGroupName } from "../../shared/modes"
2828
import { checkExistKey } from "../../shared/checkExistApiConfig"
2929
import { EXPERIMENT_IDS, experiments as Experiments, experimentDefault, ExperimentId } from "../../shared/experiments"
3030
import { formatLanguage } from "../../shared/language"
@@ -2009,9 +2009,25 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
20092009

20102010
const rooIgnoreInstructions = this.getCurrentCline()?.rooIgnoreController?.getInstructions()
20112011

2012-
// Determine if browser tools can be used based on model support and user settings
2013-
const modelSupportsComputerUse = this.getCurrentCline()?.api.getModel().info.supportsComputerUse ?? false
2014-
const canUseBrowserTool = modelSupportsComputerUse && (browserToolEnabled ?? true)
2012+
// Determine if browser tools can be used based on model support, mode, and user settings
2013+
let modelSupportsComputerUse = false
2014+
2015+
// Create a temporary API handler to check if the model supports computer use
2016+
// This avoids relying on an active Cline instance which might not exist during preview
2017+
try {
2018+
const tempApiHandler = buildApiHandler(apiConfiguration)
2019+
modelSupportsComputerUse = tempApiHandler.getModel().info.supportsComputerUse ?? false
2020+
} catch (error) {
2021+
console.error("Error checking if model supports computer use:", error)
2022+
}
2023+
2024+
// Check if the current mode includes the browser tool group
2025+
const modeConfig = getModeBySlug(mode, customModes)
2026+
const modeSupportsBrowser = modeConfig?.groups.some((group) => getGroupName(group) === "browser") ?? false
2027+
2028+
// Only enable browser tools if the model supports it, the mode includes browser tools,
2029+
// and browser tools are enabled in settings
2030+
const canUseBrowserTool = modelSupportsComputerUse && modeSupportsBrowser && (browserToolEnabled ?? true)
20152031

20162032
const systemPrompt = await SYSTEM_PROMPT(
20172033
this.context,

‎src/core/webview/__tests__/ClineProvider.test.ts‎

Lines changed: 87 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1414,29 +1414,27 @@ describe("ClineProvider", () => {
14141414
})
14151415

14161416
// Tests for browser tool support
1417-
test("correctly extracts modelSupportsComputerUse from Cline instance", async () => {
1418-
// Setup Cline instance with mocked api.getModel()
1419-
const { Cline } = require("../../Cline")
1420-
const mockCline = new Cline()
1421-
mockCline.api = {
1417+
test("correctly determines model support for computer use without Cline instance", async () => {
1418+
// Mock buildApiHandler to return an API handler with supportsComputerUse: true
1419+
const { buildApiHandler } = require("../../../api")
1420+
;(buildApiHandler as jest.Mock).mockImplementation(() => ({
14221421
getModel: jest.fn().mockReturnValue({
14231422
id: "claude-3-sonnet",
14241423
info: { supportsComputerUse: true },
14251424
}),
1426-
}
1427-
await provider.addClineToStack(mockCline)
1425+
}))
14281426

14291427
// Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly
14301428
const systemPromptModule = require("../../prompts/system")
14311429
const systemPromptSpy = jest.spyOn(systemPromptModule, "SYSTEM_PROMPT")
14321430

1433-
// Mock getState to return browserToolEnabled: true
1431+
// Mock getState to return browserToolEnabled: true and a mode that supports browser
14341432
jest.spyOn(provider, "getState").mockResolvedValue({
14351433
apiConfiguration: {
14361434
apiProvider: "openrouter",
14371435
},
14381436
browserToolEnabled: true,
1439-
mode: "code",
1437+
mode: "code", // code mode includes browser tool group
14401438
experiments: experimentDefault,
14411439
} as any)
14421440

@@ -1455,16 +1453,14 @@ describe("ClineProvider", () => {
14551453
})
14561454

14571455
test("correctly handles when model doesn't support computer use", async () => {
1458-
// Setup Cline instance with mocked api.getModel() that doesn't support computer use
1459-
const { Cline } = require("../../Cline")
1460-
const mockCline = new Cline()
1461-
mockCline.api = {
1456+
// Mock buildApiHandler to return an API handler with supportsComputerUse: false
1457+
const { buildApiHandler } = require("../../../api")
1458+
;(buildApiHandler as jest.Mock).mockImplementation(() => ({
14621459
getModel: jest.fn().mockReturnValue({
14631460
id: "non-computer-use-model",
14641461
info: { supportsComputerUse: false },
14651462
}),
1466-
}
1467-
await provider.addClineToStack(mockCline)
1463+
}))
14681464

14691465
// Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly
14701466
const systemPromptModule = require("../../prompts/system")
@@ -1496,16 +1492,14 @@ describe("ClineProvider", () => {
14961492
})
14971493

14981494
test("correctly handles when browserToolEnabled is false", async () => {
1499-
// Setup Cline instance with mocked api.getModel() that supports computer use
1500-
const { Cline } = require("../../Cline")
1501-
const mockCline = new Cline()
1502-
mockCline.api = {
1495+
// Mock buildApiHandler to return an API handler with supportsComputerUse: true
1496+
const { buildApiHandler } = require("../../../api")
1497+
;(buildApiHandler as jest.Mock).mockImplementation(() => ({
15031498
getModel: jest.fn().mockReturnValue({
15041499
id: "claude-3-sonnet",
15051500
info: { supportsComputerUse: true },
15061501
}),
1507-
}
1508-
await provider.addClineToStack(mockCline)
1502+
}))
15091503

15101504
// Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly
15111505
const systemPromptModule = require("../../prompts/system")
@@ -1536,38 +1530,92 @@ describe("ClineProvider", () => {
15361530
expect(callArgs[2]).toBe(false)
15371531
})
15381532

1539-
test("correctly calculates canUseBrowserTool as combination of model support and setting", async () => {
1540-
// Setup Cline instance with mocked api.getModel()
1541-
const { Cline } = require("../../Cline")
1542-
const mockCline = new Cline()
1543-
mockCline.api = {
1533+
test("correctly handles when mode doesn't include browser tool group", async () => {
1534+
// Mock buildApiHandler to return an API handler with supportsComputerUse: true
1535+
const { buildApiHandler } = require("../../../api")
1536+
;(buildApiHandler as jest.Mock).mockImplementation(() => ({
15441537
getModel: jest.fn().mockReturnValue({
15451538
id: "claude-3-sonnet",
15461539
info: { supportsComputerUse: true },
15471540
}),
1548-
}
1549-
await provider.addClineToStack(mockCline)
1541+
}))
1542+
1543+
// Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly
1544+
const systemPromptModule = require("../../prompts/system")
1545+
const systemPromptSpy = jest.spyOn(systemPromptModule, "SYSTEM_PROMPT")
1546+
1547+
// Mock getState to return a mode that doesn't include browser tool group
1548+
jest.spyOn(provider, "getState").mockResolvedValue({
1549+
apiConfiguration: {
1550+
apiProvider: "openrouter",
1551+
},
1552+
browserToolEnabled: true,
1553+
mode: "custom-mode-without-browser", // Custom mode without browser tool group
1554+
experiments: experimentDefault,
1555+
} as any)
1556+
1557+
// Mock getModeBySlug to return a mode without browser tool group
1558+
const modesModule = require("../../../shared/modes")
1559+
jest.spyOn(modesModule, "getModeBySlug").mockReturnValue({
1560+
slug: "custom-mode-without-browser",
1561+
name: "Custom Mode",
1562+
roleDefinition: "Custom role",
1563+
groups: ["read", "edit"], // No browser group
1564+
})
1565+
1566+
// Trigger getSystemPrompt
1567+
const handler = getMessageHandler()
1568+
await handler({ type: "getSystemPrompt", mode: "custom-mode-without-browser" })
1569+
1570+
// Verify SYSTEM_PROMPT was called
1571+
expect(systemPromptSpy).toHaveBeenCalled()
1572+
1573+
// Get the actual arguments passed to SYSTEM_PROMPT
1574+
const callArgs = systemPromptSpy.mock.calls[0]
1575+
1576+
// Verify the supportsComputerUse parameter (3rd parameter, index 2)
1577+
// Even though model supports it and browserToolEnabled is true, the mode doesn't include browser tool group
1578+
expect(callArgs[2]).toBe(false)
1579+
})
1580+
1581+
test("correctly calculates canUseBrowserTool based on all three conditions", async () => {
1582+
// Mock buildApiHandler
1583+
const { buildApiHandler } = require("../../../api")
15501584

15511585
// Mock SYSTEM_PROMPT
15521586
const systemPromptModule = require("../../prompts/system")
15531587
const systemPromptSpy = jest.spyOn(systemPromptModule, "SYSTEM_PROMPT")
15541588

1555-
// Test all combinations of model support and browserToolEnabled
1589+
// Mock getModeBySlug
1590+
const modesModule = require("../../../shared/modes")
1591+
1592+
// Test all combinations of model support, mode support, and browserToolEnabled
15561593
const testCases = [
1557-
{ modelSupports: true, settingEnabled: true, expected: true },
1558-
{ modelSupports: true, settingEnabled: false, expected: false },
1559-
{ modelSupports: false, settingEnabled: true, expected: false },
1560-
{ modelSupports: false, settingEnabled: false, expected: false },
1594+
{ modelSupports: true, modeSupports: true, settingEnabled: true, expected: true },
1595+
{ modelSupports: true, modeSupports: true, settingEnabled: false, expected: false },
1596+
{ modelSupports: true, modeSupports: false, settingEnabled: true, expected: false },
1597+
{ modelSupports: false, modeSupports: true, settingEnabled: true, expected: false },
1598+
{ modelSupports: false, modeSupports: false, settingEnabled: false, expected: false },
15611599
]
15621600

15631601
for (const testCase of testCases) {
15641602
// Reset mocks
15651603
systemPromptSpy.mockClear()
15661604

1567-
// Update mock Cline instance
1568-
mockCline.api.getModel = jest.fn().mockReturnValue({
1569-
id: "test-model",
1570-
info: { supportsComputerUse: testCase.modelSupports },
1605+
// Mock buildApiHandler to return appropriate model support
1606+
;(buildApiHandler as jest.Mock).mockImplementation(() => ({
1607+
getModel: jest.fn().mockReturnValue({
1608+
id: "test-model",
1609+
info: { supportsComputerUse: testCase.modelSupports },
1610+
}),
1611+
}))
1612+
1613+
// Mock getModeBySlug to return appropriate mode support
1614+
jest.spyOn(modesModule, "getModeBySlug").mockReturnValue({
1615+
slug: "test-mode",
1616+
name: "Test Mode",
1617+
roleDefinition: "Test role",
1618+
groups: testCase.modeSupports ? ["read", "browser"] : ["read"],
15711619
})
15721620

15731621
// Mock getState
@@ -1576,13 +1624,13 @@ describe("ClineProvider", () => {
15761624
apiProvider: "openrouter",
15771625
},
15781626
browserToolEnabled: testCase.settingEnabled,
1579-
mode: "code",
1627+
mode: "test-mode",
15801628
experiments: experimentDefault,
15811629
} as any)
15821630

15831631
// Trigger getSystemPrompt
15841632
const handler = getMessageHandler()
1585-
await handler({ type: "getSystemPrompt", mode: "code" })
1633+
await handler({ type: "getSystemPrompt", mode: "test-mode" })
15861634

15871635
// Verify SYSTEM_PROMPT was called
15881636
expect(systemPromptSpy).toHaveBeenCalled()

0 commit comments

Comments
 (0)