From f444f2b37497246c37dad0a4ec02281cd3cf468b Mon Sep 17 00:00:00 2001 From: lymerin <884917500@qq.com> Date: Mon, 28 Sep 2026 02:58:00 +0800 Subject: [PATCH 1/3] feat(session): support tabs in the current browser window --- README.md | 2 + README.zh-CN.md | 2 + apps/extension/PRIVACY.md | 8 +- .../src/debug/__tests__/manager.test.ts | 21 + apps/extension/src/debug/manager.ts | 34 +- .../__tests__/background-overlays.test.ts | 1 + apps/extension/src/entrypoints/background.ts | 59 +-- .../__tests__/connection-controller.test.ts | 12 +- .../src/lib/__tests__/task-preview.test.ts | 2 +- apps/extension/src/lib/task-preview.ts | 12 +- .../__tests__/event-handler.test.ts | 24 +- .../session-manager/__tests__/manager.test.ts | 11 +- .../__tests__/shared-window.test.ts | 397 ++++++++++++++++++ .../src/session-manager/event-handler.ts | 110 +++-- apps/extension/src/session-manager/manager.ts | 211 +++++++++- .../src/session-manager/shared-window.ts | 22 + .../src/session-manager/task-popups.ts | 14 +- .../src/session-manager/ui-activity.ts | 13 +- .../src/tools/__tests__/dispatcher.test.ts | 4 +- .../src/tools/__tests__/human-loop.test.ts | 18 +- .../src/tools/__tests__/record-steps.test.ts | 3 +- .../src/tools/__tests__/shared.test.ts | 10 +- apps/extension/src/tools/audit-context.ts | 39 +- .../src/tools/background-execution.ts | 8 +- .../src/tools/borrow-confirmation.ts | 9 +- apps/extension/src/tools/human-loop.ts | 14 +- apps/extension/src/tools/observation.ts | 5 +- apps/extension/src/tools/record.ts | 13 +- apps/extension/src/tools/session.ts | 74 +++- apps/extension/src/tools/shared.ts | 54 ++- apps/extension/src/tools/tabs.ts | 334 ++++++++++----- apps/extension/src/tools/waits.ts | 5 +- apps/extension/src/tools/window.ts | 7 +- .../src/transport/__tests__/handshake.test.ts | 2 +- apps/extension/src/transport/handshake.ts | 2 +- crates/bsk-cli/src/cli/error.rs | 4 +- crates/bsk-cli/src/cli/render_error.rs | 6 +- crates/bsk-cli/src/cli/session.rs | 43 +- crates/bsk-cli/src/daemon/audit.rs | 1 + crates/bsk-cli/src/daemon/ipc.rs | 7 + crates/bsk-cli/src/daemon/session_requests.rs | 1 + crates/bsk-cli/src/daemon/sessions.rs | 38 ++ crates/bsk-cli/src/daemon/state.rs | 2 +- crates/bsk-cli/src/daemon/ws.rs | 9 +- crates/bsk-cli/tests/browser_liveness.rs | 1 + crates/bsk-cli/tests/cancel_forwarding.rs | 3 + crates/bsk-cli/tests/cli_parse.rs | 32 ++ crates/bsk-cli/tests/daemon_discovery.rs | 24 ++ crates/bsk-cli/tests/handshake_compat.rs | 40 +- crates/bsk-cli/tests/per_session_queue.rs | 1 + .../bsk-cli/tests/record_export_recovery.rs | 1 + crates/bsk-cli/tests/record_stop_retry.rs | 1 + .../bsk-cli/tests/session_user_interrupt.rs | 4 + crates/bsk-cli/tests/sessions_ipc.rs | 123 +++++- crates/bsk-cli/tests/shared_window_compat.rs | 63 +++ crates/bsk-cli/tests/support/mod.rs | 15 + crates/bsk-cli/tests/tools_ipc.rs | 1 + crates/bsk-cli/tests/tools_m7_ipc.rs | 1 + crates/bsk-cli/tests/tools_m8_ipc.rs | 1 + crates/bsk-cli/tests/tools_m9_ipc.rs | 1 + .../schema/system_status_result.json | 6 + .../schema/tool_session_start_params.json | 3 + .../schema/tool_session_start_result.json | 6 + .../schema/tool_tab_create_params.json | 4 +- .../schema/tool_tab_list_params.json | 2 +- .../schema/tool_tab_list_result.json | 2 +- crates/bsk-protocol/src/frame.rs | 2 + crates/bsk-protocol/src/system.rs | 2 + crates/bsk-protocol/src/tools/session.rs | 40 ++ crates/bsk-protocol/src/tools/tabs.rs | 12 +- docs/architecture.md | 28 +- docs/long-screenshot.md | 11 +- docs/remote-extension-connection.md | 2 +- docs/scroll-to.md | 10 +- docs/wheel.md | 6 +- 75 files changed, 1746 insertions(+), 369 deletions(-) create mode 100644 apps/extension/src/session-manager/__tests__/shared-window.test.ts create mode 100644 apps/extension/src/session-manager/shared-window.ts create mode 100644 crates/bsk-cli/tests/shared_window_compat.rs diff --git a/README.md b/README.md index d156ac3f..35e6bcc8 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,8 @@ bsk session stop Use `bsk --help` or `bsk --help` for command options. Always stop your session when finished, including after a failed task; borrowed tabs are returned to their original window. +For a local session in the last-focused user window, start with `bsk session start --in-window --json`. It creates a session-owned tab; stopping closes that tab, not the user window. Existing user tabs still require an explicit borrow. Window dimensions and remote connections do not support this option. + If your agent sandbox removes background processes after each command, use the [sandbox setup guide](docs/sandboxed-agents.md). It explains how to keep the daemon in a persistent host environment and connect with shared `BSK_HOME` and `BSK_AUTO_START=0`. diff --git a/README.zh-CN.md b/README.zh-CN.md index 4287d2cb..f91a4404 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -147,6 +147,8 @@ bsk session stop 通过 `bsk --help` 或 `bsk <命令> --help` 查看参数。完成或失败后都应结束会话；借用的标签页会归还到原窗口。 +本地会话可用 `bsk session start --in-window --json` 在最近聚焦的用户窗口中新建会话页签。停止时只关闭会话页签，不关闭用户窗口；已有用户页签仍需显式借用。此选项不支持窗口尺寸参数或远程连接。 + 如果 Agent 沙盒会在每条命令后回收后台进程，请使用[沙盒配置指南](docs/sandboxed-agents.md)：在宿主环境保持 daemon 运行，Agent 通过共享的 `BSK_HOME` 和 `BSK_AUTO_START=0` 连接。 diff --git a/apps/extension/PRIVACY.md b/apps/extension/PRIVACY.md index aae0ff66..890d8f6d 100644 --- a/apps/extension/PRIVACY.md +++ b/apps/extension/PRIVACY.md @@ -24,7 +24,7 @@ Depending on the commands the user (via their AI agent) sends to the selected da | Category | What is accessed | Why | |---|---|---| -| **Web page content** | The DOM, accessibility tree, HTML, and screenshots of pages controlled in the "Agent Window," tabs borrowed according to the browser's confirmation setting, or pages selected for user-initiated Quick Actions. | Required to read pages, locate elements, verify results, and capture requested screenshots. | +| **Web page content** | The DOM, accessibility tree, HTML, and screenshots of pages controlled in a dedicated Agent Window or an opt-in local shared-window session, tabs borrowed according to the browser's confirmation setting, or pages selected for user-initiated Quick Actions. | Required to read pages, locate elements, verify results, and capture requested screenshots. | | **User input simulated by the agent** | Mouse clicks, keystrokes, and form values that the AI agent dispatches through the Chrome DevTools Protocol (CDP). | Required to perform automation actions the user has asked the agent to do. | | **Website debugging evidence** | For the selected tab while capture is active: request URLs, methods, headers, request and response bodies when available, status, timing, errors and related network metadata; console messages; agent actions and supported manual input, click, submission and navigation events; bounded visible page text, form-field values and page/performance context. Configured HTTP rules and replay outcomes are also recorded. | Used to inspect and reproduce website behavior, associate operations with requests and page changes, analyze performance, and review or export debugging history. | | **Tab and window metadata** | Tab IDs, URLs, titles and window IDs, including user tabs listed to select a tab for borrowing. | Required to target automation commands at the correct tab/window. | @@ -50,8 +50,8 @@ The Extension requests the following Chrome permissions. Each is used solely for - **`activeTab`** — Allow temporary access to the active tab when the user invokes the Extension, for user-initiated Quick Actions. - **`scripting`** — Inject the full-page screenshot helper into the selected page when it is missing, such as after an extension reload. - **`webNavigation`** — Track page navigation and frames so captures, recordings, and human-help completion checks follow the correct document. -- **`tabs`** — Inspect, create, and close tabs in the Agent Window; query tab metadata. -- **`windows`** — Create and manage the dedicated Agent Window that isolates agent activity from the user's normal browsing. +- **`tabs`** — Inspect, create, and close session-owned tabs; query tab metadata. An opt-in local shared-window session creates tabs in a user window but does not thereby control other user tabs. +- **`windows`** — Create and manage dedicated Agent Windows, or identify the user window selected for an opt-in local shared-window session. Shared-session cleanup does not close that user window. - **`alarms`** — Periodically wake the service worker to keep the selected connection alive and renew remote device authorization. - **`idle`** — Detect when the device returns from idle/locked so the Extension can promptly re-establish the selected WebSocket connection after the machine wakes. No idle data is stored or transmitted. - **`notifications`** — Show a system notification to obtain user approval before borrowing a user-owned tab when browser confirmation is enabled. @@ -88,7 +88,7 @@ Users can at any time: - Uninstall the Extension from `chrome://extensions`, which removes extension storage. Audit files on the daemon host and exported copies must be deleted separately. - Stop website debugging from Quick Actions → Website debugging, or ask the agent to stop capture. Open its history page to review or export records and delete stopped records, including when no daemon is connected. Stop an active capture before deleting its record. - Turn operation audit off in Quick Features to stop collecting new audit operations while retaining existing audit history. Previously recorded tasks still receive their final lifecycle status. This switch does not stop website debugging capture or delete its history. -- Close the Agent Window to stop all agent automation immediately. +- Stop a session with `bsk session stop SESSION_ID`. A dedicated session also ends when its Agent Window closes; a shared-window session ends when its last controlled tab closes. Stopping a shared session does not close the user window. - Enable confirmation before borrowing and deny tab-borrow prompts to keep existing tabs off-limits. - Disable the connection, choose Local connection, or stop the selected daemon to disconnect. - Revoke a paired device from the server with `bsk daemon revoke DEVICE_ID`, or use the gateway operator’s revocation controls. diff --git a/apps/extension/src/debug/__tests__/manager.test.ts b/apps/extension/src/debug/__tests__/manager.test.ts index 19abc0e5..588ce606 100644 --- a/apps/extension/src/debug/__tests__/manager.test.ts +++ b/apps/extension/src/debug/__tests__/manager.test.ts @@ -88,6 +88,27 @@ afterEach(() => { }); describe("task-scoped debug lifecycle", () => { + it("keeps a shared task listed when its host tab query fails", async () => { + const f = await fixture(); + const sessions = new SessionManager({ + sharedWindow: { + host: async () => ({ id: 200, type: "normal", incognito: false }) as chrome.windows.Window, + create: async () => 8, + get: async () => ({ id: 8, windowId: 200 }) as chrome.tabs.Tab, + remove: async () => {}, + }, + }); + await sessions.start("shared", { inWindow: true, focused: false }); + const debug = new DebugManager(sessions, f.cdp, { + get: f.tabs.get, + query: vi.fn(async () => { + throw new Error("host query denied"); + }), + }); + active.push(debug); + await expect(debug.tasks()).resolves.toMatchObject([{ session_id: "shared" }]); + }); + it("reserves global capacity before concurrent startups yield", async () => { const f = await fixture(); active.push(f.manager); diff --git a/apps/extension/src/debug/manager.ts b/apps/extension/src/debug/manager.ts index 7ae3134f..957c21e4 100644 --- a/apps/extension/src/debug/manager.ts +++ b/apps/extension/src/debug/manager.ts @@ -6,8 +6,10 @@ import { } from "@/browser-driver/chromium-cdp"; import { isAgentControlledTab, + preferredSharedTab, type SessionContext, type SessionManager, + sessionWindowId, } from "@/session-manager/manager"; import type { CdpRunner, ChromeTabsApi } from "@/tools/shared"; import type { RequestFrame } from "@/transport/types"; @@ -318,7 +320,7 @@ export class DebugManager { if (this.sessions.get(sessionId) !== owner) throw new Error("task ended during debug start"); const tab = await wait(this.tabs.get(tabId)); - if (tab.windowId !== this.sessions.get(sessionId)?.agentWindowId) + if (tab.windowId !== sessionWindowId(owner)) throw new Error("debug tab must remain in its Agent Window"); if (!this.runs.has(id) || state.run.state !== "capturing") throw new Error("capture stopped during debug start"); @@ -557,15 +559,24 @@ export class DebugManager { if (!params?.session_id || !this.active(params.session_id)) return; const context = this.sessions.get(params.session_id); if (!context) return; - const tabId = - params.tab_id ?? - ( + let tabId = params.tab_id; + if (tabId === undefined && context.container.mode === "in_window") { + const tabs = await deadline( + this.tabs.query({ windowId: sessionWindowId(context) }), + OBSERVATION_TIMEOUT_MS, + signal, + ).catch(() => undefined); + // If Chrome cannot list the host window, retain the prior debug behavior. + tabId = tabs ? preferredSharedTab(context, tabs)?.id : context.activeTabId; + } else if (tabId === undefined) { + tabId = ( await deadline( - this.tabs.query({ windowId: context.agentWindowId, active: true }), + this.tabs.query({ windowId: sessionWindowId(context), active: true }), OBSERVATION_TIMEOUT_MS, signal, ) )[0]?.id; + } if (tabId === undefined || !this.owned(params.session_id, tabId)) return; const state = this.active(params.session_id, tabId); if (!state) return; @@ -841,8 +852,7 @@ export class DebugManager { ); if (!this.owned(state.run.session_id, state.run.tab_id) || state.run.state !== "capturing") return { at, state: "unavailable" }; - if (tab.windowId !== this.sessions.get(state.run.session_id)?.agentWindowId) - return { at, state: "unavailable" }; + if (tab.windowId !== sessionWindowId(state.owner)) return { at, state: "unavailable" }; const lines: string[] = []; let chars = 0; let truncated = false; @@ -988,7 +998,13 @@ export class DebugManager { this.sync(); return Promise.all( this.sessions.list().map(async (context) => { - const tab = (await this.tabs.query({ windowId: context.agentWindowId, active: true }))[0]; + const tab = + context.container.mode === "in_window" + ? preferredSharedTab( + context, + await this.tabs.query({ windowId: sessionWindowId(context) }).catch(() => []), + ) + : (await this.tabs.query({ windowId: sessionWindowId(context), active: true }))[0]; const latest = [...this.runs.values()] .filter(({ run, released }) => !released && run.session_id === context.sessionId) .at(-1); @@ -1354,7 +1370,7 @@ export class DebugManager { const tab = await this.tabs.get(state.run.tab_id); if ( !this.owned(params.session_id, state.run.tab_id) || - tab.windowId !== this.sessions.get(params.session_id)?.agentWindowId + tab.windowId !== sessionWindowId(state.owner) ) throw new Error("debug tab must remain in its Agent Window"); if (signal?.aborted) throw new Error("debug action cancelled"); diff --git a/apps/extension/src/entrypoints/__tests__/background-overlays.test.ts b/apps/extension/src/entrypoints/__tests__/background-overlays.test.ts index fc7a76e5..090e019b 100644 --- a/apps/extension/src/entrypoints/__tests__/background-overlays.test.ts +++ b/apps/extension/src/entrypoints/__tests__/background-overlays.test.ts @@ -70,6 +70,7 @@ async function fixture() { tabs: { sendMessage, onDetached, + onAttached: event(), onActivated: event(), onUpdated: event(), onCreated: event(), diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index 4dc779de..7230d5d8 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -35,7 +35,7 @@ import { attachUiChannel } from "@/lib/ui-channel"; import { attachLongScreenshot } from "@/long-screenshot/background"; import { createDisconnectCleanup } from "@/session-manager/disconnect-cleanup"; import { attachSessionEventHandler } from "@/session-manager/event-handler"; -import { isAgentControlledTab, SessionManager } from "@/session-manager/manager"; +import { isAgentControlledTab, SessionManager, sessionWindowId } from "@/session-manager/manager"; import { attachBorrowNotificationButtonHandler, attachBorrowNotificationClickHandler, @@ -88,8 +88,7 @@ export default defineBackground(() => { onDocumentChanged: (tabId) => sessions.invalidateTabRefs(tabId), shouldAutoAcceptDialog: async (tabId) => { const tab = await chrome.tabs.get(tabId); - const session = sessions.findByWindowId(tab.windowId); - return session !== null && (!session.remote || isAgentControlledTab(session, tabId)); + return sessions.canAutoAcceptDialog(tabId, tab.windowId); }, }); const debug = new DebugManager(sessions, cdp, chrome.tabs, Date.now, new LocalDebugArchive()); @@ -142,25 +141,7 @@ export default defineBackground(() => { controlModes.set(sessionId, mode); overlayGeneration += 1; const ctx = sessions.get(sessionId); - if (ctx) void pushOverlayStateForWindow(ctx.agentWindowId); - } - - function overlayStateForWindow(windowId?: number): OverlayAgentStateMessage { - const ctx = typeof windowId === "number" ? sessions.findByWindowId(windowId) : null; - if (!ctx) { - return { - type: OVERLAY_AGENT_STATE, - sessionId: null, - mode: "hidden", - generation: overlayGeneration, - }; - } - return { - type: OVERLAY_AGENT_STATE, - sessionId: ctx.sessionId, - mode: controlModes.get(ctx.sessionId) ?? "control", - generation: overlayGeneration, - }; + if (ctx) void pushOverlayStateForWindow(sessionWindowId(ctx)); } /** @@ -170,8 +151,14 @@ export default defineBackground(() => { */ function overlayStateForTab(tabId?: number, windowId?: number): OverlayAgentStateMessage { if (typeof tabId === "number" && typeof windowId === "number") { - const ctx = sessions.findByWindowId(windowId); - if (ctx && isAgentControlledTab(ctx, tabId)) return overlayStateForWindow(windowId); + const ctx = sessions.findByTabId(tabId); + if (ctx && sessionWindowId(ctx) === windowId) + return { + type: OVERLAY_AGENT_STATE, + sessionId: ctx.sessionId, + mode: controlModes.get(ctx.sessionId) ?? "control", + generation: overlayGeneration, + }; } return { type: OVERLAY_AGENT_STATE, @@ -208,7 +195,7 @@ export default defineBackground(() => { } function pushAllAgentOverlayStates(): void { - const windowIds = new Set(sessions.list().map((ctx) => ctx.agentWindowId)); + const windowIds = new Set(sessions.list().map((ctx) => sessionWindowId(ctx))); for (const windowId of windowIds) { void pushOverlayStateForWindow(windowId); } @@ -233,10 +220,12 @@ export default defineBackground(() => { } function pushOverlayStateForAgentWindow(windowId: number): void { - if (!sessions.findByWindowId(windowId)) return; + if (!sessions.sessionsInWindow(windowId).length) return; void pushOverlayStateForWindow(windowId); } chrome.tabs.onActivated.addListener((activeInfo) => { + const ctx = sessions.findByTabId(activeInfo.tabId); + if (ctx?.container.mode === "in_window") ctx.activeTabId = activeInfo.tabId; pushOverlayStateForAgentWindow(activeInfo.windowId); }); chrome.tabs.onUpdated.addListener((_tabId, changeInfo, tab) => { @@ -249,7 +238,7 @@ export default defineBackground(() => { // state; it never infers or mutates ownership from event ordering. chrome.tabs.onCreated.addListener((tab) => { if (typeof tab.windowId !== "number" || typeof tab.id !== "number") return; - if (!sessions.findByWindowId(tab.windowId)) return; + if (!sessions.sessionsInWindow(tab.windowId).length) return; void pushOverlayStateForTab(tab.id, tab.windowId); }); chrome.tabs.onDetached.addListener((tabId) => { @@ -266,6 +255,18 @@ export default defineBackground(() => { sessions.forgetClosedTab(tabId, { isWindowClosing: removeInfo.isWindowClosing }); debug.sync(); }); + chrome.tabs.onAttached.addListener((tabId, info) => { + const ctx = sessions.findByTabId(tabId); + if (!ctx || ctx.container.mode !== "in_window" || sessionWindowId(ctx) === info.newWindowId) + return; + ctx.agentCreatedTabs.delete(tabId); + ctx.borrowedTabs.delete(tabId); + ctx.observedTabs?.delete(tabId); + ctx.refStore.invalidateTab(tabId); + void cdp.releaseSessionTab(ctx.sessionId, tabId).catch(console.warn); + void pushOverlayStateForTab(tabId, info.newWindowId); + sessions.checkEmpty(ctx); + }); // Re-sync the storage.session flag on SW startup so a previous SW's // stale `true` does not keep waking us on every page load until the // first mutation (review M4/M5 round 3 m-R3-1). @@ -345,6 +346,7 @@ export default defineBackground(() => { // overlay — Agent Windows boot on about:blank, which has no // content script, so they cannot surface an authorization decision. isAgentWindowId: (windowId) => sessions.findByWindowId(windowId) !== null, + isTabAllowed: (tabId) => sessions.findByTabId(tabId) === null, // Resolve i18n strings per-borrow so language switches take effect // without re-creating the dispatcher. notificationCopy: makeBorrowNotificationCopy(), @@ -462,8 +464,7 @@ export default defineBackground(() => { if (!msg || typeof msg !== "object" || !("kind" in msg)) return false; if (msg.kind === OVERLAY_MSG_WHO_AM_I) { - const windowId = sender.tab?.windowId; - const ctx = typeof windowId === "number" ? sessions.findByWindowId(windowId) : null; + const ctx = typeof sender.tab?.id === "number" ? sessions.findByTabId(sender.tab.id) : null; sendResponse({ sessionId: ctx?.sessionId ?? null }); return false; } diff --git a/apps/extension/src/lib/__tests__/connection-controller.test.ts b/apps/extension/src/lib/__tests__/connection-controller.test.ts index c2eba3cd..bbe6934a 100644 --- a/apps/extension/src/lib/__tests__/connection-controller.test.ts +++ b/apps/extension/src/lib/__tests__/connection-controller.test.ts @@ -28,13 +28,13 @@ function handshake( describe("computeConnectedState (protocol-based compat)", () => { it("returns connected when daemon protocol equals extension protocol", () => { - expect(computeConnectedState(handshake("1.3", "1.3"), MIN_COMPATIBLE_PROTOCOL)).toEqual({ + expect(computeConnectedState(handshake("1.4", "1.3"), MIN_COMPATIBLE_PROTOCOL)).toEqual({ kind: "connected", }); }); it("returns version_skew when daemon protocol minor is newer", () => { - expect(computeConnectedState(handshake("1.4", "1.3"))).toEqual({ + expect(computeConnectedState(handshake("1.5", "1.3"))).toEqual({ kind: "version_skew", }); }); @@ -66,7 +66,7 @@ describe("computeConnectedState (protocol-based compat)", () => { const result = computeConnectedState({ server: "browser-skill-daemon", version: "0.1.0", - protocol_version: "1.3", + protocol_version: "1.4", min_compatible_peer: "0.1.0", }); expect(result).toEqual({ kind: "connected" }); @@ -234,7 +234,7 @@ describe("ConnectionController connectionEnabled", () => { onDisconnected, }); const first = transport.send.mock.calls[0]?.[0] as { id: string }; - transport.emitMessage({ id: first.id, result: handshake("1.3", "1.3") }); + transport.emitMessage({ id: first.id, result: handshake("1.4", "1.3") }); await vi.waitFor(() => expect(controller.snapshot().state).toBe("connected")); vi.mocked(getLabel).mockResolvedValueOnce("Work profile"); @@ -299,11 +299,11 @@ describe("ConnectionController connectionEnabled", () => { const second = transport.send.mock.calls[1]?.[0] as { id: string }; expect(second.id).not.toBe(first.id); - transport.emitMessage({ id: first.id, result: handshake("1.3", "1.3") }); + transport.emitMessage({ id: first.id, result: handshake("1.4", "1.3") }); await Promise.resolve(); expect(controller.snapshot().state).not.toBe("connected"); - transport.emitMessage({ id: second.id, result: handshake("1.3", "1.3") }); + transport.emitMessage({ id: second.id, result: handshake("1.4", "1.3") }); await vi.waitFor(() => expect(controller.snapshot().state).toBe("connected")); }); }); diff --git a/apps/extension/src/lib/__tests__/task-preview.test.ts b/apps/extension/src/lib/__tests__/task-preview.test.ts index 658039cd..bc435364 100644 --- a/apps/extension/src/lib/__tests__/task-preview.test.ts +++ b/apps/extension/src/lib/__tests__/task-preview.test.ts @@ -12,7 +12,7 @@ afterEach(() => { function fixture() { const task = { remote: true, - agentWindowId: 10, + container: { mode: "window" as const, agentWindowId: 10 }, refStore: { documentRevision: () => 1 }, agentCreatedTabs: new Set([5]), borrowedTabs: new Map(), diff --git a/apps/extension/src/lib/task-preview.ts b/apps/extension/src/lib/task-preview.ts index 597bf162..b8f5cce5 100644 --- a/apps/extension/src/lib/task-preview.ts +++ b/apps/extension/src/lib/task-preview.ts @@ -1,6 +1,10 @@ import type { ChromiumCdp } from "@/browser-driver/chromium-cdp"; import type { SessionContext } from "@/session-manager/manager"; -import { isAgentControlledTab, type SessionManager } from "@/session-manager/manager"; +import { + isAgentControlledTab, + type SessionManager, + sessionWindowId, +} from "@/session-manager/manager"; import { checkedUiTab, runTaskUi, @@ -32,7 +36,7 @@ export function captureTaskPreview( if (!claim) return; const tab = await chrome.tabs.get(tabId).catch(() => undefined); if (manager.get(sessionId) !== task) return; - if (!isAgentControlledTab(task, tabId) || (tab && tab.windowId !== task.agentWindowId)) + if (!isAgentControlledTab(task, tabId) || (tab && tab.windowId !== sessionWindowId(task))) await cdp.releaseSessionTab(sessionId, tabId, { ifClaim: claim }); }; const onAbort = () => { @@ -69,7 +73,7 @@ export function focusTask(manager: SessionManager, sessionId: string) { await checkedUiTab(task, op, tabId); await op.mutate(tabId, () => chrome.tabs.update(tabId, { active: true })); await checkedUiTab(task, op, tabId); - await op.mutate(tabId, () => chrome.windows.update(task.agentWindowId, { focused: true })); + await op.mutate(tabId, () => chrome.windows.update(sessionWindowId(task), { focused: true })); await checkedUiTab(task, op, tabId); return { focused: true }; }); @@ -191,7 +195,7 @@ async function downscale(jpegBase64: string, op: UiOperation): Promise { export async function taskTarget(manager: SessionManager, sessionId: string): Promise { const task = manager.get(sessionId); if (!task?.remote) throw new UiTaskError("not_found", "Task unavailable", "task_unavailable"); - const tabs = await chrome.tabs.query({ windowId: task.agentWindowId }); + const tabs = await chrome.tabs.query({ windowId: sessionWindowId(task) }); if (manager.get(sessionId) !== task) throw new UiTaskError("not_found", "Task unavailable", "task_unavailable"); const owned = tabs.filter( diff --git a/apps/extension/src/session-manager/__tests__/event-handler.test.ts b/apps/extension/src/session-manager/__tests__/event-handler.test.ts index e7af4ac3..e8dd7aaf 100644 --- a/apps/extension/src/session-manager/__tests__/event-handler.test.ts +++ b/apps/extension/src/session-manager/__tests__/event-handler.test.ts @@ -131,15 +131,26 @@ describe("attachSessionEventHandler", () => { ]); }); - it("reports borrowed tabs as return failures when the Agent Window was already closed", async () => { + it.each([ + false, + true, + ])("reports borrowed-tab failures when the session window closes (shared=%s)", async (inWindow) => { + const removeWindow = vi.fn(async () => {}); + const removeTab = vi.fn(async () => {}); const manager = new SessionManager({ agentWindow: { create: vi.fn(async () => ({ windowId: 4242, initialTabIds: [] })), - remove: vi.fn(async () => {}), + remove: removeWindow, ensureActiveTab: vi.fn(async () => 1), }, + sharedWindow: { + host: async () => ({ id: 4242, type: "normal", incognito: false }) as chrome.windows.Window, + create: async () => 1, + get: async () => ({ id: 1, windowId: 4242 }) as chrome.tabs.Tab, + remove: removeTab, + }, }); - const ctx = await manager.start("aa11"); + const ctx = await manager.start("aa11", { inWindow }); ctx.borrowedTabs.set(7, { tabId: 7, originalWindowId: 200, originalIndex: 0 }); const transport = fakeTransport(); const events = fakeWindowEvents(); @@ -152,7 +163,10 @@ describe("attachSessionEventHandler", () => { manager.forgetClosedTab(7, { isWindowClosing: true }); events.emit(4242); - for (let i = 0; i < 4; i += 1) await Promise.resolve(); + await vi.waitUntil(() => transport.sent.length > 0); + expect(manager.has("aa11")).toBe(false); + expect(removeWindow).not.toHaveBeenCalled(); + expect(removeTab).not.toHaveBeenCalled(); expect(transport.sent).toEqual([ { @@ -164,7 +178,7 @@ describe("attachSessionEventHandler", () => { { tab_id: 7, code: "cdp_failed", - message: "Agent Window was closed before borrowed tab could be returned", + message: "Session window was closed before borrowed tab could be returned", }, ], }, diff --git a/apps/extension/src/session-manager/__tests__/manager.test.ts b/apps/extension/src/session-manager/__tests__/manager.test.ts index 357414e7..06b61b57 100644 --- a/apps/extension/src/session-manager/__tests__/manager.test.ts +++ b/apps/extension/src/session-manager/__tests__/manager.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import { sessionWindowId } from "@/session-manager/manager"; import type { AgentWindowApi, AgentWindowCreateOptions, @@ -38,7 +39,7 @@ describe("SessionManager", () => { expect(aw.ensureActiveTabMock).toHaveBeenCalledOnce(); expect(aw.ensureActiveTabMock).toHaveBeenCalledWith(100, "about:blank", expect.any(Set)); expect(ctx.sessionId).toBe("aa11"); - expect(ctx.agentWindowId).toBe(100); + expect(sessionWindowId(ctx)).toBe(100); expect(ctx.createdAtMs).toBe(1700000000000); expect(ctx.refStore.isEmpty()).toBe(true); expect(ctx.borrowedTabs.size).toBe(0); @@ -50,7 +51,7 @@ describe("SessionManager", () => { expect(aw.createMock).toHaveBeenCalledWith("about:blank", { size: { width: 1280, height: 800 }, }); - expect(ctx.agentWindowId).toBe(100); + expect(sessionWindowId(ctx)).toBe(100); }); it("forwards an explicit unfocused start to the Agent Window", async () => { @@ -68,7 +69,7 @@ describe("SessionManager", () => { const ctx = await sm.start("aa11"); expect(sm.has("aa11")).toBe(true); expect(sm.get("aa11")).toBe(ctx); - expect(sm.findByWindowId(ctx.agentWindowId)).toBe(ctx); + expect(sm.findByWindowId(sessionWindowId(ctx))).toBe(ctx); expect(sm.findByWindowId(99999)).toBeNull(); expect(sm.list().length).toBe(1); }); @@ -136,9 +137,9 @@ describe("SessionManager", () => { const ctx = await sm.start("aa11"); const removed = await sm.stop("aa11"); expect(removed).toBe(ctx); - expect(aw.removeMock).toHaveBeenCalledWith(ctx.agentWindowId); + expect(aw.removeMock).toHaveBeenCalledWith(sessionWindowId(ctx)); expect(sm.has("aa11")).toBe(false); - expect(sm.findByWindowId(ctx.agentWindowId)).toBeNull(); + expect(sm.findByWindowId(sessionWindowId(ctx))).toBeNull(); }); it("stop({ dropOnly: true }) skips the chrome.windows.remove call", async () => { diff --git a/apps/extension/src/session-manager/__tests__/shared-window.test.ts b/apps/extension/src/session-manager/__tests__/shared-window.test.ts new file mode 100644 index 00000000..1d47751c --- /dev/null +++ b/apps/extension/src/session-manager/__tests__/shared-window.test.ts @@ -0,0 +1,397 @@ +import { describe, expect, it, vi } from "vitest"; +import { handleSessionStart, handleSessionStop } from "@/tools/session"; +import { enforceAgentWindow, resolveTargetTab } from "@/tools/shared"; +import { + handleTabBorrow, + handleTabClose, + handleTabCreate, + handleTabList, + handleTabReturn, +} from "@/tools/tabs"; +import { attachSessionEventHandler } from "../event-handler"; +import { SessionManager } from "../manager"; + +function fixture() { + let nextId = 20; + const pages = new Map([ + [ + 1, + { + id: 1, + windowId: 10, + active: true, + index: 0, + url: "https://user.example/", + } as chrome.tabs.Tab, + ], + ]); + const host = vi.fn( + async () => ({ id: 10, type: "normal", incognito: false }) as chrome.windows.Window, + ); + const get = vi.fn(async (id: number) => { + const tab = pages.get(id); + if (!tab) throw new Error("No tab with id"); + return tab; + }); + const remove = vi.fn(async (id: number) => { + pages.delete(id); + }); + const create = vi.fn(async (windowId: number, active: boolean) => { + const id = nextId++; + pages.set(id, { + id, + windowId, + active, + index: id, + url: "https://agent.example/", + } as chrome.tabs.Tab); + return id; + }); + const windows = { + create: vi.fn(async () => ({ windowId: 99, initialTabIds: [] })), + ensureActiveTab: vi.fn(async () => 90), + remove: vi.fn(async () => {}), + }; + const manager = new SessionManager({ + agentWindow: windows, + sharedWindow: { host, get, create, remove }, + }); + const query = vi.fn(async () => [...pages.values()]); + return { manager, pages, host, get, create, remove, windows, query }; +} + +describe("shared user window sessions (#243)", () => { + it("creates a new inactive page without claiming user pages or indexing the host as owned", async () => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true, focused: false }); + expect(f.create).toHaveBeenCalledWith(10, false); + expect([...ctx.agentCreatedTabs]).toEqual([20]); + expect(f.manager.findByWindowId(10)).toBeNull(); + expect(f.windows.create).not.toHaveBeenCalled(); + }); + + it.each([ + true, + false, + ])("rejects an ineligible last-focused host (incognito=%s)", async (incognito) => { + const f = fixture(); + f.host.mockResolvedValue({ id: 99, type: "normal", incognito } as chrome.windows.Window); + if (!incognito) await f.manager.start("owned"); + await expect(f.manager.start("a", { inWindow: true })).rejects.toThrow("Focus"); + expect(f.create).not.toHaveBeenCalled(); + }); + + it.each([ + false, + true, + ])("isolates two sessions regardless of registration order (%s)", async (reverse) => { + const f = fixture(); + for (const id of reverse ? ["b", "a"] : ["a", "b"]) + await f.manager.start(id, { inWindow: true }); + const a = f.manager.get("a")!; + const b = f.manager.get("b")!; + const api = { get: f.get, query: f.query }; + expect(await resolveTargetTab(f.manager, a, b.activeTabId, api)).toMatchObject({ + code: "not_found", + }); + expect(await resolveTargetTab(f.manager, b, a.activeTabId, api)).toMatchObject({ + code: "not_found", + }); + expect(enforceAgentWindow(a, { tabId: 1, windowId: 10 }, "click")).toMatchObject({ + code: "permission_denied", + }); + expect(await resolveTargetTab(f.manager, a, undefined, api)).toMatchObject({ + tabId: a.activeTabId, + }); + const list = await handleTabList(f.manager, { session_id: "a", scope: "all" }, api); + expect(list).toMatchObject({ + tabs: [ + { tab_id: 1, scope: "user" }, + { tab_id: a.activeTabId, scope: "agent" }, + ], + }); + await f.manager.stop("a"); + expect(f.pages.has(1)).toBe(true); + expect(f.pages.has(b.activeTabId!)).toBe(true); + expect(f.manager.get("b")).toBe(b); + expect(f.windows.remove).not.toHaveBeenCalled(); + }); + + it("does not accept dialogs on user pages, even if those pages have been read", async () => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + expect(await resolveTargetTab(f.manager, ctx, 1, { get: f.get, query: f.query })).toMatchObject( + { tabId: 1 }, + ); + expect(f.manager.canAutoAcceptDialog(1, 10)).toBe(false); + expect(f.manager.canAutoAcceptDialog(ctx.activeTabId!, 10)).toBe(true); + expect(f.manager.canAutoAcceptDialog(ctx.activeTabId!, 11)).toBe(false); + }); + + it("falls back to a live controlled page when the remembered active tab is gone", async () => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + ctx.activeTabId = 999; + expect( + await resolveTargetTab(f.manager, ctx, undefined, { get: f.get, query: f.query }), + ).toMatchObject({ tabId: 20 }); + }); + + it("normal tool stop never queries or removes the host window", async () => { + const f = fixture(); + await f.manager.start("a", { inWindow: true }); + f.query.mockRejectedValue(new Error("query denied")); + expect( + await handleSessionStop( + f.manager, + { session_id: "a" }, + { tabsQuery: { get: f.get, query: f.query } }, + ), + ).toEqual({}); + expect(f.query).not.toHaveBeenCalled(); + expect(f.windows.remove).not.toHaveBeenCalled(); + expect(f.pages.has(1)).toBe(true); + }); + + it.each([ + "stop", + "stopAll", + ] as const)("direct %s keeps retryable state on deletion failure", async (method) => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + f.remove.mockRejectedValue(new Error("delete denied")); + await expect(method === "stop" ? f.manager.stop("a") : f.manager.stopAll()).rejects.toThrow( + "delete denied", + ); + expect(f.manager.get("a")).toBe(ctx); + expect(ctx.agentCreatedTabs.has(20)).toBe(true); + expect(f.windows.remove).not.toHaveBeenCalled(); + }); + + it("cancellation after creation removes only the created page", async () => { + const f = fixture(); + const abort = new AbortController(); + f.create.mockImplementation(async () => { + abort.abort(); + return 20; + }); + await expect(f.manager.start("a", { inWindow: true, signal: abort.signal })).rejects.toThrow( + "aborted", + ); + expect(f.remove).toHaveBeenCalledWith(20); + expect(f.windows.remove).not.toHaveBeenCalled(); + }); + + it("rejects dimensions and remote mode before creating resources", async () => { + const f = fixture(); + expect( + await handleSessionStart(f.manager, { + session_id: "a", + in_window: true, + width: 800, + height: 600, + }), + ).toMatchObject({ code: "invalid_params" }); + const remote = new SessionManager({ remote: () => true }); + expect(await handleSessionStart(remote, { session_id: "a", in_window: true })).toMatchObject({ + code: "unsupported", + }); + expect(f.create).not.toHaveBeenCalled(); + }); + + it("reports failed cancellation cleanup as a tab resource and never closes the host", async () => { + const f = fixture(); + const abort = new AbortController(); + f.create.mockImplementation(async () => { + abort.abort(); + return 20; + }); + f.remove.mockRejectedValue(new Error("delete denied")); + expect( + await handleSessionStart( + f.manager, + { session_id: "a", in_window: true }, + { signal: abort.signal }, + ), + ).toMatchObject({ + code: "protocol_error", + data: { reason: "cleanup_failed", resource_type: "tab", resource_id: 20 }, + }); + expect(f.manager.get("a")?.agentCreatedTabs.has(20)).toBe(true); + expect(f.windows.remove).not.toHaveBeenCalled(); + }); + + it("does not close reclaimed pages or the host during normal stop", async () => { + const f = fixture(); + await f.manager.start("a", { inWindow: true }); + f.pages.get(20)!.windowId = 11; + await f.manager.stop("a"); + expect(f.pages.has(20)).toBe(true); + expect(f.remove).not.toHaveBeenCalled(); + expect(f.windows.remove).not.toHaveBeenCalled(); + }); + + it("does not roll back a page reclaimed during startup by deleting it", async () => { + const f = fixture(); + f.get.mockImplementation(async (id) => ({ ...f.pages.get(id)!, windowId: 11 })); + await expect(f.manager.start("a", { inWindow: true })).rejects.toThrow("moved"); + expect(f.manager.has("a")).toBe(false); + expect(f.pages.has(20)).toBe(true); + expect(f.remove).not.toHaveBeenCalled(); + expect(f.windows.remove).not.toHaveBeenCalled(); + }); + + it("preserves cleanup state when a live-page lookup fails", async () => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + f.get.mockRejectedValue(new Error("query denied")); + await expect(f.manager.stop("a")).rejects.toThrow("query denied"); + expect(f.manager.get("a")).toBe(ctx); + expect(ctx.agentCreatedTabs.has(20)).toBe(true); + expect(f.windows.remove).not.toHaveBeenCalled(); + }); + + it("defers empty lifecycle until a tab transaction commits and refuses concurrent stop", async () => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + const empty = vi.fn(); + f.manager.onEmpty(empty); + await f.manager.withTabOperation(ctx, async () => { + f.manager.forgetClosedTab(20); + expect(empty).not.toHaveBeenCalled(); + await expect(f.manager.stop("a")).rejects.toThrow("pending"); + ctx.agentCreatedTabs.add(21); + }); + expect(empty).not.toHaveBeenCalled(); + f.manager.forgetClosedTab(21); + expect(empty).toHaveBeenCalledOnce(); + expect(f.windows.remove).not.toHaveBeenCalled(); + }); + + it("defers empty cleanup until tab_close finishes after Chrome's removal event", async () => { + const f = fixture(); + await f.manager.start("a", { inWindow: true }); + const empty = vi.fn(); + f.manager.onEmpty(empty); + f.remove.mockImplementation(async (id) => { + f.pages.delete(id); + f.manager.forgetClosedTab(id); + expect(empty).not.toHaveBeenCalled(); + }); + const result = await handleTabClose( + f.manager, + { session_id: "a", tab_id: 20 }, + { tabs: { get: f.get, remove: f.remove, create: vi.fn(), move: vi.fn(), update: vi.fn() } }, + ); + expect(result).toEqual({ tab_id: 20 }); + expect(empty).toHaveBeenCalledOnce(); + }); + + it.each([ + true, + false, + ])("does not close a created tab moved during CDP setup (attach event=%s)", async (attachEvent) => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + const empty = vi.fn(); + f.manager.onEmpty(empty); + const update = vi.fn(); + const releaseSessionTab = vi.fn(async () => {}); + const result = await handleTabCreate( + f.manager, + { session_id: "a", url: "https://agent.example/new" }, + { + tabs: { + create: async (props) => { + const id = await f.create(props.windowId!, props.active ?? true); + return f.get(id); + }, + get: f.get, + remove: f.remove, + move: vi.fn(), + update, + }, + cdp: { + acquireBackgroundExecution: vi.fn(async (_sessionId, tabId) => { + expect(ctx.pendingOperations).toBe(1); + f.pages.delete(20); + f.manager.forgetClosedTab(20); + f.pages.get(tabId)!.windowId = 11; + if (attachEvent) ctx.agentCreatedTabs.delete(tabId); + f.manager.checkEmpty(ctx); + expect(empty).not.toHaveBeenCalled(); + }), + releaseSessionTab, + }, + }, + ); + expect(result).toMatchObject({ code: "cdp_failed" }); + expect(f.pages.get(21)?.windowId).toBe(11); + expect(f.remove).not.toHaveBeenCalled(); + expect(update).not.toHaveBeenCalled(); + expect(ctx.agentCreatedTabs.has(21)).toBe(false); + expect(releaseSessionTab).toHaveBeenCalledWith("a", 21); + expect(empty).toHaveBeenCalledOnce(); + }); + + it("borrows and returns a same-window user page without moving or closing it", async () => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + const move = vi.fn(); + const releaseSessionTab = vi.fn(async () => {}); + const deps = { + tabs: { + get: f.get, + create: vi.fn(), + remove: f.remove, + move, + update: vi.fn(async () => f.pages.get(1)!), + }, + approveBorrow: vi.fn(async () => true), + cdp: { releaseSessionTab }, + overlayReset: { resetAgentOverlays: vi.fn(async () => {}) }, + }; + expect(await handleTabBorrow(f.manager, { session_id: "a", tab_id: 1 }, deps)).toMatchObject({ + tab_id: 1, + }); + expect(ctx.borrowedTabs.get(1)?.originalWindowId).toBe(10); + expect(move).not.toHaveBeenCalled(); + f.pages.get(1)!.windowId = 11; + expect(await handleTabReturn(f.manager, { session_id: "a", tab_id: 1 }, deps)).toMatchObject({ + tab_id: 1, + }); + expect(releaseSessionTab).toHaveBeenCalledWith("a", 1); + await f.manager.stop("a"); + expect(f.pages.has(1)).toBe(true); + expect(f.pages.get(1)?.windowId).toBe(11); + expect(move).not.toHaveBeenCalled(); + }); + + it("resolves an empty target without ending the session; lifecycle ends it once", async () => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + const send = vi.fn(); + const detachSession = vi.fn(async () => {}); + const events = { addListener: vi.fn(), removeListener: vi.fn() }; + const handler = attachSessionEventHandler({ + manager: f.manager, + transport: { send } as never, + windowEvents: events, + cdp: { detachSession }, + }); + f.pages.delete(20); + expect( + await resolveTargetTab(f.manager, ctx, undefined, { get: f.get, query: f.query }), + ).toMatchObject({ code: "not_found" }); + expect(f.manager.has("a")).toBe(true); + f.manager.forgetClosedTab(20); + f.manager.forgetClosedTab(20); + await vi.waitFor(() => expect(f.manager.has("a")).toBe(false)); + expect(send).toHaveBeenCalledOnce(); + expect(send).toHaveBeenCalledWith({ + event: "session.tabs_closed", + payload: { session_id: "a", reason: "no_controlled_tabs" }, + }); + handler.dispose(); + }); +}); diff --git a/apps/extension/src/session-manager/event-handler.ts b/apps/extension/src/session-manager/event-handler.ts index a62c0dcc..2f232ba3 100644 --- a/apps/extension/src/session-manager/event-handler.ts +++ b/apps/extension/src/session-manager/event-handler.ts @@ -19,8 +19,8 @@ export interface SessionEventHandlerOptions { detachSession(sessionId: string): Promise; }; /** - * Invoked after a user-closed Agent Window has been removed from the - * SessionManager. Lets the caller refresh side caches such as the + * Invoked after a session has been removed from the SessionManager. + * Lets the caller refresh side caches such as the * `chrome.storage.session` "sessions live" flag (review M4/M5 I3). */ onSessionsChanged?: () => void; @@ -34,7 +34,7 @@ function chromeWindowEvents(): WindowRemovedListener { } /** - * Watch for the user closing an Agent Window. When that happens we: + * Watch for the user closing a dedicated or shared session window. We: * 1. Drop the local SessionContext (without trying to close the window * again — it's already gone). * 2. Emit a `session.window_closed` event to the daemon so it can @@ -49,54 +49,78 @@ export function attachSessionEventHandler(options: SessionEventHandlerOptions): const events = options.windowEvents ?? chromeWindowEvents(); const onRemoved = (windowId: number): void => { - const ctx = manager.findByWindowId(windowId); - if (!ctx) return; - // Chrome also emits onRemoved when session.stop removes the last tab or - // the window itself. Capture the cause before asynchronous cleanup so the - // normal stop response, rather than a user-close event, ends the audit. - const expectedClose = manager.isWindowCloseExpected(ctx); - const returnFailures = Array.from(ctx.borrowedTabs.keys()).map((tabId) => ({ - tab_id: tabId, - code: "cdp_failed", - message: "Agent Window was closed before borrowed tab could be returned", - })); - if (returnFailures.length > 0) { - console.warn( - `[bh] Agent Window ${windowId} closed with borrowed tabs that could not be returned`, - returnFailures, - ); - } - const detach = options.cdp - ? options.cdp.detachSession(ctx.sessionId).catch((err) => { - console.debug("[bh] session-event cdp detach failed", err); + for (const ctx of manager.sessionsInWindow(windowId)) { + // Chrome also emits onRemoved when session.stop removes the last tab or + // the window itself. Capture the cause before asynchronous cleanup so the + // normal stop response, rather than a user-close event, ends the audit. + const expectedClose = manager.isWindowCloseExpected(ctx); + if (ctx.container.mode === "in_window") ctx.stopping = true; + const returnFailures = Array.from(ctx.borrowedTabs.keys()).map((tabId) => ({ + tab_id: tabId, + code: "cdp_failed", + message: "Session window was closed before borrowed tab could be returned", + })); + if (returnFailures.length > 0) { + console.warn( + `[bh] Session window ${windowId} closed with borrowed tabs that could not be returned`, + returnFailures, + ); + } + const detach = options.cdp + ? options.cdp.detachSession(ctx.sessionId).catch((err) => { + console.debug("[bh] session-event cdp detach failed", err); + }) + : Promise.resolve(); + void detach + .then(() => manager.stop(ctx.sessionId, { dropOnly: true })) + .then((removed) => { + if (!removed) return; + onSessionsChanged?.(); + if (expectedClose) return; + const event: EventFrame = { + event: "session.window_closed", + payload: { + session_id: ctx.sessionId, + reason: "user_closed_window", + ...(returnFailures.length > 0 ? { return_failures: returnFailures } : {}), + }, + }; + try { + transport.send(event); + } catch (err) { + console.warn("[bh] could not push session.window_closed event", err); + } }) - : Promise.resolve(); - void detach + .catch((err) => { + console.warn("[bh] session-event handler failed", err); + }); + } + }; + + const disposeEmpty = manager.onEmpty((ctx) => { + if (ctx.stopping) return; + ctx.stopping = true; + void (options.cdp?.detachSession(ctx.sessionId) ?? Promise.resolve()) .then(() => manager.stop(ctx.sessionId, { dropOnly: true })) - .then(() => { + .then((removed) => { + if (!removed) return; onSessionsChanged?.(); - if (expectedClose) return; - const event: EventFrame = { - event: "session.window_closed", - payload: { - session_id: ctx.sessionId, - reason: "user_closed_window", - ...(returnFailures.length > 0 ? { return_failures: returnFailures } : {}), - }, - }; - try { - transport.send(event); - } catch (err) { - console.warn("[bh] could not push session.window_closed event", err); - } + transport.send({ + event: "session.tabs_closed", + payload: { session_id: ctx.sessionId, reason: "no_controlled_tabs" }, + }); }) .catch((err) => { - console.warn("[bh] session-event handler failed", err); + ctx.stopping = false; + console.warn("[bh] empty session cleanup failed", err); }); - }; + }); events.addListener(onRemoved); return { - dispose: () => events.removeListener(onRemoved), + dispose: () => { + events.removeListener(onRemoved); + disposeEmpty(); + }, }; } diff --git a/apps/extension/src/session-manager/manager.ts b/apps/extension/src/session-manager/manager.ts index 3e351d3e..9f00ba9e 100644 --- a/apps/extension/src/session-manager/manager.ts +++ b/apps/extension/src/session-manager/manager.ts @@ -1,11 +1,17 @@ import { AGENT_WINDOW_HOME, type AgentWindowApi, chromeAgentWindowApi } from "./agent-window"; import { RefStore } from "./ref-store"; +import { chromeSharedWindowApi, type SharedWindowApi } from "./shared-window"; export interface SessionContext { /** Remote connections retain dedicated windows, with explicit page ownership. */ remote?: boolean; sessionId: string; - agentWindowId: number; + container: + | { mode: "window"; agentWindowId: number } + | { mode: "in_window"; hostWindowId: number }; + activeTabId?: number; + pendingOperations?: number; + stopping?: boolean; refStore: RefStore; borrowedTabs: Map; /** @@ -19,6 +25,14 @@ export interface SessionContext { createdAtMs: number; } +export function sessionWindowId(ctx: SessionContext): number { + return ctx.container.mode === "window" ? ctx.container.agentWindowId : ctx.container.hostWindowId; +} + +export function isSharedSession(ctx: SessionContext): boolean { + return ctx.container.mode === "in_window"; +} + /** Whether this session has explicitly claimed control of `tabId`. */ export function isAgentControlledTab(ctx: SessionContext, tabId: number): boolean { return ( @@ -34,6 +48,17 @@ export interface BorrowedTab { originalIndex: number; } +/** Resolve a shared session's preferred live tab using only its owned pages. */ +export function preferredSharedTab( + ctx: SessionContext, + tabs: chrome.tabs.Tab[], +): chrome.tabs.Tab | undefined { + const owned = tabs.filter((tab) => tab.id !== undefined && isAgentControlledTab(ctx, tab.id)); + return ( + owned.find((tab) => tab.id === ctx.activeTabId) ?? owned.sort((a, b) => a.index - b.index)[0] + ); +} + export interface BorrowReservation { release(): void; commit(entry: BorrowedTab): void; @@ -42,11 +67,13 @@ export interface BorrowReservation { export interface SessionManagerOptions { remote?: () => boolean; agentWindow?: AgentWindowApi; + sharedWindow?: SharedWindowApi; now?: () => number; } /** Options for starting a session's Agent Window. */ export interface SessionStartOptions { + inWindow?: boolean; /** Optional Agent Window outer size in CSS pixels. */ size?: { width: number; height: number }; /** Defaults to true so existing clients keep visible Agent Windows. */ @@ -75,6 +102,19 @@ export class SessionStartCleanupError extends Error { } } +export class SharedSessionStartCleanupError extends Error { + constructor( + readonly tabId: number, + startupError: unknown, + cleanupError: unknown, + ) { + super( + `Session startup failed: ${String(startupError)}; cleanup of tab ${tabId} failed: ${String(cleanupError)}`, + ); + this.name = "SharedSessionStartCleanupError"; + } +} + function sessionStartAbortError(): Error { const error = new Error("session_start aborted"); error.name = "AbortError"; @@ -105,11 +145,15 @@ export class SessionManager { private readonly expectedWindowClosures = new WeakSet(); private readonly agentWindow: AgentWindowApi; private readonly now: () => number; + private readonly sharedWindow: SharedWindowApi; + private readonly starting = new Set(); + private readonly emptyListeners = new Set<(ctx: SessionContext) => void>(); constructor(options: SessionManagerOptions = {}) { this.remote = options.remote ?? (() => false); this.agentWindow = options.agentWindow ?? chromeAgentWindowApi; this.now = options.now ?? Date.now; + this.sharedWindow = options.sharedWindow ?? chromeSharedWindowApi; } has(sessionId: string): boolean { @@ -124,7 +168,7 @@ export class SessionManager { return this.expectedWindowClosures.has(ctx); } - /** Mark only the committed window/tab removal stage of session.stop. */ + /** Suppress close/empty callbacks while session.stop owns teardown. */ async withExpectedWindowClose(ctx: SessionContext, close: () => Promise): Promise { this.expectedWindowClosures.add(ctx); try { @@ -144,6 +188,59 @@ export class SessionManager { return Array.from(this.sessions.values()); } + isRemote(): boolean { + return this.remote(); + } + + findByTabId(tabId: number): SessionContext | null { + return this.list().find((ctx) => isAgentControlledTab(ctx, tabId)) ?? null; + } + + canAutoAcceptDialog(tabId: number, windowId: number): boolean { + const ctx = this.findByTabId(tabId) ?? this.findByWindowId(windowId); + return ( + ctx !== null && + !ctx.stopping && + sessionWindowId(ctx) === windowId && + ((!ctx.remote && ctx.container.mode === "window") || isAgentControlledTab(ctx, tabId)) + ); + } + + sessionsInWindow(windowId: number): SessionContext[] { + return this.list().filter((ctx) => sessionWindowId(ctx) === windowId); + } + + onEmpty(listener: (ctx: SessionContext) => void): () => void { + this.emptyListeners.add(listener); + return () => this.emptyListeners.delete(listener); + } + + checkEmpty(ctx: SessionContext): void { + if ( + this.get(ctx.sessionId) !== ctx || + !isSharedSession(ctx) || + ctx.stopping || + ctx.pendingOperations || + this.isWindowCloseExpected(ctx) || + ctx.agentCreatedTabs.size || + ctx.borrowedTabs.size || + ctx.observedTabs?.size + ) + return; + for (const listener of this.emptyListeners) listener(ctx); + } + + async withTabOperation(ctx: SessionContext, action: () => Promise): Promise { + if (ctx.stopping || this.get(ctx.sessionId) !== ctx) throw new Error("Session is stopping"); + ctx.pendingOperations = (ctx.pendingOperations ?? 0) + 1; + try { + return await action(); + } finally { + ctx.pendingOperations--; + this.checkEmpty(ctx); + } + } + invalidateTabRefs(tabId: number): void { for (const ctx of this.sessions.values()) ctx.refStore.invalidateTab(tabId); } @@ -160,6 +257,7 @@ export class SessionManager { ctx.agentCreatedTabs.delete(tabId); ctx.observedTabs?.delete(tabId); if (!isWindowClosing) ctx.borrowedTabs.delete(tabId); + if (!isWindowClosing) this.checkEmpty(ctx); } } @@ -192,7 +290,7 @@ export class SessionManager { findBorrowingSession(tabId: number, currentSessionId: string | null): string | null { for (const ctx of this.sessions.values()) { if (ctx.sessionId === currentSessionId) continue; - if (ctx.borrowedTabs.has(tabId)) return ctx.sessionId; + if (isAgentControlledTab(ctx, tabId)) return ctx.sessionId; } const reservedBy = this.borrowReservations.get(tabId); if (reservedBy && reservedBy !== currentSessionId) return reservedBy; @@ -225,7 +323,7 @@ export class SessionManager { commit: (entry) => { if (closed) return; const ctx = this.sessions.get(sessionId); - if (!ctx) { + if (!ctx || ctx.stopping) { release(); throw new Error(`session ${sessionId} disappeared during tab_borrow`); } @@ -239,18 +337,18 @@ export class SessionManager { } /** - * Spin up a fresh session: open a new Agent Window with an - * `about:blank` tab and register the context. + * Create a dedicated window or a shared-host tab and register the context. * - * Returns the created window id so callers can echo it back to the - * daemon in the `tool.session_start` reply. + * The context records window location separately from resource ownership. */ async start(sessionId: string, opts: SessionStartOptions = {}): Promise { - if (this.sessions.has(sessionId)) { + if (this.sessions.has(sessionId) || this.starting.has(sessionId)) { throw new Error(`[bh] session ${sessionId} already exists`); } throwIfSessionStartAborted(opts.signal); + if (opts.inWindow) return this.startShared(sessionId, opts); + let windowId: number | null = null; const agentCreatedTabs = new Set(); try { @@ -270,7 +368,7 @@ export class SessionManager { const ctx: SessionContext = { ...(this.remote() ? { remote: true } : {}), sessionId, - agentWindowId: windowId, + container: { mode: "window", agentWindowId: windowId }, refStore: new RefStore(), borrowedTabs: new Map(), // Capture ownership at creation, before initialization can fail. @@ -292,7 +390,7 @@ export class SessionManager { const pending: SessionContext = { ...(this.remote() ? { remote: true } : {}), sessionId, - agentWindowId: windowId, + container: { mode: "window", agentWindowId: windowId }, refStore: new RefStore(), borrowedTabs: new Map(), agentCreatedTabs, @@ -307,8 +405,68 @@ export class SessionManager { } } + private async startShared(sessionId: string, opts: SessionStartOptions): Promise { + if (this.remote()) throw new Error("Shared windows are unsupported for remote connections"); + if (opts.size) throw new Error("Window dimensions cannot be used with in_window"); + this.starting.add(sessionId); + let tabId: number | undefined; + let ctx: SessionContext | undefined; + let reclaimed = false; + try { + const host = await this.sharedWindow.host(); + if ( + host.id === undefined || + host.incognito || + host.type !== "normal" || + this.findByWindowId(host.id) + ) { + throw new Error("Focus a normal user window before starting an in-window session"); + } + throwIfSessionStartAborted(opts.signal); + tabId = await this.sharedWindow.create(host.id, opts.focused !== false); + ctx = { + sessionId, + container: { mode: "in_window", hostWindowId: host.id }, + activeTabId: tabId, + refStore: new RefStore(), + borrowedTabs: new Map(), + agentCreatedTabs: new Set([tabId]), + createdAtMs: this.now(), + }; + throwIfSessionStartAborted(opts.signal); + const tab = await this.sharedWindow.get(tabId); + if (tab.windowId !== host.id) { + reclaimed = true; + throw new Error("Session tab moved during startup"); + } + if (opts.focused !== false) await this.sharedWindow.focus?.(host.id); + const finalTab = await this.sharedWindow.get(tabId); + if (finalTab.windowId !== host.id) { + reclaimed = true; + throw new Error("Session tab moved during startup"); + } + throwIfSessionStartAborted(opts.signal); + this.sessions.set(sessionId, ctx); + return ctx; + } catch (error) { + if (tabId !== undefined && !reclaimed) { + try { + await this.sharedWindow.remove(tabId); + } catch (cleanup) { + if (/No tab with id|Invalid tab ID|not found/i.test(String(cleanup))) throw error; + // Keep a retryable claim, never use window removal as a fallback. + if (ctx) this.sessions.set(sessionId, ctx); + throw new SharedSessionStartCleanupError(tabId, error, cleanup); + } + } + throw error; + } finally { + this.starting.delete(sessionId); + } + } + /** - * Tear down a session: close its Agent Window and drop the context. + * Tear down owned resources and drop the context. Shared hosts are never removed. * * `dropOnly = true` skips closing the window — used when the user * already closed it manually (M5.4 path) so we don't accidentally @@ -321,10 +479,35 @@ export class SessionManager { const ctx = this.sessions.get(sessionId); if (!ctx) return null; if (!options.dropOnly) { - await this.agentWindow.remove(ctx.agentWindowId); + if (ctx.container.mode === "window") + await this.agentWindow.remove(ctx.container.agentWindowId); + else { + if (ctx.pendingOperations) throw new Error("Session has pending tab operations"); + // Borrowed pages must be returned by the tool-level teardown first. + if (ctx.borrowedTabs.size) + throw new Error("Return borrowed tabs before stopping this session"); + ctx.stopping = true; + try { + for (const tabId of [...ctx.agentCreatedTabs]) { + try { + const tab = await this.sharedWindow.get(tabId); + // Moving a shared session page out is a user reclaim, including + // when onAttached has not yet reached the service worker. + if (tab.windowId === ctx.container.hostWindowId) + await this.sharedWindow.remove(tabId); + } catch (err) { + if (!/No tab with id|Invalid tab ID|not found/i.test(String(err))) throw err; + } + ctx.agentCreatedTabs.delete(tabId); + } + } catch (err) { + ctx.stopping = false; + throw err; + } + } } this.sessions.delete(sessionId); - this.windowIndex.delete(ctx.agentWindowId); + if (ctx.container.mode === "window") this.windowIndex.delete(ctx.container.agentWindowId); return ctx; } diff --git a/apps/extension/src/session-manager/shared-window.ts b/apps/extension/src/session-manager/shared-window.ts new file mode 100644 index 00000000..4590ea1e --- /dev/null +++ b/apps/extension/src/session-manager/shared-window.ts @@ -0,0 +1,22 @@ +/** A shared session owns tabs, never its host window. */ +export interface SharedWindowApi { + host(): Promise; + create(windowId: number, focused: boolean): Promise; + get(tabId: number): Promise; + remove(tabId: number): Promise; + focus?(windowId: number): Promise; +} + +export const chromeSharedWindowApi: SharedWindowApi = { + host: () => chrome.windows.getLastFocused({ windowTypes: ["normal"] }), + async create(windowId, focused) { + const tab = await chrome.tabs.create({ windowId, url: "about:blank", active: focused }); + if (tab.id === undefined) throw new Error("Could not create session tab"); + return tab.id; + }, + get: (tabId) => chrome.tabs.get(tabId), + remove: (tabId) => chrome.tabs.remove(tabId), + focus: async (windowId) => { + await chrome.windows.update(windowId, { focused: true }); + }, +}; diff --git a/apps/extension/src/session-manager/task-popups.ts b/apps/extension/src/session-manager/task-popups.ts index ead6a38d..9070d290 100644 --- a/apps/extension/src/session-manager/task-popups.ts +++ b/apps/extension/src/session-manager/task-popups.ts @@ -1,4 +1,4 @@ -import { isAgentControlledTab, type SessionManager } from "./manager"; +import { isAgentControlledTab, type SessionManager, sessionWindowId } from "./manager"; const INPUT_WINDOW_MS = 100; const CANDIDATE_BUDGET_MS = 500; @@ -30,15 +30,19 @@ export async function withTaskPopups( manager.get(task.sessionId) === task && !manager.isWindowCloseExpected(task); const validSource = async (id: number) => { - if (!live() || invalid.has(id) || (task.remote && !isAgentControlledTab(task, id))) + if ( + !live() || + invalid.has(id) || + ((task.remote || task.container.mode === "in_window") && !isAgentControlledTab(task, id)) + ) return false; try { const tab = await chrome.tabs.get(id); return ( live() && !invalid.has(id) && - tab.windowId === task.agentWindowId && - (!task.remote || isAgentControlledTab(task, id)) + tab.windowId === sessionWindowId(task) && + (!(task.remote || task.container.mode === "in_window") || isAgentControlledTab(task, id)) ); } catch { return false; @@ -62,7 +66,7 @@ export async function withTaskPopups( if ( !(await validSource(sourceTabId)) || invalid.has(tabId) || - tab.windowId !== task.agentWindowId || + tab.windowId !== sessionWindowId(task) || manager.findBorrowingSession(tabId, task.sessionId) || manager.findControllingSession(tabId) ) diff --git a/apps/extension/src/session-manager/ui-activity.ts b/apps/extension/src/session-manager/ui-activity.ts index f7bf3ebf..245eb89b 100644 --- a/apps/extension/src/session-manager/ui-activity.ts +++ b/apps/extension/src/session-manager/ui-activity.ts @@ -1,5 +1,10 @@ import type { ErrorCode, RpcError, RpcErrorReason } from "@/transport/types"; -import { isAgentControlledTab, type SessionContext, type SessionManager } from "./manager"; +import { + isAgentControlledTab, + type SessionContext, + type SessionManager, + sessionWindowId, +} from "./manager"; export class UiTaskError extends Error { constructor( @@ -175,7 +180,11 @@ export async function checkedUiTab(task: SessionContext, op: UiOperation, tabId: ); } op.check(); - if (tab.id !== tabId || tab.windowId !== task.agentWindowId || !isAgentControlledTab(task, tabId)) + if ( + tab.id !== tabId || + tab.windowId !== sessionWindowId(task) || + !isAgentControlledTab(task, tabId) + ) throw new UiTaskError("not_found", "Task ended during capture", "target_unavailable"); return tab; } diff --git a/apps/extension/src/tools/__tests__/dispatcher.test.ts b/apps/extension/src/tools/__tests__/dispatcher.test.ts index 6e88dba0..614d1a07 100644 --- a/apps/extension/src/tools/__tests__/dispatcher.test.ts +++ b/apps/extension/src/tools/__tests__/dispatcher.test.ts @@ -278,7 +278,9 @@ describe("ToolDispatcher", () => { // ...and the window is released rather than closed, so the user's tab survives. expect(closeWindow).not.toHaveBeenCalled(); expect(sessions.has("aa11")).toBe(false); - expect(sent[0]).toEqual({ id: "r-1", result: { window_released: true } }); + await vi.waitFor(() => { + expect(sent[0]).toEqual({ id: "r-1", result: { window_released: true } }); + }); }); it("routes tool.console through the CDP console buffer", async () => { diff --git a/apps/extension/src/tools/__tests__/human-loop.test.ts b/apps/extension/src/tools/__tests__/human-loop.test.ts index 6472c464..e48357f0 100644 --- a/apps/extension/src/tools/__tests__/human-loop.test.ts +++ b/apps/extension/src/tools/__tests__/human-loop.test.ts @@ -60,8 +60,15 @@ function fakeManager(sessionId: string, agentWindowId: number, tabId: number, un const mgr = { get: (id: string) => id === sessionId - ? { sessionId, agentWindowId, refStore, borrowedTabs: new Map(), unattended } + ? { + sessionId, + container: { mode: "window", agentWindowId }, + refStore, + borrowedTabs: new Map(), + unattended, + } : null, + findByTabId: () => null, findByWindowId: (wid: number) => (wid === agentWindowId ? { sessionId } : null), } as unknown as SessionManager; return mgr; @@ -1006,11 +1013,12 @@ describe("handleRequestHelp", () => { id === "abcd" ? { sessionId: "abcd", - agentWindowId: 99, + container: { mode: "window", agentWindowId: 99 }, refStore, borrowedTabs: new Map(), } : null, + findByTabId: () => null, findByWindowId: (wid: number) => (wid === 99 ? { sessionId: "abcd" } : null), } as unknown as SessionManager; const deps = baseDeps({ @@ -1054,7 +1062,7 @@ describe("handleRequestHelp", () => { id === "abcd" ? { sessionId: "abcd", - agentWindowId: 99, + container: { mode: "window", agentWindowId: 99 }, refStore: { resolveEntry: () => ({ kind: "dom", @@ -1068,6 +1076,7 @@ describe("handleRequestHelp", () => { borrowedTabs: new Map(), } : null, + findByTabId: () => null, findByWindowId: (wid: number) => (wid === 99 ? { sessionId: "abcd" } : null), } as unknown as SessionManager; const send = vi.fn(async (_tabId, method) => { @@ -1149,11 +1158,12 @@ describe("handleRequestHelp", () => { id === "abcd" ? { sessionId: "abcd", - agentWindowId: 99, + container: { mode: "window", agentWindowId: 99 }, refStore, borrowedTabs: new Map(), } : null, + findByTabId: () => null, findByWindowId: (wid: number) => (wid === 99 ? { sessionId: "abcd" } : null), } as unknown as SessionManager; const deps = baseDeps(); diff --git a/apps/extension/src/tools/__tests__/record-steps.test.ts b/apps/extension/src/tools/__tests__/record-steps.test.ts index 504f9076..71211453 100644 --- a/apps/extension/src/tools/__tests__/record-steps.test.ts +++ b/apps/extension/src/tools/__tests__/record-steps.test.ts @@ -81,11 +81,12 @@ function fakeManager() { id === "abcd" ? { sessionId: "abcd", - agentWindowId: AGENT_WINDOW_ID, + container: { mode: "window", agentWindowId: AGENT_WINDOW_ID }, refStore: { resolve: () => null, replace: () => {} }, borrowedTabs: new Map(), } : null, + findByTabId: () => null, findByWindowId: (windowId: number) => windowId === AGENT_WINDOW_ID ? { sessionId: "abcd" } : null, } as unknown as SessionManager; diff --git a/apps/extension/src/tools/__tests__/shared.test.ts b/apps/extension/src/tools/__tests__/shared.test.ts index 970ecb4c..aed541c6 100644 --- a/apps/extension/src/tools/__tests__/shared.test.ts +++ b/apps/extension/src/tools/__tests__/shared.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import { SessionManager } from "@/session-manager/manager"; +import { SessionManager, sessionWindowId } from "@/session-manager/manager"; import { cdpBlockedUrlReason, enforceCdpAccessibleTarget, @@ -117,7 +117,7 @@ describe("CDP target URL guard", () => { return [ { id: 7, - windowId: ctx.agentWindowId, + windowId: sessionWindowId(ctx), active: true, url: "chrome-extension://other-extension/page.html", } as chrome.tabs.Tab, @@ -126,13 +126,13 @@ describe("CDP target URL guard", () => { return [ { id: 7, - windowId: ctx.agentWindowId, + windowId: sessionWindowId(ctx), active: true, url: "chrome-extension://other-extension/page.html", } as chrome.tabs.Tab, { id: 8, - windowId: ctx.agentWindowId, + windowId: sessionWindowId(ctx), active: false, url: "https://example.test/", } as chrome.tabs.Tab, @@ -156,7 +156,7 @@ describe("CDP target URL guard", () => { async () => ({ id: 7, - windowId: ctx.agentWindowId, + windowId: sessionWindowId(ctx), active: true, url: "chrome-extension://other-extension/page.html", }) as chrome.tabs.Tab, diff --git a/apps/extension/src/tools/audit-context.ts b/apps/extension/src/tools/audit-context.ts index 9d61ae44..5a7fb72d 100644 --- a/apps/extension/src/tools/audit-context.ts +++ b/apps/extension/src/tools/audit-context.ts @@ -1,5 +1,10 @@ -import { isAgentControlledTab, type SessionManager } from "@/session-manager/manager"; +import { + isAgentControlledTab, + type SessionManager, + sessionWindowId, +} from "@/session-manager/manager"; import type { RequestFrame } from "@/transport/types"; +import { chromeTabsApi, isRpcError, resolveTargetTab } from "./shared"; /** Read cached element labels and tab metadata only; never stimulate the page. */ export async function auditContext( @@ -12,23 +17,39 @@ export async function auditContext( if (!context) return null; const ref = typeof params.ref === "string" ? context.refStore.resolveEntry(params.ref) : null; const requestedTab = typeof params.tab_id === "number" ? params.tab_id : null; - const tab = - requestedTab !== null - ? await chrome.tabs.get(requestedTab) - : (await chrome.tabs.query({ windowId: context.agentWindowId, active: true }))[0]; - if (!tab?.id || !isAgentControlledTab(context, tab.id)) return null; + let tabId: number | undefined; + let tabUrl: string | undefined; + if (context.container.mode === "in_window") { + const target = await resolveTargetTab( + sessions, + context, + requestedTab ?? undefined, + chromeTabsApi, + ); + if (isRpcError(target)) return null; + tabId = target.tabId; + tabUrl = target.url; + } else { + const tab = + requestedTab !== null + ? await chrome.tabs.get(requestedTab) + : (await chrome.tabs.query({ windowId: sessionWindowId(context), active: true }))[0]; + tabId = tab?.id; + tabUrl = tab?.url; + } + if (!tabId || !isAgentControlledTab(context, tabId)) return null; let url: string | undefined; try { - const parsed = new URL(tab.url ?? ""); + const parsed = new URL(tabUrl ?? ""); if (["http:", "https:"].includes(parsed.protocol)) url = parsed.origin; } catch { /* Restricted or empty URL. */ } return { operation_id: params._audit_id, - tab_id: tab.id, + tab_id: tabId, ...(url ? { url } : {}), - ...(ref?.kind === "dom" && ref.tabId === tab.id && ref.name + ...(ref?.kind === "dom" && ref.tabId === tabId && ref.name ? { target: ref.name.slice(0, 160) } : {}), }; diff --git a/apps/extension/src/tools/background-execution.ts b/apps/extension/src/tools/background-execution.ts index 710c1640..80398761 100644 --- a/apps/extension/src/tools/background-execution.ts +++ b/apps/extension/src/tools/background-execution.ts @@ -1,4 +1,8 @@ -import { isAgentControlledTab, type SessionManager } from "@/session-manager/manager"; +import { + isAgentControlledTab, + type SessionManager, + sessionWindowId, +} from "@/session-manager/manager"; import type { RequestFrame, RpcError } from "@/transport/types"; import { type CdpRunner, @@ -67,7 +71,7 @@ export async function prepareBackgroundExecution( ) ) return; - if (!isAgentControlledTab(ctx, target.tabId) || target.windowId !== ctx.agentWindowId) return; + if (!isAgentControlledTab(ctx, target.tabId) || target.windowId !== sessionWindowId(ctx)) return; // Other page tools cannot establish execution on browser-internal documents. if (cdpBlockedUrlReason(target.url)) return; if (signal.aborted) return { code: "cancelled", message: "Background execution setup cancelled" }; diff --git a/apps/extension/src/tools/borrow-confirmation.ts b/apps/extension/src/tools/borrow-confirmation.ts index ae91c278..d583d23c 100644 --- a/apps/extension/src/tools/borrow-confirmation.ts +++ b/apps/extension/src/tools/borrow-confirmation.ts @@ -91,6 +91,7 @@ export interface RequestBorrowConfirmationDeps { notifications?: ChromeNotificationsForBorrow | null; /** Returns `true` when `windowId` belongs to *any* live session's Agent Window. */ isAgentWindowId?: (windowId: number) => boolean; + isTabAllowed?: (tabId: number) => boolean; notificationCopy?: BorrowNotificationCopy; } @@ -297,6 +298,7 @@ interface ConfirmationCandidate { async function listConfirmationCandidates( windows: ChromeWindowsForBorrow, isAgentWindowId: (windowId: number) => boolean, + isTabAllowed: (tabId: number) => boolean = () => true, ): Promise { const seen = new Set(); const ordered: chrome.windows.Window[] = []; @@ -337,6 +339,7 @@ async function listConfirmationCandidates( if (isAgentWindowId(w.id)) continue; const activeTab = w.tabs?.find((t) => t.active === true) ?? null; if (!activeTab || typeof activeTab.id !== "number") continue; + if (!isTabAllowed(activeTab.id)) continue; const tabUrl = activeTab.url ?? activeTab.pendingUrl ?? undefined; if (!isInjectableContentScriptUrl(tabUrl)) continue; candidates.push({ tabId: activeTab.id, windowId: w.id, tabUrl }); @@ -407,7 +410,11 @@ export async function requestBorrowConfirmation( tabTitle = String(tabId); } - const candidates = await listConfirmationCandidates(windowsApi, isAgentWindowId); + const candidates = await listConfirmationCandidates( + windowsApi, + isAgentWindowId, + deps.isTabAllowed, + ); if (signal?.aborted) return { code: "cancelled", message: "tab_borrow aborted" }; if (options.autoAllow?.get()) return true; if (candidates.length === 0) { diff --git a/apps/extension/src/tools/human-loop.ts b/apps/extension/src/tools/human-loop.ts index 33cf5781..70bc1618 100644 --- a/apps/extension/src/tools/human-loop.ts +++ b/apps/extension/src/tools/human-loop.ts @@ -18,7 +18,12 @@ import { isHelpResponseMessage, } from "@/lib/help-bridge"; import type { InteractionPreferenceStore } from "@/lib/interaction-preferences"; -import type { SessionContext, SessionManager } from "@/session-manager/manager"; +import { + isAgentControlledTab, + type SessionContext, + type SessionManager, + sessionWindowId, +} from "@/session-manager/manager"; import type { HelpCompletionCondition, HelpCompletionCriteria, @@ -365,7 +370,12 @@ async function findHelpForTab( const windowId = tab.windowId; if (typeof windowId !== "number") return null; for (const help of activeHelpRequests.values()) { - if (!help.settled && help.ctx.agentWindowId === windowId) return help; + if ( + !help.settled && + sessionWindowId(help.ctx) === windowId && + (help.ctx.container.mode === "window" || isAgentControlledTab(help.ctx, tabId)) + ) + return help; } } catch { return null; diff --git a/apps/extension/src/tools/observation.ts b/apps/extension/src/tools/observation.ts index 6e1f9c97..31cc6ba4 100644 --- a/apps/extension/src/tools/observation.ts +++ b/apps/extension/src/tools/observation.ts @@ -39,6 +39,7 @@ import { isAgentControlledTab, type SessionContext, type SessionManager, + sessionWindowId, } from "@/session-manager/manager"; import type { GetHtmlParams, @@ -360,7 +361,7 @@ export async function handleScreenshot( // Pin controlled captures to their page, including when initially active: // the user can select another tab while overlay suppression is pending. - if (isAgentControlledTab(ctx, target.tabId) && target.windowId === ctx.agentWindowId) { + if (isAgentControlledTab(ctx, target.tabId) && target.windowId === sessionWindowId(ctx)) { const cdp = deps.cdp; if (!cdp) { return rpcError( @@ -381,7 +382,7 @@ export async function handleScreenshot( manager.get(ctx.sessionId) === ctx && isAgentControlledTab(ctx, target.tabId) && tab.id === target.tabId && - tab.windowId === ctx.agentWindowId + tab.windowId === sessionWindowId(ctx) ); }, signal, diff --git a/apps/extension/src/tools/record.ts b/apps/extension/src/tools/record.ts index c974776e..a7115ae1 100644 --- a/apps/extension/src/tools/record.ts +++ b/apps/extension/src/tools/record.ts @@ -32,7 +32,11 @@ import { import { RecordingTabCoordinator, type TabActivation } from "@/lib/recording/tab-coordinator"; import { buildTraceV2 } from "@/lib/recording/trace-reducer-v2"; import type { RecordingDraftStep } from "@/lib/recording/types"; -import { isAgentControlledTab, type SessionManager } from "@/session-manager/manager"; +import { + isAgentControlledTab, + type SessionManager, + sessionWindowId, +} from "@/session-manager/manager"; import { EXTENSION_VERSION } from "@/transport/handshake"; import type { RecordAwaitParams, @@ -495,7 +499,7 @@ async function clearRearmTimersForRecording( try { const tabs = await deps.tabsApi.query({ windowId: recording.agentWindowId }); for (const tab of tabs) { - if (typeof tab.id === "number") clearRearmTimer(tab.id); + if (typeof tab.id === "number" && recording.isTabAllowed(tab.id)) clearRearmTimer(tab.id); } } catch { // Best-effort cleanup. @@ -884,7 +888,8 @@ export async function handleRecordStart( // Cancellation can precede record_await; keep its rejection handled. void finishPromise.catch(() => {}); const isTabAllowed = (tabId: number) => - !ctx.remote || (manager.get(ctx.sessionId) === ctx && isAgentControlledTab(ctx, tabId)); + (!ctx.remote && ctx.container.mode === "window") || + (manager.get(ctx.sessionId) === ctx && isAgentControlledTab(ctx, tabId)); const navigateUrl = params.url ?? RECORD_DEFAULT_START_URL; const startedAtMs = Date.now(); const maxPageTokens = params.max_page_tokens; @@ -892,7 +897,7 @@ export async function handleRecordStart( recordings.set(params.session_id, { requestId, tabs: new RecordingTabCoordinator(target.tabId, navigateUrl), - agentWindowId: ctx.agentWindowId, + agentWindowId: sessionWindowId(ctx), isTabAllowed, startUrl: navigateUrl, ...(params.purpose ? { purpose: params.purpose } : {}), diff --git a/apps/extension/src/tools/session.ts b/apps/extension/src/tools/session.ts index d398945c..874d8899 100644 --- a/apps/extension/src/tools/session.ts +++ b/apps/extension/src/tools/session.ts @@ -4,7 +4,12 @@ import { interactionPolicy, } from "@/lib/interaction-preferences"; import { withTaskPreviewStop } from "@/lib/task-preview"; -import { type SessionManager, SessionStartCleanupError } from "@/session-manager/manager"; +import { + type SessionManager, + SessionStartCleanupError, + SharedSessionStartCleanupError, + sessionWindowId, +} from "@/session-manager/manager"; import type { InteractionPolicy, RpcError } from "@/transport/types"; import { rpcError } from "./errors"; import { clearRecordingForSession } from "./record"; @@ -52,6 +57,7 @@ export function validateWindowSize( } export interface SessionStartParams { + in_window?: boolean; session_id: string; browser_instance_id?: string; /** Optional Agent Window outer width in CSS pixels (100..=7680). */ @@ -65,6 +71,7 @@ export interface SessionStartParams { } export interface SessionStartResult { + container_mode?: "window" | "in_window"; interaction?: InteractionPolicy; agent_window_id?: number; } @@ -132,6 +139,18 @@ export async function handleSessionStart( if (params.unattended !== undefined && typeof params.unattended !== "boolean") { return { code: "invalid_params", message: "unattended must be a boolean" }; } + if (params.in_window !== undefined && typeof params.in_window !== "boolean") { + return { code: "invalid_params", message: "in_window must be a boolean" }; + } + if (params.in_window && (params.width !== undefined || params.height !== undefined)) { + return { + code: "invalid_params", + message: "in_window cannot be combined with window dimensions", + }; + } + if (params.in_window && manager.isRemote()) { + return { code: "unsupported", message: "Shared windows are only supported for local sessions" }; + } const sizeOrErr = validateWindowSize(params.width, params.height); if (isRpcError(sizeOrErr)) return sizeOrErr; try { @@ -140,12 +159,20 @@ export async function handleSessionStart( size: sizeOrErr, focused: params.focused, signal: deps.signal, + inWindow: params.in_window, }); return { - agent_window_id: ctx.agentWindowId, + agent_window_id: sessionWindowId(ctx), + ...(ctx.container.mode === "in_window" ? { container_mode: ctx.container.mode } : {}), interaction: interactionPolicy(deps.preferences?.get() ?? DEFAULT_INTERACTION_PREFERENCES), }; } catch (err) { + if (err instanceof SharedSessionStartCleanupError) { + return rpcError("protocol_error", "cleanup_failed", err.message, { + resource_type: "tab", + resource_id: err.tabId, + }); + } if (err instanceof SessionStartCleanupError) { return rpcError("protocol_error", "cleanup_failed", err.message, { resource_type: "agent_window", @@ -193,7 +220,7 @@ export async function handleSessionStart( * state. The daemon/CLI surface the failure and keep the session * retryable. */ -export async function handleSessionStop( +async function handleSessionStopCore( manager: SessionManager, params: SessionStopParams, deps: SessionStopDeps = {}, @@ -222,7 +249,6 @@ async function stopSession( if (deps.signal?.aborted) { return { code: "cancelled", message: "session_stop aborted before teardown" }; } - // Remote access must end before returning a tab. Preserve the local recording // when a failed return keeps its session alive for a retry. if (ctx.remote) clearRecordingForSession(params.session_id); @@ -292,7 +318,7 @@ async function stopSession( if (pendingReturnFailures.length > 0) { result.return_failures = pendingReturnFailures; // A failed return means at least one borrowed user tab may still be - // inside the Agent Window. Keep the session/window alive so the user + // controlled by this session. Keep the session/window alive so the user // can retry `bsk session stop` or explicitly `bsk tab return` after the // underlying Chrome issue is resolved. return result; @@ -310,7 +336,16 @@ async function stopSession( return { code: "cancelled", message: "session_stop aborted before window close" }; } - return manager.withExpectedWindowClose(ctx, async () => { + const teardown = async (): Promise => { + if (ctx.container.mode === "in_window") { + // The manager enforces tab-only cleanup even when Chrome queries fail. + try { + await manager.stop(params.session_id); + } catch (err) { + return { code: "protocol_error", message: String(err) }; + } + 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 @@ -337,7 +372,7 @@ async function stopSession( let shouldRelease = (ctx.observedTabs?.size ?? 0) > 0; if (queryApi) { try { - const liveWindowTabs = await queryApi.query({ windowId: ctx.agentWindowId }); + const liveWindowTabs = await queryApi.query({ windowId: sessionWindowId(ctx) }); // 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 @@ -359,7 +394,7 @@ async function stopSession( "Agent tabs could not be closed; user tabs were preserved and cleanup can be retried", { resource_type: "agent_window", - resource_id: ctx.agentWindowId, + resource_id: sessionWindowId(ctx), }, ); } @@ -396,5 +431,28 @@ async function stopSession( } return result; + }; + // Shared sessions already mark the full stop transaction at the entry point. + return ctx.container.mode === "in_window" + ? teardown() + : manager.withExpectedWindowClose(ctx, teardown); +} + +export async function handleSessionStop( + manager: SessionManager, + params: SessionStopParams, + deps: SessionStopDeps = {}, +): Promise { + const ctx = manager.get(params?.session_id); + if (!ctx || ctx.container.mode === "window") return handleSessionStopCore(manager, params, deps); + if (ctx.stopping || ctx.pendingOperations) + return { code: "cancelled", message: "Session has pending operations; retry stop" }; + ctx.stopping = true; + return manager.withExpectedWindowClose(ctx, async () => { + try { + return await handleSessionStopCore(manager, params, deps); + } finally { + if (manager.get(ctx.sessionId) === ctx) ctx.stopping = false; + } }); } diff --git a/apps/extension/src/tools/shared.ts b/apps/extension/src/tools/shared.ts index 8e2f819c..bda24b2b 100644 --- a/apps/extension/src/tools/shared.ts +++ b/apps/extension/src/tools/shared.ts @@ -10,8 +10,10 @@ import type { CdpDebuggee, CdpDispatchGuard, DialogCursor } from "@/browser-driv import type { CdpFrameGraph, CdpTarget } from "@/browser-driver/frame-graph"; import { isAgentControlledTab, + preferredSharedTab, type SessionContext, type SessionManager, + sessionWindowId, } from "@/session-manager/manager"; import { normaliseRef } from "@/session-manager/ref-store"; import type { ConsoleResult, JavaScriptDialogInfo, RpcError } from "@/transport/types"; @@ -175,6 +177,7 @@ export function lookupSession( message: `session ${params.session_id} unknown`, }; } + if (ctx.stopping) return { code: "cancelled", message: "Session is stopping" }; return ctx; } @@ -231,7 +234,7 @@ async function resolveVisibleTargetTab( message: `tab ${tabId} not found`, }; } - const owner = manager.findByWindowId(tab.windowId); + const owner = manager.findByTabId(tabId) ?? manager.findByWindowId(tab.windowId); if (owner && owner.sessionId !== ctx.sessionId) { return { code: "not_found", @@ -246,17 +249,24 @@ async function resolveVisibleTargetTab( pendingUrl: tab.pendingUrl, }; } - const tabs = await api.query({ active: true, windowId: ctx.agentWindowId }); + if (ctx.container.mode === "in_window") { + const tabs = await api.query({ windowId: sessionWindowId(ctx) }); + const tab = preferredSharedTab(ctx, tabs); + if (!tab || tab.id === undefined) + return { code: "not_found", message: "No controlled tab in session" }; + return { tabId: tab.id, windowId: tab.windowId, active: tab.active, url: tab.url }; + } + const tabs = await api.query({ active: true, windowId: sessionWindowId(ctx) }); const first = tabs.find((t) => typeof t.id === "number"); if (!first || typeof first.id !== "number") { return { code: "not_found", - message: `no active tab in Agent Window ${ctx.agentWindowId}`, + message: `no active tab in Agent Window ${sessionWindowId(ctx)}`, }; } return { tabId: first.id, - windowId: ctx.agentWindowId, + windowId: sessionWindowId(ctx), active: first.active === true, url: first.url, pendingUrl: first.pendingUrl, @@ -315,7 +325,7 @@ export function enforceCdpAccessibleTarget( return rpcError( "permission_denied", "restricted_tab_url", - `${toolName} cannot access tab ${target.tabId} because its URL is ${target.url}; navigate the Agent Window to a web page first`, + `${toolName} cannot access tab ${target.tabId} because its URL is ${target.url}; navigate a session-controlled tab to a web page first`, ); } @@ -352,10 +362,18 @@ export async function resolveCdpAccessibleTargetTab( if (!restricted) return target; if (tabId !== undefined) return restricted; - const tabs = await api.query({ windowId: ctx.agentWindowId }); + const tabs = await api.query({ windowId: sessionWindowId(ctx) }); for (const tab of tabs) { - if (ctx.remote && (tab.id === undefined || !isAgentControlledTab(ctx, tab.id))) continue; - const candidate = resolvedTargetFromChromeTab(tab, ctx.agentWindowId); + if ( + (ctx.remote || ctx.container.mode === "in_window") && + (tab.id === undefined || !isAgentControlledTab(ctx, tab.id)) + ) + continue; + if (tab.id !== undefined) { + const owner = manager.findByTabId(tab.id); + if (owner && owner !== ctx) continue; + } + const candidate = resolvedTargetFromChromeTab(tab, sessionWindowId(ctx)); if (!candidate) continue; if (!enforceCdpAccessibleTarget(candidate, toolName)) return candidate; } @@ -364,28 +382,30 @@ export async function resolveCdpAccessibleTargetTab( /** * Sandbox guard: M7 write tools (click / fill / press / navigate*) - * MUST refuse to touch a tab outside the session's Agent Window - * (§6 — borrowing brings the tab into the Agent Window first). + * require the session's window and, for shared/remote sessions, explicit + * page ownership. Same-window borrowing grants ownership without a move. * - * Returns an `RpcError` when the resolved target sits in a user window; - * `null` on success. + * Returns an `RpcError` for an unauthorized target; `null` on success. */ export function enforceAgentWindow( ctx: SessionContext, target: { tabId: number; windowId: number }, toolName: string, ): RpcError | null { - if (ctx.remote && !isAgentControlledTab(ctx, target.tabId)) { + if ( + ctx.stopping || + ((ctx.remote || ctx.container.mode === "in_window") && !isAgentControlledTab(ctx, target.tabId)) + ) { return { code: "permission_denied", - message: "This tab has not been authorized for the remote task", + message: "This tab has not been authorized for this session", }; } - if (target.windowId !== ctx.agentWindowId) { + if (target.windowId !== sessionWindowId(ctx)) { return rpcError( "permission_denied", "agent_window_scope", - `${toolName} can only act on tabs inside the Agent Window (tab ${target.tabId} is in window ${target.windowId}; borrow it first)`, + `${toolName} can only act on tabs in its session window (tab ${target.tabId} is in window ${target.windowId}; borrow it into this session first)`, ); } return null; @@ -393,7 +413,7 @@ export function enforceAgentWindow( /** * Unified target-scope policy by tool effect. Passive reads may inspect user - * tabs; any tool that dispatches page input must stay inside the Agent Window. + * tabs; page input must respect the session window and its ownership policy. */ export function enforceToolTargetScope( ctx: SessionContext, diff --git a/apps/extension/src/tools/tabs.ts b/apps/extension/src/tools/tabs.ts index 02788fdc..a99d6ef1 100644 --- a/apps/extension/src/tools/tabs.ts +++ b/apps/extension/src/tools/tabs.ts @@ -1,3 +1,4 @@ +import { sessionWindowId } from "@/session-manager/manager"; import { withUiTeardown } from "@/session-manager/ui-activity"; // Tab-tool handlers. M6 wired `tool.tab_list`; M8 adds the rest of // the tab namespace: `tab_create`, `tab_close`, `tab_select`, @@ -249,11 +250,11 @@ export async function handleTabList( // per-tab classification is O(1). const otherAgentWindowIds = new Set(); for (const s of manager.list()) { - if (s.sessionId !== params.session_id) { - otherAgentWindowIds.add(s.agentWindowId); + if (s.sessionId !== params.session_id && s.container.mode === "window") { + otherAgentWindowIds.add(sessionWindowId(s)); } } - const myAgentWindowId = ctx.agentWindowId; + const myAgentWindowId = sessionWindowId(ctx); const allTabs = await api.query({}); if (signal?.aborted) return { code: "cancelled", message: "tab_list aborted" }; @@ -262,7 +263,15 @@ export async function handleTabList( if (typeof t.id !== "number") continue; const winId = typeof t.windowId === "number" ? t.windowId : -1; if (otherAgentWindowIds.has(winId)) continue; - const tabScope: "user" | "agent" = winId === myAgentWindowId ? "agent" : "user"; + const owner = manager.findByTabId(t.id); + if (owner && owner !== ctx) continue; + const tabScope: "user" | "agent" = ( + ctx.container.mode === "in_window" + ? isAgentControlledTab(ctx, t.id) + : winId === myAgentWindowId + ) + ? "agent" + : "user"; if (scope === "user" && tabScope !== "user") continue; if (scope === "agent" && tabScope !== "agent") continue; tabs.push({ @@ -362,7 +371,7 @@ function buildCreateProps( params: TabCreateParams, ): chrome.tabs.CreateProperties { const createProps: chrome.tabs.CreateProperties = { - windowId: ctx.agentWindowId, + windowId: sessionWindowId(ctx), url: params.url ?? NEW_TAB_DEFAULT_URL, active: params.active ?? true, }; @@ -370,12 +379,33 @@ function buildCreateProps( return createProps; } -/** - * Create a tab and handle post-creation abort cleanup. - * On abort the opened tab is immediately removed so it doesn't leak. - * Returns the created tab on success, or an `RpcError` on failure. - */ +/** A shared session may close only a tab it still owns in its host window. */ +async function stillOwnsCreatedTab( + manager: SessionManager, + ctx: SessionContext, + deps: TabManagementDeps, + tabId: number, +): Promise { + if (ctx.container.mode !== "in_window") return true; + if (manager.get(ctx.sessionId) !== ctx || !ctx.agentCreatedTabs.has(tabId)) return false; + let live: chrome.tabs.Tab; + try { + live = await getTabsApi(deps).get(tabId); + } catch (err) { + if (!/No tab with id|Invalid tab ID|not found/i.test(String(err))) throw err; + ctx.agentCreatedTabs.delete(tabId); + return false; + } + if (live.windowId !== sessionWindowId(ctx)) { + ctx.agentCreatedTabs.delete(tabId); + return false; + } + return manager.get(ctx.sessionId) === ctx && ctx.agentCreatedTabs.has(tabId); +} + +/** Create and claim a tab, removing it on abort only while ownership persists. */ async function createTabAndCleanup( + manager: SessionManager, ctx: SessionContext, deps: TabManagementDeps, createProps: chrome.tabs.CreateProperties, @@ -392,13 +422,34 @@ async function createTabAndCleanup( if (typeof tab.id !== "number") { return { code: "protocol_error", message: "chrome.tabs.create returned no tab id" }; } + if (ctx.container.mode === "in_window") { + ctx.agentCreatedTabs.add(tab.id); + try { + const live = await getTabsApi(deps).get(tab.id); + if (live.windowId !== sessionWindowId(ctx)) { + ctx.agentCreatedTabs.delete(tab.id); + return { code: "cancelled", message: "Created tab was moved out of the session" }; + } + } catch (err) { + if (/No tab with id|Invalid tab ID|not found/i.test(String(err))) + ctx.agentCreatedTabs.delete(tab.id); + return { + code: "not_found", + message: `Created tab is no longer available: ${describeError(err)}`, + }; + } + } // Claim the concrete id returned by Chrome. Ownership never depends on // matching this request to an asynchronous onCreated event. - ctx.agentCreatedTabs.add(tab.id); + if (ctx.container.mode === "window") ctx.agentCreatedTabs.add(tab.id); + if (ctx.container.mode === "in_window" && (createProps.active || ctx.activeTabId === undefined)) + ctx.activeTabId = tab.id; if (aborted(deps.signal, "tab_create")) { try { - await getTabsApi(deps).remove(tab.id); - ctx.agentCreatedTabs.delete(tab.id); + if (await stillOwnsCreatedTab(manager, ctx, deps, tab.id)) { + await getTabsApi(deps).remove(tab.id); + ctx.agentCreatedTabs.delete(tab.id); + } } catch (cleanupErr) { // Keep the claim when cleanup fails so session_stop can retry instead // of releasing an agent-owned tab to the user. @@ -419,9 +470,9 @@ async function createTabAndCleanup( // --------------------------------------------------------------------------- /** - * Open a fresh tab *inside* the requesting session's Agent Window. - * The Agent Window scope is enforced by always passing - * `windowId: ctx.agentWindowId` to `chrome.tabs.create` (design §6). + * Open and claim a fresh tab inside the requesting session's window. + * The destination is enforced by always passing + * `windowId: sessionWindowId(ctx)` to `chrome.tabs.create` (design §6). */ export async function handleTabCreate( manager: SessionManager, @@ -442,60 +493,76 @@ export async function handleTabCreate( // document, not Chrome's restricted New Tab page. if (deps.cdp?.acquireBackgroundExecution && params.url === undefined) props.url = "about:blank"; const prepare = deps.cdp?.acquireBackgroundExecution && !cdpBlockedUrlReason(props.url); - const tab = await createTabAndCleanup( - ctx, - deps, - prepare ? { ...props, url: "about:blank" } : props, - ); - if (isRpcError(tab)) return tab; - if (prepare) { - try { - await deps.cdp!.acquireBackgroundExecution!(ctx.sessionId, tab.id); - if ( - deps.signal?.aborted || - manager.get(ctx.sessionId) !== ctx || - !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"); - } catch (error) { - const cleanupErrors: string[] = []; + const create = async (): Promise => { + const createTab = () => + createTabAndCleanup(manager, ctx, deps, prepare ? { ...props, url: "about:blank" } : props); + const tab = + ctx.container.mode === "in_window" + ? await createTab() + : await manager.withTabOperation(ctx, createTab); + if (isRpcError(tab)) return tab; + if (prepare) { try { - await deps.cdp?.releaseSessionTab?.(ctx.sessionId, tab.id); - } catch (cleanupError) { - cleanupErrors.push(`release: ${describeError(cleanupError)}`); + await deps.cdp!.acquireBackgroundExecution!(ctx.sessionId, tab.id); + if ( + deps.signal?.aborted || + manager.get(ctx.sessionId) !== ctx || + !isAgentControlledTab(ctx, tab.id) || + !(await stillOwnsCreatedTab(manager, ctx, deps, 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"); + } catch (error) { + const cleanupErrors: string[] = []; + try { + await deps.cdp?.releaseSessionTab?.(ctx.sessionId, tab.id); + } catch (cleanupError) { + cleanupErrors.push(`release: ${describeError(cleanupError)}`); + } + try { + if (await stillOwnsCreatedTab(manager, ctx, deps, tab.id)) { + await getTabsApi(deps).remove(tab.id); + ctx.agentCreatedTabs.delete(tab.id); + } + } catch (cleanupError) { + // Keep the claim only when closing fails, so session cleanup can retry. + cleanupErrors.push(`close: ${describeError(cleanupError)}`); + } + if (cleanupErrors.length) { + return rpcError( + "protocol_error", + "cleanup_failed", + `Background tab initialization failed: ${describeError(error)}; cleanup failed: ${cleanupErrors.join("; ")}`, + { resource_type: "tab", resource_id: tab.id }, + ); + } + return { + code: deps.signal?.aborted ? "cancelled" : "cdp_failed", + message: describeError(error), + }; } + } + + if (prepare && ctx.container.mode === "in_window") { try { - await getTabsApi(deps).remove(tab.id); - ctx.agentCreatedTabs.delete(tab.id); - } catch (cleanupError) { - // Keep the claim only when closing fails, so session cleanup can retry. - cleanupErrors.push(`close: ${describeError(cleanupError)}`); + if (!(await stillOwnsCreatedTab(manager, ctx, deps, tab.id))) + return { code: "cancelled", message: "Created tab was moved out of the session" }; + } catch (err) { + return { code: "protocol_error", message: describeError(err) }; } - if (cleanupErrors.length) { - return rpcError( - "protocol_error", - "cleanup_failed", - `Background tab initialization failed: ${describeError(error)}; cleanup failed: ${cleanupErrors.join("; ")}`, - { resource_type: "tab", resource_id: tab.id }, - ); - } - return { - code: deps.signal?.aborted ? "cancelled" : "cdp_failed", - message: describeError(error), - }; } - } - return { - tab_id: tab.id, - window_id: ctx.agentWindowId, - url: prepare ? (props.url ?? "about:blank") : (tab.url ?? tab.pendingUrl ?? ""), + return { + tab_id: tab.id, + window_id: sessionWindowId(ctx), + url: prepare ? (props.url ?? "about:blank") : (tab.url ?? tab.pendingUrl ?? ""), + }; }; + return ctx.container.mode === "in_window" ? manager.withTabOperation(ctx, create) : create(); } // --------------------------------------------------------------------------- @@ -503,11 +570,8 @@ export async function handleTabCreate( // --------------------------------------------------------------------------- /** - * Verify that `tabId` is either inside the session's own Agent Window - * (the normal case) or is a tab the session has borrowed (sitting - * inside the Agent Window after the move). Cross-session borrows from - * other sessions are rejected. Other sessions' Agent Window tabs are - * also rejected via `permission_denied`. + * Verify session window scope. Shared/remote sessions additionally require + * explicit creation or borrowing; other sessions' controlled tabs are rejected. * * Returns the resolved tab on success, otherwise an `RpcError` to * propagate verbatim. @@ -531,10 +595,13 @@ async function authoriseAgentTab( if (typeof tab.id !== "number" || typeof tab.windowId !== "number") { return { code: "not_found", message: `tab ${tabId} not found` }; } - if (ctx.remote && !isAgentControlledTab(ctx, tabId)) { + const owner = manager.findByTabId(tabId); + if (owner && owner !== ctx && !owner.borrowedTabs.has(tabId)) + return { code: "not_found", message: "Tab is outside session scope" }; + if ((ctx.remote || ctx.container.mode === "in_window") && !isAgentControlledTab(ctx, tabId)) { return { code: "permission_denied", - message: `${toolName}: borrow this tab before controlling it remotely`, + message: `${toolName}: borrow this tab before controlling it in this session`, }; } const otherBorrower = manager.findBorrowingSession(tabId, ctx.sessionId); @@ -545,13 +612,13 @@ async function authoriseAgentTab( `${toolName}: tab ${tabId} is borrowed by session ${otherBorrower}`, ); } - if (tab.windowId === ctx.agentWindowId) { + if (tab.windowId === sessionWindowId(ctx)) { return tab; } return rpcError( "permission_denied", "agent_window_scope", - `${toolName}: tab ${tabId} is not in Agent Window ${ctx.agentWindowId}`, + `${toolName}: tab ${tabId} is not in session window ${sessionWindowId(ctx)}`, ); } @@ -592,19 +659,22 @@ export async function handleTabClose( if (aborted(deps.signal, "tab_close")) { return { code: "cancelled", message: "tab_close aborted" }; } - try { - await getTabsApi(deps).remove(params.tab_id); - // Keep the tracking set accurate so session_stop won't try to close a - // tab that's already gone (design §3.1). - ctx.agentCreatedTabs.delete(params.tab_id); - ctx.observedTabs?.delete(params.tab_id); - } catch (err) { - return { - code: "protocol_error", - message: err instanceof Error ? err.message : String(err), - }; - } - return { tab_id: params.tab_id }; + // Chrome may deliver onRemoved before remove() resolves. Keep the session + // alive until both the browser event and our ownership update finish. + const close = async (): Promise => { + try { + await getTabsApi(deps).remove(params.tab_id); + ctx.agentCreatedTabs.delete(params.tab_id); + ctx.observedTabs?.delete(params.tab_id); + } catch (err) { + return { + code: "protocol_error", + message: err instanceof Error ? err.message : String(err), + }; + } + return { tab_id: params.tab_id }; + }; + return ctx.container.mode === "in_window" ? manager.withTabOperation(ctx, close) : close(); } // --------------------------------------------------------------------------- @@ -637,6 +707,7 @@ export async function handleTabSelect( } try { await getTabsApi(deps).update(params.tab_id, { active: true }); + if (ctx.container.mode === "in_window") ctx.activeTabId = params.tab_id; } catch (err) { return { code: "protocol_error", @@ -645,7 +716,7 @@ export async function handleTabSelect( } // windowId stable across `update({active:true})`; reuse what // authoriseAgentTab already loaded so we don't issue a second `get`. - const windowId = typeof tabOrErr.windowId === "number" ? tabOrErr.windowId : ctx.agentWindowId; + const windowId = typeof tabOrErr.windowId === "number" ? tabOrErr.windowId : sessionWindowId(ctx); return { tab_id: params.tab_id, window_id: windowId }; } @@ -706,17 +777,21 @@ async function validateBorrowTarget( "borrow_conflict", `tab_borrow: tab ${tabId} is already controlled by session ${owner}`, ); - if (tab.windowId === ctx.agentWindowId) { + if (tab.windowId === sessionWindowId(ctx) && ctx.container.mode === "window") { return { code: "invalid_params", message: ctx.remote && !isAgentControlledTab(ctx, tabId) ? `tab_borrow: tab ${tabId} is not authorized and already lives in the Agent Window; move it to a regular browser window, then borrow it` - : `tab_borrow: tab ${tabId} already lives in the Agent Window`, + : `tab_borrow: tab ${tabId} is already within this session's control scope`, }; } for (const s of manager.list()) { - if (s.sessionId !== ctx.sessionId && s.agentWindowId === tab.windowId) { + if ( + s.sessionId !== ctx.sessionId && + s.container.mode === "window" && + sessionWindowId(s) === tab.windowId + ) { return rpcError( "permission_denied", "agent_window_scope", @@ -832,20 +907,23 @@ async function executeBorrowCore( const originalWindowId = currentTab.windowId; const originalIndex = typeof currentTab.index === "number" ? currentTab.index : 0; - const moveErr = await moveTabForBorrow( - p.tabsApi, - p.tabId, - p.ctx.agentWindowId, - originalWindowId, - originalIndex, - p.signal, - ); + const moveErr = + originalWindowId === sessionWindowId(p.ctx) + ? null + : await moveTabForBorrow( + p.tabsApi, + p.tabId, + sessionWindowId(p.ctx), + originalWindowId, + originalIndex, + p.signal, + ); if (moveErr) return moveErr; return { originalWindowId, originalIndex }; } -export async function handleTabBorrow( +async function handleTabBorrowCore( manager: SessionManager, params: TabBorrowParams, deps: TabManagementDeps = {}, @@ -878,7 +956,7 @@ export async function handleTabBorrow( try { const tab = await getTabsApi(deps).get(params.tab_id); if ( - tab.windowId === ctx.agentWindowId && + tab.windowId === sessionWindowId(ctx) && manager.get(ctx.sessionId) === ctx && !deps.signal?.aborted ) { @@ -886,7 +964,7 @@ export async function handleTabBorrow( tab_id: params.tab_id, original_window_id: existing.originalWindowId, original_index: existing.originalIndex, - agent_window_id: ctx.agentWindowId, + agent_window_id: sessionWindowId(ctx), }; } } catch { @@ -894,7 +972,7 @@ export async function handleTabBorrow( } return { code: "invalid_params", - message: "Borrowed tab is no longer in this session's Agent Window", + message: "Borrowed tab is no longer in this session's window", }; } const reservation = manager.tryReserveBorrow(params.tab_id, ctx.sessionId); @@ -935,12 +1013,14 @@ export async function handleTabBorrow( originalIndex: coreResult.originalIndex, }); committed = true; + if (ctx.container.mode === "in_window") ctx.activeTabId = params.tab_id; } catch (err) { try { - await tabsApi.move(params.tab_id, { - windowId: coreResult.originalWindowId, - index: coreResult.originalIndex, - }); + if (coreResult.originalWindowId !== sessionWindowId(ctx)) + await tabsApi.move(params.tab_id, { + windowId: coreResult.originalWindowId, + index: coreResult.originalIndex, + }); } catch (rollbackError) { return rpcError( "protocol_error", @@ -986,7 +1066,7 @@ export async function handleTabBorrow( tab_id: params.tab_id, original_window_id: coreResult.originalWindowId, original_index: coreResult.originalIndex, - agent_window_id: ctx.agentWindowId, + agent_window_id: sessionWindowId(ctx), }; } finally { if (!committed) reservation.release(); @@ -997,6 +1077,16 @@ export async function handleTabBorrow( // tool.tab_return (M8.3) // --------------------------------------------------------------------------- +export async function handleTabBorrow( + manager: SessionManager, + params: TabBorrowParams, + deps: TabManagementDeps = {}, +): Promise { + const ctx = lookupSession(manager, params, "tab_borrow"); + if (isRpcError(ctx)) return ctx; + return manager.withTabOperation(ctx, () => handleTabBorrowCore(manager, params, deps)); +} + export interface ReturnOutcome { tabId: number; toWindowId: number; @@ -1033,7 +1123,11 @@ async function chooseFallbackWindow( // returned tab in another session's Agent Window would let that // session write to it and, worse, see it destroyed when that session // stops and closes its window. - if (lastId !== null && lastId !== ctx.agentWindowId && !isAgentWindowId(lastId)) { + if ( + lastId !== null && + (ctx.container.mode === "in_window" || lastId !== sessionWindowId(ctx)) && + !isAgentWindowId(lastId) + ) { return { windowId: lastId, index: -1, created: false }; } } catch (err) { @@ -1151,6 +1245,19 @@ async function returnBorrowedTabCore( const alreadyCancelled = aborted(deps.signal, "tab_return"); if (alreadyCancelled) return alreadyCancelled; + // A same-host borrow did not move the user's page. Return must release it + // in place even if the user subsequently moved it elsewhere. + if (ctx.container.mode === "in_window" && entry.originalWindowId === sessionWindowId(ctx)) { + await deps.cdp?.releaseSessionTab?.(ctx.sessionId, tabId); + await releaseReturnedTabState(ctx, tabId, { ...deps, cdp: undefined }); + return { + tabId, + toWindowId: entry.originalWindowId, + toIndex: entry.originalIndex, + fallback: false, + }; + } + let targetWindowId = entry.originalWindowId; let targetIndex = entry.originalIndex; let fallback = false; @@ -1264,7 +1371,7 @@ async function returnBorrowedTabCore( } } -export async function handleTabReturn( +async function handleTabReturnCore( manager: SessionManager, params: TabReturnParams, deps: TabManagementDeps = {}, @@ -1291,6 +1398,7 @@ export async function handleTabReturn( }); if (isRpcError(outcome)) return outcome; ctx.borrowedTabs.delete(params.tab_id); + manager.checkEmpty(ctx); const result: TabReturnResult = { tab_id: outcome.tabId, returned_to_window_id: outcome.toWindowId, @@ -1299,3 +1407,13 @@ export async function handleTabReturn( if (outcome.fallback) result.fallback = true; return result; } + +export async function handleTabReturn( + manager: SessionManager, + params: TabReturnParams, + deps: TabManagementDeps = {}, +): Promise { + const ctx = lookupSession(manager, params, "tab_return"); + if (isRpcError(ctx)) return ctx; + return manager.withTabOperation(ctx, () => handleTabReturnCore(manager, params, deps)); +} diff --git a/apps/extension/src/tools/waits.ts b/apps/extension/src/tools/waits.ts index 5795676b..57a8334a 100644 --- a/apps/extension/src/tools/waits.ts +++ b/apps/extension/src/tools/waits.ts @@ -3,9 +3,8 @@ // §4 / §7, plan M9.2. // // Sandbox follows the same rules as the navigate / interaction tools: -// `resolveTargetTab` + `enforceAgentWindow`. Borrowed tabs already -// inside the Agent Window are allowed; user-window tabs are refused -// with `permission_denied`. +// `resolveTargetTab` + `enforceAgentWindow`. Targets must be in the session +// window; shared/remote sessions also require explicit page ownership. // // Implementation: reuses the M7 helpers `ensureCdpReady` + // `waitForLifecyclePassive` (readyState probe + event listener) from diff --git a/apps/extension/src/tools/window.ts b/apps/extension/src/tools/window.ts index ccec54e6..1c58ae90 100755 --- a/apps/extension/src/tools/window.ts +++ b/apps/extension/src/tools/window.ts @@ -1,3 +1,4 @@ +import { sessionWindowId } from "@/session-manager/manager"; // Window-tool handlers: `tool.window_resize` resizes the session's // Agent Window via `chrome.windows.update`. @@ -53,6 +54,8 @@ export async function handleWindowResize( const ctxOrErr = lookupSession(manager, params, "window_resize"); if (isRpcError(ctxOrErr)) return ctxOrErr; const ctx = ctxOrErr; + if (ctx.container.mode === "in_window") + return { code: "unsupported", message: "Cannot resize a shared user window" }; const sizeOrErr = validateWindowSize(params.width, params.height); if (isRpcError(sizeOrErr)) return sizeOrErr; @@ -67,7 +70,7 @@ export async function handleWindowResize( try { if (signal?.aborted) return { code: "cancelled", message: "window_resize aborted" }; - await api.update(ctx.agentWindowId, { + await api.update(sessionWindowId(ctx), { width: sizeOrErr.width, height: sizeOrErr.height, }); @@ -78,7 +81,7 @@ export async function handleWindowResize( }; } return { - window_id: ctx.agentWindowId, + window_id: sessionWindowId(ctx), width: sizeOrErr.width, height: sizeOrErr.height, }; diff --git a/apps/extension/src/transport/__tests__/handshake.test.ts b/apps/extension/src/transport/__tests__/handshake.test.ts index 8f4741c6..7374b5cd 100644 --- a/apps/extension/src/transport/__tests__/handshake.test.ts +++ b/apps/extension/src/transport/__tests__/handshake.test.ts @@ -66,7 +66,7 @@ function deferredFakeTransport(): { transport: Transport; emit: (frame: Protocol describe("performHandshake", () => { it("advertises the protocol compatibility boundary", () => { - expect(PROTOCOL_VERSION).toBe("1.3"); + expect(PROTOCOL_VERSION).toBe("1.4"); expect(MIN_COMPATIBLE_PROTOCOL).toBe("1.0"); }); diff --git a/apps/extension/src/transport/handshake.ts b/apps/extension/src/transport/handshake.ts index c2536cb7..f3b068ab 100644 --- a/apps/extension/src/transport/handshake.ts +++ b/apps/extension/src/transport/handshake.ts @@ -7,7 +7,7 @@ import type { ResponseFrame, } from "./types"; -export const PROTOCOL_VERSION = "1.3"; +export const PROTOCOL_VERSION = "1.4"; /** * Extension semver, injected at build time from `package.json` via * Vite's `define` (see `wxt.config.ts` and `vitest.config.ts`). diff --git a/crates/bsk-cli/src/cli/error.rs b/crates/bsk-cli/src/cli/error.rs index 26e8abc3..5b9dfdbb 100644 --- a/crates/bsk-cli/src/cli/error.rs +++ b/crates/bsk-cli/src/cli/error.rs @@ -433,7 +433,7 @@ mod tests { let stderr = render_human_to_string(&cli, None); assert!(stderr.contains("target element has no visible geometry")); assert!(stderr.contains("rerun snapshot")); - assert!(!stderr.contains("Agent Window sandbox")); + assert!(!stderr.contains("session's access policy")); assert!(!stderr.contains("tab borrow")); assert!(stderr.contains("details: element not visible")); } @@ -446,7 +446,7 @@ mod tests { data: Some(serde_json::json!({ "reason": "agent_window_scope" })), }); let stderr = render_human_to_string(&cli, None); - assert!(stderr.contains("operation denied by the Agent Window sandbox")); + assert!(stderr.contains("operation denied by the session's access policy")); assert!(stderr.contains("tab borrow")); } diff --git a/crates/bsk-cli/src/cli/render_error.rs b/crates/bsk-cli/src/cli/render_error.rs index 1063ff58..76174ad9 100644 --- a/crates/bsk-cli/src/cli/render_error.rs +++ b/crates/bsk-cli/src/cli/render_error.rs @@ -124,9 +124,9 @@ pub fn info_for(code: ErrorCode) -> RenderInfo { exit_code: 1, }, ErrorCode::PermissionDenied => RenderInfo { - summary: "operation denied by the Agent Window sandbox", + summary: "operation denied by the session's access policy", hint: Some( - "tabs outside an Agent Window must first be borrowed via `bsk tab borrow --session `", + "user tabs require authorization via `bsk tab borrow --session `; sharing a window does not grant control", ), exit_code: 1, }, @@ -433,7 +433,7 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend (ErrorCode::InvalidParams, reason::TAB_NOT_ACTIVE) => RenderInfo { summary: "screenshot requires the visible active tab", hint: Some( - "select the tab with `bsk tab select --session ` or omit `--tab-id` to capture the Agent Window's active tab", + "select a session-controlled tab with `bsk tab select --session ` before capturing it", ), exit_code: base.exit_code, }, diff --git a/crates/bsk-cli/src/cli/session.rs b/crates/bsk-cli/src/cli/session.rs index 9bf3ce07..473c6fec 100644 --- a/crates/bsk-cli/src/cli/session.rs +++ b/crates/bsk-cli/src/cli/session.rs @@ -52,6 +52,9 @@ pub enum SessionSub { #[derive(Debug, Clone, Args)] pub struct SessionStartArgs { + /// Create session tabs in the last-focused user window (local only). + #[arg(long, conflicts_with_all = ["width", "height"])] + pub in_window: bool, /// Deprecated compatibility flag. Automation settings in the extension take precedence. #[arg(long)] pub unattended: bool, @@ -76,7 +79,7 @@ pub struct SessionStartArgs { #[arg(long, value_parser = window_size)] pub height: Option, - /// Open the Agent Window in the background without stealing focus. + /// Start without stealing focus (an inactive tab with --in-window). #[arg(long)] pub no_focus: bool, } @@ -119,6 +122,8 @@ pub struct SessionRequestArgs { #[derive(Debug, Serialize)] struct StartParams { + #[serde(skip_serializing_if = "std::ops::Not::not")] + in_window: bool, #[serde(skip_serializing_if = "Option::is_none")] request_id: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -135,6 +140,8 @@ struct StartParams { #[derive(Debug, Deserialize)] pub struct StartReply { + #[serde(default)] + pub container_mode: Option, #[serde(default)] pub interaction: Option, pub session_id: String, @@ -239,6 +246,7 @@ fn run_start(sock: PathBuf, args: SessionStartArgs, format: Format) -> Result<() let result = start_session( sock, SessionStartOptions { + in_window: args.in_window, name: args.name, request_id: args.request_id, browser: args.browser, @@ -257,6 +265,7 @@ fn run_start(sock: PathBuf, args: SessionStartArgs, format: Format) -> Result<() "session_id": reply.session_id, "browser_instance_id": reply.browser_instance_id, "agent_window_id": reply.agent_window_id, + "container_mode": reply.container_mode, "interaction": reply.interaction, })) .map_err(|e| CliError::Local(anyhow::anyhow!(e)))? @@ -277,6 +286,7 @@ fn run_start(sock: PathBuf, args: SessionStartArgs, format: Format) -> Result<() #[derive(Debug, Default, Clone)] pub struct SessionStartOptions { pub request_id: Option, + pub in_window: bool, pub name: Option, pub browser: Option, pub width: Option, @@ -286,6 +296,26 @@ pub struct SessionStartOptions { /// Start a session and open the Agent Window. Used by `session start` and `record start`. pub fn start_session(sock: PathBuf, opts: SessionStartOptions) -> Result { + if opts.in_window { + let status: bsk_protocol::StatusResult = call( + sock.clone(), + Method::SystemStatus, + None::, + Duration::from_secs(10), + )?; + if !bsk_protocol::tools::session::supports_shared_window(&status.protocol_version) { + return Err(CliError::from_rpc(bsk_protocol::RpcError { + code: bsk_protocol::ErrorCode::Unsupported, + message: + "Shared sessions require daemon protocol 1.4; restart or update the daemon" + .into(), + data: Some(serde_json::json!({ + "reason": "unsupported_feature", "component": "daemon", + "required_protocol": "1.4", "actual_protocol": status.protocol_version, + })), + })); + } + } call( sock, if opts.request_id.is_some() { @@ -294,6 +324,7 @@ pub fn start_session(sock: PathBuf, opts: SessionStartOptions) -> Result Result<(), CliError> { println!("(no active sessions)"); return Ok(()); } - let headers = ("SESSION", "BROWSER", "AGENT WINDOW"); + let headers = ("SESSION", "BROWSER", "WINDOW / MODE"); let session_w = reply .sessions .iter() @@ -567,8 +598,11 @@ fn run_list(sock: PathBuf, format: Format) -> Result<(), CliError> { .map(|w| w.to_string()) .unwrap_or_else(|| "-".into()); println!( - "{: Result, #[serde(default)] @@ -887,6 +889,8 @@ struct CliSessionStartParams { #[derive(Debug, Clone, Serialize, Deserialize)] struct CliSessionStartResult { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub container_mode: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub interaction: Option, pub session_id: String, @@ -993,6 +997,7 @@ pub(super) async fn handle_session_start( let cancel = abort_guard.token().clone(); let params: CliSessionStartParams = if params.is_null() { CliSessionStartParams { + in_window: false, browser_instance_id: None, width: None, height: None, @@ -1024,6 +1029,7 @@ pub(super) async fn handle_session_start( &state.tool_queues, params.browser_instance_id.as_deref(), AgentWindowOptions { + in_window: params.in_window, size: window_size, focused: params.focused, }, @@ -1039,6 +1045,7 @@ pub(super) async fn handle_session_start( state.audit.set_name(&session.id.0, &name); } let result = CliSessionStartResult { + container_mode: session.container_mode.clone(), interaction: session.interaction, session_id: session.id.0.clone(), browser_instance_id: session.browser_id.0.clone(), diff --git a/crates/bsk-cli/src/daemon/session_requests.rs b/crates/bsk-cli/src/daemon/session_requests.rs index 56380349..1fd907f2 100644 --- a/crates/bsk-cli/src/daemon/session_requests.rs +++ b/crates/bsk-cli/src/daemon/session_requests.rs @@ -506,6 +506,7 @@ mod ownership_tests { fn session(id: &str, window: i64) -> Session { Session { + container_mode: None, id: SessionId(id.into()), browser_id: BrowserId("browser".into()), agent_window_id: Some(window), diff --git a/crates/bsk-cli/src/daemon/sessions.rs b/crates/bsk-cli/src/daemon/sessions.rs index c4201026..6e8b502e 100644 --- a/crates/bsk-cli/src/daemon/sessions.rs +++ b/crates/bsk-cli/src/daemon/sessions.rs @@ -47,6 +47,7 @@ impl std::fmt::Display for SessionId { #[derive(Debug, Clone)] pub struct Session { + pub container_mode: Option, pub interaction: Option, pub id: SessionId, pub browser_id: BrowserId, @@ -57,6 +58,7 @@ pub struct Session { impl Session { pub fn status_entry(&self) -> SessionStatusEntry { SessionStatusEntry { + container_mode: self.container_mode.clone(), interaction: self.interaction, session_id: self.id.0.clone(), browser_instance_id: self.browser_id.0.clone(), @@ -153,6 +155,7 @@ impl SessionRegistry { guard.insert( candidate.clone(), Session { + container_mode: None, interaction: None, id: candidate.clone(), browser_id: browser_id.clone(), @@ -454,6 +457,7 @@ const SESSION_ID_MAX_RESERVE_ATTEMPTS: u32 = 64; /// (focused window, browser-chosen size). #[derive(Debug, Default, Clone, Copy)] pub struct AgentWindowOptions { + pub in_window: bool, /// Optional outer size as `(width, height)` CSS pixels. pub size: Option<(u32, u32)>, /// Optional focus hint (`None` = extension default: focused). @@ -527,6 +531,22 @@ pub(crate) async fn start_session_recoverable( instance_ids, }, })?; + if window.in_window + && !bsk_protocol::tools::session::supports_shared_window(&client.extension_protocol_version) + { + return Err(StartSessionError::ExtensionError(RpcError { + code: ErrorCode::Unsupported, + message: "Shared sessions require extension protocol 1.4".into(), + data: None, + })); + } + if window.in_window && window.size.is_some() { + return Err(StartSessionError::ExtensionError(RpcError { + code: ErrorCode::InvalidParams, + message: "Shared sessions do not accept window dimensions".into(), + data: None, + })); + } let session_id = sessions .reserve_id(client.id.clone(), SESSION_ID_MAX_RESERVE_ATTEMPTS, now_ms) .ok_or(StartSessionError::IdExhausted)?; @@ -534,6 +554,7 @@ pub(crate) async fn start_session_recoverable( sessions.starting.lock().unwrap().insert(session_id.clone()); } let params = SessionStartParams { + in_window: window.in_window, session_id: session_id.0.clone(), browser_instance_id: Some(client.id.0.clone()), width: window.size.map(|(width, _)| width), @@ -669,10 +690,27 @@ pub(crate) async fn start_session_recoverable( return Err(StartSessionError::ExtensionError(err)); } }; + if window.in_window && start_result.container_mode.as_deref() != Some("in_window") { + let cleanup = rollback_extension_session(&client, &session_id).await; + sessions.cancel_reservation(&session_id); + if let Err(message) = cleanup { + return Err(StartSessionError::CleanupFailed { + session_id, + agent_window_id: start_result.agent_window_id, + message, + }); + } + return Err(StartSessionError::ExtensionError(RpcError { + code: ErrorCode::ProtocolError, + message: "Extension did not confirm in_window mode".into(), + data: None, + })); + } { let mut guard = sessions.inner.lock().expect("session registry poisoned"); if let Some(session) = guard.get_mut(&session_id) { session.interaction = session.interaction.or(start_result.interaction); + session.container_mode = start_result.container_mode.clone(); } } let session = sessions diff --git a/crates/bsk-cli/src/daemon/state.rs b/crates/bsk-cli/src/daemon/state.rs index dffb96d8..090b9ed7 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.3"; +pub const PROTOCOL_VERSION: &str = "1.4"; /// 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 96d60e93..31205f15 100644 --- a/crates/bsk-cli/src/daemon/ws.rs +++ b/crates/bsk-cli/src/daemon/ws.rs @@ -501,7 +501,8 @@ async fn handle_inbound_text(state: &Arc, client: &Arc { + bsk_protocol::EventKind::SessionWindowClosed + | bsk_protocol::EventKind::SessionTabsClosed => { handle_session_window_closed(state, &client.id, &ev.payload); } bsk_protocol::EventKind::SessionUserInterrupt => { @@ -631,9 +632,9 @@ fn handle_session_window_closed( &session_id, ) { state.transfers.release_session(&session_id.0); - info!(session = %session_id, "session removed: user closed Agent Window"); + info!(session = %session_id, "session removed: session pages closed"); } else { - debug!(session = %session_id, "session.window_closed for unknown session id"); + debug!(session = %session_id, "session closure event for unknown session id"); } for failure in return_failures { warn!( @@ -641,7 +642,7 @@ fn handle_session_window_closed( tab_id = failure.tab_id, code = ?failure.code, message = %failure.message, - "borrowed tab could not be returned before Agent Window closed" + "borrowed tab could not be returned before session window closed" ); } } diff --git a/crates/bsk-cli/tests/browser_liveness.rs b/crates/bsk-cli/tests/browser_liveness.rs index c386e191..3df1254e 100644 --- a/crates/bsk-cli/tests/browser_liveness.rs +++ b/crates/bsk-cli/tests/browser_liveness.rs @@ -41,6 +41,7 @@ fn fake_client(id: &str, heartbeat_seen: bool, idle_secs: u64) -> std::sync::Arc fn fake_session(session_id: &str, browser_id: &str) -> Session { Session { + container_mode: None, interaction: None, id: SessionId(session_id.into()), diff --git a/crates/bsk-cli/tests/cancel_forwarding.rs b/crates/bsk-cli/tests/cancel_forwarding.rs index 34610513..1d2d8856 100644 --- a/crates/bsk-cli/tests/cancel_forwarding.rs +++ b/crates/bsk-cli/tests/cancel_forwarding.rs @@ -203,6 +203,7 @@ async fn cancel_forwards_to_extension_when_tool_is_inflight() { match req.method { Method::ToolSessionStart => { let result = SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(1), }; @@ -427,6 +428,7 @@ async fn cancel_arriving_during_promote_critical_section_keeps_request_cancel_in match req.method { Method::ToolSessionStart => { let result = SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(1), }; @@ -627,6 +629,7 @@ async fn cancel_keeps_session_busy_until_delayed_extension_cleanup_finishes() { match req.method { Method::ToolSessionStart => { let result = SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(1), }; diff --git a/crates/bsk-cli/tests/cli_parse.rs b/crates/bsk-cli/tests/cli_parse.rs index 59773de5..8df1cf8e 100644 --- a/crates/bsk-cli/tests/cli_parse.rs +++ b/crates/bsk-cli/tests/cli_parse.rs @@ -14,6 +14,38 @@ fn parse(args: &[&str]) -> Cli { Cli::try_parse_from(args).expect("clap parse should succeed") } +#[test] +fn shared_window_is_opt_in_and_rejects_dimensions() { + for in_window in [false, true] { + let mut argv = vec!["bsk", "session", "start", "--no-focus"]; + if in_window { + argv.push("--in-window"); + } + let Command::Session(SessionCmd { + sub: SessionSub::Start(args), + }) = parse(&argv).command + else { + panic!("session start expected") + }; + assert_eq!(args.in_window, in_window); + assert!(args.no_focus); + } + assert!( + Cli::try_parse_from([ + "bsk", + "session", + "start", + "--in-window", + "--width", + "800", + "--height", + "600" + ]) + .is_err() + ); + assert!(Cli::try_parse_from(["bsk", "record", "start", "--in-window"]).is_err()); +} + #[test] fn parses_unattended_session_without_changing_normal_defaults() { for unattended in [false, true] { diff --git a/crates/bsk-cli/tests/daemon_discovery.rs b/crates/bsk-cli/tests/daemon_discovery.rs index d8b6d7a7..2a182c6d 100644 --- a/crates/bsk-cli/tests/daemon_discovery.rs +++ b/crates/bsk-cli/tests/daemon_discovery.rs @@ -309,6 +309,30 @@ fn sessions_and_default_borrowing_work_with_legacy_daemons_without_forwarding_ov } } +#[test] +fn shared_session_preflight_never_sends_start_to_a_legacy_daemon() { + let starts = Arc::new(AtomicUsize::new(0)); + let observed = starts.clone(); + let daemon = MockDaemon::with_requests(FOREIGN_PID, move |_, info, request| { + if request.method == Method::SessionStart { + observed.fetch_add(1, Ordering::Relaxed); + } + Some(status(info)) + }); + let result = command( + daemon.home(), + &["session", "start", "--in-window", "--json"], + ) + .env("BSK_AUTO_START", "0") + .output() + .unwrap(); + assert!(!result.status.success()); + let error: serde_json::Value = serde_json::from_slice(&result.stdout).unwrap(); + assert_eq!(error["code"], "unsupported", "{error}"); + assert_eq!(error["data"]["required_protocol"], "1.4"); + assert_eq!(starts.load(Ordering::Relaxed), 0); +} + #[test] fn discovery_accepts_ipc_without_local_pid_but_management_refuses_it() { assert!(!pid_alive(FOREIGN_PID)); diff --git a/crates/bsk-cli/tests/handshake_compat.rs b/crates/bsk-cli/tests/handshake_compat.rs index 18d4168c..76b1ecf7 100644 --- a/crates/bsk-cli/tests/handshake_compat.rs +++ b/crates/bsk-cli/tests/handshake_compat.rs @@ -1,6 +1,15 @@ //! M10.4: end-to-end coverage for the daemon WS handshake's //! version-compatibility decision tree (Reject / Skew / Ok). +mod support; + +use bsk::daemon::state::PROTOCOL_VERSION; + +fn newer_minor_protocol() -> String { + let (major, minor) = PROTOCOL_VERSION.split_once('.').unwrap(); + format!("{major}.{}", minor.parse::().unwrap() + 1) +} + use std::path::PathBuf; use std::time::Duration; @@ -9,7 +18,6 @@ use bsk::ipc_client::IpcClient; use bsk_protocol::system::{HandshakeParams, HandshakeResult, StatusResult}; use bsk_protocol::{BrowserPeerInfo, ErrorCode, Method, RequestFrame, ResponseBody, ResponseFrame}; use futures_util::{SinkExt, StreamExt}; -use rand::Rng; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -17,13 +25,7 @@ use tokio_tungstenite::tungstenite::protocol::Message; const TEST_EXT_ID: &str = "abcdefghijklmnopabcdefghijklmnop"; fn tempfile_path(prefix: &str) -> PathBuf { - let mut p = std::env::temp_dir(); - let mut rng = rand::thread_rng(); - let suffix: String = (0..8) - .map(|_| char::from_digit(rng.gen_range(0..16), 16).unwrap()) - .collect(); - p.push(format!("{prefix}-{}-{suffix}.sock", std::process::id())); - p + support::ipc_endpoint(prefix) } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { @@ -108,12 +110,12 @@ async fn send_handshake_with_floors( async fn handshake_ok_when_protocol_matches() { let (handle, _sock) = spawn_daemon().await; let mut ws = open_ws(handle.ws_addr()).await; - let resp = send_handshake(&mut ws, "1.3", env!("CARGO_PKG_VERSION")).await; + let resp = send_handshake(&mut ws, PROTOCOL_VERSION, env!("CARGO_PKG_VERSION")).await; let result: HandshakeResult = match resp.body { ResponseBody::Ok(v) => serde_json::from_value(v).unwrap(), ResponseBody::Err(e) => panic!("expected ok handshake, got {e:?}"), }; - assert_eq!(result.protocol_version, "1.3"); + assert_eq!(result.protocol_version, PROTOCOL_VERSION); assert_eq!( result .min_compatible_peer @@ -134,8 +136,14 @@ async fn handshake_ok_when_protocol_matches() { async fn handshake_ok_when_app_versions_differ_but_protocol_matches() { let (handle, _sock) = spawn_daemon().await; let mut ws = open_ws(handle.ws_addr()).await; - let resp = - send_handshake_with_floors(&mut ws, "1.3", "9.9.9", Some("0.0.0"), Some("1.3")).await; + let resp = send_handshake_with_floors( + &mut ws, + PROTOCOL_VERSION, + "9.9.9", + Some("0.0.0"), + Some(PROTOCOL_VERSION), + ) + .await; match resp.body { ResponseBody::Ok(_) => {} other => panic!("expected ok when protocol matches, got {other:?}"), @@ -149,7 +157,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.4", + &newer_minor_protocol(), env!("CARGO_PKG_VERSION"), Some("0.0.0"), Some("1.3"), @@ -235,7 +243,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.4".into(), + extension_protocol_version: newer_minor_protocol(), label: "Older".into(), sink: bsk::daemon::browsers::BrowserSink { tx }, pending: Mutex::new(bsk::daemon::browsers::Pending::default()), @@ -263,8 +271,8 @@ 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.4"); - assert_eq!(skew.server_protocol_version, "1.3"); + assert_eq!(skew.client_protocol_version, newer_minor_protocol()); + assert_eq!(skew.server_protocol_version, PROTOCOL_VERSION); assert_eq!(skew.client_version, "9.9.9"); let entry = status .browsers diff --git a/crates/bsk-cli/tests/per_session_queue.rs b/crates/bsk-cli/tests/per_session_queue.rs index 4210be92..2cdfd7dd 100644 --- a/crates/bsk-cli/tests/per_session_queue.rs +++ b/crates/bsk-cli/tests/per_session_queue.rs @@ -178,6 +178,7 @@ async fn run_fake_extension_with_reply( id: req.id.clone(), body: ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(id), }) diff --git a/crates/bsk-cli/tests/record_export_recovery.rs b/crates/bsk-cli/tests/record_export_recovery.rs index a216c7de..3b512edc 100644 --- a/crates/bsk-cli/tests/record_export_recovery.rs +++ b/crates/bsk-cli/tests/record_export_recovery.rs @@ -120,6 +120,7 @@ fn run_extension( serde_json::from_value(request.params.clone().unwrap()).unwrap(); ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(100), }) diff --git a/crates/bsk-cli/tests/record_stop_retry.rs b/crates/bsk-cli/tests/record_stop_retry.rs index 7d42a57c..2c8ac27a 100644 --- a/crates/bsk-cli/tests/record_stop_retry.rs +++ b/crates/bsk-cli/tests/record_stop_retry.rs @@ -123,6 +123,7 @@ fn run_extension( serde_json::from_value(request.params.clone().unwrap()).unwrap(); ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(100), }) diff --git a/crates/bsk-cli/tests/session_user_interrupt.rs b/crates/bsk-cli/tests/session_user_interrupt.rs index d8782195..7564b3d6 100644 --- a/crates/bsk-cli/tests/session_user_interrupt.rs +++ b/crates/bsk-cli/tests/session_user_interrupt.rs @@ -158,6 +158,7 @@ async fn session_user_interrupt_event_cancels_inflight_with_user_aborted() { match req.method { Method::ToolSessionStart => { let result = SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(1), }; @@ -335,6 +336,7 @@ async fn assert_idle_interrupt_rejects(method: Method) { if let Frame::Request(req) = frame { if req.method == Method::ToolSessionStart { let result = SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(1), }; @@ -464,6 +466,7 @@ async fn read_only_tool_passes_through_without_consuming_interrupt_marker() { let body = match req.method { Method::ToolSessionStart => ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(1), }) @@ -623,6 +626,7 @@ async fn user_interrupt_marker_survives_long_delay_before_next_tool() { let body = match req.method { Method::ToolSessionStart => ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(1), }) diff --git a/crates/bsk-cli/tests/sessions_ipc.rs b/crates/bsk-cli/tests/sessions_ipc.rs index be4db29a..8f5f4992 100644 --- a/crates/bsk-cli/tests/sessions_ipc.rs +++ b/crates/bsk-cli/tests/sessions_ipc.rs @@ -19,7 +19,6 @@ use bsk_protocol::{ BrowserPeerInfo, ErrorCode, Frame, Method, RequestFrame, ResponseBody, ResponseFrame, RpcError, }; use futures_util::{SinkExt, StreamExt}; -use rand::Rng; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -28,17 +27,110 @@ use support::{wait_for_browser_count, wait_for_no_sessions}; const TEST_EXT_ID: &str = "abcdefghijklmnopabcdefghijklmnop"; +#[tokio::test] +async fn shared_session_rejects_legacy_extension_before_start_request() { + let (handle, sock) = spawn_daemon().await; + let mut ws = connect_ext(handle.ws_addr()).await; + handshake_with_protocol(&mut ws, "1.3").await; + let mut ipc = IpcClient::connect(&sock).await.unwrap(); + let result = ipc + .call::<_, serde_json::Value>( + "shared-old", + Method::SessionStart, + Some(serde_json::json!({"in_window":true})), + Duration::from_secs(3), + ) + .await + .unwrap(); + assert_eq!(result.unwrap_err().code, ErrorCode::Unsupported); + assert!( + tokio::time::timeout(Duration::from_millis(100), next_extension_request(&mut ws)) + .await + .is_err() + ); + handle.shutdown().await; +} + +#[tokio::test] +async fn shared_mode_round_trips_and_tabs_closed_removes_only_its_session() { + let (handle, sock) = spawn_daemon().await; + let mut ws = connect_ext(handle.ws_addr()).await; + handshake_as_ext(&mut ws).await; + let ext = tokio::spawn(async move { + let mut ids = Vec::new(); + for _ in 0..2 { + let req = next_extension_request(&mut ws).await; + assert_eq!(req.method, Method::ToolSessionStart); + let params: SessionStartParams = serde_json::from_value(req.params.unwrap()).unwrap(); + assert!(params.in_window); + ids.push(params.session_id); + send_extension_response( + &mut ws, + ResponseFrame { + id: req.id, + body: ResponseBody::Ok( + serde_json::json!({"agent_window_id":10,"container_mode":"in_window"}), + ), + }, + ) + .await; + } + ws.send(Message::Text(serde_json::json!({"event":"session.tabs_closed","payload":{"session_id":ids[0],"reason":"no_controlled_tabs"}}).to_string())).await.unwrap(); + ws + }); + let mut ipc = IpcClient::connect(&sock).await.unwrap(); + let mut starts = Vec::new(); + for id in ["shared-a", "shared-b"] { + let result: serde_json::Value = ipc + .call( + id, + Method::SessionStart, + Some(serde_json::json!({"in_window":true})), + Duration::from_secs(5), + ) + .await + .unwrap() + .unwrap(); + assert_eq!(result["container_mode"], "in_window"); + starts.push(result); + } + let _ws = ext.await.unwrap(); + tokio::time::timeout(Duration::from_secs(3), async { + loop { + let status: StatusResult = ipc + .call::<(), _>( + "shared-status", + Method::SystemStatus, + None, + Duration::from_secs(2), + ) + .await + .unwrap() + .unwrap(); + if status.sessions.len() == 1 { + assert_eq!( + status.sessions[0].session_id, + starts[1]["session_id"].as_str().unwrap() + ); + assert_eq!( + status.sessions[0].container_mode.as_deref(), + Some("in_window") + ); + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await + .unwrap(); + handle.shutdown().await; +} + type TestWs = tokio_tungstenite::WebSocketStream>; fn tempfile_path(prefix: &str) -> PathBuf { - let mut p = std::env::temp_dir(); - let mut rng = rand::thread_rng(); - let suffix: String = (0..8) - .map(|_| char::from_digit(rng.gen_range(0..16), 16).unwrap()) - .collect(); - p.push(format!("{prefix}-{}-{suffix}.sock", std::process::id())); - p + support::ipc_endpoint(prefix) } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { @@ -178,6 +270,7 @@ async fn respond_to_aborted_start( id: start.id, body: ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(agent_window_id), }) @@ -214,6 +307,7 @@ async fn respond_to_aborted_stop( id: start.id, body: ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(4242), }) @@ -278,6 +372,7 @@ async fn session_start_stop_round_trip_via_ipc() { serde_json::from_value(req.params.clone().unwrap()).unwrap(); assert_eq!(params.focused, Some(false)); let result = SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(4242), }; @@ -623,6 +718,7 @@ async fn session_idle_timeout_stops_and_unregisters_session() { let state = handle.state(); let session_id = bsk::daemon::sessions::SessionId("idle".into()); state.sessions.insert(bsk::daemon::sessions::Session { + container_mode: None, interaction: None, id: session_id.clone(), @@ -714,6 +810,7 @@ async fn session_start_waits_for_late_extension_handshake() { continue; } let result = SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(4242), }; @@ -854,6 +951,7 @@ async fn session_start_with_browser_instance_id_picks_target() { && req.method == Method::ToolSessionStart { let result = SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(123), }; @@ -959,6 +1057,7 @@ async fn session_start_label_match_picks_target() { && req.method == Method::ToolSessionStart { let result = SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(456), }; @@ -1047,6 +1146,7 @@ async fn session_window_closed_event_purges_session() { let _ = handshake_as_ext(&mut ws).await; let state = handle.state(); let session = bsk::daemon::sessions::Session { + container_mode: None, interaction: None, id: bsk::daemon::sessions::SessionId("zzzz".into()), @@ -1086,6 +1186,7 @@ async fn browser_disconnect_purges_sessions() { // post-start state without bothering with the round-trip. let state = handle.state(); let session = bsk::daemon::sessions::Session { + container_mode: None, interaction: None, id: bsk::daemon::sessions::SessionId("zzzz".into()), @@ -1117,6 +1218,7 @@ async fn session_stop_self_heals_when_extension_reports_not_found() { // restart from the daemon's point of view). let state = handle.state(); let session = bsk::daemon::sessions::Session { + container_mode: None, interaction: None, id: bsk::daemon::sessions::SessionId("yyyy".into()), @@ -1218,6 +1320,7 @@ async fn reconnect_with_same_instance_id_purges_stale_sessions_but_keeps_new_bro let state = handle.state(); // Pretend ext A registered a real session. let session = bsk::daemon::sessions::Session { + container_mode: None, interaction: None, id: bsk::daemon::sessions::SessionId("xxxx".into()), @@ -1355,6 +1458,7 @@ fn interaction_updates_follow_the_owning_browser_in_both_directions() { let owner = BrowserId("owner".into()); let id = SessionId("existing".into()); registry.insert(Session { + container_mode: None, id: id.clone(), browser_id: owner.clone(), agent_window_id: Some(100), @@ -1395,6 +1499,7 @@ async fn borrow_deadline_cancels_the_extension_and_preserves_a_committed_result( let state = handle.state(); let session_id = SessionId("borrow-deadline".into()); state.sessions.insert(Session { + container_mode: None, id: session_id.clone(), browser_id: bsk::daemon::browsers::BrowserId(TEST_EXT_ID.into()), agent_window_id: Some(100), @@ -1466,6 +1571,7 @@ async fn borrow_reports_unknown_outcome_when_cancel_cleanup_never_finishes() { let state = handle.state(); let sid = SessionId("borrow-unknown".into()); state.sessions.insert(Session { + container_mode: None, id: sid.clone(), browser_id: bsk::daemon::browsers::BrowserId(TEST_EXT_ID.into()), agent_window_id: Some(100), @@ -1636,6 +1742,7 @@ mod recoverable_starts { .unwrap(); assert_eq!(conflicting.unwrap_err().code, ErrorCode::InvalidParams); let foreign = bsk::daemon::sessions::Session { + container_mode: None, id: bsk::daemon::sessions::SessionId("foreign".into()), browser_id: bsk::daemon::browsers::BrowserId(TEST_EXT_ID.into()), agent_window_id: Some(999), diff --git a/crates/bsk-cli/tests/shared_window_compat.rs b/crates/bsk-cli/tests/shared_window_compat.rs new file mode 100644 index 00000000..404f7471 --- /dev/null +++ b/crates/bsk-cli/tests/shared_window_compat.rs @@ -0,0 +1,63 @@ +//! Windows counterpart of the Unix discovery test: a legacy daemon must never +//! receive a session.start request for a shared-window session. +#![cfg(windows)] + +use bsk::cli::session::{SessionStartOptions, start_session}; +use bsk_protocol::{ErrorCode, Frame, Method, ResponseBody, ResponseFrame}; +use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; +use tokio::net::windows::named_pipe::ServerOptions; + +#[tokio::test] +async fn shared_session_preflight_rejects_legacy_daemon_over_named_pipe() { + let endpoint: std::path::PathBuf = format!( + r"\\.\pipe\bsk-shared-preflight-{}-{}", + std::process::id(), + uuid::Uuid::new_v4().simple() + ) + .into(); + let pipe = ServerOptions::new() + .first_pipe_instance(true) + .create(&endpoint) + .unwrap(); + let client_endpoint = endpoint.clone(); + let client = tokio::task::spawn_blocking(move || { + start_session( + client_endpoint, + SessionStartOptions { + in_window: true, + ..Default::default() + }, + ) + }); + pipe.connect().await.unwrap(); + // Keep another listener available so an erroneous second RPC cannot merely + // fail to connect and masquerade as a successful compatibility check. + let next = ServerOptions::new().create(&endpoint).unwrap(); + let mut connection = BufReader::new(pipe); + let mut line = String::new(); + connection.read_line(&mut line).await.unwrap(); + let Frame::Request(request) = serde_json::from_str(&line).unwrap() else { + panic!("expected status request"); + }; + assert_eq!(request.method, Method::SystemStatus); + let reply = Frame::Response(ResponseFrame { + id: request.id, + body: ResponseBody::Ok(serde_json::json!({ + "daemon_version": "0.3.0", "protocol_version": "1.3", + "pid": std::process::id(), "uptime_secs": 1, "ws_port": 0, + "sock_path": endpoint, "browsers": [], "sessions": [] + })), + }); + connection + .get_mut() + .write_all(format!("{}\n", serde_json::to_string(&reply).unwrap()).as_bytes()) + .await + .unwrap(); + let error = tokio::select! { + result = client => result.unwrap().unwrap_err(), + result = next.connect() => panic!("legacy daemon received a second connection: {result:?}"), + _ = tokio::time::sleep(std::time::Duration::from_secs(3)) => panic!("preflight timed out"), + }; + assert_eq!(error.code(), Some(ErrorCode::Unsupported)); + assert_eq!(error.data().unwrap()["required_protocol"], "1.4"); +} diff --git a/crates/bsk-cli/tests/support/mod.rs b/crates/bsk-cli/tests/support/mod.rs index 0ef9d531..0036233f 100644 --- a/crates/bsk-cli/tests/support/mod.rs +++ b/crates/bsk-cli/tests/support/mod.rs @@ -15,6 +15,21 @@ use bsk_protocol::RpcId; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(2); +/// Give each embedded daemon a private endpoint on both supported platforms. +pub fn ipc_endpoint(prefix: &str) -> std::path::PathBuf { + #[cfg(windows)] + { + let _ = prefix; + return bsk::daemon::paths::pipe_name().into(); + } + #[cfg(not(windows))] + { + let suffix = uuid::Uuid::new_v4().simple().to_string(); + let name = format!("{prefix}-{}-{}", std::process::id(), &suffix[..8]); + std::env::temp_dir().join(format!("{name}.sock")) + } +} + /// Poll `condition` until it returns `true` or `timeout` elapses. pub async fn wait_until(label: &str, timeout: Duration, mut condition: F) where diff --git a/crates/bsk-cli/tests/tools_ipc.rs b/crates/bsk-cli/tests/tools_ipc.rs index 9f17f060..694138fc 100644 --- a/crates/bsk-cli/tests/tools_ipc.rs +++ b/crates/bsk-cli/tests/tools_ipc.rs @@ -135,6 +135,7 @@ where window_id += 1; ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(id), }) diff --git a/crates/bsk-cli/tests/tools_m7_ipc.rs b/crates/bsk-cli/tests/tools_m7_ipc.rs index ac21f67a..5d5a807b 100644 --- a/crates/bsk-cli/tests/tools_m7_ipc.rs +++ b/crates/bsk-cli/tests/tools_m7_ipc.rs @@ -136,6 +136,7 @@ where window_id += 1; ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(id), }) diff --git a/crates/bsk-cli/tests/tools_m8_ipc.rs b/crates/bsk-cli/tests/tools_m8_ipc.rs index 91c5813b..4924f931 100644 --- a/crates/bsk-cli/tests/tools_m8_ipc.rs +++ b/crates/bsk-cli/tests/tools_m8_ipc.rs @@ -134,6 +134,7 @@ where window_id += 1; ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(id), }) diff --git a/crates/bsk-cli/tests/tools_m9_ipc.rs b/crates/bsk-cli/tests/tools_m9_ipc.rs index 631cedf3..e13ab308 100644 --- a/crates/bsk-cli/tests/tools_m9_ipc.rs +++ b/crates/bsk-cli/tests/tools_m9_ipc.rs @@ -144,6 +144,7 @@ where window_id += 1; ResponseBody::Ok( serde_json::to_value(SessionStartResult { + container_mode: None, interaction: None, agent_window_id: Some(id), }) diff --git a/crates/bsk-protocol/schema/system_status_result.json b/crates/bsk-protocol/schema/system_status_result.json index 4e378e0d..624225e2 100644 --- a/crates/bsk-protocol/schema/system_status_result.json +++ b/crates/bsk-protocol/schema/system_status_result.json @@ -167,6 +167,12 @@ "browser_instance_id": { "type": "string" }, + "container_mode": { + "type": [ + "string", + "null" + ] + }, "created_at_ms": { "description": "Unix epoch milliseconds.", "type": "integer", diff --git a/crates/bsk-protocol/schema/tool_session_start_params.json b/crates/bsk-protocol/schema/tool_session_start_params.json index 0b8eb212..5313f264 100644 --- a/crates/bsk-protocol/schema/tool_session_start_params.json +++ b/crates/bsk-protocol/schema/tool_session_start_params.json @@ -28,6 +28,9 @@ "format": "uint32", "minimum": 0.0 }, + "in_window": { + "type": "boolean" + }, "session_id": { "type": "string" }, diff --git a/crates/bsk-protocol/schema/tool_session_start_result.json b/crates/bsk-protocol/schema/tool_session_start_result.json index 6649f7ba..4b6a4034 100644 --- a/crates/bsk-protocol/schema/tool_session_start_result.json +++ b/crates/bsk-protocol/schema/tool_session_start_result.json @@ -10,6 +10,12 @@ ], "format": "int64" }, + "container_mode": { + "type": [ + "string", + "null" + ] + }, "interaction": { "anyOf": [ { diff --git a/crates/bsk-protocol/schema/tool_tab_create_params.json b/crates/bsk-protocol/schema/tool_tab_create_params.json index 25a7727a..713708bc 100644 --- a/crates/bsk-protocol/schema/tool_tab_create_params.json +++ b/crates/bsk-protocol/schema/tool_tab_create_params.json @@ -1,7 +1,7 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "title": "TabCreateParams", - "description": "Params for `tool.tab_create`. The new tab is always created inside the requesting session's Agent Window (design §6 sandbox rule — agents never spawn tabs in user windows).", + "description": "Params for `tool.tab_create`. The new tab is created in the session's dedicated Agent Window or explicitly selected shared host window.", "type": "object", "required": [ "session_id" @@ -15,7 +15,7 @@ ] }, "index": { - "description": "Insertion index within the Agent Window's tab strip. Omit to append at the end.", + "description": "Insertion index within the session window's tab strip. Omit to append at the end.", "type": [ "integer", "null" diff --git a/crates/bsk-protocol/schema/tool_tab_list_params.json b/crates/bsk-protocol/schema/tool_tab_list_params.json index af5e95e6..6665ac0c 100644 --- a/crates/bsk-protocol/schema/tool_tab_list_params.json +++ b/crates/bsk-protocol/schema/tool_tab_list_params.json @@ -20,7 +20,7 @@ }, "definitions": { "TabScope": { - "description": "View scope for [`TabListParams`] (§6 sandbox table).\n\n* `User` — tabs that live in any window other than an Agent Window. * `Agent` — tabs in the requesting session's Agent Window only. * `All` — both of the above; never reveals other sessions' Agent Windows (cross-session isolation per §6).", + "description": "View scope for [`TabListParams`] (§6 sandbox table).\n\n* `User` — user pages outside dedicated Agent Windows, including shared hosts. * `Agent` — pages in the requesting session's dedicated Agent Window, or explicitly controlled pages in its shared host. * `All` — both of the above; never reveals other sessions' Agent Windows (cross-session isolation per §6).", "type": "string", "enum": [ "user", diff --git a/crates/bsk-protocol/schema/tool_tab_list_result.json b/crates/bsk-protocol/schema/tool_tab_list_result.json index dc05e633..9469ad7d 100644 --- a/crates/bsk-protocol/schema/tool_tab_list_result.json +++ b/crates/bsk-protocol/schema/tool_tab_list_result.json @@ -64,7 +64,7 @@ } }, "TabScope": { - "description": "View scope for [`TabListParams`] (§6 sandbox table).\n\n* `User` — tabs that live in any window other than an Agent Window. * `Agent` — tabs in the requesting session's Agent Window only. * `All` — both of the above; never reveals other sessions' Agent Windows (cross-session isolation per §6).", + "description": "View scope for [`TabListParams`] (§6 sandbox table).\n\n* `User` — user pages outside dedicated Agent Windows, including shared hosts. * `Agent` — pages in the requesting session's dedicated Agent Window, or explicitly controlled pages in its shared host. * `All` — both of the above; never reveals other sessions' Agent Windows (cross-session isolation per §6).", "type": "string", "enum": [ "user", diff --git a/crates/bsk-protocol/src/frame.rs b/crates/bsk-protocol/src/frame.rs index 9af29730..26ed61c3 100644 --- a/crates/bsk-protocol/src/frame.rs +++ b/crates/bsk-protocol/src/frame.rs @@ -54,6 +54,8 @@ pub enum EventKind { SessionActivity, #[serde(rename = "session.window_closed")] SessionWindowClosed, + #[serde(rename = "session.tabs_closed")] + SessionTabsClosed, #[serde(rename = "session.user_interrupt")] SessionUserInterrupt, #[serde(rename = "session.interaction_changed")] diff --git a/crates/bsk-protocol/src/system.rs b/crates/bsk-protocol/src/system.rs index 1bbbf416..df24ff2c 100644 --- a/crates/bsk-protocol/src/system.rs +++ b/crates/bsk-protocol/src/system.rs @@ -523,6 +523,8 @@ pub struct BrowserStatusEntry { /// Snapshot of a single live session. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] pub struct SessionStatusEntry { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub container_mode: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub interaction: Option, pub session_id: String, diff --git a/crates/bsk-protocol/src/tools/session.rs b/crates/bsk-protocol/src/tools/session.rs index aa945e14..2c0b26f3 100644 --- a/crates/bsk-protocol/src/tools/session.rs +++ b/crates/bsk-protocol/src/tools/session.rs @@ -40,6 +40,8 @@ pub struct InteractionPolicy { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct SessionStartParams { + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub in_window: bool, pub session_id: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub browser_instance_id: Option, @@ -61,6 +63,8 @@ pub struct SessionStartParams { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct SessionStartResult { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub container_mode: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub interaction: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -161,3 +165,39 @@ mod tests { assert_eq!(encoded["return_failures"][0]["code"], "cdp_failed"); } } + +/// Shared-window support is optional within protocol major 1. +pub fn supports_shared_window(protocol: &str) -> bool { + crate::system::compare_protocol(protocol, "2.0") == Some(std::cmp::Ordering::Less) + && matches!( + crate::system::compare_protocol(protocol, "1.4"), + Some(std::cmp::Ordering::Equal | std::cmp::Ordering::Greater) + ) +} + +#[cfg(test)] +mod shared_window_tests { + use super::*; + #[test] + fn support_is_optional_and_bounded_to_this_major() { + for version in ["1.0", "1.3", "2.0", "invalid"] { + assert!(!supports_shared_window(version)); + } + for version in ["1.4", "1.5"] { + assert!(supports_shared_window(version)); + } + let legacy: SessionStartParams = + serde_json::from_value(serde_json::json!({"session_id":"test"})).unwrap(); + assert!(!legacy.in_window); + assert!( + serde_json::to_value(legacy) + .unwrap() + .get("in_window") + .is_none() + ); + let shared: SessionStartParams = + serde_json::from_value(serde_json::json!({"session_id":"test", "in_window":true})) + .unwrap(); + assert!(shared.in_window); + } +} diff --git a/crates/bsk-protocol/src/tools/tabs.rs b/crates/bsk-protocol/src/tools/tabs.rs index 6e7921ae..cab21a72 100644 --- a/crates/bsk-protocol/src/tools/tabs.rs +++ b/crates/bsk-protocol/src/tools/tabs.rs @@ -5,8 +5,9 @@ use serde::{Deserialize, Serialize}; /// View scope for [`TabListParams`] (§6 sandbox table). /// -/// * `User` — tabs that live in any window other than an Agent Window. -/// * `Agent` — tabs in the requesting session's Agent Window only. +/// * `User` — user pages outside dedicated Agent Windows, including shared hosts. +/// * `Agent` — pages in the requesting session's dedicated Agent Window, +/// or explicitly controlled pages in its shared host. /// * `All` — both of the above; never reveals other sessions' Agent /// Windows (cross-session isolation per §6). #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema, Default)] @@ -58,9 +59,8 @@ pub struct TabListResult { // tab_create (M8.1) // --------------------------------------------------------------------------- -/// Params for `tool.tab_create`. The new tab is always created inside -/// the requesting session's Agent Window (design §6 sandbox rule — -/// agents never spawn tabs in user windows). +/// Params for `tool.tab_create`. The new tab is created in the session's +/// dedicated Agent Window or explicitly selected shared host window. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct TabCreateParams { pub session_id: String, @@ -70,7 +70,7 @@ pub struct TabCreateParams { /// Focus the new tab? Defaults to `true` (matches `chrome.tabs.create`). #[serde(default, skip_serializing_if = "Option::is_none")] pub active: Option, - /// Insertion index within the Agent Window's tab strip. Omit to + /// Insertion index within the session window's tab strip. Omit to /// append at the end. #[serde(default, skip_serializing_if = "Option::is_none")] pub index: Option, diff --git a/docs/architecture.md b/docs/architecture.md index c98d771e..2c3bfe6d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -53,7 +53,7 @@ Key modules: - In local mode, listens on loopback WebSocket (default **52800**, configurable with `bsk daemon start --port`) for extensions. The extension popup saves the matching connection port; saving ends existing sessions and reconnects when enabled. - Server mode supports authenticated remote extension connections with device pairing, renewal and revocation. Native TLS or a TLS reverse proxy provides WSS; the deployment supervisor owns server restarts. - Validates `Origin: chrome-extension://…` on handshake. -- Maintains `browsers` (connected extensions) and `sessions` (Agent Window bindings). +- Maintains `browsers` (connected extensions) and `sessions` (window containers and page ownership). - **Per-session queue** serializes tool calls targeting one session. - Forwards `tool.*` RPCs to the correct extension connection. @@ -74,10 +74,10 @@ WXT / MV3 Chromium extension. Built with React popup and a service worker backgr | --- | --- | | `transport/` | Pluggable `Transport` (v1: `WSTransport`) | | `tools/` | `ToolDispatcher` → 21 tool handlers | -| `session-manager/` | Sessions, Agent Window, ref-store (`@e1`) | +| `session-manager/` | Dedicated/shared session containers, tab ownership, ref-store (`@e1`) | | `browser-driver/` | CDP-backed browser operations | | `entrypoints/popup/` | Connection status UI | -| `content/` | Control overlay in Agent Windows | +| `content/` | Control overlay on session-controlled pages | ### bsk-protocol (`crates/bsk-protocol`) @@ -99,13 +99,17 @@ mutation for session queueing and user-interruption gating. ## Session and sandbox model -- **Session** = opaque ID (4 lowercase letters in v0.1) + dedicated **Agent Window** - + session-scoped ref-store + borrow table. -- **Sandbox-only**: write tools require tabs inside the Agent Window unless the tab - was **borrowed** from the user profile. +- **Session** = opaque ID + window container + session-scoped ref-store and tab ownership. + The default container is a dedicated **Agent Window**. Local `session start --in-window` + creates a controlled tab in the last-focused normal user window (protocol 1.4). +- **Write scope**: dedicated sessions use their Agent Window. Shared sessions require + both explicit tab ownership (created or borrowed) and location in their host window. + Sharing a host never grants control of user pages or another session's pages. - **Session stop is mandatory** in agent workflows (`bsk session stop`); idle timeout (default 5 min) is a safety net only. -- Multiple sessions on one browser → multiple Agent Windows, fully isolated. +- Multiple sessions may use separate Agent Windows or share a local user window; + controlled pages remain isolated by session. Shared cleanup returns borrowed pages + and removes its created pages without actively closing the host window. - Remote content reads and actions require task-created or explicitly borrowed tabs. A page-opened popup or a user tab moved into the Agent Window does not become controlled automatically; see [remote tab ownership](remote-extension-connection.md#browser-permissions-and-task-lifetime). @@ -114,9 +118,9 @@ mutation for session queueing and user-interruption gating. | `scope` | Visible tabs | | --- | --- | -| `user` | User profile windows (default) | -| `agent` | Current session's Agent Window only | -| `all` | Agent Window + user windows for this session | +| `user` | User pages, excluding other sessions' controlled pages and dedicated windows | +| `agent` | Current dedicated Agent Window, or explicitly controlled pages in a shared host | +| `all` | Both scopes, excluding other sessions' controlled pages and dedicated windows | ## Concurrency @@ -161,7 +165,7 @@ flowchart LR - Website cookies stay in the user's browser profile. Remote device credentials are stored in extension-origin IndexedDB; the built-in server stores credential hashes in its private `BSK_HOME`. Pairing and device grants govern remote access. -- `evaluate` restricted to Agent Window tabs in sandbox mode. +- `evaluate` follows session write scope, including explicit ownership in shared mode. - Operation audit, when enabled, is stored on the daemon host, including the server in remote mode. See [operation audit](operation-audit.md). diff --git a/docs/long-screenshot.md b/docs/long-screenshot.md index 4c562946..3afae114 100644 --- a/docs/long-screenshot.md +++ b/docs/long-screenshot.md @@ -53,9 +53,12 @@ and retry a clearly stale frame twice before failing with `stale_frame`. Blank o ambiguous content alone does not trigger this error. Layout and stale-frame retries have separate consecutive-failure counters. -On other platforms, Agent captures retain a working window-surface source and fall -back to their session renderer only if the initial probe fails. Popup captures keep -their existing backend selection and do not enable the Agent freshness check. +On other platforms, dedicated Agent Window captures retain a working window-surface +source and fall back to their session renderer only if the initial probe fails. +Local shared-window sessions (`--in-window`) always use their tab-specific session +renderer on every platform; failure never falls back to window-surface capture. +Only Windows enables the renderer freshness check. Popup captures keep their +existing backend selection and do not enable the Agent freshness check. Screenshot backends never switch midway through an image. In Agent `follow` mode, 30 seconds at an unchanged bottom with a rendered loading @@ -67,7 +70,7 @@ page contact. Keeping the tab selected is necessary; hiding it can stop capture. Individual browser operations retain their own shorter deadlines. The default viewport screenshot and `--ref` crop are unchanged. `--full-page` is exclusive -with `--ref`. An optional `--tab-id` identifies a tab in the session's Agent Window; +with `--ref`. An optional `--tab-id` identifies a tab in the session's dedicated or shared host window; the tab must have been created or borrowed by that session. Background targets are supported without selecting the tab or focusing the window. Agent capture uses target-scoped CDP for every viewport, with no fallback to the window's selected tab. Automatic document diff --git a/docs/remote-extension-connection.md b/docs/remote-extension-connection.md index a4723e43..94da1d75 100644 --- a/docs/remote-extension-connection.md +++ b/docs/remote-extension-connection.md @@ -199,7 +199,7 @@ Cross-window popups are never automatically moved or claimed. They use the ordin Disconnecting cancels task work, returns borrowed tabs and closes task-created tabs. User-created tabs survive cleanup. Failed returns preserve the window and must be resolved before reconnecting. Reconnection starts new tasks; commands and sessions are never replayed. Failed remote authentication does not select a local connection automatically. -Remote upload and download are unsupported in this version and return the `unsupported` error. Existing local file transfer behavior is unchanged. Screenshots and other existing RPC content results remain supported. There is no background task tab group or alternative window model. +Remote upload and download are unsupported in this version and return the `unsupported` error. Existing local file transfer behavior is unchanged. Screenshots and other existing RPC content results remain supported. Remote mode has no background task tab group or alternative window model; local `session start --in-window` is not supported remotely. Device credentials live in extension-origin IndexedDB; ordinary extension settings contain only the selected connection mode and a non-secret revision. Fresh local profiles and explicitly selected local mode do not read that credential database. If remote storage fails, the popup reports the error and the extension does not fall back automatically. Explicitly selecting the local connection can recover startup even when the credential database is unavailable. Legacy remote settings still migrate before ordinary settings access is restored. The standalone server persists hashed credentials with private file permissions and atomic writes. Treat the whole browser profile and `BSK_HOME` as trusted local data. Protect TLS private keys separately. diff --git a/docs/scroll-to.md b/docs/scroll-to.md index d8176d18..439d98b6 100644 --- a/docs/scroll-to.md +++ b/docs/scroll-to.md @@ -26,10 +26,12 @@ Refs accept `@e3` and `e3`. CSS selectors search only the main document; use ref for elements in iframes (including out-of-process iframes) or shadow roots. Refs belong to one session and tab and may become stale after page changes. -`--session` is required. `--tab-id` defaults to the Agent Window's active tab. +`--session` is required. `--tab-id` defaults to the Agent Window's active tab, +or the session's selected controlled tab in local `--in-window` mode. `--timeout` defaults to `30s` and must be positive; durations such as `5000ms` and -`5s` are accepted. The command operates on Agent Window tabs, including user tabs -explicitly borrowed into that window. +`5s` are accepted. The command operates on Agent Window tabs, or explicitly +controlled tabs in a shared host. User tabs require borrowing; same-window +borrowing grants control without moving the page. Example JSON output: @@ -111,7 +113,7 @@ The wire response uses the normal `error.code`, `error.message` and optional | `invalid_params` | — | Missing/conflicting target or invalid timeout/tab id | | `not_found` | `ref_not_found` | Ref is unknown, expired or belongs to another tab | | `not_found` | `selector_not_found` | Main-document selector matched no element | -| `permission_denied` | `agent_window_scope` | Target tab has not been borrowed into the Agent Window | +| `permission_denied` | `agent_window_scope` | Target tab is outside the session window; borrow it into the session first | | `permission_denied` | `element_not_visible` | No visible area remains after scrolling | | `cancelled` | — | The call was cancelled | | `timeout` | — | The action deadline expired | diff --git a/docs/wheel.md b/docs/wheel.md index 2de3d26f..50c899ee 100644 --- a/docs/wheel.md +++ b/docs/wheel.md @@ -31,8 +31,10 @@ one session and tab. CSS selectors search the main document; use refs for iframe or shadow-root elements. Unknown, stale and cross-tab refs fail instead of falling back to the viewport. -`--session` is required. `--tab-id` defaults to the Agent Window's active tab; -user tabs must first be borrowed into that window. `--timeout` defaults to +`--session` is required. `--tab-id` defaults to the Agent Window's active tab, +or the session's selected controlled tab in local `--in-window` mode. +User tabs must first be borrowed by the session; same-window borrowing does not +move the page. `--timeout` defaults to `30s` and must be positive. Modifiers are comma-separated `alt,ctrl,meta,shift`. ## Input and result contract From ae12be0955234104755e87e751b552da13f28720 Mon Sep 17 00:00:00 2001 From: lymerin <884917500@qq.com> Date: Fri, 2 Oct 2026 21:39:43 +0800 Subject: [PATCH 2/3] fix(session): preserve the shared host window during teardown --- .../src/debug/__tests__/manager.test.ts | 1 + .../__tests__/event-handler.test.ts | 1 + .../__tests__/shared-window.test.ts | 52 ++++++++++++++++++- apps/extension/src/session-manager/manager.ts | 33 +++++++----- .../src/session-manager/shared-window.ts | 2 + 5 files changed, 75 insertions(+), 14 deletions(-) diff --git a/apps/extension/src/debug/__tests__/manager.test.ts b/apps/extension/src/debug/__tests__/manager.test.ts index 588ce606..82ebb70d 100644 --- a/apps/extension/src/debug/__tests__/manager.test.ts +++ b/apps/extension/src/debug/__tests__/manager.test.ts @@ -95,6 +95,7 @@ describe("task-scoped debug lifecycle", () => { host: async () => ({ id: 200, type: "normal", incognito: false }) as chrome.windows.Window, create: async () => 8, get: async () => ({ id: 8, windowId: 200 }) as chrome.tabs.Tab, + query: async () => [{ id: 8, windowId: 200 }] as chrome.tabs.Tab[], remove: async () => {}, }, }); diff --git a/apps/extension/src/session-manager/__tests__/event-handler.test.ts b/apps/extension/src/session-manager/__tests__/event-handler.test.ts index e8dd7aaf..95b9c3a7 100644 --- a/apps/extension/src/session-manager/__tests__/event-handler.test.ts +++ b/apps/extension/src/session-manager/__tests__/event-handler.test.ts @@ -147,6 +147,7 @@ describe("attachSessionEventHandler", () => { host: async () => ({ id: 4242, type: "normal", incognito: false }) as chrome.windows.Window, create: async () => 1, get: async () => ({ id: 1, windowId: 4242 }) as chrome.tabs.Tab, + query: async () => [{ id: 1, windowId: 4242 }] as chrome.tabs.Tab[], remove: removeTab, }, }); diff --git a/apps/extension/src/session-manager/__tests__/shared-window.test.ts b/apps/extension/src/session-manager/__tests__/shared-window.test.ts index 1d47751c..f620b372 100644 --- a/apps/extension/src/session-manager/__tests__/shared-window.test.ts +++ b/apps/extension/src/session-manager/__tests__/shared-window.test.ts @@ -52,9 +52,12 @@ function fixture() { ensureActiveTab: vi.fn(async () => 90), remove: vi.fn(async () => {}), }; + const queryHost = vi.fn(async (windowId: number) => + [...pages.values()].filter((tab) => tab.windowId === windowId), + ); const manager = new SessionManager({ agentWindow: windows, - sharedWindow: { host, get, create, remove }, + sharedWindow: { host, get, create, remove, query: queryHost }, }); const query = vi.fn(async () => [...pages.values()]); return { manager, pages, host, get, create, remove, windows, query }; @@ -137,7 +140,7 @@ describe("shared user window sessions (#243)", () => { ).toMatchObject({ tabId: 20 }); }); - it("normal tool stop never queries or removes the host window", async () => { + it("normal tool stop preserves user pages without dedicated-window cleanup", async () => { const f = fixture(); await f.manager.start("a", { inWindow: true }); f.query.mockRejectedValue(new Error("query denied")); @@ -151,6 +154,51 @@ describe("shared user window sessions (#243)", () => { expect(f.query).not.toHaveBeenCalled(); expect(f.windows.remove).not.toHaveBeenCalled(); expect(f.pages.has(1)).toBe(true); + expect(f.create).toHaveBeenCalledOnce(); + }); + + it.each([ + "tool", + "stop", + "stopAll", + ] as const)("%s preserves the host when only the session tab remains, without a close event", async (method) => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + f.pages.delete(1); + const send = vi.fn(); + const events = { addListener: vi.fn(), removeListener: vi.fn() }; + const handler = attachSessionEventHandler({ + manager: f.manager, + transport: { send } as never, + windowEvents: events, + }); + let hostAlive = true; + const expectedDuringRemove: boolean[] = []; + const remove = f.remove.getMockImplementation()!; + f.remove.mockImplementation(async (id) => { + expectedDuringRemove.push(f.manager.isWindowCloseExpected(ctx)); + await remove(id); + f.manager.forgetClosedTab(id); + if (![...f.pages.values()].some((tab) => tab.windowId === 10)) { + hostAlive = false; + events.addListener.mock.calls[0][0](10); + } + }); + try { + if (method === "tool") + expect(await handleSessionStop(f.manager, { session_id: "a" })).toEqual({}); + else if (method === "stopAll") await f.manager.stopAll(); + else await f.manager.stop("a"); + expect(hostAlive).toBe(true); + expect([...f.pages.values()]).toMatchObject([{ id: 21, windowId: 10, active: false }]); + expect(f.manager.findByTabId(21)).toBeNull(); + expect(f.manager.has("a")).toBe(false); + expect(expectedDuringRemove).toEqual([true]); + expect(f.manager.isWindowCloseExpected(ctx)).toBe(false); + expect(send).not.toHaveBeenCalled(); + } finally { + handler.dispose(); + } }); it.each([ diff --git a/apps/extension/src/session-manager/manager.ts b/apps/extension/src/session-manager/manager.ts index 9f00ba9e..fd3cc615 100644 --- a/apps/extension/src/session-manager/manager.ts +++ b/apps/extension/src/session-manager/manager.ts @@ -170,12 +170,13 @@ export class SessionManager { /** Suppress close/empty callbacks while session.stop owns teardown. */ async withExpectedWindowClose(ctx: SessionContext, close: () => Promise): Promise { + const alreadyExpected = this.isWindowCloseExpected(ctx); this.expectedWindowClosures.add(ctx); try { return await close(); } finally { // Failed teardown must not hide a later user-initiated close. - this.expectedWindowClosures.delete(ctx); + if (!alreadyExpected) this.expectedWindowClosures.delete(ctx); } } @@ -486,20 +487,28 @@ export class SessionManager { // Borrowed pages must be returned by the tool-level teardown first. if (ctx.borrowedTabs.size) throw new Error("Return borrowed tabs before stopping this session"); + const hostWindowId = ctx.container.hostWindowId; ctx.stopping = true; try { - for (const tabId of [...ctx.agentCreatedTabs]) { - try { - const tab = await this.sharedWindow.get(tabId); - // Moving a shared session page out is a user reclaim, including - // when onAttached has not yet reached the service worker. - if (tab.windowId === ctx.container.hostWindowId) - await this.sharedWindow.remove(tabId); - } catch (err) { - if (!/No tab with id|Invalid tab ID|not found/i.test(String(err))) throw err; + await this.withExpectedWindowClose(ctx, async () => { + for (const tabId of [...ctx.agentCreatedTabs]) { + try { + const tab = await this.sharedWindow.get(tabId); + // Moving a shared session page out is a user reclaim, including + // when onAttached has not yet reached the service worker. + if (tab.windowId === hostWindowId) { + // Chrome closes a window when its final tab is removed. + // Leave an unowned blank page to preserve the user's host. + if ((await this.sharedWindow.query(hostWindowId)).length === 1) + await this.sharedWindow.create(hostWindowId, false); + await this.sharedWindow.remove(tabId); + } + } catch (err) { + if (!/No tab with id|Invalid tab ID|not found/i.test(String(err))) throw err; + } + ctx.agentCreatedTabs.delete(tabId); } - ctx.agentCreatedTabs.delete(tabId); - } + }); } catch (err) { ctx.stopping = false; throw err; diff --git a/apps/extension/src/session-manager/shared-window.ts b/apps/extension/src/session-manager/shared-window.ts index 4590ea1e..f1ae4d90 100644 --- a/apps/extension/src/session-manager/shared-window.ts +++ b/apps/extension/src/session-manager/shared-window.ts @@ -3,6 +3,7 @@ export interface SharedWindowApi { host(): Promise; create(windowId: number, focused: boolean): Promise; get(tabId: number): Promise; + query(windowId: number): Promise; remove(tabId: number): Promise; focus?(windowId: number): Promise; } @@ -15,6 +16,7 @@ export const chromeSharedWindowApi: SharedWindowApi = { return tab.id; }, get: (tabId) => chrome.tabs.get(tabId), + query: (windowId) => chrome.tabs.query({ windowId }), remove: (tabId) => chrome.tabs.remove(tabId), focus: async (windowId) => { await chrome.windows.update(windowId, { focused: true }); From 7c5a1249e56d813c712fa9bce1a4fbfee2e1135b Mon Sep 17 00:00:00 2001 From: lymerin <884917500@qq.com> Date: Sat, 3 Oct 2026 00:47:31 +0800 Subject: [PATCH 3/3] fix(session): preserve shared hosts and roll back unused placeholders --- .../__tests__/shared-window.test.ts | 204 ++++++++++++++- .../src/session-manager/event-handler.ts | 6 +- apps/extension/src/session-manager/manager.ts | 51 ++-- .../src/session-manager/shared-window.ts | 27 ++ .../src/session-manager/task-popups.ts | 1 + apps/extension/src/tools/session.ts | 14 +- apps/extension/src/tools/tabs.ts | 243 +++++++++++------- 7 files changed, 421 insertions(+), 125 deletions(-) diff --git a/apps/extension/src/session-manager/__tests__/shared-window.test.ts b/apps/extension/src/session-manager/__tests__/shared-window.test.ts index f620b372..6206a6d0 100644 --- a/apps/extension/src/session-manager/__tests__/shared-window.test.ts +++ b/apps/extension/src/session-manager/__tests__/shared-window.test.ts @@ -43,7 +43,7 @@ function fixture() { windowId, active, index: id, - url: "https://agent.example/", + url: "about:blank", } as chrome.tabs.Tab); return id; }); @@ -60,7 +60,7 @@ function fixture() { sharedWindow: { host, get, create, remove, query: queryHost }, }); const query = vi.fn(async () => [...pages.values()]); - return { manager, pages, host, get, create, remove, windows, query }; + return { manager, pages, host, get, create, remove, windows, query, queryHost }; } describe("shared user window sessions (#243)", () => { @@ -173,10 +173,10 @@ describe("shared user window sessions (#243)", () => { windowEvents: events, }); let hostAlive = true; - const expectedDuringRemove: boolean[] = []; + const stoppingDuringRemove: boolean[] = []; const remove = f.remove.getMockImplementation()!; f.remove.mockImplementation(async (id) => { - expectedDuringRemove.push(f.manager.isWindowCloseExpected(ctx)); + stoppingDuringRemove.push(ctx.stopping === true); await remove(id); f.manager.forgetClosedTab(id); if (![...f.pages.values()].some((tab) => tab.windowId === 10)) { @@ -193,7 +193,7 @@ describe("shared user window sessions (#243)", () => { expect([...f.pages.values()]).toMatchObject([{ id: 21, windowId: 10, active: false }]); expect(f.manager.findByTabId(21)).toBeNull(); expect(f.manager.has("a")).toBe(false); - expect(expectedDuringRemove).toEqual([true]); + expect(stoppingDuringRemove).toEqual([true]); expect(f.manager.isWindowCloseExpected(ctx)).toBe(false); expect(send).not.toHaveBeenCalled(); } finally { @@ -201,18 +201,206 @@ describe("shared user window sessions (#243)", () => { } }); + it("queries the host once and removes only live owned tabs without individual lookups", async () => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + ctx.agentCreatedTabs.add(await f.create(10, false)); + ctx.agentCreatedTabs.add(await f.create(10, false)); + ctx.agentCreatedTabs.add(await f.create(10, false)); + f.pages.get(21)!.windowId = 11; + f.pages.delete(22); + f.pages.delete(1); + f.get.mockClear(); + await f.manager.stop("a"); + expect(f.queryHost).toHaveBeenCalledExactlyOnceWith(10); + expect(f.get).not.toHaveBeenCalled(); + expect(f.remove.mock.calls).toEqual([[20], [23]]); + expect(f.pages.get(21)?.windowId).toBe(11); + expect(f.pages.get(24)?.windowId).toBe(10); + }); + + it.each([ + "url", + "pendingUrl", + ] as const)("keeps a placeholder used by the user during failed stop (%s)", async (urlField) => { + const f = fixture(); + const ctx = await f.manager.start("a", { inWindow: true }); + f.pages.delete(1); + f.remove.mockImplementationOnce(async () => { + f.pages.get(21)![urlField] = "https://user.example/"; + throw new Error("remove denied"); + }); + await expect(f.manager.stop("a")).rejects.toThrow("remove denied"); + expect(f.remove.mock.calls).toEqual([[20]]); + expect(f.pages.get(21)?.[urlField]).toBe("https://user.example/"); + expect(f.manager.get("a")).toBe(ctx); + expect(ctx.stopping).toBe(false); + }); + + it.each([ + "tab_return", + "session_stop", + "fallback", + "placeholder_failure", + "cancel_return", + "cancel_stop", + "move_failure", + "fallback_same_host", + "rollback_failure", + ] as const)("%s preserves a host containing only a cross-window borrowed tab", async (method) => { + const f = fixture(); + const controller = new AbortController(); + const ctx = await f.manager.start("a", { inWindow: true }); + f.pages.get(1)!.windowId = 11; + f.pages.set(2, { id: 2, windowId: 11 } as chrome.tabs.Tab); + const move = vi.fn(async (id: number, props: chrome.tabs.MoveProperties) => { + const tab = f.pages.get(id)!; + tab.windowId = props.windowId!; + tab.index = props.index; + return tab; + }); + const deps = { + tabs: { + get: f.get, + create: vi.fn(async (props: chrome.tabs.CreateProperties) => + f.get(await f.create(props.windowId!, props.active ?? true)), + ), + remove: f.remove, + move, + update: vi.fn(async (id: number) => f.get(id)), + }, + tabsQuery: { query: (props: chrome.tabs.QueryInfo) => f.queryHost(props.windowId!) }, + windows: { + get: vi.fn(async () => { + if (method === "fallback" || method === "fallback_same_host") + throw new Error("No window with id: 11"); + return { id: 11 } as chrome.windows.Window; + }), + getLastFocused: vi.fn( + async () => + ({ + id: method === "fallback_same_host" ? 10 : 12, + type: "normal", + }) as chrome.windows.Window, + ), + create: vi.fn(), + remove: vi.fn(), + }, + approveBorrow: vi.fn(async () => true), + agentOverlayReset: { resetAgentOverlays: vi.fn(async () => {}) }, + signal: controller.signal, + }; + expect(await handleTabBorrow(f.manager, { session_id: "a", tab_id: 1 }, deps)).toMatchObject({ + original_window_id: 11, + }); + f.pages.delete(20); + f.manager.forgetClosedTab(20); + const send = vi.fn(); + const handler = attachSessionEventHandler({ + manager: f.manager, + transport: { send } as never, + windowEvents: { addListener: vi.fn(), removeListener: vi.fn() }, + }); + try { + if (method === "placeholder_failure") + f.create.mockRejectedValueOnce(new Error("create denied")); + if (method === "cancel_return" || method === "cancel_stop" || method === "rollback_failure") { + const create = deps.tabs.create.getMockImplementation()!; + deps.tabs.create.mockImplementationOnce(async (props) => { + const tab = await create(props); + controller.abort(); + return tab; + }); + } + if (method === "move_failure") move.mockRejectedValue(new Error("move denied")); + if (method === "rollback_failure") + f.remove.mockRejectedValueOnce(new Error("rollback denied")); + const result = + method === "session_stop" || method === "cancel_stop" + ? await handleSessionStop( + f.manager, + { session_id: "a" }, + { tabManagement: deps, signal: controller.signal }, + ) + : await handleTabReturn(f.manager, { session_id: "a", tab_id: 1 }, deps); + if (method === "placeholder_failure") { + expect(result).toMatchObject({ + code: "cdp_failed", + message: expect.stringContaining("create denied"), + }); + expect(move).toHaveBeenCalledOnce(); + expect(f.pages.get(1)?.windowId).toBe(10); + expect(ctx.borrowedTabs.has(1)).toBe(true); + expect(f.manager.has("a")).toBe(true); + expect(ctx.stopping).toBeFalsy(); + return; + } + if ( + method === "cancel_return" || + method === "cancel_stop" || + method === "move_failure" || + method === "rollback_failure" + ) { + expect(result).toMatchObject({ + code: + method === "move_failure" + ? "cdp_failed" + : method === "rollback_failure" + ? "protocol_error" + : "cancelled", + }); + expect( + [...f.pages.values()].filter((tab) => tab.windowId === 10).map((tab) => tab.id), + ).toEqual(method === "rollback_failure" ? [1, 21] : [1]); + expect(ctx.borrowedTabs.has(1)).toBe(true); + expect(f.manager.has("a")).toBe(true); + expect(ctx.stopping).toBeFalsy(); + if (method === "rollback_failure") + expect(result).toMatchObject({ + data: { reason: "cleanup_failed", resource_type: "tab", resource_id: 21 }, + }); + else expect(f.remove).toHaveBeenCalledWith(21); + return; + } + expect(result).not.toHaveProperty("code"); + if (method === "fallback_same_host") { + expect(f.pages.get(1)?.windowId).toBe(10); + expect(f.create).toHaveBeenCalledOnce(); + await vi.waitFor(() => expect(f.manager.has("a")).toBe(false)); + return; + } + expect(f.pages.get(1)?.windowId).toBe(method === "fallback" ? 12 : 11); + expect([...f.pages.values()].filter((tab) => tab.windowId === 10)).toMatchObject([ + { id: 21 }, + ]); + expect(f.create).toHaveBeenCalledTimes(2); + expect(f.create).toHaveBeenLastCalledWith(10, false); + expect(f.manager.findByTabId(21)).toBeNull(); + await vi.waitFor(() => expect(f.manager.has("a")).toBe(false)); + expect(send).not.toHaveBeenCalledWith( + expect.objectContaining({ event: "session.window_closed" }), + ); + expect(ctx.borrowedTabs.size).toBe(0); + } finally { + handler.dispose(); + } + }); + it.each([ "stop", "stopAll", ] as const)("direct %s keeps retryable state on deletion failure", async (method) => { const f = fixture(); const ctx = await f.manager.start("a", { inWindow: true }); - f.remove.mockRejectedValue(new Error("delete denied")); + f.pages.delete(1); + f.remove.mockRejectedValueOnce(new Error("delete denied")); await expect(method === "stop" ? f.manager.stop("a") : f.manager.stopAll()).rejects.toThrow( "delete denied", ); expect(f.manager.get("a")).toBe(ctx); expect(ctx.agentCreatedTabs.has(20)).toBe(true); + expect([...f.pages.keys()]).toEqual([20]); + expect(f.remove).toHaveBeenCalledWith(21); expect(f.windows.remove).not.toHaveBeenCalled(); }); @@ -289,10 +477,10 @@ describe("shared user window sessions (#243)", () => { expect(f.windows.remove).not.toHaveBeenCalled(); }); - it("preserves cleanup state when a live-page lookup fails", async () => { + it("preserves cleanup state when the host query fails", async () => { const f = fixture(); const ctx = await f.manager.start("a", { inWindow: true }); - f.get.mockRejectedValue(new Error("query denied")); + f.queryHost.mockRejectedValue(new Error("query denied")); await expect(f.manager.stop("a")).rejects.toThrow("query denied"); expect(f.manager.get("a")).toBe(ctx); expect(ctx.agentCreatedTabs.has(20)).toBe(true); diff --git a/apps/extension/src/session-manager/event-handler.ts b/apps/extension/src/session-manager/event-handler.ts index 2f232ba3..cd63deda 100644 --- a/apps/extension/src/session-manager/event-handler.ts +++ b/apps/extension/src/session-manager/event-handler.ts @@ -50,10 +50,10 @@ export function attachSessionEventHandler(options: SessionEventHandlerOptions): const onRemoved = (windowId: number): void => { for (const ctx of manager.sessionsInWindow(windowId)) { - // Chrome also emits onRemoved when session.stop removes the last tab or - // the window itself. Capture the cause before asynchronous cleanup so the + // Removing or moving a window's final tab also emits onRemoved. + // Capture the stop state before asynchronous cleanup so the // normal stop response, rather than a user-close event, ends the audit. - const expectedClose = manager.isWindowCloseExpected(ctx); + const expectedClose = ctx.stopping || manager.isWindowCloseExpected(ctx); if (ctx.container.mode === "in_window") ctx.stopping = true; const returnFailures = Array.from(ctx.borrowedTabs.keys()).map((tabId) => ({ tab_id: tabId, diff --git a/apps/extension/src/session-manager/manager.ts b/apps/extension/src/session-manager/manager.ts index fd3cc615..bd15e0e9 100644 --- a/apps/extension/src/session-manager/manager.ts +++ b/apps/extension/src/session-manager/manager.ts @@ -1,6 +1,11 @@ import { AGENT_WINDOW_HOME, type AgentWindowApi, chromeAgentWindowApi } from "./agent-window"; import { RefStore } from "./ref-store"; -import { chromeSharedWindowApi, type SharedWindowApi } from "./shared-window"; +import { + chromeSharedWindowApi, + preserveHostIfEmptied, + removeUnusedSharedPlaceholder, + type SharedWindowApi, +} from "./shared-window"; export interface SessionContext { /** Remote connections retain dedicated windows, with explicit page ownership. */ @@ -170,13 +175,12 @@ export class SessionManager { /** Suppress close/empty callbacks while session.stop owns teardown. */ async withExpectedWindowClose(ctx: SessionContext, close: () => Promise): Promise { - const alreadyExpected = this.isWindowCloseExpected(ctx); this.expectedWindowClosures.add(ctx); try { return await close(); } finally { // Failed teardown must not hide a later user-initiated close. - if (!alreadyExpected) this.expectedWindowClosures.delete(ctx); + this.expectedWindowClosures.delete(ctx); } } @@ -488,29 +492,42 @@ export class SessionManager { if (ctx.borrowedTabs.size) throw new Error("Return borrowed tabs before stopping this session"); const hostWindowId = ctx.container.hostWindowId; + let placeholderTabId: number | undefined; ctx.stopping = true; try { - await this.withExpectedWindowClose(ctx, async () => { - for (const tabId of [...ctx.agentCreatedTabs]) { + const ownedTabIds = [...ctx.agentCreatedTabs]; + const preserved = await preserveHostIfEmptied( + this.sharedWindow, + hostWindowId, + ownedTabIds, + ); + placeholderTabId = preserved.placeholderTabId; + const hostTabIds = new Set(preserved.hostTabs.map((tab) => tab.id)); + for (const tabId of ownedTabIds) { + if (hostTabIds.has(tabId)) { try { - const tab = await this.sharedWindow.get(tabId); - // Moving a shared session page out is a user reclaim, including - // when onAttached has not yet reached the service worker. - if (tab.windowId === hostWindowId) { - // Chrome closes a window when its final tab is removed. - // Leave an unowned blank page to preserve the user's host. - if ((await this.sharedWindow.query(hostWindowId)).length === 1) - await this.sharedWindow.create(hostWindowId, false); - await this.sharedWindow.remove(tabId); - } + await this.sharedWindow.remove(tabId); } catch (err) { if (!/No tab with id|Invalid tab ID|not found/i.test(String(err))) throw err; } - ctx.agentCreatedTabs.delete(tabId); } - }); + ctx.agentCreatedTabs.delete(tabId); + } } catch (err) { ctx.stopping = false; + if (placeholderTabId !== undefined) { + try { + await removeUnusedSharedPlaceholder( + this.sharedWindow, + hostWindowId, + placeholderTabId, + ); + } catch (cleanupErr) { + throw new Error( + `Session cleanup failed (${String(err)}); rollback of placeholder tab ${placeholderTabId} failed: ${String(cleanupErr)}`, + ); + } + } throw err; } } diff --git a/apps/extension/src/session-manager/shared-window.ts b/apps/extension/src/session-manager/shared-window.ts index f1ae4d90..aba6a360 100644 --- a/apps/extension/src/session-manager/shared-window.ts +++ b/apps/extension/src/session-manager/shared-window.ts @@ -8,6 +8,33 @@ export interface SharedWindowApi { focus?(windowId: number): Promise; } +/** Chrome closes the host when its final tab is removed or moved out. */ +export async function preserveHostIfEmptied( + api: Pick, + windowId: number, + leavingTabIds: readonly number[], +): Promise<{ hostTabs: chrome.tabs.Tab[]; placeholderTabId: number | undefined }> { + const hostTabs = await api.query(windowId); + const leaving = new Set(leavingTabIds); + const placeholderTabId = + hostTabs.length > 0 && hostTabs.every((tab) => leaving.has(tab.id!)) + ? await api.create(windowId, false) + : undefined; + return { hostTabs, placeholderTabId }; +} + +/** Undo only this operation's blank survivor, without emptying its host. */ +export async function removeUnusedSharedPlaceholder( + api: Pick, + windowId: number, + tabId: number, +): Promise { + const tabs = await api.query(windowId); + const placeholder = tabs.find((tab) => tab.id === tabId); + if (tabs.length > 1 && (placeholder?.pendingUrl ?? placeholder?.url) === "about:blank") + await api.remove(tabId); +} + export const chromeSharedWindowApi: SharedWindowApi = { host: () => chrome.windows.getLastFocused({ windowTypes: ["normal"] }), async create(windowId, focused) { diff --git a/apps/extension/src/session-manager/task-popups.ts b/apps/extension/src/session-manager/task-popups.ts index 9070d290..3f2bdc0f 100644 --- a/apps/extension/src/session-manager/task-popups.ts +++ b/apps/extension/src/session-manager/task-popups.ts @@ -28,6 +28,7 @@ export async function withTaskPopups( active && !signal?.aborted && manager.get(task.sessionId) === task && + !task.stopping && !manager.isWindowCloseExpected(task); const validSource = async (id: number) => { if ( diff --git a/apps/extension/src/tools/session.ts b/apps/extension/src/tools/session.ts index 874d8899..c51956d2 100644 --- a/apps/extension/src/tools/session.ts +++ b/apps/extension/src/tools/session.ts @@ -432,7 +432,7 @@ async function stopSession( return result; }; - // Shared sessions already mark the full stop transaction at the entry point. + // Shared sessions use ctx.stopping for the full stop transaction. return ctx.container.mode === "in_window" ? teardown() : manager.withExpectedWindowClose(ctx, teardown); @@ -448,11 +448,9 @@ export async function handleSessionStop( if (ctx.stopping || ctx.pendingOperations) return { code: "cancelled", message: "Session has pending operations; retry stop" }; ctx.stopping = true; - return manager.withExpectedWindowClose(ctx, async () => { - try { - return await handleSessionStopCore(manager, params, deps); - } finally { - if (manager.get(ctx.sessionId) === ctx) ctx.stopping = false; - } - }); + try { + return await handleSessionStopCore(manager, params, deps); + } finally { + if (manager.get(ctx.sessionId) === ctx) ctx.stopping = false; + } } diff --git a/apps/extension/src/tools/tabs.ts b/apps/extension/src/tools/tabs.ts index 0c90be6b..7457ad10 100644 --- a/apps/extension/src/tools/tabs.ts +++ b/apps/extension/src/tools/tabs.ts @@ -1,4 +1,8 @@ import { sessionWindowId } from "@/session-manager/manager"; +import { + preserveHostIfEmptied, + removeUnusedSharedPlaceholder, +} from "@/session-manager/shared-window"; import { withUiTeardown } from "@/session-manager/ui-activity"; // Tab-tool handlers. M6 wired `tool.tab_list`; M8 adds the rest of // the tab namespace: `tab_create`, `tab_close`, `tab_select`, @@ -294,6 +298,7 @@ export async function handleTabList( export interface TabManagementDeps { tabs?: TabMutationApi; + tabsQuery?: ChromeTabsApi; windows?: ChromeWindowsApi; /** Abort hook (M10 will wire the full chain). */ signal?: AbortSignal; @@ -1226,7 +1231,12 @@ export async function returnBorrowedTab( tabId: number, deps: TabManagementDeps, ): Promise { - return withUiTeardown(ctx, tabId, () => returnBorrowedTabCore(ctx, tabId, deps)); + try { + return await withUiTeardown(ctx, tabId, () => returnBorrowedTabCore(ctx, tabId, deps)); + } catch (err) { + if (isRpcError(err)) return err; + throw err; + } } async function returnBorrowedTabCore( @@ -1264,112 +1274,167 @@ async function returnBorrowedTabCore( let targetIndex = entry.originalIndex; let fallback = false; let fallbackTarget: FallbackWindowTarget | null = null; + let placeholderTabId: number | undefined; + let movedOut = false; + const hostWindowId = sessionWindowId(ctx); - // Check the original window is still around. - let originalAlive = true; try { - await windowsApi.get(entry.originalWindowId); - } catch (err) { - console.debug("[bsk tab_return] original window gone, falling back", err); - originalAlive = false; - } - const cancelledAfterLookup = aborted(deps.signal, "tab_return"); - if (cancelledAfterLookup) return cancelledAfterLookup; - if (!originalAlive) { - fallback = true; - const target = await chooseFallbackWindow(ctx, windowsApi, isAgentWindowId, deps.signal); - if ("code" in target) return target; - fallbackTarget = target; - targetWindowId = target.windowId; - targetIndex = target.index; - } - - const cancelledBeforeMove = aborted(deps.signal, "tab_return"); - if (cancelledBeforeMove) { - if (fallbackTarget) { - const cleanupError = await cleanupUnusedFallbackWindow( - windowsApi, - fallbackTarget, - "tab_return aborted before moving the borrowed tab", - ); - if (cleanupError) return cleanupError; - } - return cancelledBeforeMove; - } - - try { - const moved = await tabsApi.move(tabId, { - windowId: targetWindowId, - index: targetIndex, - }); - const movedTab = Array.isArray(moved) ? moved[0] : moved; - const finalIndex = typeof movedTab?.index === "number" ? movedTab.index : targetIndex; - await releaseReturnedTabState(ctx, tabId, deps); - return { - tabId, - toWindowId: targetWindowId, - toIndex: finalIndex, - fallback, - }; - } catch (err) { - if (fallbackTarget) { - const cleanupError = await cleanupUnusedFallbackWindow( - windowsApi, - fallbackTarget, - `tab_return could not move tab ${tabId}`, - ); - if (cleanupError) return cleanupError; + // Check the original window is still around. + let originalAlive = true; + try { + await windowsApi.get(entry.originalWindowId); + } catch (err) { + console.debug("[bsk tab_return] original window gone, falling back", err); + originalAlive = false; } - const cancelledAfterMoveFailure = aborted(deps.signal, "tab_return"); - if (cancelledAfterMoveFailure) return cancelledAfterMoveFailure; - if (!fallback) { + const cancelledAfterLookup = aborted(deps.signal, "tab_return"); + if (cancelledAfterLookup) return cancelledAfterLookup; + if (!originalAlive) { + fallback = true; const target = await chooseFallbackWindow(ctx, windowsApi, isAgentWindowId, deps.signal); - if ("code" in target) { + if ("code" in target) return target; + fallbackTarget = target; + targetWindowId = target.windowId; + targetIndex = target.index; + } + + if (ctx.container.mode === "in_window" && targetWindowId !== hostWindowId) { + try { + const preserved = await preserveHostIfEmptied( + { + query: (windowId) => (deps.tabsQuery ?? chromeTabsApi).query({ windowId }), + create: async (windowId, active) => + (await tabsApi.create({ windowId, url: "about:blank", active })).id!, + }, + hostWindowId, + [tabId], + ); + placeholderTabId = preserved.placeholderTabId; + } catch (err) { + if (fallbackTarget) { + const cleanupError = await cleanupUnusedFallbackWindow( + windowsApi, + fallbackTarget, + "tab_return could not preserve the host window", + ); + if (cleanupError) return cleanupError; + } return { code: "cdp_failed", - message: `tab_return: chrome.tabs.move failed: ${describeError(err)}; fallback failed: ${target.message}`, + message: `tab_return: could not preserve the host window: ${describeError(err)}`, }; } - const cancelledBeforeFallbackMove = aborted(deps.signal, "tab_return"); - if (cancelledBeforeFallbackMove) { + } + + const cancelledBeforeMove = aborted(deps.signal, "tab_return"); + if (cancelledBeforeMove) { + if (fallbackTarget) { const cleanupError = await cleanupUnusedFallbackWindow( windowsApi, - target, - "tab_return aborted before fallback move", + fallbackTarget, + "tab_return aborted before moving the borrowed tab", ); - return cleanupError ?? cancelledBeforeFallbackMove; + if (cleanupError) return cleanupError; } - try { - const moved = await tabsApi.move(tabId, { - windowId: target.windowId, - index: target.index, - }); - const movedTab = Array.isArray(moved) ? moved[0] : moved; - const finalIndex = typeof movedTab?.index === "number" ? movedTab.index : target.index; - await releaseReturnedTabState(ctx, tabId, deps); - return { - tabId, - toWindowId: target.windowId, - toIndex: finalIndex, - fallback: true, - }; - } catch (fallbackErr) { + return cancelledBeforeMove; + } + + try { + const moved = await tabsApi.move(tabId, { + windowId: targetWindowId, + index: targetIndex, + }); + const movedTab = Array.isArray(moved) ? moved[0] : moved; + const finalIndex = typeof movedTab?.index === "number" ? movedTab.index : targetIndex; + movedOut = targetWindowId !== hostWindowId; + await releaseReturnedTabState(ctx, tabId, deps); + return { + tabId, + toWindowId: targetWindowId, + toIndex: finalIndex, + fallback, + }; + } catch (err) { + if (fallbackTarget) { const cleanupError = await cleanupUnusedFallbackWindow( windowsApi, - target, - `tab_return fallback move for tab ${tabId} failed`, + fallbackTarget, + `tab_return could not move tab ${tabId}`, ); if (cleanupError) return cleanupError; - return { - code: "cdp_failed", - message: `tab_return: chrome.tabs.move failed: ${describeError(err)}; fallback move failed: ${describeError(fallbackErr)}`, - }; + } + const cancelledAfterMoveFailure = aborted(deps.signal, "tab_return"); + if (cancelledAfterMoveFailure) return cancelledAfterMoveFailure; + if (!fallback) { + const target = await chooseFallbackWindow(ctx, windowsApi, isAgentWindowId, deps.signal); + if ("code" in target) { + return { + code: "cdp_failed", + message: `tab_return: chrome.tabs.move failed: ${describeError(err)}; fallback failed: ${target.message}`, + }; + } + const cancelledBeforeFallbackMove = aborted(deps.signal, "tab_return"); + if (cancelledBeforeFallbackMove) { + const cleanupError = await cleanupUnusedFallbackWindow( + windowsApi, + target, + "tab_return aborted before fallback move", + ); + return cleanupError ?? cancelledBeforeFallbackMove; + } + try { + const moved = await tabsApi.move(tabId, { + windowId: target.windowId, + index: target.index, + }); + const movedTab = Array.isArray(moved) ? moved[0] : moved; + const finalIndex = typeof movedTab?.index === "number" ? movedTab.index : target.index; + movedOut = target.windowId !== hostWindowId; + await releaseReturnedTabState(ctx, tabId, deps); + return { + tabId, + toWindowId: target.windowId, + toIndex: finalIndex, + fallback: true, + }; + } catch (fallbackErr) { + const cleanupError = await cleanupUnusedFallbackWindow( + windowsApi, + target, + `tab_return fallback move for tab ${tabId} failed`, + ); + if (cleanupError) return cleanupError; + return { + code: "cdp_failed", + message: `tab_return: chrome.tabs.move failed: ${describeError(err)}; fallback move failed: ${describeError(fallbackErr)}`, + }; + } + } + return { + code: "cdp_failed", + message: `tab_return: chrome.tabs.move failed: ${describeError(err)}`, + }; + } + } finally { + if (placeholderTabId !== undefined && !movedOut) { + try { + await removeUnusedSharedPlaceholder( + { + query: (windowId) => (deps.tabsQuery ?? chromeTabsApi).query({ windowId }), + remove: (id) => tabsApi.remove(id), + }, + hostWindowId, + placeholderTabId, + ); + } catch (cleanupErr) { + throw rpcError( + "protocol_error", + "cleanup_failed", + `tab_return: rollback of placeholder tab ${placeholderTabId} failed: ${describeError(cleanupErr)}`, + { resource_type: "tab", resource_id: placeholderTabId }, + ); } } - return { - code: "cdp_failed", - message: `tab_return: chrome.tabs.move failed: ${describeError(err)}`, - }; } }