Skip to content

Commit 20bdf53

Browse files
hannesrudolphhannesrudolph
committed
refactor: Restore browser action title in all locales and update component to use it
1 parent 8a289d9 commit 20bdf53

File tree

21 files changed

+188
-344
lines changed

21 files changed

+188
-344
lines changed

‎src/core/environment/__tests__/getEnvironmentDetails.spec.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ describe("getEnvironmentDetails", () => {
120120
} as unknown as WeakRef<ClineProvider>,
121121
browserSession: {
122122
isSessionActive: vi.fn().mockReturnValue(false),
123+
getViewportSize: vi.fn().mockReturnValue({ width: 900, height: 600 }),
123124
} as any,
124125
}
125126

@@ -459,10 +460,9 @@ describe("getEnvironmentDetails", () => {
459460
expect(getGitStatus).toHaveBeenCalledWith(mockCwd, 5)
460461
})
461462

462-
it("should include Browser Session Status when inactive", async () => {
463+
it("should NOT include Browser Session Status when inactive", async () => {
463464
const result = await getEnvironmentDetails(mockCline as Task)
464-
expect(result).toContain("# Browser Session Status")
465-
expect(result).toContain("Inactive - Browser is not launched")
465+
expect(result).not.toContain("# Browser Session Status")
466466
})
467467

468468
it("should include Browser Session Status with current viewport when active", async () => {

‎src/core/environment/getEnvironmentDetails.ts‎

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -248,37 +248,34 @@ export async function getEnvironmentDetails(cline: Task, includeFileDetails: boo
248248
}
249249
}
250250

251-
// Add browser session status - Always show to prevent LLM from trying browser actions when no session is active
251+
// Add browser session status - Only show when active to prevent cluttering context
252252
const isBrowserActive = cline.browserSession.isSessionActive()
253253

254-
// Build viewport info for status (prefer actual viewport if available, else fallback to configured setting)
255-
const configuredViewport = (state?.browserViewportSize as string | undefined) ?? "900x600"
256-
let configuredWidth: number | undefined
257-
let configuredHeight: number | undefined
258-
if (configuredViewport.includes("x")) {
259-
const parts = configuredViewport.split("x").map((v) => Number(v))
260-
configuredWidth = parts[0]
261-
configuredHeight = parts[1]
262-
}
254+
if (isBrowserActive) {
255+
// Build viewport info for status (prefer actual viewport if available, else fallback to configured setting)
256+
const configuredViewport = (state?.browserViewportSize as string | undefined) ?? "900x600"
257+
let configuredWidth: number | undefined
258+
let configuredHeight: number | undefined
259+
if (configuredViewport.includes("x")) {
260+
const parts = configuredViewport.split("x").map((v) => Number(v))
261+
configuredWidth = parts[0]
262+
configuredHeight = parts[1]
263+
}
263264

264-
let actualWidth: number | undefined
265-
let actualHeight: number | undefined
266-
// Use optional chaining to avoid issues with tests that stub browserSession
267-
const vp = isBrowserActive ? (cline.browserSession as any).getViewportSize?.() : undefined
268-
if (vp) {
269-
actualWidth = vp.width
270-
actualHeight = vp.height
271-
}
265+
let actualWidth: number | undefined
266+
let actualHeight: number | undefined
267+
const vp = cline.browserSession.getViewportSize?.()
268+
if (vp) {
269+
actualWidth = vp.width
270+
actualHeight = vp.height
271+
}
272272

273-
const width = actualWidth ?? configuredWidth
274-
const height = actualHeight ?? configuredHeight
275-
const viewportInfo = isBrowserActive && width && height ? `\nCurrent viewport size: ${width}x${height} pixels.` : ""
273+
const width = actualWidth ?? configuredWidth
274+
const height = actualHeight ?? configuredHeight
275+
const viewportInfo = width && height ? `\nCurrent viewport size: ${width}x${height} pixels.` : ""
276276

277-
details += `\n# Browser Session Status\n${
278-
isBrowserActive
279-
? "Active - A browser session is currently open and ready for browser_action commands"
280-
: "Inactive - Browser is not launched. Using any browser action except the browser_action with action='launch' to start a new session will result in an error."
281-
}${viewportInfo}\n`
277+
details += `\n# Browser Session Status\nActive - A browser session is currently open and ready for browser_action commands${viewportInfo}\n`
278+
}
282279

283280
if (includeFileDetails) {
284281
details += `\n\n# Current Workspace Directory (${cline.cwd.toPosix()}) Files\n`

‎src/core/tools/BrowserActionTool.ts‎

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,39 +8,7 @@ import {
88
} from "../../shared/ExtensionMessage"
99
import { formatResponse } from "../prompts/responses"
1010
import { Anthropic } from "@anthropic-ai/sdk"
11-
12-
/**
13-
* Parses coordinate string and scales from image dimensions to viewport dimensions
14-
* The LLM examines the screenshot it receives (which may be downscaled by the API)
15-
* and reports coordinates in format: "x,y@widthxheight" where widthxheight is what the LLM observed
16-
*
17-
* Format: "x,y@widthxheight" (required)
18-
* Returns: scaled coordinate string "x,y" in viewport coordinates
19-
* Throws: Error if format is invalid or missing image dimensions
20-
*/
21-
function scaleCoordinate(coordinate: string, viewportWidth: number, viewportHeight: number): string {
22-
// Parse coordinate with required image dimensions (accepts both 'x' and ',' as dimension separators)
23-
const match = coordinate.match(/^\s*(\d+)\s*,\s*(\d+)\s*@\s*(\d+)\s*[x,]\s*(\d+)\s*$/)
24-
25-
if (!match) {
26-
throw new Error(
27-
`Invalid coordinate format: "${coordinate}". ` +
28-
`Expected format: "x,y@widthxheight" (e.g., "450,300@1024x768")`,
29-
)
30-
}
31-
32-
const [, xStr, yStr, imgWidthStr, imgHeightStr] = match
33-
const x = parseInt(xStr, 10)
34-
const y = parseInt(yStr, 10)
35-
const imgWidth = parseInt(imgWidthStr, 10)
36-
const imgHeight = parseInt(imgHeightStr, 10)
37-
38-
// Scale coordinates from image dimensions to viewport dimensions
39-
const scaledX = Math.round((x / imgWidth) * viewportWidth)
40-
const scaledY = Math.round((y / imgHeight) * viewportHeight)
41-
42-
return `${scaledX},${scaledY}`
43-
}
11+
import { scaleCoordinate } from "../../shared/browserUtils"
4412

4513
export async function browserActionTool(
4614
cline: Task,

‎src/core/tools/__tests__/BrowserActionTool.coordinateScaling.spec.ts‎

Lines changed: 5 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
// Test coordinate scaling functionality in browser actions
2-
import { describe, it, expect, vi, beforeEach } from "vitest"
3-
4-
// Mock the scaleCoordinate function by extracting it
5-
// In a real scenario, we'd export it or test through the main function
6-
// For now, we'll test the regex pattern and logic
2+
import { describe, it, expect } from "vitest"
3+
import { scaleCoordinate } from "../../../shared/browserUtils"
74

85
describe("Browser Action Coordinate Scaling", () => {
96
describe("Coordinate format validation", () => {
@@ -18,10 +15,9 @@ describe("Browser Action Coordinate Scaling", () => {
1815
"450,300@1024,768", // comma separator for dimensions
1916
]
2017

21-
const regex = /^\s*(\d+)\s*,\s*(\d+)\s*@\s*(\d+)\s*[x,]\s*(\d+)\s*$/
22-
2318
validFormats.forEach((coord) => {
24-
expect(coord).toMatch(regex)
19+
// Should not throw
20+
expect(() => scaleCoordinate(coord, 900, 600)).not.toThrow()
2521
})
2622
})
2723

@@ -39,35 +35,14 @@ describe("Browser Action Coordinate Scaling", () => {
3935
"450,300@axb", // non-numeric dimensions
4036
]
4137

42-
const regex = /^\s*(\d+)\s*,\s*(\d+)\s*@\s*(\d+)\s*[x,]\s*(\d+)\s*$/
43-
4438
invalidFormats.forEach((coord) => {
45-
expect(coord).not.toMatch(regex)
39+
expect(() => scaleCoordinate(coord, 900, 600)).toThrow()
4640
})
4741
})
4842
})
4943

5044
describe("Coordinate scaling logic", () => {
5145
it("should correctly scale coordinates from image to viewport", () => {
52-
// Simulate the scaling logic
53-
const scaleCoordinate = (coordinate: string, viewportWidth: number, viewportHeight: number): string => {
54-
const match = coordinate.match(/^\s*(\d+)\s*,\s*(\d+)\s*@\s*(\d+)\s*[x,]\s*(\d+)\s*$/)
55-
if (!match) {
56-
throw new Error(`Invalid coordinate format: "${coordinate}"`)
57-
}
58-
59-
const [, xStr, yStr, imgWidthStr, imgHeightStr] = match
60-
const x = parseInt(xStr, 10)
61-
const y = parseInt(yStr, 10)
62-
const imgWidth = parseInt(imgWidthStr, 10)
63-
const imgHeight = parseInt(imgHeightStr, 10)
64-
65-
const scaledX = Math.round((x / imgWidth) * viewportWidth)
66-
const scaledY = Math.round((y / imgHeight) * viewportHeight)
67-
68-
return `${scaledX},${scaledY}`
69-
}
70-
7146
// Test case 1: Same dimensions (no scaling)
7247
expect(scaleCoordinate("450,300@900x600", 900, 600)).toBe("450,300")
7348

@@ -88,52 +63,13 @@ describe("Browser Action Coordinate Scaling", () => {
8863
})
8964

9065
it("should throw error for invalid coordinate format", () => {
91-
const scaleCoordinate = (coordinate: string, viewportWidth: number, viewportHeight: number): string => {
92-
const match = coordinate.match(/^\s*(\d+)\s*,\s*(\d+)\s*@\s*(\d+)\s*[x,]\s*(\d+)\s*$/)
93-
if (!match) {
94-
throw new Error(
95-
`Invalid coordinate format: "${coordinate}". ` +
96-
`Expected format: "x,y@widthxheight" (e.g., "450,300@1024x768")`,
97-
)
98-
}
99-
100-
const [, xStr, yStr, imgWidthStr, imgHeightStr] = match
101-
const x = parseInt(xStr, 10)
102-
const y = parseInt(yStr, 10)
103-
const imgWidth = parseInt(imgWidthStr, 10)
104-
const imgHeight = parseInt(imgHeightStr, 10)
105-
106-
const scaledX = Math.round((x / imgWidth) * viewportWidth)
107-
const scaledY = Math.round((y / imgHeight) * viewportHeight)
108-
109-
return `${scaledX},${scaledY}`
110-
}
111-
11266
// Test invalid formats
11367
expect(() => scaleCoordinate("450,300", 900, 600)).toThrow("Invalid coordinate format")
11468
expect(() => scaleCoordinate("450,300@1024", 900, 600)).toThrow("Invalid coordinate format")
11569
expect(() => scaleCoordinate("invalid", 900, 600)).toThrow("Invalid coordinate format")
11670
})
11771

11872
it("should handle rounding correctly", () => {
119-
const scaleCoordinate = (coordinate: string, viewportWidth: number, viewportHeight: number): string => {
120-
const match = coordinate.match(/^\s*(\d+)\s*,\s*(\d+)\s*@\s*(\d+)\s*[x,]\s*(\d+)\s*$/)
121-
if (!match) {
122-
throw new Error(`Invalid coordinate format: "${coordinate}"`)
123-
}
124-
125-
const [, xStr, yStr, imgWidthStr, imgHeightStr] = match
126-
const x = parseInt(xStr, 10)
127-
const y = parseInt(yStr, 10)
128-
const imgWidth = parseInt(imgWidthStr, 10)
129-
const imgHeight = parseInt(imgHeightStr, 10)
130-
131-
const scaledX = Math.round((x / imgWidth) * viewportWidth)
132-
const scaledY = Math.round((y / imgHeight) * viewportHeight)
133-
134-
return `${scaledX},${scaledY}`
135-
}
136-
13773
// Test rounding behavior
13874
// 333 / 1000 * 900 = 299.7 -> rounds to 300
13975
expect(scaleCoordinate("333,333@1000x1000", 900, 900)).toBe("300,300")

‎src/core/webview/ClineProvider.ts‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1925,6 +1925,7 @@ export class ClineProvider
19251925
openRouterImageGenerationSelectedModel,
19261926
openRouterUseMiddleOutTransform,
19271927
featureRoomoteControlEnabled,
1928+
isBrowserSessionActive,
19281929
} = await this.getState()
19291930

19301931
let cloudOrganizations: CloudOrganizationMembership[] = []
@@ -1974,7 +1975,7 @@ export class ClineProvider
19741975
alwaysAllowModeSwitch: alwaysAllowModeSwitch ?? false,
19751976
alwaysAllowSubtasks: alwaysAllowSubtasks ?? false,
19761977
alwaysAllowUpdateTodoList: alwaysAllowUpdateTodoList ?? false,
1977-
isBrowserSessionActive: this.getCurrentTask()?.browserSession?.isSessionActive() ?? false,
1978+
isBrowserSessionActive,
19781979
allowedMaxRequests,
19791980
allowedMaxCost,
19801981
autoCondenseContext: autoCondenseContext ?? true,
@@ -2125,10 +2126,13 @@ export class ClineProvider
21252126
providerSettings.apiProvider = apiProvider
21262127
}
21272128

2129+
const cloudService = CloudService.hasInstance() ? CloudService.instance : undefined
21282130
let organizationAllowList = ORGANIZATION_ALLOW_ALL
21292131

21302132
try {
2131-
organizationAllowList = await CloudService.instance.getAllowList()
2133+
if (cloudService) {
2134+
organizationAllowList = await cloudService.getAllowList()
2135+
}
21322136
} catch (error) {
21332137
console.error(
21342138
`[getState] failed to get organization allow list: ${error instanceof Error ? error.message : String(error)}`,
@@ -2138,7 +2142,9 @@ export class ClineProvider
21382142
let cloudUserInfo: CloudUserInfo | null = null
21392143

21402144
try {
2141-
cloudUserInfo = CloudService.instance.getUserInfo()
2145+
if (cloudService) {
2146+
cloudUserInfo = cloudService.getUserInfo()
2147+
}
21422148
} catch (error) {
21432149
console.error(
21442150
`[getState] failed to get cloud user info: ${error instanceof Error ? error.message : String(error)}`,
@@ -2148,7 +2154,9 @@ export class ClineProvider
21482154
let cloudIsAuthenticated: boolean = false
21492155

21502156
try {
2151-
cloudIsAuthenticated = CloudService.instance.isAuthenticated()
2157+
if (cloudService) {
2158+
cloudIsAuthenticated = cloudService.isAuthenticated()
2159+
}
21522160
} catch (error) {
21532161
console.error(
21542162
`[getState] failed to get cloud authentication state: ${error instanceof Error ? error.message : String(error)}`,
@@ -2158,7 +2166,9 @@ export class ClineProvider
21582166
let sharingEnabled: boolean = false
21592167

21602168
try {
2161-
sharingEnabled = await CloudService.instance.canShareTask()
2169+
if (cloudService) {
2170+
sharingEnabled = await cloudService.canShareTask()
2171+
}
21622172
} catch (error) {
21632173
console.error(
21642174
`[getState] failed to get sharing enabled state: ${error instanceof Error ? error.message : String(error)}`,
@@ -2168,8 +2178,8 @@ export class ClineProvider
21682178
let organizationSettingsVersion: number = -1
21692179

21702180
try {
2171-
if (CloudService.hasInstance()) {
2172-
const settings = CloudService.instance.getOrganizationSettings()
2181+
if (cloudService) {
2182+
const settings = cloudService.getOrganizationSettings()
21732183
organizationSettingsVersion = settings?.version ?? -1
21742184
}
21752185
} catch (error) {
@@ -2181,7 +2191,9 @@ export class ClineProvider
21812191
let taskSyncEnabled: boolean = false
21822192

21832193
try {
2184-
taskSyncEnabled = CloudService.instance.isTaskSyncEnabled()
2194+
if (cloudService) {
2195+
taskSyncEnabled = cloudService.isTaskSyncEnabled()
2196+
}
21852197
} catch (error) {
21862198
console.error(
21872199
`[getState] failed to get task sync enabled state: ${error instanceof Error ? error.message : String(error)}`,

‎src/core/webview/webviewMessageHandler.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2139,8 +2139,7 @@ export const webviewMessageHandler = async (
21392139
provider.postMessageToWebview({
21402140
type: "importModeResult",
21412141
success: true,
2142-
// Cast to any to support older ImportResult types that may not declare slug
2143-
slug: (result as any)?.slug,
2142+
slug: result.slug,
21442143
})
21452144

21462145
// Show success message

0 commit comments

Comments
 (0)