From 8f3eb0f7b4675b1e9cee946b82dfd4447d45fa92 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 2 Mar 2026 15:10:28 -0800 Subject: [PATCH] fix(browser): use CDP command probe for cdpReady health (#31421) * fix(browser): validate cdp command channel health * test(browser): cover stale cdp command channel readiness * changelog: note cdp command-channel readiness check * browser(cdp): detach ws message listener on health-probe cleanup --- CHANGELOG.md | 1 + src/browser/chrome.test.ts | 106 +++++++++++++++++++++++++++++++++++++ src/browser/chrome.ts | 59 +++++++++++++++++---- 3 files changed, 157 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b2b5da4d..50b04802a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,6 +107,7 @@ Docs: https://docs.openclaw.ai - Browser/Act request compatibility: accept legacy flattened `action="act"` params (`kind/ref/text/...`) in addition to `request={...}` so browser act calls no longer fail with `request required`. (#15120) Thanks @vincentkoc. - Browser/Extension relay stale tabs: evict stale cached targets from `/json/list` when extension targets are destroyed/crashed or commands fail with missing target/session errors. (#6175) Thanks @vincentkoc. - CLI/Browser start timeout: honor `openclaw browser --timeout start` and stop by removing the fixed 15000ms override so slower Chrome startups can use caller-provided timeouts. (#22412, #23427) Thanks @vincentkoc. +- Browser/CDP status accuracy: require a successful `Browser.getVersion` response over the CDP websocket (not just socket-open) before reporting `cdpReady`, so stale idle command channels are surfaced as unhealthy. (#23427) Thanks @vincentkoc. - Browser/CDP startup diagnostics: include Chrome stderr output and a Linux no-sandbox hint in startup timeout errors so failed launches are easier to diagnose. (#29312) Thanks @veast. - Browser/CDP startup readiness: wait for CDP websocket readiness after launching Chrome and cleanly stop/reset when readiness never arrives, reducing follow-up `PortInUseError` races after `browser start`/`open`. (#29538) Thanks @AaronWander. - Browser/Managed tab cap: limit loopback managed `openclaw` page tabs to 8 via best-effort cleanup after tab opens to reduce long-running renderer buildup while preserving attach-only and remote profile behavior. (#29724) Thanks @pandego. diff --git a/src/browser/chrome.test.ts b/src/browser/chrome.test.ts index 84839e98c..d5a6bc5b1 100644 --- a/src/browser/chrome.test.ts +++ b/src/browser/chrome.test.ts @@ -1,13 +1,17 @@ import fs from "node:fs"; import fsp from "node:fs/promises"; +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; import os from "node:os"; import path from "node:path"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { WebSocketServer } from "ws"; import { decorateOpenClawProfile, ensureProfileCleanExit, findChromeExecutableMac, findChromeExecutableWindows, + isChromeCdpReady, isChromeReachable, resolveBrowserExecutableForPlatform, stopOpenClawChrome, @@ -243,6 +247,108 @@ describe("browser chrome helpers", () => { await expect(isChromeReachable("http://127.0.0.1:12345", 50)).resolves.toBe(false); }); + it("reports cdpReady only when Browser.getVersion command succeeds", async () => { + const server = createServer((req, res) => { + if (req.url === "/json/version") { + const addr = server.address() as AddressInfo; + res.writeHead(200, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + webSocketDebuggerUrl: `ws://127.0.0.1:${addr.port}/devtools/browser/health`, + }), + ); + return; + } + res.writeHead(404); + res.end(); + }); + const wss = new WebSocketServer({ noServer: true }); + server.on("upgrade", (req, socket, head) => { + if (req.url !== "/devtools/browser/health") { + socket.destroy(); + return; + } + wss.handleUpgrade(req, socket, head, (ws) => { + wss.emit("connection", ws, req); + }); + }); + wss.on("connection", (ws) => { + ws.on("message", (raw) => { + let message: { id?: unknown; method?: unknown } | null = null; + try { + const text = + typeof raw === "string" + ? raw + : Buffer.isBuffer(raw) + ? raw.toString("utf8") + : Array.isArray(raw) + ? Buffer.concat(raw).toString("utf8") + : Buffer.from(raw).toString("utf8"); + message = JSON.parse(text) as { id?: unknown; method?: unknown }; + } catch { + return; + } + if (message?.method === "Browser.getVersion" && message.id === 1) { + ws.send( + JSON.stringify({ + id: 1, + result: { product: "Chrome/Mock" }, + }), + ); + } + }); + }); + + await new Promise((resolve, reject) => { + server.listen(0, "127.0.0.1", () => resolve()); + server.once("error", reject); + }); + const addr = server.address() as AddressInfo; + await expect(isChromeCdpReady(`http://127.0.0.1:${addr.port}`, 300, 400)).resolves.toBe(true); + + await new Promise((resolve) => wss.close(() => resolve())); + await new Promise((resolve) => server.close(() => resolve())); + }); + + it("reports cdpReady false when websocket opens but command channel is stale", async () => { + const server = createServer((req, res) => { + if (req.url === "/json/version") { + const addr = server.address() as AddressInfo; + res.writeHead(200, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + webSocketDebuggerUrl: `ws://127.0.0.1:${addr.port}/devtools/browser/stale`, + }), + ); + return; + } + res.writeHead(404); + res.end(); + }); + const wss = new WebSocketServer({ noServer: true }); + server.on("upgrade", (req, socket, head) => { + if (req.url !== "/devtools/browser/stale") { + socket.destroy(); + return; + } + wss.handleUpgrade(req, socket, head, (ws) => { + wss.emit("connection", ws, req); + }); + }); + // Simulate a stale command channel: WS opens but never responds to commands. + wss.on("connection", (_ws) => {}); + + await new Promise((resolve, reject) => { + server.listen(0, "127.0.0.1", () => resolve()); + server.once("error", reject); + }); + const addr = server.address() as AddressInfo; + await expect(isChromeCdpReady(`http://127.0.0.1:${addr.port}`, 300, 150)).resolves.toBe(false); + + await new Promise((resolve) => wss.close(() => resolve())); + await new Promise((resolve) => server.close(() => resolve())); + }); + it("stopOpenClawChrome no-ops when process is already killed", async () => { const proc = makeProc({ killed: true }); await stopOpenClawChrome( diff --git a/src/browser/chrome.ts b/src/browser/chrome.ts index ab21fd6f0..ccc820f47 100644 --- a/src/browser/chrome.ts +++ b/src/browser/chrome.ts @@ -3,6 +3,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { ensurePortAvailable } from "../infra/ports.js"; +import { rawDataToString } from "../infra/ws.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { CONFIG_DIR } from "../utils.js"; import { @@ -124,7 +125,7 @@ export async function getChromeWebSocketUrl( return normalizeCdpWsUrl(wsUrl, cdpUrl); } -async function canOpenWebSocket( +async function canRunCdpHealthCommand( wsUrl: string, timeoutMs = CHROME_WS_READY_TIMEOUT_MS, ): Promise { @@ -132,6 +133,37 @@ async function canOpenWebSocket( const ws = openCdpWebSocket(wsUrl, { handshakeTimeoutMs: timeoutMs, }); + let settled = false; + const onMessage = (raw: WebSocket.RawData) => { + if (settled) { + return; + } + let parsed: { id?: unknown; result?: unknown } | null = null; + try { + parsed = JSON.parse(rawDataToString(raw)) as { id?: unknown; result?: unknown }; + } catch { + return; + } + if (parsed?.id !== 1) { + return; + } + finish(Boolean(parsed.result && typeof parsed.result === "object")); + }; + + const finish = (value: boolean) => { + if (settled) { + return; + } + settled = true; + clearTimeout(timer); + ws.off("message", onMessage); + try { + ws.close(); + } catch { + // ignore + } + resolve(value); + }; const timer = setTimeout( () => { try { @@ -139,22 +171,31 @@ async function canOpenWebSocket( } catch { // ignore } - resolve(false); + finish(false); }, Math.max(50, timeoutMs + 25), ); + ws.once("open", () => { - clearTimeout(timer); try { - ws.close(); + ws.send( + JSON.stringify({ + id: 1, + method: "Browser.getVersion", + }), + ); } catch { - // ignore + finish(false); } - resolve(true); }); + + ws.on("message", onMessage); + ws.once("error", () => { - clearTimeout(timer); - resolve(false); + finish(false); + }); + ws.once("close", () => { + finish(false); }); }); } @@ -168,7 +209,7 @@ export async function isChromeCdpReady( if (!wsUrl) { return false; } - return await canOpenWebSocket(wsUrl, handshakeTimeoutMs); + return await canRunCdpHealthCommand(wsUrl, handshakeTimeoutMs); } export async function launchOpenClawChrome(