diff --git a/packages/browseros-agent/apps/server/src/browser/browser.ts b/packages/browseros-agent/apps/server/src/browser/browser.ts index e4a45cd83..4789dbcf0 100644 --- a/packages/browseros-agent/apps/server/src/browser/browser.ts +++ b/packages/browseros-agent/apps/server/src/browser/browser.ts @@ -249,6 +249,20 @@ export class Browser { return this.pages.get(pageId) } + private async resolvePageInfo(pageId: number): Promise { + const cached = this.pages.get(pageId) + if (cached) return cached + + await this.listPages() + return this.pages.get(pageId) + } + + private unknownPageError(pageId: number): Error { + return new Error( + `Unknown page ${pageId}. Use list_pages to see available pages.`, + ) + } + async refreshPageInfo(pageId: number): Promise { let info = this.pages.get(pageId) if (!info) { @@ -591,15 +605,31 @@ export class Browser { } async closePage(page: number): Promise { - const info = this.pages.get(page) - if (!info) - throw new Error( - `Unknown page ${page}. Use list_pages to see available pages.`, - ) - await this.cdp.Browser.closeTab({ tabId: info.tabId }) + let info = await this.resolvePageInfo(page) + if (!info) throw this.unknownPageError(page) + const targetIdsToDetach = new Set([info.targetId]) + + try { + await this.cdp.Browser.closeTab({ targetId: info.targetId }) + } catch (error) { + const refreshed = await this.refreshPageInfo(page) + if ( + !refreshed || + (refreshed.targetId === info.targetId && refreshed.tabId === info.tabId) + ) { + throw error + } + + info = refreshed + targetIdsToDetach.add(info.targetId) + await this.cdp.Browser.closeTab({ targetId: info.targetId }) + } + this.consoleCollector.detach(page) this.pages.delete(page) - this.sessions.delete(info.targetId) + for (const targetId of targetIdsToDetach) { + this.sessions.delete(targetId) + } } // --- Navigation --- diff --git a/packages/browseros-agent/apps/server/tests/browser/browser.test.ts b/packages/browseros-agent/apps/server/tests/browser/browser.test.ts new file mode 100644 index 000000000..9fb0aad4e --- /dev/null +++ b/packages/browseros-agent/apps/server/tests/browser/browser.test.ts @@ -0,0 +1,187 @@ +import { describe, it } from 'bun:test' +import assert from 'node:assert' +import type { + CloseTabParams, + GetTabInfoParams, + TabInfo, +} from '@browseros/cdp-protocol/domains/browser' +import type { ProtocolApi } from '@browseros/cdp-protocol/protocol-api' +import type { CdpBackend } from '../../src/browser/backends/types' +import { Browser } from '../../src/browser/browser' + +class FakeCdpBackend { + readonly closeTabCalls: CloseTabParams[] = [] + private nextSessionId = 1 + private readonly protocolSessions = new Map() + tabs: TabInfo[] + + Browser = { + getTabs: async () => ({ tabs: this.tabs }), + getTabInfo: async (params: GetTabInfoParams) => { + const tab = this.tabs.find( + (t) => t.tabId === params.tabId || t.targetId === params.targetId, + ) + if (!tab) throw new Error('Unknown page') + return { tab } + }, + closeTab: async (params: CloseTabParams) => { + this.closeTabCalls.push(params) + const index = this.tabs.findIndex( + (t) => t.tabId === params.tabId || t.targetId === params.targetId, + ) + if (index === -1) throw new Error('Unknown page') + this.tabs.splice(index, 1) + }, + } + + Target = { + on: () => () => {}, + attachToTarget: async () => { + const sessionId = `session-${this.nextSessionId++}` + this.protocolSessions.set(sessionId, createProtocolSession()) + return { sessionId } + }, + } + + constructor(tabs: TabInfo[]) { + this.tabs = tabs + } + + async connect(): Promise {} + async disconnect(): Promise {} + isConnected(): boolean { + return true + } + async getTargets(): Promise<[]> { + return [] + } + session(sessionId: string): ProtocolApi { + const session = this.protocolSessions.get(sessionId) + if (!session) throw new Error(`Unknown session ${sessionId}`) + return session + } + onSessionEvent(): () => void { + return () => {} + } +} + +function createProtocolSession(): ProtocolApi { + return { + Page: { enable: async () => {} }, + DOM: { enable: async () => {} }, + Runtime: { enable: async () => {} }, + Log: { enable: async () => {} }, + Accessibility: { enable: async () => {} }, + } as unknown as ProtocolApi +} + +function createTab(overrides: Partial): TabInfo { + return { + tabId: 101, + targetId: 'target-101', + url: 'https://example.com', + title: 'Example', + isActive: false, + isLoading: false, + loadProgress: 1, + isPinned: false, + isHidden: false, + windowId: 1, + index: 0, + ...overrides, + } +} + +function createBrowser(tabs: TabInfo[]): { + browser: Browser + cdp: FakeCdpBackend +} { + const cdp = new FakeCdpBackend(tabs) + return { + browser: new Browser(cdp as unknown as CdpBackend), + cdp, + } +} + +function getSessionCache(browser: Browser): Map { + return (browser as unknown as { sessions: Map }).sessions +} + +async function attachSession( + browser: Browser, + targetId: string, + pageId: number, +): Promise { + await ( + browser as unknown as { + attachToPage(targetId: string, pageId: number): Promise + } + ).attachToPage(targetId, pageId) +} + +describe('Browser', () => { + it('closes listed pages by target ID', async () => { + const { browser, cdp } = createBrowser([ + createTab({ tabId: 401, targetId: 'target-401' }), + ]) + + const pages = await browser.listPages() + assert.strictEqual(pages.length, 1) + + await browser.closePage(pages[0].pageId) + + assert.deepStrictEqual(cdp.closeTabCalls, [{ targetId: 'target-401' }]) + assert.strictEqual(cdp.tabs.length, 0) + }) + + it('refreshes pages before treating closePage page IDs as unknown', async () => { + const { browser, cdp } = createBrowser([ + createTab({ tabId: 402, targetId: 'target-402' }), + ]) + + await browser.closePage(1) + + assert.deepStrictEqual(cdp.closeTabCalls, [{ targetId: 'target-402' }]) + assert.strictEqual(cdp.tabs.length, 0) + }) + + it('retries closePage when the tab target changes before close', async () => { + const { browser, cdp } = createBrowser([ + createTab({ tabId: 403, targetId: 'target-403-old' }), + ]) + + const pages = await browser.listPages() + cdp.tabs[0].targetId = 'target-403-new' + + await browser.closePage(pages[0].pageId) + + assert.deepStrictEqual(cdp.closeTabCalls, [ + { targetId: 'target-403-old' }, + { targetId: 'target-403-new' }, + ]) + assert.strictEqual(cdp.tabs.length, 0) + }) + + it('clears stale session cache when closePage retries with a refreshed target ID', async () => { + const { browser, cdp } = createBrowser([ + createTab({ tabId: 404, targetId: 'target-404-old' }), + ]) + + const pages = await browser.listPages() + await attachSession(browser, 'target-404-old', pages[0].pageId) + assert.deepStrictEqual( + [...getSessionCache(browser).keys()], + ['target-404-old'], + ) + + cdp.tabs[0].targetId = 'target-404-new' + + await browser.closePage(pages[0].pageId) + + assert.deepStrictEqual(cdp.closeTabCalls, [ + { targetId: 'target-404-old' }, + { targetId: 'target-404-new' }, + ]) + assert.deepStrictEqual([...getSessionCache(browser).keys()], []) + }) +})