mirror of
https://github.com/Tencent/BrowserSkill.git
synced 2026-09-10 13:01:23 +08:00
fix(extension): classify the home tab by id, not a racy boot flag (issue #57)
SessionManager.classifyNewTab's `initializing` branch relied on a windowInitializing flag that start() cleared *before* the home tab's chrome.tabs.onCreated event was dispatched (the event is asynchronous), so the branch never ran in production and the home tab's classification was subject to a timing race. Match the home tab by its tab id instead — the id is already returned by ensureActiveTab and is reliable regardless of event ordering. Also refresh the overlay-bridge docstring: overlay.ready now makes the background *push* the per-tab overlay state (and the Agent Window also pushes on tab create / control-mode change), rather than a request/reply.
This commit is contained in:
@@ -175,10 +175,11 @@ 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.
|
||||
// A new tab inside an Agent Window is agent-owned (the home tab, matched by
|
||||
// homeTabId, or a tool.tab_create flagged by the pending count) or
|
||||
// user-created (via Chrome UI). Classify it so user-opened tabs are kept
|
||||
// free and can be pushed a hidden overlay immediately. See
|
||||
// SessionManager.classifyNewTab.
|
||||
chrome.tabs.onCreated.addListener((tab) => {
|
||||
if (typeof tab.windowId !== "number" || typeof tab.id !== "number") return;
|
||||
const kind = sessions.classifyNewTab(tab.id, tab.windowId);
|
||||
@@ -190,7 +191,7 @@ export default defineBackground(() => {
|
||||
mode: "hidden",
|
||||
generation: overlayGeneration,
|
||||
});
|
||||
} else if (kind === "agent" || kind === "initializing") {
|
||||
} else if (kind === "agent") {
|
||||
// Agent tab (or home tab): make sure it reflects the session's
|
||||
// current control mode.
|
||||
void pushOverlayStateForAgentWindow(tab.windowId);
|
||||
|
||||
@@ -3,8 +3,10 @@
|
||||
* between the content-script control overlay and the background SW.
|
||||
*
|
||||
* Content script → background:
|
||||
* - `{ kind: "overlay.ready" }` → background replies with the authoritative
|
||||
* overlay state for the sender's window.
|
||||
* - `{ kind: "overlay.ready" }` → background decides the per-tab state first
|
||||
* (see overlayStateForTab) and pushes the authoritative overlay state for
|
||||
* that tab. It also proactively pushes on tab create and control-mode
|
||||
* change, so a tab usually receives state before it ever sends ready.
|
||||
* - `{ kind: "overlay.interrupt", sessionId }` → background asks the
|
||||
* daemon (via a `session.user_interrupt` WS event) to cancel
|
||||
* every inflight + queued tool call for that session with
|
||||
|
||||
@@ -152,19 +152,17 @@ describe("SessionManager", () => {
|
||||
});
|
||||
|
||||
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 });
|
||||
it("classifies the home tab as agent", async () => {
|
||||
const sm = new SessionManager({ agentWindow: fakeAgentWindow() });
|
||||
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<number> }).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);
|
||||
// The home tab is agent-owned and matched by home tab id. The prior
|
||||
// windowInitializing flag was cleared before the home tab's onCreated
|
||||
// arrived, so matching it by boot timing was never reliable.
|
||||
const homeTabId = ctx.homeTabId as number;
|
||||
const kind = sm.classifyNewTab(homeTabId, ctx.agentWindowId);
|
||||
expect(kind).toBe("agent");
|
||||
expect(ctx.agentCreatedTabs.has(homeTabId)).toBe(true);
|
||||
expect(ctx.userTabs.has(homeTabId)).toBe(false);
|
||||
});
|
||||
|
||||
it("classifies a pending tab_create tab as agent", async () => {
|
||||
|
||||
@@ -113,13 +113,6 @@ export class SessionManager {
|
||||
private readonly sessions = new Map<string, SessionContext>();
|
||||
private readonly windowIndex = new Map<number, string>();
|
||||
private readonly borrowReservations = new Map<number, string>();
|
||||
/**
|
||||
* 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<number>();
|
||||
private readonly agentWindow: AgentWindowApi;
|
||||
private readonly now: () => number;
|
||||
|
||||
@@ -218,9 +211,6 @@ export class SessionManager {
|
||||
const { signal: _signal, ...createOptions } = opts;
|
||||
windowId = await this.agentWindow.create(AGENT_WINDOW_HOME, createOptions);
|
||||
throwIfSessionStartAborted(opts.signal);
|
||||
// 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);
|
||||
throwIfSessionStartAborted(opts.signal);
|
||||
|
||||
@@ -237,11 +227,9 @@ export class SessionManager {
|
||||
};
|
||||
this.sessions.set(sessionId, ctx);
|
||||
this.windowIndex.set(windowId, sessionId);
|
||||
this.windowInitializing.delete(windowId);
|
||||
return ctx;
|
||||
} catch (startupError) {
|
||||
if (windowId !== null) {
|
||||
this.windowInitializing.delete(windowId);
|
||||
try {
|
||||
await this.agentWindow.remove(windowId);
|
||||
} catch (cleanupError) {
|
||||
@@ -277,20 +265,18 @@ export class SessionManager {
|
||||
|
||||
/**
|
||||
* 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.
|
||||
* 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" | "initializing" | "unknown" {
|
||||
classifyNewTab(tabId: number, windowId: number): "agent" | "user" | "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;
|
||||
// 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";
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user