diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index ce3586f..f8ae028 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -77,6 +77,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, @@ -88,13 +111,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); + }), ); } @@ -133,6 +170,27 @@ export default defineBackground(() => { if (typeof tab.windowId !== "number") return; pushOverlayStateForAgentWindow(tab.windowId); }); + // A new tab inside an Agent Window is either agent-created (via + // 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" || kind === "initializing") { + // 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). @@ -284,7 +342,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/session-manager/__tests__/manager.test.ts b/apps/extension/src/session-manager/__tests__/manager.test.ts index cfeed29..b7c306f 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, @@ -87,6 +87,55 @@ describe("SessionManager", () => { expect(sm.list()).toEqual([]); }); + describe("classifyNewTab (user vs agent tab freedom)", () => { + it("classifies a tab born during window init as agent (home tab)", async () => { + const aw = fakeAgentWindow(); + const sm = new SessionManager({ agentWindow: aw }); + const ctx = await sm.start("aa11"); + // Simulate the onCreated event for the home tab while still initializing. + // (start() already clears the flag, so re-add to emulate the race.) + (sm as unknown as { windowInitializing: Set }).windowInitializing.add( + ctx.agentWindowId, + ); + const kind = sm.classifyNewTab(0, ctx.agentWindowId); + expect(kind).toBe("initializing"); + expect(ctx.agentCreatedTabs.has(0)).toBe(true); + expect(ctx.userTabs.has(0)).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/manager.ts b/apps/extension/src/session-manager/manager.ts index 0cbdb72..3d6f402 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; } @@ -41,6 +73,13 @@ export class SessionManager { private readonly sessions = new Map(); private readonly windowIndex = new Map(); private readonly borrowReservations = new Map(); + /** + * Windows that are still being initialised by `start()` (the home tab is + * being created). Tabs born into one of these windows via `onCreated` + * belong to the agent (the home tab), not the user, and must not be + * mistaken for user-created tabs. + */ + private readonly windowInitializing = new Set(); private readonly agentWindow: AgentWindowApi; private readonly now: () => number; @@ -133,19 +172,61 @@ export class SessionManager { throw new Error(`[bh] session ${sessionId} already exists`); } const windowId = await this.agentWindow.create(AGENT_WINDOW_HOME); - await this.agentWindow.ensureActiveTab(windowId, AGENT_WINDOW_HOME); + // Mark the window as initialising so `onCreated` (driven by the home + // tab) does not mistake the home tab for a user-created tab. + this.windowInitializing.add(windowId); + const homeTabId = await this.agentWindow.ensureActiveTab(windowId, AGENT_WINDOW_HOME); const ctx: SessionContext = { sessionId, 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); this.windowIndex.set(windowId, sessionId); + this.windowInitializing.delete(windowId); return ctx; } + /** + * 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; + } + + /** + * Called by the `onCreated` listener for each new tab in an Agent Window. + * Returns `"agent"` when the tab corresponds to a pending `tool.tab_create` + * (and registers it), `"user"` when the user opened it via Chrome UI, or + * `"initializing"` when the window is still booting its home tab. + */ + classifyNewTab(tabId: number, windowId: number): "agent" | "user" | "initializing" | "unknown" { + const ctx = this.findByWindowId(windowId); + if (!ctx) return "unknown"; + if (this.windowInitializing.has(windowId)) { + // Home tab / window boot — belongs to the agent, not the user. + ctx.agentCreatedTabs.add(tabId); + return "initializing"; + } + 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__/session.test.ts b/apps/extension/src/tools/__tests__/session.test.ts index adecac4..347f8c7 100644 --- a/apps/extension/src/tools/__tests__/session.test.ts +++ b/apps/extension/src/tools/__tests__/session.test.ts @@ -2,6 +2,24 @@ import { describe, expect, it, vi } from "vitest"; import { SessionManager } from "@/session-manager/manager"; import { handleSessionStop } from "../session"; import { type AgentOverlayResetApi, type ChromeWindowsApi, type TabMutationApi } from "../tabs"; +import type { ChromeTabsApi } from "../shared"; + +/** 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; @@ -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); @@ -221,3 +241,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/session.ts b/apps/extension/src/tools/session.ts index c83a2cd..2818a53 100644 --- a/apps/extension/src/tools/session.ts +++ b/apps/extension/src/tools/session.ts @@ -2,6 +2,7 @@ import type { SessionManager } from "@/session-manager/manager"; import type { RpcError } from "@/transport/types"; import { clearRecordingForSession } from "./record"; import { returnBorrowedTab, type TabManagementDeps } from "./tabs"; +import type { ChromeTabsApi } from "./shared"; export interface SessionStartParams { session_id: string; @@ -22,6 +23,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 { @@ -35,6 +42,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; } /** @@ -80,8 +94,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 @@ -162,8 +178,76 @@ export async function handleSessionStop( // Step 3: detach CDP sessions this session opened (no-op if none). await deps.cdp?.detachSession(params.session_id); - // 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 6bad610..6ef7e60 100644 --- a/apps/extension/src/tools/tabs.ts +++ b/apps/extension/src/tools/tabs.ts @@ -416,9 +416,18 @@ 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; + // 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, window_id: ctx.agentWindowId, @@ -516,6 +525,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",