Skip to content

Commit 069c7b8

Browse files
masatohoshinoclawsweeper[bot]Takhoffman
authored
fix(browser): thread snapshot timeoutMs through agent tool and helpers (#75702)
Summary: - Threads browser snapshot `timeoutMs` through the agent action, client/proxy request, snapshot route plan, Ch ... Playwright/CDP helpers, regression tests, changelog, and one JSDoc-only shrinkwrap script type annotation. - Reproducibility: yes. source reproduction is high-confidence: current main accepts top-level browser `timeou ... helpers drop it. I did not rerun the original macOS or Browserbase live scenario in this read-only review. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(browser): apply default snapshot timeout to proxy path and add Pl… - PR branch already contained follow-up commit before automerge: docs(changelog): add browser snapshot timeout propagation fix entry - PR branch already contained follow-up commit before automerge: fix(browser): thread snapshot timeoutMs through agent tool and helpers - PR branch already contained follow-up commit before automerge: fix(clawsweeper): address review for automerge-openclaw-openclaw-7570… Validation: - ClawSweeper review passed for head 0eec196. - Required merge gates passed before the squash merge. Prepared head SHA: 0eec196 Review: #75702 (comment) Co-authored-by: masatohoshino <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
1 parent d581415 commit 069c7b8

15 files changed

Lines changed: 384 additions & 7 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3259,6 +3259,7 @@ Docs: https://docs.openclaw.ai
32593259
- CLI/plugins: refresh persisted plugin registry policy in place for `plugins enable` and `plugins disable`, so routine toggles no longer rebuild and hash every plugin source when the target is already indexed. Thanks @vincentkoc.
32603260
- Windows/install: run npm from a writable installer temp directory and pin the Bedrock runtime dependency below a Windows ARM Node 24 npm resolver failure, so global OpenClaw installs no longer fail before onboarding. Thanks @mariozechner.
32613261
- CLI/plugins: scope install and enable slot selection to the selected plugin manifest/runtime fallback, so plugin installs no longer load every plugin runtime or broad status snapshot just to update memory/context slots. Thanks @vincentkoc.
3262+
- Browser/snapshot: propagate the configured snapshot timeout through the agent tool, Chrome MCP, and Playwright snapshot paths so snapshot actions honor the requested deadline instead of hanging. Fixes #72934. Thanks @masatohoshino.
32623263
- Plugins/TTS: keep bundled speech-provider discovery available on cold package Gateway paths and add bundled plugin matrix runtime probes for health, readiness, RPC, TTS discovery, and post-ready runtime-deps watchdog coverage. Refs #75283. Thanks @vincentkoc.
32633264
- Google Meet/Twilio: show delegated voice call ID, DTMF, and intro-greeting state in `googlemeet doctor`, and avoid claiming DTMF was sent when no Meet PIN sequence was configured. Refs #72478. Thanks @DougButdorf.
32643265
- Plugins/tools: prefer built bundled plugin code during tool discovery and skip channel runtime hydration while preserving companion provider registrations, reducing per-run plugin-tool prep cost without dropping executable plugin tools. Fixes #75290. Thanks @thanos-openclaw.

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import {
1616
resolveRuntimeImageSanitization,
1717
wrapExternalContent,
1818
} from "./browser-tool.runtime.js";
19-
import { DEFAULT_BROWSER_ACTION_TIMEOUT_MS } from "./browser/constants.js";
19+
import {
20+
DEFAULT_BROWSER_ACTION_TIMEOUT_MS,
21+
DEFAULT_BROWSER_SNAPSHOT_TIMEOUT_MS,
22+
} from "./browser/constants.js";
2023

2124
const browserToolActionDeps = {
2225
browserAct,
@@ -365,6 +368,8 @@ export async function executeSnapshotAction(params: {
365368
: hasMaxChars
366369
? maxChars
367370
: undefined;
371+
const snapshotTimeoutMs =
372+
normalizePositiveTimeoutMs(input.timeoutMs) ?? DEFAULT_BROWSER_SNAPSHOT_TIMEOUT_MS;
368373
const snapshotQuery = {
369374
...(format ? { format } : {}),
370375
targetId,
@@ -379,6 +384,7 @@ export async function executeSnapshotAction(params: {
379384
labels,
380385
urls,
381386
mode,
387+
timeoutMs: snapshotTimeoutMs,
382388
};
383389
let refsFallback: "role" | undefined;
384390
const readSnapshot = async (query: typeof snapshotQuery) =>
@@ -388,6 +394,7 @@ export async function executeSnapshotAction(params: {
388394
path: "/snapshot",
389395
profile,
390396
query,
397+
timeoutMs: snapshotTimeoutMs,
391398
})) as Awaited<ReturnType<typeof browserSnapshot>>)
392399
: await browserToolActionDeps.browserSnapshot(baseUrl, {
393400
...query,

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,86 @@ describe("browser tool snapshot maxChars", () => {
554554
expect(opts.refs).toBe("aria");
555555
});
556556

557+
it("propagates input.timeoutMs into the direct browser snapshot call", async () => {
558+
const tool = createBrowserTool();
559+
await tool.execute?.("call-1", {
560+
action: "snapshot",
561+
target: "host",
562+
snapshotFormat: "ai",
563+
timeoutMs: 9000,
564+
});
565+
566+
expect(browserClientMocks.browserSnapshot).toHaveBeenCalledWith(
567+
undefined,
568+
expect.objectContaining({
569+
format: "ai",
570+
timeoutMs: 9000,
571+
}),
572+
);
573+
});
574+
575+
it("falls back to the default snapshot timeout in the direct browser snapshot call", async () => {
576+
const tool = createBrowserTool();
577+
await tool.execute?.("call-1", {
578+
action: "snapshot",
579+
target: "host",
580+
snapshotFormat: "ai",
581+
});
582+
583+
expect(browserClientMocks.browserSnapshot).toHaveBeenCalledWith(
584+
undefined,
585+
expect.objectContaining({
586+
format: "ai",
587+
// DEFAULT_BROWSER_SNAPSHOT_TIMEOUT_MS = 20_000.
588+
timeoutMs: 20_000,
589+
}),
590+
);
591+
});
592+
593+
it("propagates input.timeoutMs into the proxied browser snapshot request", async () => {
594+
mockSingleBrowserProxyNode();
595+
setResolvedBrowserProfiles({
596+
user: { driver: "existing-session", attachOnly: true, color: "#00AA00" },
597+
});
598+
gatewayMocks.callGatewayTool.mockResolvedValue({
599+
ok: true,
600+
payload: {
601+
result: {
602+
ok: true,
603+
format: "ai",
604+
targetId: "t1",
605+
url: "https://x",
606+
snapshot: "ok",
607+
},
608+
files: [],
609+
},
610+
});
611+
const tool = createBrowserTool();
612+
await tool.execute?.("call-1", {
613+
action: "snapshot",
614+
target: "node",
615+
profile: "user",
616+
snapshotFormat: "ai",
617+
timeoutMs: 7777,
618+
});
619+
620+
expect(gatewayMocks.callGatewayTool).toHaveBeenCalledWith(
621+
"node.invoke",
622+
// proxy adds a 5_000 ms slack on top of the per-request timeout.
623+
expect.objectContaining({ timeoutMs: 7777 + 5_000 }),
624+
expect.objectContaining({
625+
command: "browser.proxy",
626+
params: expect.objectContaining({
627+
method: "GET",
628+
path: "/snapshot",
629+
profile: "user",
630+
query: expect.objectContaining({ timeoutMs: 7777 }),
631+
timeoutMs: 7777,
632+
}),
633+
}),
634+
);
635+
});
636+
557637
it("uses config snapshot defaults when mode is not provided", async () => {
558638
configMocks.loadConfig.mockReturnValue({
559639
browser: { snapshotDefaults: { mode: "efficient" } },

‎extensions/browser/src/browser/chrome-mcp.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
resetChromeMcpSessionsForTest,
1414
setChromeMcpSessionFactoryForTest,
1515
takeChromeMcpScreenshot,
16+
takeChromeMcpSnapshot,
1617
} from "./chrome-mcp.js";
1718

1819
type ToolCall = {
@@ -856,6 +857,27 @@ describe("chrome MCP page parsing", () => {
856857
expect(tabs).toHaveLength(2);
857858
});
858859

860+
it("forwards an explicit timeoutMs to take_snapshot via the callTool race", async () => {
861+
vi.useFakeTimers();
862+
const session = createFakeSession();
863+
session.client.callTool = vi.fn(
864+
async () => new Promise<never>(() => {}),
865+
) as typeof session.client.callTool;
866+
setChromeMcpSessionFactoryForTest(async () => session);
867+
868+
const snapshotPromise = takeChromeMcpSnapshot({
869+
profileName: "chrome-live",
870+
targetId: "1",
871+
timeoutMs: 75,
872+
});
873+
void snapshotPromise.catch(() => {});
874+
875+
await vi.advanceTimersByTimeAsync(75);
876+
877+
await expect(snapshotPromise).rejects.toThrow(/Chrome MCP "take_snapshot".*timed out/);
878+
vi.useRealTimers();
879+
});
880+
859881
it("honors timeoutMs for ephemeral availability probes", async () => {
860882
vi.useFakeTimers();
861883
const closeMock = vi.fn().mockResolvedValue(undefined);

‎extensions/browser/src/browser/chrome-mcp.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,6 +1019,7 @@ export async function takeChromeMcpSnapshot(params: {
10191019
profile?: ChromeMcpProfileOptions;
10201020
userDataDir?: string;
10211021
targetId: string;
1022+
timeoutMs?: number;
10221023
}): Promise<ChromeMcpSnapshotNode> {
10231024
const result = await callTool(
10241025
params.profileName,
@@ -1027,6 +1028,7 @@ export async function takeChromeMcpSnapshot(params: {
10271028
{
10281029
pageId: parsePageId(params.targetId),
10291030
},
1031+
{ timeoutMs: params.timeoutMs },
10301032
);
10311033
return extractSnapshot(result);
10321034
}

‎extensions/browser/src/browser/client.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,33 @@ describe("browser client", () => {
112112
expect(parsed.searchParams.get("refs")).toBe("aria");
113113
});
114114

115+
it("forwards an explicit snapshot timeoutMs into the query string", async () => {
116+
const calls: string[] = [];
117+
stubSnapshotFetch(calls);
118+
119+
await browserSnapshot("http://127.0.0.1:18791", {
120+
format: "ai",
121+
timeoutMs: 4321,
122+
});
123+
124+
const snapshotCall = calls.find((url) => url.includes("/snapshot?"));
125+
expect(snapshotCall).toBeTruthy();
126+
const parsed = new URL(snapshotCall as string);
127+
expect(parsed.searchParams.get("timeoutMs")).toBe("4321");
128+
});
129+
130+
it("falls back to the default snapshot timeout when none is supplied", async () => {
131+
const calls: string[] = [];
132+
stubSnapshotFetch(calls);
133+
134+
await browserSnapshot("http://127.0.0.1:18791", { format: "ai" });
135+
136+
const snapshotCall = calls.find((url) => url.includes("/snapshot?"));
137+
expect(snapshotCall).toBeTruthy();
138+
const parsed = new URL(snapshotCall as string);
139+
expect(parsed.searchParams.get("timeoutMs")).toBe("20000");
140+
});
141+
115142
it("omits format when the caller wants server-side snapshot capability defaults", async () => {
116143
const calls: string[] = [];
117144
stubSnapshotFetch(calls);

‎extensions/browser/src/browser/client.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type {
66
BrowserTransport,
77
SnapshotAriaNode,
88
} from "./client.types.js";
9+
import { DEFAULT_BROWSER_SNAPSHOT_TIMEOUT_MS } from "./constants.js";
910
import type { BrowserDoctorReport } from "./doctor.js";
1011

1112
export type { BrowserStatus, BrowserTab, BrowserTransport } from "./client.types.js";
@@ -320,6 +321,7 @@ export async function browserSnapshot(
320321
urls?: boolean;
321322
mode?: "efficient";
322323
profile?: string;
324+
timeoutMs?: number;
323325
},
324326
): Promise<SnapshotResult> {
325327
const q = new URLSearchParams();
@@ -365,8 +367,13 @@ export async function browserSnapshot(
365367
if (opts.profile) {
366368
q.set("profile", opts.profile);
367369
}
370+
const resolvedTimeoutMs =
371+
typeof opts.timeoutMs === "number" && Number.isFinite(opts.timeoutMs) && opts.timeoutMs > 0
372+
? Math.floor(opts.timeoutMs)
373+
: DEFAULT_BROWSER_SNAPSHOT_TIMEOUT_MS;
374+
q.set("timeoutMs", String(resolvedTimeoutMs));
368375
return await fetchBrowserJson<SnapshotResult>(withBaseUrl(baseUrl, `/snapshot?${q.toString()}`), {
369-
timeoutMs: 20000,
376+
timeoutMs: resolvedTimeoutMs,
370377
});
371378
}
372379

‎extensions/browser/src/browser/constants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export const DEFAULT_BROWSER_ACTION_TIMEOUT_MS = 60_000;
77
export const DEFAULT_BROWSER_LOCAL_LAUNCH_TIMEOUT_MS = 15_000;
88
export const DEFAULT_BROWSER_LOCAL_CDP_READY_TIMEOUT_MS = 8_000;
99
export const DEFAULT_BROWSER_SCREENSHOT_TIMEOUT_MS = 20_000;
10+
export const DEFAULT_BROWSER_SNAPSHOT_TIMEOUT_MS = 20_000;
1011
export const DEFAULT_BROWSER_TAB_CLEANUP_IDLE_MINUTES = 120;
1112
export const DEFAULT_BROWSER_TAB_CLEANUP_MAX_TABS_PER_SESSION = 8;
1213
export const DEFAULT_BROWSER_TAB_CLEANUP_SWEEP_MINUTES = 5;

‎extensions/browser/src/browser/pw-tools-core.snapshot.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,45 @@ describe("pw-tools-core aria snapshot storage", () => {
9292
});
9393
});
9494

95+
it("races snapshotAriaViaPlaywright against an explicit timeoutMs", async () => {
96+
vi.useFakeTimers();
97+
try {
98+
const page = { id: "page-1" };
99+
getPageForTargetId.mockResolvedValue(page);
100+
withPageScopedCdpClient.mockImplementation(() => new Promise(() => {}));
101+
102+
const mod = await import("./pw-tools-core.snapshot.js");
103+
const promise = mod.snapshotAriaViaPlaywright({
104+
cdpUrl: "http://127.0.0.1:9222",
105+
targetId: "tab-1",
106+
timeoutMs: 750,
107+
});
108+
void promise.catch(() => {});
109+
110+
await vi.advanceTimersByTimeAsync(750);
111+
112+
await expect(promise).rejects.toThrow(/Aria snapshot via Playwright timed out/);
113+
} finally {
114+
vi.useRealTimers();
115+
}
116+
});
117+
118+
it("forwards an explicit timeoutMs into the role-aria Playwright ariaSnapshot call", async () => {
119+
const ariaSnapshotMock = vi.fn().mockResolvedValue("");
120+
const page = { ariaSnapshot: ariaSnapshotMock };
121+
getPageForTargetId.mockResolvedValue(page);
122+
123+
const mod = await import("./pw-tools-core.snapshot.js");
124+
await mod.snapshotRoleViaPlaywright({
125+
cdpUrl: "http://127.0.0.1:9222",
126+
targetId: "tab-1",
127+
refsMode: "aria",
128+
timeoutMs: 8888,
129+
});
130+
131+
expect(ariaSnapshotMock).toHaveBeenCalledWith({ mode: "ai", timeout: 8888 });
132+
});
133+
95134
it("stores role fallback metadata when backend markers are unavailable", async () => {
96135
const page = { id: "page-1" };
97136
const mod = await import("./pw-tools-core.snapshot.js");

‎extensions/browser/src/browser/pw-tools-core.snapshot.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ export async function snapshotAriaViaPlaywright(opts: {
140140
cdpUrl: string;
141141
targetId?: string;
142142
limit?: number;
143+
timeoutMs?: number;
143144
ssrfPolicy?: SsrFPolicy;
144145
}): Promise<{ nodes: AriaSnapshotNode[] }> {
145146
const limit = Math.max(1, Math.min(2000, Math.floor(opts.limit ?? 500)));
@@ -157,7 +158,11 @@ export async function snapshotAriaViaPlaywright(opts: {
157158
targetId: opts.targetId,
158159
});
159160
}
160-
const res = (await withPageScopedCdpClient({
161+
const ariaTimeoutMs =
162+
typeof opts.timeoutMs === "number" && Number.isFinite(opts.timeoutMs) && opts.timeoutMs > 0
163+
? Math.max(500, Math.min(60_000, Math.floor(opts.timeoutMs)))
164+
: undefined;
165+
const collectAxTree = withPageScopedCdpClient({
161166
cdpUrl: opts.cdpUrl,
162167
page,
163168
targetId: opts.targetId,
@@ -167,7 +172,23 @@ export async function snapshotAriaViaPlaywright(opts: {
167172
nodes?: RawAXNode[];
168173
};
169174
},
170-
})) as {
175+
});
176+
const res = (await (ariaTimeoutMs === undefined
177+
? collectAxTree
178+
: (() => {
179+
let timer: ReturnType<typeof setTimeout> | undefined;
180+
const timeout = new Promise<never>((_, reject) => {
181+
timer = setTimeout(() => {
182+
reject(new Error(`Aria snapshot via Playwright timed out after ${ariaTimeoutMs}ms.`));
183+
}, ariaTimeoutMs);
184+
timer.unref?.();
185+
});
186+
return Promise.race([collectAxTree, timeout]).finally(() => {
187+
if (timer) {
188+
clearTimeout(timer);
189+
}
190+
});
191+
})())) as {
171192
nodes?: RawAXNode[];
172193
};
173194
const nodes = Array.isArray(res?.nodes) ? res.nodes : [];
@@ -241,6 +262,7 @@ export async function snapshotRoleViaPlaywright(opts: {
241262
refsMode?: "role" | "aria";
242263
options?: RoleSnapshotOptions;
243264
urls?: boolean;
265+
timeoutMs?: number;
244266
ssrfPolicy?: SsrFPolicy;
245267
}): Promise<{
246268
snapshot: string;
@@ -262,13 +284,15 @@ export async function snapshotRoleViaPlaywright(opts: {
262284
});
263285
}
264286

287+
const ariaSnapshotTimeout = Math.max(500, Math.min(60_000, Math.floor(opts.timeoutMs ?? 5000)));
288+
265289
if (opts.refsMode === "aria") {
266290
if (normalizeOptionalString(opts.selector) || normalizeOptionalString(opts.frameSelector)) {
267291
throw new Error("refs=aria does not support selector/frame snapshots yet.");
268292
}
269293
const snapshot = await page.ariaSnapshot({
270294
mode: "ai",
271-
timeout: 5000,
295+
timeout: ariaSnapshotTimeout,
272296
});
273297
const built = buildRoleSnapshotFromAiSnapshot(snapshot, opts.options);
274298
const snapshotWithUrls = opts.urls
@@ -298,7 +322,7 @@ export async function snapshotRoleViaPlaywright(opts: {
298322
? page.locator(selector)
299323
: page.locator(":root");
300324

301-
const ariaSnapshot = await locator.ariaSnapshot();
325+
const ariaSnapshot = await locator.ariaSnapshot({ timeout: ariaSnapshotTimeout });
302326
const built = buildRoleSnapshotFromAriaSnapshot(ariaSnapshot ?? "", opts.options);
303327
const snapshotWithUrls = opts.urls
304328
? appendSnapshotUrls(built.snapshot, await collectSnapshotUrls(page))

0 commit comments

Comments
 (0)