Skip to content

Commit d576c30

Browse files
committed
browser(session): enforce strict single-tab across domains; ui(chat): localize step counter and fix cost chip visibility; tests: add single-tab test and update snapshots
1 parent db4c5f8 commit d576c30

File tree

4 files changed

+100
-57
lines changed

4 files changed

+100
-57
lines changed

src/core/prompts/__tests__/__snapshots__/system-prompt/with-computer-use-support.snap

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/services/browser/BrowserSession.ts

Lines changed: 35 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -354,70 +354,51 @@ export class BrowserSession {
354354
if (!this.browser) {
355355
throw new Error("Browser is not launched")
356356
}
357-
// Remove trailing slash for comparison
357+
// Normalize for comparison
358358
const normalizedNewUrl = url.replace(/\/$/, "")
359359

360-
// Extract the root domain from the URL
361-
const rootDomain = this.getRootDomain(normalizedNewUrl)
362-
363-
// Get all current pages
360+
// Select a single target page to enforce strict single-tab behavior
364361
const pages = await this.browser.pages()
362+
let targetPage: Page | undefined = this.page ?? pages[0]
365363

366-
// Try to find a page with the same root domain
367-
let existingPage: Page | undefined
364+
// If no page exists yet, create one directly
365+
if (!targetPage) {
366+
return this.createNewTab(normalizedNewUrl)
367+
}
368368

369-
for (const page of pages) {
370-
try {
371-
const pageUrl = page.url()
372-
if (pageUrl && this.getRootDomain(pageUrl) === rootDomain) {
373-
existingPage = page
374-
break
369+
// Close all other pages to keep exactly one tab
370+
for (const p of pages) {
371+
if (p !== targetPage) {
372+
try {
373+
await p.close().catch(() => {})
374+
} catch {
375+
// best-effort
375376
}
376-
} catch (error) {
377-
// Skip pages that might have been closed or have errors
378-
console.log(`Error checking page URL: ${error}`)
379-
continue
380377
}
381378
}
382379

383-
if (existingPage) {
384-
// Tab with the same root domain exists, switch to it
385-
console.log(`Tab with domain ${rootDomain} already exists, switching to it`)
386-
387-
// Update the active page
388-
this.page = existingPage
389-
existingPage.bringToFront()
390-
391-
// Navigate to the new URL if it's different]
392-
const currentUrl = existingPage.url().replace(/\/$/, "") // Remove trailing / if present
393-
if (this.getRootDomain(currentUrl) === rootDomain && currentUrl !== normalizedNewUrl) {
394-
console.log(`Navigating to new URL: ${normalizedNewUrl}`)
395-
console.log(`Current URL: ${currentUrl}`)
396-
console.log(`Root domain: ${this.getRootDomain(currentUrl)}`)
397-
console.log(`New URL: ${normalizedNewUrl}`)
398-
// Navigate to the new URL
399-
return this.doAction(async (page) => {
400-
await this.navigatePageToUrl(page, normalizedNewUrl)
401-
})
402-
} else {
403-
console.log(`Tab with domain ${rootDomain} already exists, and URL is the same: ${normalizedNewUrl}`)
404-
// URL is the same, just reload the page to ensure it's up to date
405-
console.log(`Reloading page: ${normalizedNewUrl}`)
406-
console.log(`Current URL: ${currentUrl}`)
407-
console.log(`Root domain: ${this.getRootDomain(currentUrl)}`)
408-
console.log(`New URL: ${normalizedNewUrl}`)
409-
return this.doAction(async (page) => {
410-
await page.reload({
411-
timeout: BROWSER_NAVIGATION_TIMEOUT,
412-
waitUntil: ["domcontentloaded", "networkidle2"],
413-
})
414-
await this.waitTillHTMLStable(page)
415-
})
416-
}
380+
// Use the selected page
381+
this.page = targetPage
382+
try {
383+
targetPage.bringToFront()
384+
} catch {
385+
// no-op if bringToFront fails
386+
}
387+
388+
// Navigate if URL changed, otherwise reload to ensure freshness
389+
const currentUrl = (targetPage.url?.() || "").replace(/\/$/, "")
390+
if (currentUrl !== normalizedNewUrl) {
391+
return this.doAction(async (page) => {
392+
await this.navigatePageToUrl(page, normalizedNewUrl)
393+
})
417394
} else {
418-
// No tab with this root domain exists, create a new one
419-
console.log(`No tab with domain ${rootDomain} exists, creating a new one`)
420-
return this.createNewTab(normalizedNewUrl)
395+
return this.doAction(async (page) => {
396+
await page.reload({
397+
timeout: BROWSER_NAVIGATION_TIMEOUT,
398+
waitUntil: ["domcontentloaded", "networkidle2"],
399+
})
400+
await this.waitTillHTMLStable(page)
401+
})
421402
}
422403
}
423404

src/services/browser/__tests__/BrowserSession.spec.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,66 @@ describe("BrowserSession", () => {
257257
expect(forceSpy).toHaveBeenCalledWith(page)
258258
expect(page.mouse.click).toHaveBeenCalledWith(10, 20)
259259
})
260+
261+
it("reuses a single page across domains and closes others", async () => {
262+
const mockCtx: any = {
263+
globalState: { get: vi.fn(), update: vi.fn() },
264+
globalStorageUri: { fsPath: "/mock/global/storage/path" },
265+
extensionUri: { fsPath: "/mock/extension/path" },
266+
}
267+
const session = new BrowserSession(mockCtx)
268+
269+
// Create two pages
270+
const page1: any = {
271+
on: vi.fn(),
272+
off: vi.fn(),
273+
screenshot: vi.fn().mockResolvedValue("mockScreenshotBase64"),
274+
url: vi.fn().mockReturnValue("https://a.com"),
275+
waitForNavigation: vi.fn().mockResolvedValue(undefined),
276+
evaluate: vi.fn().mockResolvedValue(undefined),
277+
goto: vi.fn().mockResolvedValue(undefined),
278+
reload: vi.fn().mockResolvedValue(undefined),
279+
bringToFront: vi.fn().mockResolvedValue(undefined),
280+
close: vi.fn().mockResolvedValue(undefined),
281+
content: vi.fn().mockResolvedValue("<html></html>"),
282+
keyboard: { press: vi.fn(), type: vi.fn() },
283+
mouse: { click: vi.fn(), move: vi.fn() },
284+
}
285+
const page2: any = {
286+
on: vi.fn(),
287+
off: vi.fn(),
288+
screenshot: vi.fn().mockResolvedValue("mockScreenshotBase64"),
289+
url: vi.fn().mockReturnValue("https://b.com"),
290+
waitForNavigation: vi.fn().mockResolvedValue(undefined),
291+
evaluate: vi.fn().mockResolvedValue(undefined),
292+
goto: vi.fn().mockResolvedValue(undefined),
293+
reload: vi.fn().mockResolvedValue(undefined),
294+
bringToFront: vi.fn().mockResolvedValue(undefined),
295+
close: vi.fn().mockResolvedValue(undefined),
296+
content: vi.fn().mockResolvedValue("<html></html>"),
297+
keyboard: { press: vi.fn(), type: vi.fn() },
298+
mouse: { click: vi.fn(), move: vi.fn() },
299+
}
300+
301+
// Mock browser with two pages
302+
const mockBrowser: any = {
303+
pages: vi.fn().mockResolvedValue([page1, page2]),
304+
newPage: vi.fn(),
305+
close: vi.fn(),
306+
disconnect: vi.fn(),
307+
}
308+
309+
;(session as any).browser = mockBrowser
310+
;(session as any).page = page1
311+
312+
// Navigate to a different domain
313+
await session.navigateToUrl("https://other.com/path")
314+
315+
// Reuses page1 (no new page created), navigates, and closes page2
316+
expect(page1.goto).toHaveBeenCalledWith("https://other.com/path", expect.any(Object))
317+
expect(page2.close).toHaveBeenCalledTimes(1)
318+
expect((session as any).page).toBe(page1)
319+
})
260320
})
261321

262322
describe("keyboard press", () => {

webview-ui/src/components/chat/BrowserSessionRow.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ const BrowserSessionRow = memo((props: BrowserSessionRowProps) => {
375375
</span>
376376
<div
377377
className="text-xs text-vscode-dropdown-foreground border-vscode-dropdown-border/50 border px-1.5 py-0.5 rounded-lg"
378-
style={{ marginLeft: "auto", opacity: totalApiCost > 0 ? 0.4 : 0 }}>
378+
style={{ marginLeft: "auto", opacity: totalApiCost > 0 ? 1 : 0 }}>
379379
${totalApiCost.toFixed(4)}
380380
</div>
381381
</div>
@@ -527,7 +527,7 @@ const BrowserSessionRow = memo((props: BrowserSessionRowProps) => {
527527
{/* Step counter */}
528528
{pages.length > 1 && (
529529
<span className="text-xs" style={{ color: "var(--vscode-descriptionForeground)" }}>
530-
{currentPageIndex + 1} of {pages.length}
530+
{t("chat:browser.navigation.step", { current: currentPageIndex + 1, total: pages.length })}
531531
</span>
532532
)}
533533
{/* Right side: chevron */}

0 commit comments

Comments
 (0)