diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c5bfcdc420..4f40bfe65f6 100644 --- 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. diff --git a/src/agents/openclaw-tools.ts b/src/agents/openclaw-tools.ts index 4373bf83c4b..6dc694c6350 100644 --- 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({ diff --git a/src/agents/tools/browser-tool.test.ts b/src/agents/tools/browser-tool.test.ts index eaaec53f10c..3c54cb63633 100644 --- 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", () => { diff --git a/src/agents/tools/browser-tool.ts b/src/agents/tools/browser-tool.ts index 520b21f021c..80faf99a1e4 100644 --- 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 }); } diff --git a/src/browser/session-tab-registry.test.ts b/src/browser/session-tab-registry.test.ts new file mode 100644 index 00000000000..2abdcd34462 --- /dev/null +++ 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); + }); +}); diff --git a/src/browser/session-tab-registry.ts b/src/browser/session-tab-registry.ts new file mode 100644 index 00000000000..b81ceac3060 --- /dev/null +++ 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>(); + +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, +): TrackedSessionBrowserTab[] { + const uniqueSessionKeys = new Set(); + for (const key of sessionKeys) { + if (!key?.trim()) { + continue; + } + uniqueSessionKeys.add(normalizeSessionKey(key)); + } + if (uniqueSessionKeys.size === 0) { + return []; + } + const seenTrackedIds = new Set(); + 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; + closeTab?: (tab: { targetId: string; baseUrl?: string; profile?: string }) => Promise; + onWarn?: (message: string) => void; +}): Promise { + 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; +} diff --git a/src/gateway/server-methods/sessions.ts b/src/gateway/server-methods/sessions.ts index 69d49aab348..523e6655d71 100644 --- 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; sessionId?: string; }) { + const closeTrackedBrowserTabs = async () => { + const closeKeys = new Set([ + params.key, + params.target.canonicalKey, + ...params.target.storeKeys, + params.sessionId ?? "", + ]); + return await closeTrackedBrowserTabsForSessions({ + sessionKeys: [...closeKeys], + onWarn: (message) => logVerbose(message), + }); + }; + const queueKeys = new Set(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( diff --git a/src/gateway/server.sessions.gateway-server-sessions-a.test.ts b/src/gateway/server.sessions.gateway-server-sessions-a.test.ts index 90b8e656b7e..3780174cee0 100644 --- 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( @@ -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(); + 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,