fix(browser): close tracked tabs on session cleanup (#36666)

2026-03-12 07:20:45 +00:00 · 2026-03-05 16:36:29 -08:00
parent f771ba8de9
commit 0de5d27d29
8 changed files with 412 additions and 1 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,6 +27,7 @@ Docs: https://docs.openclaw.ai

 ### Fixes

+- Browser/session cleanup: track browser tabs opened by session-scoped browser tool runs and close tracked tabs during `sessions.reset`/`sessions.delete` runtime cleanup, preventing orphaned tabs and unbounded browser memory growth after session teardown. (#36666) Thanks @Harnoor6693.
 - Slack/local file upload allowlist parity: propagate `mediaLocalRoots` through the Slack send action pipeline so workspace-rooted attachments pass `assertLocalMediaAllowed` checks while non-allowlisted paths remain blocked. (synthesis: #36656; overlap considered from #36516, #36496, #36493, #36484, #32648, #30888) Thanks @2233admin.
 - Agents/compaction safeguard pre-check: skip embedded compaction before entering the Pi SDK when a session has no real conversation messages, avoiding unnecessary LLM API calls on idle sessions. (#36451) thanks @Sid-Qin.
 - Config/schema cache key stability: build merged schema cache keys with incremental hashing to avoid large single-string serialization and prevent `RangeError: Invalid string length` on high-cardinality plugin/channel metadata. (#36603) Thanks @powermaster888.
--- a/src/agents/openclaw-tools.ts
+++ b/src/agents/openclaw-tools.ts
@@ -129,6 +129,7 @@ export function createOpenClawTools(options?: {
    createBrowserTool({
      sandboxBridgeUrl: options?.sandboxBrowserBridgeUrl,
      allowHostControl: options?.allowHostBrowserControl,
+      agentSessionKey: options?.agentSessionKey,
    }),
    createCanvasTool({ config: options?.config }),
    createNodesTool({
--- a/src/agents/tools/browser-tool.test.ts
+++ b/src/agents/tools/browser-tool.test.ts
@@ -82,6 +82,12 @@ const configMocks = vi.hoisted(() => ({
 }));
 vi.mock("../../config/config.js", () => configMocks);

+const sessionTabRegistryMocks = vi.hoisted(() => ({
+  trackSessionBrowserTab: vi.fn(),
+  untrackSessionBrowserTab: vi.fn(),
+}));
+vi.mock("../../browser/session-tab-registry.js", () => sessionTabRegistryMocks);
+
 const toolCommonMocks = vi.hoisted(() => ({
  imageResultFromFile: vi.fn(),
 }));
@@ -292,6 +298,23 @@ describe("browser tool url alias support", () => {
    );
  });

+  it("tracks opened tabs when session context is available", async () => {
+    browserClientMocks.browserOpenTab.mockResolvedValueOnce({
+      targetId: "tab-123",
+      title: "Example",
+      url: "https://example.com",
+    });
+    const tool = createBrowserTool({ agentSessionKey: "agent:main:main" });
+    await tool.execute?.("call-1", { action: "open", url: "https://example.com" });
+
+    expect(sessionTabRegistryMocks.trackSessionBrowserTab).toHaveBeenCalledWith({
+      sessionKey: "agent:main:main",
+      targetId: "tab-123",
+      baseUrl: undefined,
+      profile: undefined,
+    });
+  });
+
  it("accepts url alias for navigate", async () => {
    const tool = createBrowserTool();
    await tool.execute?.("call-1", {
@@ -317,6 +340,26 @@ describe("browser tool url alias support", () => {
      "targetUrl required",
    );
  });
+
+  it("untracks explicit tab close for tracked sessions", async () => {
+    const tool = createBrowserTool({ agentSessionKey: "agent:main:main" });
+    await tool.execute?.("call-1", {
+      action: "close",
+      targetId: "tab-xyz",
+    });
+
+    expect(browserClientMocks.browserCloseTab).toHaveBeenCalledWith(
+      undefined,
+      "tab-xyz",
+      expect.objectContaining({ profile: undefined }),
+    );
+    expect(sessionTabRegistryMocks.untrackSessionBrowserTab).toHaveBeenCalledWith({
+      sessionKey: "agent:main:main",
+      targetId: "tab-xyz",
+      baseUrl: undefined,
+      profile: undefined,
+    });
+  });
 });

 describe("browser tool act compatibility", () => {
--- a/src/agents/tools/browser-tool.ts
+++ b/src/agents/tools/browser-tool.ts
@@ -19,6 +19,10 @@ import {
 import { resolveBrowserConfig } from "../../browser/config.js";
 import { DEFAULT_UPLOAD_DIR, resolveExistingPathsWithinRoot } from "../../browser/paths.js";
 import { applyBrowserProxyPaths, persistBrowserProxyFiles } from "../../browser/proxy-files.js";
+import {
+  trackSessionBrowserTab,
+  untrackSessionBrowserTab,
+} from "../../browser/session-tab-registry.js";
 import { loadConfig } from "../../config/config.js";
 import {
  executeActAction,
@@ -275,6 +279,7 @@ function resolveBrowserBaseUrl(params: {
 export function createBrowserTool(opts?: {
  sandboxBridgeUrl?: string;
  allowHostControl?: boolean;
+  agentSessionKey?: string;
 }): AnyAgentTool {
  const targetDefault = opts?.sandboxBridgeUrl ? "sandbox" : "host";
  const hostHint =
@@ -418,7 +423,14 @@ export function createBrowserTool(opts?: {
            });
            return jsonResult(result);
          }
-          return jsonResult(await browserOpenTab(baseUrl, targetUrl, { profile }));
+          const opened = await browserOpenTab(baseUrl, targetUrl, { profile });
+          trackSessionBrowserTab({
+            sessionKey: opts?.agentSessionKey,
+            targetId: opened.targetId,
+            baseUrl,
+            profile,
+          });
+          return jsonResult(opened);
        }
        case "focus": {
          const targetId = readStringParam(params, "targetId", {
@@ -455,6 +467,12 @@ export function createBrowserTool(opts?: {
          }
          if (targetId) {
            await browserCloseTab(baseUrl, targetId, { profile });
+            untrackSessionBrowserTab({
+              sessionKey: opts?.agentSessionKey,
+              targetId,
+              baseUrl,
+              profile,
+            });
          } else {
            await browserAct(baseUrl, { kind: "close" }, { profile });
          }
--- a/src/browser/session-tab-registry.test.ts
+++ b/src/browser/session-tab-registry.test.ts
@@ -0,0 +1,114 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import {
+  __countTrackedSessionBrowserTabsForTests,
+  __resetTrackedSessionBrowserTabsForTests,
+  closeTrackedBrowserTabsForSessions,
+  trackSessionBrowserTab,
+  untrackSessionBrowserTab,
+} from "./session-tab-registry.js";
+
+describe("session tab registry", () => {
+  beforeEach(() => {
+    __resetTrackedSessionBrowserTabsForTests();
+  });
+
+  afterEach(() => {
+    __resetTrackedSessionBrowserTabsForTests();
+  });
+
+  it("tracks and closes tabs for normalized session keys", async () => {
+    trackSessionBrowserTab({
+      sessionKey: "Agent:Main:Main",
+      targetId: "tab-a",
+      baseUrl: "http://127.0.0.1:9222",
+      profile: "OpenClaw",
+    });
+    trackSessionBrowserTab({
+      sessionKey: "agent:main:main",
+      targetId: "tab-b",
+      baseUrl: "http://127.0.0.1:9222",
+      profile: "OpenClaw",
+    });
+    expect(__countTrackedSessionBrowserTabsForTests("agent:main:main")).toBe(2);
+
+    const closeTab = vi.fn(async () => {});
+    const closed = await closeTrackedBrowserTabsForSessions({
+      sessionKeys: ["agent:main:main"],
+      closeTab,
+    });
+
+    expect(closed).toBe(2);
+    expect(closeTab).toHaveBeenCalledTimes(2);
+    expect(closeTab).toHaveBeenNthCalledWith(1, {
+      targetId: "tab-a",
+      baseUrl: "http://127.0.0.1:9222",
+      profile: "openclaw",
+    });
+    expect(closeTab).toHaveBeenNthCalledWith(2, {
+      targetId: "tab-b",
+      baseUrl: "http://127.0.0.1:9222",
+      profile: "openclaw",
+    });
+    expect(__countTrackedSessionBrowserTabsForTests()).toBe(0);
+  });
+
+  it("untracks specific tabs", async () => {
+    trackSessionBrowserTab({
+      sessionKey: "agent:main:main",
+      targetId: "tab-a",
+    });
+    trackSessionBrowserTab({
+      sessionKey: "agent:main:main",
+      targetId: "tab-b",
+    });
+    untrackSessionBrowserTab({
+      sessionKey: "agent:main:main",
+      targetId: "tab-a",
+    });
+
+    const closeTab = vi.fn(async () => {});
+    const closed = await closeTrackedBrowserTabsForSessions({
+      sessionKeys: ["agent:main:main"],
+      closeTab,
+    });
+
+    expect(closed).toBe(1);
+    expect(closeTab).toHaveBeenCalledTimes(1);
+    expect(closeTab).toHaveBeenCalledWith({
+      targetId: "tab-b",
+      baseUrl: undefined,
+      profile: undefined,
+    });
+  });
+
+  it("deduplicates tabs and ignores expected close errors", async () => {
+    trackSessionBrowserTab({
+      sessionKey: "agent:main:main",
+      targetId: "tab-a",
+    });
+    trackSessionBrowserTab({
+      sessionKey: "main",
+      targetId: "tab-a",
+    });
+    trackSessionBrowserTab({
+      sessionKey: "main",
+      targetId: "tab-b",
+    });
+    const warnings: string[] = [];
+    const closeTab = vi
+      .fn()
+      .mockRejectedValueOnce(new Error("target not found"))
+      .mockRejectedValueOnce(new Error("network down"));
+
+    const closed = await closeTrackedBrowserTabsForSessions({
+      sessionKeys: ["agent:main:main", "main"],
+      closeTab,
+      onWarn: (message) => warnings.push(message),
+    });
+
+    expect(closed).toBe(0);
+    expect(closeTab).toHaveBeenCalledTimes(2);
+    expect(warnings).toEqual([expect.stringContaining("network down")]);
+    expect(__countTrackedSessionBrowserTabsForTests()).toBe(0);
+  });
+});
--- a/src/browser/session-tab-registry.ts
+++ b/src/browser/session-tab-registry.ts
@@ -0,0 +1,189 @@
+import { browserCloseTab } from "./client.js";
+
+export type TrackedSessionBrowserTab = {
+  sessionKey: string;
+  targetId: string;
+  baseUrl?: string;
+  profile?: string;
+  trackedAt: number;
+};
+
+const trackedTabsBySession = new Map<string, Map<string, TrackedSessionBrowserTab>>();
+
+function normalizeSessionKey(raw: string): string {
+  return raw.trim().toLowerCase();
+}
+
+function normalizeTargetId(raw: string): string {
+  return raw.trim();
+}
+
+function normalizeProfile(raw?: string): string | undefined {
+  if (!raw) {
+    return undefined;
+  }
+  const trimmed = raw.trim();
+  return trimmed ? trimmed.toLowerCase() : undefined;
+}
+
+function normalizeBaseUrl(raw?: string): string | undefined {
+  if (!raw) {
+    return undefined;
+  }
+  const trimmed = raw.trim();
+  return trimmed ? trimmed : undefined;
+}
+
+function toTrackedTabId(params: { targetId: string; baseUrl?: string; profile?: string }): string {
+  return `${params.targetId}\u0000${params.baseUrl ?? ""}\u0000${params.profile ?? ""}`;
+}
+
+function isIgnorableCloseError(err: unknown): boolean {
+  const message = String(err).toLowerCase();
+  return (
+    message.includes("tab not found") ||
+    message.includes("target closed") ||
+    message.includes("target not found") ||
+    message.includes("no such target")
+  );
+}
+
+export function trackSessionBrowserTab(params: {
+  sessionKey?: string;
+  targetId?: string;
+  baseUrl?: string;
+  profile?: string;
+}): void {
+  const sessionKeyRaw = params.sessionKey?.trim();
+  const targetIdRaw = params.targetId?.trim();
+  if (!sessionKeyRaw || !targetIdRaw) {
+    return;
+  }
+  const sessionKey = normalizeSessionKey(sessionKeyRaw);
+  const targetId = normalizeTargetId(targetIdRaw);
+  const baseUrl = normalizeBaseUrl(params.baseUrl);
+  const profile = normalizeProfile(params.profile);
+  const tracked: TrackedSessionBrowserTab = {
+    sessionKey,
+    targetId,
+    baseUrl,
+    profile,
+    trackedAt: Date.now(),
+  };
+  const trackedId = toTrackedTabId(tracked);
+  let trackedForSession = trackedTabsBySession.get(sessionKey);
+  if (!trackedForSession) {
+    trackedForSession = new Map();
+    trackedTabsBySession.set(sessionKey, trackedForSession);
+  }
+  trackedForSession.set(trackedId, tracked);
+}
+
+export function untrackSessionBrowserTab(params: {
+  sessionKey?: string;
+  targetId?: string;
+  baseUrl?: string;
+  profile?: string;
+}): void {
+  const sessionKeyRaw = params.sessionKey?.trim();
+  const targetIdRaw = params.targetId?.trim();
+  if (!sessionKeyRaw || !targetIdRaw) {
+    return;
+  }
+  const sessionKey = normalizeSessionKey(sessionKeyRaw);
+  const trackedForSession = trackedTabsBySession.get(sessionKey);
+  if (!trackedForSession) {
+    return;
+  }
+  const trackedId = toTrackedTabId({
+    targetId: normalizeTargetId(targetIdRaw),
+    baseUrl: normalizeBaseUrl(params.baseUrl),
+    profile: normalizeProfile(params.profile),
+  });
+  trackedForSession.delete(trackedId);
+  if (trackedForSession.size === 0) {
+    trackedTabsBySession.delete(sessionKey);
+  }
+}
+
+function takeTrackedTabsForSessionKeys(
+  sessionKeys: Array<string | undefined>,
+): TrackedSessionBrowserTab[] {
+  const uniqueSessionKeys = new Set<string>();
+  for (const key of sessionKeys) {
+    if (!key?.trim()) {
+      continue;
+    }
+    uniqueSessionKeys.add(normalizeSessionKey(key));
+  }
+  if (uniqueSessionKeys.size === 0) {
+    return [];
+  }
+  const seenTrackedIds = new Set<string>();
+  const tabs: TrackedSessionBrowserTab[] = [];
+  for (const sessionKey of uniqueSessionKeys) {
+    const trackedForSession = trackedTabsBySession.get(sessionKey);
+    if (!trackedForSession || trackedForSession.size === 0) {
+      continue;
+    }
+    trackedTabsBySession.delete(sessionKey);
+    for (const tracked of trackedForSession.values()) {
+      const trackedId = toTrackedTabId(tracked);
+      if (seenTrackedIds.has(trackedId)) {
+        continue;
+      }
+      seenTrackedIds.add(trackedId);
+      tabs.push(tracked);
+    }
+  }
+  return tabs;
+}
+
+export async function closeTrackedBrowserTabsForSessions(params: {
+  sessionKeys: Array<string | undefined>;
+  closeTab?: (tab: { targetId: string; baseUrl?: string; profile?: string }) => Promise<void>;
+  onWarn?: (message: string) => void;
+}): Promise<number> {
+  const tabs = takeTrackedTabsForSessionKeys(params.sessionKeys);
+  if (tabs.length === 0) {
+    return 0;
+  }
+  const closeTab =
+    params.closeTab ??
+    (async (tab: { targetId: string; baseUrl?: string; profile?: string }) => {
+      await browserCloseTab(tab.baseUrl, tab.targetId, {
+        profile: tab.profile,
+      });
+    });
+  let closed = 0;
+  for (const tab of tabs) {
+    try {
+      await closeTab({
+        targetId: tab.targetId,
+        baseUrl: tab.baseUrl,
+        profile: tab.profile,
+      });
+      closed += 1;
+    } catch (err) {
+      if (!isIgnorableCloseError(err)) {
+        params.onWarn?.(`failed to close tracked browser tab ${tab.targetId}: ${String(err)}`);
+      }
+    }
+  }
+  return closed;
+}
+
+export function __resetTrackedSessionBrowserTabsForTests(): void {
+  trackedTabsBySession.clear();
+}
+
+export function __countTrackedSessionBrowserTabsForTests(sessionKey?: string): number {
+  if (typeof sessionKey === "string" && sessionKey.trim()) {
+    return trackedTabsBySession.get(normalizeSessionKey(sessionKey))?.size ?? 0;
+  }
+  let count = 0;
+  for (const tracked of trackedTabsBySession.values()) {
+    count += tracked.size;
+  }
+  return count;
+}
--- a/src/gateway/server-methods/sessions.ts
+++ b/src/gateway/server-methods/sessions.ts
@@ -6,6 +6,7 @@ import { clearBootstrapSnapshot } from "../../agents/bootstrap-cache.js";
 import { abortEmbeddedPiRun, waitForEmbeddedPiRunEnd } from "../../agents/pi-embedded.js";
 import { stopSubagentsForRequester } from "../../auto-reply/reply/abort.js";
 import { clearSessionQueues } from "../../auto-reply/reply/queue.js";
+import { closeTrackedBrowserTabsForSessions } from "../../browser/session-tab-registry.js";
 import { loadConfig } from "../../config/config.js";
 import {
  loadSessionStore,
@@ -186,6 +187,19 @@ async function ensureSessionRuntimeCleanup(params: {
  target: ReturnType<typeof resolveGatewaySessionStoreTarget>;
  sessionId?: string;
 }) {
+  const closeTrackedBrowserTabs = async () => {
+    const closeKeys = new Set<string>([
+      params.key,
+      params.target.canonicalKey,
+      ...params.target.storeKeys,
+      params.sessionId ?? "",
+    ]);
+    return await closeTrackedBrowserTabsForSessions({
+      sessionKeys: [...closeKeys],
+      onWarn: (message) => logVerbose(message),
+    });
+  };
+
  const queueKeys = new Set<string>(params.target.storeKeys);
  queueKeys.add(params.target.canonicalKey);
  if (params.sessionId) {
@@ -195,11 +209,13 @@ async function ensureSessionRuntimeCleanup(params: {
  clearBootstrapSnapshot(params.target.canonicalKey);
  stopSubagentsForRequester({ cfg: params.cfg, requesterSessionKey: params.target.canonicalKey });
  if (!params.sessionId) {
+    await closeTrackedBrowserTabs();
    return undefined;
  }
  abortEmbeddedPiRun(params.sessionId);
  const ended = await waitForEmbeddedPiRunEnd(params.sessionId, 15_000);
  if (ended) {
+    await closeTrackedBrowserTabs();
    return undefined;
  }
  return errorShape(
--- a/src/gateway/server.sessions.gateway-server-sessions-a.test.ts
+++ b/src/gateway/server.sessions.gateway-server-sessions-a.test.ts
@@ -44,6 +44,9 @@ const acpRuntimeMocks = vi.hoisted(() => ({
  getAcpRuntimeBackend: vi.fn(),
  requireAcpRuntimeBackend: vi.fn(),
 }));
+const browserSessionTabMocks = vi.hoisted(() => ({
+  closeTrackedBrowserTabsForSessions: vi.fn(async () => 0),
+}));

 vi.mock("../auto-reply/reply/queue.js", async () => {
  const actual = await vi.importActual<typeof import("../auto-reply/reply/queue.js")>(
@@ -111,6 +114,14 @@ vi.mock("../acp/runtime/registry.js", async (importOriginal) => {
  };
 });

+vi.mock("../browser/session-tab-registry.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../browser/session-tab-registry.js")>();
+  return {
+    ...actual,
+    closeTrackedBrowserTabsForSessions: browserSessionTabMocks.closeTrackedBrowserTabsForSessions,
+  };
+});
+
 installGatewayTestHooks({ scope: "suite" });

 let harness: GatewayServerHarness;
@@ -205,6 +216,8 @@ describe("gateway server sessions", () => {
    acpRuntimeMocks.requireAcpRuntimeBackend.mockImplementation((backendId?: string) =>
      acpRuntimeMocks.getAcpRuntimeBackend(backendId),
    );
+    browserSessionTabMocks.closeTrackedBrowserTabsForSessions.mockClear();
+    browserSessionTabMocks.closeTrackedBrowserTabsForSessions.mockResolvedValue(0);
  });

  test("lists and patches session store via sessions.* RPC", async () => {
@@ -694,6 +707,15 @@ describe("gateway server sessions", () => {
      ["discord:group:dev", "agent:main:discord:group:dev", "sess-active"],
      "sess-active",
    );
+    expect(browserSessionTabMocks.closeTrackedBrowserTabsForSessions).toHaveBeenCalledTimes(1);
+    expect(browserSessionTabMocks.closeTrackedBrowserTabsForSessions).toHaveBeenCalledWith({
+      sessionKeys: expect.arrayContaining([
+        "discord:group:dev",
+        "agent:main:discord:group:dev",
+        "sess-active",
+      ]),
+      onWarn: expect.any(Function),
+    });
    expect(subagentLifecycleHookMocks.runSubagentEnded).toHaveBeenCalledTimes(1);
    expect(subagentLifecycleHookMocks.runSubagentEnded).toHaveBeenCalledWith(
      {
@@ -925,6 +947,11 @@ describe("gateway server sessions", () => {
      ["main", "agent:main:main", "sess-main"],
      "sess-main",
    );
+    expect(browserSessionTabMocks.closeTrackedBrowserTabsForSessions).toHaveBeenCalledTimes(1);
+    expect(browserSessionTabMocks.closeTrackedBrowserTabsForSessions).toHaveBeenCalledWith({
+      sessionKeys: expect.arrayContaining(["main", "agent:main:main", "sess-main"]),
+      onWarn: expect.any(Function),
+    });
    expect(subagentLifecycleHookMocks.runSubagentEnded).toHaveBeenCalledTimes(1);
    expect(subagentLifecycleHookMocks.runSubagentEnded).toHaveBeenCalledWith(
      {
@@ -1153,6 +1180,7 @@ describe("gateway server sessions", () => {
      ["main", "agent:main:main", "sess-main"],
      "sess-main",
    );
+    expect(browserSessionTabMocks.closeTrackedBrowserTabsForSessions).not.toHaveBeenCalled();

    const store = JSON.parse(await fs.readFile(storePath, "utf-8")) as Record<
      string,
@@ -1194,6 +1222,7 @@ describe("gateway server sessions", () => {
      ["discord:group:dev", "agent:main:discord:group:dev", "sess-active"],
      "sess-active",
    );
+    expect(browserSessionTabMocks.closeTrackedBrowserTabsForSessions).not.toHaveBeenCalled();

    const store = JSON.parse(await fs.readFile(storePath, "utf-8")) as Record<
      string,