mirror of
https://github.com/Tencent/BrowserSkill.git
synced 2026-09-10 13:01:23 +08:00
fix(extension): claim agent tabs explicitly
This commit is contained in:
drakezhang
@@ -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<void> {
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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<typeof vi.fn>;
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -7,40 +7,19 @@ export interface SessionContext {
|
||||
refStore: RefStore;
|
||||
borrowedTabs: Map<number, BorrowedTab>;
|
||||
/**
|
||||
* 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<number>;
|
||||
/**
|
||||
* 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>;
|
||||
/**
|
||||
* 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.
|
||||
*
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -12,7 +12,7 @@ function fakeAgentWindow(ids: number[]) {
|
||||
return id;
|
||||
}),
|
||||
remove: vi.fn(async () => {}),
|
||||
ensureActiveTab: vi.fn(async () => 1),
|
||||
ensureActiveTab: vi.fn(async () => 4),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -21,7 +21,7 @@ function fakeAgentWindow(ids: number[]) {
|
||||
return id;
|
||||
}),
|
||||
remove: vi.fn(async () => {}),
|
||||
ensureActiveTab: vi.fn(async () => 1),
|
||||
ensureActiveTab: vi.fn(async () => 4),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -28,7 +28,7 @@ function fakeAgentWindow(ids: number[]) {
|
||||
return id;
|
||||
}),
|
||||
remove: vi.fn(async () => {}),
|
||||
ensureActiveTab: vi.fn(async () => 1),
|
||||
ensureActiveTab: vi.fn(async () => 4),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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) =>
|
||||
|
||||
@@ -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],
|
||||
|
||||
@@ -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<chrome.tabs.Tab>((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,
|
||||
|
||||
@@ -12,7 +12,7 @@ function fakeAgentWindow(ids: number[]) {
|
||||
return id;
|
||||
}),
|
||||
remove: vi.fn(async () => {}),
|
||||
ensureActiveTab: vi.fn(async () => 1),
|
||||
ensureActiveTab: vi.fn(async () => 4),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<CreatedChromeTab | RpcError> {
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user