fix: open browser popup links in managed tabs

Links requesting a new window were silently blocked. Open them in managed browser tabs with the existing session restrictions, clean up script-closed tabs, and follow click-created tabs in subsequent direct and MCP browser tools.

Validation: 232 targeted tests and Windows Electron E2E passed; typecheck and lint passed. The full suite retains six failures outside the popup path.

Release note: Browser links that open a new page now open in a managed tab, and AI browser actions follow the new page after a click.
This commit is contained in:
mesalogo
2026-09-15 23:55:52 +08:00
parent 625182c3dc
commit f1d7197480
11 changed files with 450 additions and 20 deletions
+9
View File
@@ -1712,6 +1712,15 @@ export class KnowledgeMcpGateway {
browserSignal
)
browserSignal.throwIfAborted()
const openedTabId = browserTools.getBoundTabId()
if (openedTabId !== lease.tabId && capability.browserUsageLease === lease) {
const replacement = this.browserService!.acquireTabUsage(
capability.browserConversationId!, openedTabId, capability.requestId
)
capability.browserTabId = openedTabId
capability.browserUsageLease = replacement
lease.release()
}
return {
content: result.parts.map((part) =>
part.type === 'text'
@@ -48,6 +48,24 @@ const browserTabId = browserTabIdSchema.parse(
)
describe('BrowserModelTools', () => {
it('binds subsequent tools and approval origin to the popup returned by a click', async () => {
const service = createService()
const popupId = browserTabIdSchema.parse(crypto.randomUUID())
vi.mocked(service.click).mockResolvedValue({
conversationId: 'conversation', tabId: popupId, workbarInstanceId: popupId,
primary: false, status: 'ready', isLoading: false, canGoBack: false,
url: 'https://popup.example/', createdAt: 1, updatedAt: 1
})
const tools = new BrowserModelTools({ service, conversationId: 'conversation', browserTabId })
const result = await tools.callTool('browser_click', { ref }, signal)
expect(JSON.stringify(result)).toContain(popupId)
expect(JSON.stringify(result)).toContain('browser_snapshot')
expect(tools.getBoundTabId()).toBe(popupId)
await tools.callTool('browser_snapshot', {}, signal)
expect(service.snapshot).toHaveBeenLastCalledWith('conversation', signal, popupId)
tools.getApproval('browser_snapshot', {})
expect(service.getOrigin).toHaveBeenLastCalledWith('conversation', popupId)
})
it('publishes seven strict, bounded builtin tool definitions', () => {
const tools = new BrowserModelTools({
service: createService(),
+14 -1
View File
@@ -373,6 +373,10 @@ export class BrowserModelTools {
}
}
getBoundTabId(): BrowserTabId {
return this.browserTabId
}
private async callBoundTool(
name: BrowserToolName,
argumentsValue: Record<string, unknown>,
@@ -403,12 +407,21 @@ export class BrowserModelTools {
}
if (name === 'browser_click') {
const input = browserClickInputSchema.parse(argumentsValue)
await this.service.click(
const opened = await this.service.click(
this.conversationId,
input.ref,
signal,
browserTabId
)
if (opened) {
this.browserTabId = opened.tabId
return createTextResult({
clicked: input.ref,
openedTabId: opened.tabId,
url: opened.url ?? 'about:blank',
message: '已切换到新标签页。请调用 browser_snapshot 获取新页面引用;旧页面引用不可复用。'
})
}
return createTextResult({ clicked: input.ref })
}
if (name === 'browser_type') {
+67
View File
@@ -1,4 +1,5 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import { EventEmitter } from 'node:events'
import { BrowserUrlPolicy, canonicalizeBrowserUrl } from './browser-url-policy'
import {
BrowserNavigationStoppedError,
@@ -15,6 +16,8 @@ type HarnessSlot = {
approvedOrigin?: string
emitLoading(isLoading: boolean): void
emitNavigation(url: string): void
openPopup(session: BrowserSessionLike, ready?: Promise<void>): boolean
destroy(): void
session: BrowserSessionLike
driver: BrowserDriverLike
}
@@ -54,14 +57,22 @@ function createHarness(options: {
const createSession = vi.fn(async (): Promise<BrowserSessionLike> => {
await options.sessionGate
const slot = {} as HarnessSlot
const events = new EventEmitter()
let destroyed = false
const webContents = {
on: events.on.bind(events),
off: events.off.bind(events),
isDestroyed: () => destroyed,
getURL: () => slot.currentUrl ?? `${slot.currentOrigin}/page`
} as BrowserWebContents
const loadingListeners = new Set<(isLoading: boolean) => void>()
const navigationListeners = new Set<(url: string) => void>()
let isLoading = false
let popupHandler: Parameters<NonNullable<BrowserSessionLike['setPopupHandler']>>[0]
let canOpenPopup: () => boolean
const session: BrowserSessionLike = {
webContents,
setPopupHandler: (handler, canOpen) => { popupHandler = handler; canOpenPopup = canOpen },
approveNavigation: vi.fn((target) => {
slot.approvedOrigin = target.origin
}),
@@ -126,6 +137,12 @@ function createHarness(options: {
Object.assign(slot, {
session,
driver,
openPopup: (popup: BrowserSessionLike, ready = Promise.resolve()) => {
if (!canOpenPopup()) return false
popupHandler(() => ({ session: popup, ready }))
return true
},
destroy: () => { destroyed = true; events.emit('destroyed') },
emitLoading: (loading: boolean) => {
isLoading = loading
for (const listener of loadingListeners) {
@@ -168,6 +185,56 @@ afterEach(() => {
})
describe('BrowserService', () => {
it('adopts click popups with ownership and releases script-closed tabs without closing the opener', async () => {
const harness = createHarness()
const signal = new AbortController().signal
const states: BrowserLiveState[] = []
harness.service.onState((state) => states.push(state))
try {
await harness.service.navigate('conversation', 'https://example.com/', signal, undefined, 21)
const opener = harness.slots[0]!
const openerTab = harness.service.listTabs('conversation')[0]!
const popupSession = await harness.createSession()
const popup = harness.slots[1]!
popup.currentUrl = 'https://popup.example/'
popup.currentOrigin = 'https://popup.example'
const ready = deferred<void>()
vi.mocked(opener.driver.click).mockImplementation(async () => {
expect(opener.openPopup(popupSession, ready.promise)).toBe(true)
})
const click = harness.service.click('conversation', 'b_link', signal, openerTab.tabId)
await vi.waitFor(() => expect(harness.service.getTabCount()).toBe(2))
ready.resolve()
const opened = await click
expect(opened).toMatchObject({ url: 'https://popup.example/', workbarInstanceId: expect.any(String) })
expect(states.find((state) => state.tabId === opened!.tabId)).toMatchObject({ openerTabId: openerTab.tabId, ownerWindowId: 21 })
await harness.service.snapshot('conversation', signal, opened!.tabId)
popup.destroy()
await vi.waitFor(() => expect(popupSession.dispose).toHaveBeenCalledOnce())
expect(harness.service.listTabs('conversation').map((tab) => tab.tabId)).toEqual([openerTab.tabId])
expect(opener.session.dispose).not.toHaveBeenCalled()
await vi.waitFor(() => expect(states.at(-1)).toMatchObject({ status: 'stopped', openerTabId: openerTab.tabId }))
} finally { await harness.service.dispose() }
})
it('counts pending popups against capacity and cleans up initialization after conversation close', async () => {
const harness = createHarness({ maximumTabsPerConversation: 2 })
const signal = new AbortController().signal
try {
await harness.service.navigate('conversation', 'https://example.com/', signal)
const opener = harness.slots[0]!
const popupSession = await harness.createSession()
harness.slots[1]!.currentUrl = 'about:blank'
const ready = deferred<void>()
expect(opener.openPopup(popupSession, ready.promise)).toBe(true)
expect(opener.openPopup(popupSession)).toBe(false)
await harness.service.releaseConversation('conversation')
ready.resolve()
await vi.waitFor(() => expect(popupSession.dispose).toHaveBeenCalledOnce())
expect(harness.service.getTabCount()).toBe(0)
expect(opener.openPopup(popupSession)).toBe(false)
} finally { await harness.service.dispose() }
})
const firstLeaseToken = '7d201980-0ad4-4670-81d4-dc2bf79f03b2'
const secondLeaseToken = 'db8a2c28-43a6-4aab-93e0-1c01b9374dca'
const workbarInstanceId = '0387bd61-3a12-40ce-98d7-ef5d14cc8251'
+90 -4
View File
@@ -45,6 +45,10 @@ export type BrowserSessionLike = {
target: Awaited<ReturnType<BrowserUrlPolicy['validate']>>
): void
createTab?(signal: AbortSignal): Promise<BrowserSessionLike>
setPopupHandler?(
handler: (create: () => { session: BrowserSessionLike; ready: Promise<void> }) => void,
canOpen: () => boolean
): void
getCurrentOrigin(): string | undefined
isLoading(): boolean
onLoadingChange(listener: (isLoading: boolean) => void): () => void
@@ -108,6 +112,9 @@ type BrowserTabSlot = {
stopLoadingRequested: boolean
removeLoadingListener: () => void
removeNavigationListener: () => void
removeCloseListener: () => void
popupOperations?: Promise<BrowserTabSummary>[]
openerTabId?: BrowserTabId
released: boolean
workbarInstanceId?: string
usageLeases: Map<symbol, {
@@ -360,6 +367,9 @@ export class BrowserService {
const state: BrowserLiveState = {
conversationId: tab.conversationId,
tabId: tab.tabId,
...((current?.openerTabId ?? previous?.openerTabId)
? { openerTabId: current?.openerTabId ?? previous?.openerTabId }
: {}),
...((tab.workbarInstanceId ?? current?.workbarInstanceId ?? previous?.workbarInstanceId)
? { workbarInstanceId: tab.workbarInstanceId ?? current?.workbarInstanceId ?? previous?.workbarInstanceId }
: {}),
@@ -496,10 +506,42 @@ export class BrowserService {
stopLoadingRequested: false,
removeLoadingListener: () => undefined,
removeNavigationListener: () => undefined,
removeCloseListener: () => undefined,
released: false,
usageLeases: new Map(),
...(workbarInstanceId ? { workbarInstanceId } : {})
}
session.setPopupHandler?.((create) => {
const context = this.conversations.get(conversationId)!
const releaseReservation = this.reserveTab(conversationId, context.ownerWindowId)
let popup: ReturnType<typeof create>
try {
popup = create()
} catch (error) {
releaseReservation()
throw error
}
const operation = this.adoptPopup(context, popup.session, popup.ready, tab.tabId).finally(releaseReservation)
tab.popupOperations?.push(operation)
void operation.catch((error: unknown) => {
if (this.hasTab(tab)) this.emitFailure(tab, '打开浏览器新标签页', error)
})
}, () => {
if (!this.hasTab(tab) || this.clearing || this.disposed) return false
try {
this.assertTabCapacity(this.conversations.get(conversationId), this.getOwnerWindowId(conversationId))
return true
} catch (error) {
this.emitFailure(tab, '打开浏览器新标签页', error)
return false
}
})
const closed = (): void => {
const context = this.conversations.get(conversationId)
if (context && this.hasTab(tab)) void this.releaseTab(context, tab).catch(() => undefined)
}
session.webContents.on?.('destroyed', closed)
tab.removeCloseListener = () => { session.webContents.off?.('destroyed', closed) }
tab.removeLoadingListener = session.onLoadingChange((isLoading) => {
if (!isLoading) tab.stopLoadingRequested = false
if (tab.released || tab.isLoading === isLoading || !this.hasTab(tab)) return
@@ -536,6 +578,40 @@ export class BrowserService {
return tab
}
private async adoptPopup(
conversation: BrowserConversationSlot,
session: BrowserSessionLike,
ready: Promise<void>,
openerTabId: BrowserTabId
): Promise<BrowserTabSummary> {
const tabId = createTabId()
let tab: BrowserTabSlot | undefined
void ready.catch(() => undefined)
try {
tab = await this.createTabSlot(conversation, conversation.conversationId, session, tabId, tabId)
tab.openerTabId = openerTabId
conversation.ownedTabs.set(tabId, tabId)
if (conversation.released || session.webContents.isDestroyed()) throw new Error('浏览器会话已关闭')
this.emitState(tab, 'ready')
this.applyViewport()
await waitFor(ready, AbortSignal.any([this.lifecycle.signal, conversation.lifecycle.signal]))
if (!this.hasTab(tab)) throw new Error('浏览器标签页已关闭')
const url = session.webContents.getURL()
if (url && url !== 'about:blank') {
const target = await this.policy.validateRedirect(url, conversation.lifecycle.signal)
session.approveNavigation(target)
tab.origin = target.origin
await this.refreshNavigationState(tab, conversation.lifecycle.signal)
}
this.touchConversation(conversation.conversationId)
return this.summarizeTab(tab)
} catch (error) {
if (tab) await this.releaseTab(conversation, tab).catch(() => undefined)
else await session.dispose().catch(() => undefined)
throw error
}
}
private hasTab(tab: BrowserTabSlot): boolean {
return !tab.released && this.conversations.get(tab.conversationId)?.tabs.get(tab.tabId) === tab
}
@@ -641,6 +717,7 @@ export class BrowserService {
if (signal.aborted || !hasWaiters() || this.releaseRequests.has(conversationId)) {
tab.removeLoadingListener()
tab.removeNavigationListener()
tab.removeCloseListener()
tab.driver.dispose()
await boundedCleanup(session.dispose(), this.cleanupTimeoutMs)
throw signal.reason ?? new Error('浏览器会话创建已取消')
@@ -1087,11 +1164,19 @@ export class BrowserService {
}, tabId, ownerWindowId)
}
async click(conversationId: string, ref: string, signal: AbortSignal, tabId?: BrowserTabId, ownerWindowId?: number): Promise<void> {
await this.runInTab(conversationId, signal, 'acting', true, '浏览器点击', async (tab, effectiveSignal) => {
async click(conversationId: string, ref: string, signal: AbortSignal, tabId?: BrowserTabId, ownerWindowId?: number): Promise<BrowserTabSummary | void> {
return this.runInTab(conversationId, signal, 'acting', true, '浏览器点击', async (tab, effectiveSignal) => {
await this.verifyCurrentOriginOrRelease(tab)
await tab.driver.click(ref, effectiveSignal)
await this.refreshNavigationState(tab, effectiveSignal)
const popups: Promise<BrowserTabSummary>[] = []
tab.popupOperations = popups
try {
await tab.driver.click(ref, effectiveSignal)
await this.refreshNavigationState(tab, effectiveSignal)
const opened = await waitFor(Promise.all(popups), effectiveSignal)
return opened.at(-1)
} finally {
tab.popupOperations = undefined
}
}, tabId, ownerWindowId)
}
@@ -1295,6 +1380,7 @@ export class BrowserService {
conversation.tabs.delete(tab.tabId)
tab.removeLoadingListener()
tab.removeNavigationListener()
tab.removeCloseListener()
tab.active?.controller.abort(new Error('浏览器会话已释放'))
for (const lease of tab.usageLeases.values()) {
lease.controller.abort(new Error('浏览器标签页使用租约已终止'))
@@ -25,7 +25,7 @@ function createHarness() {
const partitionEvents = new EventEmitter()
let currentUrl = ''
let loadingMainFrame = false
let openHandler: ((details: { url: string }) => { action: 'deny' }) | undefined
let openHandler: Parameters<BrowserWebContents['setWindowOpenHandler']>[0] | undefined
const sendCommand = vi.fn(async () => ({}))
const capturedImage = {
getSize: () => ({ width: 1_280, height: 800 }),
@@ -150,6 +150,48 @@ function createHarness() {
}
describe('ElectronBrowserSession', () => {
it('creates native popups in the sandboxed shared partition and rejects unsupported URLs', async () => {
const opener = createHarness()
const child = createHarness()
opener.window.createPopupView = vi.fn(() => child.window)
const session = await ElectronBrowserSession.create({
policy: opener.policy, createPartition: async () => opener.partition,
createView: async () => opener.window, createProxy: () => opener.proxy
})
let popup: ElectronBrowserSession | undefined
let ready: Promise<void> | undefined
session.setPopupHandler((create) => {
const created = create()
popup = created.session
ready = created.ready
}, () => true)
try {
for (const url of ['file:///secret', 'javascript:alert(1)', 'data:text/html,hello']) {
expect(opener.getOpenHandler()!({ url })).toEqual({ action: 'deny' })
}
const response = opener.getOpenHandler()!({ url: 'https://example.com/popup' })
expect(response).toMatchObject({ action: 'allow', outlivesOpener: true,
overrideBrowserWindowOptions: { webPreferences: {
partition: session.partition, sandbox: true, contextIsolation: true,
nodeIntegration: false, nodeIntegrationInSubFrames: false, nodeIntegrationInWorker: false
} }
})
expect(response.createWindow!(response.overrideBrowserWindowOptions!)).toBe(child.webContents)
expect(child.getOpenHandler()).toBeDefined()
const preventDefault = vi.fn()
child.contentEvents.emit('will-navigate', { preventDefault }, 'file:///secret')
expect(preventDefault).toHaveBeenCalledOnce()
await ready
expect(popup!.partition).toBe(session.partition)
await session.dispose()
expect(opener.partition.clearData).not.toHaveBeenCalled()
await popup!.dispose()
expect(opener.partition.clearData).toHaveBeenCalledOnce()
} finally {
await popup?.dispose()
await session.dispose()
}
})
it('creates an isolated sandboxed partition and denies privileged capabilities', async () => {
const harness = createHarness()
const createView = vi.fn(async (options: Record<string, unknown>) => {
+66 -11
View File
@@ -36,7 +36,12 @@ export type BrowserWebContents = {
on(event: string, listener: BrowserEventListener): unknown
off(event: string, listener: BrowserEventListener): unknown
setWindowOpenHandler(
handler: (details: { url: string }) => { action: 'deny' }
handler: (details: { url: string }) => {
action: 'deny' | 'allow'
outlivesOpener?: boolean
overrideBrowserWindowOptions?: Record<string, unknown>
createWindow?: (options: Record<string, unknown>) => BrowserWebContents
}
): void
capturePage?(): Promise<BrowserCapturedImage>
loadURL?(url: string): Promise<unknown>
@@ -51,6 +56,7 @@ export type BrowserWebContents = {
export type BrowserViewHandle = {
webContents: BrowserWebContents
nativeView?: unknown
createPopupView?: (options: Record<string, unknown>) => BrowserViewHandle
setVisible(visible: boolean): void
setBounds(bounds: BrowserWindowBounds): void
}
@@ -245,13 +251,17 @@ async function defaultCreateView(
options: Record<string, unknown>
): Promise<BrowserViewHandle> {
const electron = await import('electron')
const view = new electron.WebContentsView(options)
view.setVisible(false)
return {
nativeView: view,
webContents: view.webContents as unknown as BrowserWebContents,
setVisible: (visible) => view.setVisible(visible),
setBounds: (bounds) => view.setBounds(bounds)
return createView(options)
function createView(options: Record<string, unknown>): BrowserViewHandle {
const view = new electron.WebContentsView(options)
view.setVisible(false)
return {
nativeView: view,
createPopupView: createView,
webContents: view.webContents as unknown as BrowserWebContents,
setVisible: (visible) => view.setVisible(visible),
setBounds: (bounds) => view.setBounds(bounds)
}
}
}
@@ -264,6 +274,16 @@ export class ElectronBrowserSession {
private readonly navigationListeners = new Set<(url: string) => void>()
private loading = false
private disposed = false
private popupHandler?: (create: () => { session: ElectronBrowserSession; ready: Promise<void> }) => void
private canOpenPopup?: () => boolean
setPopupHandler(
handler: (create: () => { session: ElectronBrowserSession; ready: Promise<void> }) => void,
canOpen: () => boolean
): void {
this.popupHandler = handler
this.canOpenPopup = canOpen
}
private constructor(
private readonly policy: BrowserUrlPolicy,
@@ -462,7 +482,41 @@ export class ElectronBrowserSession {
this.listen(contents, 'did-stop-loading', () => {
this.setLoading(contents.isLoadingMainFrame?.() ?? false)
})
contents.setWindowOpenHandler(() => ({ action: 'deny' }))
contents.setWindowOpenHandler(({ url }) => {
if (this.disposed || !this.view.createPopupView || !this.popupHandler || !this.canOpenPopup?.()) {
return { action: 'deny' }
}
if (url !== 'about:blank') {
try { canonicalizeBrowserUrl(url) } catch { return { action: 'deny' } }
}
return {
action: 'allow',
outlivesOpener: true,
overrideBrowserWindowOptions: {
webPreferences: {
partition: this.partition, sandbox: true, contextIsolation: true,
nodeIntegration: false, nodeIntegrationInSubFrames: false,
nodeIntegrationInWorker: false, webSecurity: true,
allowRunningInsecureContent: false, plugins: false, devTools: false,
safeDialogs: true, backgroundThrottling: false
}
},
createWindow: (options) => {
let popup!: ElectronBrowserSession
this.popupHandler!(() => {
this.assertOpen()
const view = this.view.createPopupView!(options)
this.resources.references += 1
popup = new ElectronBrowserSession(this.policy, view, this.resources, this.parentWindow)
if (view.nativeView) this.parentWindow?.contentView?.addChildView(view.nativeView)
// Install navigation guards synchronously, before Chromium starts the popup request.
const ready = boundedSetup(popup.initialize(), new AbortController().signal, this.resources.setupTimeoutMs)
return { session: popup, ready }
})
return popup.webContents
}
}
})
this.listen(contents, 'will-navigate', (event: { preventDefault(): void }, details: { url?: string } | string) => {
const url = typeof details === 'string' ? details : details.url
if (!url || !this.updateOriginFromUrl(url)) {
@@ -494,6 +548,7 @@ export class ElectronBrowserSession {
callback()
})
this.listen(contents, 'did-navigate', (_event: unknown, url: string) => {
if (url === 'about:blank') return
if (url && !this.updateOriginFromUrl(url)) {
contents.stop()
return
@@ -742,10 +797,10 @@ export class ElectronBrowserSession {
for (const { target, event, listener } of this.listeners.splice(0)) {
target.off(event, listener)
}
if (this.webContents.debugger.isAttached()) {
if (!this.webContents.isDestroyed() && this.webContents.debugger.isAttached()) {
this.webContents.debugger.detach()
}
this.webContents.stop()
if (!this.webContents.isDestroyed()) this.webContents.stop()
this.view.setVisible(false)
if (this.view.nativeView) {
this.parentWindow?.contentView?.removeChildView(this.view.nativeView)