diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index 4edd0a24..ac8904a2 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -41,6 +41,8 @@ import { attachRecordStepListener, type RecordRuntimeDeps, } from "@/tools/record"; +import { chromeTabsApi } from "@/tools/shared"; +import { chromeTabMutationApi } from "@/tools/tabs"; import { detectBrowserMeta } from "@/transport/handshake"; import type { Transport } from "@/transport/transport"; import { WSTransport } from "@/transport/ws-transport"; @@ -80,6 +82,29 @@ export default defineBackground(() => { }; } + /** + * Authoritative overlay state for a *specific* tab. A user-created tab + * (`ctx.userTabs`) inside the Agent Window must never show the control + * mask — return hidden immediately. This is what lets a freshly-mounted + * content script receive the correct state on its first `overlay.ready` + * ping instead of flashing control and then hiding (design: decide before + * showing, never show then correct). + */ + function overlayStateForTab(tabId?: number, windowId?: number): OverlayAgentStateMessage { + if (typeof tabId === "number" && typeof windowId === "number") { + const ctx = sessions.findByWindowId(windowId); + if (ctx && ctx.userTabs.has(tabId)) { + return { + type: OVERLAY_AGENT_STATE, + sessionId: null, + mode: "hidden", + generation: overlayGeneration, + }; + } + } + return overlayStateForWindow(windowId); + } + async function pushOverlayStateToTab( tabId: number, state: OverlayAgentStateMessage, @@ -91,13 +116,27 @@ export default defineBackground(() => { } } + async function pushOverlayStateForTab(tabId: number, windowId?: number): Promise { + const state = overlayStateForTab(tabId, windowId); + await pushOverlayStateToTab(tabId, state); + } + async function pushOverlayStateForWindow(windowId: number): Promise { - const state = overlayStateForWindow(windowId); + const ctx = sessions.findByWindowId(windowId); + const baseState = overlayStateForWindow(windowId); const tabs = await chrome.tabs.query({ windowId }); await Promise.all( - tabs.map((tab) => - typeof tab.id === "number" ? pushOverlayStateToTab(tab.id, state) : Promise.resolve(), - ), + tabs.map((tab) => { + if (typeof tab.id !== "number") return Promise.resolve(); + // A user-created tab in the Agent Window must stay free for the user + // to operate — never show the control mask over it. Override the + // window-level control state with a hidden state for those tabs. + const isUserTab = ctx ? ctx.userTabs.has(tab.id) : false; + const state: OverlayAgentStateMessage = isUserTab + ? { ...baseState, sessionId: null, mode: "hidden" } + : baseState; + return pushOverlayStateToTab(tab.id, state); + }), ); } @@ -136,13 +175,41 @@ export default defineBackground(() => { if (typeof tab.windowId !== "number") return; pushOverlayStateForAgentWindow(tab.windowId); }); + // A new tab inside an Agent Window is agent-owned (the home tab, matched by + // homeTabId, or a tool.tab_create flagged by the pending count) or + // user-created (via Chrome UI). Classify it so user-opened tabs are kept + // free and can be pushed a hidden overlay immediately. See + // SessionManager.classifyNewTab. + chrome.tabs.onCreated.addListener((tab) => { + if (typeof tab.windowId !== "number" || typeof tab.id !== "number") return; + const kind = sessions.classifyNewTab(tab.id, tab.windowId); + if (kind === "user") { + // User tab: explicitly free it from the agent control mask. + void pushOverlayStateToTab(tab.id, { + type: OVERLAY_AGENT_STATE, + sessionId: null, + mode: "hidden", + generation: overlayGeneration, + }); + } else if (kind === "agent") { + // Agent tab (or home tab): make sure it reflects the session's + // current control mode. + void pushOverlayStateForAgentWindow(tab.windowId); + } + }); // 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). void sessionsLive.refresh(); const cleanupAfterDisconnect = createDisconnectCleanup({ manager: sessions, - sessionStopDeps: { cdp }, + // Same contract as the dispatcher's `tool.session_stop`: without these + // deps the agent-tab cleanup and the window-release path are dead code. + sessionStopDeps: { + cdp, + tabManagement: { tabs: chromeTabMutationApi }, + tabsQuery: chromeTabsApi, + }, onSessionsChanged: () => { void sessionsLive.syncFromManager(); }, @@ -309,7 +376,11 @@ export default defineBackground(() => { } if (msg.kind === OVERLAY_MSG_READY) { - sendResponse(overlayStateForWindow(sender.tab?.windowId)); + // Decide per-tab *before* sending: a user-created tab gets hidden + // immediately so the content script never flashes the control mask. + if (typeof sender.tab?.id === "number") { + void pushOverlayStateForTab(sender.tab.id, sender.tab.windowId); + } return false; } diff --git a/apps/extension/src/lib/overlay-bridge.ts b/apps/extension/src/lib/overlay-bridge.ts index b7f06386..199b5f0a 100644 --- a/apps/extension/src/lib/overlay-bridge.ts +++ b/apps/extension/src/lib/overlay-bridge.ts @@ -3,8 +3,10 @@ * between the content-script control overlay and the background SW. * * Content script → background: - * - `{ kind: "overlay.ready" }` → background replies with the authoritative - * overlay state for the sender's window. + * - `{ kind: "overlay.ready" }` → background decides the per-tab state first + * (see overlayStateForTab) and pushes the authoritative overlay state for + * that tab. It also proactively pushes on tab create and control-mode + * change, so a tab usually receives state before it ever sends ready. * - `{ kind: "overlay.interrupt", sessionId }` → background asks the * daemon (via a `session.user_interrupt` WS event) to cancel * every inflight + queued tool call for that session with diff --git a/apps/extension/src/session-manager/__tests__/disconnect-cleanup.test.ts b/apps/extension/src/session-manager/__tests__/disconnect-cleanup.test.ts index 3ca893a8..f35a2a2b 100644 --- a/apps/extension/src/session-manager/__tests__/disconnect-cleanup.test.ts +++ b/apps/extension/src/session-manager/__tests__/disconnect-cleanup.test.ts @@ -9,7 +9,7 @@ describe("disconnect session cleanup", () => { const manager = new SessionManager({ agentWindow: { create: vi.fn(async () => nextWindowId++), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), remove, }, }); @@ -41,7 +41,7 @@ describe("disconnect session cleanup", () => { const manager = new SessionManager({ agentWindow: { create: vi.fn(async () => 100), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), remove: vi.fn(() => removeGate), }, }); 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 7ccae333..5aaf338d 100644 --- a/apps/extension/src/session-manager/__tests__/event-handler.test.ts +++ b/apps/extension/src/session-manager/__tests__/event-handler.test.ts @@ -39,7 +39,7 @@ describe("attachSessionEventHandler", () => { agentWindow: { create: vi.fn(async () => 4242), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); await manager.start("aa11"); @@ -73,7 +73,7 @@ describe("attachSessionEventHandler", () => { agentWindow: { create: vi.fn(async () => 4242), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const ctx = await manager.start("aa11"); @@ -113,7 +113,7 @@ describe("attachSessionEventHandler", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const transport = fakeTransport(); @@ -129,7 +129,7 @@ describe("attachSessionEventHandler", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const transport = fakeTransport(); diff --git a/apps/extension/src/session-manager/__tests__/manager.test.ts b/apps/extension/src/session-manager/__tests__/manager.test.ts index 966693e8..46d44be5 100644 --- a/apps/extension/src/session-manager/__tests__/manager.test.ts +++ b/apps/extension/src/session-manager/__tests__/manager.test.ts @@ -13,7 +13,7 @@ function fakeAgentWindow(): AgentWindowApi & { return id; }); const removeMock = vi.fn(async (_id: number) => {}); - const ensureActiveTabMock = vi.fn(async (_windowId: number, _url: string) => {}); + const ensureActiveTabMock = vi.fn(async (_windowId: number, _url: string) => 0); return { create: createMock, remove: removeMock, @@ -151,6 +151,53 @@ describe("SessionManager", () => { expect(sm.list()).toEqual([]); }); + describe("classifyNewTab (user vs agent tab freedom)", () => { + it("classifies the home tab as agent", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow() }); + const ctx = await sm.start("aa11"); + // The home tab is agent-owned and matched by home tab id. The prior + // windowInitializing flag was cleared before the home tab's onCreated + // arrived, so matching it by boot timing was never reliable. + const homeTabId = ctx.homeTabId as number; + const kind = sm.classifyNewTab(homeTabId, ctx.agentWindowId); + expect(kind).toBe("agent"); + expect(ctx.agentCreatedTabs.has(homeTabId)).toBe(true); + expect(ctx.userTabs.has(homeTabId)).toBe(false); + }); + + it("classifies a pending tab_create tab as agent", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow() }); + const ctx = await sm.start("aa11"); + sm.markAgentTabPending(ctx.agentWindowId); + const kind = sm.classifyNewTab(11, ctx.agentWindowId); + expect(kind).toBe("agent"); + expect(ctx.agentCreatedTabs.has(11)).toBe(true); + expect(ctx.userTabs.has(11)).toBe(false); + expect(ctx.pendingAgentTabCount).toBe(0); + }); + + it("classifies a user-opened tab (no pending) as user and keeps it free", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow() }); + const ctx = await sm.start("aa11"); + const kind = sm.classifyNewTab(99, ctx.agentWindowId); + expect(kind).toBe("user"); + expect(ctx.userTabs.has(99)).toBe(true); + expect(ctx.agentCreatedTabs.has(99)).toBe(false); + }); + + it("matches multiple pending agent tabs to multiple onCreated events", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow() }); + const ctx = await sm.start("aa11"); + sm.markAgentTabPending(ctx.agentWindowId); + sm.markAgentTabPending(ctx.agentWindowId); + expect(sm.classifyNewTab(11, ctx.agentWindowId)).toBe("agent"); + // Second agent tab still pending → agent; not mistaken for user. + expect(sm.classifyNewTab(12, ctx.agentWindowId)).toBe("agent"); + // No more pending → a later user tab is user. + expect(sm.classifyNewTab(99, ctx.agentWindowId)).toBe("user"); + }); + }); + describe("findBorrowingSession", () => { it("returns null when no session has borrowed the tab", async () => { const sm = new SessionManager({ agentWindow: fakeAgentWindow() }); diff --git a/apps/extension/src/session-manager/agent-window.ts b/apps/extension/src/session-manager/agent-window.ts index 73f634ba..1336dc70 100644 --- a/apps/extension/src/session-manager/agent-window.ts +++ b/apps/extension/src/session-manager/agent-window.ts @@ -14,8 +14,11 @@ export interface AgentWindowApi { * Guarantee the Agent Window has an active, CDP-navigable tab. * `chrome://` pages (including the New Tab page) reject `Page.navigate`, * so sessions bootstrap with `about:blank` instead. + * + * Resolves with the id of the activated (or newly created) tab, so callers + * can track the session's home tab without re-querying Chrome. */ - ensureActiveTab(windowId: number, url: string): Promise; + ensureActiveTab(windowId: number, url: string): Promise; } /** Creation hints for a new Agent Window. */ @@ -49,15 +52,19 @@ export const chromeAgentWindowApi: AgentWindowApi = { // cancellation success while the Agent Window remains open. await chrome.windows.remove(windowId); }, - async ensureActiveTab(windowId: number, url: string): Promise { + async ensureActiveTab(windowId: number, url: string): Promise { const tabs = await chrome.tabs.query({ windowId }); const first = tabs.find((t) => typeof t.id === "number"); - if (first?.id) { + if (first?.id !== undefined) { if (!first.active) { await chrome.tabs.update(first.id, { active: true }); } - return; + return first.id; } - await chrome.tabs.create({ windowId, url, active: true }); + const created = await chrome.tabs.create({ windowId, url, active: true }); + if (typeof created?.id !== "number") { + throw new Error("[bh] chrome.tabs.create returned no tab id"); + } + return created.id; }, }; diff --git a/apps/extension/src/session-manager/manager.ts b/apps/extension/src/session-manager/manager.ts index 2fef0d76..5c4f5304 100644 --- a/apps/extension/src/session-manager/manager.ts +++ b/apps/extension/src/session-manager/manager.ts @@ -6,6 +6,38 @@ export interface SessionContext { agentWindowId: number; refStore: RefStore; borrowedTabs: Map; + /** + * Tabs created by the agent via `tool.tab_create` in this session's + * Agent Window. Tracked so `session_stop` can close them before + * releasing the window (design §3.1). User-created tabs (via Chrome UI) + * never enter this set. + */ + agentCreatedTabs: Set; + /** + * Tabs the user opened themselves inside the Agent Window via Chrome UI + * (new-tab button, Cmd+T, right-click → open in new tab, …). These must + * NOT be controlled by the agent overlay and must be left free for the + * user to operate. Distinguishing them from agent-created tabs is done + * by the `chrome.tabs.onCreated` listener in background.ts, which consults + * `pendingAgentTabCount` to tell "opened via tool.tab_create" apart from + * "opened by the user". + */ + userTabs: Set; + /** + * Number of agent tabs currently being created via `tool.tab_create` but + * whose `onCreated` event has not yet been observed. `handleTabCreate` + * increments this *before* calling `chrome.tabs.create`; the + * `onCreated` listener decrements it once per new tab in the Agent Window. + * This lets us tell agent-created tabs from user-created tabs even though + * both fire `onCreated` (design §3.1, user-tab freedom fix). + */ + pendingAgentTabCount: number; + /** + * Id of the home tab created/activated when the session started + * (`ensureActiveTab`). Used to clean up the home tab precisely on + * stop instead of matching by URL (design §3.2). + */ + homeTabId: number | null; createdAtMs: number; } @@ -179,7 +211,7 @@ export class SessionManager { const { signal: _signal, ...createOptions } = opts; windowId = await this.agentWindow.create(AGENT_WINDOW_HOME, createOptions); throwIfSessionStartAborted(opts.signal); - await this.agentWindow.ensureActiveTab(windowId, AGENT_WINDOW_HOME); + const homeTabId = await this.agentWindow.ensureActiveTab(windowId, AGENT_WINDOW_HOME); throwIfSessionStartAborted(opts.signal); const ctx: SessionContext = { @@ -187,6 +219,10 @@ export class SessionManager { agentWindowId: windowId, refStore: new RefStore(), borrowedTabs: new Map(), + agentCreatedTabs: new Set(), + userTabs: new Set(), + pendingAgentTabCount: 0, + homeTabId, createdAtMs: this.now(), }; this.sessions.set(sessionId, ctx); @@ -204,6 +240,50 @@ export class SessionManager { } } + /** + * Record that a `tool.tab_create` is about to open a tab. Called *before* + * `chrome.tabs.create` so the pending count is visible to the + * `onCreated` listener that will fire for the new tab. + */ + markAgentTabPending(windowId: number): void { + const ctx = this.findByWindowId(windowId); + if (!ctx) return; + ctx.pendingAgentTabCount += 1; + } + + /** + * Release a pending `tool.tab_create` slot when the create itself failed + * and no `chrome.tabs.onCreated` event will arrive to consume it. Without + * this the counter leaks and the *next* tab the user opens is misclassified + * as agent-created — the exact confusion this counter exists to prevent. + */ + releaseAgentTabPending(windowId: number): void { + const ctx = this.findByWindowId(windowId); + if (!ctx || ctx.pendingAgentTabCount === 0) return; + ctx.pendingAgentTabCount -= 1; + } + + /** + * Called by the `onCreated` listener for each new tab in an Agent Window. + * Returns `"agent"` when the tab is the window's home tab or corresponds to + * a pending `tool.tab_create` (and registers it), `"user"` when the user + * opened it via Chrome UI. + */ + classifyNewTab(tabId: number, windowId: number): "agent" | "user" | "unknown" { + const ctx = this.findByWindowId(windowId); + if (!ctx) return "unknown"; + // The home tab is agent-owned. Its `onCreated` fires before any + // window-initialising flag would be observable, so we match it by home + // tab id rather than by boot timing (which left the branch unreachable). + if (tabId === ctx.homeTabId || ctx.pendingAgentTabCount > 0) { + if (ctx.pendingAgentTabCount > 0) ctx.pendingAgentTabCount -= 1; + ctx.agentCreatedTabs.add(tabId); + return "agent"; + } + ctx.userTabs.add(tabId); + return "user"; + } + /** * Tear down a session: close its Agent Window and drop the context. * diff --git a/apps/extension/src/tools/__tests__/console.test.ts b/apps/extension/src/tools/__tests__/console.test.ts index a84e2beb..650bf0d7 100644 --- a/apps/extension/src/tools/__tests__/console.test.ts +++ b/apps/extension/src/tools/__tests__/console.test.ts @@ -13,7 +13,7 @@ function fakeAgentWindow(ids: number[]) { return id; }), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }; } diff --git a/apps/extension/src/tools/__tests__/dispatcher.test.ts b/apps/extension/src/tools/__tests__/dispatcher.test.ts index c05cdb2a..dc5e4212 100644 --- a/apps/extension/src/tools/__tests__/dispatcher.test.ts +++ b/apps/extension/src/tools/__tests__/dispatcher.test.ts @@ -55,7 +55,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 4242), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); await sessions.start("aa11"); @@ -117,7 +117,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 4242), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); @@ -140,7 +140,7 @@ describe("ToolDispatcher", () => { agentWindow: { create, remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); @@ -158,7 +158,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 4242), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); await sessions.start("aa11"); @@ -172,6 +172,48 @@ describe("ToolDispatcher", () => { expect(sent[0]).toEqual({ id: "r-1", result: {} }); }); + it("wires the production tab APIs into tool.session_stop so a surviving user tab releases the window", async () => { + // Regression guard for the deps that session_stop reads directly: when + // `tabManagement.tabs` / `tabsQuery` are not injected by the dispatcher, + // the agent-tab cleanup and the release decision silently no-op and the + // Agent Window is closed even though a user tab is still open (issue #57). + const removeTab = vi.fn(async () => {}); + const closeWindow = vi.fn(async () => {}); + // After cleanup the window still holds one tab (id 99) that is neither + // the home tab nor an agent-created tab — i.e. a genuine user tab. + vi.stubGlobal("chrome", { + tabs: { + remove: removeTab, + query: vi.fn(async () => [{ id: 99, windowId: 4242, active: true }]), + }, + }); + const { transport, sent, deliver } = fakeTransport(); + const sessions = new SessionManager({ + agentWindow: { + create: vi.fn(async () => 4242), + remove: closeWindow, + ensureActiveTab: vi.fn(async () => 1), + }, + }); + await sessions.start("aa11"); + // Register tab 7 as agent-created (pending counter + onCreated classify). + sessions.markAgentTabPending(4242); + sessions.classifyNewTab(7, 4242); + + const dispatcher = new ToolDispatcher({ transport, sessions }); + dispatcher.start(); + + deliver(makeRequest("tool.session_stop", { session_id: "aa11" })); + await flushMicrotasks(); + + // The agent tab is closed through the real chrome.tabs surface... + expect(removeTab).toHaveBeenCalledWith(7); + // ...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 } }); + }); + it("routes tool.console through the CDP console buffer", async () => { vi.stubGlobal("chrome", { tabs: { @@ -183,7 +225,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 4242), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); await sessions.start("aa11"); @@ -238,7 +280,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 4242), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); await sessions.start("aa11"); @@ -274,7 +316,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 4242), remove, - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); await sessions.start("aa11"); @@ -322,7 +364,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); @@ -342,7 +384,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); @@ -366,7 +408,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); @@ -383,7 +425,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); @@ -400,7 +442,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const onSessionsChanged = vi.fn(); @@ -422,7 +464,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const onBrowserControlResumed = vi.fn(); @@ -456,7 +498,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const order: string[] = []; @@ -538,7 +580,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const cdp = { @@ -599,7 +641,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); @@ -636,7 +678,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 4242), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); await sessions.start("aa11"); @@ -688,7 +730,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); @@ -722,7 +764,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 4242), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); transport.send = () => { @@ -744,7 +786,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 5555), remove, - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); transport.send = () => { @@ -772,7 +814,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(() => createPromise), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); @@ -823,7 +865,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(async () => 1), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); @@ -844,7 +886,7 @@ describe("ToolDispatcher", () => { agentWindow: { create: vi.fn(() => createPromise), remove: vi.fn(), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }, }); const dispatcher = new ToolDispatcher({ transport, sessions }); diff --git a/apps/extension/src/tools/__tests__/emulate.test.ts b/apps/extension/src/tools/__tests__/emulate.test.ts index 37b375b8..d8e27afe 100644 --- a/apps/extension/src/tools/__tests__/emulate.test.ts +++ b/apps/extension/src/tools/__tests__/emulate.test.ts @@ -20,7 +20,7 @@ function fakeAgentWindow(ids: number[]) { return id; }), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }; } diff --git a/apps/extension/src/tools/__tests__/evaluate.test.ts b/apps/extension/src/tools/__tests__/evaluate.test.ts index 5b4df701..0065499f 100644 --- a/apps/extension/src/tools/__tests__/evaluate.test.ts +++ b/apps/extension/src/tools/__tests__/evaluate.test.ts @@ -12,7 +12,7 @@ function fakeAgentWindow(ids: number[]) { return id; }), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }; } diff --git a/apps/extension/src/tools/__tests__/interaction.test.ts b/apps/extension/src/tools/__tests__/interaction.test.ts index b78877ad..45ab74e5 100644 --- a/apps/extension/src/tools/__tests__/interaction.test.ts +++ b/apps/extension/src/tools/__tests__/interaction.test.ts @@ -21,7 +21,7 @@ function fakeAgentWindow(ids: number[]) { return id; }), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }; } diff --git a/apps/extension/src/tools/__tests__/navigation.test.ts b/apps/extension/src/tools/__tests__/navigation.test.ts index f2a6bbe1..921771e2 100644 --- a/apps/extension/src/tools/__tests__/navigation.test.ts +++ b/apps/extension/src/tools/__tests__/navigation.test.ts @@ -21,7 +21,7 @@ function fakeAgentWindow(ids: number[]) { return id; }), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }; } diff --git a/apps/extension/src/tools/__tests__/network.test.ts b/apps/extension/src/tools/__tests__/network.test.ts index ed6f80b4..8af1793a 100644 --- a/apps/extension/src/tools/__tests__/network.test.ts +++ b/apps/extension/src/tools/__tests__/network.test.ts @@ -12,7 +12,7 @@ function fakeAgentWindow(ids: number[]) { return id; }), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }; } diff --git a/apps/extension/src/tools/__tests__/observation.test.ts b/apps/extension/src/tools/__tests__/observation.test.ts index 2e18a414..0763262d 100644 --- a/apps/extension/src/tools/__tests__/observation.test.ts +++ b/apps/extension/src/tools/__tests__/observation.test.ts @@ -28,7 +28,7 @@ function fakeAgentWindow(ids: number[]) { return id; }), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }; } diff --git a/apps/extension/src/tools/__tests__/session.test.ts b/apps/extension/src/tools/__tests__/session.test.ts index 118c18e7..57e53e37 100644 --- a/apps/extension/src/tools/__tests__/session.test.ts +++ b/apps/extension/src/tools/__tests__/session.test.ts @@ -1,8 +1,26 @@ import { describe, expect, it, vi } from "vitest"; import { SessionManager } from "@/session-manager/manager"; import { handleSessionStop } from "../session"; +import type { ChromeTabsApi } from "../shared"; import { type AgentOverlayResetApi, type ChromeWindowsApi, type TabMutationApi } from "../tabs"; +/** Build a read-only query api over a FakeState. */ +function makeQuery(state: FakeState): ChromeTabsApi { + return { + get: vi.fn(async (id) => { + const t = state.tabs.get(id); + if (!t) throw new Error(`tab ${id} not found`); + return t; + }), + query: vi.fn(async (q: chrome.tabs.QueryInfo) => { + const w = q.windowId; + return Array.from(state.tabs.values()).filter( + (t) => typeof w !== "number" || t.windowId === w, + ); + }), + }; +} + function fakeAgentWindow(ids: number[]) { let i = 0; const create = vi.fn(async () => { @@ -11,7 +29,7 @@ function fakeAgentWindow(ids: number[]) { return id; }); const remove = vi.fn(async () => {}); - const ensureActiveTab = vi.fn(async () => {}); + const ensureActiveTab = vi.fn(async () => 0); return { create, remove, ensureActiveTab }; } @@ -30,7 +48,9 @@ function makeApis( } { const tabs: TabMutationApi = { create: vi.fn(), - remove: vi.fn(async () => {}), + remove: vi.fn(async (id: number) => { + state.tabs.delete(id); + }), update: vi.fn(async (_id, _p) => undefined), get: vi.fn(async (id) => { const t = state.tabs.get(id); @@ -268,3 +288,216 @@ describe("handleSessionStop with auto-return", () => { expect(ctx.refStore.isEmpty()).toBe(true); }); }); + +describe("handleSessionStop window release (issue #57)", () => { + const agentWindowId = 100; + + it("releases the window when user-created tabs remain", async () => { + const aw = fakeAgentWindow([agentWindowId]); + // ensureActiveTab returns the home tab id (10). We override the fake to + // return a known id. + aw.ensureActiveTab = vi.fn(async () => 10); + const sm = new SessionManager({ agentWindow: aw }); + const ctx = await sm.start("aa11"); + ctx.agentCreatedTabs.add(11); + ctx.agentCreatedTabs.add(12); + // home tab id from ensureActiveTab override + ctx.homeTabId = 10; + // user tab 99 (created via Chrome UI, NOT in agentCreatedTabs) + const state: FakeState = { + tabs: new Map([ + [10, { id: 10, windowId: agentWindowId } as chrome.tabs.Tab], + [11, { id: 11, windowId: agentWindowId } as chrome.tabs.Tab], + [12, { id: 12, windowId: agentWindowId } as chrome.tabs.Tab], + [99, { id: 99, windowId: agentWindowId } as chrome.tabs.Tab], + ]), + windowsClosed: new Set(), + moves: [], + }; + const { tabs, windows } = makeApis(state); + const query = makeQuery(state); + + const res = await handleSessionStop( + sm, + { session_id: "aa11" }, + { tabManagement: { tabs, windows }, tabsQuery: query }, + ); + if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); + expect(res.window_released).toBe(true); + // agent tabs + home removed, user tab 99 kept + expect((tabs.remove as ReturnType).mock.calls.map((c) => c[0]).sort()).toEqual([ + 10, 11, 12, + ]); + expect(state.tabs.has(99)).toBe(true); + // window released (dropOnly), not closed + expect(aw.remove).not.toHaveBeenCalled(); + expect(sm.has("aa11")).toBe(false); + }); + + it("closes the window (not release) when no user tabs remain", async () => { + const aw = fakeAgentWindow([agentWindowId]); + aw.ensureActiveTab = vi.fn(async () => 10); + const sm = new SessionManager({ agentWindow: aw }); + const ctx = await sm.start("aa11"); + ctx.agentCreatedTabs.add(11); + ctx.homeTabId = 10; + const state: FakeState = { + tabs: new Map([ + [10, { id: 10, windowId: agentWindowId } as chrome.tabs.Tab], + [11, { id: 11, windowId: agentWindowId } as chrome.tabs.Tab], + ]), + windowsClosed: new Set(), + moves: [], + }; + const { tabs, windows } = makeApis(state); + const query = makeQuery(state); + + const res = await handleSessionStop( + sm, + { session_id: "aa11" }, + { tabManagement: { tabs, windows }, tabsQuery: query }, + ); + if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); + expect(res.window_released).toBeFalsy(); + expect(aw.remove).toHaveBeenCalledWith(agentWindowId); + expect(sm.has("aa11")).toBe(false); + }); + + it("is non-fatal when an agent tab is already gone", async () => { + const aw = fakeAgentWindow([agentWindowId]); + aw.ensureActiveTab = vi.fn(async () => 10); + const sm = new SessionManager({ agentWindow: aw }); + const ctx = await sm.start("aa11"); + ctx.agentCreatedTabs.add(11); // no longer in state → remove throws + ctx.homeTabId = 10; + const state: FakeState = { + tabs: new Map([[10, { id: 10, windowId: agentWindowId } as chrome.tabs.Tab]]), + windowsClosed: new Set(), + moves: [], + }; + const { tabs, windows } = makeApis(state); + (tabs.remove as ReturnType).mockImplementation(async (id: number) => { + if (id === 11) throw new Error("tab 11 already closed"); + state.tabs.delete(id); // home (10) removed normally + }); + const query = makeQuery(state); + + const res = await handleSessionStop( + sm, + { session_id: "aa11" }, + { tabManagement: { tabs, windows }, tabsQuery: query }, + ); + if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); + // user tab 10? no — 10 is home, removed. window would be empty → close. + expect(res.window_released).toBeFalsy(); + expect(aw.remove).toHaveBeenCalled(); + }); + + it("degrades to close-window when tabsQuery is missing", async () => { + const aw = fakeAgentWindow([agentWindowId]); + aw.ensureActiveTab = vi.fn(async () => 10); + const sm = new SessionManager({ agentWindow: aw }); + const ctx = await sm.start("aa11"); + ctx.agentCreatedTabs.add(11); + ctx.homeTabId = 10; + const state: FakeState = { + tabs: new Map([ + [10, { id: 10, windowId: agentWindowId } as chrome.tabs.Tab], + [11, { id: 11, windowId: agentWindowId } as chrome.tabs.Tab], + ]), + windowsClosed: new Set(), + moves: [], + }; + const { tabs, windows } = makeApis(state); + + const res = await handleSessionStop( + sm, + { session_id: "aa11" }, + { tabManagement: { tabs, windows } }, // no tabsQuery + ); + if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); + // No query available → conservatively close the window. + expect(res.window_released).toBeFalsy(); + expect(aw.remove).toHaveBeenCalledWith(agentWindowId); + }); + + it("degrades to close-window when tabsQuery.query throws", async () => { + const aw = fakeAgentWindow([agentWindowId]); + aw.ensureActiveTab = vi.fn(async () => 10); + const sm = new SessionManager({ agentWindow: aw }); + const ctx = await sm.start("aa11"); + ctx.agentCreatedTabs.add(11); + ctx.homeTabId = 10; + const state: FakeState = { + tabs: new Map([[11, { id: 11, windowId: agentWindowId } as chrome.tabs.Tab]]), + windowsClosed: new Set(), + moves: [], + }; + const { tabs, windows } = makeApis(state); + const query = makeQuery(state); + (query.query as ReturnType).mockImplementation(async () => { + throw new Error("query failed"); + }); + + const res = await handleSessionStop( + sm, + { session_id: "aa11" }, + { tabManagement: { tabs, windows }, tabsQuery: query }, + ); + if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); + expect(res.window_released).toBeFalsy(); + expect(aw.remove).toHaveBeenCalled(); + }); + + it("tracks agentCreatedTabs across tab_create / tab_close", async () => { + const aw = fakeAgentWindow([agentWindowId]); + aw.ensureActiveTab = vi.fn(async () => 10); + const sm = new SessionManager({ agentWindow: aw }); + const ctx = await sm.start("aa11"); + // Simulate handleTabCreate adding, then handleTabClose removing. + ctx.agentCreatedTabs.add(11); + expect(ctx.agentCreatedTabs.has(11)).toBe(true); + ctx.agentCreatedTabs.delete(11); // as handleTabClose does on success + expect(ctx.agentCreatedTabs.has(11)).toBe(false); + // Unknown id delete is a safe no-op (user-closed tab). + expect(() => ctx.agentCreatedTabs.delete(999)).not.toThrow(); + }); + + it("closes the window (not release) when an agent tab fails to close", async () => { + // Regression: a leaked agent tab (still in agentCreatedTabs, but + // still present because Step 4 remove() threw) must NOT be mistaken + // for a user tab. Otherwise the window would be released (dropOnly) + // and the agent tab would leak (issue #57 regression). + const aw = fakeAgentWindow([agentWindowId]); + aw.ensureActiveTab = vi.fn(async () => 10); + const sm = new SessionManager({ agentWindow: aw }); + const ctx = await sm.start("aa11"); + ctx.agentCreatedTabs.add(11); // Step 4 remove() will throw for this tab + ctx.homeTabId = 10; + const state: FakeState = { + tabs: new Map([ + [10, { id: 10, windowId: agentWindowId } as chrome.tabs.Tab], + [11, { id: 11, windowId: agentWindowId } as chrome.tabs.Tab], + ]), + windowsClosed: new Set(), + moves: [], + }; + const { tabs, windows } = makeApis(state); + (tabs.remove as ReturnType).mockImplementation(async (id: number) => { + if (id === 11) throw new Error("tab 11 failed to close"); + state.tabs.delete(id); // home (10) removed normally; 11 lingers + }); + const query = makeQuery(state); + + const res = await handleSessionStop( + sm, + { session_id: "aa11" }, + { tabManagement: { tabs, windows }, tabsQuery: query }, + ); + if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); + // The leaked agent tab must NOT keep the window open. + expect(res.window_released).toBeFalsy(); + expect(aw.remove).toHaveBeenCalledWith(agentWindowId); + expect(sm.has("aa11")).toBe(false); + }); +}); diff --git a/apps/extension/src/tools/__tests__/shared.test.ts b/apps/extension/src/tools/__tests__/shared.test.ts index ab44ad68..680c9516 100644 --- a/apps/extension/src/tools/__tests__/shared.test.ts +++ b/apps/extension/src/tools/__tests__/shared.test.ts @@ -12,7 +12,7 @@ function fakeAgentWindow() { return { create: vi.fn(async () => 100), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }; } diff --git a/apps/extension/src/tools/__tests__/snapshot-ref.test.ts b/apps/extension/src/tools/__tests__/snapshot-ref.test.ts index cc9459d8..8c9b0ae7 100644 --- a/apps/extension/src/tools/__tests__/snapshot-ref.test.ts +++ b/apps/extension/src/tools/__tests__/snapshot-ref.test.ts @@ -11,7 +11,7 @@ function fakeAgentWindow(ids: number[]) { return id; }, remove: async () => {}, - ensureActiveTab: async () => {}, + ensureActiveTab: async () => 1, }; } diff --git a/apps/extension/src/tools/__tests__/tabs.test.ts b/apps/extension/src/tools/__tests__/tabs.test.ts index f8fbce2e..a44fe47a 100644 --- a/apps/extension/src/tools/__tests__/tabs.test.ts +++ b/apps/extension/src/tools/__tests__/tabs.test.ts @@ -23,7 +23,7 @@ function fakeAgentWindow(ids: number[]) { return id; }), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }; } @@ -256,6 +256,23 @@ describe("handleTabCreate", () => { const res = await handleTabCreate(sm, { session_id: "aa11", index: -1 }, { tabs: api }); expect(res).toMatchObject({ code: "invalid_params" }); }); + + it("releases the pending agent-tab slot when chrome.tabs.create fails", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + await sm.start("aa11"); + const state: FakeTabState = { tabs: new Map(), nextTabId: 10, windowsClosed: new Set() }; + const { api, spies } = makeTabMutationApi(state); + spies.create.mockRejectedValueOnce(new Error("chrome.tabs.create failed")); + + const res = await handleTabCreate(sm, { session_id: "aa11" }, { tabs: api }); + expect(res).toMatchObject({ code: "protocol_error" }); + + // No tab was opened, so no `onCreated` event will consume the pending + // slot. Without the release the counter leaks and the next tab the user + // opens is misclassified as agent-created — the exact confusion the + // counter exists to prevent. + expect(sm.classifyNewTab(42, 100)).toBe("user"); + }); }); describe("handleTabClose", () => { diff --git a/apps/extension/src/tools/__tests__/waits.test.ts b/apps/extension/src/tools/__tests__/waits.test.ts index 3f82750a..ce0f3aaa 100644 --- a/apps/extension/src/tools/__tests__/waits.test.ts +++ b/apps/extension/src/tools/__tests__/waits.test.ts @@ -12,7 +12,7 @@ function fakeAgentWindow(ids: number[]) { return id; }), remove: vi.fn(async () => {}), - ensureActiveTab: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 1), }; } diff --git a/apps/extension/src/tools/__tests__/window.test.ts b/apps/extension/src/tools/__tests__/window.test.ts index 3d86504c..adab158c 100755 --- a/apps/extension/src/tools/__tests__/window.test.ts +++ b/apps/extension/src/tools/__tests__/window.test.ts @@ -11,7 +11,7 @@ function fakeAgentWindow(ids: number[]) { return id; }); const remove = vi.fn(async () => {}); - const ensureActiveTab = vi.fn(async () => {}); + const ensureActiveTab = vi.fn(async () => 1); return { create, remove, ensureActiveTab }; } diff --git a/apps/extension/src/tools/dispatcher.ts b/apps/extension/src/tools/dispatcher.ts index e0a5ac4d..8590478d 100644 --- a/apps/extension/src/tools/dispatcher.ts +++ b/apps/extension/src/tools/dispatcher.ts @@ -66,6 +66,7 @@ import { import { chromeTabsApi, lookupSession, resolveTargetTab } from "./shared"; import { type BorrowConfirmationApprover, + chromeTabMutationApi, handleTabBorrow, handleTabClose, handleTabCreate, @@ -298,6 +299,11 @@ export class ToolDispatcher { await this.releaseHoverLatch((req.params as SessionStopParams).session_id); return handleSessionStop(this.sessions, req.params as SessionStopParams, { cdp: this.cdp, + // Must be wired in production: the agent-tab cleanup and the + // window-release decision (issue #57) read these deps directly + // and silently no-op when they are absent. + tabManagement: { tabs: chromeTabMutationApi }, + tabsQuery: chromeTabsApi, signal, }); } diff --git a/apps/extension/src/tools/session.ts b/apps/extension/src/tools/session.ts index 9bd1b277..165f874a 100644 --- a/apps/extension/src/tools/session.ts +++ b/apps/extension/src/tools/session.ts @@ -2,6 +2,7 @@ import { type SessionManager, SessionStartCleanupError } from "@/session-manager import type { RpcError } from "@/transport/types"; import { rpcError } from "./errors"; import { clearRecordingForSession } from "./record"; +import type { ChromeTabsApi } from "./shared"; import { isRpcError } from "./shared"; import { returnBorrowedTab, type TabManagementDeps } from "./tabs"; @@ -73,6 +74,12 @@ export interface SessionStopResult { /** Tab ids whose return path failed; those entries remain borrowed so * shutdown can be retried without closing the Agent Window. */ return_failures?: Array<{ tab_id: number; code: string; message: string }>; + /** + * True when the Agent Window was *released* to the user (instead of being + * closed) because it still contained user-created tabs after the agent's own + * tabs were closed. Single-session scope. + */ + window_released?: boolean; } export interface SessionStopDeps { @@ -87,6 +94,13 @@ export interface SessionStopDeps { * without a real browser. */ tabManagement?: TabManagementDeps; + /** + * Read-only Chrome tabs API used to *query* remaining tabs before deciding + * whether to release the window. Kept separate from `tabManagement.tabs` + * (a `TabMutationApi` that has no `query`) so the mutation interface stays + * pure. Tests can inject a fake `ChromeTabsApi` too. + */ + tabsQuery?: ChromeTabsApi; } /** @@ -152,8 +166,10 @@ export async function handleSessionStart( * cleanly (review parity with M6). * 3. Detach CDP sessions the extension still holds for this * session (no-op if M6/M7 didn't attach to any tab). - * 4. Close the Agent Window. SessionManager.stop() removes the - * Chrome window and forgets the context. + * 4. Close the agent-created tabs and the home tab (design §3.2). + * 5. If any tab remains — those are user-created tabs — release the + * window to the user (dropOnly) instead of closing it, so user + * tabs survive. Otherwise close the window as before. * * Failures in step 1 keep the Agent Window open: a failed borrowed tab * may still be there, so closing the window would risk losing user @@ -253,8 +269,76 @@ export async function handleSessionStop( return { code: "cancelled", message: "session_stop aborted before window close" }; } - // Step 4: close the Agent Window and drop the context. - await manager.stop(params.session_id); + // Step 4: close the agent-created tabs and the home tab. `tabsApi` is a + // TabMutationApi (remove only); `queryApi` is a separate read-only + // ChromeTabsApi. If neither is injected, we conservatively fall back to + // closing the window (see Step 5). + const tabsApi = deps.tabManagement?.tabs; + const queryApi = deps.tabsQuery; + + if (tabsApi) { + // 4a: close each agent-created tab that still exists. + const agentCreatedTabIds = Array.from(ctx.agentCreatedTabs); + for (const tabId of agentCreatedTabIds) { + try { + await tabsApi.remove(tabId); + ctx.agentCreatedTabs.delete(tabId); + } catch (err) { + // Tab may already be gone (closed by the user). Non-fatal. + console.warn(`[bsk session_stop] failed to close agent tab ${tabId}`, err); + } + } + + // 4b: close the home tab by id (not by URL — avoids deleting a user's + // `about:blank` tab). Non-fatal if it's already gone. + if (ctx.homeTabId != null) { + try { + await tabsApi.remove(ctx.homeTabId); + } catch (err) { + console.warn(`[bsk session_stop] failed to close home tab`, err); + } + } + } + + // Step 5: decide whether to release (keep) the window or close it. + let shouldRelease = false; + if (queryApi) { + try { + const liveWindowTabs = await queryApi.query({ windowId: ctx.agentWindowId }); + // 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 + // window would be kept and that agent tab would leak (issue #57 + // regression). Exclude any id still tracked in agentCreatedTabs. + const userTabs = liveWindowTabs.filter((t) => { + if (t.id === undefined) return false; + if (t.id === ctx.homeTabId) return false; + return !ctx.agentCreatedTabs.has(t.id); + }); + const leakedAgentTabs = liveWindowTabs.filter( + (t) => t.id !== undefined && ctx.agentCreatedTabs.has(t.id), + ); + if (leakedAgentTabs.length > 0) { + console.warn( + `[bsk session_stop] ${leakedAgentTabs.length} agent tab(s) failed to close; forcing window close instead of release`, + leakedAgentTabs.map((t) => t.id), + ); + } + shouldRelease = userTabs.length > 0; + } catch { + // Query failed (e.g. window already gone) — conservatively close it. + shouldRelease = false; + } + } + + if (shouldRelease) { + // Keep the window + its user tabs; only drop the session binding. + await manager.stop(params.session_id, { dropOnly: true }); + result.window_released = true; + } else { + // Window is empty (or we couldn't verify state) — close it. + await manager.stop(params.session_id); + } return result; } diff --git a/apps/extension/src/tools/tabs.ts b/apps/extension/src/tools/tabs.ts index 04e46b84..c84c0983 100644 --- a/apps/extension/src/tools/tabs.ts +++ b/apps/extension/src/tools/tabs.ts @@ -426,8 +426,27 @@ export async function handleTabCreate( const paramErr = validateTabCreateParams(params); if (paramErr) return paramErr; + // Flag the pending agent tab *before* create so the chrome.tabs.onCreated + // listener (background.ts) can tell this tab apart from a user-opened tab. + manager.markAgentTabPending(ctx.agentWindowId); + const tab = await createTabAndCleanup(deps, buildCreateProps(ctx, params)); - if (isRpcError(tab)) return tab; + if (isRpcError(tab)) { + // Release the pending slot when no tab was actually opened: with no + // `onCreated` event to consume it, the counter would leak and the next + // tab the user opens would be misclassified as agent-created. An abort + // (`cancelled`) is the exception — the tab did open (and was cleaned up), + // so its `onCreated` still consumes the slot. + if (tab.code !== "cancelled") { + manager.releaseAgentTabPending(ctx.agentWindowId); + } + return tab; + } + + // Track agent-created tabs so session_stop can clean them up (design §3.1). + // The onCreated listener also adds this id; this is an idempotent fallback + // in case the listener has not fired yet. + ctx.agentCreatedTabs.add(tab.id); return { tab_id: tab.id, @@ -526,6 +545,9 @@ export async function handleTabClose( } 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); } catch (err) { return { code: "protocol_error",