diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5e8222fba..4ecaffcbb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -322,6 +322,12 @@ jobs: cargo build -p bsk --locked pnpm --filter @browser-skill/extension build pnpm --filter @wxg-prc-cpg/browser-skill-dsh-plugin build + - name: Validate task termination on tab detachment + working-directory: apps/extension + env: + BSK_TAB_DETACH_CHROME: ${{ env.BSK_VIDEO_CHROME }} + BSK_TAB_DETACH_REPORT: ${{ env.BSK_VIDEO_ARTIFACTS }}/tab-detachment.json + run: pnpm exec vitest run src/session-manager/__tests__/tab-detachment.browser.test.ts - name: Validate capture, playback, recovery and CLI/DSH export working-directory: apps/extension env: diff --git a/README.md b/README.md index 3759c78a5..312b73f0a 100644 --- a/README.md +++ b/README.md @@ -130,6 +130,8 @@ Use browser-skill to open https://example.com, summarize the page, and end the b The agent opens its own window in the background, reads the page, returns the summary, and stops its task. External links or manually created tabs that land in the task window are moved to your most recently used normal window; a user window is created if none remain. Routing happens after tab creation, so a tab may briefly appear in the task window. Page-opened tabs and human assistance or recording are preserved. Harnesses that support skill slash commands can also use `/browser-skill`. +Moving any controlled tab out of its original Agent Window ends the entire browser session, preserves every page in place, and removes its control indicators. Borrowed tabs also stay where they are. Moving a tab back does not resume the task; starting again requires a user decision. Reordering tabs within the same window is unaffected. Normal session stop still returns borrowed tabs and cleans up task pages. + Chrome can still activate a window when a page opens a native popup. For observed task popups, the extension restores the previous Chrome window or minimized state if you have not switched windows. Chrome's public extension APIs cannot guarantee zero transient activation or restore focus to another application.
diff --git a/README.zh-CN.md b/README.zh-CN.md index 9e4d19e46..34a1e8c2c 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -130,6 +130,8 @@ bsk doctor Agent 默认在后台打开独立代理窗口、读取页面、返回总结,并结束任务。误入代理窗口的外部链接或手动新建标签会移回最近使用的普通用户窗口;没有用户窗口时会新建一个。分流发生在标签创建之后,可能短暂显示在代理窗口中。页面打开的子标签、人类协助和录制流程会保留。支持 skill 斜杠命令的 Agent,也可以通过 `/browser-skill` 调用。 +将任一受控标签移出原代理窗口会结束整个浏览器会话,保留全部页面并清除控制标识;借用页也留在当前位置。拖回标签不会恢复任务,需要由用户决定是否重新开始。同窗口内调整标签顺序不受影响,正常结束会话仍按原规则归还借用页、清理任务页。 + 网页打开原生弹窗时,Chrome 仍可能激活窗口。对于观察到的任务弹窗,若用户没有切换窗口,扩展会恢复之前的 Chrome 用户窗口或最小化状态。Chrome 公开扩展 API 无法保证完全没有瞬时激活,也无法恢复其他应用的焦点。
diff --git a/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts b/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts index 91b80777a..519e1f478 100644 --- a/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts +++ b/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts @@ -37,6 +37,62 @@ afterEach(() => { }); describe("ChromiumCdp", () => { + it.each([ + "abort", + "replacement", + ] as const)("does not downgrade %s during an OOPIF tree read to a missing child", async (ending) => { + const { api, onEvent } = fakeApi(); + let finishChild!: (value: object) => void; + let childStarted!: () => void; + const started = new Promise((resolve) => { + childStarted = resolve; + }); + const childReply = new Promise((resolve) => { + finishChild = resolve; + }); + vi.mocked(api.sendCommand).mockImplementation(async (target, method) => { + if (method === "Page.getFrameTree") { + if (target.sessionId === "child-session") { + childStarted(); + return childReply; + } + return { + frameTree: { + frame: { id: "main" }, + childFrames: [{ frame: { id: "child", parentId: "main" } }], + }, + }; + } + return {}; + }); + const cdp = new ChromiumCdp(api); + const signal = new AbortController(); + try { + await cdp.ensureAttached(4); + onEvent.fire({ tabId: 4 }, "Target.attachedToTarget", { + sessionId: "child-session", + targetInfo: { type: "iframe", targetId: "child" }, + }); + const graph = cdp.getFrameGraph(4, signal.signal); + await started; + if (ending === "abort") signal.abort(); + else { + await cdp.detach(4); + await cdp.ensureAttached(4); + } + const callsBeforeReply = vi.mocked(api.sendCommand).mock.calls.length; + finishChild({ frameTree: { frame: { id: "child" } } }); + await expect(graph).rejects.toMatchObject({ + name: ending === "abort" ? "AbortError" : "CdpAttachmentChangedError", + }); + expect(api.sendCommand).toHaveBeenCalledTimes(callsBeforeReply); + expect(api.attach).toHaveBeenCalledTimes(ending === "abort" ? 1 : 2); + } finally { + await cdp.detachAll(); + cdp.dispose(); + } + }); + it("isolates a known child tree timeout and keeps it gated until recovery", async () => { vi.useFakeTimers(); vi.spyOn(console, "warn").mockImplementation(() => {}); diff --git a/apps/extension/src/browser-driver/chromium-cdp.ts b/apps/extension/src/browser-driver/chromium-cdp.ts index 31c167b87..fd5c6fd14 100644 --- a/apps/extension/src/browser-driver/chromium-cdp.ts +++ b/apps/extension/src/browser-driver/chromium-cdp.ts @@ -421,18 +421,50 @@ export class ChromiumCdp { } } - async getFrameGraph(tabId: number): Promise { + async getFrameGraph(tabId: number, signal?: AbortSignal): Promise { + signal?.throwIfAborted(); await this.ensureAttached(tabId); + const attachmentId = this.getAttachmentId(tabId); + const check = () => { + signal?.throwIfAborted(); + if (attachmentId === undefined || this.getAttachmentId(tabId) !== attachmentId) { + const error = new Error("Debugger attachment changed during frame discovery"); + error.name = "CdpAttachmentChangedError"; + throw error; + } + }; + // Frame discovery belongs to this attachment. An old tree reply must not + // reconnect a released tab, or continue against another reader's connection. + const read = async ( + target: CdpTarget, + method: string, + params: object, + timeoutMs?: number, + ): Promise => { + check(); + try { + const result = await this.command(target, method, params, timeoutMs, check); + check(); + return result as T; + } catch (error) { + check(); + throw error; + } + }; + check(); // Ordinary configuration failures leave the graph to the root tree. A // timed-out read means the renderer is not answering: stop here instead of // queueing the tree, owner and snapshot reads behind the stuck command. await this.enableFrameDiscovery({ tabId }).catch(rethrowReadTimeout); + check(); this.retryChildFrameDiscovery(tabId); await this.drainFrameAttachTasks(tabId); + check(); const sources: CdpFrameTreeSource[] = []; const unavailable = new Map(); const isolateChildTimeout = (error: unknown, target: CdpTarget) => { + check(); if (!(error instanceof CdpReadTimeoutError)) return; const frameId = target.sessionId && this.frameDiscovery.get(tabId)?.frameIds.get(target.sessionId); @@ -442,7 +474,7 @@ export class ChromiumCdp { throw error; unavailable.set(frameId, target); }; - const root = await this.sendToTarget<{ frameTree?: CdpFrameTreeNode }>( + const root = await read<{ frameTree?: CdpFrameTreeNode }>( { tabId }, "Page.getFrameTree", {}, @@ -455,7 +487,7 @@ export class ChromiumCdp { sessions.map(async (sessionId): Promise => { const target = { tabId, sessionId }; try { - const reply = await this.sendToTarget<{ frameTree?: CdpFrameTreeNode }>( + const reply = await read<{ frameTree?: CdpFrameTreeNode }>( target, "Page.getFrameTree", {}, @@ -484,11 +516,9 @@ export class ChromiumCdp { const parent = frameById.get(frame.parentFrameId); if (!parent) return; try { - const owner = await this.sendToTarget<{ backendNodeId?: number }>( - parent.target, - "DOM.getFrameOwner", - { frameId: frame.frameId }, - ); + const owner = await read<{ backendNodeId?: number }>(parent.target, "DOM.getFrameOwner", { + frameId: frame.frameId, + }); if (owner.backendNodeId !== undefined) frame.ownerBackendNodeId = owner.backendNodeId; } catch (err) { // The frame may have navigated between tree capture and owner lookup. @@ -496,6 +526,7 @@ export class ChromiumCdp { } }), ); + check(); return omitFrameSubtrees(discovered, unavailable); } diff --git a/apps/extension/src/cooperation/controller.test.ts b/apps/extension/src/cooperation/controller.test.ts index a5b2090d5..2461893f2 100644 --- a/apps/extension/src/cooperation/controller.test.ts +++ b/apps/extension/src/cooperation/controller.test.ts @@ -2145,6 +2145,45 @@ describe("tab management respects human control of the task window", () => { expect(other.windowId).toBe(200); }); + it.each([ + "input", + "hold", + ] as const)("rechecks return after %s during the source tab lookup", async (intervention) => { + const f = await fixture(); + f.ctx.agentCreatedTabs.delete(7); + f.ctx.borrowedTabs.set(7, { tabId: 7, originalWindowId: 200, originalIndex: 0 }); + const move = vi.fn(async (_id, props) => ({ ...f.tab, ...props })); + const get = vi.fn(async () => { + if (intervention === "input") f.change({ human: 1, editing: true, lastInput: 10000 }); + else await f.controller.control(7, f.controller.view(7)!.revision, "hold"); + return { ...f.tab } as chrome.tabs.Tab; + }); + const params = { session_id: "shared", tab_id: 7 }; + expect( + await f.controller.run( + { id: "return", method: "tool.tab_return", params }, + f.cdp, + f.abort.signal, + (cdp, signal) => + handleTabReturn(f.sessions, params, { + cdp, + signal, + tabs: { get, move, create: vi.fn(), remove: vi.fn(), update: vi.fn() }, + windows: { + get: async () => ({ id: 200 }) as chrome.windows.Window, + getLastFocused: async () => ({ id: 200 }) as chrome.windows.Window, + create: vi.fn(), + remove: vi.fn(), + }, + agentOverlayReset: { resetAgentOverlays: async () => {} }, + }), + ), + ).toMatchObject({ data: { reason: "cooperation_sync_required", effect_state: "none" } }); + expect(get).toHaveBeenCalled(); + expect(move).not.toHaveBeenCalled(); + expect(f.ctx.borrowedTabs.has(7)).toBe(true); + }); + it("rechecks return after locating the original window", async () => { const f = await fixture(); f.ctx.agentCreatedTabs.delete(7); diff --git a/apps/extension/src/entrypoints/__tests__/background-overlays.test.ts b/apps/extension/src/entrypoints/__tests__/background-overlays.test.ts index 54e25832a..908e6eb7b 100644 --- a/apps/extension/src/entrypoints/__tests__/background-overlays.test.ts +++ b/apps/extension/src/entrypoints/__tests__/background-overlays.test.ts @@ -79,6 +79,7 @@ function event() { async function fixture() { const overlay = new OverlayController(); const onDetached = event<[number]>(); + const onAttached = event<[number, chrome.tabs.TabAttachInfo]>(); const onCreated = event<[chrome.tabs.Tab]>(); const onCreatedNavigationTarget = event<[{ tabId: number; sourceTabId: number }]>(); const sendMessage = vi.fn( @@ -96,7 +97,7 @@ async function fixture() { onActivated: event(), onUpdated: event(), onCreated, - onAttached: event(), + onAttached, onRemoved: event(), }, debugger: { onDetach: event() }, @@ -119,9 +120,33 @@ async function fixture() { (background as unknown as () => void)(); const deps = vi.mocked(ToolDispatcher).mock.calls.at(-1)![0]; const task = await deps.sessions.start("one"); - return { overlay, onDetached, onCreated, onCreatedNavigationTarget, sendMessage, deps, task }; + return { + overlay, + onDetached, + onAttached, + onCreated, + onCreatedNavigationTarget, + sendMessage, + deps, + task, + }; } +it("refreshes released-page controls and groups after attachment completes", async () => { + const f = await fixture(); + f.overlay.applyAgentControlMode("one", "control"); + f.deps.sessions.release("one"); + f.onAttached.emit(10, { newWindowId: 90, newPosition: 0 }); + await Promise.resolve(); + expect(f.overlay.snapshot().controlVisible).toBe(false); + expect(f.sendMessage).toHaveBeenCalledWith( + 10, + expect.objectContaining({ sessionId: null, mode: "hidden" }), + ); + const groups = vi.mocked(AgentWindowTabGroups).mock.instances.at(-1)!; + expect(groups.sync).toHaveBeenCalledOnce(); +}); + it("refreshes native window identification after session changes and explicit tab claims", async () => { const f = await fixture(); const groups = vi.mocked(AgentWindowTabGroups).mock.instances.at(-1)!; diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index 4204798f2..34424a3fb 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -55,6 +55,7 @@ import { attachRecordFinishListener, attachRecordQueryListener, attachRecordStepListener, + clearRecordingForSession, hasActiveRecording, type RecordRuntimeDeps, } from "@/tools/record"; @@ -92,6 +93,21 @@ export default defineBackground(() => { }); const sessions = new SessionManager({ remote: () => remoteEndpoint !== null }); const windowTabGroups = new AgentWindowTabGroups(sessions); + // Capture the original owner before cooperation releases a detached page. + attachSessionEventHandler({ + manager: sessions, + transport, + cdp: { + detachSession: (id) => cdp.detachSession(id), + releaseSessionTab: (id, tabId) => cdp.releaseSessionTab(id, tabId), + }, + onSessionReleased: (ctx) => { + dispatcher.cancelSession(ctx.sessionId); + cooperation.release(ctx.sessionId); + clearRecordingForSession(ctx.sessionId); + }, + onSessionsChanged: onOverlaySessionStateChanged, + }); const cooperation = attachCooperation( sessions, (tabId) => { @@ -357,6 +373,12 @@ export default defineBackground(() => { if (!sessions.findByWindowId(tab.windowId)) return; void pushOverlayStateForTab(tab.id, tab.windowId); }); + chrome.tabs.onAttached.addListener((tabId, { newWindowId }) => { + // Reconcile again after Chrome finishes the move; editing a group or + // messaging a page can fail transiently while it is being dragged. + void pushOverlayStateForTab(tabId, newWindowId); + windowTabGroups.sync(); + }); chrome.tabs.onDetached.addListener((tabId) => { void video.stopTab(tabId, "session_ended").catch(() => {}); debug.releaseTab(tabId); @@ -493,13 +515,6 @@ export default defineBackground(() => { "[browser-skill] chrome.notifications.onButtonClicked unavailable; borrow Allow/Deny buttons will be inactive", ); } - attachSessionEventHandler({ - manager: sessions, - transport, - cdp, - onSessionsChanged: onOverlaySessionStateChanged, - }); - // MV3 service worker keepalive + reconnect supervisor (review M4/M5 // C3 + C4). Every 30s the alarm wakes the SW; if the transport is // not connected we force a fresh connect attempt so we never sit on diff --git a/apps/extension/src/lib/__tests__/connection-controller.test.ts b/apps/extension/src/lib/__tests__/connection-controller.test.ts index 8bf42ffcb..d33b43094 100644 --- a/apps/extension/src/lib/__tests__/connection-controller.test.ts +++ b/apps/extension/src/lib/__tests__/connection-controller.test.ts @@ -41,7 +41,7 @@ describe("computeConnectedState (protocol-based compat)", () => { }); it("returns version_skew when daemon protocol minor is newer", () => { - expect(computeConnectedState(handshake("1.6", PROTOCOL_VERSION))).toEqual({ + expect(computeConnectedState(handshake("1.7", PROTOCOL_VERSION))).toEqual({ kind: "version_skew", }); }); @@ -61,11 +61,11 @@ describe("computeConnectedState (protocol-based compat)", () => { }); it("rejects when extension is below daemon min_compatible_protocol", () => { - const result = computeConnectedState(handshake("1.5", "1.6")); + const result = computeConnectedState(handshake(PROTOCOL_VERSION, "1.7")); expect(result.kind).toBe("rejected"); if (result.kind === "rejected") { expect(result.reason).toContain("min_compatible_protocol"); - expect(result.reason).toContain("1.6"); + expect(result.reason).toContain("1.7"); } }); @@ -85,17 +85,18 @@ describe("computeConnectedState (protocol-based compat)", () => { "1.2", "1.3", "1.4", - ])("rejects a daemon %s without the out-of-band dialog control protocol", (protocol) => { + "1.5", + ])("rejects a daemon %s without terminal session events", (protocol) => { for (const floor of [undefined, "1.0"]) { expect(computeConnectedState(handshake(protocol, floor))).toMatchObject({ kind: "rejected", - reason: expect.stringContaining("extension min_compatible_protocol 1.5"), + reason: expect.stringContaining("extension min_compatible_protocol 1.6"), }); } }); it("rejects malformed daemon min_compatible_protocol with a daemon-floor reason", () => { - const result = computeConnectedState(handshake("1.5", "not-a-protocol")); + const result = computeConnectedState(handshake(PROTOCOL_VERSION, "not-a-protocol")); expect(result.kind).toBe("rejected"); if (result.kind === "rejected") { expect(result.reason).toContain("daemon min_compatible_protocol"); @@ -160,7 +161,8 @@ describe("ConnectionController connectionEnabled", () => { "1.2", "1.3", "1.4", - ])("disconnects a daemon %s that cannot release a blocked dialog", async (protocol) => { + "1.5", + ])("disconnects a daemon %s that cannot handle terminal session events", async (protocol) => { const controller = new ConnectionController(); const transport = makeMockTransport(); const onDisconnected = vi.fn(); @@ -309,7 +311,7 @@ describe("ConnectionController connectionEnabled", () => { const oldDaemonError: RpcError = { code: "version_too_old", - message: "local protocol 1.3 is below peer min_compatible_protocol 1.5", + message: "local protocol 1.3 is below peer min_compatible_protocol 1.6", data: { daemon_version: "0.3.3", daemon_protocol_version: "1.3", @@ -320,7 +322,7 @@ describe("ConnectionController connectionEnabled", () => { const expectedUpgrade = { daemonVersion: "0.3.3", daemonProtocolVersion: "1.3", - requiredProtocolVersion: "1.5", + requiredProtocolVersion: "1.6", }; function rejectHandshake( diff --git a/apps/extension/src/session-manager/__tests__/tab-detachment-cdp.test.ts b/apps/extension/src/session-manager/__tests__/tab-detachment-cdp.test.ts new file mode 100644 index 000000000..c5cb965b3 --- /dev/null +++ b/apps/extension/src/session-manager/__tests__/tab-detachment-cdp.test.ts @@ -0,0 +1,323 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { type CdpDebuggerApi, ChromiumCdp } from "@/browser-driver/chromium-cdp"; +import { CooperationController } from "@/cooperation/controller"; +import { handleConsole } from "@/tools/console"; +import { handleSnapshot } from "@/tools/observation"; +import { attachSessionEventHandler } from "../event-handler"; +import { SessionManager } from "../manager"; +import { expectedTabMoves } from "../tab-moves"; + +function event() { + const listeners = new Set<(...args: T) => void>(); + return { + addListener(listener: (...args: T) => void) { + listeners.add(listener); + }, + removeListener(listener: (...args: T) => void) { + listeners.delete(listener); + }, + emit(...args: T) { + for (const listener of listeners) listener(...args); + }, + }; +} + +function deferred() { + let resolve!: () => void; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + +afterEach(() => { + expectedTabMoves.clear(); + vi.unstubAllGlobals(); + vi.useRealTimers(); +}); + +async function fixture() { + const attaching = deferred(), + finishAttach = deferred(); + const api = { + attach: vi.fn(async () => { + attaching.resolve(); + await finishAttach.promise; + }), + detach: vi.fn(async () => {}), + sendCommand: vi.fn(async () => ({})), + onEvent: event<[chrome.debugger.Debuggee, string, unknown]>(), + onDetach: event<[chrome.debugger.Debuggee, string]>(), + }; + const cdp = new ChromiumCdp(api as unknown as CdpDebuggerApi); + let nextWindow = 10; + const manager = new SessionManager({ + agentWindow: { + create: async () => ({ windowId: nextWindow++, initialTabIds: [] }), + ensureActiveTab: async (windowId) => (windowId === 10 ? 7 : 8), + remove: async () => {}, + }, + }); + await manager.start("one"); + const signal = new AbortController(); + const tabsApi = { + get: async () => ({ id: 7, windowId: 10, url: "https://example.test" }) as chrome.tabs.Tab, + query: async () => [], + }; + vi.stubGlobal("chrome", { tabs: tabsApi }); + const cooperation = new CooperationController({ + sessions: manager, + changed: () => {}, + resumeFrames: async () => {}, + readFrames: async () => + new Map([ + [ + 0, + { + document: "doc", + revision: 0, + structure: 0, + human: 0, + busy: false, + editing: false, + lastInput: 0, + fields: [], + events: [], + }, + ], + ]), + }); + cooperation.startSession("one"); + await cooperation.observeTab(7); + expect(api.attach).not.toHaveBeenCalled(); + const tabEvents = { + onDetached: event<[number, chrome.tabs.TabDetachInfo]>(), + onAttached: event<[number, chrome.tabs.TabAttachInfo]>(), + onRemoved: event<[number, chrome.tabs.TabRemoveInfo]>(), + }; + const cleanups: Promise[] = []; + const lifecycle = attachSessionEventHandler({ + manager, + transport: { send: () => {} }, + windowEvents: event<[number]>(), + tabEvents, + cdp: { + releaseSessionTab: (sessionId, tabId) => { + const cleanup = cdp.releaseSessionTab(sessionId, tabId); + cleanups.push(cleanup); + return cleanup; + }, + detachSession: (sessionId) => { + const cleanup = cdp.detachSession(sessionId); + cleanups.push(cleanup); + return cleanup; + }, + }, + agentOverlayReset: { resetAgentOverlays: async () => {} }, + onSessionReleased: () => { + signal.abort(); + cooperation.release("one"); + }, + }); + const readConsole = () => + cooperation.run( + { id: "read", method: "tool.console", params: { session_id: "one", tab_id: 7 } }, + cdp, + signal.signal, + (wrapped) => + handleConsole( + manager, + { session_id: "one", tab_id: 7 }, + { cdp: wrapped, tabsApi }, + signal.signal, + ), + ); + const readSnapshot = () => + cooperation.run( + { id: "read", method: "tool.snapshot", params: { session_id: "one", tab_id: 7 } }, + cdp, + signal.signal, + (wrapped) => + handleSnapshot( + manager, + { session_id: "one", tab_id: 7 }, + { cdp: wrapped, tabsApi }, + signal.signal, + ), + ); + const moveOut = () => { + tabEvents.onDetached.emit(7, { oldWindowId: 10, oldPosition: 0 }); + tabEvents.onAttached.emit(7, { newWindowId: 30, newPosition: 0 }); + expect(manager.has("one")).toBe(false); + expect(cooperation.enabled(7)).toBe(false); + }; + const dispose = async () => { + lifecycle.dispose(); + await cdp.detachAll(); + cdp.dispose(); + }; + return { + api, + cdp, + manager, + attaching, + finishAttach, + cleanups, + readConsole, + readSnapshot, + moveOut, + dispose, + }; +} + +describe("released task CDP attachments", () => { + it("cleans up the first console reader even before it has registered its CDP owner", async () => { + const f = await fixture(); + try { + const result = f.readConsole(); + await f.attaching.promise; + f.moveOut(); + f.finishAttach.resolve(); + expect(await result).toMatchObject({ code: "cancelled" }); + await Promise.all(f.cleanups); + expect(f.cdp.isAttached(7)).toBe(false); + expect(f.api.detach).toHaveBeenCalledExactlyOnceWith({ tabId: 7 }); + } finally { + await f.dispose(); + } + }); + + it("fences an attachment that completes after the existing cleanup budget", async () => { + vi.useFakeTimers(); + const f = await fixture(); + try { + const result = f.readConsole(); + await f.attaching.promise; + f.moveOut(); + await vi.advanceTimersByTimeAsync(1001); + await Promise.all(f.cleanups); + expect(f.cdp.isAttached(7)).toBe(false); + f.finishAttach.resolve(); + expect(await result).toHaveProperty("code"); + expect(f.cdp.isAttached(7)).toBe(false); + expect(f.api.detach).toHaveBeenCalledExactlyOnceWith({ tabId: 7 }); + expect(f.api.sendCommand).not.toHaveBeenCalled(); + } finally { + await f.dispose(); + } + }); + + it("preserves an attachment still owned by another session", async () => { + const f = await fixture(); + try { + const other = await f.manager.start("other"); + f.finishAttach.resolve(); + await f.cdp.ensureConsoleCapture(7); + f.cdp.trackSessionTab("one", 7); + f.cdp.trackSessionTab("other", 7); + f.moveOut(); + await Promise.all(f.cleanups); + expect(f.manager.get("other")).toBe(other); + expect(f.cdp.isAttached(7)).toBe(true); + expect(f.api.detach).not.toHaveBeenCalled(); + // The surviving owner can continue reading until it releases its claim. + await f.cdp.ensureConsoleCapture(7); + await f.cdp.releaseSessionTab("other", 7); + expect(f.cdp.isAttached(7)).toBe(false); + expect(f.api.detach).toHaveBeenCalledExactlyOnceWith({ tabId: 7 }); + } finally { + await f.dispose(); + } + }); +}); + +async function pendingSnapshot() { + const f = await fixture(); + const readingTree = deferred(), + finishTree = deferred(); + f.finishAttach.resolve(); + f.api.sendCommand.mockImplementation(async (_target, method) => { + if (method === "Page.getFrameTree") { + readingTree.resolve(); + await finishTree.promise; + return { + frameTree: { + frame: { id: "root", url: "https://example.test" }, + childFrames: [ + { frame: { id: "child", parentId: "root", url: "https://example.test/frame" } }, + ], + }, + }; + } + if (method === "DOM.getFrameOwner") return { backendNodeId: 3 }; + return {}; + }); + const result = f.readSnapshot(); + await readingTree.promise; + return { ...f, result, finishTree }; +} + +describe("released snapshot frame discovery", () => { + it("does not reconnect after a late frame-tree reply from a released task", async () => { + const f = await pendingSnapshot(); + try { + f.moveOut(); + await Promise.all(f.cleanups); + expect(f.cdp.isAttached(7)).toBe(false); + const commandsBeforeReply = f.api.sendCommand.mock.calls.length; + f.finishTree.resolve(); + expect(await f.result).toMatchObject({ code: "cancelled" }); + expect(f.api.attach).toHaveBeenCalledTimes(1); + expect(f.api.detach).toHaveBeenCalledTimes(1); + expect(f.cdp.isAttached(7)).toBe(false); + expect(f.api.sendCommand).toHaveBeenCalledTimes(commandsBeforeReply); + } finally { + await f.dispose(); + } + }); + + it("stops the cancelled snapshot while another session retains the same connection", async () => { + const f = await pendingSnapshot(); + try { + await f.manager.start("other"); + f.cdp.trackSessionTab("other", 7); + const attachment = f.cdp.getAttachmentId(7); + f.moveOut(); + await Promise.all(f.cleanups); + expect(f.cdp.getAttachmentId(7)).toBe(attachment); + const commandsBeforeReply = f.api.sendCommand.mock.calls.length; + f.finishTree.resolve(); + expect(await f.result).toMatchObject({ code: "cancelled" }); + expect(f.api.sendCommand).toHaveBeenCalledTimes(commandsBeforeReply); + expect(f.cdp.getAttachmentId(7)).toBe(attachment); + expect(f.api.detach).not.toHaveBeenCalled(); + } finally { + await f.dispose(); + } + }); + + it("does not use a replacement connection or fallback when attachment identity changes", async () => { + const f = await pendingSnapshot(); + try { + const oldAttachment = f.cdp.getAttachmentId(7); + await f.cdp.releaseSessionTab("one", 7); + await f.manager.start("other"); + f.cdp.trackSessionTab("other", 7); + await f.cdp.ensureConsoleCapture(7); + const replacement = f.cdp.getAttachmentId(7); + expect(replacement).not.toBe(oldAttachment); + const commandsBeforeReply = f.api.sendCommand.mock.calls.length; + f.finishTree.resolve(); + expect(await f.result).toMatchObject({ + code: "cdp_failed", + message: expect.stringContaining("attachment changed"), + }); + expect(f.api.sendCommand).toHaveBeenCalledTimes(commandsBeforeReply); + expect(f.api.attach).toHaveBeenCalledTimes(2); + expect(f.api.detach).toHaveBeenCalledTimes(1); + expect(f.cdp.getAttachmentId(7)).toBe(replacement); + } finally { + await f.dispose(); + } + }); +}); diff --git a/apps/extension/src/session-manager/__tests__/tab-detachment.browser.test.ts b/apps/extension/src/session-manager/__tests__/tab-detachment.browser.test.ts new file mode 100644 index 000000000..0661a87ae --- /dev/null +++ b/apps/extension/src/session-manager/__tests__/tab-detachment.browser.test.ts @@ -0,0 +1,390 @@ +// @vitest-environment node +// BSK_TAB_DETACH_CHROME=/path/to/chrome runs in an isolated headless profile. +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { createServer } from "node:http"; +import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +type Send = >( + method: string, + params?: object, + sessionId?: string, +) => Promise; + +// Keep the real overlay state machine and message protocol, without mounting +// the full application UI or connecting this isolated test browser to a daemon. +const content = ` +import { OverlayController } from '@/content/overlay-controller'; +import { OVERLAY_AGENT_STATE, OVERLAY_AGENT_OVERLAY_RESET, OverlayVersionGate } from '@/lib/overlay-bridge'; +const overlay = new OverlayController(), versions = new OverlayVersionGate(), identity = crypto.randomUUID(); +chrome.runtime.onMessage.addListener((message, _sender, reply) => { + if (message.type === OVERLAY_AGENT_STATE && versions.admit(message)) + overlay.applyAgentControlMode(message.sessionId, message.mode); + if (message.type === OVERLAY_AGENT_OVERLAY_RESET && versions.admit(message)) + overlay.resetAgentOverlays(message.sessionId); + if (message.type === 'fixture.overlay') reply({...overlay.snapshot(), identity}); + else reply({ok:true}); +}); +`; + +// Use the production lifecycle listener, ownership, tab tools, CDP driver and +// native group reconciler. Only the daemon transport is replaced by a recorder. +// A direct Chrome move deliberately bypasses the tool's expected-move registry, +// producing the same browser events as a user dragging a tab between windows. +const entry = ` +import { ChromiumCdp } from '@/browser-driver/chromium-cdp'; +import { attachCooperation } from '@/cooperation/browser-bridge'; +import { nextOverlayVersion } from '@/lib/overlay-version'; +import { OVERLAY_AGENT_STATE } from '@/lib/overlay-bridge'; +import { AgentWindowTabGroups } from '@/session-manager/agent-window-tab-group'; +import { attachSessionEventHandler } from '@/session-manager/event-handler'; +import { SessionManager } from '@/session-manager/manager'; +import { handleSessionStop } from '@/tools/session'; +import { chromeTabMutationApi, chromeTabsApi, handleTabBorrow, handleTabCreate, handleTabReturn } from '@/tools/tabs'; +const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); +const check = (condition, message) => { if (!condition) throw new Error(message); }; +const wait = async (predicate, label) => { + const deadline = Date.now() + 10000; + while (!(await predicate())) { + if (Date.now() > deadline) throw new Error('Timed out: ' + label); + await sleep(25); + } +}; +globalThis.run = async url => { + const controller = await chrome.tabs.getCurrent(); + await chrome.windows.create({tabId: controller.id, type: 'popup', focused: false}); + const user = (await chrome.windows.getAll({windowTypes: ['normal']}))[0]; + const manager = new SessionManager(), cdp = new ChromiumCdp(chrome.debugger); + const groups = new AgentWindowTabGroups(manager), events = [], completed = []; + let cooperation; + const lifecycle = attachSessionEventHandler({ + manager, cdp, transport: {send: event => events.push(event)}, + onSessionReleased: task => cooperation.release(task.sessionId), + onSessionsChanged: () => { cooperation.syncSessions(); groups.sync(); }, + }); + cooperation = attachCooperation(manager, () => {}); + const readOverlay = id => chrome.tabs.sendMessage(id, {type: 'fixture.overlay'}); + const active = async (task, id) => { + await wait(async () => { + try { return Boolean((await readOverlay(id))?.identity); } catch { return false; } + }, 'content ready ' + id); + await cooperation.observeTab(id); + await cdp.acquireBackgroundExecution(task.sessionId, id); + await chrome.tabs.sendMessage(id, { + type: OVERLAY_AGENT_STATE, mode: 'control', sessionId: task.sessionId, ...nextOverlayVersion(), + }); + check((await readOverlay(id)).controlVisible, 'control overlay missing ' + id); + return id; + }; + const start = async (label, extra = 1) => { + globalThis.phase = label; + const task = await manager.start(label); + cooperation.startSession(task.sessionId); + const home = [...task.agentCreatedTabs][0]; + await chrome.tabs.update(home, {url: url + '/' + label}); + await active(task, home); + for (let index = 0; index < extra; index++) { + const result = await handleTabCreate(manager, { + session_id: task.sessionId, url: url + '/' + label + '/' + index, + }, {cdp}); + check(!result.code, JSON.stringify(result)); + await active(task, result.tab_id); + } + groups.sync(); + return task; + }; + const borrow = async task => { + const tab = await chrome.tabs.create({windowId: user.id, url: url + '/borrow-' + task.sessionId, active: false}); + const result = await handleTabBorrow(manager, {session_id: task.sessionId, tab_id: tab.id}, { + approveBorrow: async () => true, cdp, + }); + check(!result.code, JSON.stringify(result)); + check(manager.has(task.sessionId), 'borrowing ended the task'); + await active(task, tab.id); + groups.sync(); + return tab.id; + }; + const observe = async task => { + const tab = await chrome.tabs.create({windowId: task.agentWindowId, url: url + '/observed-' + task.sessionId, active: false}); + (task.observedTabs ??= new Set()).add(tab.id); + await active(task, tab.id); + groups.sync(); + return tab.id; + }; + const claimed = task => [...task.agentCreatedTabs, ...task.borrowedTabs.keys(), ...(task.observedTabs ?? [])]; + const snapshot = async task => Promise.all(claimed(task).map(async id => ({ + id, windowId: (await chrome.tabs.get(id)).windowId, identity: (await readOverlay(id)).identity, + }))); + const sessionEvents = task => events.filter(event => event.payload?.session_id === task.sessionId); + const assertReleased = async (task, before, movedIds) => { + await wait(() => !manager.has(task.sessionId), 'task release'); + await wait(() => sessionEvents(task).length > 0, 'terminal event'); + await wait(async () => (await Promise.all(before.map(async ({id}) => { + const tab = await chrome.tabs.get(id), overlay = await readOverlay(id); + return tab.groupId === -1 && overlay.controlMode === 'hidden' && !overlay.controlVisible; + }))).every(Boolean), 'labels and overlays released'); + await wait(async () => { + const targets = await chrome.debugger.getTargets(); + return before.every(({id}) => !targets.find(target => target.tabId === id)?.attached); + }, 'debuggers released'); + const targets = await chrome.debugger.getTargets(); + for (const old of before) { + const tab = await chrome.tabs.get(old.id), overlay = await readOverlay(old.id); + check(tab.windowId === (movedIds.includes(old.id) ? user.id : old.windowId), 'unexpected page relocation ' + old.id); + check(overlay.identity === old.identity, 'document was recreated ' + old.id); + check(manager.findControllingSession(old.id) === null, 'ownership survived ' + old.id); + check(!cooperation.enabled(old.id), 'cooperation survived ' + old.id); + check(!cdp.ownsBackgroundExecution(task.sessionId, old.id), 'background execution survived ' + old.id); + check(!targets.find(target => target.tabId === old.id)?.attached, 'debugger survived ' + old.id); + } + check(!cooperation.hasSession(task.sessionId), 'default cooperation survived'); + await sleep(150); + const terminal = sessionEvents(task); + check(terminal.length === 1, 'duplicate terminal events: ' + JSON.stringify(terminal)); + check(terminal[0].event === 'session.ended' && terminal[0].payload.reason === 'user_moved_tab', 'wrong termination reason: ' + JSON.stringify(terminal)); + check((await chrome.windows.get(user.id)).type === 'normal', 'ordinary window disappeared'); + completed.push(task.sessionId); + }; + try { + const sibling = await start('independent-task', 0); + const created = await start('created-tab'); + const borrowed = await borrow(created); + await observe(created); + const before = await snapshot(created), moved = [...created.agentCreatedTabs][1]; + await chrome.tabs.move(moved, {windowId: user.id, index: -1}); + await assertReleased(created, before, [moved]); + check((await chrome.tabs.get(borrowed)).windowId === created.agentWindowId, 'drag exit returned a borrowed tab'); + check((await chrome.windows.get(created.agentWindowId)).type === 'normal', 'remaining task pages lost their window'); + check(manager.has(sibling.sessionId), 'drag ended another task'); + check((await readOverlay([...sibling.agentCreatedTabs][0])).controlVisible, 'another task lost its overlay'); + + for (const kind of ['borrowed-tab', 'observed-tab']) { + const task = await start(kind, 0); + const movedId = kind === 'borrowed-tab' ? await borrow(task) : await observe(task); + const saved = await snapshot(task); + await chrome.tabs.move(movedId, {windowId: user.id, index: -1}); + await assertReleased(task, saved, [movedId]); + } + + const grouped = await start('whole-group'); + const groupedTabs = await snapshot(grouped); + await wait(async () => { + const tabs = await Promise.all(groupedTabs.map(({id}) => chrome.tabs.get(id))); + return tabs.every(tab => tab.groupId !== -1 && tab.groupId === tabs[0].groupId); + }, 'native group ready'); + const groupId = (await chrome.tabs.get(groupedTabs[0].id)).groupId; + await chrome.tabGroups.move(groupId, {windowId: user.id, index: -1}); + await assertReleased(grouped, groupedTabs, groupedTabs.map(tab => tab.id)); + check(!(await chrome.windows.get(grouped.agentWindowId).catch(() => null)), 'empty source window remained'); + + const last = await start('last-tab', 0), lastTabs = await snapshot(last); + await chrome.tabs.move(lastTabs[0].id, {windowId: user.id, index: -1}); + await assertReleased(last, lastTabs, [lastTabs[0].id]); + check(!(await chrome.windows.get(last.agentWindowId).catch(() => null)), 'last-tab source window remained'); + + const reordered = await start('same-window-reorder', 2); + const reorderId = [...reordered.agentCreatedTabs][0]; + await chrome.tabs.move(reorderId, {index: -1}); + await sleep(150); + check(manager.has(reordered.sessionId) && sessionEvents(reordered).length === 0, 'same-window reorder ended task'); + check((await readOverlay(reorderId)).controlVisible, 'same-window reorder hid overlay'); + completed.push(reordered.sessionId); + const unrelated = await chrome.tabs.create({windowId: reordered.agentWindowId, url: url + '/unrelated', active: false}); + await chrome.tabs.move(unrelated.id, {windowId: user.id, index: -1}); + await sleep(150); + check(manager.has(reordered.sessionId) && sessionEvents(reordered).length === 0, 'unrelated tab move ended task'); + completed.push('unrelated-tab'); + + const returning = await start('programmatic-return', 0); + const returnId = await borrow(returning); + const returned = await handleTabReturn(manager, {session_id: returning.sessionId, tab_id: returnId}, {cdp}); + check(!returned.code, JSON.stringify(returned)); + await sleep(150); + check(manager.has(returning.sessionId) && sessionEvents(returning).length === 0, 'tab_return ended task'); + check((await chrome.tabs.get(returnId)).windowId === user.id, 'tab_return did not restore user window'); + completed.push(returning.sessionId); + + const stopping = await start('ordinary-stop'); + const stopBorrowed = await borrow(stopping), stopCreated = [...stopping.agentCreatedTabs]; + const stopped = await handleSessionStop(manager, {session_id: stopping.sessionId}, { + cdp, tabManagement: {tabs: chromeTabMutationApi}, tabsQuery: chromeTabsApi, + }); + check(!stopped.code && !stopped.return_failures, JSON.stringify(stopped)); + await sleep(150); + check(!manager.has(stopping.sessionId), 'ordinary stop retained session'); + check(sessionEvents(stopping).length === 0, 'ordinary stop emitted a user termination event'); + check((await chrome.tabs.get(stopBorrowed)).windowId === user.id, 'ordinary stop did not return borrow'); + for (const id of stopCreated) check(!(await chrome.tabs.get(id).catch(() => null)), 'ordinary stop retained agent-created tab'); + completed.push(stopping.sessionId); + return {completed, terminalEvents: events}; + } finally { + lifecycle.dispose(); groups.dispose(); await cdp.detachAll(); cdp.dispose(); + } +}; +`; + +describe.skipIf(!process.env.BSK_TAB_DETACH_CHROME)("tab detachment browser regression", () => { + it("releases the whole task after an unregistered cross-window move", async () => { + const directory = await mkdtemp(path.join(tmpdir(), "bsk-tab-detachment-")); + const server = createServer((_request, response) => { + response.setHeader("Content-Type", "text/html"); + response.end("Tab detachment fixture"); + }); + try { + const require = createRequire(import.meta.url); + const { build } = await import( + require.resolve("vite", { paths: [require.resolve("vitest")] }) + ); + for (const [name, source] of [ + ["entry", entry], + ["content", content], + ]) { + const file = path.join(directory, `${name}.js`); + await writeFile(file, source); + await build({ + configFile: false, + logLevel: "error", + define: { + __BSK_EXT_VERSION__: JSON.stringify("0.0.0"), + __BSK_DAEMON_WS_URL__: JSON.stringify("ws://127.0.0.1:1"), + }, + resolve: { + alias: { + "@": path.resolve("src"), + "@browser-skill/i18n": path.resolve("../../packages/i18n/src/index.ts"), + }, + }, + build: { + outDir: directory, + emptyOutDir: false, + lib: { + entry: file, + formats: ["iife"], + name: `Detachment${name}`, + fileName: () => `${name}.bundle.js`, + }, + }, + }); + } + await writeFile( + path.join(directory, "manifest.json"), + JSON.stringify({ + manifest_version: 3, + name: "Tab detachment regression", + version: "1.0", + background: { service_worker: "worker.js" }, + permissions: ["debugger", "tabs", "tabGroups", "storage", "webNavigation"], + host_permissions: ["http://127.0.0.1/*"], + content_scripts: [{ matches: ["http://127.0.0.1/*"], js: ["content.bundle.js"] }], + }), + ); + await writeFile( + path.join(directory, "worker.js"), + "chrome.runtime.onInstalled.addListener(() => {});", + ); + // Install diagnostics before the bundle executes. Keep this in a separate + // script because Manifest V3 forbids inline scripts in extension pages. + await writeFile( + path.join(directory, "bootstrap.js"), + `globalThis.bootstrapErrors = []; +window.addEventListener('error', event => { + globalThis.bootstrapErrors.push({ + kind: 'error', message: event.message, filename: event.filename, + line: event.lineno, column: event.colno, stack: event.error?.stack, + resource: event.target?.src, + }); +}, true); +window.addEventListener('unhandledrejection', event => { + globalThis.bootstrapErrors.push({kind: 'unhandledrejection', message: String(event.reason), stack: event.reason?.stack}); +});`, + ); + await writeFile( + path.join(directory, "regression.html"), + '', + ); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const url = `http://127.0.0.1:${(server.address() as { port: number }).port}`; + const { withChrome } = await import( + new URL( + "../../../../../evals/browser/cases/regression/snapshot-coordinates/chrome.mjs", + import.meta.url, + ).href + ); + await withChrome( + { + executable: process.env.BSK_TAB_DETACH_CHROME, + deviceScale: 1, + zoom: 1, + extensionPath: directory, + }, + async (send: Send) => { + let origin = ""; + await expect + .poll( + async () => { + const { targetInfos } = await send<{ + targetInfos: { type: string; url: string }[]; + }>("Target.getTargets"); + const worker = targetInfos.find( + (target) => target.type === "service_worker" && target.url.endsWith("/worker.js"), + ); + origin = worker ? worker.url.slice(0, -"/worker.js".length) : ""; + return origin; + }, + { timeout: 10000 }, + ) + .not.toBe(""); + const { targetId } = await send<{ targetId: string }>("Target.createTarget", { + url: `${origin}/regression.html`, + background: true, + }); + const { sessionId } = await send<{ sessionId: string }>("Target.attachToTarget", { + targetId, + flatten: true, + }); + const evaluate = async (expression: string) => { + const reply = await send<{ result: { value: unknown }; exceptionDetails?: unknown }>( + "Runtime.evaluate", + { expression, returnByValue: true }, + sessionId, + ); + expect(reply.exceptionDetails).toBeUndefined(); + return reply.result.value; + }; + try { + await expect + .poll(() => evaluate("typeof globalThis.run"), { timeout: 5000 }) + .toBe("function"); + } catch (error) { + const diagnostics = await evaluate(`({ + url: location.href, readyState: document.readyState, + run: typeof globalThis.run, errors: globalThis.bootstrapErrors, + scripts: [...document.scripts].map(script => script.src), + })`); + throw new Error(`Regression bootstrap failed: ${JSON.stringify(diagnostics)}`, { + cause: error, + }); + } + await evaluate( + `globalThis.run(${JSON.stringify(url)}).then(value=>globalThis.done={value},error=>globalThis.done={error:error.stack,phase:globalThis.phase});void 0`, + ); + await expect + .poll(() => evaluate("globalThis.done"), { timeout: 45000, interval: 100 }) + .toBeDefined(); + const done = (await evaluate("globalThis.done")) as { value?: unknown; error?: string }; + expect(done.error).toBeUndefined(); + console.log(JSON.stringify(done.value)); + if (process.env.BSK_TAB_DETACH_REPORT) { + await writeFile(process.env.BSK_TAB_DETACH_REPORT, JSON.stringify(done.value, null, 2)); + } + }, + ); + } finally { + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + await rm(directory, { recursive: true, force: true }); + } + }, 60000); +}); diff --git a/apps/extension/src/session-manager/__tests__/tab-detachment.test.ts b/apps/extension/src/session-manager/__tests__/tab-detachment.test.ts new file mode 100644 index 000000000..d268ff6fa --- /dev/null +++ b/apps/extension/src/session-manager/__tests__/tab-detachment.test.ts @@ -0,0 +1,191 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { handleSessionStop } from "@/tools/session"; +import type { TabMutationApi } from "@/tools/tabs"; +import { attachSessionEventHandler } from "../event-handler"; +import { SessionManager } from "../manager"; +import { expectedTabMoves } from "../tab-moves"; + +function event() { + const listeners = new Set<(...args: T) => void>(); + return { + addListener: (listener: (...args: T) => void) => listeners.add(listener), + removeListener: (listener: (...args: T) => void) => listeners.delete(listener), + emit: (...args: T) => { + for (const listener of listeners) listener(...args); + }, + }; +} + +afterEach(() => { + expectedTabMoves.clear(); + vi.restoreAllMocks(); +}); + +async function fixture() { + let nextWindow = 10; + const remove = vi.fn(); + const manager = new SessionManager({ + agentWindow: { + create: async () => ({ windowId: nextWindow++, initialTabIds: [] }), + ensureActiveTab: async (id) => id, + remove, + }, + }); + const task = await manager.start("task"); + task.borrowedTabs.set(20, { tabId: 20, originalWindowId: 90, originalIndex: 0 }); + task.observedTabs = new Set([30]); + const sibling = await manager.start("sibling"); + const windowEvents = event<[number]>(); + const tabEvents = { + onDetached: event<[number, chrome.tabs.TabDetachInfo]>(), + onAttached: event<[number, chrome.tabs.TabAttachInfo]>(), + onRemoved: event<[number, chrome.tabs.TabRemoveInfo]>(), + }; + const send = vi.fn(); + const resetAgentOverlays = vi.fn(async () => {}); + const detachSession = vi.fn(async () => {}); + const onSessionReleased = vi.fn(() => { + expect(manager.has("task")).toBe(false); + }); + const onSessionsChanged = vi.fn(); + const handler = attachSessionEventHandler({ + manager, + transport: { send }, + windowEvents, + tabEvents, + cdp: { detachSession }, + agentOverlayReset: { resetAgentOverlays }, + onSessionReleased, + onSessionsChanged, + }); + const moveOut = (id: number) => + tabEvents.onDetached.emit(id, { oldWindowId: 10, oldPosition: 0 }); + return { + manager, + task, + sibling, + remove, + windowEvents, + tabEvents, + send, + resetAgentOverlays, + detachSession, + onSessionReleased, + onSessionsChanged, + handler, + moveOut, + }; +} + +describe("moving a task page out", () => { + it.each([10, 20, 30])("releases the entire session immediately for owned tab %s", async (id) => { + const f = await fixture(); + f.moveOut(id); + expect(f.manager.get("task")).toBeNull(); + expect(f.manager.get("sibling")).toBe(f.sibling); + expect(f.remove).not.toHaveBeenCalled(); + expect(f.send).toHaveBeenCalledExactlyOnceWith({ + event: "session.ended", + payload: { session_id: "task", reason: "user_moved_tab", tab_id: id }, + }); + expect(f.onSessionReleased).toHaveBeenCalledExactlyOnceWith(f.task); + expect(f.detachSession).toHaveBeenCalledExactlyOnceWith("task"); + expect(f.resetAgentOverlays.mock.calls).toEqual([ + [10, "task"], + [20, "task"], + [30, "task"], + ]); + // A delayed daemon stop cannot close the now-user-owned window. + await f.manager.stop("task"); + expect(f.remove).not.toHaveBeenCalled(); + f.handler.dispose(); + }); + + it("does not stop for free tabs and reports a multi-tab/last-tab move once", async () => { + const f = await fixture(); + f.moveOut(40); + expect(f.manager.has("task")).toBe(true); + f.moveOut(10); + f.moveOut(20); + f.moveOut(30); + f.windowEvents.emit(10); + await Promise.resolve(); + expect(f.send).toHaveBeenCalledTimes(1); + expect(f.onSessionReleased).toHaveBeenCalledTimes(1); + expect(f.remove).not.toHaveBeenCalled(); + f.handler.dispose(); + }); + + it("waits for a registered move's destination without ending a normal return", async () => { + const f = await fixture(); + await expectedTabMoves.run({ tabId: 20, fromWindowId: 10, toWindowId: 90 }, async () => {}); + f.moveOut(20); + f.tabEvents.onAttached.emit(20, { newWindowId: 90, newPosition: 0 }); + await Promise.resolve(); + expect(f.manager.get("task")).toBe(f.task); + expect(f.send).not.toHaveBeenCalled(); + f.handler.dispose(); + }); + + it("ends on an unexpected destination even if a later detach listener releases ownership", async () => { + const f = await fixture(); + await expectedTabMoves.run({ tabId: 30, fromWindowId: 10, toWindowId: 90 }, async () => {}); + f.moveOut(30); + f.manager.releaseObservedTab(30); + f.tabEvents.onAttached.emit(30, { newWindowId: 80, newPosition: 0 }); + await Promise.resolve(); + expect(f.manager.has("task")).toBe(false); + expect(f.resetAgentOverlays).toHaveBeenCalledWith(30, "task"); + f.handler.dispose(); + }); + + it("never revives control when transport or renderer cleanup fails", async () => { + vi.spyOn(console, "debug").mockImplementation(() => {}); + const f = await fixture(); + f.send.mockImplementation(() => { + throw new Error("disconnected"); + }); + f.resetAgentOverlays.mockRejectedValue(new Error("no receiver")); + f.detachSession.mockRejectedValue(new Error("already detached")); + f.moveOut(20); + await Promise.resolve(); + expect(f.manager.has("task")).toBe(false); + expect(f.onSessionReleased).toHaveBeenCalledOnce(); + expect(f.resetAgentOverlays).toHaveBeenCalledTimes(3); + expect(f.remove).not.toHaveBeenCalled(); + f.handler.dispose(); + }); + + it.each([ + false, + true, + ])("does not finish an old close list after release, cancelled=%s", async (cancelled) => { + const f = await fixture(); + f.task.borrowedTabs.clear(); + f.task.agentCreatedTabs.add(40).add(50); + const abort = new AbortController(); + const removed: number[] = []; + const tabs = { + remove: vi.fn(async (id: number) => { + removed.push(id); + if (id === 10) { + f.moveOut(40); + if (cancelled) abort.abort(); + } + }), + }; + await handleSessionStop( + f.manager, + { session_id: "task" }, + { + signal: abort.signal, + tabManagement: { tabs: tabs as unknown as TabMutationApi }, + tabsQuery: { query: async () => [], get: vi.fn() }, + }, + ); + expect(removed).toEqual([10]); + expect(f.remove).not.toHaveBeenCalled(); + expect(f.manager.has("task")).toBe(false); + f.handler.dispose(); + }); +}); diff --git a/apps/extension/src/session-manager/__tests__/tab-moves.test.ts b/apps/extension/src/session-manager/__tests__/tab-moves.test.ts new file mode 100644 index 000000000..334998971 --- /dev/null +++ b/apps/extension/src/session-manager/__tests__/tab-moves.test.ts @@ -0,0 +1,426 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + handleTabBorrow, + handleTabCreate, + handleTabReturn, + type TabMutationApi, +} from "@/tools/tabs"; +import { SessionManager } from "../manager"; +import { ExpectedTabMoves, expectedTabMoves } from "../tab-moves"; + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (error: Error) => void; + const promise = new Promise((yes, no) => { + resolve = yes; + reject = no; + }); + return { promise, resolve, reject }; +} + +const movement = { tabId: 7, fromWindowId: 10, toWindowId: 20 }; +afterEach(() => expectedTabMoves.clear()); + +describe("expected tab moves", () => { + it("returns an immediate false for an unregistered tab or source window", async () => { + const moves = new ExpectedTabMoves(); + await moves.run(movement, async () => {}); + expect(moves.onDetached(8, 10)).toBe(false); + expect(moves.onDetached(7, 30)).toBe(false); + moves.clear(); + }); + + it("requires the actual destination and consumes only one detach", async () => { + const moves = new ExpectedTabMoves(); + const action = deferred(); + const running = moves.run(movement, () => action.promise); + const detached = moves.onDetached(7, 10); + expect(detached).toBeInstanceOf(Promise); + moves.onAttached(7, 20); + await expect(detached).resolves.toBe(true); + expect(moves.onDetached(7, 10)).toBe(false); + action.resolve(); + await running; + }); + + it("retains an operation when the API completes before browser events", async () => { + const moves = new ExpectedTabMoves(); + await moves.run(movement, async () => {}); + const detached = moves.onDetached(7, 10); + moves.onAttached(7, 20); + await expect(detached).resolves.toBe(true); + }); + + it("does not mistake a concurrent user move for the program's destination", async () => { + const moves = new ExpectedTabMoves(); + const action = deferred(); + const running = moves.run(movement, () => action.promise); + const userDetach = moves.onDetached(7, 10); + // The API can already report success for its later C -> B move. That + // cannot authorize the user's earlier A -> C detach still awaiting attach. + action.resolve(); + await running; + moves.onAttached(7, 30); + await expect(userDetach).resolves.toBe(false); + expect(moves.onDetached(7, 30)).toBe(false); + moves.onAttached(7, 20); + }); + + it("pairs consecutive detach/attach events instead of skipping unmatched moves", async () => { + const moves = new ExpectedTabMoves(); + expect(moves.onDetached(7, 30)).toBe(false); + await moves.run(movement, async () => {}); + const ownDetach = moves.onDetached(7, 10); + moves.onAttached(7, 10); + moves.onAttached(7, 20); + await expect(ownDetach).resolves.toBe(true); + }); + + it("invalidates unused operations when the tab moves from an unrelated origin", async () => { + const moves = new ExpectedTabMoves(); + await moves.run(movement, async () => {}); + expect(moves.onDetached(7, 20)).toBe(false); + moves.onAttached(7, 10); + expect(moves.onDetached(7, 10)).toBe(false); + }); + + it("clears failed calls without waiting for browser events", async () => { + const moves = new ExpectedTabMoves(); + await expect( + moves.run(movement, async () => { + throw new Error("move failed"); + }), + ).rejects.toThrow("move failed"); + expect(moves.onDetached(7, 10)).toBe(false); + }); + + it("releases a waiting detach when its API call fails", async () => { + const moves = new ExpectedTabMoves(); + const action = deferred(); + const running = moves.run(movement, () => action.promise); + const detached = moves.onDetached(7, 10); + action.reject(new Error("move failed")); + await expect(running).rejects.toThrow("move failed"); + await expect(detached).resolves.toBe(false); + }); + + it("does not register same-window reordering or ambiguous overlapping operations", async () => { + const moves = new ExpectedTabMoves(); + await moves.run({ ...movement, toWindowId: 10 }, async () => {}); + expect(moves.onDetached(7, 10)).toBe(false); + moves.onAttached(7, 20); + await moves.run(movement, async () => {}); + await moves.run(movement, async () => {}); + expect(moves.onDetached(7, 10)).toBe(false); + }); + + it("releases pending classification when the tab closes", async () => { + const moves = new ExpectedTabMoves(); + await moves.run(movement, async () => {}); + const detached = moves.onDetached(7, 10); + moves.forget(7); + await expect(detached).resolves.toBe(false); + expect(moves.onDetached(7, 10)).toBe(false); + }); +}); + +async function toolFixture() { + const manager = new SessionManager({ + agentWindow: { + create: async () => ({ windowId: 10, initialTabIds: [1] }), + ensureActiveTab: async () => 1, + remove: async () => {}, + }, + }); + const task = await manager.start("one"); + const tab = { id: 7, windowId: 20, index: 3, url: "https://example.com" } as chrome.tabs.Tab; + const classifications: (boolean | Promise)[] = []; + let afterMove = () => {}; + const api: TabMutationApi = { + create: vi.fn(), + remove: vi.fn(), + update: async () => ({ ...tab }), + get: async () => ({ ...tab }), + move: vi.fn(async (_id, properties) => { + const oldWindow = tab.windowId; + classifications.push(expectedTabMoves.onDetached(tab.id!, oldWindow)); + tab.windowId = properties.windowId!; + tab.index = properties.index; + expectedTabMoves.onAttached(tab.id!, tab.windowId); + afterMove(); + return { ...tab }; + }), + }; + const deps = { + tabs: api, + approveBorrow: async () => true, + windows: { + get: async (id: number) => ({ id }) as chrome.windows.Window, + getLastFocused: vi.fn(), + create: vi.fn(), + remove: vi.fn(), + }, + agentOverlayReset: { resetAgentOverlays: async () => {} }, + }; + return { + manager, + task, + tab, + deps, + classifications, + afterMove: (fn: () => void) => (afterMove = fn), + }; +} + +describe("tab tool move registration", () => { + it.each([ + "borrow", + "return", + ] as const)("does not move a tab when %s is cancelled before native dispatch", async (kind) => { + const f = await toolFixture(); + const abort = new AbortController(); + if (kind === "return") { + f.tab.windowId = 10; + f.task.borrowedTabs.set(7, { tabId: 7, originalWindowId: 20, originalIndex: 3 }); + } + const handler = kind === "borrow" ? handleTabBorrow : handleTabReturn; + expect( + await handler( + f.manager, + { session_id: "one", tab_id: 7 }, + { + ...f.deps, + signal: abort.signal, + cdp: { + runBrowserMutation: async (operation) => { + abort.abort(); + return operation(); + }, + }, + }, + ), + ).toMatchObject({ code: "cancelled" }); + expect(f.deps.tabs.move).not.toHaveBeenCalled(); + expect(expectedTabMoves.onDetached(7, f.tab.windowId)).toBe(false); + }); + + it("recognizes ordinary borrowing and returning as separate program moves", async () => { + const f = await toolFixture(); + expect( + await handleTabBorrow(f.manager, { session_id: "one", tab_id: 7 }, f.deps), + ).toMatchObject({ agent_window_id: 10 }); + expect( + await handleTabReturn(f.manager, { session_id: "one", tab_id: 7 }, f.deps), + ).toMatchObject({ returned_to_window_id: 20 }); + expect(await Promise.all(f.classifications)).toEqual([true, true]); + expect(f.manager.get("one")).toBe(f.task); + }); + + it("registers cancellation rollback independently of the original borrow", async () => { + const f = await toolFixture(); + const abort = new AbortController(); + f.afterMove(() => abort.abort()); + expect( + await handleTabBorrow( + f.manager, + { session_id: "one", tab_id: 7 }, + { ...f.deps, signal: abort.signal }, + ), + ).toMatchObject({ code: "cancelled" }); + expect(f.tab.windowId).toBe(20); + expect(await Promise.all(f.classifications)).toEqual([true, true]); + }); + + it("registers the fallback return destination after the original window is gone", async () => { + const f = await toolFixture(); + f.tab.windowId = 10; + f.task.borrowedTabs.set(7, { tabId: 7, originalWindowId: 20, originalIndex: 3 }); + f.deps.windows.get = async () => { + throw new Error("window closed"); + }; + f.deps.windows.getLastFocused.mockResolvedValue({ id: 30, type: "normal", incognito: false }); + expect( + await handleTabReturn(f.manager, { session_id: "one", tab_id: 7 }, f.deps), + ).toMatchObject({ returned_to_window_id: 30, fallback: true }); + expect(await Promise.all(f.classifications)).toEqual([true]); + }); +}); + +describe("released session transactions", () => { + it("preserves a fallback window released to the user while its creation completes", async () => { + const f = await toolFixture(); + f.tab.windowId = 10; + f.task.borrowedTabs.set(7, { tabId: 7, originalWindowId: 20, originalIndex: 3 }); + f.deps.windows.get = async () => { + throw new Error("window gone"); + }; + f.deps.windows.getLastFocused.mockResolvedValue({ id: 10 }); + f.deps.windows.create.mockImplementation(async () => { + f.manager.release("one"); + return { id: 30 }; + }); + expect( + await handleTabReturn(f.manager, { session_id: "one", tab_id: 7 }, f.deps), + ).toMatchObject({ code: "cancelled" }); + expect(f.deps.windows.remove).not.toHaveBeenCalled(); + expect(f.deps.tabs.move).not.toHaveBeenCalled(); + }); + + it.each([ + false, + true, + ])("preserves a borrowed page moved by the user during setup (abort: %s)", async (shouldAbort) => { + const f = await toolFixture(); + const abort = new AbortController(); + const result = await handleTabBorrow( + f.manager, + { session_id: "one", tab_id: 7 }, + { + ...f.deps, + signal: abort.signal, + cdp: { + acquireBackgroundExecution: async () => { + f.tab.windowId = 30; + f.manager.release("one"); + if (shouldAbort) abort.abort(); + }, + }, + }, + ); + expect(result).toMatchObject({ code: "cancelled" }); + expect(f.deps.tabs.move).toHaveBeenCalledTimes(1); + expect(f.tab.windowId).toBe(30); + }); + + it("does not roll back an uncommitted borrow after its session is released", async () => { + const f = await toolFixture(); + const abort = new AbortController(); + f.afterMove(() => { + f.tab.windowId = 30; + f.manager.release("one"); + abort.abort(); + }); + expect( + await handleTabBorrow( + f.manager, + { session_id: "one", tab_id: 7 }, + { + ...f.deps, + signal: abort.signal, + }, + ), + ).toMatchObject({ code: "cancelled" }); + expect(f.deps.tabs.move).toHaveBeenCalledTimes(1); + expect(f.tab.windowId).toBe(30); + }); + + it("stops cancellation rollback if the session is released during its source lookup", async () => { + const f = await toolFixture(); + const abort = new AbortController(); + f.afterMove(() => abort.abort()); + f.deps.tabs.get = async () => { + if (abort.signal.aborted) { + f.tab.windowId = 30; + f.manager.release("one"); + } + return { ...f.tab }; + }; + expect( + await handleTabBorrow( + f.manager, + { session_id: "one", tab_id: 7 }, + { + ...f.deps, + signal: abort.signal, + }, + ), + ).toMatchObject({ code: "cancelled" }); + expect(f.deps.tabs.move).toHaveBeenCalledTimes(1); + expect(f.tab.windowId).toBe(30); + }); + + it("stops setup rollback if the session is released while resolving the return window", async () => { + const f = await toolFixture(); + f.deps.windows.get = async (id) => { + f.tab.windowId = 30; + f.manager.release("one"); + return { id } as chrome.windows.Window; + }; + expect( + await handleTabBorrow( + f.manager, + { session_id: "one", tab_id: 7 }, + { + ...f.deps, + cdp: { + acquireBackgroundExecution: async () => { + throw new Error("setup failed"); + }, + }, + }, + ), + ).toMatchObject({ code: "cancelled" }); + expect(f.deps.tabs.move).toHaveBeenCalledTimes(1); + expect(f.tab.windowId).toBe(30); + }); + + it("preserves a newly created page when its response arrives after session release", async () => { + const f = await toolFixture(); + const abort = new AbortController(); + f.deps.tabs.create = async () => { + f.tab.windowId = 30; + f.manager.release("one"); + abort.abort(); + return { ...f.tab }; + }; + expect( + await handleTabCreate( + f.manager, + { session_id: "one" }, + { + ...f.deps, + signal: abort.signal, + }, + ), + ).toMatchObject({ code: "cancelled" }); + expect(f.deps.tabs.remove).not.toHaveBeenCalled(); + expect(f.task.agentCreatedTabs.has(7)).toBe(false); + }); + + it.each([ + "setup", + "release", + ] as const)("preserves a created page released during background %s cleanup", async (stage) => { + const f = await toolFixture(); + const abort = new AbortController(); + f.deps.tabs.create = async () => ({ ...f.tab, windowId: 10 }); + const relinquish = () => { + f.tab.windowId = 30; + f.manager.release("one"); + abort.abort(); + }; + const releaseSessionTab = vi.fn(async () => { + if (stage === "release") relinquish(); + }); + expect( + await handleTabCreate( + f.manager, + { session_id: "one", url: "https://example.com" }, + { + ...f.deps, + signal: abort.signal, + cdp: { + acquireBackgroundExecution: async () => { + if (stage === "setup") relinquish(); + throw new Error("setup failed"); + }, + releaseSessionTab, + }, + }, + ), + ).toMatchObject({ code: "cancelled" }); + expect(releaseSessionTab).toHaveBeenCalledWith("one", 7); + expect(f.deps.tabs.remove).not.toHaveBeenCalled(); + expect(f.tab.windowId).toBe(30); + }); +}); diff --git a/apps/extension/src/session-manager/event-handler.ts b/apps/extension/src/session-manager/event-handler.ts index a62c0dcc7..e3ab5d95c 100644 --- a/apps/extension/src/session-manager/event-handler.ts +++ b/apps/extension/src/session-manager/event-handler.ts @@ -1,6 +1,8 @@ +import { type AgentOverlayResetApi, chromeAgentOverlayResetApi } from "@/tools/tabs"; import type { Transport } from "@/transport/transport"; import type { EventFrame } from "@/transport/types"; -import type { SessionManager } from "./manager"; +import { isAgentControlledTab, type SessionContext, type SessionManager } from "./manager"; +import { expectedTabMoves } from "./tab-moves"; /** * Listener interface that mirrors `chrome.windows.onRemoved` so vitest @@ -13,10 +15,20 @@ export interface WindowRemovedListener { export interface SessionEventHandlerOptions { manager: SessionManager; - transport: Transport; + transport: Pick; windowEvents?: WindowRemovedListener; + tabEvents?: { + [K in "onDetached" | "onAttached" | "onRemoved"]: Pick< + (typeof chrome.tabs)[K], + "addListener" | "removeListener" + >; + }; + agentOverlayReset?: AgentOverlayResetApi; + /** Runs synchronously after ownership is removed, before renderer/CDP cleanup. */ + onSessionReleased?: (session: SessionContext) => void; cdp?: { detachSession(sessionId: string): Promise; + releaseSessionTab?(sessionId: string, tabId: number): Promise; }; /** * Invoked after a user-closed Agent Window has been removed from the @@ -47,6 +59,70 @@ export function attachSessionEventHandler(options: SessionEventHandlerOptions): } { const { manager, transport, onSessionsChanged } = options; const events = options.windowEvents ?? chromeWindowEvents(); + const tabs = options.tabEvents ?? globalThis.chrome?.tabs; + + const onDetached = (tabId: number, info: chrome.tabs.TabDetachInfo): void => { + const expected = expectedTabMoves.onDetached(tabId, info.oldWindowId); + const ctx = manager.findByWindowId(info.oldWindowId); + if (!ctx || !isAgentControlledTab(ctx, tabId)) return; + // Capture before cooperation's detach handler can release this page's claim. + const ownedTabs = new Set([ + ...ctx.agentCreatedTabs, + ...ctx.borrowedTabs.keys(), + ...(ctx.observedTabs ?? []), + ]); + const end = () => { + if (manager.get(ctx.sessionId) !== ctx) return; + manager.release(ctx.sessionId); + ctx.refStore.clear(); + // Publish the reason before cancellation replies can overtake it. + try { + transport.send({ + event: "session.ended", + payload: { session_id: ctx.sessionId, reason: "user_moved_tab", tab_id: tabId }, + }); + } catch (error) { + console.debug("[bsk] could not report moved task tab", error); + } + try { + options.onSessionReleased?.(ctx); + } catch (error) { + console.debug("[bsk] session release callback failed", error); + } + try { + onSessionsChanged?.(); + } catch (error) { + console.debug("[bsk] released session display refresh failed", error); + } + // A user move releases the entire task. Never close or return any of its + // pages: more tabs from the same group may still be crossing windows. + const reset = options.agentOverlayReset ?? chromeAgentOverlayResetApi; + for (const id of ownedTabs) { + void reset.resetAgentOverlays(id, ctx.sessionId).catch((error) => { + console.debug("[bsk] released tab overlay reset failed", error); + }); + // A first reader can still be attaching before it records its CDP + // owner. Release the known task pages too, using the driver's pending + // attachment fence and its protection for another session's owner. + void options.cdp?.releaseSessionTab?.(ctx.sessionId, id).catch((error) => { + console.debug("[bsk] released tab CDP cleanup failed", error); + }); + } + void options.cdp?.detachSession(ctx.sessionId).catch((error) => { + console.debug("[bsk] released session CDP cleanup failed", error); + }); + }; + if (typeof expected === "boolean") { + if (!expected) end(); + } else { + void expected.then((movedByUs) => { + if (!movedByUs) end(); + }); + } + }; + const onAttached = (tabId: number, info: chrome.tabs.TabAttachInfo) => + expectedTabMoves.onAttached(tabId, info.newWindowId); + const onTabRemoved = (tabId: number) => expectedTabMoves.forget(tabId); const onRemoved = (windowId: number): void => { const ctx = manager.findByWindowId(windowId); @@ -96,7 +172,15 @@ export function attachSessionEventHandler(options: SessionEventHandlerOptions): }; events.addListener(onRemoved); + tabs?.onDetached.addListener(onDetached); + tabs?.onAttached.addListener(onAttached); + tabs?.onRemoved.addListener(onTabRemoved); return { - dispose: () => events.removeListener(onRemoved), + dispose: () => { + events.removeListener(onRemoved); + tabs?.onDetached.removeListener(onDetached); + tabs?.onAttached.removeListener(onAttached); + tabs?.onRemoved.removeListener(onTabRemoved); + }, }; } diff --git a/apps/extension/src/session-manager/manager.ts b/apps/extension/src/session-manager/manager.ts index e0747c747..a464ace5a 100644 --- a/apps/extension/src/session-manager/manager.ts +++ b/apps/extension/src/session-manager/manager.ts @@ -336,8 +336,19 @@ export class SessionManager { if (!options.dropOnly) { await this.agentWindow.remove(ctx.agentWindowId); } + if (this.sessions.get(sessionId) === ctx) this.release(sessionId); + return ctx; + } + + /** End ownership synchronously, without moving or closing any browser page. */ + release(sessionId: string): SessionContext | null { + const ctx = this.sessions.get(sessionId); + if (!ctx) return null; this.sessions.delete(sessionId); this.windowIndex.delete(ctx.agentWindowId); + for (const [tabId, owner] of this.borrowReservations) { + if (owner === sessionId) this.borrowReservations.delete(tabId); + } return ctx; } diff --git a/apps/extension/src/session-manager/tab-moves.ts b/apps/extension/src/session-manager/tab-moves.ts new file mode 100644 index 000000000..169e5b365 --- /dev/null +++ b/apps/extension/src/session-manager/tab-moves.ts @@ -0,0 +1,84 @@ +export interface ExpectedTabMove { + tabId: number; + fromWindowId: number; + toWindowId: number; +} + +interface MoveOperation extends ExpectedTabMove { + detached: boolean; + resolve?: (expected: boolean) => void; +} + +/** Match one browser detach/attach pair to one explicitly issued tab move. */ +export class ExpectedTabMoves { + private readonly operations = new Map>(); + private readonly detached = new Map(); + + async run(move: ExpectedTabMove, action: () => Promise): Promise { + // Reordering in the same window does not generate detach/attach events. + if (move.fromWindowId === move.toWindowId) return action(); + const operation: MoveOperation = { ...move, detached: false }; + const operations = this.operations.get(move.tabId) ?? new Set(); + operations.add(operation); + this.operations.set(move.tabId, operations); + try { + return await action(); + } catch (error) { + this.finish(operation, false); + throw error; + } + // A successful API response may precede Chrome's browser events. Keep this + // specific operation until its detach/attach pair arrives, never a timer. + } + + onDetached(tabId: number, oldWindowId: number): boolean | Promise { + const candidates = [...(this.operations.get(tabId) ?? [])].filter( + (operation) => !operation.detached && operation.fromWindowId === oldWindowId, + ); + // Concurrent moves with the same origin cannot be attributed safely. + const operation = candidates.length === 1 ? candidates[0] : undefined; + if (!operation) { + // An unrelated move makes any unused snapshot for this tab stale. Do + // not let an operation that produced no events authorize a later drag. + for (const candidate of this.operations.get(tabId) ?? []) { + if (!candidate.detached) this.finish(candidate, false); + } + } + const queue = this.detached.get(tabId) ?? []; + queue.push(operation); + this.detached.set(tabId, queue); + if (!operation) return false; + operation.detached = true; + return new Promise((resolve) => { + operation.resolve = resolve; + }); + } + + onAttached(tabId: number, newWindowId: number): void { + const queue = this.detached.get(tabId); + if (!queue) return; + const operation = queue.shift(); + if (!queue.length) this.detached.delete(tabId); + if (operation) this.finish(operation, operation.toWindowId === newWindowId); + } + + forget(tabId: number): void { + for (const operation of this.operations.get(tabId) ?? []) this.finish(operation, false); + this.detached.delete(tabId); + } + + clear(): void { + for (const tabId of this.operations.keys()) this.forget(tabId); + this.detached.clear(); + } + + private finish(operation: MoveOperation, expected: boolean): void { + const operations = this.operations.get(operation.tabId); + operations?.delete(operation); + if (!operations?.size) this.operations.delete(operation.tabId); + operation.resolve?.(expected); + operation.resolve = undefined; + } +} + +export const expectedTabMoves = new ExpectedTabMoves(); diff --git a/apps/extension/src/tools/__tests__/dispatcher.test.ts b/apps/extension/src/tools/__tests__/dispatcher.test.ts index 4068baeb7..006c1e632 100644 --- a/apps/extension/src/tools/__tests__/dispatcher.test.ts +++ b/apps/extension/src/tools/__tests__/dispatcher.test.ts @@ -1415,7 +1415,7 @@ describe("ToolDispatcher", () => { return { id: 7, windowId: 200, index: 4 }; }); vi.stubGlobal("chrome", { - tabs: { sendMessage, move }, + tabs: { sendMessage, move, get: vi.fn(async () => ({ id: 7, windowId: 100 })) }, windows: { get: vi.fn(async () => ({ id: 200 })) }, }); const { transport, sent, deliver } = fakeTransport(); @@ -1709,6 +1709,49 @@ describe("ToolDispatcher", () => { expect(sessions.list()).toEqual([]); }); + it("cancels only a released session and prevents its pending stop from closing preserved pages", async () => { + const { transport, sent, deliver } = fakeTransport(); + let windowId = 10; + const remove = vi.fn(async () => {}); + const sessions = new SessionManager({ + agentWindow: { + create: async () => ({ windowId: windowId++, initialTabIds: [] }), + ensureActiveTab: async (id) => id, + remove, + }, + }); + await sessions.start("one"); + await sessions.start("two"); + let finish!: () => void; + const pending = new Promise((resolve) => { + finish = resolve; + }); + const detachSession = vi.fn(() => pending); + const dispatcher = new ToolDispatcher({ + transport, + sessions, + cdp: { detachSession } as unknown as TestDispatcherCdp, + }); + dispatcher.start(); + deliver({ id: "one-stop", method: "tool.session_stop", params: { session_id: "one" } }); + deliver({ id: "two-stop", method: "tool.session_stop", params: { session_id: "two" } }); + await vi.waitFor(() => expect(detachSession).toHaveBeenCalledTimes(2)); + sessions.release("one"); + dispatcher.cancelSession("one"); + expect(dispatcher.inflightAbortControllers.get("one-stop")?.signal.aborted).toBe(true); + expect(dispatcher.inflightAbortControllers.get("two-stop")?.signal.aborted).toBe(false); + finish(); + await vi.waitFor(() => expect(sent).toHaveLength(2)); + expect(sent).toContainEqual({ + id: "one-stop", + error: expect.objectContaining({ code: "cancelled" }), + }); + expect(sent).toContainEqual({ id: "two-stop", result: {} }); + expect(remove).toHaveBeenCalledExactlyOnceWith(11); + expect(dispatcher.inflightAbortControllers.size).toBe(0); + dispatcher.stop(); + }); + it("cancel for an unknown rpc_id replies with cancelled=false", async () => { const { transport, sent, deliver } = fakeTransport(); const sessions = new SessionManager({ diff --git a/apps/extension/src/tools/__tests__/tabs.test.ts b/apps/extension/src/tools/__tests__/tabs.test.ts index 6d4a7d755..16d101103 100644 --- a/apps/extension/src/tools/__tests__/tabs.test.ts +++ b/apps/extension/src/tools/__tests__/tabs.test.ts @@ -625,7 +625,7 @@ describe("handleTabBorrow", () => { }); }); - it("returns a moved tab if its session ends before the borrow commits", async () => { + it("does not issue a compensating move after the borrowing session ends", async () => { const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); await sm.start("aa11"); const state: FakeTabState = { @@ -645,7 +645,7 @@ describe("handleTabBorrow", () => { { tabs: api, approveBorrow: async () => true }, ), ).toMatchObject({ code: "cancelled" }); - expect(spies.move).toHaveBeenLastCalledWith(7, { windowId: 200, index: 4 }); + expect(spies.move).toHaveBeenCalledExactlyOnceWith(7, { windowId: 100, index: -1 }); }); it.each([ diff --git a/apps/extension/src/tools/dispatcher.ts b/apps/extension/src/tools/dispatcher.ts index ab660af39..de3bffdf2 100644 --- a/apps/extension/src/tools/dispatcher.ts +++ b/apps/extension/src/tools/dispatcher.ts @@ -218,6 +218,7 @@ export class ToolDispatcher { * controllers. Made public for tests. */ readonly inflightAbortControllers = new Map(); + private readonly requestSessions = new Map(); constructor(deps: DispatcherDeps) { this.cooperation = deps.cooperation; @@ -314,11 +315,19 @@ export class ToolDispatcher { } } this.inflightAbortControllers.clear(); + this.requestSessions.clear(); const exports = this.screenshotExports; this.screenshotExports = new ScreenshotExports((id) => this.sessions.has(id)); void exports.dispose(); } + /** Revoke in-flight work locally; do not wait for the daemon's round trip. */ + cancelSession(sessionId: string): void { + for (const [rpcId, owner] of this.requestSessions) { + if (owner === sessionId) this.inflightAbortControllers.get(rpcId)?.abort(); + } + } + private async dispatch(msg: ProtocolFrame): Promise { if (!isRequestFrame(msg)) return; const req = msg as RequestFrame; @@ -361,6 +370,7 @@ export class ToolDispatcher { const operationParams = req.params as | { session_id?: string; _operation_id?: string } | undefined; + if (operationParams?.session_id) this.requestSessions.set(req.id, operationParams.session_id); let body: ResponseFrame; let startedSession: string | null = null; let debugTicket: DebugTicket | undefined; @@ -464,6 +474,7 @@ export class ToolDispatcher { if (startsSession) this.pendingSessionStarts -= 1; this.debug?.after(debugTicket, "operation failed"); this.inflightAbortControllers.delete(req.id); + this.requestSessions.delete(req.id); if ( operationParams?.session_id && this.activeDialogOperations.get(operationParams.session_id)?.request.id === req.id diff --git a/apps/extension/src/tools/session.ts b/apps/extension/src/tools/session.ts index 411030f7f..c14a632ea 100644 --- a/apps/extension/src/tools/session.ts +++ b/apps/extension/src/tools/session.ts @@ -240,6 +240,7 @@ async function stopSession( const returnFailures: SessionStopResult["return_failures"] = []; const borrowedIds = Array.from(ctx.borrowedTabs.keys()); for (const tabId of borrowedIds) { + if (manager.get(ctx.sessionId) !== ctx) return {}; // A close event may have removed this borrow while another tab returned. if (!ctx.borrowedTabs.has(tabId)) continue; try { @@ -247,6 +248,7 @@ async function stopSession( ...(deps.tabManagement ?? {}), cdp: deps.cdp ?? deps.tabManagement?.cdp, signal: deps.signal, + isSessionLive: () => manager.get(ctx.sessionId) === ctx, isAgentWindowId: deps.tabManagement?.isAgentWindowId ?? ((windowId: number) => manager.findByWindowId(windowId) !== null), @@ -318,6 +320,12 @@ async function stopSession( } return manager.withExpectedWindowClose(ctx, async () => { + const interrupted = (): SessionStopResult | RpcError | undefined => { + if (deps.signal?.aborted) return { code: "cancelled", message: "session_stop aborted" }; + // Another lifecycle event may have already released every page. A normal + // last-tab close also removes the session; both paths need no more work. + if (manager.get(ctx.sessionId) !== ctx) return result; + }; // Step 4: close every tab explicitly created by the agent, including the // home tab. `tabsApi` is a // TabMutationApi (remove only); `queryApi` is a separate read-only @@ -336,9 +344,13 @@ async function stopSession( // Close each agent-created tab that still exists. const agentCreatedTabIds = Array.from(ctx.agentCreatedTabs); for (const tabId of agentCreatedTabIds) { + const ended = interrupted(); + if (ended) return ended; try { if (closeLastTabWithWindow && queryApi) { const liveTabs = await queryApi.query({ windowId: ctx.agentWindowId }); + const ended = interrupted(); + if (ended) return ended; if (liveTabs.length === 1 && liveTabs[0]?.id === tabId) { deferredAgentTabs.add(tabId); continue; @@ -353,11 +365,16 @@ async function stopSession( } } + const ended = interrupted(); + if (ended) return ended; + // Step 5: decide whether to release (keep) the window or close it. let shouldRelease = (ctx.observedTabs?.size ?? 0) > 0; if (queryApi) { try { const liveWindowTabs = await queryApi.query({ windowId: ctx.agentWindowId }); + const ended = interrupted(); + if (ended) return ended; // Only genuine *user* tabs count toward keeping the window open. // An agent tab that failed to close in Step 4 may still be present // here; if we counted it as a reason to release (dropOnly), the @@ -380,6 +397,8 @@ async function stopSession( // A user opened a tab after Step 4 deferred the final agent tab. // It is now safe to remove that tab without closing the window. for (const tab of leakedAgentTabs) { + const ended = interrupted(); + if (ended) return ended; try { await tabsApi.remove(tab.id); ctx.agentCreatedTabs.delete(tab.id); @@ -416,6 +435,9 @@ async function stopSession( } } + const finished = interrupted(); + if (finished) return finished; + if (shouldRelease) { // Keep the window + its user tabs; only drop the session binding. const observedTabIds = Array.from(ctx.observedTabs ?? []); diff --git a/apps/extension/src/tools/shared.ts b/apps/extension/src/tools/shared.ts index 8bc7f8f1d..91f9c5095 100644 --- a/apps/extension/src/tools/shared.ts +++ b/apps/extension/src/tools/shared.ts @@ -82,7 +82,7 @@ export interface CdpRunner { guard: CdpDispatchGuard, ): Promise; detach?(tabId: number, expectedAttachmentId?: string): Promise; - getFrameGraph?(tabId: number): Promise; + getFrameGraph?(tabId: number, signal?: AbortSignal): Promise; getAttachmentId?(tabId: number): string | undefined; ensureAttachedToUrl?(tabId: number, expectedUrl: string | undefined): Promise; acquireBackgroundExecution?(sessionId: string, tabId: number): Promise; diff --git a/apps/extension/src/tools/tabs.ts b/apps/extension/src/tools/tabs.ts index 458f2c649..2c97c73cc 100644 --- a/apps/extension/src/tools/tabs.ts +++ b/apps/extension/src/tools/tabs.ts @@ -15,6 +15,7 @@ import { type SessionManager, } from "@/session-manager/manager"; import { beginTabCreation } from "@/session-manager/tab-creation"; +import { expectedTabMoves } from "@/session-manager/tab-moves"; import { detectBrowserMeta } from "@/transport/handshake"; import type { RpcError } from "@/transport/types"; import { rpcError } from "./errors"; @@ -156,6 +157,35 @@ export const chromeTabMutationApi: TabMutationApi = { move: (id, p) => chrome.tabs.move(id, p), }; +async function moveExpectedTab( + tabs: TabMutationApi, + tabId: number, + properties: chrome.tabs.MoveProperties, + signal?: AbortSignal, + isLive?: () => boolean, + mutate?: CdpRunner["runBrowserMutation"], +): Promise { + const current = await tabs.get(tabId); + const move = () => { + if (signal?.aborted || isLive?.() === false) { + const error = new Error("Tab move aborted before browser mutation"); + error.name = "AbortError"; + throw error; + } + return expectedTabMoves.run( + { + tabId, + fromWindowId: current.windowId, + toWindowId: properties.windowId ?? current.windowId, + }, + () => tabs.move(tabId, properties), + ); + }; + // Finish browser reads before the final authority check. Registration and + // native dispatch must then run together without another asynchronous gap. + return mutate ? mutate(move) : move(); +} + /** * Subset of `chrome.windows` we use for tab_return's fallback path * (original window has been closed; pick another normal window or @@ -297,6 +327,8 @@ export interface TabManagementDeps { windows?: ChromeWindowsApi; /** Abort hook (M10 will wire the full chain). */ signal?: AbortSignal; + /** Released sessions must not move or close pages during transaction cleanup. */ + isSessionLive?: () => boolean; /** Browser-policy approver. Missing wiring must never silently approve a borrow. */ approveBorrow?: BorrowConfirmationApprover; /** Clears Agent-scoped overlays after a borrowed tab is returned. */ @@ -337,8 +369,14 @@ function getAgentOverlayResetApi(deps: TabManagementDeps): AgentOverlayResetApi return deps.agentOverlayReset ?? chromeAgentOverlayResetApi; } -function aborted(signal: AbortSignal | undefined, tool: string): RpcError | null { - return signal?.aborted ? { code: "cancelled", message: `${tool} aborted` } : null; +function aborted( + signal: AbortSignal | undefined, + tool: string, + isLive?: () => boolean, +): RpcError | null { + return signal?.aborted || isLive?.() === false + ? { code: "cancelled", message: `${tool} aborted` } + : null; } function validatePositiveInt(name: string, value: unknown): RpcError | null { @@ -410,8 +448,14 @@ async function createAndClaimTab( ): Promise { let tab: chrome.tabs.Tab; try { - tab = await mutateBrowser(deps, () => getTabsApi(deps).create(createProps)); + tab = await mutateBrowser(deps, () => { + if (aborted(deps.signal, "tab_create", deps.isSessionLive)) + throw new Error("tab_create aborted before browser mutation"); + return getTabsApi(deps).create(createProps); + }); } catch (err) { + const cancelled = aborted(deps.signal, "tab_create", deps.isSessionLive); + if (cancelled) return cancelled; return { code: "protocol_error", message: err instanceof Error ? err.message : String(err), @@ -420,6 +464,9 @@ async function createAndClaimTab( if (typeof tab.id !== "number") { return { code: "protocol_error", message: "chrome.tabs.create returned no tab id" }; } + if (deps.isSessionLive?.() === false) { + return { code: "cancelled", message: "tab_create session ended" }; + } // Claim the concrete id returned by Chrome. Ownership never depends on // matching this request to an asynchronous onCreated event. ctx.agentCreatedTabs.add(tab.id); @@ -459,7 +506,8 @@ export async function handleTabCreate( const ctxOrErr = lookupSession(manager, params, "tab_create"); if (isRpcError(ctxOrErr)) return ctxOrErr; const ctx = ctxOrErr; - const ab = aborted(deps.signal, "tab_create"); + const isLive = () => manager.get(ctx.sessionId) === ctx && deps.isSessionLive?.() !== false; + const ab = aborted(deps.signal, "tab_create", isLive); if (ab) return ab; const paramErr = validateTabCreateParams(params); @@ -472,24 +520,22 @@ export async function handleTabCreate( const prepare = deps.cdp?.acquireBackgroundExecution && !cdpBlockedUrlReason(props.url); const tab = await createTabAndCleanup( ctx, - deps, + { ...deps, isSessionLive: isLive }, prepare ? { ...props, url: "about:blank" } : props, ); if (isRpcError(tab)) return tab; + if (!isLive()) return { code: "cancelled", message: "tab_create session ended" }; if (prepare) { try { await deps.cdp!.acquireBackgroundExecution!(ctx.sessionId, tab.id); - if ( - deps.signal?.aborted || - manager.get(ctx.sessionId) !== ctx || - !isAgentControlledTab(ctx, tab.id) - ) { + if (deps.signal?.aborted || !isLive() || !isAgentControlledTab(ctx, tab.id)) { throw new Error("Tab creation cancelled during background execution setup"); } // Preserve create's no-load-wait contract. The destination script cannot run // before the blank document's execution policy has been acknowledged. if (props.url !== "about:blank") await getTabsApi(deps).update(tab.id, { url: props.url }); - if (deps.signal?.aborted) throw new Error("Tab creation cancelled during navigation"); + if (deps.signal?.aborted || !isLive()) + throw new Error("Tab creation cancelled during navigation"); } catch (error) { const cleanupErrors: string[] = []; try { @@ -497,6 +543,7 @@ export async function handleTabCreate( } catch (cleanupError) { cleanupErrors.push(`release: ${describeError(cleanupError)}`); } + if (!isLive()) return { code: "cancelled", message: "tab_create session ended" }; try { await getTabsApi(deps).remove(tab.id); ctx.agentCreatedTabs.delete(tab.id); @@ -829,22 +876,39 @@ async function moveTabForBorrow( agentWindowId: number, originalWindowId: number, originalIndex: number, + isLive: () => boolean, signal?: AbortSignal, mutate?: CdpRunner["runBrowserMutation"], ): Promise { try { - const move = () => tabsApi.move(tabId, { windowId: agentWindowId, index: -1 }); - await (mutate ? mutate(move) : move()); + await moveExpectedTab( + tabsApi, + tabId, + { windowId: agentWindowId, index: -1 }, + signal, + isLive, + mutate, + ); } catch (err) { + const cancelled = aborted(signal, "tab_borrow", isLive); + if (cancelled) return cancelled; return { code: "cdp_failed", message: `tab_borrow: chrome.tabs.move failed: ${err instanceof Error ? err.message : String(err)}`, }; } + if (!isLive()) return { code: "cancelled", message: "tab_borrow session ended" }; if (aborted(signal, "tab_borrow")) { try { - await tabsApi.move(tabId, { windowId: originalWindowId, index: originalIndex }); + await moveExpectedTab( + tabsApi, + tabId, + { windowId: originalWindowId, index: originalIndex }, + undefined, + isLive, + ); } catch (rollbackErr) { + if (!isLive()) return { code: "cancelled", message: "tab_borrow session ended" }; return rpcError( "protocol_error", "borrow_outcome_unknown", @@ -896,6 +960,7 @@ async function executeBorrowCore( p.ctx.agentWindowId, originalWindowId, originalIndex, + () => p.manager.get(p.ctx.sessionId) === p.ctx, p.signal, p.mutate, ); @@ -1003,12 +1068,19 @@ export async function handleTabBorrow( }); committed = true; } catch (err) { + if (manager.get(ctx.sessionId) !== ctx) + return { code: "cancelled", message: "tab_borrow session ended" }; try { - await tabsApi.move(params.tab_id, { - windowId: coreResult.originalWindowId, - index: coreResult.originalIndex, - }); + await moveExpectedTab( + tabsApi, + params.tab_id, + { windowId: coreResult.originalWindowId, index: coreResult.originalIndex }, + undefined, + () => manager.get(ctx.sessionId) === ctx, + ); } catch (rollbackError) { + if (manager.get(ctx.sessionId) !== ctx) + return { code: "cancelled", message: "tab_borrow session ended" }; return rpcError( "protocol_error", "borrow_outcome_unknown", @@ -1026,6 +1098,9 @@ export async function handleTabBorrow( // user tab switches; selecting a tab does not focus its window. try { const tab = await tabsApi.get(params.tab_id); + if (deps.signal?.aborted || manager.get(ctx.sessionId) !== ctx) { + throw new Error("Borrow cancelled during background execution setup"); + } if (!cdpBlockedUrlReason(tab.url)) { await deps.cdp?.acquireBackgroundExecution?.(ctx.sessionId, params.tab_id); } @@ -1037,11 +1112,14 @@ export async function handleTabBorrow( throw new Error("Borrow cancelled during background execution setup"); } } catch (error) { + if (manager.get(ctx.sessionId) !== ctx) + return { code: "cancelled", message: "tab_borrow session ended" }; const rollback = await returnBorrowedTab(ctx, params.tab_id, { ...deps, // Returning an already moved tab is this transaction's rollback, not new input. cdp: { releaseSessionTab: deps.cdp?.releaseSessionTab?.bind(deps.cdp) }, signal: undefined, + isSessionLive: () => manager.get(ctx.sessionId) === ctx, isAgentWindowId: (id) => manager.findByWindowId(id) !== null, }); if (isRpcError(rollback)) return rollback; @@ -1089,11 +1167,12 @@ async function chooseFallbackWindow( isAgentWindowId: (windowId: number) => boolean, signal?: AbortSignal, mutate?: CdpRunner["runBrowserMutation"], + isLive?: () => boolean, ): Promise { let lastFocusedError: unknown; try { const last = await windowsApi.getLastFocused({ windowTypes: ["normal"] }); - const cancelled = aborted(signal, "tab_return"); + const cancelled = aborted(signal, "tab_return", isLive); if (cancelled) return cancelled; const lastId = typeof last?.id === "number" ? last.id : null; // Never relocate a user tab into *any* session's Agent Window: this @@ -1117,12 +1196,15 @@ async function chooseFallbackWindow( lastFocusedError = err; } - const cancelled = aborted(signal, "tab_return"); + const cancelled = aborted(signal, "tab_return", isLive); if (cancelled) return cancelled; try { - const create = () => - windowsApi.create({ url: NEW_TAB_DEFAULT_URL, focused: false, incognito: false }); + const create = () => { + if (aborted(signal, "tab_return", isLive)) + throw new Error("tab_return aborted before fallback window creation"); + return windowsApi.create({ url: NEW_TAB_DEFAULT_URL, focused: false, incognito: false }); + }; const created = await (mutate ? mutate(create) : create()); if (typeof created?.id !== "number") { return { @@ -1130,7 +1212,8 @@ async function chooseFallbackWindow( message: "tab_return: failed to create fallback window", }; } - if (aborted(signal, "tab_return")) { + if (isLive?.() === false) return { code: "cancelled", message: "tab_return session ended" }; + if (aborted(signal, "tab_return", isLive)) { try { await windowsApi.remove(created.id); } catch (cleanupErr) { @@ -1145,6 +1228,8 @@ async function chooseFallbackWindow( } return { windowId: created.id, index: 0, created: true }; } catch (err) { + const cancelled = aborted(signal, "tab_return", isLive); + if (cancelled) return cancelled; const suffix = lastFocusedError ? ` (after getLastFocused failed: ${describeError(lastFocusedError)})` : ""; @@ -1159,8 +1244,9 @@ async function cleanupUnusedFallbackWindow( windowsApi: ChromeWindowsApi, target: FallbackWindowTarget, reason: string, + isLive?: () => boolean, ): Promise { - if (!target.created) return null; + if (!target.created || isLive?.() === false) return null; try { await windowsApi.remove(target.windowId); return null; @@ -1224,7 +1310,7 @@ async function returnBorrowedTabCore( const tabsApi = getTabsApi(deps); const windowsApi = getWindowsApi(deps); const isAgentWindowId = getIsAgentWindowId(deps); - const alreadyCancelled = aborted(deps.signal, "tab_return"); + const alreadyCancelled = aborted(deps.signal, "tab_return", deps.isSessionLive); if (alreadyCancelled) return alreadyCancelled; let targetWindowId = entry.originalWindowId; @@ -1240,7 +1326,7 @@ async function returnBorrowedTabCore( console.debug("[bsk tab_return] original window gone, falling back", err); originalAlive = false; } - const cancelledAfterLookup = aborted(deps.signal, "tab_return"); + const cancelledAfterLookup = aborted(deps.signal, "tab_return", deps.isSessionLive); if (cancelledAfterLookup) return cancelledAfterLookup; if (!originalAlive) { fallback = true; @@ -1250,6 +1336,7 @@ async function returnBorrowedTabCore( isAgentWindowId, deps.signal, deps.cdp?.runBrowserMutation, + deps.isSessionLive, ); if ("code" in target) return target; fallbackTarget = target; @@ -1257,13 +1344,14 @@ async function returnBorrowedTabCore( targetIndex = target.index; } - const cancelledBeforeMove = aborted(deps.signal, "tab_return"); + const cancelledBeforeMove = aborted(deps.signal, "tab_return", deps.isSessionLive); if (cancelledBeforeMove) { if (fallbackTarget) { const cleanupError = await cleanupUnusedFallbackWindow( windowsApi, fallbackTarget, "tab_return aborted before moving the borrowed tab", + deps.isSessionLive, ); if (cleanupError) return cleanupError; } @@ -1271,11 +1359,13 @@ async function returnBorrowedTabCore( } try { - const moved = await mutateBrowser(deps, () => - tabsApi.move(tabId, { - windowId: targetWindowId, - index: targetIndex, - }), + const moved = await moveExpectedTab( + tabsApi, + tabId, + { windowId: targetWindowId, index: targetIndex }, + deps.signal, + deps.isSessionLive, + deps.cdp?.runBrowserMutation, ); const movedTab = Array.isArray(moved) ? moved[0] : moved; const finalIndex = typeof movedTab?.index === "number" ? movedTab.index : targetIndex; @@ -1292,10 +1382,11 @@ async function returnBorrowedTabCore( windowsApi, fallbackTarget, `tab_return could not move tab ${tabId}`, + deps.isSessionLive, ); if (cleanupError) return cleanupError; } - const cancelledAfterMoveFailure = aborted(deps.signal, "tab_return"); + const cancelledAfterMoveFailure = aborted(deps.signal, "tab_return", deps.isSessionLive); if (cancelledAfterMoveFailure) return cancelledAfterMoveFailure; if (!fallback) { const target = await chooseFallbackWindow( @@ -1304,6 +1395,7 @@ async function returnBorrowedTabCore( isAgentWindowId, deps.signal, deps.cdp?.runBrowserMutation, + deps.isSessionLive, ); if ("code" in target) { return { @@ -1311,21 +1403,24 @@ async function returnBorrowedTabCore( message: `tab_return: chrome.tabs.move failed: ${describeError(err)}; fallback failed: ${target.message}`, }; } - const cancelledBeforeFallbackMove = aborted(deps.signal, "tab_return"); + const cancelledBeforeFallbackMove = aborted(deps.signal, "tab_return", deps.isSessionLive); if (cancelledBeforeFallbackMove) { const cleanupError = await cleanupUnusedFallbackWindow( windowsApi, target, "tab_return aborted before fallback move", + deps.isSessionLive, ); return cleanupError ?? cancelledBeforeFallbackMove; } try { - const moved = await mutateBrowser(deps, () => - tabsApi.move(tabId, { - windowId: target.windowId, - index: target.index, - }), + const moved = await moveExpectedTab( + tabsApi, + tabId, + { windowId: target.windowId, index: target.index }, + deps.signal, + deps.isSessionLive, + deps.cdp?.runBrowserMutation, ); const movedTab = Array.isArray(moved) ? moved[0] : moved; const finalIndex = typeof movedTab?.index === "number" ? movedTab.index : target.index; @@ -1341,8 +1436,11 @@ async function returnBorrowedTabCore( windowsApi, target, `tab_return fallback move for tab ${tabId} failed`, + deps.isSessionLive, ); if (cleanupError) return cleanupError; + const cancelled = aborted(deps.signal, "tab_return", deps.isSessionLive); + if (cancelled) return cancelled; return { code: "cdp_failed", message: `tab_return: chrome.tabs.move failed: ${describeError(err)}; fallback move failed: ${describeError(fallbackErr)}`, @@ -1378,6 +1476,7 @@ export async function handleTabReturn( await deps.beforeReturn?.(ctx.sessionId, params.tab_id); const outcome = await returnBorrowedTab(ctx, params.tab_id, { ...deps, + isSessionLive: () => manager.get(ctx.sessionId) === ctx && deps.isSessionLive?.() !== false, isAgentWindowId: deps.isAgentWindowId ?? ((windowId) => manager.findByWindowId(windowId) !== null), }); diff --git a/apps/extension/src/tools/vom/capture-abort.ts b/apps/extension/src/tools/vom/capture-abort.ts index f9ea5118d..17337f89f 100644 --- a/apps/extension/src/tools/vom/capture-abort.ts +++ b/apps/extension/src/tools/vom/capture-abort.ts @@ -17,9 +17,12 @@ export function isAbortError(error: unknown): boolean { /** Errors after which the capture pipeline must stop instead of falling back. * A timed-out renderer read is terminal too: AX/geometry fallback must not keep * issuing reads after the renderer stopped answering. It is still reported as - * a browser failure, not as a caller abort. */ + * a browser failure, not as a caller abort. A replaced debugger attachment is + * terminal too: fallback must not capture against the new connection. */ export function isCaptureTerminalError(error: unknown): boolean { - return ["AbortError", "CdpReadTimeoutError"].includes(errorName(error)); + return ["AbortError", "CdpReadTimeoutError", "CdpAttachmentChangedError"].includes( + errorName(error), + ); } export function throwIfAborted(signal: AbortSignal | undefined): void { diff --git a/apps/extension/src/tools/vom/capture-coordinator.ts b/apps/extension/src/tools/vom/capture-coordinator.ts index adcd8acec..01111c548 100644 --- a/apps/extension/src/tools/vom/capture-coordinator.ts +++ b/apps/extension/src/tools/vom/capture-coordinator.ts @@ -171,7 +171,7 @@ async function collectObservationFacts( throwCaptureAborted(signal); let graph: CdpFrameGraph | undefined; try { - graph = await cdp.getFrameGraph?.(tabId); + graph = await cdp.getFrameGraph?.(tabId, signal); } catch (error) { forgetPendingAccessibility(axEnabled, error); if (isCaptureAbort(error)) throw error; diff --git a/apps/extension/src/transport/__tests__/handshake.test.ts b/apps/extension/src/transport/__tests__/handshake.test.ts index ba4fbc6b5..952bf02e8 100644 --- a/apps/extension/src/transport/__tests__/handshake.test.ts +++ b/apps/extension/src/transport/__tests__/handshake.test.ts @@ -67,8 +67,8 @@ function deferredFakeTransport(): { transport: Transport; emit: (frame: Protocol describe("performHandshake", () => { it("advertises the protocol compatibility boundary", () => { - expect(PROTOCOL_VERSION).toBe("1.5"); - expect(MIN_COMPATIBLE_PROTOCOL).toBe("1.5"); + expect(PROTOCOL_VERSION).toBe("1.6"); + expect(MIN_COMPATIBLE_PROTOCOL).toBe("1.6"); }); it("sends system.handshake with identity and both compat fields", async () => { @@ -174,7 +174,7 @@ describe("performHandshake", () => { const abort = new AbortController(); const rejection = { code: "version_too_old" as const, - message: "local protocol 1.3 is below peer min_compatible_protocol 1.5", + message: "local protocol 1.3 is below peer min_compatible_protocol 1.6", data: { daemon_version: "0.3.3", daemon_protocol_version: "1.3", diff --git a/apps/extension/src/transport/handshake.ts b/apps/extension/src/transport/handshake.ts index 4e419fd04..11234a2ce 100644 --- a/apps/extension/src/transport/handshake.ts +++ b/apps/extension/src/transport/handshake.ts @@ -8,7 +8,7 @@ import type { RpcError, } from "./types"; -export const PROTOCOL_VERSION = "1.5"; +export const PROTOCOL_VERSION = "1.6"; /** * Extension semver, injected at build time from `package.json` via * Vite's `define` (see `wxt.config.ts` and `vitest.config.ts`). @@ -19,9 +19,9 @@ export const EXTENSION_VERSION: string = * Lowest **protocol** version this extension accepts (e.g. `"1.0"`). * This is the daemon floor; the daemon can independently accept older extensions. */ -// Native dialogs suspend RPCs. An older daemon cannot return the pending event -// to a sequential agent, so refuse it before controlling any browser tabs. -export const MIN_COMPATIBLE_PROTOCOL = "1.5"; +// Moving a controlled tab ends the session. Older daemons cannot consume the +// terminal event and would leave the session active, so require this protocol. +export const MIN_COMPATIBLE_PROTOCOL = "1.6"; /** * **Deprecated** — legacy app-semver floor for wire compat with old * daemons. New code sends `"0.0.0"`; compat decisions ignore this. diff --git a/apps/extension/src/transport/types.ts b/apps/extension/src/transport/types.ts index e729e5c13..ffb0ed81e 100644 --- a/apps/extension/src/transport/types.ts +++ b/apps/extension/src/transport/types.ts @@ -24,6 +24,7 @@ export type ErrorCode = /** Stable `RpcError.data.reason` values for CLI hint selection. */ export type RpcErrorReason = + | "user_moved_tab" | "cooperation_sync_required" | "control_authority_required" | "extract_params_invalid" @@ -135,6 +136,16 @@ export interface EventFrame { payload?: unknown; } +/** Terminal user action: pages stay open, but the session cannot resume. */ +export interface SessionEndedEvent extends EventFrame { + event: "session.ended"; + payload: { + session_id: string; + reason: "user_moved_tab"; + tab_id: number; + }; +} + export type ProtocolFrame = RequestFrame | ResponseFrame | EventFrame; export function isRequestFrame(f: ProtocolFrame): f is RequestFrame { diff --git a/crates/bsk-cli/src/cli/render_error.rs b/crates/bsk-cli/src/cli/render_error.rs index 290a73c23..d2672033c 100644 --- a/crates/bsk-cli/src/cli/render_error.rs +++ b/crates/bsk-cli/src/cli/render_error.rs @@ -79,6 +79,7 @@ pub mod reason { pub const SESSION_BUSY: &str = crate::rpc_reason::SESSION_BUSY; pub const SESSION_IDLE_TIMEOUT: &str = crate::rpc_reason::SESSION_IDLE_TIMEOUT; pub const SESSION_WINDOW_CLOSED: &str = crate::rpc_reason::SESSION_WINDOW_CLOSED; + pub const SESSION_USER_MOVED_TAB: &str = crate::rpc_reason::SESSION_USER_MOVED_TAB; pub const BROWSER_DISCONNECTED: &str = crate::rpc_reason::BROWSER_DISCONNECTED; pub const OBSERVE_IDENTITY_RELOAD_LOOP: &str = crate::rpc_reason::OBSERVE_IDENTITY_RELOAD_LOOP; pub const BORROW_AGENT_WINDOW_TAB: &str = crate::rpc_reason::BORROW_AGENT_WINDOW_TAB; @@ -302,6 +303,13 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend ), exit_code: base.exit_code, }, + (_, reason::SESSION_USER_MOVED_TAB) => RenderInfo { + summary: "the session ended because the user moved a controlled tab out of its Agent Window", + hint: Some( + "all pages were preserved and browser control ended; this session cannot resume. Ask the user before starting a new session or borrowing a page again", + ), + exit_code: base.exit_code, + }, (ErrorCode::NotFound, reason::SESSION_WINDOW_CLOSED) => RenderInfo { summary: "the session ended because the user closed the Agent Window", hint: Some( @@ -1093,6 +1101,29 @@ mod tests { /// a generic not-found, and the anti-debugging / borrow-target /// reasons get non-retry copy instead of the `cdp_failed` / /// `invalid_params` defaults. + #[test] + fn user_moved_tab_is_terminal_for_running_and_later_commands() { + for code in [ + ErrorCode::UserAborted, + ErrorCode::NotFound, + ErrorCode::Timeout, + ErrorCode::Cancelled, + ErrorCode::CdpFailed, + ErrorCode::ProtocolError, + ] { + let rendered = info_for_error( + code, + Some(&serde_json::json!({"reason": crate::rpc_reason::SESSION_USER_MOVED_TAB})), + ); + assert!(rendered.summary.contains("user moved a controlled tab")); + let hint = rendered.hint.unwrap(); + assert!(hint.contains("all pages were preserved")); + assert!(hint.contains("cannot resume")); + assert!(hint.contains("Ask the user")); + assert_eq!(rendered.exit_code, info_for(code).exit_code); + } + } + #[test] fn ended_sessions_and_recurring_failures_get_reasoned_copy() { let idle = info_for_error( diff --git a/crates/bsk-cli/src/daemon/inflight.rs b/crates/bsk-cli/src/daemon/inflight.rs index 020f4538a..a32a70346 100644 --- a/crates/bsk-cli/src/daemon/inflight.rs +++ b/crates/bsk-cli/src/daemon/inflight.rs @@ -86,6 +86,8 @@ pub fn __set_promote_delay_for_tests(delay: Duration) { pub enum CancelReason { Cancelled, UserAborted, + /// Terminal user intervention; the session cannot resume. + UserMovedTab, } /// Mutable state inside [`ToolInflightEntry`]. Behind a `Mutex` so @@ -419,6 +421,14 @@ impl ToolInflightRegistry { /// cancel; `ws_rpc_id == None` means the worker will short-circuit /// in its next pre-flight. pub fn cancel_session(&self, sid: &SessionId) -> Vec { + self.cancel_session_with_reason(sid, CancelReason::UserAborted) + } + + pub(super) fn cancel_session_with_reason( + &self, + sid: &SessionId, + reason: CancelReason, + ) -> Vec { // Snapshot the entries we want to cancel under the registry // lock, then trip each one outside the registry lock so a // concurrent unregister (driven by Drop on InflightGuard) can @@ -438,7 +448,7 @@ impl ToolInflightRegistry { }; to_cancel .into_iter() - .map(|entry| entry.cancel_and_snapshot(CancelReason::UserAborted)) + .map(|entry| entry.cancel_and_snapshot(reason)) .collect() } diff --git a/crates/bsk-cli/src/daemon/queue.rs b/crates/bsk-cli/src/daemon/queue.rs index 81b3babf4..99476e467 100644 --- a/crates/bsk-cli/src/daemon/queue.rs +++ b/crates/bsk-cli/src/daemon/queue.rs @@ -141,6 +141,25 @@ pub enum DispatchError { } impl DispatchError { + fn with_session_end_reason(self, entry: Option<&ToolInflightEntry>) -> Self { + if entry.and_then(ToolInflightEntry::cancel_reason) + != Some(super::inflight::CancelReason::UserMovedTab) + { + return self; + } + // Apply the terminal cause when returning to the caller, including when + // the outer wait expires before the worker finishes cancellation cleanup. + let mut error = self.into_rpc(); + let data = error.data.get_or_insert_with(|| serde_json::json!({})); + if data.is_null() { + *data = serde_json::json!({}); + } else if !data.is_object() { + *data = serde_json::json!({ "details": data.take() }); + } + data["reason"] = serde_json::json!(crate::rpc_reason::SESSION_USER_MOVED_TAB); + Self::Rpc(error) + } + /// Convert this dispatch outcome into a structured [`RpcError`] /// suitable for sending back over the IPC line. pub fn into_rpc(self) -> RpcError { @@ -370,7 +389,7 @@ impl ToolQueueRegistry { params, timeout, false, - inflight, + inflight.clone(), None, CANCEL_CLEANUP_TIMEOUT, ) @@ -380,7 +399,7 @@ impl ToolQueueRegistry { // while it is running, session.stop observes SessionBusy and retries // on a later sweep rather than interrupting the tool. self.sessions.touch(sid); - outcome + outcome.map_err(|error| error.with_session_end_reason(inflight.as_deref())) } pub fn activity(&self, sid: &SessionId) -> Result { @@ -462,10 +481,11 @@ impl ToolQueueRegistry { cancel_cleanup_timeout: CANCEL_CLEANUP_TIMEOUT, session_id: sid.clone(), }; - match forward_one(sid, &self.browsers, &self.sessions, &job).await { - Ok(v) => Ok(v), - Err(err) => Err(DispatchError::Rpc(err)), - } + forward_one(sid, &self.browsers, &self.sessions, &job) + .await + .map_err(|error| { + DispatchError::Rpc(error).with_session_end_reason(job.inflight.as_deref()) + }) } /// Stop accepting new jobs for `sid`, enqueue one final control RPC, @@ -1333,6 +1353,17 @@ fn cancelled_error(entry: Option<&ToolInflightEntry>, default_msg: &str) -> RpcE ErrorCode::UserAborted, "tool dispatch interrupted by user".to_string(), ), + Some(CancelReason::UserMovedTab) => { + return RpcError { + code: ErrorCode::UserAborted, + message: + "session ended because the user moved a controlled tab out of its Agent Window" + .into(), + data: Some( + serde_json::json!({ "reason": crate::rpc_reason::SESSION_USER_MOVED_TAB }), + ), + }; + } Some(CancelReason::Cancelled) | None => (ErrorCode::Cancelled, default_msg.to_string()), }; RpcError { @@ -1583,6 +1614,21 @@ mod forward_one_cancel_reason_tests { assert_eq!(err.message, "tool dispatch interrupted by user"); } + #[test] + fn user_moved_tab_cancellation_retains_terminal_reason() { + let reg = Arc::new(ToolInflightRegistry::new()); + let sid = SessionId("ended".into()); + let guard = reg.register("move".into(), sid.clone()).unwrap(); + reg.cancel_session_with_reason(&sid, CancelReason::UserMovedTab); + let error = cancelled_error(Some(&guard.entry()), "fallback"); + assert_eq!(error.code, ErrorCode::UserAborted); + assert_eq!( + error.data.unwrap()["reason"], + crate::rpc_reason::SESSION_USER_MOVED_TAB + ); + assert!(error.message.contains("session ended")); + } + #[tokio::test] async fn pre_flight_cancel_with_cancelled_reason_yields_cancelled_code() { let reg = Arc::new(ToolInflightRegistry::new()); diff --git a/crates/bsk-cli/src/daemon/state.rs b/crates/bsk-cli/src/daemon/state.rs index 056fc39b1..26494d57a 100644 --- a/crates/bsk-cli/src/daemon/state.rs +++ b/crates/bsk-cli/src/daemon/state.rs @@ -17,7 +17,7 @@ use super::start::DaemonConfig; use super::ws::WsHandle; pub const DAEMON_VERSION: &str = env!("CARGO_PKG_VERSION"); -pub const PROTOCOL_VERSION: &str = "1.5"; +pub const PROTOCOL_VERSION: &str = "1.6"; /// Base wire compatibility. New interaction semantics are checked per operation. pub const MIN_COMPATIBLE_PROTOCOL: &str = "1.0"; /// Legacy app-semver floor used only when `HandshakeResult.min_compatible_peer` diff --git a/crates/bsk-cli/src/daemon/ws.rs b/crates/bsk-cli/src/daemon/ws.rs index eda738084..24945e41e 100644 --- a/crates/bsk-cli/src/daemon/ws.rs +++ b/crates/bsk-cli/src/daemon/ws.rs @@ -577,6 +577,9 @@ async fn handle_inbound_text(state: &Arc, client: &Arc { handle_session_window_closed(state, &client.id, &ev.payload); } + bsk_protocol::EventKind::SessionEnded => { + handle_session_ended(state, &client.id, &ev.payload); + } bsk_protocol::EventKind::SessionUserInterrupt => { handle_session_user_interrupt(state, &client.id, &ev.payload); } @@ -697,15 +700,7 @@ fn handle_session_window_closed( ); return; } - if super::sessions::forget_session( - &state.sessions, - &state.tool_queues, - &state.session_interrupts, - &session_id, - Some(crate::rpc_reason::SESSION_WINDOW_CLOSED), - ) { - state.transfers.release_session(&session_id.0); - state.dialog_operations.remove_session(&session_id.0); + if forget_browser_session(state, &session_id, crate::rpc_reason::SESSION_WINDOW_CLOSED) { info!(session = %session_id, "session removed: user closed Agent Window"); } else { debug!(session = %session_id, "session.window_closed for unknown session id"); @@ -721,6 +716,79 @@ fn handle_session_window_closed( } } +/// Logical termination is independent of whether the browser window stays open. +fn forget_browser_session( + state: &Arc, + session_id: &super::sessions::SessionId, + reason: &'static str, +) -> bool { + let removed = super::sessions::forget_session( + &state.sessions, + &state.tool_queues, + &state.session_interrupts, + session_id, + Some(reason), + ); + if removed { + state.transfers.release_session(&session_id.0); + state.dialog_operations.remove_session(&session_id.0); + } + removed +} + +fn handle_session_ended(state: &Arc, sender: &BrowserId, payload: &serde_json::Value) { + #[derive(serde::Deserialize)] + struct Ended { + session_id: String, + reason: String, + tab_id: u64, + } + let Ok(ended) = serde_json::from_value::(payload.clone()) else { + warn!("invalid session.ended payload"); + return; + }; + if ended.session_id.is_empty() + || ended.reason != crate::rpc_reason::SESSION_USER_MOVED_TAB + || ended.tab_id == 0 + { + warn!("invalid session.ended reason or resource identity"); + return; + } + let sid = super::sessions::SessionId(ended.session_id); + let Some(session) = state.sessions.get(&sid) else { + // Duplicates and late events must not cancel work for an unverified id. + debug!(session = %sid, "session.ended for unknown session id"); + return; + }; + if session.browser_id != *sender { + warn!(session = %sid, %sender, "ignoring session.ended from a browser that does not own this session"); + return; + } + + // Record the terminal cause on queued and forwarded calls before dropping + // the queue. This is not the resumable, one-shot user-interrupt marker. + cancel_session_tools(state, &sid, super::inflight::CancelReason::UserMovedTab); + if forget_browser_session(state, &sid, crate::rpc_reason::SESSION_USER_MOVED_TAB) { + info!(session = %sid, tab_id = ended.tab_id, "session ended: user moved a controlled tab"); + } +} + +fn cancel_session_tools( + state: &Arc, + sid: &super::sessions::SessionId, + reason: super::inflight::CancelReason, +) { + let snapshots = state.tool_inflight.cancel_session_with_reason(sid, reason); + for snap in snapshots { + if let (Some(browser_id), Some(ws_rpc_id)) = (snap.browser_id, snap.ws_rpc_id) + && let Err(err) = + super::cancel_forward::forward_cancel_to_browser(state, &browser_id, &ws_rpc_id) + { + warn!(%browser_id, %ws_rpc_id, %err, "failed to forward session cancellation to extension"); + } + } +} + fn extract_session_id(payload: &serde_json::Value) -> Option { payload .get("session_id") @@ -976,6 +1044,185 @@ mod session_user_interrupt_tests { ); } + fn user_moved_tab_payload(sid: &SessionId) -> serde_json::Value { + serde_json::json!({ "session_id": sid.0, "reason": "user_moved_tab", "tab_id": 42 }) + } + + #[tokio::test] + async fn session_ended_cancels_running_and_queued_tools_and_retains_reason() { + use crate::daemon::browsers::{Liveness, next_browser_generation}; + use crate::daemon::inflight::PromoteOutcome; + use bsk_protocol::{ErrorCode, Method}; + use std::sync::Mutex; + + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let (tx, mut rx) = mpsc::unbounded_channel(); + let browser = Arc::new(BrowserClient { + id: owner.clone(), + browser_name: "test".into(), + browser_version: "1".into(), + extension_version: "1".into(), + extension_protocol_version: "1".into(), + label: "test".into(), + sink: BrowserSink { tx }, + pending: Mutex::new(Pending::default()), + generation: next_browser_generation(), + connected_at_ms: 0, + version_skew: false, + liveness: Liveness::default(), + }); + state.browsers.insert(Arc::clone(&browser)); + let sid = state.sessions.reserve_id(owner.clone(), 8, || 0).unwrap(); + state.tool_queues.spawn(sid.clone()); + let running = state + .tool_inflight + .register("running".into(), sid.clone()) + .unwrap(); + let queued = state + .tool_inflight + .register("queued".into(), sid.clone()) + .unwrap(); + let unrelated = state + .tool_inflight + .register("other".into(), SessionId("other".into())) + .unwrap(); + let queues = Arc::clone(&state.tool_queues); + let current_sid = sid.clone(); + let current_entry = running.entry(); + let command = tokio::spawn(async move { + queues + .dispatch( + ¤t_sid, + Method::ToolTabList, + serde_json::json!({ "session_id": current_sid.0 }), + Duration::from_secs(5), + Some(current_entry), + ) + .await + }); + let Frame::Request(request) = tokio::time::timeout(Duration::from_secs(1), rx.recv()) + .await + .unwrap() + .unwrap() + else { + panic!("tool request expected"); + }; + assert_eq!(request.method, Method::ToolTabList); + + handle_session_ended(&state, &owner, &user_moved_tab_payload(&sid)); + assert_eq!( + running.entry().cancel_reason(), + Some(CancelReason::UserMovedTab) + ); + assert_eq!( + queued.entry().cancel_reason(), + Some(CancelReason::UserMovedTab) + ); + assert!(!unrelated.entry().is_cancelled()); + assert_eq!( + queued + .entry() + .promote_to_forwarded_with(owner.clone(), "never".into(), || panic!( + "queued work must not reach Chrome" + )), + PromoteOutcome::Cancelled + ); + assert!(state.sessions.get(&sid).is_none()); + assert!(!state.tool_queues.is_accepting(&sid)); + assert!(!state.session_interrupts.try_consume(&sid)); + let ended = state.sessions.end_info(&sid).unwrap(); + assert_eq!(ended.reason, crate::rpc_reason::SESSION_USER_MOVED_TAB); + + let Frame::Request(cancel) = rx.recv().await.unwrap() else { + panic!("cancel expected") + }; + assert_eq!(cancel.method, Method::Cancel); + assert_eq!(cancel.params.unwrap()["rpc_id"], request.id); + browser.pending.lock().unwrap().resolve(ResponseFrame { + id: request.id, + body: ResponseBody::Err(RpcError { + code: ErrorCode::Cancelled, + message: "stopped".into(), + data: None, + }), + }); + let error = command.await.unwrap().unwrap_err().into_rpc(); + assert_eq!(error.code, ErrorCode::UserAborted); + assert_eq!( + error.data.unwrap()["reason"], + crate::rpc_reason::SESSION_USER_MOVED_TAB + ); + + let later = state + .tool_queues + .dispatch( + &sid, + Method::ToolTabList, + serde_json::json!({ "session_id": sid.0 }), + Duration::from_secs(1), + None, + ) + .await + .unwrap_err() + .into_rpc(); + assert_eq!(later.code, ErrorCode::NotFound); + assert_eq!( + later.data.unwrap()["reason"], + crate::rpc_reason::SESSION_USER_MOVED_TAB + ); + handle_session_ended(&state, &owner, &user_moved_tab_payload(&sid)); + assert_eq!( + state.sessions.end_info(&sid).unwrap().ended_unix_ms, + ended.ended_unix_ms + ); + assert!(state.sessions.get(&sid).is_none()); + } + + #[test] + fn session_ended_rejects_foreign_browser_and_invalid_payloads() { + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let sid = state.sessions.reserve_id(owner.clone(), 8, || 0).unwrap(); + let guard = state + .tool_inflight + .register("pending".into(), sid.clone()) + .unwrap(); + handle_session_ended( + &state, + &BrowserId("foreign".into()), + &user_moved_tab_payload(&sid), + ); + for payload in [ + serde_json::json!({ "session_id": sid.0, "reason": "unknown", "tab_id": 42 }), + serde_json::json!({ "session_id": sid.0, "reason": "user_moved_tab", "tab_id": 0 }), + serde_json::json!({ "session_id": sid.0, "reason": "user_moved_tab" }), + ] { + handle_session_ended(&state, &owner, &payload); + } + assert!(state.sessions.get(&sid).is_some()); + assert!(state.sessions.end_info(&sid).is_none()); + assert!(!guard.entry().is_cancelled()); + assert!(!state.session_interrupts.try_consume(&sid)); + } + + #[test] + fn session_ended_does_not_cancel_an_unknown_session() { + let state = test_only_daemon_state(); + let sid = SessionId("unknown".into()); + let guard = state + .tool_inflight + .register("pending".into(), sid.clone()) + .unwrap(); + handle_session_ended( + &state, + &BrowserId("sender".into()), + &user_moved_tab_payload(&sid), + ); + assert!(!guard.entry().is_cancelled()); + assert!(state.sessions.end_info(&sid).is_none()); + } + fn test_only_daemon_state() -> std::sync::Arc { use crate::daemon::start::DaemonConfig; std::sync::Arc::new(DaemonState::new(DaemonConfig::new(0))) diff --git a/crates/bsk-cli/src/lib.rs b/crates/bsk-cli/src/lib.rs index 0b92ad30e..0282ec95f 100644 --- a/crates/bsk-cli/src/lib.rs +++ b/crates/bsk-cli/src/lib.rs @@ -10,6 +10,7 @@ pub mod rpc_reason { /// render why it vanished instead of a generic not-found. pub const SESSION_IDLE_TIMEOUT: &str = "session_idle_timeout"; pub const SESSION_WINDOW_CLOSED: &str = "session_window_closed"; + pub const SESSION_USER_MOVED_TAB: &str = "user_moved_tab"; pub const BROWSER_DISCONNECTED: &str = "browser_disconnected"; /// Extension-side: repeated observation identity failures on the /// same tab in a short window — the site reloads itself when the diff --git a/crates/bsk-cli/tests/handshake_compat.rs b/crates/bsk-cli/tests/handshake_compat.rs index 345b5071c..5505ab362 100644 --- a/crates/bsk-cli/tests/handshake_compat.rs +++ b/crates/bsk-cli/tests/handshake_compat.rs @@ -156,7 +156,7 @@ async fn handshake_skew_when_protocol_minor_differs() { let mut ws = open_ws(handle.ws_addr()).await; let resp = send_handshake_with_floors( &mut ws, - "1.6", + "1.7", env!("CARGO_PKG_VERSION"), Some("0.0.0"), Some("1.5"), @@ -211,6 +211,28 @@ async fn handshake_rejected_when_peer_protocol_major_is_below_daemon() { handle.shutdown().await; } +#[tokio::test] +async fn handshake_keeps_pre_terminal_event_extension_connected() { + let (handle, _sock) = spawn_daemon().await; + let mut ws = open_ws(handle.ws_addr()).await; + let response = send_handshake_with_floors( + &mut ws, + "1.5", + env!("CARGO_PKG_VERSION"), + Some("0.0.0"), + Some("1.5"), + ) + .await; + assert!(matches!(response.body, ResponseBody::Ok(_))); + let browser = handle + .state() + .browsers + .get(&bsk::daemon::browsers::BrowserId(TEST_EXT_ID.into())) + .expect("older extension stays registered"); + assert!(browser.version_skew); + handle.shutdown().await; +} + #[tokio::test] async fn handshake_legacy_ext_without_protocol_floor_still_ok() { let (handle, _sock) = spawn_daemon().await; @@ -242,7 +264,7 @@ async fn status_surfaces_version_skew_for_skewed_browser() { browser_name: "chrome".into(), browser_version: "131.0".into(), extension_version: "9.9.9".into(), - extension_protocol_version: "1.6".into(), + extension_protocol_version: "1.5".into(), label: "Older".into(), sink: bsk::daemon::browsers::BrowserSink { tx }, pending: Mutex::new(bsk::daemon::browsers::Pending::default()), @@ -269,7 +291,7 @@ async fn status_surfaces_version_skew_for_skewed_browser() { .iter() .find(|s| s.instance_id == "skew-only-test") .expect("status must list our skew client"); - assert_eq!(skew.client_protocol_version, "1.6"); + assert_eq!(skew.client_protocol_version, "1.5"); assert_eq!(skew.server_protocol_version, PROTOCOL_VERSION); assert_eq!(skew.client_version, "9.9.9"); let entry = status diff --git a/crates/bsk-cli/tests/per_session_queue.rs b/crates/bsk-cli/tests/per_session_queue.rs index 80190d931..f003d38f2 100644 --- a/crates/bsk-cli/tests/per_session_queue.rs +++ b/crates/bsk-cli/tests/per_session_queue.rs @@ -809,6 +809,512 @@ async fn native_input_effect_survives_deadline_and_user_cancellation() { } } +async fn next_request(ws: &mut Ws) -> RequestFrame { + tokio::time::timeout(Duration::from_secs(2), async { + loop { + if let Message::Text(text) = ws.next().await.unwrap().unwrap() + && let Frame::Request(request) = serde_json::from_str(&text).unwrap() + { + return request; + } + } + }) + .await + .expect("expected a request from the daemon") +} + +async fn end_session_after_tab_move(ws: &mut Ws, session_id: &str, rpc_id: &str) { + ws.send(Message::Text( + json!({ + "event": "session.ended", + "payload": {"session_id": session_id, "reason": "user_moved_tab", "tab_id": 42}, + }) + .to_string(), + )) + .await + .unwrap(); + let cancel = next_request(ws).await; + assert_eq!(cancel.method, Method::Cancel); + assert_eq!(cancel.params.unwrap()["rpc_id"], rpc_id); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn native_input_retains_terminal_reason_after_user_moves_tab() { + let (handle, _sock) = spawn_daemon().await; + let mut ws = connect_ext(handle.ws_addr()).await; + let _ = handshake_as_ext(&mut ws).await; + let state = handle.state(); + for method in [Method::ToolClick, Method::ToolPress, Method::ToolWheel] { + for (deadline_first, cleanup_effect) in [ + (false, Some("none")), + (false, Some("unknown")), + (false, None), + (true, Some("none")), + (true, None), + ] { + if deadline_first && method != Method::ToolClick { + continue; + } + let sid = state + .sessions + .reserve_id( + bsk::daemon::browsers::BrowserId(TEST_EXT_ID.into()), + 8, + || 0, + ) + .unwrap(); + state.tool_queues.spawn(sid.clone()); + let guard = state + .tool_inflight + .register("input".into(), sid.clone()) + .unwrap(); + let task = { + let queues = Arc::clone(&state.tool_queues); + let sid = sid.clone(); + let method = method.clone(); + let inflight = guard.entry(); + tokio::spawn(async move { + queues + .dispatch( + &sid, + method, + json!({"session_id": sid.0, "tab_id": 42}), + Duration::from_millis(if deadline_first { 20 } else { 5000 }), + Some(inflight), + ) + .await + }) + }; + let request = next_request(&mut ws).await; + assert_eq!(request.method, method); + if deadline_first { + let cancel = next_request(&mut ws).await; + assert_eq!(cancel.method, Method::Cancel); + assert_eq!(cancel.id, format!("deadline-cancel-{}", request.id)); + assert_eq!(guard.entry().cancel_reason(), None); + } + end_session_after_tab_move(&mut ws, &sid.0, &request.id).await; + let mut data = json!({"effect_state": cleanup_effect.unwrap_or("unknown")}); + if let Some(effect) = cleanup_effect { + data["cleanup_error"] = json!("focus cleanup failed"); + if effect == "unknown" { + data["reason"] = json!("input_outcome_unknown"); + } + ws.send(Message::Text( + serde_json::to_string(&ResponseFrame { + id: request.id, + body: ResponseBody::Err(bsk_protocol::RpcError { + code: ErrorCode::Cancelled, + message: "input aborted".into(), + data: Some(data.clone()), + }), + }) + .unwrap(), + )) + .await + .unwrap(); + } + let error = tokio::time::timeout(Duration::from_secs(4), task) + .await + .unwrap() + .unwrap() + .unwrap_err() + .into_rpc(); + if deadline_first { + assert_eq!(error.code, ErrorCode::Timeout); + assert_eq!( + error.message, + if cleanup_effect.is_some() { + "tool RPC timed out after 20ms" + } else { + "input timed out and extension cleanup could not be confirmed" + } + ); + } else if cleanup_effect.is_some() { + assert_eq!(error.code, ErrorCode::UserAborted); + } else { + assert_eq!(error.code, ErrorCode::Timeout); + assert_eq!( + error.message, + "cancelled tool did not finish cleanup within 2s" + ); + } + data["reason"] = json!(bsk::rpc_reason::SESSION_USER_MOVED_TAB); + assert_eq!( + error.data, + Some(data), + "{method:?}, deadline_first={deadline_first}, cleanup_effect={cleanup_effect:?}" + ); + let rendered = bsk::cli::render_error::info_for_error(error.code, error.data.as_ref()); + assert!( + rendered + .hint + .unwrap() + .contains("this session cannot resume") + ); + assert_eq!( + state.sessions.end_info(&sid).unwrap().reason, + "user_moved_tab" + ); + } + } + drop(ws); + handle.shutdown().await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn snapshot_outer_timeout_preserves_terminal_reason_without_extending_wait() { + let (handle, _sock) = spawn_daemon().await; + let mut ws = connect_ext(handle.ws_addr()).await; + let _ = handshake_as_ext(&mut ws).await; + let state = handle.state(); + let browser_id = bsk::daemon::browsers::BrowserId(TEST_EXT_ID.into()); + for end_session in [false, true] { + let sid = state + .sessions + .reserve_id(browser_id.clone(), 8, || 0) + .unwrap(); + state.tool_queues.spawn(sid.clone()); + let guard = state + .tool_inflight + .register("snapshot".into(), sid.clone()) + .unwrap(); + let task = { + let queues = Arc::clone(&state.tool_queues); + let sid = sid.clone(); + let inflight = guard.entry(); + tokio::spawn(async move { + queues + .dispatch( + &sid, + Method::ToolSnapshot, + json!({"session_id": sid.0, "tab_id": 42}), + Duration::from_millis(1500), + Some(inflight), + ) + .await + }) + }; + let request = next_request(&mut ws).await; + assert_eq!(request.method, Method::ToolSnapshot); + // Cancellation at 1.2s pushes worker cleanup to ~3.2s, past the + // ordinary outer dispatch deadline of 1.5s + 1s. + tokio::time::sleep(Duration::from_millis(1200)).await; + assert!( + !task.is_finished(), + "cancel must precede the worker's deadline" + ); + if end_session { + end_session_after_tab_move(&mut ws, &sid.0, &request.id).await; + } else { + ws.send(Message::Text( + json!({ + "event": "session.user_interrupt", + "payload": {"session_id": sid.0}, + }) + .to_string(), + )) + .await + .unwrap(); + let cancel = next_request(&mut ws).await; + assert_eq!(cancel.method, Method::Cancel); + assert_eq!(cancel.params.unwrap()["rpc_id"], request.id); + } + let error = tokio::time::timeout(Duration::from_secs(5), task) + .await + .unwrap() + .unwrap() + .unwrap_err() + .into_rpc(); + assert_eq!(error.code, ErrorCode::Timeout); + assert_eq!(error.message, "tool dispatch timed out"); + // An outstanding extension waiter proves dispatch returned before + // worker cleanup. Resolve it now instead of using a narrow elapsed bound. + assert!( + state + .browsers + .get(&browser_id) + .unwrap() + .pending + .lock() + .unwrap() + .resolve(ResponseFrame { + id: request.id, + body: ResponseBody::Err(bsk_protocol::RpcError { + code: ErrorCode::Cancelled, + message: "snapshot cleanup completed".into(), + data: None, + }), + }), + "the outer timeout must not wait for the worker's cleanup deadline" + ); + let rendered = bsk::cli::render_error::info_for_error(error.code, error.data.as_ref()); + if end_session { + assert_eq!( + error.data, + Some(json!({"reason": bsk::rpc_reason::SESSION_USER_MOVED_TAB})) + ); + assert!( + rendered + .hint + .unwrap() + .contains("this session cannot resume") + ); + assert_eq!( + state.sessions.end_info(&sid).unwrap().reason, + "user_moved_tab" + ); + assert!(state.sessions.get(&sid).is_none()); + } else { + assert!(error.data.is_none()); + assert!(!rendered.hint.unwrap_or_default().contains("cannot resume")); + assert!(state.sessions.end_info(&sid).is_none()); + assert!(state.sessions.get(&sid).is_some()); + } + } + drop(ws); + handle.shutdown().await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn evaluate_cleanup_timeout_preserves_session_termination() { + let (handle, _sock) = spawn_daemon().await; + let mut ws = connect_ext(handle.ws_addr()).await; + let _ = handshake_as_ext(&mut ws).await; + let state = handle.state(); + for (deadline_first, end_session) in [(true, false), (false, true), (true, true)] { + let sid = state + .sessions + .reserve_id( + bsk::daemon::browsers::BrowserId(TEST_EXT_ID.into()), + 8, + || 0, + ) + .unwrap(); + state.tool_queues.spawn(sid.clone()); + let guard = state + .tool_inflight + .register("evaluate".into(), sid.clone()) + .unwrap(); + let task = { + let queues = Arc::clone(&state.tool_queues); + let sid = sid.clone(); + let inflight = guard.entry(); + tokio::spawn(async move { + let timeout_ms = if deadline_first { 20 } else { 5000 }; + queues + .dispatch( + &sid, + Method::ToolEvaluate, + json!({ + "session_id": sid.0, + "tab_id": 42, + "expression": "new Promise(() => {})", + "await_promise": true, + "timeout_ms": timeout_ms, + }), + Duration::from_millis(timeout_ms), + Some(inflight), + ) + .await + }) + }; + let request = next_request(&mut ws).await; + assert_eq!(request.method, Method::ToolEvaluate); + assert_eq!( + request.params.as_ref().unwrap()["expression"], + "new Promise(() => {})" + ); + assert_eq!(request.params.as_ref().unwrap()["await_promise"], true); + if deadline_first { + let cancel = next_request(&mut ws).await; + assert_eq!(cancel.method, Method::Cancel); + assert_eq!(cancel.id, format!("deadline-cancel-{}", request.id)); + assert_eq!(guard.entry().cancel_reason(), None); + } + if end_session { + end_session_after_tab_move(&mut ws, &sid.0, &request.id).await; + } + // Keep the connection open without replying, so the actual cleanup deadline expires. + let error = tokio::time::timeout(Duration::from_secs(4), task) + .await + .unwrap() + .unwrap() + .unwrap_err() + .into_rpc(); + assert_eq!(error.code, ErrorCode::Timeout); + assert_eq!( + error.message, + if deadline_first { + "page operation timed out and extension cleanup could not be confirmed; page effects may have occurred" + } else { + "cancelled tool did not finish cleanup within 2s" + } + ); + let mut expected = json!({"reason": if end_session { + bsk::rpc_reason::SESSION_USER_MOVED_TAB + } else { + "cancel_cleanup_timeout" + }}); + if deadline_first { + expected["effect_state"] = json!("unknown"); + } + assert_eq!( + error.data, + Some(expected), + "deadline_first={deadline_first}, end_session={end_session}" + ); + let rendered = bsk::cli::render_error::info_for_error(error.code, error.data.as_ref()); + let hint = rendered.hint.unwrap(); + if end_session { + assert!(hint.contains("this session cannot resume")); + assert_eq!( + state.sessions.end_info(&sid).unwrap().reason, + "user_moved_tab" + ); + assert!(state.sessions.get(&sid).is_none()); + } else { + assert!(hint.contains("observe the page before deciding whether to retry")); + assert!(!hint.contains("cannot resume")); + assert!(state.sessions.end_info(&sid).is_none()); + assert!(state.sessions.get(&sid).is_some()); + } + } + drop(ws); + handle.shutdown().await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn session_end_preserves_cleanup_details_and_transfer_results() { + let (handle, _sock) = spawn_daemon().await; + let mut ws = connect_ext(handle.ws_addr()).await; + let _ = handshake_as_ext(&mut ws).await; + let state = handle.state(); + let evaluate_params = json!({"expression": "new Promise(() => {})", "await_promise": true}); + for (method, mut params, reply) in [ + ( + Method::ToolEvaluate, + evaluate_params.clone(), + ResponseBody::Err(bsk_protocol::RpcError { + code: ErrorCode::CdpFailed, + message: "evaluate cleanup failed".into(), + data: Some(json!({ + "reason": "evaluate_cleanup_failed", + "effect_state": "unknown", + "cleanup_error": "debugger command did not settle", + })), + }), + ), + ( + Method::ToolEvaluate, + evaluate_params, + ResponseBody::Err(bsk_protocol::RpcError { + code: ErrorCode::CdpFailed, + message: "evaluate cleanup failed".into(), + data: Some(json!("extension cleanup diagnostic")), + }), + ), + ( + Method::ToolUpload, + json!({"selector": "input[type=file]", "files": [{"transfer_id": "upload-ready", "name": "report.txt"}]}), + ResponseBody::Ok( + json!({"tab_id": 42, "used_selector": "input[type=file]", "file_names": ["report.txt"]}), + ), + ), + ( + Method::ToolDownload, + json!({"selector": "#download"}), + ResponseBody::Err(bsk_protocol::RpcError { + code: ErrorCode::Cancelled, + message: "download cleanup incomplete".into(), + data: Some(json!({ + "reason": "transfer_outcome_unknown", + "effect_state": "unknown", + "phase": "cleanup", + "cleanup_error": "download cancellation was not acknowledged", + })), + }), + ), + ] { + let sid = state + .sessions + .reserve_id( + bsk::daemon::browsers::BrowserId(TEST_EXT_ID.into()), + 8, + || 0, + ) + .unwrap(); + state.tool_queues.spawn(sid.clone()); + let guard = state + .tool_inflight + .register("cleanup".into(), sid.clone()) + .unwrap(); + params["session_id"] = json!(sid.0); + params["tab_id"] = json!(42); + let task = { + let queues = Arc::clone(&state.tool_queues); + let sid = sid.clone(); + let inflight = guard.entry(); + let method = method.clone(); + tokio::spawn(async move { + queues + .dispatch(&sid, method, params, Duration::from_secs(5), Some(inflight)) + .await + }) + }; + let request = next_request(&mut ws).await; + assert_eq!(request.method, method); + end_session_after_tab_move(&mut ws, &sid.0, &request.id).await; + ws.send(Message::Text( + serde_json::to_string(&ResponseFrame { + id: request.id, + body: reply.clone(), + }) + .unwrap(), + )) + .await + .unwrap(); + let result = tokio::time::timeout(Duration::from_secs(2), task) + .await + .unwrap() + .unwrap(); + match reply { + ResponseBody::Ok(expected) => assert_eq!(result.unwrap(), expected), + ResponseBody::Err(expected) => { + let actual = result.unwrap_err().into_rpc(); + assert_eq!(actual.code, expected.code); + assert_eq!(actual.message, expected.message); + let data = actual.data.as_ref().unwrap(); + assert_eq!(data["reason"], bsk::rpc_reason::SESSION_USER_MOVED_TAB); + match expected.data.unwrap() { + serde_json::Value::Object(fields) => { + for (key, value) in fields { + if key != "reason" { + assert_eq!(data[&key], value, "{method:?}, field={key}"); + } + } + } + diagnostic => assert_eq!(data["details"], diagnostic), + } + let rendered = + bsk::cli::render_error::info_for_error(actual.code, actual.data.as_ref()); + assert!( + rendered + .hint + .unwrap() + .contains("this session cannot resume") + ); + } + } + assert_eq!( + state.sessions.end_info(&sid).unwrap().reason, + "user_moved_tab" + ); + } + drop(ws); + handle.shutdown().await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn full_page_screenshot_deadline_keeps_the_session_busy_until_page_cleanup() { assert_deadline_waits_for_cleanup( diff --git a/crates/bsk-protocol/src/frame.rs b/crates/bsk-protocol/src/frame.rs index d26bfdb1d..cbc90ac71 100644 --- a/crates/bsk-protocol/src/frame.rs +++ b/crates/bsk-protocol/src/frame.rs @@ -56,6 +56,9 @@ pub enum EventKind { SessionActivity, #[serde(rename = "session.window_closed")] SessionWindowClosed, + /// The extension ended the session after a user moved a controlled tab. + #[serde(rename = "session.ended")] + SessionEnded, #[serde(rename = "session.user_interrupt")] SessionUserInterrupt, #[serde(rename = "session.interaction_changed")] @@ -295,6 +298,17 @@ mod tests { ); } + #[test] + fn session_ended_round_trips_from_extension_shape() { + let wire = serde_json::json!({ + "event": "session.ended", + "payload": { "session_id": "sess-1", "reason": "user_moved_tab", "tab_id": 42 }, + }); + let frame: Frame = serde_json::from_value(wire.clone()).unwrap(); + assert!(matches!(&frame, Frame::Event(event) if event.event == EventKind::SessionEnded)); + assert_eq!(serde_json::to_value(frame).unwrap(), wire); + } + #[test] fn explicit_null_result_decodes_as_ok_null() { let wire = serde_json::json!({ "id": "rpc-1", "result": null }); diff --git a/docs/shared-browser-control.zh-CN.md b/docs/shared-browser-control.zh-CN.md index 8edff5f9e..5318f9825 100644 --- a/docs/shared-browser-control.zh-CN.md +++ b/docs/shared-browser-control.zh-CN.md @@ -30,7 +30,8 @@ DSH 使用 `bsk_session` 的 `sync`、`wait-user`、`cooperation-status`。旧 ` - 普通点击或滚动后,短暂安静即可继续。按住鼠标、按键、输入法组合和文本编辑期间保持人工控制。 - 暂停控件用于阅读或较长操作;“交给 Agent”结束编辑保持。输入法组合或拖拽尚未结束时,交回不会强行抢走输入。 - 工具条只占自身区域,页面不再铺设输入遮罩。“正在停手”表示旧操作还在收尾;“人工操作中”表示没有 Agent 动作执行。 -- 真实人工输入、主动接管或完成求助后的页面在会话结束或连接断开时保留,避免丢失人工编辑;无人介入的 Agent 创建页面仍按原路径关闭。用户把任务页移出任务窗口时交还页面,不再列入会话停止的关闭清单。 +- 真实人工输入、主动接管或完成求助后的页面在正常会话结束或连接断开时保留,避免丢失人工编辑;无人介入的 Agent 创建页面仍按原路径关闭。 +- 任一受控页面移出原任务窗口会结束整个会话,取消正在执行的操作并释放全部页面的控制标识。所有页面保留在当前位置,包括借用页;不自动关闭或归还,也不支持拖回恢复。同窗口内排序、程序主动借用和归还不触发该行为。收到 `user_moved_tab` 后不能自动重建会话重试,应由用户决定是否开始新任务;已发出的操作可能已生效。 - “停止任务”与临时保持人工控制不同:后续动作和协作等待均返回 `user_aborted`,不能自动重新规划后恢复。 - 会话创建时固定初始任务标签页。人工切换或打开别的标签页不会改变默认任务页;需要转到另一任务页时,显式选择标签页,或同步时指定 `--tab-id`。原任务页关闭或移出窗口后,旧上下文失效,默认请求报错,不退回当前激活页。