diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index ac8904a..d75aca1 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -25,7 +25,7 @@ import { recordFrameCoordinator } from "@/lib/recording/frame-coordinator"; import { attachSessionsLiveFlag } from "@/lib/sessions-live-flag"; import { createDisconnectCleanup } from "@/session-manager/disconnect-cleanup"; import { attachSessionEventHandler } from "@/session-manager/event-handler"; -import { SessionManager } from "@/session-manager/manager"; +import { isAgentControlledTab, SessionManager } from "@/session-manager/manager"; import { attachBorrowNotificationButtonHandler, attachBorrowNotificationClickHandler, @@ -83,26 +83,21 @@ 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). + * Authoritative overlay state for a specific tab. Agent Window tabs are + * free by default; only tabs explicitly claimed through session startup, + * `tab_create`, or `tab_borrow` receive the control overlay. */ 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, - }; - } + if (ctx && isAgentControlledTab(ctx, tabId)) return overlayStateForWindow(windowId); } - return overlayStateForWindow(windowId); + return { + type: OVERLAY_AGENT_STATE, + sessionId: null, + mode: "hidden", + generation: overlayGeneration, + }; } async function pushOverlayStateToTab( @@ -122,20 +117,11 @@ export default defineBackground(() => { } async function pushOverlayStateForWindow(windowId: number): Promise { - const ctx = sessions.findByWindowId(windowId); - const baseState = overlayStateForWindow(windowId); const tabs = await chrome.tabs.query({ windowId }); await Promise.all( 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); + return pushOverlayStateForTab(tab.id, windowId); }), ); } @@ -175,27 +161,13 @@ 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. + // Newly observed tabs are free unless the agent's own creation path has + // already claimed their concrete id. This listener only renders that + // 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; - 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); - } + if (!sessions.findByWindowId(tab.windowId)) return; + void pushOverlayStateForTab(tab.id, 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 @@ -369,15 +341,18 @@ export default defineBackground(() => { if (!msg || typeof msg !== "object" || !("kind" in msg)) return false; if (msg.kind === OVERLAY_MSG_WHO_AM_I) { + const tabId = sender.tab?.id; const windowId = sender.tab?.windowId; const ctx = typeof windowId === "number" ? sessions.findByWindowId(windowId) : null; - sendResponse({ sessionId: ctx?.sessionId ?? null }); + const sessionId = + ctx && typeof tabId === "number" && isAgentControlledTab(ctx, tabId) ? ctx.sessionId : null; + sendResponse({ sessionId }); return false; } if (msg.kind === OVERLAY_MSG_READY) { - // Decide per-tab *before* sending: a user-created tab gets hidden - // immediately so the content script never flashes the control mask. + // Resolve from explicit tab ownership before sending so free tabs never + // flash the control mask. if (typeof sender.tab?.id === "number") { void pushOverlayStateForTab(sender.tab.id, sender.tab.windowId); } diff --git a/apps/extension/src/session-manager/__tests__/manager.test.ts b/apps/extension/src/session-manager/__tests__/manager.test.ts index 46d44be..fbbbf7e 100644 --- a/apps/extension/src/session-manager/__tests__/manager.test.ts +++ b/apps/extension/src/session-manager/__tests__/manager.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import type { AgentWindowApi, AgentWindowCreateOptions } from "../agent-window"; -import { SessionManager } from "../manager"; +import { isAgentControlledTab, SessionManager } from "../manager"; function fakeAgentWindow(): AgentWindowApi & { createMock: ReturnType; @@ -151,50 +151,29 @@ describe("SessionManager", () => { expect(sm.list()).toEqual([]); }); - describe("classifyNewTab (user vs agent tab freedom)", () => { - it("classifies the home tab as agent", async () => { + describe("explicit tab claims", () => { + it("claims the exact home tab returned by AgentWindowApi", 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); + expect(ctx.agentCreatedTabs).toEqual(new Set([0])); + expect(isAgentControlledTab(ctx, 0)).toBe(true); }); - it("classifies a pending tab_create tab as agent", async () => { + it("keeps every other tab free without classification state", 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); + expect(isAgentControlledTab(ctx, 99)).toBe(false); + expect(ctx).not.toHaveProperty("userTabs"); + expect(ctx).not.toHaveProperty("pendingAgentTabCount"); }); - it("classifies a user-opened tab (no pending) as user and keeps it free", async () => { + it("treats a committed borrow as an explicit claim", 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"); + const reservation = sm.tryReserveBorrow(42, ctx.sessionId); + if ("borrowedBy" in reservation) throw new Error("unexpected borrow conflict"); + reservation.commit({ tabId: 42, originalWindowId: 7, originalIndex: 3 }); + expect(isAgentControlledTab(ctx, 42)).toBe(true); }); }); diff --git a/apps/extension/src/session-manager/manager.ts b/apps/extension/src/session-manager/manager.ts index 5c4f530..d721c41 100644 --- a/apps/extension/src/session-manager/manager.ts +++ b/apps/extension/src/session-manager/manager.ts @@ -7,40 +7,19 @@ export interface SessionContext { 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. + * Tabs explicitly claimed by the agent because it created them. This + * includes the Agent Window's home tab and tabs created by `tool.tab_create`. + * Tabs opened through 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; } +/** Whether this session has explicitly claimed control of `tabId`. */ +export function isAgentControlledTab(ctx: SessionContext, tabId: number): boolean { + return ctx.agentCreatedTabs.has(tabId) || ctx.borrowedTabs.has(tabId); +} + export interface BorrowedTab { tabId: number; originalWindowId: number; @@ -219,10 +198,10 @@ export class SessionManager { agentWindowId: windowId, refStore: new RefStore(), borrowedTabs: new Map(), - agentCreatedTabs: new Set(), - userTabs: new Set(), - pendingAgentTabCount: 0, - homeTabId, + // The home tab is the session's first explicit claim. Every other + // tab remains free until `tab_create` or `tab_borrow` identifies it + // by its concrete Chrome tab id. + agentCreatedTabs: new Set([homeTabId]), createdAtMs: this.now(), }; this.sessions.set(sessionId, ctx); @@ -240,50 +219,6 @@ 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__/dispatcher.test.ts b/apps/extension/src/tools/__tests__/dispatcher.test.ts index dc5e421..6d7dceb 100644 --- a/apps/extension/src/tools/__tests__/dispatcher.test.ts +++ b/apps/extension/src/tools/__tests__/dispatcher.test.ts @@ -196,9 +196,8 @@ describe("ToolDispatcher", () => { }, }); await sessions.start("aa11"); - // Register tab 7 as agent-created (pending counter + onCreated classify). - sessions.markAgentTabPending(4242); - sessions.classifyNewTab(7, 4242); + // Register the exact tab id returned by the agent's creation path. + sessions.get("aa11")?.agentCreatedTabs.add(7); const dispatcher = new ToolDispatcher({ transport, sessions }); dispatcher.start(); diff --git a/apps/extension/src/tools/__tests__/emulate.test.ts b/apps/extension/src/tools/__tests__/emulate.test.ts index d8e27af..3782fdc 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 () => 1), + ensureActiveTab: vi.fn(async () => 7), }; } @@ -197,6 +197,7 @@ describe("handleEmulate", () => { it("targets an explicit tab_id", async () => { const sm = await makeManager(); + sm.get("aa11")?.agentCreatedTabs.add(9); const deps = makeDeps(); const result = await handleEmulate( sm, @@ -276,6 +277,7 @@ describe("handleEmulate", () => { it("keeps emulation state per tab", async () => { const sm = await makeManager(); + sm.get("aa11")?.agentCreatedTabs.add(9); const deps = makeDeps(); await handleEmulate(sm, { session_id: "aa11", overrides: fullOverrides }, deps); await handleEmulate( diff --git a/apps/extension/src/tools/__tests__/evaluate.test.ts b/apps/extension/src/tools/__tests__/evaluate.test.ts index 0065499..d9c352d 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 () => 1), + ensureActiveTab: vi.fn(async () => 4), }; } diff --git a/apps/extension/src/tools/__tests__/human-loop.test.ts b/apps/extension/src/tools/__tests__/human-loop.test.ts index a500735..f5a0f18 100644 --- a/apps/extension/src/tools/__tests__/human-loop.test.ts +++ b/apps/extension/src/tools/__tests__/human-loop.test.ts @@ -52,10 +52,16 @@ function installHelpLifecycleChrome() { function fakeManager(sessionId: string, agentWindowId: number, tabId: number) { const refStore = new RefStore(); + const session = { + sessionId, + agentWindowId, + refStore, + borrowedTabs: new Map(), + agentCreatedTabs: new Set([tabId]), + }; const mgr = { - get: (id: string) => - id === sessionId ? { sessionId, agentWindowId, refStore, borrowedTabs: new Map() } : null, - findByWindowId: (wid: number) => (wid === agentWindowId ? { sessionId } : null), + get: (id: string) => (id === sessionId ? session : null), + findByWindowId: (wid: number) => (wid === agentWindowId ? session : null), } as unknown as SessionManager; return mgr; } @@ -357,6 +363,7 @@ describe("handleRequestHelp", () => { agentWindowId: 99, refStore, borrowedTabs: new Map(), + agentCreatedTabs: new Set([5]), } : null, findByWindowId: (wid: number) => (wid === 99 ? { sessionId: "abcd" } : null), @@ -413,6 +420,7 @@ describe("handleRequestHelp", () => { }), }, borrowedTabs: new Map(), + agentCreatedTabs: new Set([5]), } : null, findByWindowId: (wid: number) => (wid === 99 ? { sessionId: "abcd" } : null), @@ -498,6 +506,7 @@ describe("handleRequestHelp", () => { agentWindowId: 99, refStore, borrowedTabs: new Map(), + agentCreatedTabs: new Set([5]), } : null, findByWindowId: (wid: number) => (wid === 99 ? { sessionId: "abcd" } : null), diff --git a/apps/extension/src/tools/__tests__/interaction.test.ts b/apps/extension/src/tools/__tests__/interaction.test.ts index 45ab74e..bf4313b 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 () => 1), + ensureActiveTab: vi.fn(async () => 4), }; } @@ -113,6 +113,7 @@ describe("handleClick", () => { const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); const ctx = await sm.start("aa11"); ctx.refStore.set("e3", 1234, { tabId: 4 }); + ctx.agentCreatedTabs.add(5); const fake = makeFakeCdp({}); const res = await handleClick( sm, diff --git a/apps/extension/src/tools/__tests__/navigation.test.ts b/apps/extension/src/tools/__tests__/navigation.test.ts index 921771e..5e08f55 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 () => 1), + ensureActiveTab: vi.fn(async () => 4), }; } diff --git a/apps/extension/src/tools/__tests__/observation.test.ts b/apps/extension/src/tools/__tests__/observation.test.ts index 0763262..d5a5855 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 () => 1), + ensureActiveTab: vi.fn(async () => 4), }; } diff --git a/apps/extension/src/tools/__tests__/record-steps.test.ts b/apps/extension/src/tools/__tests__/record-steps.test.ts index f933b49..e37a793 100644 --- a/apps/extension/src/tools/__tests__/record-steps.test.ts +++ b/apps/extension/src/tools/__tests__/record-steps.test.ts @@ -84,6 +84,7 @@ function fakeManager() { agentWindowId: AGENT_WINDOW_ID, refStore: { resolve: () => null, replace: () => {} }, borrowedTabs: new Map(), + agentCreatedTabs: new Set([TAB_ID]), } : null, findByWindowId: (windowId: number) => diff --git a/apps/extension/src/tools/__tests__/session.test.ts b/apps/extension/src/tools/__tests__/session.test.ts index 57e53e3..4853939 100644 --- a/apps/extension/src/tools/__tests__/session.test.ts +++ b/apps/extension/src/tools/__tests__/session.test.ts @@ -302,7 +302,7 @@ describe("handleSessionStop window release (issue #57)", () => { ctx.agentCreatedTabs.add(11); ctx.agentCreatedTabs.add(12); // home tab id from ensureActiveTab override - ctx.homeTabId = 10; + ctx.agentCreatedTabs.add(10); // home tab // user tab 99 (created via Chrome UI, NOT in agentCreatedTabs) const state: FakeState = { tabs: new Map([ @@ -340,7 +340,7 @@ describe("handleSessionStop window release (issue #57)", () => { const sm = new SessionManager({ agentWindow: aw }); const ctx = await sm.start("aa11"); ctx.agentCreatedTabs.add(11); - ctx.homeTabId = 10; + ctx.agentCreatedTabs.add(10); // home tab const state: FakeState = { tabs: new Map([ [10, { id: 10, windowId: agentWindowId } as chrome.tabs.Tab], @@ -369,7 +369,7 @@ describe("handleSessionStop window release (issue #57)", () => { 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; + ctx.agentCreatedTabs.add(10); // home tab const state: FakeState = { tabs: new Map([[10, { id: 10, windowId: agentWindowId } as chrome.tabs.Tab]]), windowsClosed: new Set(), @@ -399,7 +399,7 @@ describe("handleSessionStop window release (issue #57)", () => { const sm = new SessionManager({ agentWindow: aw }); const ctx = await sm.start("aa11"); ctx.agentCreatedTabs.add(11); - ctx.homeTabId = 10; + ctx.agentCreatedTabs.add(10); // home tab const state: FakeState = { tabs: new Map([ [10, { id: 10, windowId: agentWindowId } as chrome.tabs.Tab], @@ -427,7 +427,7 @@ describe("handleSessionStop window release (issue #57)", () => { const sm = new SessionManager({ agentWindow: aw }); const ctx = await sm.start("aa11"); ctx.agentCreatedTabs.add(11); - ctx.homeTabId = 10; + ctx.agentCreatedTabs.add(10); // home tab const state: FakeState = { tabs: new Map([[11, { id: 11, windowId: agentWindowId } as chrome.tabs.Tab]]), windowsClosed: new Set(), @@ -473,7 +473,7 @@ describe("handleSessionStop window release (issue #57)", () => { 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; + ctx.agentCreatedTabs.add(10); // home tab const state: FakeState = { tabs: new Map([ [10, { id: 10, windowId: agentWindowId } as chrome.tabs.Tab], diff --git a/apps/extension/src/tools/__tests__/tabs.test.ts b/apps/extension/src/tools/__tests__/tabs.test.ts index a44fe47..4a120a1 100644 --- a/apps/extension/src/tools/__tests__/tabs.test.ts +++ b/apps/extension/src/tools/__tests__/tabs.test.ts @@ -60,19 +60,24 @@ describe("handleTabList", () => { expect(res.tabs.map((t) => t.scope).sort()).toEqual(["agent", "user"]); }); - it("scope=user excludes the requesting session's Agent Window", async () => { + it("scope=user includes unclaimed tabs inside the Agent Window", async () => { const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); await sm.start("aa11"); const tabs = fakeChromeTabsApi([ { windowId: 100, url: "chrome://newtab/", title: "Agent New Tab", active: true }, + { windowId: 100, url: "https://user.example/", title: "Free user tab", active: false }, { windowId: 200, url: "https://example.com", title: "Example", active: true }, { windowId: 200, url: "https://github.com", title: "GitHub", active: false }, ]); const res = await handleTabList(sm, { session_id: "aa11", scope: "user" }, tabs); if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); - expect(res.tabs).toHaveLength(2); + expect(res.tabs).toHaveLength(3); expect(res.tabs.every((t) => t.scope === "user")).toBe(true); - expect(res.tabs.map((t) => t.url)).toEqual(["https://example.com", "https://github.com"]); + expect(res.tabs.map((t) => t.url)).toEqual([ + "https://user.example/", + "https://example.com", + "https://github.com", + ]); }); it("scope=agent returns only the requesting session's Agent Window tabs", async () => { @@ -221,6 +226,7 @@ describe("handleTabCreate", () => { active: true, }); expect(res).toMatchObject({ tab_id: 10, window_id: 100 }); + expect(sm.get("aa11")?.agentCreatedTabs.has(10)).toBe(true); }); it("forwards explicit url + index + active=false", async () => { @@ -257,7 +263,7 @@ describe("handleTabCreate", () => { expect(res).toMatchObject({ code: "invalid_params" }); }); - it("releases the pending agent-tab slot when chrome.tabs.create fails", async () => { + it("does not claim any tab 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() }; @@ -267,18 +273,37 @@ describe("handleTabCreate", () => { 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"); + expect(sm.get("aa11")?.agentCreatedTabs).toEqual(new Set([1])); + }); + + it("claims the returned id even when a user tab appears during tab creation", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + const ctx = await sm.start("aa11"); + const state: FakeTabState = { tabs: new Map(), nextTabId: 10, windowsClosed: new Set() }; + const { api, spies } = makeTabMutationApi(state); + let resolveCreate!: (tab: chrome.tabs.Tab) => void; + const createResult = new Promise((resolve) => { + resolveCreate = resolve; + }); + spies.create.mockImplementationOnce(() => createResult); + + const pending = handleTabCreate(sm, { session_id: "aa11" }, { tabs: api }); + // A user can create an unrelated same-window tab while Chrome is still + // resolving the agent request. There is no pending slot for it to steal. + expect(ctx.agentCreatedTabs.has(99)).toBe(false); + resolveCreate({ id: 10, windowId: 100, active: true } as chrome.tabs.Tab); + + await expect(pending).resolves.toMatchObject({ tab_id: 10 }); + expect(ctx.agentCreatedTabs.has(10)).toBe(true); + expect(ctx.agentCreatedTabs.has(99)).toBe(false); }); }); describe("handleTabClose", () => { it("removes a tab inside the Agent Window", async () => { const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); - await sm.start("aa11"); + const ctx = await sm.start("aa11"); + ctx.agentCreatedTabs.add(5); const state: FakeTabState = { tabs: new Map([[5, { id: 5, windowId: 100 } as chrome.tabs.Tab]]), nextTabId: 50, @@ -291,6 +316,23 @@ describe("handleTabClose", () => { expect(spies.remove).toHaveBeenCalledWith(5); }); + it("refuses to close a free tab in the Agent Window", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + await sm.start("aa11"); + const state: FakeTabState = { + tabs: new Map([[5, { id: 5, windowId: 100 } as chrome.tabs.Tab]]), + nextTabId: 50, + windowsClosed: new Set(), + }; + const { api, spies } = makeTabMutationApi(state); + const res = await handleTabClose(sm, { session_id: "aa11", tab_id: 5 }, { tabs: api }); + expect(res).toMatchObject({ + code: "permission_denied", + data: { reason: "agent_window_scope" }, + }); + expect(spies.remove).not.toHaveBeenCalled(); + }); + it("rejects closing a borrowed tab — caller must tab_return first", async () => { const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); const ctx = await sm.start("aa11"); @@ -341,7 +383,8 @@ describe("handleTabClose", () => { describe("handleTabSelect", () => { it("activates a tab inside the Agent Window", async () => { const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); - await sm.start("aa11"); + const ctx = await sm.start("aa11"); + ctx.agentCreatedTabs.add(3); const state: FakeTabState = { tabs: new Map([[3, { id: 3, windowId: 100 } as chrome.tabs.Tab]]), nextTabId: 50, diff --git a/apps/extension/src/tools/__tests__/waits.test.ts b/apps/extension/src/tools/__tests__/waits.test.ts index ce0f3aa..81346be 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 () => 1), + ensureActiveTab: vi.fn(async () => 4), }; } diff --git a/apps/extension/src/tools/session.ts b/apps/extension/src/tools/session.ts index 165f874..b3af268 100644 --- a/apps/extension/src/tools/session.ts +++ b/apps/extension/src/tools/session.ts @@ -269,7 +269,8 @@ export async function handleSessionStop( return { code: "cancelled", message: "session_stop aborted before window close" }; } - // Step 4: close the agent-created tabs and the home tab. `tabsApi` is a + // 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 // ChromeTabsApi. If neither is injected, we conservatively fall back to // closing the window (see Step 5). @@ -277,7 +278,7 @@ export async function handleSessionStop( const queryApi = deps.tabsQuery; if (tabsApi) { - // 4a: close each agent-created tab that still exists. + // Close each agent-created tab that still exists. const agentCreatedTabIds = Array.from(ctx.agentCreatedTabs); for (const tabId of agentCreatedTabIds) { try { @@ -288,16 +289,6 @@ export async function handleSessionStop( 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. @@ -312,7 +303,6 @@ export async function handleSessionStop( // 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( diff --git a/apps/extension/src/tools/shared.ts b/apps/extension/src/tools/shared.ts index 3b2cb79..829ba0f 100644 --- a/apps/extension/src/tools/shared.ts +++ b/apps/extension/src/tools/shared.ts @@ -8,7 +8,11 @@ import type { CdpDebuggee, DialogCursor } from "@/browser-driver/chromium-cdp"; import type { CdpFrameGraph, CdpTarget } from "@/browser-driver/frame-graph"; -import type { SessionContext, SessionManager } from "@/session-manager/manager"; +import { + isAgentControlledTab, + type SessionContext, + type SessionManager, +} from "@/session-manager/manager"; import { normaliseRef } from "@/session-manager/ref-store"; import type { ConsoleResult, JavaScriptDialogInfo, RpcError } from "@/transport/types"; import { rpcError } from "./errors"; @@ -323,23 +327,22 @@ 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). + * Sandbox guard: M7 write tools (click / fill / press / navigate*) must + * refuse tabs the session has not explicitly claimed. A controlled tab must + * also remain inside the session's Agent Window. * - * Returns an `RpcError` when the resolved target sits in a user window; - * `null` on success. + * Returns an `RpcError` for free tabs and tabs outside the Agent Window. */ export function enforceAgentWindow( ctx: SessionContext, target: { tabId: number; windowId: number }, toolName: string, ): RpcError | null { - if (target.windowId !== ctx.agentWindowId) { + if (target.windowId !== ctx.agentWindowId || !isAgentControlledTab(ctx, target.tabId)) { 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 claimed by this session inside the Agent Window (tab ${target.tabId} is free or outside window ${ctx.agentWindowId}; borrow it first)`, ); } return null; diff --git a/apps/extension/src/tools/tabs.ts b/apps/extension/src/tools/tabs.ts index c84c098..1192fa6 100644 --- a/apps/extension/src/tools/tabs.ts +++ b/apps/extension/src/tools/tabs.ts @@ -7,7 +7,11 @@ import { OVERLAY_AGENT_OVERLAY_RESET, type OverlayAgentOverlayResetMessage, } from "@/lib/overlay-bridge"; -import type { SessionContext, SessionManager } from "@/session-manager/manager"; +import { + isAgentControlledTab, + type SessionContext, + type SessionManager, +} from "@/session-manager/manager"; import type { RpcError } from "@/transport/types"; import { rpcError } from "./errors"; import { isRpcError, lookupSession } from "./shared"; @@ -25,11 +29,9 @@ export interface TabInfo { window_id?: number; active?: boolean; /** - * Where the tab sits relative to the requesting session: tabs in any - * window other than an Agent Window are `user`; tabs in this - * session's own Agent Window are `agent`. Tabs in *other* sessions' - * Agent Windows are filtered out entirely (cross-session isolation, - * design §6). + * Ownership relative to the requesting session. Explicitly created or + * borrowed tabs in this session's Agent Window are `agent`; every other + * visible tab is `user`. Other sessions' Agent Windows are filtered out. */ scope?: "user" | "agent"; } @@ -259,7 +261,8 @@ 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 tabScope: "user" | "agent" = + winId === myAgentWindowId && isAgentControlledTab(ctx, t.id) ? "agent" : "user"; if (scope === "user" && tabScope !== "user") continue; if (scope === "agent" && tabScope !== "agent") continue; tabs.push({ @@ -369,6 +372,7 @@ function buildCreateProps( * Returns the created tab on success, or an `RpcError` on failure. */ async function createTabAndCleanup( + ctx: SessionContext, deps: TabManagementDeps, createProps: chrome.tabs.CreateProperties, ): Promise { @@ -381,25 +385,28 @@ async function createTabAndCleanup( message: err instanceof Error ? err.message : String(err), }; } - if (aborted(deps.signal, "tab_create")) { - // We already opened the tab; close it on abort so we don't leak. - if (typeof tab.id === "number") { - try { - await getTabsApi(deps).remove(tab.id); - } catch (cleanupErr) { - return rpcError( - "protocol_error", - "cleanup_failed", - `tab_create aborted but cleanup of tab ${tab.id} failed: ${describeError(cleanupErr)}`, - { resource_type: "tab", resource_id: tab.id }, - ); - } - } - return { code: "cancelled", message: "tab_create aborted" }; - } if (typeof tab.id !== "number") { return { code: "protocol_error", message: "chrome.tabs.create returned no tab id" }; } + // 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 (aborted(deps.signal, "tab_create")) { + try { + 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. + return rpcError( + "protocol_error", + "cleanup_failed", + `tab_create aborted but cleanup of tab ${tab.id} failed: ${describeError(cleanupErr)}`, + { resource_type: "tab", resource_id: tab.id }, + ); + } + return { code: "cancelled", message: "tab_create aborted" }; + } return { ...tab, id: tab.id }; } @@ -426,27 +433,8 @@ 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)) { - // 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); + const tab = await createTabAndCleanup(ctx, deps, buildCreateProps(ctx, params)); + if (isRpcError(tab)) return tab; return { tab_id: tab.id, @@ -496,13 +484,13 @@ async function authoriseAgentTab( `${toolName}: tab ${tabId} is borrowed by session ${otherBorrower}`, ); } - if (tab.windowId === ctx.agentWindowId) { + if (tab.windowId === ctx.agentWindowId && isAgentControlledTab(ctx, tab.id)) { return tab; } return rpcError( "permission_denied", "agent_window_scope", - `${toolName}: tab ${tabId} is not in Agent Window ${ctx.agentWindowId}`, + `${toolName}: tab ${tabId} is not claimed by this session in Agent Window ${ctx.agentWindowId}`, ); } @@ -775,13 +763,6 @@ async function executeBorrowCore( ); if (moveErr) return moveErr; - // Best-effort activation — failure is non-fatal. - try { - await p.tabsApi.update(p.tabId, { active: true }); - } catch (err) { - console.debug("[bsk tab_borrow] activate after move failed", err); - } - return { originalWindowId, originalIndex }; } @@ -845,6 +826,12 @@ export async function handleTabBorrow( message: err instanceof Error ? err.message : String(err), }; } + // Activate after commit so overlay/event observers see the tab as claimed. + try { + await tabsApi.update(params.tab_id, { active: true }); + } catch (err) { + console.debug("[bsk tab_borrow] activate after move failed", err); + } return { tab_id: params.tab_id, original_window_id: coreResult.originalWindowId,