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 1303b4bd..3afb98d7 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 ae9f62d5..249d491a 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -36,7 +36,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, @@ -89,8 +89,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()); @@ -141,25 +140,7 @@ export default defineBackground(() => { if (controlModes.get(sessionId) === mode) return; controlModes.set(sessionId, mode); 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", - ...nextOverlayVersion(), - }; - } - return { - type: OVERLAY_AGENT_STATE, - sessionId: ctx.sessionId, - mode: controlModes.get(ctx.sessionId) ?? "control", - ...nextOverlayVersion(), - }; + if (ctx) void pushOverlayStateForWindow(sessionWindowId(ctx)); } /** @@ -169,8 +150,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", + ...nextOverlayVersion(), + }; } return { type: OVERLAY_AGENT_STATE, @@ -207,7 +194,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); } @@ -231,10 +218,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) => { @@ -247,7 +236,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) => { @@ -264,6 +253,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). @@ -343,6 +344,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(), @@ -460,8 +462,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..5737fdce --- /dev/null +++ b/apps/extension/src/session-manager/__tests__/shared-window.test.ts @@ -0,0 +1,490 @@ +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"; +import { chromeSharedWindowApi } from "../shared-window"; + +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 (_excluded: ReadonlySet, preferredWindowId?: number) => + ({ id: preferredWindowId ?? 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("skips the focused Agent Window when another normal user window exists", async () => { + const agent = { id: 99, type: "normal", incognito: false } as chrome.windows.Window; + const user = { id: 10, type: "normal", incognito: false } as chrome.windows.Window; + const getLastFocused = vi.fn(async () => agent); + const getAll = vi.fn(async () => [agent, user]); + const getWindow = vi.fn(async () => user); + vi.stubGlobal("chrome", { windows: { getLastFocused, getAll, get: getWindow } }); + try { + expect(await chromeSharedWindowApi.host(new Set([99]))).toBe(user); + expect(getAll).toHaveBeenCalledWith({ windowTypes: ["normal"] }); + expect(await chromeSharedWindowApi.host(new Set([99]), 10)).toBe(user); + expect(getWindow).toHaveBeenCalledWith(10); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("starts on an approved existing tab and releases it without creating or closing a tab", async () => { + const f = fixture(); + const approveBorrow = vi.fn(async () => true); + expect( + await handleSessionStart( + f.manager, + { session_id: "a", in_window: true, tab_id: 1 }, + { approveBorrow }, + ), + ).toMatchObject({ claimed_tab_id: 1, agent_window_id: 10 }); + const ctx = f.manager.get("a")!; + expect(ctx.activeTabId).toBe(1); + expect(ctx.borrowedTabs.has(1)).toBe(true); + expect(ctx.agentCreatedTabs.size).toBe(0); + expect(f.create).not.toHaveBeenCalled(); + expect(f.windows.create).not.toHaveBeenCalled(); + expect(approveBorrow).toHaveBeenCalledWith({ sessionId: "a", tabId: 1 }); + const move = vi.fn(); + expect( + await handleSessionStop( + f.manager, + { session_id: "a" }, + { + cdp: { releaseSessionTab: vi.fn(async () => {}), detachSession: vi.fn(async () => {}) }, + tabManagement: { + tabs: { get: f.get, create: vi.fn(), remove: f.remove, move, update: vi.fn() }, + }, + }, + ), + ).toMatchObject({ returned_tab_ids: [1] }); + expect(f.pages.has(1)).toBe(true); + expect(f.remove).not.toHaveBeenCalled(); + expect(move).not.toHaveBeenCalled(); + expect(f.windows.remove).not.toHaveBeenCalled(); + }); + + it("claims a selected user tab outside the last-focused window", async () => { + const f = fixture(); + f.pages.set(2, { + id: 2, + windowId: 11, + active: true, + index: 0, + url: "https://other.example/", + } as chrome.tabs.Tab); + expect( + await handleSessionStart( + f.manager, + { session_id: "other", in_window: true, tab_id: 2 }, + { approveBorrow: vi.fn(async () => true) }, + ), + ).toMatchObject({ claimed_tab_id: 2, agent_window_id: 11 }); + expect(f.host).toHaveBeenCalledWith(expect.any(Set), 11); + expect(f.create).not.toHaveBeenCalled(); + expect(f.windows.create).not.toHaveBeenCalled(); + }); + + it("does not claim or mutate an existing tab when borrow confirmation is denied", async () => { + const f = fixture(); + expect( + await handleSessionStart( + f.manager, + { session_id: "a", in_window: true, tab_id: 1 }, + { + approveBorrow: vi.fn(async () => false), + }, + ), + ).toMatchObject({ code: "cancelled" }); + expect(f.manager.has("a")).toBe(false); + expect(f.pages.has(1)).toBe(true); + expect(f.create).not.toHaveBeenCalled(); + expect(f.remove).not.toHaveBeenCalled(); + }); + + 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..6b5fc7af 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,15 @@ 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; + existingTabId?: number; + approveExistingTab?: (sessionId: string, tabId: number, signal?: AbortSignal) => Promise; /** 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 +104,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 +147,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 +170,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 +190,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 +259,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 +292,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 +325,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 +339,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 +370,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 +392,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 +407,107 @@ 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; + let reservation: BorrowReservation | undefined; + try { + const target = + opts.existingTabId !== undefined + ? await this.sharedWindow.get(opts.existingTabId) + : undefined; + const host = await this.sharedWindow.host(new Set(this.windowIndex.keys()), target?.windowId); + 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); + if (opts.existingTabId !== undefined) { + if (target?.windowId !== host.id) + throw new Error("Existing tab must be in the selected user window"); + const claim = this.tryReserveBorrow(opts.existingTabId, sessionId); + if ("borrowedBy" in claim) + throw new Error(`Tab is controlled by session ${claim.borrowedBy}`); + reservation = claim; + if ( + !opts.approveExistingTab || + !(await opts.approveExistingTab(sessionId, opts.existingTabId, opts.signal)) + ) + throw new DOMException("Tab borrow denied", "AbortError"); + throwIfSessionStartAborted(opts.signal); + const current = await this.sharedWindow.get(opts.existingTabId); + if (current.windowId !== host.id) throw new Error("Existing tab moved during startup"); + ctx = { + sessionId, + container: { mode: "in_window", hostWindowId: host.id }, + activeTabId: opts.existingTabId, + refStore: new RefStore(), + borrowedTabs: new Map(), + agentCreatedTabs: new Set(), + createdAtMs: this.now(), + }; + this.sessions.set(sessionId, ctx); + reservation.commit({ + tabId: opts.existingTabId, + originalWindowId: host.id, + originalIndex: typeof current.index === "number" ? current.index : 0, + }); + return ctx; + } + 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 (opts.existingTabId !== undefined) this.sessions.delete(sessionId); + 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 { + reservation?.release(); + 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 +520,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..39407d74 --- /dev/null +++ b/apps/extension/src/session-manager/shared-window.ts @@ -0,0 +1,36 @@ +/** A shared session owns tabs, never its host window. */ +export interface SharedWindowApi { + host( + excludedWindowIds: ReadonlySet, + preferredWindowId?: number, + ): Promise; + create(windowId: number, focused: boolean): Promise; + get(tabId: number): Promise; + remove(tabId: number): Promise; + focus?(windowId: number): Promise; +} + +export const chromeSharedWindowApi: SharedWindowApi = { + async host(excludedWindowIds, preferredWindowId) { + if (preferredWindowId !== undefined) return chrome.windows.get(preferredWindowId); + const last = await chrome.windows.getLastFocused({ windowTypes: ["normal"] }); + if (last.id !== undefined && !last.incognito && !excludedWindowIds.has(last.id)) return last; + const windows = await chrome.windows.getAll({ windowTypes: ["normal"] }); + return ( + windows.find( + (window) => + window.id !== undefined && !window.incognito && !excludedWindowIds.has(window.id), + ) ?? last + ); + }, + 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/dispatcher.ts b/apps/extension/src/tools/dispatcher.ts index 0a658904..725235bf 100644 --- a/apps/extension/src/tools/dispatcher.ts +++ b/apps/extension/src/tools/dispatcher.ts @@ -444,6 +444,7 @@ export class ToolDispatcher { return handleSessionStart(this.sessions, req.params as SessionStartParams, { signal, preferences: this.interactionPreferences, + approveBorrow: this.approveBorrow, }); case "tool.session_stop": { this.debug?.releaseSession((req.params as SessionStopParams).session_id); 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..37554d25 100644 --- a/apps/extension/src/tools/session.ts +++ b/apps/extension/src/tools/session.ts @@ -4,13 +4,23 @@ 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"; import type { CdpRunner, ChromeTabsApi } from "./shared"; import { isRpcError } from "./shared"; -import { chromeAgentOverlayResetApi, returnBorrowedTab, type TabManagementDeps } from "./tabs"; +import { + chromeAgentOverlayResetApi, + returnBorrowedTab, + type BorrowConfirmationApprover, + type TabManagementDeps, +} from "./tabs"; /** Valid range for Agent Window dimensions in CSS pixels. */ export const WINDOW_SIZE_MIN = 100; @@ -52,6 +62,8 @@ export function validateWindowSize( } export interface SessionStartParams { + in_window?: boolean; + tab_id?: number; session_id: string; browser_instance_id?: string; /** Optional Agent Window outer width in CSS pixels (100..=7680). */ @@ -65,13 +77,22 @@ export interface SessionStartParams { } export interface SessionStartResult { + container_mode?: "window" | "in_window"; interaction?: InteractionPolicy; agent_window_id?: number; + claimed_tab_id?: number; } export interface SessionStartDeps { preferences?: InteractionPreferenceStore; signal?: AbortSignal; + approveBorrow?: BorrowConfirmationApprover; +} + +class ExistingTabApprovalError extends Error { + constructor(readonly response: RpcError) { + super(response.message); + } } export interface SessionStopParams { @@ -132,6 +153,27 @@ 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.tab_id !== undefined && + (!params.in_window || !Number.isSafeInteger(params.tab_id) || params.tab_id <= 0) + ) { + return { + code: "invalid_params", + message: "tab_id requires in_window and must be a positive integer", + }; + } + 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 +182,32 @@ export async function handleSessionStart( size: sizeOrErr, focused: params.focused, signal: deps.signal, + inWindow: params.in_window, + existingTabId: params.tab_id, + approveExistingTab: async (sessionId, tabId, signal) => { + const result = await deps.approveBorrow?.({ + sessionId, + tabId, + ...(signal ? { signal } : {}), + }); + if (result && typeof result === "object") throw new ExistingTabApprovalError(result); + return result === true; + }, }); return { - agent_window_id: ctx.agentWindowId, + agent_window_id: sessionWindowId(ctx), + ...(params.tab_id !== undefined ? { claimed_tab_id: params.tab_id } : {}), + ...(ctx.container.mode === "in_window" ? { container_mode: ctx.container.mode } : {}), interaction: interactionPolicy(deps.preferences?.get() ?? DEFAULT_INTERACTION_PREFERENCES), }; } catch (err) { + if (err instanceof ExistingTabApprovalError) return err.response; + 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 +255,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 +284,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 +353,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 +371,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 +407,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 +429,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 +466,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 5d30fa2b..0c90be6b 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`, @@ -251,11 +252,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" }; @@ -264,7 +265,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({ @@ -364,7 +373,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, }; @@ -372,12 +381,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, @@ -394,13 +424,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. @@ -421,9 +472,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, @@ -444,60 +495,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(); } // --------------------------------------------------------------------------- @@ -505,11 +572,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. @@ -533,10 +597,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); @@ -547,13 +614,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)}`, ); } @@ -594,19 +661,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(); } // --------------------------------------------------------------------------- @@ -639,6 +709,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", @@ -647,7 +718,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 }; } @@ -708,17 +779,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", @@ -834,20 +909,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 = {}, @@ -880,7 +958,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 ) { @@ -888,7 +966,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 { @@ -896,7 +974,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); @@ -937,12 +1015,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", @@ -988,7 +1068,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(); @@ -999,6 +1079,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; @@ -1035,7 +1125,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) { @@ -1153,6 +1247,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; @@ -1266,7 +1373,7 @@ async function returnBorrowedTabCore( } } -export async function handleTabReturn( +async function handleTabReturnCore( manager: SessionManager, params: TabReturnParams, deps: TabManagementDeps = {}, @@ -1293,6 +1400,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, @@ -1301,3 +1409,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..af827b70 100644 --- a/crates/bsk-cli/src/cli/session.rs +++ b/crates/bsk-cli/src/cli/session.rs @@ -52,12 +52,21 @@ pub enum SessionSub { #[derive(Debug, Clone, Args)] pub struct SessionStartArgs { + /// Create session tabs in a normal user window, skipping Agent Windows (local only). + #[arg(long, conflicts_with_all = ["width", "height"])] + pub in_window: bool, + /// Reuse an existing tab in its user window (requires --in-window). + #[arg(long, requires = "in_window")] + pub tab_id: Option, /// Deprecated compatibility flag. Automation settings in the extension take precedence. #[arg(long)] pub unattended: bool, /// Recoverable request token: :. Valid for at most ten minutes. #[arg(long)] pub request_id: Option, + /// Stop a managed session when its owner stops renewing its lease. + #[arg(long, requires = "request_id")] + pub ephemeral: bool, /// Optional task name displayed in local operation history. #[arg(long)] pub name: Option, @@ -76,7 +85,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, } @@ -115,10 +124,18 @@ pub struct SessionRequestArgs { pub prepare: bool, #[arg(long)] pub claim: bool, + #[arg(long, conflicts_with_all = ["claim", "prepare", "cancel"])] + pub renew: bool, } #[derive(Debug, Serialize)] struct StartParams { + #[serde(skip_serializing_if = "std::ops::Not::not")] + in_window: bool, + #[serde(skip_serializing_if = "Option::is_none")] + tab_id: Option, + #[serde(skip_serializing_if = "std::ops::Not::not")] + ephemeral: bool, #[serde(skip_serializing_if = "Option::is_none")] request_id: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -135,6 +152,8 @@ struct StartParams { #[derive(Debug, Deserialize)] pub struct StartReply { + #[serde(default)] + pub container_mode: Option, #[serde(default)] pub interaction: Option, pub session_id: String, @@ -193,6 +212,8 @@ pub fn dispatch(cmd: SessionCmd, format: Format) -> Result<(), CliError> { "cancel" } else if args.claim { "claim" + } else if args.renew { + "renew" } else { "status" }; @@ -239,6 +260,9 @@ fn run_start(sock: PathBuf, args: SessionStartArgs, format: Format) -> Result<() let result = start_session( sock, SessionStartOptions { + in_window: args.in_window, + tab_id: args.tab_id, + ephemeral: args.ephemeral, name: args.name, request_id: args.request_id, browser: args.browser, @@ -257,6 +281,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 +302,9 @@ 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 tab_id: Option, + pub ephemeral: bool, pub name: Option, pub browser: Option, pub width: Option, @@ -286,6 +314,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 +342,9 @@ 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 / OWNER / STATE / LEASE EXPIRY", + ); let session_w = reply .sessions .iter() @@ -567,8 +622,15 @@ fn run_list(sock: PathBuf, format: Format) -> Result<(), CliError> { .map(|w| w.to_string()) .unwrap_or_else(|| "-".into()); println!( - "{: Result, + #[serde(default)] + pub ephemeral: bool, #[serde(default)] pub browser_instance_id: Option, #[serde(default)] @@ -887,6 +893,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 +1001,9 @@ pub(super) async fn handle_session_start( let cancel = abort_guard.token().clone(); let params: CliSessionStartParams = if params.is_null() { CliSessionStartParams { + in_window: false, + tab_id: None, + ephemeral: false, browser_instance_id: None, width: None, height: None, @@ -1005,6 +1016,13 @@ pub(super) async fn handle_session_start( data: None, })? }; + if params.ephemeral && !recoverable { + return Err(RpcError { + code: ErrorCode::InvalidParams, + message: "ephemeral sessions require a managed request_id".into(), + data: None, + }); + } // `--width` without `--height` (or vice versa) is rejected: the // extension only accepts a complete size pair. let window_size = match (params.width, params.height) { @@ -1024,6 +1042,8 @@ pub(super) async fn handle_session_start( &state.tool_queues, params.browser_instance_id.as_deref(), AgentWindowOptions { + in_window: params.in_window, + tab_id: params.tab_id, size: window_size, focused: params.focused, }, @@ -1039,6 +1059,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(), @@ -1047,7 +1068,7 @@ pub(super) async fn handle_session_start( Ok(serde_json::to_value(result).unwrap_or(Value::Null)) } Err(err) => { - if recoverable && let StartSessionError::CleanupFailed { session_id, .. } = &err { + if let StartSessionError::CleanupFailed { session_id, .. } = &err { state.tool_queues.spawn(session_id.clone()); } Err(map_start_error(err)) @@ -1301,16 +1322,29 @@ async fn handle_session_stop_all( Ok(serde_json::to_value(result).unwrap_or(Value::Null)) } -fn handle_session_list(state: &Arc) -> ResponseBody { +pub(super) fn handle_session_list(state: &Arc) -> ResponseBody { let sessions: Vec<_> = state .sessions .snapshot() .into_iter() - .map(|s| s.status_entry()) + .map(|s| session_status_entry(state, s)) .collect(); ResponseBody::Ok(serde_json::to_value(SessionListResult { sessions }).unwrap_or(Value::Null)) } +fn session_status_entry( + state: &DaemonState, + session: super::sessions::Session, +) -> SessionStatusEntry { + let (owner_kind, lifecycle_state, lease_expires_at_ms) = + state.session_requests.describe_session(&session); + let mut entry = session.status_entry(); + entry.owner_kind = Some(owner_kind); + entry.lifecycle_state = Some(lifecycle_state); + entry.lease_expires_at_ms = lease_expires_at_ms; + entry +} + async fn handle_browser_list(state: &Arc, params: Value) -> Result { let params: BrowserListParams = parse_params_or_default(params)?; maybe_wait_for_browser(state, params.wait_for_browser_ms).await; diff --git a/crates/bsk-cli/src/daemon/session_requests.rs b/crates/bsk-cli/src/daemon/session_requests.rs index 56380349..947c556f 100644 --- a/crates/bsk-cli/src/daemon/session_requests.rs +++ b/crates/bsk-cli/src/daemon/session_requests.rs @@ -20,10 +20,43 @@ use super::state::DaemonState; const MAX_ADMISSION_MS: u64 = 10 * 60 * 1000; const MAX_REQUESTS: usize = 8192; +const OWNER_LEASE_MS: u64 = 45_000; #[derive(Debug, Default)] pub struct SessionRequests(Mutex>>); +impl SessionRequests { + pub(super) fn describe_session(&self, session: &Session) -> (String, String, Option) { + let requests: Vec<_> = self.0.lock().unwrap().values().cloned().collect(); + for request in requests { + let data = request.data.lock().unwrap(); + if data.session.as_ref().is_some_and(|owned| { + owned.id == session.id + && owned.browser_id == session.browser_id + && owned.created_at_ms == session.created_at_ms + && owned.agent_window_id == session.agent_window_id + }) { + let kind = if data.ephemeral { + "ephemeral" + } else { + "managed" + }; + let state = if data.cleanup_error.is_some() { + "cleanup_failed" + } else if data.cancelled { + "cancelling" + } else if data.claimed { + "active" + } else { + "starting" + }; + return (kind.into(), state.into(), data.lease_expires_at_ms); + } + } + ("unmanaged".into(), "active".into(), None) + } +} + #[derive(Debug)] struct StartRequest { id: String, @@ -40,6 +73,8 @@ struct StartRequest { struct RequestData { cancelled: bool, claimed: bool, + ephemeral: bool, + lease_expires_at_ms: Option, closed: bool, session: Option, result: Option, @@ -121,6 +156,7 @@ impl StartRequest { "request_id": self.id, "state": phase, "session": data.session.as_ref().map(Session::status_entry), "cleanup_error": data.cleanup_error, + "lease_expires_at_ms": data.lease_expires_at_ms, }) } } @@ -159,6 +195,7 @@ pub(super) async fn start( ); } params.as_object_mut().unwrap().remove("request_id"); + let ephemeral = params.get("ephemeral").and_then(Value::as_bool) == Some(true); let (entry, fresh) = { let requests = state.session_requests.0.lock().unwrap(); let Some(entry) = requests.get(&id) else { @@ -172,6 +209,7 @@ pub(super) async fn start( let mut original = entry.params.lock().unwrap(); if fresh { *original = params.clone(); + entry.data.lock().unwrap().ephemeral = ephemeral; } else if !cancelled && *original != params { return error( ErrorCode::InvalidParams, @@ -253,10 +291,10 @@ pub(super) async fn operate(state: &Arc, params: Value) -> Response .get("action") .and_then(Value::as_str) .unwrap_or("status"); - if !matches!(action, "status" | "prepare" | "cancel" | "claim") { + if !matches!(action, "status" | "prepare" | "cancel" | "claim" | "renew") { return error( ErrorCode::InvalidParams, - "action must be status, prepare, cancel, or claim", + "action must be status, prepare, cancel, claim, or renew", ); } let entry = { @@ -285,7 +323,7 @@ pub(super) async fn operate(state: &Arc, params: Value) -> Response } }; let Some(entry) = entry else { - return if action == "claim" { + return if matches!(action, "claim" | "renew") { error(ErrorCode::NotFound, "start request not found") } else { ResponseBody::Ok( @@ -301,6 +339,9 @@ pub(super) async fn operate(state: &Arc, params: Value) -> Response if data.cancelled || data.closed || (entry.expires <= now_ms() && !data.claimed) + || data + .lease_expires_at_ms + .is_some_and(|deadline| deadline <= now_ms()) || !matches!(data.result, Some(ResponseBody::Ok(_))) || !data .session @@ -309,8 +350,32 @@ pub(super) async fn operate(state: &Arc, params: Value) -> Response { return error(ErrorCode::Cancelled, "start request cannot be claimed"); } + if data.ephemeral && !data.claimed { + data.lease_expires_at_ms = Some(now_ms().saturating_add(OWNER_LEASE_MS)); + } data.claimed = true; } + if action == "renew" { + let mut data = entry.data.lock().unwrap(); + if !data.claimed + || !data.ephemeral + || data.cancelled + || data.closed + || data + .lease_expires_at_ms + .is_none_or(|deadline| deadline <= now_ms()) + || !data + .session + .as_ref() + .is_some_and(|s| same_session(state, s)) + { + return error(ErrorCode::Cancelled, "session lease is not active"); + } + data.lease_expires_at_ms = Some(now_ms().saturating_add(OWNER_LEASE_MS)); + if let Some(session) = &data.session { + state.sessions.touch(&session.id); + } + } ResponseBody::Ok(entry.snapshot(state)) } @@ -380,11 +445,17 @@ pub(super) fn reap(state: &Arc) { for entry in entries { entry.snapshot(state); let (closed, needs_cleanup) = { - let data = entry.data.lock().unwrap(); - ( - data.closed, - !data.closed && (data.cancelled || (!data.claimed && entry.expires <= now)), - ) + let mut data = entry.data.lock().unwrap(); + let expired = (!data.claimed && entry.expires <= now) + || (data.ephemeral + && data.claimed + && data + .lease_expires_at_ms + .is_some_and(|deadline| deadline <= now)); + if expired && !data.closed { + data.cancelled = true; + } + (data.closed, !data.closed && data.cancelled) }; if closed && entry.expires <= now { state.session_requests.0.lock().unwrap().remove(&entry.id); @@ -506,6 +577,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), @@ -514,6 +586,61 @@ mod ownership_tests { } } + #[tokio::test] + async fn ephemeral_claim_is_renewable_and_expired_lease_enters_cleanup() { + let state = Arc::new(DaemonState::new(DaemonConfig::new(0))); + let session = session("live", 7); + state.sessions.insert(session.clone()); + let expires = now_ms() + 60_000; + let id = format!("{expires}:{}", uuid::Uuid::new_v4()); + let entry = Arc::new(StartRequest::new(id.clone(), expires, Value::Null, false)); + entry.started.store(true, Ordering::SeqCst); + entry.finished.send_replace(true); + { + let mut data = entry.data.lock().unwrap(); + data.ephemeral = true; + data.session = Some(session); + data.result = Some(ResponseBody::Ok(json!({}))); + } + state + .session_requests + .0 + .lock() + .unwrap() + .insert(id.clone(), entry.clone()); + let ResponseBody::Ok(claimed) = + operate(&state, json!({"request_id":id,"action":"claim"})).await + else { + panic!("claim failed") + }; + assert_eq!(claimed["state"], "active"); + let first_deadline = entry.data.lock().unwrap().lease_expires_at_ms.unwrap(); + let ResponseBody::Ok(renewed) = + operate(&state, json!({"request_id":id,"action":"renew"})).await + else { + panic!("renew failed") + }; + assert_eq!(renewed["state"], "active"); + let renewed_deadline = entry.data.lock().unwrap().lease_expires_at_ms.unwrap(); + assert!(renewed_deadline >= first_deadline); + let ResponseBody::Ok(listed) = super::super::ipc::handle_session_list(&state) else { + panic!("session list failed") + }; + let session = &listed["sessions"][0]; + assert_eq!(session["session_id"], "live"); + assert_eq!(session["owner_kind"], "ephemeral"); + assert_eq!(session["lifecycle_state"], "active"); + assert_eq!(session["lease_expires_at_ms"], renewed_deadline); + entry.data.lock().unwrap().lease_expires_at_ms = Some(now_ms() - 1); + reap(&state); + tokio::task::yield_now().await; + assert!(entry.data.lock().unwrap().cancelled); + assert!(matches!( + operate(&state, json!({"request_id":id,"action":"renew"})).await, + ResponseBody::Err(_) + )); + } + #[tokio::test] async fn lease_reaps_ready_requests_but_keeps_claimed_sessions() { let state = Arc::new(DaemonState::new(DaemonConfig::new(0))); diff --git a/crates/bsk-cli/src/daemon/sessions.rs b/crates/bsk-cli/src/daemon/sessions.rs index c4201026..9a72379a 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,11 +58,15 @@ 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(), agent_window_id: self.agent_window_id, created_at_ms: self.created_at_ms, + owner_kind: None, + lifecycle_state: None, + lease_expires_at_ms: None, } } } @@ -73,7 +78,7 @@ pub struct SessionRegistry { /// Operational metadata kept outside the public `Session` wire/domain /// shape so idle enforcement does not break external struct users. last_activity: Mutex>, - /// Managed creates whose original extension operation has not settled. + /// Creates whose original extension operation has not settled. starting: Mutex>, } @@ -153,6 +158,7 @@ impl SessionRegistry { guard.insert( candidate.clone(), Session { + container_mode: None, interaction: None, id: candidate.clone(), browser_id: browser_id.clone(), @@ -454,6 +460,8 @@ 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, + pub tab_id: Option, /// Optional outer size as `(width, height)` CSS pixels. pub size: Option<(u32, u32)>, /// Optional focus hint (`None` = extension default: focused). @@ -527,6 +535,29 @@ 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, + })); + } + if window.tab_id.is_some_and(|id| id <= 0) || (window.tab_id.is_some() && !window.in_window) { + return Err(StartSessionError::ExtensionError(RpcError { + code: ErrorCode::InvalidParams, + message: "tab_id requires in_window and must be positive".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 +565,8 @@ pub(crate) async fn start_session_recoverable( sessions.starting.lock().unwrap().insert(session_id.clone()); } let params = SessionStartParams { + in_window: window.in_window, + tab_id: window.tab_id, session_id: session_id.0.clone(), browser_instance_id: Some(client.id.0.clone()), width: window.size.map(|(width, _)| width), @@ -593,6 +626,7 @@ pub(crate) async fn start_session_recoverable( StartWaitOutcome::WaiterClosed => { client.pending.lock().unwrap().cancel(&rpc_id); if preserve_cleanup { + sessions.commit_reservation(&session_id, None); return Err(StartSessionError::CleanupFailed { session_id, agent_window_id: None, @@ -606,6 +640,7 @@ pub(crate) async fn start_session_recoverable( return Err(finish_aborted_start( &client, sessions, + queues, &session_id, &rpc_id, waiter, @@ -618,6 +653,7 @@ pub(crate) async fn start_session_recoverable( return Err(finish_aborted_start( &client, sessions, + queues, &session_id, &rpc_id, waiter, @@ -631,28 +667,18 @@ pub(crate) async fn start_session_recoverable( ResponseBody::Ok(v) => match serde_json::from_value::(v) { Ok(parsed) => parsed, Err(_) => { - if preserve_cleanup { - sessions.commit_reservation(&session_id, None); - return Err(StartSessionError::CleanupFailed { - session_id, - agent_window_id: None, - message: "invalid tool.session_start payload; cleanup required".into(), - }); - } - sessions.cancel_reservation(&session_id); - return Err(StartSessionError::ExtensionError(RpcError { - code: bsk_protocol::ErrorCode::ProtocolError, - message: "invalid tool.session_start payload".into(), - data: None, - })); + sessions.commit_reservation(&session_id, None); + return Err(StartSessionError::CleanupFailed { + session_id, + agent_window_id: None, + message: "invalid tool.session_start payload; cleanup required".into(), + }); } }, ResponseBody::Err(err) => { - if preserve_cleanup - && err.data.as_ref().is_some_and(|d| { - d.get("reason").and_then(serde_json::Value::as_str) == Some("cleanup_failed") - }) - { + if err.data.as_ref().is_some_and(|d| { + d.get("reason").and_then(serde_json::Value::as_str) == Some("cleanup_failed") + }) { let agent_window_id = err .data .as_ref() @@ -669,10 +695,30 @@ 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")) + || (window.tab_id.is_some() && start_result.claimed_tab_id != window.tab_id) + { + let cleanup = rollback_extension_session(&client, &session_id).await; + if let Err(message) = cleanup { + sessions.commit_reservation(&session_id, start_result.agent_window_id); + return Err(StartSessionError::CleanupFailed { + session_id, + agent_window_id: start_result.agent_window_id, + message, + }); + } + sessions.cancel_reservation(&session_id); + return Err(StartSessionError::ExtensionError(RpcError { + code: ErrorCode::ProtocolError, + message: "Extension did not confirm the requested session container and tab".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 @@ -709,15 +755,13 @@ impl StartAbortReason { async fn finish_aborted_start( client: &Arc, sessions: &Arc, + queues: &Arc, session_id: &SessionId, rpc_id: &RpcId, mut waiter: tokio::sync::oneshot::Receiver, reason: StartAbortReason, preserve_cleanup: bool, ) -> StartSessionError { - if !preserve_cleanup { - sessions.cancel_reservation(session_id); - } let cancel = Frame::Request(RequestFrame { id: format!("cancel-{rpc_id}"), method: Method::Cancel, @@ -725,6 +769,11 @@ async fn finish_aborted_start( }); if client.sink.send(cancel).is_err() { client.pending.lock().unwrap().cancel(rpc_id); + if preserve_cleanup { + sessions.commit_reservation(session_id, None); + } else { + sessions.cancel_reservation(session_id); + } return if preserve_cleanup { StartSessionError::CleanupFailed { session_id: session_id.clone(), @@ -736,9 +785,9 @@ async fn finish_aborted_start( }; } - // Managed starts outlive the receiving CLI. Keep the original waiter until - // the extension settles: an early stop/not_found is not proof that a slow - // chrome.windows.create cannot still produce a window. + // Managed starts wait for the extension to settle. Ordinary starts use a + // bounded wait and then retain the waiter in a daemon-owned task: an early + // stop/not_found is not proof that a slow create cannot still produce a window. let settled = if preserve_cleanup { Ok((&mut waiter).await) } else { @@ -751,6 +800,21 @@ async fn finish_aborted_start( { reason.error() } + ResponseBody::Err(err) + if err.data.as_ref().is_some_and(|data| { + data.get("reason").and_then(serde_json::Value::as_str) == Some("cleanup_failed") + }) => + { + StartSessionError::CleanupFailed { + session_id: session_id.clone(), + agent_window_id: err + .data + .as_ref() + .and_then(|data| data.get("resource_id")) + .and_then(serde_json::Value::as_i64), + message: err.message, + } + } ResponseBody::Err(err) if preserve_cleanup => StartSessionError::CleanupFailed { session_id: session_id.clone(), agent_window_id: None, @@ -784,25 +848,63 @@ async fn finish_aborted_start( } } Err(_) => { - // The cancel frame is already queued behind session_start. Even - // if chrome.windows.create is still blocked, the extension will - // observe the aborted signal and compensate before replying. - client.pending.lock().unwrap().cancel(rpc_id); - reason.error() + // Keep the original reply alive after the ordinary CLI's bounded + // wait. A late cleanup failure must retain the session identity. + let client = Arc::clone(client); + let sessions = Arc::clone(sessions); + let queues = Arc::clone(queues); + let session_id = session_id.clone(); + sessions.starting.lock().unwrap().insert(session_id.clone()); + tokio::spawn(async move { + let failed_window = match waiter.await { + Ok(response) => match response.body { + ResponseBody::Ok(value) => { + let window_id = serde_json::from_value::(value) + .ok() + .and_then(|result| result.agent_window_id); + rollback_extension_session(&client, &session_id) + .await + .err() + .map(|_| window_id) + } + ResponseBody::Err(err) + if err.data.as_ref().is_some_and(|data| { + data.get("reason").and_then(serde_json::Value::as_str) + == Some("cleanup_failed") + }) => + { + Some( + err.data + .as_ref() + .and_then(|data| data.get("resource_id")) + .and_then(serde_json::Value::as_i64), + ) + } + ResponseBody::Err(_) => None, + }, + Err(_) => Some(None), + }; + if let Some(window_id) = failed_window { + if sessions + .commit_reservation(&session_id, window_id) + .is_some() + { + queues.spawn(session_id); + } + } else { + sessions.cancel_reservation(&session_id); + } + }); + return reason.error(); } }; - if preserve_cleanup { - match &outcome { - StartSessionError::Cancelled | StartSessionError::Timeout => { - sessions.cancel_reservation(session_id); - } - StartSessionError::CleanupFailed { - agent_window_id, .. - } => { - sessions.commit_reservation(session_id, *agent_window_id); - } - _ => {} + match &outcome { + StartSessionError::CleanupFailed { + agent_window_id, .. + } => { + sessions.commit_reservation(session_id, *agent_window_id); } + _ => sessions.cancel_reservation(session_id), } outcome } @@ -983,3 +1085,106 @@ fn drop_session_local( queues.remove(session_id); interrupts.drop_session(session_id); } + +#[cfg(test)] +mod cancelled_start_tests { + use super::*; + use crate::daemon::browsers::{BrowserSink, Pending}; + use std::sync::atomic::AtomicBool; + use std::time::Instant; + use tokio::sync::mpsc; + + #[tokio::test] + async fn cleanup_failure_keeps_an_ordinary_cancelled_start_before_and_after_wait_timeout() { + for delay in [ + Duration::ZERO, + CANCEL_CLEANUP_TIMEOUT + Duration::from_millis(50), + ] { + let (tx, mut outbound) = mpsc::unbounded_channel(); + let client = Arc::new(BrowserClient { + id: BrowserId("test".into()), + browser_name: "chrome".into(), + browser_version: "1".into(), + extension_version: "1".into(), + extension_protocol_version: "1.4".into(), + label: "test".into(), + sink: BrowserSink { tx }, + pending: Mutex::new(Pending::default()), + generation: 1, + connected_at_ms: 0, + version_skew: false, + last_seen: Mutex::new(Instant::now()), + heartbeat_seen: AtomicBool::new(false), + }); + let sessions = Arc::new(SessionRegistry::new()); + let session_id = sessions.reserve_id(client.id.clone(), 1, || 1).unwrap(); + let queues = Arc::new(ToolQueueRegistry::new( + Arc::new(BrowserRegistry::new()), + sessions.clone(), + )); + let rpc_id = "start".to_string(); + let waiter = client.pending.lock().unwrap().register(rpc_id.clone()); + let mut task = tokio::spawn({ + let client = client.clone(); + let sessions = sessions.clone(); + let queues = queues.clone(); + let session_id = session_id.clone(); + async move { + finish_aborted_start( + &client, + &sessions, + &queues, + &session_id, + &rpc_id, + waiter, + StartAbortReason::Cancelled, + false, + ) + .await + } + }); + assert!(matches!(outbound.recv().await, Some(Frame::Request(_)))); + if !delay.is_zero() { + tokio::time::sleep(delay).await; + assert!(matches!( + (&mut task).await.unwrap(), + StartSessionError::Cancelled + )); + } + client + .pending + .lock() + .unwrap() + .resolve(bsk_protocol::ResponseFrame { + id: "start".into(), + body: ResponseBody::Err(RpcError { + code: ErrorCode::ProtocolError, + message: "window cleanup failed".into(), + data: Some(serde_json::json!({"reason":"cleanup_failed","resource_id":42})), + }), + }); + if delay.is_zero() { + assert!(matches!( + (&mut task).await.unwrap(), + StartSessionError::CleanupFailed { .. } + )); + } + tokio::time::timeout(Duration::from_secs(1), async { + loop { + if sessions + .get(&session_id) + .is_some_and(|session| session.agent_window_id == Some(42)) + { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + if !delay.is_zero() { + assert!(queues.is_accepting(&session_id)); + } + } + } +} 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..e1f8e322 100644 --- a/crates/bsk-cli/tests/cancel_forwarding.rs +++ b/crates/bsk-cli/tests/cancel_forwarding.rs @@ -203,6 +203,8 @@ async fn cancel_forwards_to_extension_when_tool_is_inflight() { match req.method { Method::ToolSessionStart => { let result = SessionStartResult { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(1), }; @@ -427,6 +429,8 @@ async fn cancel_arriving_during_promote_critical_section_keeps_request_cancel_in match req.method { Method::ToolSessionStart => { let result = SessionStartResult { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(1), }; @@ -627,6 +631,8 @@ async fn cancel_keeps_session_busy_until_delayed_extension_cleanup_finishes() { match req.method { Method::ToolSessionStart => { let result = SessionStartResult { + claimed_tab_id: None, + 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..fe486328 100644 --- a/crates/bsk-cli/tests/cli_parse.rs +++ b/crates/bsk-cli/tests/cli_parse.rs @@ -14,6 +14,63 @@ 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 existing_tab_and_ephemeral_start_require_explicit_modes() { + let cli = parse(&[ + "bsk", + "session", + "start", + "--in-window", + "--tab-id", + "42", + "--request-id", + "123:00000000-0000-0000-0000-000000000001", + "--ephemeral", + ]); + let Command::Session(SessionCmd { + sub: SessionSub::Start(args), + }) = cli.command + else { + panic!("session start expected") + }; + assert_eq!(args.tab_id, Some(42)); + assert!(args.ephemeral); + assert!(Cli::try_parse_from(["bsk", "session", "start", "--tab-id", "42"]).is_err()); + assert!(Cli::try_parse_from(["bsk", "session", "start", "--ephemeral"]).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..d828c4a8 100644 --- a/crates/bsk-cli/tests/per_session_queue.rs +++ b/crates/bsk-cli/tests/per_session_queue.rs @@ -178,6 +178,8 @@ async fn run_fake_extension_with_reply( id: req.id.clone(), body: ResponseBody::Ok( serde_json::to_value(SessionStartResult { + claimed_tab_id: None, + 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..8248d587 100644 --- a/crates/bsk-cli/tests/record_export_recovery.rs +++ b/crates/bsk-cli/tests/record_export_recovery.rs @@ -120,6 +120,8 @@ fn run_extension( serde_json::from_value(request.params.clone().unwrap()).unwrap(); ResponseBody::Ok( serde_json::to_value(SessionStartResult { + claimed_tab_id: None, + 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..d3c00460 100644 --- a/crates/bsk-cli/tests/record_stop_retry.rs +++ b/crates/bsk-cli/tests/record_stop_retry.rs @@ -123,6 +123,8 @@ fn run_extension( serde_json::from_value(request.params.clone().unwrap()).unwrap(); ResponseBody::Ok( serde_json::to_value(SessionStartResult { + claimed_tab_id: None, + 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..420dbd2e 100644 --- a/crates/bsk-cli/tests/session_user_interrupt.rs +++ b/crates/bsk-cli/tests/session_user_interrupt.rs @@ -158,6 +158,8 @@ async fn session_user_interrupt_event_cancels_inflight_with_user_aborted() { match req.method { Method::ToolSessionStart => { let result = SessionStartResult { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(1), }; @@ -335,6 +337,8 @@ async fn assert_idle_interrupt_rejects(method: Method) { if let Frame::Request(req) = frame { if req.method == Method::ToolSessionStart { let result = SessionStartResult { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(1), }; @@ -464,6 +468,8 @@ 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 { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(1), }) @@ -623,6 +629,8 @@ 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 { + claimed_tab_id: None, + 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..b1b643ff 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,180 @@ 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; +} + +#[tokio::test] +async fn existing_tab_start_requires_extension_confirmation() { + let (handle, sock) = spawn_daemon().await; + let mut ws = connect_ext(handle.ws_addr()).await; + handshake_as_ext(&mut ws).await; + let extension = tokio::spawn(async move { + let first = next_extension_request(&mut ws).await; + let params: SessionStartParams = serde_json::from_value(first.params.unwrap()).unwrap(); + assert_eq!(params.tab_id, Some(7)); + send_extension_response( + &mut ws, + ResponseFrame { + id: first.id, + body: ResponseBody::Ok(serde_json::json!({ + "agent_window_id": 10, "container_mode": "in_window", "claimed_tab_id": 7 + })), + }, + ) + .await; + let second = next_extension_request(&mut ws).await; + let params: SessionStartParams = serde_json::from_value(second.params.unwrap()).unwrap(); + assert_eq!(params.tab_id, Some(8)); + send_extension_response( + &mut ws, + ResponseFrame { + id: second.id, + body: ResponseBody::Ok(serde_json::json!({ + "agent_window_id": 10, "container_mode": "in_window" + })), + }, + ) + .await; + let rollback = next_extension_request(&mut ws).await; + assert_eq!(rollback.method, Method::ToolSessionStop); + send_extension_response( + &mut ws, + ResponseFrame { + id: rollback.id, + body: ResponseBody::Ok(serde_json::json!({})), + }, + ) + .await; + }); + let mut ipc = IpcClient::connect(&sock).await.unwrap(); + let accepted: serde_json::Value = ipc + .call( + "existing-tab-ok", + Method::SessionStart, + Some(serde_json::json!({"in_window":true,"tab_id":7})), + Duration::from_secs(5), + ) + .await + .unwrap() + .unwrap(); + assert_eq!(accepted["container_mode"], "in_window"); + let rejected = ipc + .call::<_, serde_json::Value>( + "existing-tab-old-extension", + Method::SessionStart, + Some(serde_json::json!({"in_window":true,"tab_id":8})), + Duration::from_secs(5), + ) + .await + .unwrap() + .unwrap_err(); + assert_eq!(rejected.code, ErrorCode::ProtocolError); + extension.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 +340,8 @@ async fn respond_to_aborted_start( id: start.id, body: ResponseBody::Ok( serde_json::to_value(SessionStartResult { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(agent_window_id), }) @@ -214,6 +378,8 @@ async fn respond_to_aborted_stop( id: start.id, body: ResponseBody::Ok( serde_json::to_value(SessionStartResult { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(4242), }) @@ -278,6 +444,8 @@ 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 { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(4242), }; @@ -448,6 +616,132 @@ async fn cancelling_session_start_rolls_back_a_late_extension_success() { handle.shutdown().await; } +#[tokio::test] +async fn cancelled_start_keeps_failed_cleanup_reachable_before_cli_timeout() { + cancelled_start_keeps_failed_cleanup_reachable(Duration::ZERO).await; +} + +#[tokio::test] +async fn cancelled_start_keeps_failed_cleanup_reachable_after_cli_timeout() { + cancelled_start_keeps_failed_cleanup_reachable(Duration::from_millis(2_100)).await; +} + +async fn cancelled_start_keeps_failed_cleanup_reachable(delay: Duration) { + let (handle, sock) = spawn_daemon().await; + let mut ws = connect_ext(handle.ws_addr()).await; + handshake_as_ext(&mut ws).await; + let start_sock = sock.clone(); + let mut start_call = tokio::spawn(async move { + IpcClient::connect(&start_sock) + .await + .unwrap() + .call_with_id::<_, serde_json::Value>( + "cancelled-start".into(), + Method::SessionStart, + Some(serde_json::json!({})), + Duration::from_secs(8), + ) + .await + .unwrap() + }); + let start = next_extension_request(&mut ws).await; + let session_id = start.params.as_ref().unwrap()["session_id"] + .as_str() + .unwrap() + .to_owned(); + let mut cancel_ipc = IpcClient::connect(&sock).await.unwrap(); + let cancelled: CancelResult = cancel_ipc + .call( + "cancel-start-request", + Method::Cancel, + Some(CancelParams { + rpc_id: "cancelled-start".into(), + }), + Duration::from_secs(3), + ) + .await + .unwrap() + .unwrap(); + assert!(cancelled.cancelled); + let cancel = next_extension_request(&mut ws).await; + acknowledge_extension_cancel(&mut ws, cancel, &start.id).await; + if !delay.is_zero() { + tokio::time::sleep(delay).await; + assert_eq!( + (&mut start_call).await.unwrap().unwrap_err().code, + ErrorCode::Cancelled + ); + } + send_extension_response( + &mut ws, + ResponseFrame { + id: start.id, + body: ResponseBody::Err(RpcError { + code: ErrorCode::ProtocolError, + message: "Agent Window cleanup failed".into(), + data: Some(serde_json::json!({ + "reason":"cleanup_failed", "resource_type":"agent_window", "resource_id":4242 + })), + }), + }, + ) + .await; + if delay.is_zero() { + assert_eq!( + (&mut start_call).await.unwrap().unwrap_err().code, + ErrorCode::ProtocolError + ); + } + tokio::time::timeout(Duration::from_secs(3), async { + loop { + let sid = bsk::daemon::sessions::SessionId(session_id.clone()); + if handle + .state() + .sessions + .get(&sid) + .is_some_and(|session| session.agent_window_id == Some(4242)) + && handle.state().tool_queues.is_accepting(&sid) + { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + let stop = tokio::spawn({ + let sock = sock.clone(); + let session_id = session_id.clone(); + async move { + IpcClient::connect(&sock) + .await + .unwrap() + .call::<_, serde_json::Value>( + "stop-after-failed-start", + Method::SessionStop, + Some(serde_json::json!({"session_id":session_id})), + Duration::from_secs(5), + ) + .await + .unwrap() + .unwrap() + } + }); + let stop_request = next_extension_request(&mut ws).await; + assert_eq!(stop_request.method, Method::ToolSessionStop); + send_extension_response( + &mut ws, + ResponseFrame { + id: stop_request.id, + body: ResponseBody::Ok(serde_json::to_value(SessionStopResult::default()).unwrap()), + }, + ) + .await; + stop.await.unwrap(); + assert!(handle.state().sessions.is_empty()); + handle.shutdown().await; +} + #[tokio::test] async fn timing_out_session_start_rolls_back_a_late_extension_success() { let (handle, _sock) = spawn_daemon().await; @@ -623,6 +917,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 +1009,8 @@ async fn session_start_waits_for_late_extension_handshake() { continue; } let result = SessionStartResult { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(4242), }; @@ -854,6 +1151,8 @@ async fn session_start_with_browser_instance_id_picks_target() { && req.method == Method::ToolSessionStart { let result = SessionStartResult { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(123), }; @@ -959,6 +1258,8 @@ async fn session_start_label_match_picks_target() { && req.method == Method::ToolSessionStart { let result = SessionStartResult { + claimed_tab_id: None, + container_mode: None, interaction: None, agent_window_id: Some(456), }; @@ -1047,6 +1348,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 +1388,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 +1420,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 +1522,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 +1660,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 +1701,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 +1773,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 +1944,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..2d89adaf 100644 --- a/crates/bsk-cli/tests/tools_ipc.rs +++ b/crates/bsk-cli/tests/tools_ipc.rs @@ -135,6 +135,8 @@ where window_id += 1; ResponseBody::Ok( serde_json::to_value(SessionStartResult { + claimed_tab_id: None, + 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..942c9393 100644 --- a/crates/bsk-cli/tests/tools_m7_ipc.rs +++ b/crates/bsk-cli/tests/tools_m7_ipc.rs @@ -136,6 +136,8 @@ where window_id += 1; ResponseBody::Ok( serde_json::to_value(SessionStartResult { + claimed_tab_id: None, + 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..80e2ee72 100644 --- a/crates/bsk-cli/tests/tools_m8_ipc.rs +++ b/crates/bsk-cli/tests/tools_m8_ipc.rs @@ -134,6 +134,8 @@ where window_id += 1; ResponseBody::Ok( serde_json::to_value(SessionStartResult { + claimed_tab_id: None, + 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..2d76c8ba 100644 --- a/crates/bsk-cli/tests/tools_m9_ipc.rs +++ b/crates/bsk-cli/tests/tools_m9_ipc.rs @@ -144,6 +144,8 @@ where window_id += 1; ResponseBody::Ok( serde_json::to_value(SessionStartResult { + claimed_tab_id: None, + 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..021eb8ca 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", @@ -182,6 +188,26 @@ } ] }, + "lease_expires_at_ms": { + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0.0 + }, + "lifecycle_state": { + "type": [ + "string", + "null" + ] + }, + "owner_kind": { + "type": [ + "string", + "null" + ] + }, "session_id": { "type": "string" } diff --git a/crates/bsk-protocol/schema/tool_session_start_params.json b/crates/bsk-protocol/schema/tool_session_start_params.json index 0b8eb212..dda1ae53 100644 --- a/crates/bsk-protocol/schema/tool_session_start_params.json +++ b/crates/bsk-protocol/schema/tool_session_start_params.json @@ -28,9 +28,19 @@ "format": "uint32", "minimum": 0.0 }, + "in_window": { + "type": "boolean" + }, "session_id": { "type": "string" }, + "tab_id": { + "type": [ + "integer", + "null" + ], + "format": "int64" + }, "width": { "description": "Optional Agent Window outer width in CSS pixels (100..=7680).", "type": [ diff --git a/crates/bsk-protocol/schema/tool_session_start_result.json b/crates/bsk-protocol/schema/tool_session_start_result.json index 6649f7ba..d7dd519a 100644 --- a/crates/bsk-protocol/schema/tool_session_start_result.json +++ b/crates/bsk-protocol/schema/tool_session_start_result.json @@ -10,6 +10,19 @@ ], "format": "int64" }, + "claimed_tab_id": { + "type": [ + "integer", + "null" + ], + "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..df58c94b 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, @@ -531,6 +533,12 @@ pub struct SessionStatusEntry { pub agent_window_id: Option, /// Unix epoch milliseconds. pub created_at_ms: i64, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub owner_kind: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub lifecycle_state: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub lease_expires_at_ms: Option, } /// `system.status` request payload. diff --git a/crates/bsk-protocol/src/tools/session.rs b/crates/bsk-protocol/src/tools/session.rs index aa945e14..13c14f27 100644 --- a/crates/bsk-protocol/src/tools/session.rs +++ b/crates/bsk-protocol/src/tools/session.rs @@ -40,6 +40,10 @@ 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, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub tab_id: Option, pub session_id: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub browser_instance_id: Option, @@ -61,10 +65,14 @@ 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")] pub agent_window_id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub claimed_tab_id: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] @@ -161,3 +169,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..94ebbe39 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,22 @@ 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 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. +- **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 a normal user window, skipping Agent Windows (protocol 1.4). + Adding `--tab-id ` instead claims an existing tab in its user window after the + existing borrow confirmation; startup creates no window or tab in that mode. +- **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. +- **Lifecycle**: unmanaged CLI sessions require `bsk session stop`; the default + 5-minute idle timeout is their safety net. Managed `--ephemeral` sessions have + a 45-second owner lease. The DSH plugin renews it while its agent runs and + stops its own sessions when that agent becomes idle. The daemon reaps expired + leases and retries failed cleanup. +- 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 +123,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 +170,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/recoverable-session-starts.md b/docs/recoverable-session-starts.md index 04315d22..a4e66208 100644 --- a/docs/recoverable-session-starts.md +++ b/docs/recoverable-session-starts.md @@ -16,6 +16,15 @@ bsk session request --json bsk session request --cancel --json ``` +An owner that wants automatic cleanup adds `--ephemeral` to the start command +and renews the claimed request with `bsk session request --renew --json`. +The lease lasts 45 seconds from claim or renewal. The DSH plugin renews active +sessions every 10 seconds and starts cleanup when the owning agent becomes idle. +The daemon reaper closes expired leases and retries failed stops. `session list` +shows owner kind, lifecycle state, and lease expiry without exposing the token. +Ordinary starts also retain late startup cleanup failures in the session registry +so `session list` and `session stop` can reach them after the CLI has returned. + A token is `:`, generated with a random UUID. Admission deadlines must be in the next ten minutes; the plugin uses five minutes. Treat tokens as ownership handles: they are not labels, task names, or short session IDs. Ordinary `session start` remains unchanged and does not require these calls. Managed starts do not sync unrelated CLI harness skills; the plugin carries its own instructions. Preparation creates no browser window. Start transitions from prepared to starting, then ready. Claim happens after the plugin durably records the returned session and finishes initial navigation/emulation; it changes ready to active. A repeated start with the same token and parameters returns the existing result instead of opening another window; different parameters are rejected. Expired tokens cannot start again. @@ -26,7 +35,7 @@ Cleanup failures retain the exact session/window identity and can be retried. Th The extension captures the initial tab IDs directly from window creation, before initialization or cancellation can fail. Those IDs remain owned throughout failed compensation and subsequent stops. Retry closes only agent-created tabs; later user tabs are preserved. If an agent tab cannot close beside a user tab, stop reports a cleanup failure and retains the session binding instead of releasing a leaking window. -The existing daemon reaper cancels unclaimed requests after their admission deadline (normally within the next 30-second tick), retries failed cleanup, and removes expired terminal tombstones. Claimed sessions retain the existing session idle policy. Browser disconnection still follows the existing session teardown behavior. An unresponsive browser can delay confirmed cleanup; that state remains owned and visible as pending rather than being reported as closed. +The existing daemon reaper cancels unclaimed requests after their admission deadline (normally within the next 30-second tick), retries failed cleanup, and removes expired terminal tombstones. Claimed non-ephemeral sessions retain the existing session idle policy. Ephemeral renewals refresh both the owner lease and session activity. Browser disconnection still follows the existing session teardown behavior. An unresponsive browser can delay confirmed cleanup; that state remains owned and visible as pending rather than being reported as closed. ## Plugin recovery 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 diff --git a/packages/dsh-plugin-browserskill/src/index.ts b/packages/dsh-plugin-browserskill/src/index.ts index 50952387..8cb7bacd 100644 --- a/packages/dsh-plugin-browserskill/src/index.ts +++ b/packages/dsh-plugin-browserskill/src/index.ts @@ -134,6 +134,12 @@ export function apply( // archived sessions are hidden from every surface, so their Agent Windows // would otherwise linger unreachable until idle timeout or unload. const disarmArchiveCleanup = armArchiveCleanup(ctx, starts); + const disarmTurnCleanup = + typeof ctx.on === "function" + ? ctx.on("agent/status", ({ agent, status }) => { + if (status === "idle") starts.turnEnded(agent.id); + }) + : () => {}; // Non-blocking install probe: warn early when bsk is missing instead of // failing the first tool call with a bare spawn error. Uses --version on @@ -161,6 +167,7 @@ export function apply( unregisterSkill(); removeRoutes(); disarmArchiveCleanup(); + disarmTurnCleanup(); return starts.dispose().then(() => observation.dispose()); }; }); diff --git a/packages/dsh-plugin-browserskill/src/session-starts.ts b/packages/dsh-plugin-browserskill/src/session-starts.ts index 213de8b9..49037575 100644 --- a/packages/dsh-plugin-browserskill/src/session-starts.ts +++ b/packages/dsh-plugin-browserskill/src/session-starts.ts @@ -40,6 +40,8 @@ export interface StopResult { export class SessionStarts { private closing = false; private timer?: ReturnType; + private leaseTimer?: ReturnType; + private renewing = false; private readonly cleaning = new Map>(); private readonly reserved = new Set(); @@ -153,6 +155,7 @@ export class SessionStarts { this.assertStarting(record); this.deps.registry.activate(record.session.sessionId); this.deps.observation.endAction(record.session.sessionId); + this.startLeaseRenewal(); } /** Accept a durable stop; aborting its caller only cancels waiting, never cleanup. */ @@ -358,6 +361,15 @@ export class SessionStarts { this.deps.observation.removeSession(record.session.sessionId); } if (this.reserved.delete(record.requestId)) this.deps.registry.abandonStart(); + if ( + ![...this.journal.records.values()].some( + (r) => + !r.cleanup && r.session && this.deps.registry.stateFor(r.session.sessionId) === "active", + ) + ) { + if (this.leaseTimer) clearInterval(this.leaseTimer); + this.leaseTimer = undefined; + } } private completeCleanup(record: StartRecord): void { @@ -407,6 +419,12 @@ export class SessionStarts { } } + turnEnded(owner: string): void { + for (const record of this.journal.records.values()) { + if (record.owners[0] === owner) void this.fail(record).catch(() => {}); + } + } + private schedule(): void { if (this.closing || this.timer || this.pendingCleanup() === 0) return; this.timer = setTimeout(() => { @@ -419,9 +437,42 @@ export class SessionStarts { this.timer.unref(); } + private startLeaseRenewal(): void { + if (this.leaseTimer || this.closing) return; + this.leaseTimer = setInterval(() => { + if (this.renewing) return; + this.renewing = true; + void Promise.allSettled( + [...this.journal.records.values()] + .filter( + (record) => + !record.cleanup && + record.session && + this.deps.registry.stateFor(record.session.sessionId) === "active", + ) + .map(async (record) => { + try { + const result = await this.deps.runner.run( + ["session", "request", record.requestId, "--renew"], + { timeoutMs: 15_000 }, + ); + const status = parseBskJson(result, "session lease renewal") as RequestStatus; + if (status.state !== "active") throw new Error("browser session lease is not active"); + } catch (error) { + console.warn("Browser session lease renewal failed", error); + } + }), + ).finally(() => { + this.renewing = false; + }); + }, 10_000); + this.leaseTimer.unref(); + } + async dispose(): Promise { this.closing = true; if (this.timer) clearTimeout(this.timer); + if (this.leaseTimer) clearInterval(this.leaseTimer); this.deps.runner.killAll(); await Promise.allSettled([...this.journal.records.values()].map((r) => this.fail(r))); this.journal.release(); diff --git a/packages/dsh-plugin-browserskill/src/tools.ts b/packages/dsh-plugin-browserskill/src/tools.ts index 5f9835eb..b3b7a883 100644 --- a/packages/dsh-plugin-browserskill/src/tools.ts +++ b/packages/dsh-plugin-browserskill/src/tools.ts @@ -176,7 +176,7 @@ function defineBrowserOperations(deps: ToolDeps, register: DefinitionRegistrar): defineTool({ name: "session.start", description: - "Start a new browser session: opens an Agent Window in the connected browser and returns " + + "Start a new browser session in an Agent Window, a user window, or an existing tab and return " + "its session id. The new session becomes the current session for subsequent browser_* calls. " + "Optionally navigate to an initial URL and/or apply a mobile device emulation preset.", parameters: { @@ -196,6 +196,15 @@ function defineBrowserOperations(deps: ToolDeps, register: DefinitionRegistrar): type: "boolean", description: "Open the Agent Window in the background without stealing focus.", }, + inWindow: { + type: "boolean", + description: "Use a normal user window without creating an Agent Window.", + }, + tabId: { + type: "integer", + description: + "Reuse this existing tab in its user window after borrow confirmation. Requires inWindow.", + }, browser: BROWSER_PARAM, device: { type: "string", @@ -231,12 +240,21 @@ function defineBrowserOperations(deps: ToolDeps, register: DefinitionRegistrar): if ((args.width === undefined) !== (args.height === undefined)) { throw new Error("width and height must be given together"); } + if ( + args.tabId !== undefined && + (args.inWindow !== true || !Number.isSafeInteger(args.tabId) || args.tabId <= 0) + ) + throw new Error("tabId requires inWindow and must be a positive integer"); + if (args.inWindow === true && args.width !== undefined) + throw new Error("inWindow cannot be combined with window dimensions"); // Reserve the slot BEFORE spawning: check-and-reserve is synchronous, // so concurrent starts can never both pass the cap and leak a session. const starts = (deps.starts ??= new SessionStarts(deps)); await starts.reconcile(); const record = starts.begin(ownerSessionIds(deps.ctx, exec.agent?.id)); - const startArgs = ["session", "start", "--request-id", record.requestId]; + const startArgs = ["session", "start", "--request-id", record.requestId, "--ephemeral"]; + if (args.inWindow === true) startArgs.push("--in-window"); + if (args.tabId !== undefined) startArgs.push("--tab-id", String(args.tabId)); if (args.width !== undefined && args.height !== undefined) { startArgs.push("--width", String(args.width), "--height", String(args.height)); } diff --git a/packages/dsh-plugin-browserskill/tests/profile-selection.test.ts b/packages/dsh-plugin-browserskill/tests/profile-selection.test.ts index 402142fe..c59b712c 100644 --- a/packages/dsh-plugin-browserskill/tests/profile-selection.test.ts +++ b/packages/dsh-plugin-browserskill/tests/profile-selection.test.ts @@ -24,6 +24,7 @@ describe("profile selection through browser_session", () => { "start", "--request-id", expect.any(String), + "--ephemeral", "--browser", browser, ]); diff --git a/packages/dsh-plugin-browserskill/tests/session-starts.test.ts b/packages/dsh-plugin-browserskill/tests/session-starts.test.ts index 8f90c8ab..7620235b 100644 --- a/packages/dsh-plugin-browserskill/tests/session-starts.test.ts +++ b/packages/dsh-plugin-browserskill/tests/session-starts.test.ts @@ -7,6 +7,57 @@ import { DiskStartJournal, memoryStartJournal } from "../src/start-journal"; import { cleanups, exec, failed, harness, ok } from "./session-lifecycle-harness"; describe("recoverable plugin starts", () => { + it("renews an active ephemeral session while its agent is running", async () => { + vi.useFakeTimers(); + const h = harness(async (args) => { + if (args[1] === "start") return ok({ session_id: "live", browser_instance_id: "browser" }); + if (args.includes("--claim") || args.includes("--renew")) return ok({ state: "active" }); + return ok({ state: "closed" }); + }); + await h.session({ action: "start" }); + await vi.advanceTimersByTimeAsync(10_000); + expect(h.calls.some(({ args }) => args.includes("--renew"))).toBe(true); + expect(h.registry.stateFor("live")).toBe("active"); + }); + + it("keeps a live session after one renewal failure and renews it on the next tick", async () => { + vi.useFakeTimers(); + let renewals = 0; + const h = harness(async (args) => { + if (args[1] === "start") return ok({ session_id: "live", browser_instance_id: "browser" }); + if (args.includes("--claim")) return ok({ state: "active" }); + if (args.includes("--renew")) { + renewals += 1; + return renewals === 1 ? failed("temporary renewal failure") : ok({ state: "active" }); + } + return ok({ state: "closed" }); + }); + await h.session({ action: "start" }); + await vi.advanceTimersByTimeAsync(20_000); + expect(renewals).toBe(2); + expect(h.registry.stateFor("live")).toBe("active"); + expect(h.calls.some(({ args }) => args.includes("--cancel"))).toBe(false); + }); + + it("ends only the finished agent's sessions, leaving a running child's session intact", async () => { + let next = 0; + const h = harness(async (args) => { + if (args[1] === "start") + return ok({ session_id: ++next === 1 ? "root" : "child", browser_instance_id: "browser" }); + if (args.includes("--claim")) return ok({ state: "active" }); + return ok({ state: "closed" }); + }); + await h.session({ action: "start" }, { ...exec(), agent: { id: "root" } } as never); + await h.session({ action: "start" }, { ...exec(), agent: { id: "child" } } as never); + const child = [...h.journal.records.values()].find( + (record) => record.session?.sessionId === "child", + )!; + child.owners.push("root"); + h.starts.turnEnded("root"); + await vi.waitFor(() => expect(h.registry.isOwned("root")).toBe(false)); + expect(h.registry.stateFor("child")).toBe("active"); + }); + it("publishes a session only after both initialization and claim succeed", async () => { let finishNavigation!: (result: BskRunResult) => void; let finishClaim!: (result: BskRunResult) => void; diff --git a/packages/dsh-plugin-browserskill/tests/tools.test.ts b/packages/dsh-plugin-browserskill/tests/tools.test.ts index 911b5eeb..5fb183a0 100644 --- a/packages/dsh-plugin-browserskill/tests/tools.test.ts +++ b/packages/dsh-plugin-browserskill/tests/tools.test.ts @@ -318,7 +318,7 @@ describe("action dispatch", () => { expect(registry.current()).toBe("s1"); expect(calls.map(({ args }) => args)).toEqual([ - ["session", "start", "--request-id", expect.any(String)], + ["session", "start", "--request-id", expect.any(String), "--ephemeral"], ["navigate", "--session", "s1", "https://example.test/"], ["observe", "--session", "s1"], ["fill", "--session", "s1", "--value", "hello", "@e1"], @@ -336,6 +336,24 @@ describe("action dispatch", () => { }); describe("session.start", () => { + it("forwards existing-tab startup without requesting a new window", async () => { + const { tools, calls } = setup({ "session start": START_REPLY("s1") }); + await tools.get("session.start")?.execute({ inWindow: true, tabId: 7 }, makeExec()); + expect(calls[0].args).toEqual([ + "session", + "start", + "--request-id", + expect.any(String), + "--ephemeral", + "--in-window", + "--tab-id", + "7", + ]); + await expect(tools.get("session.start")?.execute({ tabId: 7 }, makeExec())).rejects.toThrow( + "tabId requires inWindow", + ); + }); + it("maps the start reply and tracks the session as current", async () => { const { tools, registry } = setup({ "session start": START_REPLY("s1") }); const value = await startSession(tools); @@ -364,6 +382,7 @@ describe("session.start", () => { "start", "--request-id", expect.any(String), + "--ephemeral", "--width", "1280", "--height",