Skip to content

Commit 06a229f

Browse files
vignesh07vignesh07
committed
fix(browser): close tracked tabs on session cleanup (#36666)
1 parent 6dfd39c commit 06a229f

8 files changed

Lines changed: 412 additions & 1 deletion

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ Docs: https://docs.openclaw.ai
2727

2828
### Fixes
2929

30+
- 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.
3031
- 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.
3132
- 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.
3233
- 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.

‎src/agents/openclaw-tools.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ export function createOpenClawTools(options?: {
129129
createBrowserTool({
130130
sandboxBridgeUrl: options?.sandboxBrowserBridgeUrl,
131131
allowHostControl: options?.allowHostBrowserControl,
132+
agentSessionKey: options?.agentSessionKey,
132133
}),
133134
createCanvasTool({ config: options?.config }),
134135
createNodesTool({

‎src/agents/tools/browser-tool.test.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@ const configMocks = vi.hoisted(() => ({
8282
}));
8383
vi.mock("../../config/config.js", () => configMocks);
8484

85+
const sessionTabRegistryMocks = vi.hoisted(() => ({
86+
trackSessionBrowserTab: vi.fn(),
87+
untrackSessionBrowserTab: vi.fn(),
88+
}));
89+
vi.mock("../../browser/session-tab-registry.js", () => sessionTabRegistryMocks);
90+
8591
const toolCommonMocks = vi.hoisted(() => ({
8692
imageResultFromFile: vi.fn(),
8793
}));
@@ -292,6 +298,23 @@ describe("browser tool url alias support", () => {
292298
);
293299
});
294300

301+
it("tracks opened tabs when session context is available", async () => {
302+
browserClientMocks.browserOpenTab.mockResolvedValueOnce({
303+
targetId: "tab-123",
304+
title: "Example",
305+
url: "https://example.com",
306+
});
307+
const tool = createBrowserTool({ agentSessionKey: "agent:main:main" });
308+
await tool.execute?.("call-1", { action: "open", url: "https://example.com" });
309+
310+
expect(sessionTabRegistryMocks.trackSessionBrowserTab).toHaveBeenCalledWith({
311+
sessionKey: "agent:main:main",
312+
targetId: "tab-123",
313+
baseUrl: undefined,
314+
profile: undefined,
315+
});
316+
});
317+
295318
it("accepts url alias for navigate", async () => {
296319
const tool = createBrowserTool();
297320
await tool.execute?.("call-1", {
@@ -317,6 +340,26 @@ describe("browser tool url alias support", () => {
317340
"targetUrl required",
318341
);
319342
});
343+
344+
it("untracks explicit tab close for tracked sessions", async () => {
345+
const tool = createBrowserTool({ agentSessionKey: "agent:main:main" });
346+
await tool.execute?.("call-1", {
347+
action: "close",
348+
targetId: "tab-xyz",
349+
});
350+
351+
expect(browserClientMocks.browserCloseTab).toHaveBeenCalledWith(
352+
undefined,
353+
"tab-xyz",
354+
expect.objectContaining({ profile: undefined }),
355+
);
356+
expect(sessionTabRegistryMocks.untrackSessionBrowserTab).toHaveBeenCalledWith({
357+
sessionKey: "agent:main:main",
358+
targetId: "tab-xyz",
359+
baseUrl: undefined,
360+
profile: undefined,
361+
});
362+
});
320363
});
321364

322365
describe("browser tool act compatibility", () => {

‎src/agents/tools/browser-tool.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ import {
1919
import { resolveBrowserConfig } from "../../browser/config.js";
2020
import { DEFAULT_UPLOAD_DIR, resolveExistingPathsWithinRoot } from "../../browser/paths.js";
2121
import { applyBrowserProxyPaths, persistBrowserProxyFiles } from "../../browser/proxy-files.js";
22+
import {
23+
trackSessionBrowserTab,
24+
untrackSessionBrowserTab,
25+
} from "../../browser/session-tab-registry.js";
2226
import { loadConfig } from "../../config/config.js";
2327
import {
2428
executeActAction,
@@ -275,6 +279,7 @@ function resolveBrowserBaseUrl(params: {
275279
export function createBrowserTool(opts?: {
276280
sandboxBridgeUrl?: string;
277281
allowHostControl?: boolean;
282+
agentSessionKey?: string;
278283
}): AnyAgentTool {
279284
const targetDefault = opts?.sandboxBridgeUrl ? "sandbox" : "host";
280285
const hostHint =
@@ -418,7 +423,14 @@ export function createBrowserTool(opts?: {
418423
});
419424
return jsonResult(result);
420425
}
421-
return jsonResult(await browserOpenTab(baseUrl, targetUrl, { profile }));
426+
const opened = await browserOpenTab(baseUrl, targetUrl, { profile });
427+
trackSessionBrowserTab({
428+
sessionKey: opts?.agentSessionKey,
429+
targetId: opened.targetId,
430+
baseUrl,
431+
profile,
432+
});
433+
return jsonResult(opened);
422434
}
423435
case "focus": {
424436
const targetId = readStringParam(params, "targetId", {
@@ -455,6 +467,12 @@ export function createBrowserTool(opts?: {
455467
}
456468
if (targetId) {
457469
await browserCloseTab(baseUrl, targetId, { profile });
470+
untrackSessionBrowserTab({
471+
sessionKey: opts?.agentSessionKey,
472+
targetId,
473+
baseUrl,
474+
profile,
475+
});
458476
} else {
459477
await browserAct(baseUrl, { kind: "close" }, { profile });
460478
}

‎src/browser/session-tab-registry.test.ts‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
import {
3+
__countTrackedSessionBrowserTabsForTests,
4+
__resetTrackedSessionBrowserTabsForTests,
5+
closeTrackedBrowserTabsForSessions,
6+
trackSessionBrowserTab,
7+
untrackSessionBrowserTab,
8+
} from "./session-tab-registry.js";
9+
10+
describe("session tab registry", () => {
11+
beforeEach(() => {
12+
__resetTrackedSessionBrowserTabsForTests();
13+
});
14+
15+
afterEach(() => {
16+
__resetTrackedSessionBrowserTabsForTests();
17+
});
18+
19+
it("tracks and closes tabs for normalized session keys", async () => {
20+
trackSessionBrowserTab({
21+
sessionKey: "Agent:Main:Main",
22+
targetId: "tab-a",
23+
baseUrl: "http://127.0.0.1:9222",
24+
profile: "OpenClaw",
25+
});
26+
trackSessionBrowserTab({
27+
sessionKey: "agent:main:main",
28+
targetId: "tab-b",
29+
baseUrl: "http://127.0.0.1:9222",
30+
profile: "OpenClaw",
31+
});
32+
expect(__countTrackedSessionBrowserTabsForTests("agent:main:main")).toBe(2);
33+
34+
const closeTab = vi.fn(async () => {});
35+
const closed = await closeTrackedBrowserTabsForSessions({
36+
sessionKeys: ["agent:main:main"],
37+
closeTab,
38+
});
39+
40+
expect(closed).toBe(2);
41+
expect(closeTab).toHaveBeenCalledTimes(2);
42+
expect(closeTab).toHaveBeenNthCalledWith(1, {
43+
targetId: "tab-a",
44+
baseUrl: "http://127.0.0.1:9222",
45+
profile: "openclaw",
46+
});
47+
expect(closeTab).toHaveBeenNthCalledWith(2, {
48+
targetId: "tab-b",
49+
baseUrl: "http://127.0.0.1:9222",
50+
profile: "openclaw",
51+
});
52+
expect(__countTrackedSessionBrowserTabsForTests()).toBe(0);
53+
});
54+
55+
it("untracks specific tabs", async () => {
56+
trackSessionBrowserTab({
57+
sessionKey: "agent:main:main",
58+
targetId: "tab-a",
59+
});
60+
trackSessionBrowserTab({
61+
sessionKey: "agent:main:main",
62+
targetId: "tab-b",
63+
});
64+
untrackSessionBrowserTab({
65+
sessionKey: "agent:main:main",
66+
targetId: "tab-a",
67+
});
68+
69+
const closeTab = vi.fn(async () => {});
70+
const closed = await closeTrackedBrowserTabsForSessions({
71+
sessionKeys: ["agent:main:main"],
72+
closeTab,
73+
});
74+
75+
expect(closed).toBe(1);
76+
expect(closeTab).toHaveBeenCalledTimes(1);
77+
expect(closeTab).toHaveBeenCalledWith({
78+
targetId: "tab-b",
79+
baseUrl: undefined,
80+
profile: undefined,
81+
});
82+
});
83+
84+
it("deduplicates tabs and ignores expected close errors", async () => {
85+
trackSessionBrowserTab({
86+
sessionKey: "agent:main:main",
87+
targetId: "tab-a",
88+
});
89+
trackSessionBrowserTab({
90+
sessionKey: "main",
91+
targetId: "tab-a",
92+
});
93+
trackSessionBrowserTab({
94+
sessionKey: "main",
95+
targetId: "tab-b",
96+
});
97+
const warnings: string[] = [];
98+
const closeTab = vi
99+
.fn()
100+
.mockRejectedValueOnce(new Error("target not found"))
101+
.mockRejectedValueOnce(new Error("network down"));
102+
103+
const closed = await closeTrackedBrowserTabsForSessions({
104+
sessionKeys: ["agent:main:main", "main"],
105+
closeTab,
106+
onWarn: (message) => warnings.push(message),
107+
});
108+
109+
expect(closed).toBe(0);
110+
expect(closeTab).toHaveBeenCalledTimes(2);
111+
expect(warnings).toEqual([expect.stringContaining("network down")]);
112+
expect(__countTrackedSessionBrowserTabsForTests()).toBe(0);
113+
});
114+
});

0 commit comments

Comments
 (0)