fix: allow closing browser tabs during active requests

Allow users to close leased browser tabs without cancelling the AI request or revoking its other tools. Closed-page operations report a recoverable error; explicit navigation creates a dedicated replacement tab for direct-model and MCP calls. Add regression coverage and real Electron lifecycle checks, and update browser behavior documentation. Release note: 浏览器标签页被 AI 请求占用时也可关闭，其他任务继续运行；再次导航会打开独立新页。
2026-09-13 11:21:37 +08:00
parent 032ebc313f
commit b84250f62a
14 changed files with 534 additions and 72 deletions
@@ -326,6 +326,7 @@ describe('KnowledgeMcpGateway', () => {
      })
    )
    const browserService = {
+      createTab: vi.fn(),
      getOrigin: vi.fn(() => undefined),
      navigate: vi.fn(async () => ({
        url: 'https://example.com/',
@@ -464,6 +465,89 @@ describe('KnowledgeMcpGateway', () => {
    }
  })

+  it('keeps shared tools and the MCP session alive after tab closure and navigates a dedicated replacement', async () => {
+    const { service } = createService()
+    const browserService = new BrowserService()
+    const controller = new AbortController()
+    const lease = browserService.reserveRequestTab('conversation', 'request', 21)
+    const gateway = new KnowledgeMcpGateway(service, { browserService })
+    gateways.push(gateway)
+    const token = gateway.grant('request', [firstLibraryId], controller.signal, 'none', undefined, 'conversation', lease.tabId, lease)!
+    const navigate = vi.spyOn(browserService, 'navigate').mockImplementation(
+      async (_conversationId, _url, signal) => new Promise<never>((_resolve, reject) => {
+        signal.addEventListener('abort', () => reject(signal.reason), { once: true })
+      })
+    )
+    const snapshot = vi.spyOn(browserService, 'snapshot').mockResolvedValue({} as never)
+    const replacementController = new AbortController()
+    const replacementTabId = browserTabIdSchema.parse(crypto.randomUUID())
+    const replacementLease = {
+      conversationId: 'conversation', tabId: replacementTabId, owner: 'request',
+      signal: replacementController.signal, release: vi.fn()
+    }
+    const createTab = vi.spyOn(browserService, 'createTab').mockResolvedValue({ tabId: replacementTabId } as never)
+    const acquire = vi.spyOn(browserService, 'acquireTabUsage').mockReturnValue(replacementLease)
+    await gateway.start()
+    const client = new Client({ name: 'closed-tab-test', version: '1.0.0' })
+    try {
+      await client.connect(new StreamableHTTPClientTransport(new URL(gateway.getEndpoint()!), {
+        requestInit: { headers: { Authorization: `Bearer ${token}` } }
+      }))
+      const pending = client.callTool({ name: 'browser_navigate', arguments: { url: 'https://example.com/' } })
+      await vi.waitFor(() => expect(navigate).toHaveBeenCalledOnce())
+      lease.release()
+      await expect(pending).resolves.toMatchObject({
+        isError: true, content: [{ type: 'text', text: expect.stringContaining('标签页已关闭') }]
+      })
+      expect(navigate.mock.calls[0]![2].aborted).toBe(true)
+      expect(controller.signal.aborted).toBe(false)
+      await expect(client.callTool({ name: 'knowledge_search', arguments: { query: 'evidence' } }))
+        .resolves.toMatchObject({ content: [{ type: 'text', text: expect.stringContaining(firstLibraryId) }] })
+      expect((await client.listTools()).tools.map(tool => tool.name)).toContain('browser_navigate')
+      for (const name of browserToolNames.filter(name => name !== 'browser_navigate')) {
+        await expect(client.callTool({ name, arguments: {} })).resolves.toMatchObject({
+          isError: true, content: [{ type: 'text', text: expect.stringContaining('标签页已关闭') }]
+        })
+      }
+      expect(snapshot).not.toHaveBeenCalled()
+      expect(createTab).not.toHaveBeenCalled()
+      await expect(client.callTool({ name: 'browser_navigate', arguments: {} })).resolves.toMatchObject({ isError: true })
+      expect(createTab).not.toHaveBeenCalled()
+      navigate.mockResolvedValue({ url: 'https://example.com/', origin: 'https://example.com' })
+      await client.callTool({ name: 'browser_navigate', arguments: { url: 'https://example.com/' } })
+      expect(createTab).toHaveBeenCalledWith('conversation', undefined, expect.any(AbortSignal),
+        `browser-recovery:request:${lease.tabId}`)
+      expect(acquire).toHaveBeenCalledExactlyOnceWith('conversation', replacementTabId, 'request')
+      expect(navigate).toHaveBeenLastCalledWith('conversation', 'https://example.com/', expect.any(AbortSignal), replacementTabId)
+      await client.callTool({ name: 'browser_snapshot', arguments: {} })
+      expect(snapshot).toHaveBeenCalledWith('conversation', expect.any(AbortSignal), replacementTabId)
+      expect(createTab).toHaveBeenCalledOnce()
+      snapshot.mockImplementationOnce(async (_conversationId, signal) => new Promise<never>((_resolve, reject) => {
+        signal.addEventListener('abort', () => reject(signal.reason), { once: true })
+      }))
+      const callController = new AbortController()
+      const canceled = client.callTool({ name: 'browser_snapshot', arguments: {} }, undefined, {
+        signal: callController.signal
+      })
+      const cancellation = expect(canceled).rejects.toThrow('cancel this call')
+      await vi.waitFor(() => expect(snapshot).toHaveBeenCalledTimes(2))
+      callController.abort(new Error('cancel this call'))
+      await cancellation
+      await vi.waitFor(() => expect(snapshot.mock.calls[1]![1].aborted).toBe(true))
+      expect(replacementController.signal.aborted).toBe(false)
+      expect(controller.signal.aborted).toBe(false)
+      await expect(client.callTool({ name: 'browser_navigate', arguments: { url: 'https://example.com/' } }))
+        .resolves.not.toHaveProperty('isError', true)
+      expect(createTab).toHaveBeenCalledOnce()
+      gateway.revoke(token)
+      expect(replacementLease.release).toHaveBeenCalledOnce()
+    } finally {
+      await client.close()
+      await gateway.dispose()
+      await browserService.dispose()
+    }
+  })
+
  it('exposes GoodBuddy config reads in Ask and apply only in Execute', async () => {
    const { service } = createService()
    const configService = {
@@ -70,11 +70,12 @@ import {
 import type { LaunchEnvironmentProvider } from '../local-tool-environment'
 import {
  BrowserModelTools,
+  browserNavigateInputSchema,
  browserToolNames,
  type BrowserToolName,
  type BrowserToolService
 } from '../browser/browser-model-tools'
-import type { BrowserTabUsageLease } from '../browser/browser-service'
+import type { BrowserService, BrowserTabUsageLease } from '../browser/browser-service'
 import { BrowserStaleReferenceError } from '../browser/cdp-browser-driver'
 import { safeToolErrorDetail } from './approval-summary'

@@ -238,6 +239,7 @@ export type KnowledgeMcpGatewayOptions = {
  magicNotesDatabase?: MagicNotesDatabase
  configService?: GoodBuddyConfigService
  browserService?: BrowserToolService & {
+    createTab: BrowserService['createTab']
    listTabs(conversationId: string): BrowserTabSummary[]
    acquireTabUsage(
      conversationId: string,
@@ -371,6 +373,7 @@ export class KnowledgeMcpGateway {
  private readonly magicNotesDatabase?: MagicNotesDatabase
  private readonly configService?: GoodBuddyConfigService
  private readonly browserService?: BrowserToolService & {
+    createTab: BrowserService['createTab']
    listTabs(conversationId: string): BrowserTabSummary[]
    acquireTabUsage(
      conversationId: string,
@@ -581,15 +584,7 @@ export class KnowledgeMcpGateway {
    const abort = (): void => {
      this.revoke(token)
    }
-    const abortFromBrowser = (): void => {
-      this.revoke(token)
-    }
    value.signal.addEventListener('abort', abort, { once: true })
-    value.browserUsageLease?.signal.addEventListener(
-      'abort',
-      abortFromBrowser,
-      { once: true }
-    )
    this.capabilities.set(token, {
      ...value,
      expiresAt: this.now() + this.capabilityTtlMs,
@@ -597,10 +592,6 @@ export class KnowledgeMcpGateway {
      references: new Map(),
      removeAbortListener: () => {
        value.signal.removeEventListener('abort', abort)
-        value.browserUsageLease?.signal.removeEventListener(
-          'abort',
-          abortFromBrowser
-        )
      }
    })
    return token
@@ -1658,7 +1649,7 @@ export class KnowledgeMcpGateway {
            }
          }
          const capability = this.getCapability(token)
-          const browserTools = capability.browserConversationId && capability.browserTabId
+          let browserTools = capability.browserConversationId && capability.browserTabId
            ? new BrowserModelTools({
                service: this.browserService!,
                conversationId: capability.browserConversationId,
@@ -1666,12 +1657,52 @@ export class KnowledgeMcpGateway {
              })
            : undefined
          if (browserTools?.ownsTool(name)) {
+            let lease = capability.browserUsageLease!
            try {
+              const callSignal = AbortSignal.any([
+                extra.signal,
+                capability.signal,
+                capability.brokerController.signal
+              ])
+              callSignal.throwIfAborted()
+              if (lease.signal.aborted) {
+                if (name !== 'browser_navigate') {
+                  throw new Error('浏览器标签页已关闭，请调用 browser_navigate 打开新标签页。')
+                }
+                browserNavigateInputSchema.parse(input)
+                // A stable, request-specific ID also deduplicates concurrent recovery calls.
+                const tab = await this.browserService!.createTab(
+                  capability.browserConversationId!,
+                  undefined,
+                  callSignal,
+                  `browser-recovery:${capability.requestId}:${lease.tabId}`
+                )
+                callSignal.throwIfAborted()
+                if (capability.browserUsageLease === lease) {
+                  const replacement = this.browserService!.acquireTabUsage(
+                    capability.browserConversationId!,
+                    tab.tabId,
+                    capability.requestId
+                  )
+                  capability.browserTabId = tab.tabId
+                  capability.browserUsageLease = replacement
+                  lease.release()
+                }
+                lease = capability.browserUsageLease!
+                browserTools = new BrowserModelTools({
+                  service: this.browserService!,
+                  conversationId: capability.browserConversationId!,
+                  browserTabId: capability.browserTabId!
+                })
+              }
+              const browserSignal = AbortSignal.any([callSignal, lease.signal])
+              browserSignal.throwIfAborted()
              const result = await browserTools.callTool(
                name,
                input,
-                extra.signal
+                browserSignal
              )
+              browserSignal.throwIfAborted()
              return {
                content: result.parts.map((part) =>
                  part.type === 'text'
@@ -1685,7 +1716,9 @@ export class KnowledgeMcpGateway {
              }
            } catch (error) {
              const detail =
-                safeToolErrorDetail(error, 2_000) ??
+                (lease.signal.aborted
+                  ? '浏览器标签页已关闭，请调用 browser_navigate 打开新标签页。'
+                  : safeToolErrorDetail(error, 2_000)) ??
                '内置浏览器工具执行失败'
              return {
                isError: true,
@@ -11,6 +11,7 @@ import { rgPath } from '@vscode/ripgrep'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import type { ResolvedMcpServer } from '../capabilities/capability-service'
 import type { BrowserToolService } from '../browser/browser-model-tools'
+import { BrowserService } from '../browser/browser-service'
 import { browserTabIdSchema } from '../../shared/contracts'
 import { BrowserStaleReferenceError } from '../browser/cdp-browser-driver'
 import {
@@ -1432,6 +1433,71 @@ describe('ModelToolProvider', () => {
    })
  })

+  it('recovers a closed direct-model tab across calls without cancelling the request', async () => {
+    const workspace = await createWorkspace()
+    await writeFile(join(workspace, 'note.txt'), 'request still active')
+    const service = new BrowserService()
+    const original = service.reserveRequestTab('browser-owner', 'request')
+    const provider = new ModelToolProvider(workspace, [], service)
+    const context = {
+      ...toolContext, requestId: 'request', browserConversationId: 'browser-owner', browserTabId: original.tabId
+    }
+    const controller = new AbortController()
+    const navigate = vi.spyOn(service, 'navigate').mockImplementation(
+      async (_conversationId, _url, signal) => new Promise<never>((_resolve, reject) => {
+        signal.addEventListener('abort', () => reject(signal.reason), { once: true })
+      })
+    )
+    const snapshot = vi.spyOn(service, 'snapshot').mockResolvedValue({} as never)
+    const createTab = vi.spyOn(service, 'createTab').mockImplementation(async () => {
+      const replacement = service.reserveRequestTab('browser-owner', 'replacement')
+      return { tabId: replacement.tabId } as never
+    })
+    const acquire = vi.spyOn(service, 'acquireTabUsage')
+    try {
+      const pending = provider.callTool('browser_navigate', { url: 'https://example.com/' }, controller.signal, context)
+      const closed = expect(pending).rejects.toMatchObject({
+        name: 'RecoverableModelToolError', nextAction: expect.stringContaining('browser_navigate')
+      })
+      await vi.waitFor(() => expect(navigate).toHaveBeenCalledOnce())
+      await service.closeTab('browser-owner', original.tabId)
+      await closed
+      expect(controller.signal.aborted).toBe(false)
+      for (const [name, args] of [
+        ['browser_snapshot', {}], ['browser_click', { ref: 'b_old' }],
+        ['browser_type', { ref: 'b_old', text: 'value' }],
+        ['browser_select', { ref: 'b_old', value: 'value' }],
+        ['browser_back', {}], ['browser_screenshot', {}]
+      ] as const) {
+        await expect(provider.callTool(name, args, controller.signal, context)).rejects.toMatchObject({
+          name: 'RecoverableModelToolError', nextAction: expect.stringContaining('browser_navigate')
+        })
+      }
+      expect(createTab).not.toHaveBeenCalled()
+      expect(snapshot).not.toHaveBeenCalled()
+      await expect(provider.callTool('workspace_read_text', { path: 'note.txt' }, controller.signal, context))
+        .resolves.toMatchObject({ parts: [{ type: 'text', text: expect.stringContaining('request still active') }] })
+      navigate.mockResolvedValue({ url: 'https://example.com/', origin: 'https://example.com' })
+      await provider.callTool('browser_navigate', { url: 'https://example.com/' }, controller.signal, context)
+      const replacementTabId = navigate.mock.calls[1]![3]!
+      expect(replacementTabId).not.toBe(original.tabId)
+      expect(createTab).toHaveBeenCalledWith('browser-owner', undefined, controller.signal,
+        expect.stringContaining(`:${original.tabId}`))
+      expect(acquire).toHaveBeenCalledWith('browser-owner', replacementTabId, expect.any(String))
+      await provider.listTools(context, controller.signal)
+      await provider.callTool('browser_snapshot', {}, controller.signal, context)
+      expect(snapshot).toHaveBeenLastCalledWith('browser-owner', expect.any(AbortSignal), replacementTabId)
+      await expect(provider.callTool('browser_snapshot', {}, controller.signal, { ...context, requestId: 'other-request' }))
+        .rejects.toMatchObject({ name: 'RecoverableModelToolError' })
+      expect(createTab).toHaveBeenCalledOnce()
+      expect(controller.signal.aborted).toBe(false)
+    } finally {
+      original.release()
+      await provider.dispose()
+      await service.dispose()
+    }
+  })
+
  it('keeps a request on its bound tab when another tab changes or closes', async () => {
    const workspace = await createWorkspace()
    const browserService = createBrowserService()
@@ -24,6 +24,7 @@ import type {
 } from './runtime'
 import {
  BrowserModelTools,
+  BrowserTabClosedError,
  browserToolNames,
  type BrowserToolService
 } from '../browser/browser-model-tools'
@@ -661,6 +662,8 @@ function normalizeMcpResult(result: unknown): ModelToolResult {
 }

 export class ModelToolProvider implements ModelToolProviderLike {
+  // ModelAgentRuntime reuses one context for all tool calls in a request.
+  private readonly browserToolsByContext = new WeakMap<ModelToolCallContext, BrowserModelTools>()
  private readonly mcpConnections = new Map<
    ResolvedMcpServer,
    Promise<ConnectedMcp | undefined>
@@ -738,16 +741,20 @@ export class ModelToolProvider implements ModelToolProviderLike {
  private getBrowserTools(
    context: ModelToolCallContext
  ): BrowserModelTools | undefined {
-    return this.browserService &&
-      context.workMode === 'execute' &&
-      context.browserTabId
-      ? new BrowserModelTools({
-          service: this.browserService,
-          conversationId:
-            context.browserConversationId ?? context.conversationId,
-          browserTabId: context.browserTabId
-        })
-      : undefined
+    if (!this.browserService || context.workMode !== 'execute' || !context.browserTabId) {
+      return undefined
+    }
+    let tools = this.browserToolsByContext.get(context)
+    if (!tools) {
+      tools = new BrowserModelTools({
+        service: this.browserService,
+        conversationId: context.browserConversationId ?? context.conversationId,
+        browserTabId: context.browserTabId,
+        recoverClosedTab: true
+      })
+      this.browserToolsByContext.set(context, tools)
+    }
+    return tools
  }

  private getReservedToolCount(): number {
@@ -1707,6 +1714,13 @@ export class ModelToolProvider implements ModelToolProviderLike {
      try {
        return await browserTools.callTool(name, argumentsValue, signal)
      } catch (error) {
+        if (error instanceof BrowserTabClosedError) {
+          throw new RecoverableModelToolError(
+            error.message,
+            '调用 browser_navigate 打开新标签页，然后获取新快照继续操作',
+            { cause: error }
+          )
+        }
        if (error instanceof BrowserStaleReferenceError) {
          throw new RecoverableModelToolError(
            error.message,