diff --git a/docs/features/assistant-workbar/browser-tabs-technical-design.md b/docs/features/assistant-workbar/browser-tabs-technical-design.md index 580525b5..06dac33f 100644 --- a/docs/features/assistant-workbar/browser-tabs-technical-design.md +++ b/docs/features/assistant-workbar/browser-tabs-technical-design.md @@ -6,7 +6,7 @@ | --- | --- | | 状态 | 基础多实例已实施，Agent 多 Tab 工具待需求 | | 版本 | 1.0 | -| 日期 | 2026-09-10 | +| 日期 | 2026-09-13 | | 关联 PRD | [通用助手工作栏与执行空间 PRD](./prd.md) | | 页签策略 | [助手工作栏多终端页签 PRD](./terminal-tabs-prd.md) | @@ -86,7 +86,7 @@ Record> 默认或恢复实例可在首次使用时取得当前会话绑定。 Tab 的跨会话可见性与 MCP 授权分开处理。B 的请求不能选择属于 A 的可见 Tab；A 已取得的 -capability 在用户切到 B 后仍固定路由至 A 的原 Tab，使用租约也继续阻止关闭该 Tab。 +capability 在用户切到 B 后仍固定路由至 A 的原 Tab；使用租约不阻止用户关闭该 Tab。 内部点击实例只同步应用类型，不再次按当前 Conversation 改选实例。Main 状态携带对应的 `workbarInstanceId`，Renderer 据此绑定请求创建的同一 Tab，而不是另建空白页。收到 `stopped` 后清除该实例的旧 Tab 绑定，下一次使用通过原逻辑实例重新创建，保留地址草稿。 @@ -136,11 +136,16 @@ Conversation 归属，不新建租约或把页面重新归属到子级对话。 直连模型上下文和请求级 MCP capability 同时保存 `conversationId` 与 `boundBrowserTabId`。请求执行期间，用户切换、创建或关闭其他工作栏 Tab 不改变工具目标； -模型请求使用某个 Tab 时禁用该 Tab 的工作栏关闭操作，并说明请求仍在执行。用户先取消请求 -后才能关闭；系统不得把进行中的操作改投其他 Tab。 +用户可直接关闭模型正在使用的 Tab。关闭释放该 Tab 的页面和使用租约，中止其浏览器操作， +不取消整个模型请求、不撤销 MCP capability，也不关闭同级 Tab。系统不得把进行中的操作改投其他 Tab。 + +关闭后，快照等非导航工具返回“浏览器标签页已关闭，请调用 browser_navigate 打开新标签页”， +不自动创建页面或使用现有 primary/sibling。只有显式且参数有效的 `browser_navigate` 才创建 +专用替代 Tab 并绑定后续工具；旧 Tab 身份与元素引用不复用。Gateway 与直连 Provider 均隔离 +单次工具取消信号、Tab 使用租约信号和请求信号，取消一次工具调用不终止后续调用。 该阶段 MCP transport、endpoint 和 token 生命周期不变，只扩展服务端 capability 内容和内部 -路由。旧工具调用继续使用被冻结的 Tab。 +路由。工具调用使用当前请求绑定的 Tab，只有上述显式导航恢复会更换绑定。 ### 6.2 Agent 多 Tab 阶段 @@ -189,6 +194,8 @@ primary, including one opened by an Agent, is not adopted by a new instance. New - 一个 Tab 的元素 `ref` 不能在另一个 Tab 中执行。 - 用户切换工作栏 Tab 不改变进行中模型请求的 MCP 浏览器目标。 - 关闭一个 Tab 不释放 Conversation Runtime，也不关闭同一 Conversation 的其他 Tab。 +- 关闭已租用 Tab 后，同一 MCP 会话仍可列出工具；旧目标快照报错且不创建页面，显式导航创建 + 不同身份的替代 Tab，同级页面、导航状态与其他 Conversation 的可见 viewport 保持不变。 - 旧版无 `tabId` 的模型工具调用仍操作请求绑定的默认 Tab。 ## 10. 实施顺序 diff --git a/docs/features/assistant-workbar/prd.md b/docs/features/assistant-workbar/prd.md index 3d3cf031..45cf228b 100644 --- a/docs/features/assistant-workbar/prd.md +++ b/docs/features/assistant-workbar/prd.md @@ -483,6 +483,10 @@ Renderer 不接收任意系统 PID 控制能力。控制动作引用 Main 签发 改变绑定到同一 Tab 的 Agent 后续操作。所有请求都进入 `BrowserService`；Renderer 不建立 第二套导航队列、抢占规则或恢复状态机。 +用户可关闭 Agent 正在使用的标签页，无需先取消整轮请求。关闭只中止该页的浏览器操作， +其他标签页和请求的其他工具继续可用。后续快照等操作应提示标签页已关闭；Agent 显式导航 +时打开专用新标签页并继续操作，不能自动接管用户的其他页面。 + 浏览器工具栏读取的权威状态至少包含： ```ts diff --git a/docs/features/assistant-workbar/progress.md b/docs/features/assistant-workbar/progress.md index 560573d4..c1919d3d 100644 --- a/docs/features/assistant-workbar/progress.md +++ b/docs/features/assistant-workbar/progress.md @@ -1,5 +1,30 @@ # 工作栏实现与验证进度 +## 2026-09-13 浏览器租用标签页关闭与显式恢复 + +- 更新 `tests/browser-tabs-electron-e2e.ts`，移除两处租用期间拒绝关闭的旧断言。 + 当前行为见 [Agent 与 MCP 路由](./browser-tabs-technical-design.md#6-agent-与-mcp-路由) + 和 [浏览器需求](./prd.md#77-浏览器)。同步修改 BrowserService、IPC、Gateway 和直连模型工具。 +- Windows 真实 Electron/Chromium 与 loopback HTTP/MCP 验证通过：关闭已物化的预留页和 + 已租用主标签页，等待原 WebContents 的 `destroyed` 事件，确认租约结束而请求信号未取消。 + 同一 token 和 MCP session 仍可列出工具；旧目标快照返回关闭提示且不创建页面。 + 显式 `browser_navigate` 创建不同身份的专用替代页，后续 MCP 快照读取该页。 +- 同 Conversation 的 sibling 仅在主标签页关闭后取得 `primary` 标记，页面 URL、导航状态、 + 时间戳、工作栏身份和 Cookie 内容保持不变；另一 Conversation 的页面与可见 viewport + 保持不变。撤销 capability 后完成替代页、同级页和 Context 清理。 +- 已先检查 runner：esbuild 输出临时 CJS，启动真实 Electron，子进程上限 120 秒，结束后 + 删除临时 bundle。`node build/run-browser-tabs-electron-e2e.cjs` 最终退出码 0； + 前三轮分别修正测试对销毁事件时序、服务旧 ID 错误文案和 sibling 主标签标记的假设。 + 测试文件 ESLint 与限定文件的 `git diff --check` 通过。 +- 覆盖边界：本轮通过 Main BrowserService 和真实 MCP transport 操作页面，未点击完整 + Renderer/Preload 关闭入口，未注入执行中的工具取消，也未运行直连模型循环或其他共享工具。 + 浏览器、Gateway、直连 Provider、Model Runtime 和 IPC 回归共 359 项通过、1 项跳过， + 包括执行中关闭、请求继续完成和其他工具继续可用。全库 typecheck、lint 和 diff 检查通过。 + 全量 `npm test` 在 Agent 离线依赖安装用例失败后达到 200 秒执行上限；无外部模型调用。 +- 本轮未修改 Agent、桥接或 Runtime 产品源码，也未连接远程 Host。远程调用若进入同一桌面 + Gateway，会使用上述服务端路由；远端 Runtime 到桌面的实际传输、取消传播及恢复仍需真实 + Host 验证，本记录不将本机 loopback 结果算作远程覆盖。 + ## 2026-09-12 浏览器刷新与关闭后的错误清理 - 按 [浏览器工具栏规则](./prd.md#77-浏览器) 修复空白标签页允许刷新的问题； @@ -83,7 +108,7 @@ 132 项通过。随后新增的 Ask 指令、继承 Runtime 用量持久化及计划模式 App 场景，3 项通过。 - 扩展真实 Electron/MCP E2E 在独立临时 profile 通过：四个未使用的请求预留不创建会话、 WebContents 或 UI 事件；首次 MCP 导航物化相同 Tab/工作栏身份，恢复不额外创建页面， - 并验证独立页面、共享 Cookie、请求租约关闭保护和清理。 + 并验证独立页面、共享 Cookie、当时的请求租约关闭保护和清理；关闭保护已于 2026-09-13 移除。 - 当前源码的共享 Linux x64 Host 验证已通过，覆盖工作区目标保护和重命名/Git Diff， 并通过桌面托管 ACP → 隔离 Agent → OpenCode 完成 Ask/Execute 和继承 Runtime 指标持久化； 该组共 3 次真实模型调用，清理后无所属进程残留。完整记录见 @@ -120,13 +145,13 @@ - Renderer 使用工作栏实例 UUID 原子创建或恢复 Browser Tab，并以 UUID viewport token 防止 延迟 cleanup 隐藏新活动 Tab。关闭单个实例只释放对应 Tab。 - 直连模型和请求级 MCP capability 在请求开始时优先绑定可见 Tab，其次绑定 primary Tab， - 并持有使用租约。请求结束或 capability 撤销前，绑定 Tab 拒绝关闭；工具参数未增加模型可见 + 并持有使用租约。当时绑定 Tab 拒绝关闭，该行为已于 2026-09-13 改为允许关闭和显式导航恢复；工具参数未增加模型可见 `tabId`，已有浏览器工具 schema 保持不变。 - 聚焦回归命令覆盖 Shared、BrowserService、Electron Session、IPC、Preload、MCP、Renderer 和 Workbar，13 个测试文件共 453 项通过；`npm run lint` 与 Web TypeScript 检查通过。 - `node build/run-browser-tabs-electron-e2e.cjs` 通过。该测试启动真实 Electron/Chromium 和本地 HTTP 页面，创建两个 `WebContentsView`，验证页面隔离、同 Conversation Cookie 共享、真实 - loopback MCP 的固定 Tab 导航与快照、使用租约关闭保护，以及单 Tab 和最终 Context 清理。 + loopback MCP 的固定 Tab 导航与快照、当时的使用租约关闭保护，以及单 Tab 和最终 Context 清理。 - `npm run typecheck`、`npm run lint`、`npm run build:bundle` 和 `git diff --check` 通过。 - 本轮整库 `npm test` 为 3686 项通过、61 项跳过、4 项失败。失败位于 portable/release 的 ASAR 元数据 fixture、Runtime 版本探测和 DeepSeek Harness 的本地 MCP fixture；定向复跑仍 diff --git a/src/main/agent/knowledge-mcp-gateway.test.ts b/src/main/agent/knowledge-mcp-gateway.test.ts index f494e7c3..7ee5d565 100644 --- a/src/main/agent/knowledge-mcp-gateway.test.ts +++ b/src/main/agent/knowledge-mcp-gateway.test.ts @@ -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((_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((_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 = { diff --git a/src/main/agent/knowledge-mcp-gateway.ts b/src/main/agent/knowledge-mcp-gateway.ts index e5e40d8b..ac105f86 100644 --- a/src/main/agent/knowledge-mcp-gateway.ts +++ b/src/main/agent/knowledge-mcp-gateway.ts @@ -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, diff --git a/src/main/agent/model-tool-provider.test.ts b/src/main/agent/model-tool-provider.test.ts index 6f371684..a8157778 100644 --- a/src/main/agent/model-tool-provider.test.ts +++ b/src/main/agent/model-tool-provider.test.ts @@ -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((_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() diff --git a/src/main/agent/model-tool-provider.ts b/src/main/agent/model-tool-provider.ts index 1c9eb383..ede261b7 100644 --- a/src/main/agent/model-tool-provider.ts +++ b/src/main/agent/model-tool-provider.ts @@ -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() private readonly mcpConnections = new Map< ResolvedMcpServer, Promise @@ -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, diff --git a/src/main/browser/browser-model-tools.test.ts b/src/main/browser/browser-model-tools.test.ts index 8820aa53..1138c2d6 100644 --- a/src/main/browser/browser-model-tools.test.ts +++ b/src/main/browser/browser-model-tools.test.ts @@ -245,4 +245,48 @@ describe('BrowserModelTools', () => { tools.callTool('browser_snapshot', {}, controller.signal) ).rejects.toHaveProperty('name', 'AbortError') }) + + it('validates explicit recovery navigation before creating a dedicated replacement', async () => { + const service = createService() + service.acquireTabUsage = vi.fn(() => { throw new Error('closed') }) + service.createTab = vi.fn() + const tools = new BrowserModelTools({ + service, conversationId: 'conversation', browserTabId, recoverClosedTab: true + }) + for (const args of [{}, { url: 'file:///private' }, { url: 'https://example.com', extra: true }]) { + await expect(tools.callTool('browser_navigate', args, signal)).rejects.toThrow() + } + await expect(tools.callTool('browser_snapshot', {}, signal)).rejects.toHaveProperty('name', 'BrowserTabClosedError') + await expect(tools.callTool('browser_navigate', { url: 'https://example.com' }, AbortSignal.abort())) + .rejects.toHaveProperty('name', 'AbortError') + expect(service.createTab).not.toHaveBeenCalled() + expect(service.navigate).not.toHaveBeenCalled() + }) + + it('preserves request cancellation and releases the per-call lease', async () => { + const service = createService() + const leaseController = new AbortController() + const release = vi.fn() + service.acquireTabUsage = vi.fn(() => ({ + conversationId: 'conversation', tabId: browserTabId, owner: 'request', + signal: leaseController.signal, release + })) + service.createTab = vi.fn() + vi.mocked(service.snapshot).mockImplementation(async (_conversationId, signal) => + new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => reject(signal.reason), { once: true }) + }) + ) + const tools = new BrowserModelTools({ + service, conversationId: 'conversation', browserTabId, recoverClosedTab: true + }) + const controller = new AbortController() + const reason = new Error('cancel request') + const pending = tools.callTool('browser_snapshot', {}, controller.signal) + const cancelled = expect(pending).rejects.toBe(reason) + controller.abort(reason) + await cancelled + expect(release).toHaveBeenCalledOnce() + expect(service.createTab).not.toHaveBeenCalled() + }) }) diff --git a/src/main/browser/browser-model-tools.ts b/src/main/browser/browser-model-tools.ts index fd40022b..bfc75048 100644 --- a/src/main/browser/browser-model-tools.ts +++ b/src/main/browser/browser-model-tools.ts @@ -7,7 +7,7 @@ import type { } from '../agent/model-tool-provider' import type { RuntimeApprovalRequest } from '../agent/runtime' import { canonicalizeBrowserUrl } from './browser-url-policy' -import type { BrowserService } from './browser-service' +import type { BrowserService, BrowserTabUsageLease } from './browser-service' import { MAX_BROWSER_INPUT_LENGTH as MAX_INPUT_LENGTH, MAX_BROWSER_SELECT_LENGTH as MAX_SELECT_LENGTH @@ -191,12 +191,21 @@ export type BrowserToolService = Pick< | 'back' | 'screenshot' | 'releaseConversation' -> +> & Partial> export type BrowserModelToolsOptions = { service: BrowserToolService conversationId: string browserTabId: BrowserTabId + // The scoped gateway manages its own binding and lease lifecycle. + recoverClosedTab?: boolean +} + +export class BrowserTabClosedError extends Error { + constructor(options?: ErrorOptions) { + super('浏览器标签页已关闭，请调用 browser_navigate 打开新标签页。', options) + this.name = 'BrowserTabClosedError' + } } function createTextResult(value: unknown): ModelToolResult { @@ -220,12 +229,15 @@ function navigationLabel(url: URL): string { export class BrowserModelTools { private readonly service: BrowserToolService private readonly conversationId: string - private readonly browserTabId: BrowserTabId + private browserTabId: BrowserTabId + private readonly recoveryOwner = randomUUID() + private readonly recoverClosedTab: boolean constructor(options: BrowserModelToolsOptions) { this.service = options.service this.conversationId = options.conversationId this.browserTabId = options.browserTabId + this.recoverClosedTab = options.recoverClosedTab === true if (!this.conversationId || this.conversationId.length > 500) { throw new Error('浏览器对话标识无效') } @@ -321,6 +333,53 @@ export class BrowserModelTools { if (!this.ownsTool(name)) { throw new Error(`未知浏览器工具：${name}`) } + if (!this.recoverClosedTab || !this.service.acquireTabUsage || !this.service.createTab) { + return this.callBoundTool(name, argumentsValue, signal, this.browserTabId) + } + let lease: BrowserTabUsageLease + try { + lease = this.service.acquireTabUsage( + this.conversationId, this.browserTabId, this.recoveryOwner + ) + } catch (error) { + if (name !== 'browser_navigate') { + throw new BrowserTabClosedError({ cause: error }) + } + const input = browserNavigateInputSchema.parse(argumentsValue) + canonicalizeBrowserUrl(input.url) + const tab = await this.service.createTab( + this.conversationId, undefined, signal, + `browser-recovery:${this.recoveryOwner}:${this.browserTabId}` + ) + signal.throwIfAborted() + lease = this.service.acquireTabUsage( + this.conversationId, tab.tabId, this.recoveryOwner + ) + this.browserTabId = tab.tabId + } + const browserSignal = AbortSignal.any([signal, lease.signal]) + try { + const result = await this.callBoundTool(name, argumentsValue, browserSignal, lease.tabId) + browserSignal.throwIfAborted() + return result + } catch (error) { + signal.throwIfAborted() + if (lease.signal.aborted) { + throw new BrowserTabClosedError({ cause: error }) + } + throw error + } finally { + lease.release() + } + } + + private async callBoundTool( + name: BrowserToolName, + argumentsValue: Record, + signal: AbortSignal, + browserTabId: BrowserTabId + ): Promise { + signal.throwIfAborted() if (name === 'browser_navigate') { const input = browserNavigateInputSchema.parse(argumentsValue) return createTextResult( @@ -328,7 +387,7 @@ export class BrowserModelTools { this.conversationId, input.url, signal, - this.browserTabId + browserTabId ) ) } @@ -338,7 +397,7 @@ export class BrowserModelTools { await this.service.snapshot( this.conversationId, signal, - this.browserTabId + browserTabId ) ) } @@ -348,7 +407,7 @@ export class BrowserModelTools { this.conversationId, input.ref, signal, - this.browserTabId + browserTabId ) return createTextResult({ clicked: input.ref }) } @@ -359,7 +418,7 @@ export class BrowserModelTools { input.ref, input.text, signal, - this.browserTabId + browserTabId ) return createTextResult({ typed: input.ref, @@ -374,7 +433,7 @@ export class BrowserModelTools { input.ref, input.value, signal, - this.browserTabId + browserTabId ) return createTextResult({ selected: input.ref, value: '[已隐藏]' }) } @@ -384,7 +443,7 @@ export class BrowserModelTools { await this.service.back( this.conversationId, signal, - this.browserTabId + browserTabId ) ) } @@ -392,7 +451,7 @@ export class BrowserModelTools { const screenshot = await this.service.screenshot( this.conversationId, signal, - this.browserTabId + browserTabId ) return { parts: [ diff --git a/src/main/browser/browser-service.test.ts b/src/main/browser/browser-service.test.ts index 11178511..77e30bf3 100644 --- a/src/main/browser/browser-service.test.ts +++ b/src/main/browser/browser-service.test.ts @@ -227,7 +227,8 @@ describe('BrowserService', () => { expect(restored.tabId).toBe(first.tabId) expect(harness.service.getTabCount()).toBe(1) first.release() - await expect(harness.service.closeTab('conversation', first.tabId, 21)).rejects.toThrow('正在被活动请求使用') + await harness.service.closeTab('conversation', first.tabId, 21) + expect(second.signal.aborted).toBe(true) second.release() await harness.service.closeTab('conversation', first.tabId, 21) await harness.service.closeTab('conversation', first.tabId, 21) @@ -557,7 +558,7 @@ describe('BrowserService', () => { await harness.service.dispose() }) - it('blocks closing a leased tab until every request releases it', async () => { + it('closes a leased tab and ends its leases without closing sibling tabs', async () => { const harness = createHarness() const signal = new AbortController().signal const primary = await harness.service.createTab('conversation', 11, signal) @@ -577,15 +578,14 @@ describe('BrowserService', () => { await expect( harness.service.closeTab('conversation', primary.tabId, 11) - ).rejects.toThrow('浏览器标签页正在被活动请求使用，无法关闭') - await expect( - harness.service.closeTab('conversation', sibling.tabId, 11) ).resolves.toBeUndefined() + expect(first.signal.aborted).toBe(true) + expect(second.signal.aborted).toBe(true) + expect(harness.service.listTabs('conversation', 11).map((tab) => tab.tabId)).toEqual([sibling.tabId]) + expect(harness.slots[0]?.session.dispose).toHaveBeenCalledOnce() + expect(harness.slots[1]?.session.dispose).not.toHaveBeenCalled() first.release() first.release() - await expect( - harness.service.closeTab('conversation', primary.tabId, 11) - ).rejects.toThrow('浏览器标签页正在被活动请求使用，无法关闭') second.release() await expect( harness.service.closeTab('conversation', primary.tabId, 11) diff --git a/src/main/browser/browser-service.ts b/src/main/browser/browser-service.ts index 2bdf0e56..93ac5b79 100644 --- a/src/main/browser/browser-service.ts +++ b/src/main/browser/browser-service.ts @@ -31,9 +31,6 @@ export class BrowserNavigationStoppedError extends Error { } } -export const BROWSER_TAB_IN_USE_ERROR = - '浏览器标签页正在被活动请求使用，无法关闭' - export type BrowserTabUsageLease = { readonly conversationId: string readonly tabId: BrowserTabId @@ -908,12 +905,14 @@ export class BrowserService { [...this.creations.entries()].some(([id, slot]) => slot.tabId === tabId && id !== conversationId) || [...this.requestTabs.values()].some((slot) => slot.tabId === tabId && slot.conversationId !== conversationId) ) throw new Error('浏览器标签页不存在或不属于当前对话') - if (reservation?.tabId === tabId && reservation.usageLeases.size > 0) { - throw new Error(BROWSER_TAB_IN_USE_ERROR) + if (reservation?.tabId === tabId) { + this.releaseRequestTab(conversationId) + } + if (creation?.tabId === tabId) { + creation.controller.abort(new Error('浏览器会话已关闭')) } return } - if (tab.usageLeases.size > 0) throw new Error(BROWSER_TAB_IN_USE_ERROR) await this.releaseTab(conversation!, tab) } diff --git a/src/main/ipc.test.ts b/src/main/ipc.test.ts index 5cb3ac4d..bd5c64bc 100644 --- a/src/main/ipc.test.ts +++ b/src/main/ipc.test.ts @@ -7739,6 +7739,94 @@ describe('registerIpcHandlers agent terminal state', () => { await harness.dispose() }) + it('completes the runtime request and releases its lease after browser lease abort', async () => { + const requestId = '00000000-0000-4000-8000-000000000031' + const tabId = browserTabIdSchema.parse( + '00000000-0000-4000-8000-000000000423' + ) + const leaseController = new AbortController() + const release = vi.fn() + const lease = { + conversationId: 'browser-lease-abort', + tabId, + owner: requestId, + signal: leaseController.signal, + release + } + let runtimeSignal: AbortSignal | undefined + const runtime = { + runtimeId: 'model', + capability: 'chat', + supportsToolExecution: true, + async *run(request: { requestId: string }, signal: AbortSignal) { + runtimeSignal = signal + leaseController.abort(new Error('browser tab closed')) + signal.throwIfAborted() + yield { requestId: request.requestId, type: 'done' } + } + } + let grantedLease: { release(): void } | undefined + const knowledgeGateway = { + grant: vi.fn((...args: unknown[]) => { + grantedLease = args[7] as { release(): void } + return 'browser-capability' + }), + getAvailableToolNames: vi.fn(() => ['browser_navigate']), + drainReferences: vi.fn(() => []), + revoke: vi.fn(() => grantedLease?.release()) + } + const browserControl = { + listTabs: vi.fn(() => [{ tabId, primary: true }]), + getVisibleTabId: vi.fn(() => tabId), + acquireTabUsage: vi.fn(() => lease), + createTab: vi.fn(), + onState: vi.fn(() => () => undefined) + } + const harness = createHarness( + runtime, + undefined, + 'always', + undefined, + false, + undefined, + undefined, + knowledgeGateway, + false, + undefined, + { getEnabledBuiltinMcpServerIds: vi.fn(async () => ['builtin-browser']) }, + browserControl + ) + + try { + await harness.handler?.(trustedEvent(harness.webContents), { + requestId, + conversationId: lease.conversationId, + prompt: 'Continue after the browser tab closes', + workMode: 'execute', + knowledgeLibraryIds: [] + }) + await vi.waitFor(() => expect(release).toHaveBeenCalledOnce()) + + expect(grantedLease).toBe(lease) + expect(lease.signal.aborted).toBe(true) + expect(runtimeSignal?.aborted).toBe(false) + expect(harness.assistantDatabase.updateTaskStatus).toHaveBeenCalledWith( + requestId, + 'completed' + ) + const publicEvents = harness.webContents.send.mock.calls + .filter(([channel]) => channel === ipcChannels.agentEvent) + .map(([, payload]) => payload) + expect(publicEvents).toContainEqual({ requestId, type: 'done' }) + expect(publicEvents).not.toEqual(expect.arrayContaining([ + expect.objectContaining({ type: 'error' }) + ])) + expect(knowledgeGateway.revoke).toHaveBeenCalledWith('browser-capability') + } finally { + await harness.dispose() + } + }) + it.each(['error', 'cancel'] as const)( 'releases a request-created browser tab lease on runtime %s', async (outcome) => { diff --git a/src/main/ipc.ts b/src/main/ipc.ts index c143b1ec..852cf96f 100644 --- a/src/main/ipc.ts +++ b/src/main/ipc.ts @@ -552,7 +552,6 @@ async function grantScopedDataCapability(input: { signal: AbortSignal ) => Promise signal: AbortSignal - abort?: (reason: unknown) => void }): Promise { const enabledServers = new Set(input.enabledServers) const libraryIds = enabledServers.has('knowledge-base') @@ -590,11 +589,6 @@ async function grantScopedDataCapability(input: { }) : undefined const browserTabId = browserUsageLease?.tabId - browserUsageLease?.signal.addEventListener( - 'abort', - () => input.abort?.(browserUsageLease.signal.reason), - { once: true } - ) const config = configAccess !== 'none' && input.workspacePath ? { @@ -2485,8 +2479,7 @@ export function registerIpcHandlers( ? runtimeConversationId : undefined, ownerWindowId: window.webContents.id, - signal: controller.signal, - abort: (reason) => controller.abort(reason) + signal: controller.signal }) knowledgeCapabilityToken = notesCapability.token const noteTools = [ @@ -4060,8 +4053,7 @@ export function registerIpcHandlers( ? enrichedRequest.conversationId : undefined, ownerWindowId: event.sender.id, - signal: controller.signal, - abort: (reason) => controller.abort(reason) + signal: controller.signal }) const knowledgeCapabilityToken = scopedCapability.token const availableTools = [ diff --git a/tests/browser-tabs-electron-e2e.ts b/tests/browser-tabs-electron-e2e.ts index 56c1cfee..4486c242 100644 --- a/tests/browser-tabs-electron-e2e.ts +++ b/tests/browser-tabs-electron-e2e.ts @@ -1,4 +1,5 @@ import assert from 'node:assert/strict' +import { once } from 'node:events' import { createServer } from 'node:http' import { app, BrowserWindow, webContents } from 'electron' import { Client } from '@modelcontextprotocol/sdk/client/index.js' @@ -88,7 +89,15 @@ async function main(): Promise { assert.equal(restoredRequestTab.workbarInstanceId, reserved.tabId) assert.equal(browser.getTabCount(), 1) assert.match(JSON.stringify(await client.callTool({ name: 'browser_snapshot', arguments: {} })), /MCP target/u) - await assert.rejects(browser.closeTab(conversationId, reserved.tabId, window.webContents.id), /正在被.*请求使用/u) + await browser.closeTab(conversationId, reserved.tabId, window.webContents.id) + assert.equal(reserved.signal.aborted, true) + assert.equal(controller.signal.aborted, false) + assert.equal(browser.getSessionCount(), 0) + const closedReservationSnapshot = await client.callTool({ name: 'browser_snapshot', arguments: {} }) + assert.equal(closedReservationSnapshot.isError, true) + assert.match(JSON.stringify(closedReservationSnapshot), /标签页已关闭.*browser_navigate/u) + assert((await client.listTools()).tools.some((tool) => tool.name === 'browser_navigate')) + assert.equal(browser.getSessionCount(), 0) gateway.revoke(token) token = undefined await client.close() @@ -97,7 +106,7 @@ async function main(): Promise { await browser.closeTab(conversationId, reserved.tabId, window.webContents.id) assert.equal(browser.getSessionCount(), 0) removeStateListener() - console.log('MCP navigation materialized the exact reserved tab/workbar identity and released its request lease') + console.log('MCP reservation materialized the exact tab/workbar identity; leased close preserved the MCP session without recreating a page') await browser.navigate(conversationId, `${origin}/first`, controller.signal, undefined, window.webContents.id) const [unownedPrimary] = browser.listTabs(conversationId, window.webContents.id) @@ -180,16 +189,54 @@ async function main(): Promise { assert.equal(browser.getVisibleTabId(otherConversationId, window.webContents.id), other.tabId) assert.match(JSON.stringify(await browser.snapshot(conversationId, controller.signal, second.tabId, window.webContents.id)), /shared-browser-tab=visible/u) + const closedContents = webContents.getAllWebContents().find((contents) => contents.getURL() === `${origin}/mcp`) + assert(closedContents) + const siblingBeforeClose = browser.listTabs(conversationId, window.webContents.id).find((tab) => tab.tabId === second.tabId) + assert(siblingBeforeClose) + const destroyed = once(closedContents, 'destroyed', { signal: AbortSignal.timeout(5_000) }) + await browser.closeTab(conversationId, first.tabId, window.webContents.id) + await destroyed + assert.equal(closedContents.isDestroyed(), true) + assert.equal(usage.signal.aborted, true) + assert.equal(controller.signal.aborted, false) + assert.equal(browser.getTabCount(conversationId), 1) + assert(gateway.getAvailableToolNames(token).includes('browser_navigate')) + assert((await client.listTools()).tools.some((tool) => tool.name === 'browser_snapshot')) + const closedSnapshot = await client.callTool({ name: 'browser_snapshot', arguments: {} }) + assert.equal(closedSnapshot.isError, true) + assert.match(JSON.stringify(closedSnapshot), /标签页已关闭.*browser_navigate/u) await assert.rejects( - browser.closeTab(conversationId, first.tabId, window.webContents.id), - /正在被.*请求使用/u + browser.snapshot(conversationId, controller.signal, first.tabId, window.webContents.id), + /当前浏览器标签页尚未导航/u ) + assert.equal(browser.getTabCount(conversationId), 1) + const replacementNavigation = await client.callTool({ name: 'browser_navigate', arguments: { url: `${origin}/mcp` } }) + assert.notEqual(replacementNavigation.isError, true) + const replacement = browser.listTabs(conversationId, window.webContents.id).find((tab) => tab.tabId !== second.tabId) + assert(replacement) + assert.notEqual(replacement.tabId, first.tabId) + assert.equal(browser.getTabCount(conversationId), 2) + const replacementSnapshot = await client.callTool({ name: 'browser_snapshot', arguments: {} }) + assert.notEqual(replacementSnapshot.isError, true) + assert.match(JSON.stringify(replacementSnapshot), /MCP target/u) + assert.match(JSON.stringify(await browser.snapshot(conversationId, controller.signal, replacement.tabId, window.webContents.id)), /MCP target/u) + // Closing the primary promotes its sibling without changing that sibling's page. + assert.deepEqual(browser.listTabs(conversationId, window.webContents.id).find((tab) => tab.tabId === second.tabId), { + ...siblingBeforeClose, + primary: true + }) + assert.match(JSON.stringify(await browser.snapshot(conversationId, controller.signal, second.tabId, window.webContents.id)), /shared-browser-tab=visible/u) + assert.match(JSON.stringify(await browser.snapshot(otherConversationId, controller.signal, other.tabId, window.webContents.id)), /Second tab/u) + assert.equal(browser.getVisibleTabId(otherConversationId, window.webContents.id), other.tabId) + console.log('Leased Electron tab destroyed; same MCP capability reported the closed target and explicitly navigated a dedicated replacement, leaving siblings untouched') await browser.closeTab(otherConversationId, other.tabId, window.webContents.id) assert.equal(browser.getTabCount(otherConversationId), 0) assert.equal(browser.getTabCount(conversationId), 2) gateway.revoke(token) token = undefined await browser.closeTab(conversationId, first.tabId, window.webContents.id) + assert.equal(browser.getTabCount(conversationId), 2) + await browser.closeTab(conversationId, replacement.tabId, window.webContents.id) assert.equal(browser.getTabCount(conversationId), 1) await browser.closeTab(conversationId, second.tabId, window.webContents.id) assert.equal(browser.getTabCount(conversationId), 0)