Skip to content

Commit 0e51404

Browse files
steipetesteipete
authored
fix(browser): disambiguate restart tab aliases
1 parent f4a9f39 commit 0e51404

33 files changed

Lines changed: 2359 additions & 1869 deletions

‎docs/docs_map.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9602,6 +9602,7 @@ Do not edit it by hand; run `pnpm docs:map:gen`.
96029602
- H2: Missing browser command or tool
96039603
- H2: Profiles: openclaw, user, chrome
96049604
- H2: Configuration
9605+
- H3: Tab cleanup ownership
96059606
- H3: Screenshot vision (text-only model support)
96069607
- H2: Use Brave or another Chromium-based browser
96079608
- H2: Local vs remote control

‎docs/gateway/configuration-reference.md‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -457,15 +457,17 @@ See [Inferred commitments](/concepts/commitments).
457457
`maxTabsPerSession: 0` to disable those individual cleanup modes. Disabling
458458
`tabCleanup` does not disable explicit session lifecycle cleanup.
459459
- Host-local opens with a stable native CDP target and browser identity are
460-
stored in shared SQLite state and can remain eligible across Gateway restarts
461-
for `/new`, session lifecycle, idle, and cap cleanup. This includes
462-
OpenClaw-managed and regular remote CDP profiles, plus existing-session
463-
profiles with an explicit `cdpUrl`, when stable identity is available.
464-
OpenClaw verifies the profile and browser instance before closing. Chrome MCP
465-
auto-connect, missing `/json/version` browser identity, and unresolved native
466-
targets remain process-local, so they are not automatically closed after a
467-
restart. Older untracked tabs require manual closure. Transient failures stay
468-
pending for a later retry. See [Tab cleanup ownership](/tools/browser#tab-cleanup-ownership).
460+
stored in shared SQLite state and remain eligible across Gateway restarts for
461+
`/new` and session lifecycle cleanup. Native tool-facing CDP targets also
462+
remain eligible for idle and cap cleanup after restart. Chrome MCP uses
463+
process-local target handles, so cold existing-session records wait for
464+
lifecycle cleanup rather than risking an idle sweep against unattributable
465+
post-restart activity. OpenClaw verifies the profile and browser instance
466+
before closing. Chrome MCP auto-connect, missing `/json/version` browser
467+
identity, and unresolved native targets remain fully process-local, so they
468+
are not automatically closed after a restart. Older untracked tabs require
469+
manual closure. Transient failures stay pending for a later retry. See
470+
[Tab cleanup ownership](/tools/browser#tab-cleanup-ownership).
469471
- `ssrfPolicy.dangerouslyAllowPrivateNetwork` is disabled when unset, so browser navigation stays strict by default.
470472
- Set `ssrfPolicy.dangerouslyAllowPrivateNetwork: true` only when you intentionally trust private-network browser navigation.
471473
- In strict mode, remote CDP profile endpoints (`profiles.*.cdpUrl`) are subject to the same private-network blocking during reachability/discovery checks.

‎docs/tools/browser.md‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,17 @@ sessions; disabling it does not disable explicit session lifecycle cleanup.
215215

216216
For host-local opens, ownership with a stable native CDP target and browser
217217
identity is stored in the shared SQLite state. Those records survive a Gateway
218-
restart and remain eligible for `/new`, other session lifecycle cleanup, and
219-
the idle and per-session cap sweeps. Session lifecycle cleanup includes
220-
subagent, cron, and ACP session endings. This durable path can cover
221-
OpenClaw-managed profiles, regular remote CDP profiles, and existing-session
222-
profiles with an explicit `cdpUrl`, provided OpenClaw can resolve both the
223-
native target and a stable browser identity. Before closing a durable record,
224-
OpenClaw verifies that the configured profile and browser instance still match.
218+
restart and remain eligible for `/new` and other session lifecycle cleanup;
219+
session lifecycle cleanup includes subagent, cron, and ACP session endings.
220+
Records whose tool-facing target is the native CDP target also remain eligible
221+
for idle and per-session cap sweeps after restart. Chrome MCP target handles are
222+
process-local, so cold existing-session records wait for lifecycle cleanup
223+
rather than risking an idle sweep against activity that cannot be attributed
224+
safely after restart. This durable path can cover OpenClaw-managed profiles,
225+
regular remote CDP profiles, and existing-session profiles with an explicit
226+
`cdpUrl`, provided OpenClaw can resolve both the native target and a stable
227+
browser identity. Before closing a durable record, OpenClaw verifies that the
228+
configured profile and browser instance still match.
225229

226230
Chrome MCP `--autoConnect`, CDP endpoints whose `/json/version` response lacks
227231
a stable browser identity, and opens whose native target cannot be resolved

‎extensions/browser/src/browser-runtime-state.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { PluginStateSyncKeyedStore } from "openclaw/plugin-sdk/plugin-state
22
// Browser plugin runtime state shared across lazy bundles and duplicate SDK module instances.
33
import { createPluginRuntimeStore } from "openclaw/plugin-sdk/runtime-store";
44

5-
export type BrowserStateRuntime = {
5+
type BrowserStateRuntime = {
66
sessionTabs: PluginStateSyncKeyedStore<unknown>;
77
};
88

‎extensions/browser/src/browser-tool-session-tabs.ts‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ type SessionTabParams = {
88
targetId?: string;
99
baseUrl?: string;
1010
profile?: string;
11+
profileAliases?: Array<string | undefined>;
1112
ownership?: BrowserTabOwnership;
1213
aliases?: Array<string | undefined>;
1314
};
@@ -62,33 +63,39 @@ export function stripBrowserOpenInternalMetadata(value: unknown): unknown {
6263
return agentVisible;
6364
}
6465

65-
async function trackOpenedHostBrowserTab(params: {
66+
async function trackOpenedBrowserTab(params: {
6667
result: unknown;
6768
sessionKey?: string;
68-
fallbackProfile: string;
69+
fallbackProfile?: string;
70+
baseUrl?: string;
6971
track: SessionTabRegistry["trackSessionBrowserTab"];
70-
closeTab: (targetId: string, profile: string) => Promise<void>;
72+
closeTab: (targetId: string, profile?: string) => Promise<void>;
7173
}): Promise<void> {
7274
const opened = readOpenedTab(params.result);
7375
const profile = opened.profile ?? params.fallbackProfile;
7476
try {
7577
params.track({
7678
sessionKey: params.sessionKey,
7779
targetId: opened.targetId,
78-
baseUrl: undefined,
80+
baseUrl: params.baseUrl,
7981
profile,
80-
ownership: opened.ownership,
82+
...(params.fallbackProfile && opened.profile && opened.profile !== params.fallbackProfile
83+
? { profileAliases: [params.fallbackProfile] }
84+
: {}),
85+
// Sandbox/browser-bridge tabs belong to a different browser process.
86+
// Keep them process-local even if that server returned durable metadata.
87+
ownership: params.baseUrl ? undefined : opened.ownership,
8188
aliases: opened.aliases,
8289
});
8390
} catch (trackingError) {
84-
if (opened.ownership?.status !== "durable" || !opened.targetId) {
91+
if (!opened.targetId) {
8592
throw trackingError;
8693
}
8794
try {
8895
await params.closeTab(opened.targetId, profile);
8996
} catch (closeError) {
9097
throw Object.assign(
91-
new Error("Failed to persist browser tab ownership and close the newly opened tab", {
98+
new Error("Failed to register browser tab cleanup and close the newly opened tab", {
9299
cause: closeError,
93100
}),
94101
{
@@ -110,36 +117,39 @@ export function createBrowserToolSessionTabs(params: {
110117
registry: SessionTabRegistry;
111118
}) {
112119
const profile = params.requestedProfile ?? params.defaultProfile;
113-
const isTrackedHostRoute = () =>
114-
!params.baseUrl && (!params.isHostFallbackActive || params.isHostFallbackActive());
120+
const isTrackedRoute = () => !params.isHostFallbackActive || params.isHostFallbackActive();
121+
const trackedBaseUrl = () => (params.isHostFallbackActive ? undefined : params.baseUrl);
122+
const trackedProfile = () => (trackedBaseUrl() && !params.requestedProfile ? undefined : profile);
115123
const identity = (targetId: string) => ({
116124
sessionKey: params.sessionKey,
117125
targetId,
118-
baseUrl: undefined,
119-
profile,
126+
baseUrl: trackedBaseUrl(),
127+
profile: trackedProfile(),
120128
});
121129
return {
122130
touch: (targetId: string | undefined): void => {
123-
if (targetId && isTrackedHostRoute()) {
131+
if (targetId && isTrackedRoute()) {
124132
params.registry.touchSessionBrowserTab(identity(targetId));
125133
}
126134
},
127135
untrack: (targetId: string | undefined): void => {
128-
if (targetId && isTrackedHostRoute()) {
136+
if (targetId && isTrackedRoute()) {
129137
params.registry.untrackSessionBrowserTab(identity(targetId));
130138
}
131139
},
132140
trackOpened: async (
133141
result: unknown,
134-
closeTab: (targetId: string, openedProfile: string) => Promise<void>,
142+
closeTab: (targetId: string, openedProfile?: string) => Promise<void>,
135143
): Promise<void> => {
136-
if (!isTrackedHostRoute()) {
144+
if (!isTrackedRoute()) {
137145
return;
138146
}
139-
await trackOpenedHostBrowserTab({
147+
const baseUrl = trackedBaseUrl();
148+
await trackOpenedBrowserTab({
140149
result,
141150
sessionKey: params.sessionKey,
142-
fallbackProfile: profile,
151+
fallbackProfile: baseUrl && !params.requestedProfile ? undefined : profile,
152+
baseUrl,
143153
track: params.registry.trackSessionBrowserTab,
144154
closeTab,
145155
});

‎extensions/browser/src/browser-tool.test.ts‎

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,6 +1062,7 @@ describe("browser tool snapshot maxChars", () => {
10621062
targetId: "host-tab-opened",
10631063
baseUrl: undefined,
10641064
profile: "host-actual",
1065+
profileAliases: ["openclaw"],
10651066
ownership: {
10661067
status: "durable",
10671068
nativeTargetId: "HOST-NATIVE-7",
@@ -1815,6 +1816,7 @@ describe("browser tool url alias support", () => {
18151816
targetId: "tab-123",
18161817
baseUrl: undefined,
18171818
profile: "hot-profile",
1819+
profileAliases: ["openclaw"],
18181820
ownership: {
18191821
status: "durable",
18201822
nativeTargetId: "NATIVE-123",
@@ -1855,6 +1857,40 @@ describe("browser tool url alias support", () => {
18551857
expect(browserClientMocks.browserCloseTab).not.toHaveBeenCalled();
18561858
});
18571859

1860+
it("closes a newly opened non-durable tab when process tracking fails", async () => {
1861+
const trackingError = new Error("tracking unavailable");
1862+
browserClientMocks.browserOpenTab.mockResolvedValueOnce({
1863+
targetId: "tab-volatile-compensate",
1864+
resolvedProfile: "work-actual",
1865+
title: "Example",
1866+
url: "https://example.com",
1867+
ownership: {
1868+
status: "non-durable",
1869+
reason: "browser-identity-lookup-failed",
1870+
},
1871+
});
1872+
sessionTabRegistryMocks.trackSessionBrowserTab.mockImplementationOnce(() => {
1873+
throw trackingError;
1874+
});
1875+
const tool = createBrowserTool({ agentSessionKey: "agent:main:main" });
1876+
1877+
await expect(
1878+
tool.execute?.("call-1", {
1879+
action: "open",
1880+
profile: "work",
1881+
url: "https://example.com",
1882+
}),
1883+
).rejects.toBe(trackingError);
1884+
expect(browserClientMocks.browserCloseTab).toHaveBeenCalledWith(
1885+
undefined,
1886+
"tab-volatile-compensate",
1887+
{
1888+
profile: "work-actual",
1889+
timeoutMs: undefined,
1890+
},
1891+
);
1892+
});
1893+
18581894
it("does not persist durable ownership from a legacy open result without resolved profile", async () => {
18591895
browserClientMocks.browserOpenTab.mockResolvedValueOnce({
18601896
targetId: "legacy-tab",
@@ -1943,7 +1979,7 @@ describe("browser tool url alias support", () => {
19431979

19441980
expect(error).toMatchObject({
19451981
name: "BrowserTabTrackingCompensationError",
1946-
message: "Failed to persist browser tab ownership and close the newly opened tab",
1982+
message: "Failed to register browser tab cleanup and close the newly opened tab",
19471983
});
19481984
const errors = (error as Error & { errors: unknown[] }).errors;
19491985
expect(errors[0]).toBe(trackingError);
@@ -1954,7 +1990,7 @@ describe("browser tool url alias support", () => {
19541990
}
19551991
});
19561992

1957-
it("does not track or compensate sandbox open results", async () => {
1993+
it("keeps legacy sandbox opens process-local without inventing a host profile", async () => {
19581994
browserClientMocks.browserOpenTab.mockResolvedValueOnce({
19591995
targetId: "sandbox-tab",
19601996
title: "Sandbox",
@@ -1977,7 +2013,15 @@ describe("browser tool url alias support", () => {
19772013
url: "https://example.com",
19782014
});
19792015

1980-
expect(sessionTabRegistryMocks.trackSessionBrowserTab).not.toHaveBeenCalled();
2016+
expect(sessionTabRegistryMocks.trackSessionBrowserTab).toHaveBeenCalledWith(
2017+
expect.objectContaining({
2018+
sessionKey: "agent:main:main",
2019+
targetId: "sandbox-tab",
2020+
baseUrl: "http://127.0.0.1:9999",
2021+
profile: undefined,
2022+
ownership: undefined,
2023+
}),
2024+
);
19812025
expect(browserClientMocks.browserCloseTab).not.toHaveBeenCalled();
19822026
expect(result?.details).not.toHaveProperty("ownership");
19832027
});

‎extensions/browser/src/browser-tool.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ export function createBrowserTool(opts?: {
642642
body: { url: targetUrl, ...(label ? { label } : {}) },
643643
timeoutMs: toolTimeoutMs,
644644
});
645-
const closeOpenedTab = async (targetId: string, openedProfile: string) => {
645+
const closeOpenedTab = async (targetId: string, openedProfile?: string) => {
646646
await proxyRequest({
647647
method: "DELETE",
648648
path: `/tabs/${encodeURIComponent(targetId)}`,
@@ -658,7 +658,7 @@ export function createBrowserTool(opts?: {
658658
label,
659659
timeoutMs: toolTimeoutMs,
660660
});
661-
const closeOpenedTab = async (targetId: string, openedProfile: string) => {
661+
const closeOpenedTab = async (targetId: string, openedProfile?: string) => {
662662
await browserToolDeps.browserCloseTab(baseUrl, targetId, {
663663
profile: openedProfile,
664664
timeoutMs: toolTimeoutMs,
@@ -682,11 +682,11 @@ export function createBrowserTool(opts?: {
682682
sessionTabs.touch(targetId);
683683
return jsonResult(result);
684684
}
685-
await browserToolDeps.browserFocusTab(baseUrl, targetId, {
685+
const result = await browserToolDeps.browserFocusTab(baseUrl, targetId, {
686686
profile,
687687
timeoutMs: toolTimeoutMs,
688688
});
689-
sessionTabs.touch(targetId);
689+
sessionTabs.touch(readStringValue(result.targetId) ?? targetId);
690690
return jsonResult({ ok: true });
691691
}
692692
case "close": {

‎extensions/browser/src/browser/cdp.close-tracked.test.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { createServer } from "node:http";
2+
import type { AddressInfo } from "node:net";
3+
import { afterEach, describe, expect, it } from "vitest";
4+
import { type WebSocket, WebSocketServer } from "ws";
5+
import { rawDataToString } from "../infra/ws.js";
6+
import "../test-support/browser-security.mock.js";
7+
import { closeTrackedCdpTarget, resolveCdpTabOwnership } from "./cdp.helpers.js";
8+
9+
const servers: Array<{ close: (callback: () => void) => void }> = [];
10+
11+
async function listen(server: {
12+
once: (event: string, callback: () => void) => void;
13+
}): Promise<void> {
14+
await new Promise<void>((resolve) => {
15+
server.once("listening", resolve);
16+
});
17+
}
18+
19+
function replyToCloseMessages(socket: WebSocket): void {
20+
socket.on("message", (data) => {
21+
const message = JSON.parse(rawDataToString(data)) as { id?: number; method?: string };
22+
if (message.method === "Target.getTargets") {
23+
socket.send(
24+
JSON.stringify({
25+
id: message.id,
26+
result: { targetInfos: [{ targetId: "OWNED", type: "page" }] },
27+
}),
28+
);
29+
} else if (message.method === "Target.closeTarget") {
30+
socket.send(JSON.stringify({ id: message.id, result: { success: false } }));
31+
}
32+
});
33+
}
34+
35+
afterEach(async () => {
36+
await Promise.all(
37+
servers.splice(0).map(
38+
(server) =>
39+
new Promise<void>((resolve) => {
40+
server.close(resolve);
41+
}),
42+
),
43+
);
44+
});
45+
46+
describe("closeTrackedCdpTarget", () => {
47+
it("keeps ownership retryable when CDP declines the close", async () => {
48+
const wsServer = new WebSocketServer({ port: 0, host: "127.0.0.1" });
49+
servers.push(wsServer);
50+
await listen(wsServer);
51+
wsServer.on("connection", replyToCloseMessages);
52+
53+
const browserWebSocketUrl = `ws://127.0.0.1:${(wsServer.address() as AddressInfo).port}/devtools/browser/TEST`;
54+
const httpServer = createServer((_, response) => {
55+
response.setHeader("content-type", "application/json");
56+
response.end(JSON.stringify({ webSocketDebuggerUrl: browserWebSocketUrl }));
57+
});
58+
servers.push(httpServer);
59+
httpServer.listen(0, "127.0.0.1");
60+
await listen(httpServer);
61+
const cdpUrl = `http://127.0.0.1:${(httpServer.address() as AddressInfo).port}`;
62+
const ownership = await resolveCdpTabOwnership({
63+
profileName: "remote",
64+
cdpUrl,
65+
nativeTargetId: "OWNED",
66+
});
67+
if (ownership.status !== "durable") {
68+
throw new Error("expected durable ownership");
69+
}
70+
71+
await expect(
72+
closeTrackedCdpTarget({
73+
profileName: "remote",
74+
cdpUrl,
75+
nativeTargetId: "OWNED",
76+
expectedProfileFingerprint: ownership.profileFingerprint,
77+
expectedBrowserInstanceFingerprint: ownership.browserInstanceFingerprint,
78+
}),
79+
).resolves.toEqual({ status: "unavailable", reason: "target-close-failed" });
80+
});
81+
});

0 commit comments

Comments
 (0)