Skip to content

Commit a9773b7

Browse files
cannuricannuri
committed
fix browser_action system prompt
1 parent 42287ad commit a9773b7

File tree

2 files changed

+238
-43
lines changed

2 files changed

+238
-43
lines changed

‎src/core/webview/ClineProvider.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1826,6 +1826,7 @@ export class ClineProvider implements vscode.WebviewViewProvider {
18261826
fuzzyMatchThreshold,
18271827
experiments,
18281828
enableMcpServerCreation,
1829+
browserToolEnabled,
18291830
} = await this.getState()
18301831

18311832
// Create diffStrategy based on current model and settings
@@ -1841,10 +1842,14 @@ export class ClineProvider implements vscode.WebviewViewProvider {
18411842

18421843
const rooIgnoreInstructions = this.getCurrentCline()?.rooIgnoreController?.getInstructions()
18431844

1845+
// Determine if browser tools can be used based on model support and user settings
1846+
const modelSupportsComputerUse = this.getCurrentCline()?.api.getModel().info.supportsComputerUse ?? false
1847+
const canUseBrowserTool = modelSupportsComputerUse && (browserToolEnabled ?? true)
1848+
18441849
const systemPrompt = await SYSTEM_PROMPT(
18451850
this.context,
18461851
cwd,
1847-
apiConfiguration.openRouterModelInfo?.supportsComputerUse ?? false,
1852+
canUseBrowserTool,
18481853
mcpEnabled ? this.mcpHub : undefined,
18491854
diffStrategy,
18501855
browserViewportSize ?? "900x600",

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

Lines changed: 232 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,6 +1160,17 @@ describe("ClineProvider", () => {
11601160
})
11611161

11621162
test("passes diffStrategy and diffEnabled to SYSTEM_PROMPT when previewing", async () => {
1163+
// Setup Cline instance with mocked api.getModel()
1164+
const { Cline } = require("../../Cline")
1165+
const mockCline = new Cline()
1166+
mockCline.api = {
1167+
getModel: jest.fn().mockReturnValue({
1168+
id: "claude-3-sonnet",
1169+
info: { supportsComputerUse: true },
1170+
}),
1171+
}
1172+
await provider.addClineToStack(mockCline)
1173+
11631174
// Mock getState to return experimentalDiffStrategy, diffEnabled and fuzzyMatchThreshold
11641175
jest.spyOn(provider, "getState").mockResolvedValue({
11651176
apiConfiguration: {
@@ -1176,6 +1187,7 @@ describe("ClineProvider", () => {
11761187
diffEnabled: true,
11771188
fuzzyMatchThreshold: 0.8,
11781189
experiments: experimentDefault,
1190+
browserToolEnabled: true,
11791191
} as any)
11801192

11811193
// Mock SYSTEM_PROMPT to verify diffStrategy and diffEnabled are passed
@@ -1186,34 +1198,37 @@ describe("ClineProvider", () => {
11861198
const handler = getMessageHandler()
11871199
await handler({ type: "getSystemPrompt", mode: "code" })
11881200

1189-
// Verify SYSTEM_PROMPT was called with correct arguments
1190-
expect(systemPromptSpy).toHaveBeenCalledWith(
1191-
expect.anything(), // context
1192-
expect.any(String), // cwd
1193-
true, // supportsComputerUse
1194-
undefined, // mcpHub (disabled)
1195-
expect.objectContaining({
1196-
// diffStrategy
1197-
getToolDescription: expect.any(Function),
1198-
}),
1199-
"900x600", // browserViewportSize
1200-
"code", // mode
1201-
{}, // customModePrompts
1202-
{ customModes: [] }, // customModes
1203-
undefined, // effectiveInstructions
1204-
undefined, // preferredLanguage
1205-
true, // diffEnabled
1206-
experimentDefault,
1207-
true,
1208-
undefined, // rooIgnoreInstructions
1209-
)
1201+
// Verify SYSTEM_PROMPT was called
1202+
expect(systemPromptSpy).toHaveBeenCalled()
1203+
1204+
// Get the actual arguments passed to SYSTEM_PROMPT
1205+
const callArgs = systemPromptSpy.mock.calls[0]
1206+
1207+
// Verify key parameters
1208+
expect(callArgs[2]).toBe(true) // supportsComputerUse
1209+
expect(callArgs[3]).toBeUndefined() // mcpHub (disabled)
1210+
expect(callArgs[4]).toHaveProperty("getToolDescription") // diffStrategy
1211+
expect(callArgs[5]).toBe("900x600") // browserViewportSize
1212+
expect(callArgs[6]).toBe("code") // mode
1213+
expect(callArgs[11]).toBe(true) // diffEnabled
12101214

12111215
// Run the test again to verify it's consistent
12121216
await handler({ type: "getSystemPrompt", mode: "code" })
12131217
expect(systemPromptSpy).toHaveBeenCalledTimes(2)
12141218
})
12151219

12161220
test("passes diffEnabled: false to SYSTEM_PROMPT when diff is disabled", async () => {
1221+
// Setup Cline instance with mocked api.getModel()
1222+
const { Cline } = require("../../Cline")
1223+
const mockCline = new Cline()
1224+
mockCline.api = {
1225+
getModel: jest.fn().mockReturnValue({
1226+
id: "claude-3-sonnet",
1227+
info: { supportsComputerUse: true },
1228+
}),
1229+
}
1230+
await provider.addClineToStack(mockCline)
1231+
12171232
// Mock getState to return diffEnabled: false
12181233
jest.spyOn(provider, "getState").mockResolvedValue({
12191234
apiConfiguration: {
@@ -1230,6 +1245,7 @@ describe("ClineProvider", () => {
12301245
fuzzyMatchThreshold: 0.8,
12311246
experiments: experimentDefault,
12321247
enableMcpServerCreation: true,
1248+
browserToolEnabled: true,
12331249
} as any)
12341250

12351251
// Mock SYSTEM_PROMPT to verify diffEnabled is passed as false
@@ -1240,27 +1256,19 @@ describe("ClineProvider", () => {
12401256
const handler = getMessageHandler()
12411257
await handler({ type: "getSystemPrompt", mode: "code" })
12421258

1243-
// Verify SYSTEM_PROMPT was called with diffEnabled: false
1244-
expect(systemPromptSpy).toHaveBeenCalledWith(
1245-
expect.anything(), // context
1246-
expect.any(String), // cwd
1247-
true, // supportsComputerUse
1248-
undefined, // mcpHub (disabled)
1249-
expect.objectContaining({
1250-
// diffStrategy
1251-
getToolDescription: expect.any(Function),
1252-
}),
1253-
"900x600", // browserViewportSize
1254-
"code", // mode
1255-
{}, // customModePrompts
1256-
{ customModes: [] }, // customModes
1257-
undefined, // effectiveInstructions
1258-
undefined, // preferredLanguage
1259-
false, // diffEnabled
1260-
experimentDefault,
1261-
true,
1262-
undefined, // rooIgnoreInstructions
1263-
)
1259+
// Verify SYSTEM_PROMPT was called
1260+
expect(systemPromptSpy).toHaveBeenCalled()
1261+
1262+
// Get the actual arguments passed to SYSTEM_PROMPT
1263+
const callArgs = systemPromptSpy.mock.calls[0]
1264+
1265+
// Verify key parameters
1266+
expect(callArgs[2]).toBe(true) // supportsComputerUse
1267+
expect(callArgs[3]).toBeUndefined() // mcpHub (disabled)
1268+
expect(callArgs[4]).toHaveProperty("getToolDescription") // diffStrategy
1269+
expect(callArgs[5]).toBe("900x600") // browserViewportSize
1270+
expect(callArgs[6]).toBe("code") // mode
1271+
expect(callArgs[11]).toBe(false) // diffEnabled should be false
12641272
})
12651273

12661274
test("uses correct mode-specific instructions when mode is specified", async () => {
@@ -1299,6 +1307,188 @@ describe("ClineProvider", () => {
12991307
expect.any(String),
13001308
)
13011309
})
1310+
1311+
// Tests for browser tool support
1312+
test("correctly extracts modelSupportsComputerUse from Cline instance", async () => {
1313+
// Setup Cline instance with mocked api.getModel()
1314+
const { Cline } = require("../../Cline")
1315+
const mockCline = new Cline()
1316+
mockCline.api = {
1317+
getModel: jest.fn().mockReturnValue({
1318+
id: "claude-3-sonnet",
1319+
info: { supportsComputerUse: true },
1320+
}),
1321+
}
1322+
await provider.addClineToStack(mockCline)
1323+
1324+
// Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly
1325+
const systemPromptModule = require("../../prompts/system")
1326+
const systemPromptSpy = jest.spyOn(systemPromptModule, "SYSTEM_PROMPT")
1327+
1328+
// Mock getState to return browserToolEnabled: true
1329+
jest.spyOn(provider, "getState").mockResolvedValue({
1330+
apiConfiguration: {
1331+
apiProvider: "openrouter",
1332+
},
1333+
browserToolEnabled: true,
1334+
mode: "code",
1335+
experiments: experimentDefault,
1336+
} as any)
1337+
1338+
// Trigger getSystemPrompt
1339+
const handler = getMessageHandler()
1340+
await handler({ type: "getSystemPrompt", mode: "code" })
1341+
1342+
// Verify SYSTEM_PROMPT was called
1343+
expect(systemPromptSpy).toHaveBeenCalled()
1344+
1345+
// Get the actual arguments passed to SYSTEM_PROMPT
1346+
const callArgs = systemPromptSpy.mock.calls[0]
1347+
1348+
// Verify the supportsComputerUse parameter (3rd parameter, index 2)
1349+
expect(callArgs[2]).toBe(true)
1350+
})
1351+
1352+
test("correctly handles when model doesn't support computer use", async () => {
1353+
// Setup Cline instance with mocked api.getModel() that doesn't support computer use
1354+
const { Cline } = require("../../Cline")
1355+
const mockCline = new Cline()
1356+
mockCline.api = {
1357+
getModel: jest.fn().mockReturnValue({
1358+
id: "non-computer-use-model",
1359+
info: { supportsComputerUse: false },
1360+
}),
1361+
}
1362+
await provider.addClineToStack(mockCline)
1363+
1364+
// Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly
1365+
const systemPromptModule = require("../../prompts/system")
1366+
const systemPromptSpy = jest.spyOn(systemPromptModule, "SYSTEM_PROMPT")
1367+
1368+
// Mock getState to return browserToolEnabled: true
1369+
jest.spyOn(provider, "getState").mockResolvedValue({
1370+
apiConfiguration: {
1371+
apiProvider: "openrouter",
1372+
},
1373+
browserToolEnabled: true,
1374+
mode: "code",
1375+
experiments: experimentDefault,
1376+
} as any)
1377+
1378+
// Trigger getSystemPrompt
1379+
const handler = getMessageHandler()
1380+
await handler({ type: "getSystemPrompt", mode: "code" })
1381+
1382+
// Verify SYSTEM_PROMPT was called
1383+
expect(systemPromptSpy).toHaveBeenCalled()
1384+
1385+
// Get the actual arguments passed to SYSTEM_PROMPT
1386+
const callArgs = systemPromptSpy.mock.calls[0]
1387+
1388+
// Verify the supportsComputerUse parameter (3rd parameter, index 2)
1389+
// Even though browserToolEnabled is true, the model doesn't support it
1390+
expect(callArgs[2]).toBe(false)
1391+
})
1392+
1393+
test("correctly handles when browserToolEnabled is false", async () => {
1394+
// Setup Cline instance with mocked api.getModel() that supports computer use
1395+
const { Cline } = require("../../Cline")
1396+
const mockCline = new Cline()
1397+
mockCline.api = {
1398+
getModel: jest.fn().mockReturnValue({
1399+
id: "claude-3-sonnet",
1400+
info: { supportsComputerUse: true },
1401+
}),
1402+
}
1403+
await provider.addClineToStack(mockCline)
1404+
1405+
// Mock SYSTEM_PROMPT to verify supportsComputerUse is passed correctly
1406+
const systemPromptModule = require("../../prompts/system")
1407+
const systemPromptSpy = jest.spyOn(systemPromptModule, "SYSTEM_PROMPT")
1408+
1409+
// Mock getState to return browserToolEnabled: false
1410+
jest.spyOn(provider, "getState").mockResolvedValue({
1411+
apiConfiguration: {
1412+
apiProvider: "openrouter",
1413+
},
1414+
browserToolEnabled: false,
1415+
mode: "code",
1416+
experiments: experimentDefault,
1417+
} as any)
1418+
1419+
// Trigger getSystemPrompt
1420+
const handler = getMessageHandler()
1421+
await handler({ type: "getSystemPrompt", mode: "code" })
1422+
1423+
// Verify SYSTEM_PROMPT was called
1424+
expect(systemPromptSpy).toHaveBeenCalled()
1425+
1426+
// Get the actual arguments passed to SYSTEM_PROMPT
1427+
const callArgs = systemPromptSpy.mock.calls[0]
1428+
1429+
// Verify the supportsComputerUse parameter (3rd parameter, index 2)
1430+
// Even though model supports it, browserToolEnabled is false
1431+
expect(callArgs[2]).toBe(false)
1432+
})
1433+
1434+
test("correctly calculates canUseBrowserTool as combination of model support and setting", async () => {
1435+
// Setup Cline instance with mocked api.getModel()
1436+
const { Cline } = require("../../Cline")
1437+
const mockCline = new Cline()
1438+
mockCline.api = {
1439+
getModel: jest.fn().mockReturnValue({
1440+
id: "claude-3-sonnet",
1441+
info: { supportsComputerUse: true },
1442+
}),
1443+
}
1444+
await provider.addClineToStack(mockCline)
1445+
1446+
// Mock SYSTEM_PROMPT
1447+
const systemPromptModule = require("../../prompts/system")
1448+
const systemPromptSpy = jest.spyOn(systemPromptModule, "SYSTEM_PROMPT")
1449+
1450+
// Test all combinations of model support and browserToolEnabled
1451+
const testCases = [
1452+
{ modelSupports: true, settingEnabled: true, expected: true },
1453+
{ modelSupports: true, settingEnabled: false, expected: false },
1454+
{ modelSupports: false, settingEnabled: true, expected: false },
1455+
{ modelSupports: false, settingEnabled: false, expected: false },
1456+
]
1457+
1458+
for (const testCase of testCases) {
1459+
// Reset mocks
1460+
systemPromptSpy.mockClear()
1461+
1462+
// Update mock Cline instance
1463+
mockCline.api.getModel = jest.fn().mockReturnValue({
1464+
id: "test-model",
1465+
info: { supportsComputerUse: testCase.modelSupports },
1466+
})
1467+
1468+
// Mock getState
1469+
jest.spyOn(provider, "getState").mockResolvedValue({
1470+
apiConfiguration: {
1471+
apiProvider: "openrouter",
1472+
},
1473+
browserToolEnabled: testCase.settingEnabled,
1474+
mode: "code",
1475+
experiments: experimentDefault,
1476+
} as any)
1477+
1478+
// Trigger getSystemPrompt
1479+
const handler = getMessageHandler()
1480+
await handler({ type: "getSystemPrompt", mode: "code" })
1481+
1482+
// Verify SYSTEM_PROMPT was called
1483+
expect(systemPromptSpy).toHaveBeenCalled()
1484+
1485+
// Get the actual arguments passed to SYSTEM_PROMPT
1486+
const callArgs = systemPromptSpy.mock.calls[0]
1487+
1488+
// Verify the supportsComputerUse parameter (3rd parameter, index 2)
1489+
expect(callArgs[2]).toBe(testCase.expected)
1490+
}
1491+
})
13021492
})
13031493

13041494
describe("handleModeSwitch", () => {

0 commit comments

Comments
 (0)