Skip to content

In-Editor Browser Improvements #1601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mrubens merged 7 commits into from
Mar 29, 2025
Merged

In-Editor Browser Improvements #1601

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5bec372
Browser Automation Improvements
afshawnlotfi Mar 12, 2025
aefd7d1
Merge branch 'main' into browser-improvements
afshawnlotfi Mar 12, 2025
75017a3
Updated for changes after merge
afshawnlotfi Mar 25, 2025
d5bb8c7
Merge branch 'main' of https://github.com/RooVetGit/Roo-Code into bro…
afshawnlotfi Mar 28, 2025
748c4b7
Added www. exception for common tabs
afshawnlotfi Mar 28, 2025
fede7c8
Update src/core/webview/ClineProvider.ts
mrubens Mar 29, 2025
8810d67
Revert README changes
mrubens Mar 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/core/Cline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2437,7 +2437,8 @@ export class Cline {
}
break
} else {
let browserActionResult: BrowserActionResult
// Initialize with empty object to avoid "used before assigned" errors
let browserActionResult: BrowserActionResult = {}
if (action === "launch") {
if (!url) {
this.consecutiveMistakeCount++
Expand Down Expand Up @@ -2523,9 +2524,9 @@ export class Cline {
pushToolResult(
formatResponse.toolResult(
`The browser action has been executed. The console logs and screenshot have been captured for your analysis.\n\nConsole logs:\n${
browserActionResult.logs || "(No new logs)"
browserActionResult?.logs || "(No new logs)"
}\n\n(REMEMBER: if you need to proceed to using non-\`browser_action\` tools or launch a new browser, you MUST first close this browser. For example, if after analyzing the logs and screenshot you need to edit a file, you must first close the browser before you can use the write_to_file tool.)`,
browserActionResult.screenshot ? [browserActionResult.screenshot] : [],
browserActionResult?.screenshot ? [browserActionResult.screenshot] : [],
),
)
break
Expand Down
89 changes: 20 additions & 69 deletions src/core/webview/ClineProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { McpHub } from "../../services/mcp/McpHub"
import { McpServerManager } from "../../services/mcp/McpServerManager"
import { ShadowCheckpointService } from "../../services/checkpoints/ShadowCheckpointService"
import { BrowserSession } from "../../services/browser/BrowserSession"
import { discoverChromeInstances } from "../../services/browser/browserDiscovery"
import { discoverChromeHostUrl, tryChromeHostUrl } from "../../services/browser/browserDiscovery"
import { fileExistsAtPath } from "../../utils/fs"
import { playSound, setSoundEnabled, setSoundVolume } from "../../utils/sound"
import { singleCompletionHandler } from "../../utils/single-completion-handler"
Expand Down Expand Up @@ -1279,74 +1279,17 @@ export class ClineProvider implements vscode.WebviewViewProvider {
await this.postStateToWebview()
break
case "testBrowserConnection":
try {
const browserSession = new BrowserSession(this.context)
// If no text is provided, try auto-discovery
if (!message.text) {
try {
const discoveredHost = await discoverChromeInstances()
if (discoveredHost) {
// Test the connection to the discovered host
const result = await browserSession.testConnection(discoveredHost)
// Send the result back to the webview
await this.postMessageToWebview({
type: "browserConnectionResult",
success: result.success,
text: `Auto-discovered and tested connection to Chrome at ${discoveredHost}: ${result.message}`,
values: { endpoint: result.endpoint },
})
} else {
await this.postMessageToWebview({
type: "browserConnectionResult",
success: false,
text: "No Chrome instances found on the network. Make sure Chrome is running with remote debugging enabled (--remote-debugging-port=9222).",
})
}
} catch (error) {
await this.postMessageToWebview({
type: "browserConnectionResult",
success: false,
text: `Error during auto-discovery: ${error instanceof Error ? error.message : String(error)}`,
})
}
} else {
// Test the provided URL
const result = await browserSession.testConnection(message.text)

// Send the result back to the webview
await this.postMessageToWebview({
type: "browserConnectionResult",
success: result.success,
text: result.message,
values: { endpoint: result.endpoint },
})
}
} catch (error) {
await this.postMessageToWebview({
type: "browserConnectionResult",
success: false,
text: `Error testing connection: ${error instanceof Error ? error.message : String(error)}`,
})
}
break
case "discoverBrowser":
try {
const discoveredHost = await discoverChromeInstances()

if (discoveredHost) {
// Don't update the remoteBrowserHost state when auto-discovering
// This way we don't override the user's preference

// Test the connection to get the endpoint
const browserSession = new BrowserSession(this.context)
const result = await browserSession.testConnection(discoveredHost)

// If no text is provided, try auto-discovery
if (!message.text) {
// Use testBrowserConnection for auto-discovery
const chromeHostUrl = await discoverChromeHostUrl()
if (chromeHostUrl) {
// Send the result back to the webview
await this.postMessageToWebview({
type: "browserConnectionResult",
success: true,
text: `Successfully discovered and connected to Chrome at ${discoveredHost}`,
values: { endpoint: result.endpoint },
success: !!chromeHostUrl,
text: `Auto-discovered and tested connection to Chrome: ${chromeHostUrl}`,
values: { endpoint: chromeHostUrl },
})
} else {
await this.postMessageToWebview({
Expand All @@ -1355,11 +1298,17 @@ export class ClineProvider implements vscode.WebviewViewProvider {
text: "No Chrome instances found on the network. Make sure Chrome is running with remote debugging enabled (--remote-debugging-port=9222).",
})
}
} catch (error) {
} else {
// Test the provided URL
const customHostUrl = message.text
const hostIsValid = await tryChromeHostUrl(message.text)
// Send the result back to the webview
await this.postMessageToWebview({
type: "browserConnectionResult",
success: false,
text: `Error discovering browser: ${error instanceof Error ? error.message : String(error)}`,
success: hostIsValid,
text: hostIsValid
? `Successfully connected to Chrome: ${customHostUrl}`
: "Failed to connect to Chrome",
})
}
break
Expand Down Expand Up @@ -2393,6 +2342,7 @@ export class ClineProvider implements vscode.WebviewViewProvider {
screenshotQuality: screenshotQuality ?? 75,
remoteBrowserHost,
remoteBrowserEnabled: remoteBrowserEnabled ?? false,
cachedChromeHostUrl: (await this.getGlobalState("cachedChromeHostUrl")) as string | undefined,
preferredLanguage: preferredLanguage ?? "English",
writeDelayMs: writeDelayMs ?? 1000,
terminalOutputLineLimit: terminalOutputLineLimit ?? 500,
Expand Down Expand Up @@ -2548,6 +2498,7 @@ export class ClineProvider implements vscode.WebviewViewProvider {
screenshotQuality: stateValues.screenshotQuality ?? 75,
remoteBrowserHost: stateValues.remoteBrowserHost,
remoteBrowserEnabled: stateValues.remoteBrowserEnabled ?? false,
cachedChromeHostUrl: stateValues.cachedChromeHostUrl as string | undefined,
fuzzyMatchThreshold: stateValues.fuzzyMatchThreshold ?? 1.0,
writeDelayMs: stateValues.writeDelayMs ?? 1000,
terminalOutputLineLimit: stateValues.terminalOutputLineLimit ?? 500,
Expand Down
82 changes: 7 additions & 75 deletions src/core/webview/__tests__/ClineProvider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,12 @@ jest.mock("../../../services/browser/BrowserSession", () => ({

// Mock browserDiscovery
jest.mock("../../../services/browser/browserDiscovery", () => ({
discoverChromeInstances: jest.fn().mockImplementation(async () => {
discoverChromeHostUrl: jest.fn().mockImplementation(async () => {
return "http://localhost:9222"
}),
tryChromeHostUrl: jest.fn().mockImplementation(async (url) => {
return url === "http://localhost:9222"
}),
}))
jest.mock(
"@modelcontextprotocol/sdk/types.js",
Expand Down Expand Up @@ -1867,9 +1870,9 @@ describe("ClineProvider", () => {
type: "testBrowserConnection",
})

// Verify discoverChromeInstances was called
const { discoverChromeInstances } = require("../../../services/browser/browserDiscovery")
expect(discoverChromeInstances).toHaveBeenCalled()
// Verify discoverChromeHostUrl was called
const { discoverChromeHostUrl } = require("../../../services/browser/browserDiscovery")
expect(discoverChromeHostUrl).toHaveBeenCalled()

// Verify postMessage was called with success result
expect(mockPostMessage).toHaveBeenCalledWith(
Expand All @@ -1880,77 +1883,6 @@ describe("ClineProvider", () => {
}),
)
})

test("handles discoverBrowser message", async () => {
// Get the message handler
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0]

// Test browser discovery
await messageHandler({
type: "discoverBrowser",
})

// Verify discoverChromeInstances was called
const { discoverChromeInstances } = require("../../../services/browser/browserDiscovery")
expect(discoverChromeInstances).toHaveBeenCalled()

// Verify postMessage was called with success result
expect(mockPostMessage).toHaveBeenCalledWith(
expect.objectContaining({
type: "browserConnectionResult",
success: true,
text: expect.stringContaining("Successfully discovered and connected to Chrome"),
}),
)
})

test("handles errors during browser discovery", async () => {
// Mock discoverChromeInstances to throw an error
const { discoverChromeInstances } = require("../../../services/browser/browserDiscovery")
discoverChromeInstances.mockImplementationOnce(() => {
throw new Error("Discovery error")
})

// Get the message handler
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0]

// Test browser discovery with error
await messageHandler({
type: "discoverBrowser",
})

// Verify postMessage was called with error result
expect(mockPostMessage).toHaveBeenCalledWith(
expect.objectContaining({
type: "browserConnectionResult",
success: false,
text: expect.stringContaining("Error discovering browser"),
}),
)
})

test("handles case when no browsers are discovered", async () => {
// Mock discoverChromeInstances to return null (no browsers found)
const { discoverChromeInstances } = require("../../../services/browser/browserDiscovery")
discoverChromeInstances.mockImplementationOnce(() => null)

// Get the message handler
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0]

// Test browser discovery with no browsers found
await messageHandler({
type: "discoverBrowser",
})

// Verify postMessage was called with failure result
expect(mockPostMessage).toHaveBeenCalledWith(
expect.objectContaining({
type: "browserConnectionResult",
success: false,
text: expect.stringContaining("No Chrome instances found"),
}),
)
})
})
})

Expand Down
Loading